Additions to the server_definitions RPC response#5616
Closed
ckeshava wants to merge 29 commits intoXRPLF:developfrom
Closed
Additions to the server_definitions RPC response#5616ckeshava wants to merge 29 commits intoXRPLF:developfrom
ckeshava wants to merge 29 commits intoXRPLF:developfrom
Conversation
Update server_defintions RPC command to return LedgerEntry format for all the published LedgerObjects.
The RPC response contains the following structure:
result: {
LEDGER_ENTRIES: {
"AMM": {
"hexCode": "0xAA",
"sfields": [
{"sfield_Name": "Account", "optionality": "REQUIRED"},
{"sfield_Name": "Date", "optionality": "DEFAULT"},
// other SFields
]
},
// other LedgerEntry objects ...
},
// other sections of the server_definitions RPC remain as is
}
TODO: Use relative file-path for ledger_entries.macro, instead of the absolute filepath
…s RPC call
The flags associated with all the ledger entries are recorded in the following format:
result: {
"LEDGER_ENTRY_FLAGS": {
"lsfFlag1": "0xABCD1234",
"lsfFlag2": "0x1234ABCD",
// all the other ledger-entry flags
}
}
This data is parsed from the file located at: include/xrpl/protocol/LedgerFormats.h
…ide the server_definitions RPC response.
All the transactions supported by the rippled node are parsed from the file located at: include/xrpl/protocol/detail/transactions.macro
THe following details are recorded about each transaction:
(the below values are only for illustrative purposes. Please consult the latest docs for accurate values)
result: {
TRANSACTION_FORMAT: {
"OracleSet": {
"hexCode": "0xabcd",
"delegatability": "notDelegatable",
"sfields": [
{"sfieldName": "LastUpdateTime", "optionality": "REQUIRED", "isMPTSuppported": "MPTSuppported"},
{"sfieldName": "DocumentID", "optionality": "OPTIONAL"},
// other sfields relevant to the transaction ...
],
},
// similar details for the other 65 transactions currently supported in XRPL ...
},
// other sections of the server_definitions RPC response...
}
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #5616 +/- ##
=========================================
- Coverage 79.4% 79.4% -0.0%
=========================================
Files 839 839
Lines 71625 71694 +69
Branches 8241 8260 +19
=========================================
+ Hits 56864 56896 +32
- Misses 14761 14798 +37
... and 9 files with indirect coverage changes 🚀 New features to boost your workflow:
|
Collaborator
|
This feature is currently undergoing review and analysis on how it should be structured, and this PR shouldn't be merged until that is resolved. |
mvadari
requested changes
Jul 25, 2025
| parseLedgerEntryFlags() | ||
| { | ||
| Json::Value solution = Json::objectValue; | ||
| std::string const ledgerFormatFile{ |
Collaborator
There was a problem hiding this comment.
Source code regex won't work on production systems. It actually doesn't even work on my machine, because my build directory isn't in my source directory -
% ./rippled server_definitions
Loading: "../rippled.cfg"
2025-Jul-25 21:07:08.911682 UTC HTTPClient:NFO Connecting to 127.0.0.1:5005
{
"result" : {
"error" : "internal",
"error_code" : 73,
"error_message" : "Internal error.",
"request" : {
"api_version" : 1,
"command" : "server_definitions"
},
"status" : "error"
}Also, none of the tests pass.
% ./rippled --unittest ripple.app.ServerInfo
ripple.app.ServerInfo server_info
ripple.app.ServerInfo server_definitions
...
failed: ripple.app.ServerInfo had 155 failures.
Longest suite times:
2.5s ripple.app.ServerInfo
2.5s, 1 suite, 2 cases, 196 tests total, 155 failures
use LedgerFormats() singleton object to read ledger-entry-formats, instead of the regex approach removed parseLedgerEntries regex solution
13 tasks
… std::unordered_map simultaneously. Update the tests to validate the new structure of the LedgerSpecificFlags in server definitions
TODO: Differentiate the tf from asf in the output of the RPC command.
…C response TODO: Check if this implementation holds good from a modularity perspective. Is it alright to #include libxrpl file inside xrpld folder?
…or on some CI configurations
…to newServerDef
…to newServerDef
13 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
High Level Overview of Change
This PR provides the
LedgerEntry-format,Transaction-format andLedgerEntryFlagsin the response of theserver_definitionsRPC. I have explained the response format in these commit messages. If your use-case requires additional fields / modified-response-layout, let me know. For the sake of brevity, I chose to not include them here.I'd like to address this issue in this PR.
The PR uses relative file-paths to parse the
transactions.macro,ledger_entries.macroandLedgerFormats.hfiles. Definitions in these files are captured using regex to construct a structured output in theserver_definitionsRPC.Type of Change
Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Refactor (non-breaking change that only restructures code)
Performance (increase or change in throughput and/or latency)
I have not executed any performance tests. The regexes used in this PR are not complex, hence I do not expect significant perf impact. However, if there is an existing baseline for the
server_definitionsRPC, I'm happy to test against it. The pertinent methods will beparse<TransactionFormat|LedgerEntries|LEFlags>in this file.Tests (you added tests for code that already exists, or your new feature included in this PR)
Appropriate tests have been added.
Documentation update
Chore (no impact to binary, e.g.
.gitignore, formatting, dropping support for older tooling)Release
API Impact
libxrplchange (any change that may affectlibxrplor dependents oflibxrpl)For performance-impacting changes, please provide these details:
Is this a new feature, bug fix, or improvement to existing functionality?
New feature
What behavior/functionality does the change impact?
This change impacts the
server_definitionsRPC.In what processing can the impact be measured? Be as specific as possible - e.g. RPC client call, payment transaction that involves LOB, AMM, caching, DB operations, etc.
One would measure the parameters of the system during the execution of the above-mentioned RPC. I expect minor changes in latency of RPC execution, memory consumed by the process to satisfy this RPC (for regex evaluation).
Does this change affect concurrent processing - e.g. does it involve acquiring locks, multi-threaded processing, or async processing?
No
I am not accepting any untrusted user input into the evaluation of regex. I do not expect any unwarranted Regex DoS circumstances due to this PR.
Future Tasks
server_definitionsresponse. This reduces the repeated resource-consumption that occurs with every RPC call. The existence ofhashfield in the response makes this easier.(The use of the
statickeyword in the affected functions instructs the cpp compiler to cache the results of the method. The benefits of an explicit cache might need to be investigated)Notes:
Thank you @mvadari for suggesting the approach which uses runtime cpp objects, instead of the regex solution. I based this work off of her initial branch here.
This PR does not provide support for Transaction-Flags. I wanted to keep the change-log as small as possible. If this feature is essential for your work, let me know. I can work on a different? PR.