🚀 Advanced: fix: Cap the base fee for LoanPay by ximinez · Pull Request #6969 · XRPLF/rippled · GitHub

ximinez · 2026-04-17T23:10:28Z

High Level Overview of Change

If a LoanPay pays more than periodicPayment * loanMaximumPaymentsPerTransaction calculate the maximum fee as maxFeeIncrements * baseFee.

maxFeeIncrements is loanMaximumPaymentsPerTransaction / loanPaymentsPerFeeIncrement.

develop port: #6970

xrplf-ai-reviewer

No issues.

Review by Claude Opus 4.6 · Prompt: V15

ximinez · 2026-04-17T23:13:53Z

I've got a handful of failing unit tests. I'll fix those on Monday.

vlntb

The proposed change fixes the root of the problem by capping the fee at loanMaximumPaymentsPerTransaction * normalCost, avoiding fee calculation for impossibly high payment counts that could cause overflow.

I'll review again once unit tests are finalised. The current Loan_test.cpp lacks explicit coverage for the fee capping logic. Please add tests for:

Amount exactly at threshold → capped fee
Amount below threshold → normal calculation
Amount well above threshold → capped fee
Amendment guard (fixSecurity3_1_3 on/off)

Note: testDosLoanPay() verifies payment capping but not fee capping.

xrplf-ai-reviewer

No issues.

Review by Claude Opus 4.6 · Prompt: V15

mathbunnyru

The name of this PR will have to be changed along with the on based on develop, to follow conventional commit rules

xrplf-ai-reviewer

No issues.

Review by Claude Opus 4.6 · Prompt: V15

- Old and busted: Computing max fee as loanMaximumPaymentsPerTransaction * normalCost; - New hotness: Computing max fee increments as loanMaximumPaymentsPerTransaction / loanPaymentsPerFeeIncrement, then computing max fee as maxFeeIncrements * normalCost, if payment amount >= maxFeeIncrements * periodicPayment.

xrplf-ai-reviewer

Went through the changes

Ungated assert will abort pre-amendment when amount is large — see inline.

Review by ReviewBot 🤖

Review by Claude Opus 4.6 · Prompt: V15

codecov · 2026-04-20T23:45:56Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.4%. Comparing base (31d4d08) to head (90b1284).
⚠️ Report is 1 commits behind head on ripple/staging-313.

Additional details and impacted files

@@                Coverage Diff                 @@
##           ripple/staging-313   #6969   +/-   ##
==================================================
  Coverage                80.4%   80.4%           
==================================================
  Files                     840     840           
  Lines                   75347   75352    +5     
  Branches                 8200    8200           
==================================================
+ Hits                    60585   60590    +5     
  Misses                  14762   14762

Files with missing lines	Coverage Δ
src/xrpld/app/tx/detail/LoanPay.cpp	`94.6% <100.0%> (+0.1%)`	⬆️

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:

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

xrplf-ai-reviewer

No issues.

Review by Claude Opus 4.6 · Prompt: V15

Copilot

Pull request overview

This PR adjusts LoanPay::calculateBaseFee to cap the minimum fee when the transaction amount implies more payments than can actually be processed per transaction, specifically when fixSecurity3_1_3 is enabled.

Changes:

Introduces a computed maxFeeIncrements based on protocol limits.
Adds an early-return cap intended to prevent charging more fee increments than the transaction can consume in work.
Adds an XRPL_ASSERT to enforce the (intended) invariant that fee increments do not exceed the cap under the amendment.

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

- Add test cases to exercise the LoanPay fee cap calculation - Fix a bug in the fee cap calculation.

xrplf-ai-reviewer

No issues.

Review by Claude Opus 4.6 · Prompt: V15

xrplf-ai-reviewer

No issues.

Review by Claude Opus 4.6 · Prompt: V15

xrplf-ai-reviewer

No issues.

Review by Claude Opus 4.6 · Prompt: V15

xrplf-ai-reviewer

No issues.

Review by Claude Opus 4.6 · Prompt: V15

The thing I asked for was addressed

Tapanito · 2026-04-22T09:15:16Z

    NumberRoundModeGuard mg(
        tx.isFlag(tfLoanOverpayment) ? Number::upward : Number::downward);
