Lending Protocol: Fix Overpayment ValueChange calculation #6114

Tapanito · 2025-12-05T12:34:33Z

Fixes two bugs in overpayment:

Fixes overpayment calculation, adds tests and refactors tryOverpayment to be without side effects.
Adds missing management fee to overpayments.

codecov · 2025-12-08T10:54:32Z

Codecov Report

❌ Patch coverage is 93.84615% with 4 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (ximinez/lending-XLS-66-ongoing@40c29bb). Learn more about missing BASE report.

Files with missing lines	Patch %	Lines
src/xrpld/app/misc/detail/LendingHelpers.cpp	96.6%	2 Missing ⚠️
src/xrpld/app/tx/detail/LoanSet.cpp	71.4%	2 Missing ⚠️

Additional details and impacted files

@@                       Coverage Diff                        @@
##             ximinez/lending-XLS-66-ongoing   #6114   +/-   ##
================================================================
  Coverage                                  ?   79.1%           
================================================================
  Files                                     ?     839           
  Lines                                     ?   71385           
  Branches                                  ?    8320           
================================================================
  Hits                                      ?   56476           
  Misses                                    ?   14909           
  Partials                                  ?       0

Files with missing lines	Coverage Δ
src/xrpld/app/misc/LendingHelpers.h	`95.0% <ø> (ø)`
src/xrpld/app/misc/detail/LendingHelpers.cpp	`90.1% <96.6%> (ø)`
src/xrpld/app/tx/detail/LoanSet.cpp	`91.4% <71.4%> (ø)`

🚀 New features to boost your workflow:

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

gregtatcam · 2025-12-09T17:58:18Z

src/xrpld/app/misc/detail/LendingHelpers.cpp

-        hypotheticalValueOutstanding - deltas.managementFee;
+    // The value change is derived from the reduction in interest due to
+    // the lower principal.
+    auto const valueChange = -deltas.interest;


Has the implementation deviated from the specs? Even the original implementation looks different. Should the specs be updated?

The rationale behind this change is as follows:

An overpayment decreases the principal outstanding value, which in turn reduces the future interest of the loan. Thus the loan value decreases by the difference between the current interest due, and the new interest due (excluding management fee).

Sure, but shouldn't the specs be updated?

Yes, of course!

Please update!

gregtatcam

LGTM

ximinez · 2025-12-10T23:28:56Z

src/xrpld/app/misc/detail/LendingHelpers.cpp

+            // Value change includes both the reduction from paying down
+            // principal
+            // (negative) and any untracked interest penalties (positive, e.g.,
+            // if
+            // the overpayment itself incurs a fee)


Could you reformat this comment so it doesn't look so choppy?

ximinez · 2025-12-10T23:29:01Z

src/xrpld/app/misc/detail/LendingHelpers.cpp

+            // Fee paid includes both the reduction in tracked management fees
+            // and
+            // any untracked fees on the overpayment itself


Could you reformat this comment so it doesn't look so choppy?

ximinez · 2025-12-10T23:39:49Z

src/test/app/LendingHelpers_test.cpp

+        TenthBips16 managementFeeRate{20'000};  // 10%
+        TenthBips32 loanInterestRate{10'000};   // 20%


The comments on these values are switched.

ximinez · 2025-12-10T23:41:02Z

src/test/app/LendingHelpers_test.cpp

+        std::cout << loanProperites.periodicPayment << std::endl;
+        std::cout << loanProperites.loanState.valueOutstanding << std::endl;
+        std::cout << loanProperites.loanState.interestOutstanding()
+                  << std::endl;


Please change std::cout to log so the messages can be suppressed at runtime if the caller doesn't want to see them.

Oooops. this wasn't suppose to be there. Removed!

ximinez · 2025-12-10T23:42:15Z

src/test/app/LendingHelpers_test.cpp

+        BEAST_EXPECT(ret);
+
+        auto const& [actualPaymentParts, newLoanProperties] = *ret;
+        auto const newState = newLoanProperties.loanState;


Suggested change

auto const newState = newLoanProperties.loanState;

auto const& newState = newLoanProperties.loanState;

ximinez · 2025-12-10T23:43:53Z

src/test/app/Loan_test.cpp

-    void
-    testBasicMath()
-    {
-        // Test the functions defined in LendingHelpers.h
-        testcase("Basic Math");
-
-        pass();
-    }


Woops. That one slipped through the cracks. LOL.

…verpayment-value-change

Tapanito requested a review from ximinez December 5, 2025 12:34

Tapanito requested a review from a team as a code owner December 5, 2025 12:34

Tapanito added 3 commits December 8, 2025 11:00

adds loan state to LoanProperties

7216120

cleans up computeLoanProperties

e27fecd

fixes overpayment valuechange calculation

cf33c9d

Tapanito force-pushed the tapanito/lending-overpayment-value-change branch from cb69f06 to cf33c9d Compare December 8, 2025 10:30

Tapanito changed the title ~~draft: Lending protocol overpayment~~ Lending Protocol: Fix Overpayment ValueChange calculation Dec 8, 2025

Tapanito added 4 commits December 8, 2025 12:02

fixes formatting

f9438cc

fixes missing management fee from overpayment

d7c1fbb

clang-format

7f25661

improves unit test wording

e47d691

Tapanito requested a review from gregtatcam December 9, 2025 15:17

gregtatcam reviewed Dec 9, 2025

View reviewed changes

gregtatcam approved these changes Dec 10, 2025

View reviewed changes

ximinez requested changes Dec 10, 2025

View reviewed changes

Tapanito added 4 commits December 11, 2025 11:29

addresses review comments

aa1386f

Merge branch 'ximinez/lending-XLS-66-ongoing' into tapanito/lending-o…

9a95502

…verpayment-value-change

removes ripple namespace usage

478cb46

Merge branch 'ximinez/lending-XLS-66-ongoing' into tapanito/lending-o…

5546ffa

…verpayment-value-change

Tapanito requested a review from ximinez December 15, 2025 12:32

Merge branch 'ximinez/lending-XLS-66-ongoing' into tapanito/lending-o…

5e4f593

…verpayment-value-change

Tapanito 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 Dec 16, 2025

		TenthBips16 managementFeeRate{20'000}; // 10%
		TenthBips32 loanInterestRate{10'000}; // 20%

	auto const newState = newLoanProperties.loanState;
	auto const& newState = newLoanProperties.loanState;

Lending Protocol: Fix Overpayment ValueChange calculation #6114

Are you sure you want to change the base?

Lending Protocol: Fix Overpayment ValueChange calculation #6114

Conversation

Tapanito commented Dec 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Dec 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

gregtatcam Dec 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gregtatcam left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Tapanito commented Dec 5, 2025 •

edited

Loading

codecov bot commented Dec 8, 2025 •

edited

Loading

gregtatcam Dec 9, 2025 •

edited

Loading