🚀 Advanced: fix: Enforce aggregate MaximumAmount in multi-send MPT by Tapanito · Pull Request #6644 · XRPLF/rippled · GitHub

Tapanito · 2026-03-25T11:23:28Z

rippleSendMultiMPT used a read-only SLE snapshot (view.read) to check MaximumAmount per iteration. Since rippleCreditMPT updates a separate mutable copy (view.peek), the snapshot's sfOutstandingAmount was stale after the first iteration, allowing the aggregate to exceed MaximumAmount.

Replace the per-iteration check with a running total that validates the aggregate against MaximumAmount within the send loop. The old per-iteration check is retained behind a !fixAssortedFixes gate for ledger replay compatibility.

High Level Overview of Change

Context of Change

API Impact

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)

rippleSendMultiMPT used a read-only SLE snapshot (view.read) to check MaximumAmount per iteration. Since rippleCreditMPT updates a separate mutable copy (view.peek), the snapshot's sfOutstandingAmount was stale after the first iteration, allowing the aggregate to exceed MaximumAmount. Replace the per-iteration check with a running total that validates the aggregate against MaximumAmount within the send loop. The old per-iteration check is retained behind a !fixAssortedFixes gate for ledger replay compatibility.

github-actions · 2026-03-26T00:44:34Z

This PR has conflicts, please resolve them in order for the PR to be reviewed.

…-send-max-amount # Conflicts: # include/xrpl/protocol/detail/features.macro # src/libxrpl/ledger/View.cpp

github-actions · 2026-03-26T09:59:10Z

All conflicts have been resolved. Assigned reviewers can now start or resume their review.

Tapanito · 2026-03-26T10:51:20Z

/ai-review

xrplf-ai-reviewer

Went through the changes

The core fix is correct, but four issues need attention: a uint64_t wrap-around risk in the pre-amendment arithmetic (line 1215), a semantic divergence from the original subtraction-underflow behavior that may break ledger replay fidelity (line 1215), missing // KNOWN BUG comments at the broken-behavior assertion sites in the test (line 3351), and a reminder to queue fixSecurity3_1_3 for prompt activation and audit other multi-send paths for the same stale-snapshot pattern. See inline comments.

Review by ReviewBot 🤖

Review by Claude Opus 4.6 · Prompt: V12

codecov · 2026-03-26T11:15:40Z

Codecov Report

❌ Patch coverage is 94.44444% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 81.4%. Comparing base (b6aa4a8) to head (8e684b9).
⚠️ Report is 27 commits behind head on develop.

Files with missing lines	Patch %	Lines
src/libxrpl/ledger/helpers/TokenHelpers.cpp	94.4%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff            @@
##           develop   #6644     +/-   ##
=========================================
- Coverage     81.5%   81.4%   -0.0%     
=========================================
  Files          999     999             
  Lines        74458   74467      +9     
  Branches      7553    7557      +4     
=========================================
- Hits         60648   60646      -2     
- Misses       13810   13821     +11

Files with missing lines	Coverage Δ
src/libxrpl/ledger/helpers/TokenHelpers.cpp	`93.6% <94.4%> (+0.1%)`	⬆️

... and 4 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Use subtraction-based guards instead of addition to prevent uint64_t overflow in both the post-amendment aggregate check and the pre-amendment per-iteration check. Each condition in the cascade protects the subtraction in the next from underflow. Move totalSendAmount accumulation after the check so the guard operates on the pre-addition value.

xrplf-ai-reviewer

Looked through this one

One high-severity security note flagged inline: the pre-fixSecurity3_1_3 path allows issuer to bypass MaximumAmount via stale snapshot — activate the amendment promptly.

Review by ReviewBot 🤖

Review by Claude Opus 4.6 · Prompt: V12

pratikmankawde

LGTM

Copilot

Pull request overview

Fixes a correctness gap in MPToken multi-destination sends where MaximumAmount could be exceeded because per-iteration checks observed a stale view.read() snapshot instead of the updated outstanding amount.

Changes:

Update rippleSendMultiMPT to enforce MaximumAmount using an aggregate/running-total check inside the send loop (with pre-amendment behavior retained for replay compatibility).
Add a unit test covering multi-send aggregate MaximumAmount enforcement, including a pre-amendment “known bug” case.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
`src/libxrpl/ledger/helpers/TokenHelpers.cpp`	Implements aggregate `MaximumAmount` enforcement for issuer multi-send, with amendment gating for replay compatibility.
`src/test/app/MPToken_test.cpp`	Adds a regression test validating correct aggregate enforcement and preserving pre-amendment behavior expectations.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Kassaking7

LGTM

…-send-max-amount

xrplf-ai-reviewer

Two issues flagged inline: pre-amendment over-issuance path lacks replay-only guard in production code, and the test leaves fixSecurity3_1_3 disabled with no cleanup.

Review by Claude Opus 4.6 · Prompt: V12

xrplf-ai-reviewer

Went through the changes

