feat: add more fields to server_definitions#5702
feat: add more fields to server_definitions#5702mvadari wants to merge 43 commits intoXRPLF:developfrom
server_definitions#5702Conversation
mvadari
added
the
DraftRunCI
mvadari
force-pushed
the
server_definitions2
branch
from
0e8fb16 to
fa0d5a6
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #5702 +/- ##
=======================================
Coverage 79.9% 79.9%
=======================================
Files 842 842
Lines 65538 65597 +59
Branches 7253 7250 -3
=======================================
+ Hits 52380 52443 +63
+ Misses 13158 13154 -4
... and 1 file with indirect coverage changes 🚀 New features to boost your workflow:
|
mvadari
removed
the
DraftRunCI
bthomee
added
the
DraftRunCI
There was a problem hiding this comment.
Pull request overview
This PR significantly expands the server_definitions RPC endpoint to provide comprehensive protocol definition information. The main change is adding transaction formats, ledger entry formats, transaction flags, ledger entry flags, and AccountSet flags to the RPC response. To support this, the code refactors flag definitions using X-macros for better maintainability and to enable programmatic generation of flag maps for the RPC output.
Key changes:
- Adds new fields to
server_definitionsRPC:TRANSACTION_FORMATS,LEDGER_ENTRY_FORMATS,TRANSACTION_FLAGS,LEDGER_ENTRY_FLAGS, andACCOUNT_SET_FLAGS - Refactors
TxFlags.handLedgerFormats.hto use X-macros for defining flags, generating masks, and creating flag maps - Updates flag mask names across transaction handlers for consistency (e.g.,
tfDepositMask→tfAMMDepositMask,tfBridgeModifyMask→tfXChainModifyBridgeMask)
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/xrpld/rpc/handlers/ServerDefinitions.cpp | Populates new RPC fields with transaction formats, ledger entry formats, and flag definitions |
| include/xrpl/protocol/TxFlags.h | Major refactoring to use X-macros for flag definitions, mask generation, and flag map creation |
| include/xrpl/protocol/LedgerFormats.h | Refactors ledger flags using X-macros and adds getCommonFields() method |
| include/xrpl/protocol/TxFormats.h | Adds getCommonFields() method declaration |
| src/libxrpl/protocol/TxFormats.cpp | Extracts common transaction fields into getCommonFields() method |
| src/libxrpl/protocol/LedgerFormats.cpp | Extracts common ledger entry fields into getCommonFields() method |
| include/xrpl/protocol/jss.h | Adds JSON field name constants for new RPC response fields |
| src/xrpld/app/tx/detail/*.cpp | Updates flag mask references to use new standardized names |
| src/test/rpc/ServerDefinitions_test.cpp | Contains debug output that should be removed |
| src/test/app/AccountSet_test.cpp | Fixes test suite category from rpc to app |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| { | ||
| Env env(*this); | ||
| auto const result = env.rpc("server_definitions"); | ||
| std::cout << result.toStyledString() << std::endl; |
There was a problem hiding this comment.
Debug output statement (std::cout) should be removed before merging. This appears to be leftover from development/testing.
| std::cout << result.toStyledString() << std::endl; |
| TRANSACTION(Batch, \ | ||
| TF_FLAG(tfAllOrNothing, 0x00010000) \ | ||
| TF_FLAG(tfOnlyOne, 0x00020000) \ | ||
| TF_FLAG(tfUntilFailure, 0x00040000) \ | ||
| TF_FLAG(tfIndependent, 0x00080000)) \ |
There was a problem hiding this comment.
The old tfBatchMask definition included special handling: ~(tfUniversal | ...) | tfInnerBatchTxn which added back the tfInnerBatchTxn flag to make it explicitly allowed. The macro-generated version will produce ~(tfUniversal | tfAllOrNothing | tfOnlyOne | tfUntilFailure | tfIndependent) which inherently disallows tfInnerBatchTxn since tfInnerBatchTxn is part of tfUniversal. This changes the behavior - the old code explicitly allowed tfInnerBatchTxn for Batch transactions, but the new macro-generated mask will disallow it. The original code had a comment indicating this was intentional to prevent nested Batch transactions. Please verify this behavior change is intended.
| TF_FLAG(tfVaultPrivate, lsfVaultPrivate) \ | ||
| TF_FLAG(tfVaultShareNonTransferable, 0x00020000)) \ |
There was a problem hiding this comment.
The old code included static_assert(tfVaultPrivate == lsfVaultPrivate); to ensure these flags maintain the same value. This compile-time check has been removed in the new macro-based implementation. While tfVaultPrivate is defined as lsfVaultPrivate (line 156), removing the static assertion means there's no compile-time verification if lsfVaultPrivate changes. Consider adding this assertion back after the macro expansions to maintain this safety check.
mvadari
force-pushed
the
server_definitions2
branch
from
390185c to
571950d
Compare
mvadari
added
the
Blocked: Needs Draft XLS
This change adds the project configuration directory to `.gitignore` for the `zed` editor. As per the [documentation](https://zed.dev/docs/remote-development?highlight=.zed#zed-settings), the project configuration files are stored in the `.zed` directory at the project root dir.
This change introduces the `fixExpiredNFTokenOfferRemoval` amendment that allows expired offers to pass through `preclaim()` and be deleted in `doApply()`, following the same pattern used for expired credentials.
Currently we're passing the `Application` object around, whereby the `Application` class acts more like a service registry that gives other classes access to other services. In order to allow modularization, we should replace `Application` with a service registry class so that modules depending on `Application` for other services can be moved easily. This change adds the `ServiceRegistry` class.
Unity builds were intended to speed up builds, by bundling multiple files into compilation units. However, now that ccache is available on all platforms, there is no need for unity builds anymore, as ccache stores compiled individual build objects for reuse. This change therefore removes the ability to make unity builds.
This change replaces all include guards in the `src/` and `include/` directories by `#pragma once`.
|
Closing in favor of #6321 |
High Level Overview of Change
This PR greatly expands what is included in the
server_definitionsRPC. The following are now added:AccountSetflags (asfflags)Context of Change
Alternative to #5616
Closes #5151
Type of Change
.gitignore, formatting, dropping support for older tooling)API Impact
libxrplchange (any change that may affectlibxrplor dependents oflibxrpl)Test Plan
TODO, waiting for feedback on the format before putting in that effort