refactor: Use named constant for leaf item size (#39) - #7130
Merged
Conversation
## High Level Overview of Change Tightens input validation in SHAMap leaf-node construction and unifies short-node error messages across makeTransaction, makeTransactionWithMeta, and makeAccountState. ## Context of Change The SHAMapLeafNode constructor assumed a minimum payload size via XRPL_ASSERT, which is a no-op in release builds and leaves no guard against undersized input at the wire-parsing boundary. This PR: - Adds an explicit size check at each wire-parsing factory (makeTransaction, makeTransactionWithMeta, makeAccountState) so undersized payloads are rejected before leaf-node construction, in both debug and release builds. - Replaces the hardcoded 12 with a named constant (minSHAMapItemBytes) so the threshold is visible and consistent across the three entry points. - Normalizes the existing short-node error messages to include actual and expected sizes, so logs and debugging are uniform. <!-- Please include the context of a change. If a bug fix, when was the bug introduced? What was the behavior? If a new feature, why was this architecture chosen? What were the alternatives? If a refactor, how is this better than the previous implementation? If there is a spec or design document for this feature, please link it here. --> ### API Impact <!-- Please check [x] relevant options, delete irrelevant ones. * If there is any impact to the public API methods (HTTP / WebSocket), please update https://github.com/xrplf/rippled/blob/develop/API-CHANGELOG.md * Update API-CHANGELOG.md and add the change directly in this PR by pushing to your PR branch. * libxrpl: See https://github.com/XRPLF/rippled/blob/develop/docs/build/depend.md * Peer Protocol: See https://xrpl.org/peer-protocol.html --> - [ ] Public API: New feature (new methods and/or new fields) - [ ] Public API: Breaking change (in general, breaking changes should only impact the next api_version) - [ ] `libxrpl` change (any change that may affect `libxrpl` or dependents of `libxrpl`) - [ ] Peer protocol change (must be backward compatible or bump the peer protocol version) <!-- ## Before / After If relevant, use this section for an English description of the change at a technical level. If this change affects an API, examples should be included here. For performance-impacting changes, please provide these details: 1. Is this a new feature, bug fix, or improvement to existing functionality? 2. What behavior/functionality does the change impact? 3. 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. 4. Does this change affect concurrent processing - e.g. does it involve acquiring locks, multi-threaded processing, or async processing? --> <!-- ## Test Plan If helpful, please describe the tests that you ran to verify your changes and provide instructions so that others can reproduce. This section may not be needed if your change includes thoroughly commented unit tests. --> <!-- ## Future Tasks For future tasks related to PR. -->
![xrplf-ai-reviewer[bot]](https://avatars.githubusercontent.com/u/67929741?s=60&v=4){kind=small}
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #7130 +/- ##
=========================================
- Coverage 82.1% 82.1% -0.0%
=========================================
Files 1010 1010
Lines 76145 76164 +19
Branches 7376 7387 +11
=========================================
+ Hits 62548 62551 +3
- Misses 13597 13613 +16
🚀 New features to boost your workflow:
|
ximinez
approved these changes
May 12, 2026
pratikmankawde
left a comment
pratikmankawde
left a comment
Contributor
There was a problem hiding this comment.
Left a minor comment
Contributor
Author
vlntb
added
the
Ready to merge
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
May 13, 2026
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
4 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
Tightens input validation in SHAMap leaf-node construction and unifies short-node error messages across makeTransaction, makeTransactionWithMeta, and makeAccountState.
Context of Change
The SHAMapLeafNode constructor assumed a minimum payload size via XRPL_ASSERT, which is a no-op in release builds and leaves no guard against undersized input at the wire-parsing boundary. This PR:
API Impact
libxrplchange (any change that may affectlibxrplor dependents oflibxrpl)