+
+    static_assert(


Why is this using static assert instead of XRPL_ASSERT?

Because static_assert fails at compile time. If the values are ever changed to something that doesn't match this expectation, it won't build, and thus will fail immediately.

kuznetsss

👍

xrplf-ai-reviewer

No issues.

Review by Claude Opus 4.6 · Prompt: V15

vlntb

The changes since my last approval look correct. All the missing test cases that I highlighted have been addressed.
Approving.

Co-authored-by: Bart <bthomee@users.noreply.github.com>

Cap the base fee for LoanPay based on loanMaximumPaymentsPerTransaction

55ca501

ximinez added this to the 3.1.3 milestone Apr 17, 2026

xrplf-ai-reviewer Bot approved these changes Apr 17, 2026

View reviewed changes

vlntb self-requested a review April 20, 2026 14:04

vlntb reviewed Apr 20, 2026

View reviewed changes

kuznetsss reviewed Apr 20, 2026

View reviewed changes

Comment thread src/xrpld/app/tx/detail/LoanPay.cpp Outdated

Merge branch 'ripple/staging-313' into ximinez/loanpay-fee-overflow-313

e059f6c

xrplf-ai-reviewer Bot approved these changes Apr 20, 2026

View reviewed changes

mathbunnyru previously requested changes Apr 20, 2026

View reviewed changes

Merge branch 'ripple/staging-313' into ximinez/loanpay-fee-overflow-313

b7055c0

xrplf-ai-reviewer Bot approved these changes Apr 20, 2026

View reviewed changes

xrplf-ai-reviewer Bot reviewed Apr 20, 2026

View reviewed changes

Comment thread src/xrpld/app/tx/detail/LoanPay.cpp

ximinez changed the title ~~Cap the base fee for LoanPay based on loanMaximumPaymentsPerTransaction~~ fix: Cap the base fee for LoanPay Apr 20, 2026

fixup! fix: base max fee on max payment increments

c82d414

xrplf-ai-reviewer Bot approved these changes Apr 21, 2026

View reviewed changes

kuznetsss approved these changes Apr 21, 2026

View reviewed changes

vlntb approved these changes Apr 21, 2026

View reviewed changes

bthomee requested a review from Copilot April 21, 2026 15:21

Copilot started reviewing on behalf of bthomee April 21, 2026 15:21 View session

Copilot AI reviewed Apr 21, 2026

View reviewed changes

Comment thread src/xrpld/app/tx/detail/LoanPay.cpp Outdated

Comment thread src/xrpld/app/tx/detail/LoanPay.cpp

Comment thread src/xrpld/app/tx/detail/LoanPay.cpp

Comment thread src/xrpld/app/tx/detail/LoanPay.cpp Outdated

ximinez mentioned this pull request Apr 21, 2026

fix: Cap the base fee for LoanPay (#6969) #6970

Merged

Review feedback from @vlntb: Add tests, fix bug

20e4bae

- Add test cases to exercise the LoanPay fee cap calculation - Fix a bug in the fee cap calculation.

xrplf-ai-reviewer Bot approved these changes Apr 21, 2026

View reviewed changes

ximinez requested review from kuznetsss, mathbunnyru and vlntb April 21, 2026 19:11

xrplf-ai-reviewer Bot approved these changes Apr 22, 2026

View reviewed changes

AI review feedback: don't specify "seq", and don't call "env.jt" twice

d3e31de

xrplf-ai-reviewer Bot approved these changes Apr 22, 2026

View reviewed changes

ximinez force-pushed the ximinez/loanpay-fee-overflow-313 branch from b3d6f82 to d3e31de Compare April 22, 2026 00:16

xrplf-ai-reviewer Bot approved these changes Apr 22, 2026

View reviewed changes

Tapanito reviewed Apr 22, 2026

View reviewed changes

kuznetsss approved these changes Apr 22, 2026

View reviewed changes

Merge branch 'ripple/staging-313' into ximinez/loanpay-fee-overflow-313

90b1284

xrplf-ai-reviewer Bot approved these changes Apr 22, 2026

View reviewed changes

vlntb approved these changes Apr 22, 2026

View reviewed changes

ximinez added the Ready to merge *PR author* thinks it's ready to merge. Has passed code review. Perf sign-off may still be required. label Apr 22, 2026

ximinez added this pull request to the merge queue Apr 22, 2026

Merged via the queue into ripple/staging-313 Apr 22, 2026
3 checks passed

ximinez deleted the ximinez/loanpay-fee-overflow-313 branch April 22, 2026 18:44

ximinez added a commit that referenced this pull request Apr 22, 2026

fix: Cap the base fee for LoanPay (#6969)

7ea4a5f

godexsoft pushed a commit to godexsoft/rippled that referenced this pull request May 7, 2026

fix: Cap the base fee for LoanPay (XRPLF#6969) (XRPLF#6970)

d6c4e6c

Co-authored-by: Bart <bthomee@users.noreply.github.com>

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

fix: Cap the base fee for LoanPay (XRPLF#6969) (XRPLF#6970)

1348d50

Co-authored-by: Bart <bthomee@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

ximinez commented Apr 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

High Level Overview of Change

Uh oh!

xrplf-ai-reviewer Bot left a comment

Choose a reason for hiding this comment

Uh oh!

ximinez commented Apr 17, 2026

Uh oh!

vlntb left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

xrplf-ai-reviewer Bot left a comment

Choose a reason for hiding this comment

Uh oh!

mathbunnyru 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!

xrplf-ai-reviewer Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codecov Bot commented Apr 20, 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!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

xrplf-ai-reviewer Bot 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!

xrplf-ai-reviewer Bot 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!

Tapanito Apr 22, 2026

Choose a reason for hiding this comment

Uh oh!

ximinez Apr 22, 2026

Choose a reason for hiding this comment

Uh oh!

kuznetsss 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!

vlntb left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

ximinez commented Apr 17, 2026 •

edited

Loading

codecov Bot commented Apr 20, 2026 •

edited

Loading