feat: add full support for all objects in ledger_entry#6319
feat: add full support for all objects in ledger_entry#6319mvadari wants to merge 15 commits intoXRPLF:developfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds keylet-based lookup support in the ledger_entry RPC for several ledger object types that previously only supported direct hash/index lookup, aligning behavior with the goal of supporting all object types.
Changes:
- Extend
ledger_entryparsing to compute keylets for Checks, NFTokenOffers, PayChannels, and SignerLists from their component fields. - Add/expand RPC tests to cover lookups by both hash and keylet parts, including non-existent entry cases.
- Update malformed-parameter test helpers/mappings to include newly used fields (e.g.,
destination).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/xrpld/rpc/handlers/LedgerEntry.cpp | Adds object-form parsing for check / nft_offer / payment_channel / signer_list to compute the correct keylet-derived ledger index. |
| src/test/rpc/LedgerEntry_test.cpp | Adds positive and negative coverage for the new object-form requests and updates malformed-field testing to match the expanded input types. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #6319 +/- ##
=======================================
Coverage 79.8% 79.8%
=======================================
Files 858 858
Lines 67757 67789 +32
Branches 7557 7557
=======================================
+ Hits 54064 54096 +32
Misses 13693 13693
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| jss::payment_channel, | ||
| { | ||
| {jss::account, "malformedAddress"}, | ||
| {jss::destination, "malformedDestination"}, |
There was a problem hiding this comment.
This test expects the new error code malformedDestination for invalid payment_channel.destination. If the handler switches to an existing error code (e.g. malformedAddress) for consistency with other AccountID fields, adjust this expectation accordingly so the negative tests match the API contract.
| {jss::destination, "malformedDestination"}, | |
| {jss::destination, "malformedAddress"}, |
| return Unexpected(account.error()); | ||
|
|
||
| auto const destination = | ||
| LedgerEntryHelpers::requiredAccountID(params, jss::destination, "malformedDestination"); |
There was a problem hiding this comment.
parsePayChannel introduces a new error code string malformedDestination for an invalid destination AccountID. This error code doesn’t appear to be used anywhere else in the codebase, while other AccountID fields in this handler consistently use malformedAddress (or malformedOwner for owner fields). Consider using an existing error code (e.g. malformedAddress) to avoid expanding the public error surface with a one-off code; update the corresponding unit test expectations if you change it.
| LedgerEntryHelpers::requiredAccountID(params, jss::destination, "malformedDestination"); | |
| LedgerEntryHelpers::requiredAccountID(params, jss::destination, "malformedAddress"); |
High Level Overview of Change
This PR adds full support (including the keylet parts) for checks, NFT offers, payment channels, and signer lists.
Context of Change
Fixes #5198
Type of Change
API Impact
Before / After
Before: checks, NFT offers, payment channels, and signer lists only had basic support, to identify the object from its hash.
After: Those objects can now also be identified from the pieces that form their keylets.
Test Plan
Tests are added and pass.