HopType.toJson() sets issuer instead of account#799
Open
cybele-ripple wants to merge 6 commits into
Open
Conversation
…#756) In toJson(), when the TYPE_ISSUER flag was set the code called builder.account() instead of builder.issuer(). This overwrote the account field with the issuer address and left the issuer field unset, silently corrupting round-tripped payment path data. The fix changes the one incorrect call to builder.issuer(). A new HopTypeTest covers both the issuer-preserved and the account-not-overwritten cases.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #799 +/- ##
============================================
+ Coverage 93.51% 93.63% +0.12%
- Complexity 2606 2608 +2
============================================
Files 486 486
Lines 6580 6580
Branches 566 566
============================================
+ Hits 6153 6161 +8
+ Misses 270 259 -11
- Partials 157 160 +3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Collaborator
Author
|
/ai-review |
![xrplf-ai-reviewer[bot]](https://avatars.githubusercontent.com/u/67929741?s=60&v=4){kind=small}
sappenin
previously approved these changes
Jun 12, 2026
* XrpCurrencyAmount.equals() and hashCode() now include isNegative (Issue #757) equals() was only comparing the unsigned magnitude via value(), so +1_000_000 drops and -1_000_000 drops were considered equal. hashCode() had the same defect, breaking HashMap/HashSet keying and deduplication. Fixed equals() to also compare the isNegative() flag, and fixed hashCode() to hash both value() and isNegative() via Objects.hash(). Also corrected a pre-existing test assertion in plusXrp that only passed due to the buggy equals() behavior (negative + negative was compared against a positive expected value). * Restore Immutables null-safety comment in XrpCurrencyAmount.equals() The comment noting that the cast result can't be null due to Immutables was lost when the intermediate variable was renamed in the equals() fix. Restore it on the new variable for clarity. * Fix Math.abs(Long.MIN_VALUE) overflow in XrpCurrencyAmount.ofDrops(long) (#798) * Fix Math.abs(Long.MIN_VALUE) overflow in XrpCurrencyAmount.ofDrops(long) Guard against Long.MIN_VALUE before calling Math.abs(), which overflows back to Long.MIN_VALUE in two's complement arithmetic and produces a corrupted UnsignedLong value. Now throws IllegalArgumentException immediately with a clear message. Adds a regression test. Fixes #755 * Move Long.MIN_VALUE guard inside negative-drops branch Only relevant when drops < 0 (Math.abs overflow), so no need to check on the happy-path. --------- Co-authored-by: David Fuelling <sappenin@gmail.com> --------- Co-authored-by: David Fuelling <sappenin@gmail.com>
Add LockedAmount to BASE_10_UINT64_FIELD_NAMES in UInt64Type (Issue #792) LockedAmount is a UInt64 field used in MPT escrow ledger objects. Because it was missing from BASE_10_UINT64_FIELD_NAMES, the binary codec treated it as base-16 on both encode and decode. This meant a JSON value of "1000" was silently encoded as 0x0000000000001000 (decimal 4096) instead of the correct 0x00000000000003E8 (decimal 1000), corrupting MPT escrow amounts. The fix adds "LockedAmount" to the set alongside MaximumAmount, OutstandingAmount, and MPTAmount, which are the other MPT-related UInt64 fields that share this base-10 convention. Co-authored-by: David Fuelling <sappenin@gmail.com>
sappenin
approved these changes
Jun 15, 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
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.
In
HopType.toJson()inHopType.java, there is a check forTYPE_ISSUER. If present, the account field was filled with the issuer data, instead of the issuer field. This PR fixes this issue.Unit tests were added for
HopType.java(inHopTypeTest.java) to test this change.