Two correctness/security issues flagged inline: an unsigned underflow risk in the aggregate cap check (line 1210) and a missing explicit security warning on the live pre-amendment bypass path (line 1216). The test also mutates the shared Env feature set without re-enabling the amendment — see line 3353.

Review by ReviewBot 🤖

Review by Claude Opus 4.6 · Prompt: V12

xrplf-ai-reviewer · 2026-03-31T08:39:44Z

+        // exceeds MaximumAmount. Preserved for ledger replay.
+        {
+            // KNOWN BUG (pre-fixSecurity3_1_3): preserved for ledger replay only
+            env.disableFeature(fixSecurity3_1_3);


Mutating the shared env feature set is fragile — isolate the pre-amendment test in its own Env to prevent amendment-disabled state from potentially affecting later tests:

Replace Number/STAmount with std::int64_t for the totalSendAmount accumulator. STAmount implicitly converts to Number, whose small-scale mantissa (~16 digits) can lose precision for values near maxMPTokenAmount (19 digits), potentially producing incorrect MaximumAmount comparisons. Also fix test: re-enable fixSecurity3_1_3 after the pre-amendment block to avoid leaking disabled state into subsequent tests.

xrplf-ai-reviewer

Two security issues flagged inline: a signed-overflow in totalSendAmount that can bypass the MaximumAmount cap (high), a signed/unsigned mismatch in the comparison chain (medium), and a test-correctness risk where disableFeature may not affect the already-constructed view — see inline comments.

Review by Claude Opus 4.6 · Prompt: V12

Co-authored-by: xrplf-ai-reviewer[bot] <266832837+xrplf-ai-reviewer[bot]@users.noreply.github.com>

xrplf-ai-reviewer

Reviewed this MR

The fix is logically correct, but three safety concerns flagged inline: the pre-amendment supply-cap bypass path needs a runtime visibility guard; the unsigned subtraction chain relies on fragile short-circuit ordering; and the test lacks a paired negative assertion after re-enabling the amendment.

Review by ReviewBot 🤖

Review by Claude Opus 4.6 · Prompt: V12

bthomee · 2026-03-31T20:44:44Z

@Tapanito I can merge this PR once the conflict has been resolved.

…-send-max-amount

github-actions · 2026-04-01T08:12:31Z

All conflicts have been resolved. Assigned reviewers can now start or resume their review.

…ount Exercise the third guard condition (outstandingAmount + sendAmount + totalSendAmount > maximumAmount) by issuing tokens before the multi-send, so the check runs against a nonzero baseline.

…-send-max-amount

Co-authored-by: xrplf-ai-reviewer[bot] <266832837+xrplf-ai-reviewer[bot]@users.noreply.github.com>

Tapanito added Amendment AI Triage Bugs and fixes that have been triaged via AI initiatives labels Mar 25, 2026

github-actions Bot added the PR: has conflicts label Mar 26, 2026

Merge remote-tracking branch 'origin/develop' into tapanito/fix-multi…

e5b2d9a

…-send-max-amount # Conflicts: # include/xrpl/protocol/detail/features.macro # src/libxrpl/ledger/View.cpp

github-actions Bot removed the PR: has conflicts label Mar 26, 2026

Tapanito requested review from pratikmankawde and shawnxie999 March 26, 2026 10:51

Tapanito marked this pull request as ready for review March 26, 2026 10:51

xrplf-ai-reviewer Bot reviewed Mar 26, 2026

View reviewed changes

Comment thread src/libxrpl/ledger/helpers/TokenHelpers.cpp Outdated

pratikmankawde approved these changes Mar 27, 2026

View reviewed changes

kuznetsss requested a review from Copilot March 27, 2026 14:01

Copilot started reviewing on behalf of kuznetsss March 27, 2026 14:02 View session

Copilot AI reviewed Mar 27, 2026

View reviewed changes

Comment thread src/libxrpl/ledger/helpers/TokenHelpers.cpp Outdated

Comment thread src/libxrpl/ledger/helpers/TokenHelpers.cpp

Kassaking7 approved these changes Mar 30, 2026

View reviewed changes

Merge remote-tracking branch 'origin/develop' into tapanito/fix-multi…

6105bf7

…-send-max-amount

xrplf-ai-reviewer Bot reviewed Mar 31, 2026

View reviewed changes

Comment thread src/libxrpl/ledger/helpers/TokenHelpers.cpp Outdated

Comment thread src/test/app/MPToken_test.cpp

fix: build errors

867f219

xrplf-ai-reviewer Bot reviewed Mar 31, 2026

View reviewed changes

Comment thread src/libxrpl/ledger/helpers/TokenHelpers.cpp Outdated

Comment thread src/libxrpl/ledger/helpers/TokenHelpers.cpp

Comment thread src/test/app/MPToken_test.cpp

fix: change totalSendAmount to uint64

e56d7c0

Co-authored-by: xrplf-ai-reviewer[bot] <266832837+xrplf-ai-reviewer[bot]@users.noreply.github.com>

xrplf-ai-reviewer Bot reviewed Mar 31, 2026

View reviewed changes

Comment thread src/libxrpl/ledger/helpers/TokenHelpers.cpp

Comment thread src/libxrpl/ledger/helpers/TokenHelpers.cpp

Comment thread src/libxrpl/ledger/helpers/TokenHelpers.cpp

Comment thread src/test/app/MPToken_test.cpp

shawnxie999 reviewed Mar 31, 2026

View reviewed changes

Comment thread src/libxrpl/ledger/helpers/TokenHelpers.cpp

Merge remote-tracking branch 'origin/develop' into tapanito/fix-multi…

da16234

…-send-max-amount

github-actions Bot removed the PR: has conflicts label Apr 1, 2026

Tapanito added 2 commits April 1, 2026 12:52

fix: make types explicit

3c2fd7d

test: add nonzero outstandingAmount coverage for multi-send MaximumAm…

d6cd286

…ount Exercise the third guard condition (outstandingAmount + sendAmount + totalSendAmount > maximumAmount) by issuing tokens before the multi-send, so the check runs against a nonzero baseline.

bthomee reviewed Apr 1, 2026

View reviewed changes

Comment thread src/libxrpl/ledger/helpers/TokenHelpers.cpp Outdated

Tapanito added 2 commits April 1, 2026 15:09

fix: improve comment

4ec9e36

Merge remote-tracking branch 'origin/develop' into tapanito/fix-multi…

8e684b9

…-send-max-amount

bthomee enabled auto-merge April 1, 2026 13:14

bthomee added this pull request to the merge queue Apr 1, 2026

Merged via the queue into develop with commit bee1056 Apr 1, 2026
3 checks passed

bthomee deleted the tapanito/fix-multi-send-max-amount branch April 1, 2026 13:51

bthomee pushed a commit that referenced this pull request Apr 1, 2026

fix: Enforce aggregate MaximumAmount in multi-send MPT (#6644)

14749fd

Co-authored-by: xrplf-ai-reviewer[bot] <266832837+xrplf-ai-reviewer[bot]@users.noreply.github.com>

ximinez pushed a commit that referenced this pull request Apr 1, 2026

fix: Enforce aggregate MaximumAmount in multi-send MPT (#6644)

490fe68

Co-authored-by: xrplf-ai-reviewer[bot] <266832837+xrplf-ai-reviewer[bot]@users.noreply.github.com>

ximinez mentioned this pull request Apr 1, 2026

Staging branch for 3.1.3 #6703

Merged

ximinez pushed a commit that referenced this pull request Apr 1, 2026

fix: Enforce aggregate MaximumAmount in multi-send MPT (#6644)

0eb77f9

Co-authored-by: xrplf-ai-reviewer[bot] <266832837+xrplf-ai-reviewer[bot]@users.noreply.github.com>

ximinez pushed a commit that referenced this pull request Apr 6, 2026

fix: Enforce aggregate MaximumAmount in multi-send MPT (#6644)

11bab06

Co-authored-by: xrplf-ai-reviewer[bot] <266832837+xrplf-ai-reviewer[bot]@users.noreply.github.com>

ximinez added this to the 3.1.3 milestone Apr 15, 2026

ximinez added Needs 2 PRs (staging / develop) and removed Needs 2 PRs (staging / develop) labels Apr 15, 2026

ximinez modified the milestones: 3.1.3, 3.1.3 (develop) Apr 15, 2026

mvadari modified the milestones: 3.1.3 (develop), 3.2.0 May 20, 2026

beartec-jpg pushed a commit to beartec-jpg/FalconLedger that referenced this pull request Jun 1, 2026

fix: Enforce aggregate MaximumAmount in multi-send MPT (XRPLF#6644)

0525d12

Co-authored-by: xrplf-ai-reviewer[bot] <266832837+xrplf-ai-reviewer[bot]@users.noreply.github.com>

LeJamon mentioned this pull request Jul 7, 2026

Rippled 3.1.3 parity — tracking issue (fixCleanup3_1_3 + fix wave) LeJamon/go-xrpl#1248

Closed

32 tasks

Uh oh!

Conversation

Tapanito commented Mar 25, 2026

High Level Overview of Change

Context of Change

API Impact

Uh oh!

github-actions Bot commented Mar 26, 2026

Uh oh!

github-actions Bot commented Mar 26, 2026

Uh oh!

Tapanito commented Mar 26, 2026

Uh oh!

xrplf-ai-reviewer Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov Bot commented Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

xrplf-ai-reviewer Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pratikmankawde left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Kassaking7 left a comment

Choose a reason for hiding this comment

Uh oh!

xrplf-ai-reviewer Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

xrplf-ai-reviewer Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

xrplf-ai-reviewer Bot Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

xrplf-ai-reviewer Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

xrplf-ai-reviewer Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bthomee commented Mar 31, 2026

Uh oh!

github-actions Bot commented Apr 1, 2026

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

codecov Bot commented Mar 26, 2026 •

edited

Loading