removes side-effects from tryOverpayment

src/test/app/LendingHelpers_test.cpp

@@ -668,23 +668,14 @@ class LendingHelpers_test : public beast::unit_test::suite
         std::cout << loanProperites.loanState.interestOutstanding()
                   << std::endl;
 
-        Number totalValueOutstanding =
-            loanProperites.loanState.valueOutstanding;
-        Number principalOutstanding =
-            loanProperites.loanState.principalOutstanding;
-        Number managementFeeOutstanding =
-            loanProperites.loanState.managementFeeDue;
         Number periodicPayment = loanProperites.periodicPayment;
 
         auto const ret = tryOverpayment(
             asset,
             loanScale,
             overpaymentComponents,
-            totalValueOutstanding,
-            principalOutstanding,
-            managementFeeOutstanding,
+            loanProperites.loanState,
             periodicPayment,
-            paymentInterval,
             periodicRate,
             paymentsRemaining,
             managementFeeRate,
@@ -692,50 +683,45 @@ class LendingHelpers_test : public beast::unit_test::suite
 
         BEAST_EXPECT(ret);
 
-        auto const& actualPaymentParts = *ret;
+        auto const& [actualPaymentParts, newLoanProperties] = *ret;
+        auto const newState = newLoanProperties.loanState;
 
         BEAST_EXPECTS(
             actualPaymentParts.valueChange ==
-                ((totalValueOutstanding - principalOutstanding -
-                  managementFeeOutstanding)) -
-                    loanProperites.loanState.interestDue,
+                newState.interestDue - loanProperites.loanState.interestDue,
             " valueChange mismatch: expected " +
                 to_string(
-                    (totalValueOutstanding - principalOutstanding -
-                     managementFeeOutstanding) -
+                    newState.interestDue -
                     loanProperites.loanState.interestDue) +
                 ", got " + to_string(actualPaymentParts.valueChange));
 
         BEAST_EXPECTS(
             actualPaymentParts.feePaid ==
                 loanProperites.loanState.managementFeeDue -
-                    managementFeeOutstanding,
+                    newState.managementFeeDue,
             " feePaid mismatch: expected " +
                 to_string(
                     loanProperites.loanState.managementFeeDue -
-                    managementFeeOutstanding) +
+                    newState.managementFeeDue) +
                 ", got " + to_string(actualPaymentParts.feePaid));
 
         BEAST_EXPECTS(
             actualPaymentParts.principalPaid ==
                 loanProperites.loanState.principalOutstanding -
-                    principalOutstanding,
+                    newState.principalOutstanding,
             " principalPaid mismatch: expected " +
                 to_string(
                     loanProperites.loanState.principalOutstanding -
-                    principalOutstanding) +
+                    newState.principalOutstanding) +
                 ", got " + to_string(actualPaymentParts.principalPaid));
 
         BEAST_EXPECTS(
             actualPaymentParts.interestPaid ==
-                loanProperites.loanState.interestDue -
-                    (totalValueOutstanding - principalOutstanding -
-                     managementFeeOutstanding),
+                loanProperites.loanState.interestDue - newState.interestDue,
             " interestPaid mismatch: expected " +
                 to_string(
                     loanProperites.loanState.interestDue -
-                    (totalValueOutstanding - principalOutstanding -
-                     managementFeeOutstanding)) +
+                    newState.interestDue) +
                 ", got " + to_string(actualPaymentParts.interestPaid));
     }
 

src/xrpld/app/misc/LendingHelpers.h

@@ -361,16 +361,13 @@ struct LoanStateDeltas
     nonNegative();
 };
 
-Expected<LoanPaymentParts, TER>
+Expected<std::pair<LoanPaymentParts, LoanProperties>, TER>
 tryOverpayment(
     Asset const& asset,
     std::int32_t loanScale,
     ExtendedPaymentComponents const& overpaymentComponents,
-    Number& totalValueOutstanding,
-    Number& principalOutstanding,
-    Number& managementFeeOutstanding,
-    Number& periodicPayment,
-    std::uint32_t paymentInterval,
+    LoanState const& roundedLoanState,
+    Number const& periodicPayment,
     Number const& periodicRate,
     std::uint32_t paymentRemaining,
     TenthBips16 const managementFeeRate,

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

@@ -2,6 +2,8 @@
 // DO NOT REMOVE forces header file include to sort first
 #include <xrpld/app/tx/detail/VaultCreate.h>
 
+#include <utility>
+
 namespace ripple {
 
 bool
@@ -382,16 +384,13 @@ doPayment(
  * The function preserves accumulated rounding errors across the re-amortization
  * to ensure the loan state remains consistent with its payment history.
  */
-Expected<LoanPaymentParts, TER>
+Expected<std::pair<LoanPaymentParts, LoanProperties>, TER>
 tryOverpayment(
     Asset const& asset,
     std::int32_t loanScale,
     ExtendedPaymentComponents const& overpaymentComponents,
-    Number& totalValueOutstanding,
-    Number& principalOutstanding,
-    Number& managementFeeOutstanding,
-    Number& periodicPayment,
-    std::uint32_t paymentInterval,
+    LoanState const& roundedOldState,
+    Number const& periodicPayment,
     Number const& periodicRate,
     std::uint32_t paymentRemaining,
     TenthBips16 const managementFeeRate,
@@ -401,15 +400,11 @@ tryOverpayment(
     auto const raw = computeRawLoanState(
         periodicPayment, periodicRate, paymentRemaining, managementFeeRate);
 
-    // Get the actual loan state (with accumulated rounding from past payments)
-    auto const rounded = constructLoanState(
-        totalValueOutstanding, principalOutstanding, managementFeeOutstanding);
-
     // Calculate the accumulated rounding errors. These need to be preserved
     // across the re-amortization to maintain consistency with the loan's
     // payment history. Without preserving these errors, the loan could end
     // up with a different total value than what the borrower has actually paid.
-    auto const errors = rounded - raw;
+    auto const errors = roundedOldState - raw;
 
     // Compute the new principal by applying the overpayment to the raw
     // (theoretical) principal. Use max with 0 to ensure we never go negative.
@@ -450,37 +445,35 @@ tryOverpayment(
     // rounding errors. This ensures the loan's tracked state remains
     // consistent with its payment history.
 
-    principalOutstanding = std::clamp(
+    auto const principalOutstanding = std::clamp(
         roundToAsset(
             asset, newRaw.principalOutstanding, loanScale, Number::upward),
         numZero,
-        rounded.principalOutstanding);
-    totalValueOutstanding = std::clamp(
+        roundedOldState.principalOutstanding);
+    auto const totalValueOutstanding = std::clamp(
         roundToAsset(
             asset,
             principalOutstanding + newRaw.interestOutstanding(),
             loanScale,
             Number::upward),
         numZero,
-        rounded.valueOutstanding);
-    managementFeeOutstanding = std::clamp(
+        roundedOldState.valueOutstanding);
+    auto const managementFeeOutstanding = std::clamp(
         roundToAsset(asset, newRaw.managementFeeDue, loanScale),
         numZero,
-        rounded.managementFeeDue);
+        roundedOldState.managementFeeDue);
 
-    auto const newRounded = constructLoanState(
+    auto const roundedNewState = constructLoanState(
         totalValueOutstanding, principalOutstanding, managementFeeOutstanding);
 
     // Update newLoanProperties so that checkLoanGuards can make an accurate
     // evaluation.
-    newLoanProperties.loanState.valueOutstanding = newRounded.valueOutstanding;
+    newLoanProperties.loanState = roundedNewState;
 
-    JLOG(j.debug()) << "new rounded value: " << newRounded.valueOutstanding
-                    << ", principal: " << newRounded.principalOutstanding
-                    << ", interest gross: " << newRounded.interestOutstanding();
-
-    // Update the periodic payment to reflect the re-amortized schedule
-    periodicPayment = newLoanProperties.periodicPayment;
+    JLOG(j.debug()) << "new rounded value: " << roundedNewState.valueOutstanding
+                    << ", principal: " << roundedNewState.principalOutstanding
+                    << ", interest gross: "
+                    << roundedNewState.interestOutstanding();
 
     // check that the loan is still valid
     if (auto const ter = checkLoanGuards(
@@ -490,7 +483,7 @@ tryOverpayment(
             // small interest amounts, that may have already been paid
             // off. Check what's still outstanding. This should
             // guarantee that the interest checks pass.
-            newRounded.interestOutstanding() != beast::zero,
+            roundedNewState.interestOutstanding() != beast::zero,
             paymentRemaining,
             newLoanProperties,
             j))
@@ -520,19 +513,16 @@ tryOverpayment(
         // LCOV_EXCL_STOP
     }
 
-    auto const deltas = rounded - newRounded;
+    auto const deltas = roundedOldState - roundedNewState;
 
     // The change in loan management fee is equal to the change between the old
     // and the new outstanding management fees
     XRPL_ASSERT_PARTS(
         deltas.managementFee ==
-            rounded.managementFeeDue - managementFeeOutstanding,
+            roundedOldState.managementFeeDue - managementFeeOutstanding,
         "ripple::detail::tryOverpayment",
         "no fee change");
 
-    auto const hypotheticalValueOutstanding =
-        rounded.valueOutstanding - deltas.principal;
-
     // Calculate how the loan's value changed due to the overpayment.
     // This should be negative (value decreased) or zero. A principal
     // overpayment should never increase the loan's value.
@@ -543,21 +533,28 @@ tryOverpayment(
                           "the loan. Ignore the overpayment";
         return Unexpected(tesSUCCESS);
     }
-    return LoanPaymentParts{
-        // Principal paid is the reduction in principal outstanding
-        .principalPaid = deltas.principal,
-        // Interest paid is the reduction in interest due
-        .interestPaid =
-            deltas.interest + overpaymentComponents.untrackedInterest,
-        // 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)
-        .valueChange =
-            valueChange + overpaymentComponents.trackedInterestPart(),
-        // Fee paid includes both the reduction in tracked management fees and
-        // any untracked fees on the overpayment itself
-        .feePaid = deltas.managementFee +
-            overpaymentComponents.untrackedManagementFee};
+
+    return std::make_pair(
+        LoanPaymentParts{
+            // Principal paid is the reduction in principal outstanding
+            .principalPaid = deltas.principal,
+            // Interest paid is the reduction in interest due
+            .interestPaid =
+                deltas.interest + overpaymentComponents.untrackedInterest,
+            // 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)
+            .valueChange =
+                valueChange + overpaymentComponents.trackedInterestPart(),
+            // Fee paid includes both the reduction in tracked management fees
+            // and
+            // any untracked fees on the overpayment itself
+            .feePaid = deltas.managementFee +
+                overpaymentComponents.untrackedManagementFee,
+        },
+        newLoanProperties);
 }
 
 /* Validates and applies an overpayment to the loan state.
@@ -581,20 +578,16 @@ doOverpayment(
     NumberProxy& principalOutstandingProxy,
     NumberProxy& managementFeeOutstandingProxy,
     NumberProxy& periodicPaymentProxy,
-    std::uint32_t const paymentInterval,
     Number const& periodicRate,
     std::uint32_t const paymentRemaining,
     TenthBips16 const managementFeeRate,
     beast::Journal j)
 {
-    // Create temporary copies of the loan state that can be safely modified
-    // and discarded if the overpayment doesn't work out. This prevents
-    // corrupting the actual ledger data if validation fails.
-    Number totalValueOutstanding = totalValueOutstandingProxy;
-    Number principalOutstanding = principalOutstandingProxy;
-    Number managementFeeOutstanding = managementFeeOutstandingProxy;
-    Number periodicPayment = periodicPaymentProxy;
-
+    auto const loanState = constructLoanState(
+        totalValueOutstandingProxy,
+        principalOutstandingProxy,
+        managementFeeOutstandingProxy);
+    auto const periodicPayment = periodicPaymentProxy;
     JLOG(j.debug())
         << "overpayment components:"
         << ", totalValue before: " << *totalValueOutstandingProxy
@@ -613,11 +606,8 @@ doOverpayment(
         asset,
         loanScale,
         overpaymentComponents,
-        totalValueOutstanding,
-        principalOutstanding,
-        managementFeeOutstanding,
+        loanState,
         periodicPayment,
-        paymentInterval,
         periodicRate,
         paymentRemaining,
         managementFeeRate,
@@ -625,18 +615,19 @@ doOverpayment(
     if (!ret)
         return Unexpected(ret.error());
 
-    auto const& loanPaymentParts = *ret;
+    auto const& [loanPaymentParts, newLoanProperties] = *ret;
+    auto const newRoundedLoanState = newLoanProperties.loanState;
 
     // Safety check: the principal must have decreased. If it didn't (or
     // increased!), something went wrong in the calculation and we should
     // reject the overpayment.
-    if (principalOutstandingProxy <= principalOutstanding)
+    if (principalOutstandingProxy <= newRoundedLoanState.principalOutstanding)
     {
         // LCOV_EXCL_START
         JLOG(j.warn()) << "Overpayment not allowed: principal "
                        << "outstanding did not decrease. Before: "
-                       << *principalOutstandingProxy
-                       << ". After: " << principalOutstanding;
+                       << *principalOutstandingProxy << ". After: "
+                       << newRoundedLoanState.principalOutstanding;
         return Unexpected(tesSUCCESS);
         // LCOV_EXCL_STOP
     }
@@ -647,28 +638,29 @@ doOverpayment(
 
     XRPL_ASSERT_PARTS(
         overpaymentComponents.trackedPrincipalDelta ==
-            principalOutstandingProxy - principalOutstanding,
+            principalOutstandingProxy -
+                newRoundedLoanState.principalOutstanding,
         "ripple::detail::doOverpayment",
         "principal change agrees");
 
     // I'm not 100% sure the following asserts are correct. If in doubt, and
     // everything else works, remove any that cause trouble.
 
-    JLOG(j.debug()) << "valueChange: " << loanPaymentParts.valueChange
-                    << ", totalValue before: " << *totalValueOutstandingProxy
-                    << ", totalValue after: " << totalValueOutstanding
-                    << ", totalValue delta: "
-                    << (totalValueOutstandingProxy - totalValueOutstanding)
-                    << ", principalDelta: "
-                    << overpaymentComponents.trackedPrincipalDelta
-                    << ", principalPaid: " << loanPaymentParts.principalPaid
-                    << ", Computed difference: "
-                    << overpaymentComponents.trackedPrincipalDelta -
-            (totalValueOutstandingProxy - totalValueOutstanding);
+    JLOG(j.debug())
+        << "valueChange: " << loanPaymentParts.valueChange
+        << ", totalValue before: " << *totalValueOutstandingProxy
+        << ", totalValue after: " << newRoundedLoanState.valueOutstanding
+        << ", totalValue delta: "
+        << (totalValueOutstandingProxy - newRoundedLoanState.valueOutstanding)
+        << ", principalDelta: " << overpaymentComponents.trackedPrincipalDelta
+        << ", principalPaid: " << loanPaymentParts.principalPaid
+        << ", Computed difference: "
+        << overpaymentComponents.trackedPrincipalDelta -
+            (totalValueOutstandingProxy - newRoundedLoanState.valueOutstanding);
 
     XRPL_ASSERT_PARTS(
         loanPaymentParts.valueChange ==
-            totalValueOutstanding -
+            newRoundedLoanState.valueOutstanding -
                 (totalValueOutstandingProxy -
                  overpaymentComponents.trackedPrincipalDelta) +
                 overpaymentComponents.trackedInterestPart(),
@@ -683,10 +675,10 @@ doOverpayment(
 
     // All validations passed, so update the proxy objects (which will
     // modify the actual Loan ledger object)
-    totalValueOutstandingProxy = totalValueOutstanding;
-    principalOutstandingProxy = principalOutstanding;
-    managementFeeOutstandingProxy = managementFeeOutstanding;
-    periodicPaymentProxy = periodicPayment;
+    totalValueOutstandingProxy = newRoundedLoanState.valueOutstanding;
+    principalOutstandingProxy = newRoundedLoanState.principalOutstanding;
+    managementFeeOutstandingProxy = newRoundedLoanState.managementFeeDue;
+    periodicPaymentProxy = newLoanProperties.periodicPayment;
 
     return loanPaymentParts;
 }
@@ -1955,7 +1947,6 @@ loanMakePayment(
                     principalOutstandingProxy,
                     managementFeeOutstandingProxy,
                     periodicPaymentProxy,
-                    paymentInterval,
                     periodicRate,
                     paymentRemainingProxy,
                     managementFeeRate,