removes side-effects from tryOverpayment

src/test/app/LendingHelpers_test.cpp

@@ -620,10 +620,116 @@ class LendingHelpers_test : public beast::unit_test::suite
         }
     }
 
+    void
+    testTryOverpaymentValueChange()
+    {
+        // This test ensures that overpayment value change is computed
+        // correctly. I am sorry, this unit test will be a pain in the ass.
+        testcase("tryOverpay - Value Change is the decrease in interest");
+
+        using namespace jtx;
+        using namespace ripple::detail;
+
+        Env env{*this};
+        Account const issuer{"issuer"};
+        PrettyAsset const asset = issuer["USD"];
+
+        // Interest delta is 40 (100 - 50 - 10)
+        ExtendedPaymentComponents const overpaymentComponents = {
+            PaymentComponents{
+                .trackedValueDelta = Number{50, 0},
+                .trackedPrincipalDelta = Number{50, 0},
+                .trackedManagementFeeDelta = Number{0, 0},
+                .specialCase = PaymentSpecialCase::extra,
+            },
+            numZero,
+            numZero,
+        };
+
+        TenthBips16 managementFeeRate{20'000};  // 10%
+        TenthBips32 loanInterestRate{10'000};   // 20%
+        Number loanPrincipal{1'000};
+        std::uint32_t paymentInterval = 30 * 24 * 60 * 60;
+        std::uint32_t paymentsRemaining = 10;
+        std::int32_t loanScale = -5;
+        auto const periodicRate =
+            loanPeriodicRate(loanInterestRate, paymentInterval);
+
+        auto loanProperites = computeLoanProperties(
+            asset,
+            loanPrincipal,
+            loanInterestRate,
+            paymentInterval,
+            paymentsRemaining,
+            managementFeeRate,
+            loanScale);
+        std::cout << loanProperites.periodicPayment << std::endl;
+        std::cout << loanProperites.loanState.valueOutstanding << std::endl;
+        std::cout << loanProperites.loanState.interestOutstanding()
+                  << std::endl;
+
+        Number periodicPayment = loanProperites.periodicPayment;
+
+        auto const ret = tryOverpayment(
+            asset,
+            loanScale,
+            overpaymentComponents,
+            loanProperites.loanState,
+            periodicPayment,
+            periodicRate,
+            paymentsRemaining,
+            managementFeeRate,
+            env.journal);
+
+        BEAST_EXPECT(ret);
+
+        auto const& [actualPaymentParts, newLoanProperties] = *ret;
+        auto const newState = newLoanProperties.loanState;
+
+        BEAST_EXPECTS(
+            actualPaymentParts.valueChange ==
+                newState.interestDue - loanProperites.loanState.interestDue,
+            " valueChange mismatch: expected " +
+                to_string(
+                    newState.interestDue -
+                    loanProperites.loanState.interestDue) +
+                ", got " + to_string(actualPaymentParts.valueChange));
+
+        BEAST_EXPECTS(
+            actualPaymentParts.feePaid ==
+                loanProperites.loanState.managementFeeDue -
+                    newState.managementFeeDue,
+            " feePaid mismatch: expected " +
+                to_string(
+                    loanProperites.loanState.managementFeeDue -
+                    newState.managementFeeDue) +
+                ", got " + to_string(actualPaymentParts.feePaid));
+
+        BEAST_EXPECTS(
+            actualPaymentParts.principalPaid ==
+                loanProperites.loanState.principalOutstanding -
+                    newState.principalOutstanding,
+            " principalPaid mismatch: expected " +
+                to_string(
+                    loanProperites.loanState.principalOutstanding -
+                    newState.principalOutstanding) +
+                ", got " + to_string(actualPaymentParts.principalPaid));
+
+        BEAST_EXPECTS(
+            actualPaymentParts.interestPaid ==
+                loanProperites.loanState.interestDue - newState.interestDue,
+            " interestPaid mismatch: expected " +
+                to_string(
+                    loanProperites.loanState.interestDue -
+                    newState.interestDue) +
+                ", got " + to_string(actualPaymentParts.interestPaid));
+    }
+
 public:
     void
     run() override
     {
+        testTryOverpaymentValueChange();
         testComputeFullPaymentInterest();
         testLoanAccruedInterest();
         testLoanLatePaymentInterest();

src/test/app/Loan_test.cpp

@@ -705,8 +705,9 @@ protected:
             << "\tManagement Fee Rate: " << feeRate << std::endl
             << "\tTotal Payments: " << total << std::endl
             << "\tPeriodic Payment: " << props.periodicPayment << std::endl
-            << "\tTotal Value: " << props.totalValueOutstanding << std::endl
-            << "\tManagement Fee: " << props.managementFeeOwedToBroker
+            << "\tTotal Value: " << props.loanState.valueOutstanding
+            << std::endl
+            << "\tManagement Fee: " << props.loanState.managementFeeDue
             << std::endl
             << "\tLoan Scale: " << props.loanScale << std::endl
             << "\tFirst payment principal: " << props.firstPaymentPrincipal
@@ -1485,9 +1486,9 @@ protected:
             startDate + *loanParams.payInterval,
             *loanParams.payTotal,
             state.loanScale,
-            loanProperties.totalValueOutstanding,
+            loanProperties.loanState.valueOutstanding,
             principalRequestAmount,
-            loanProperties.managementFeeOwedToBroker,
+            loanProperties.loanState.managementFeeDue,
             loanProperties.periodicPayment,
             loanFlags | 0);
 
@@ -1542,9 +1543,9 @@ protected:
             nextDueDate,
             *loanParams.payTotal,
             loanProperties.loanScale,
-            loanProperties.totalValueOutstanding,
+            loanProperties.loanState.valueOutstanding,
             principalRequestAmount,
-            loanProperties.managementFeeOwedToBroker,
+            loanProperties.loanState.managementFeeDue,
             loanProperties.periodicPayment,
             loanFlags | 0);
 
@@ -7028,140 +7029,6 @@ protected:
             paymentParams);
     }
 
-    void
-    testLoanPayBrokerOwnerMissingTrustline()
-    {
-        testcase << "LoanPay Broker Owner Missing Trustline (PoC)";
-        using namespace jtx;
-        using namespace loan;
-        Account const issuer("issuer");
-        Account const borrower("borrower");
-        Account const broker("broker");
-        auto const IOU = issuer["IOU"];
-        Env env(*this, all);
-        env.fund(XRP(20'000), issuer, broker, borrower);
-        env.close();
-        // Set up trustlines and fund accounts
-        env(trust(broker, IOU(20'000'000)));
-        env(trust(borrower, IOU(20'000'000)));
-        env(pay(issuer, broker, IOU(10'000'000)));
-        env(pay(issuer, borrower, IOU(1'000)));
-        env.close();
-        // Create vault and broker
-        auto const brokerInfo = createVaultAndBroker(env, IOU, broker);
-        // Create a loan first (this creates debt)
-        auto const keylet = keylet::loan(brokerInfo.brokerID, 1);
-        env(set(borrower, brokerInfo.brokerID, 10'000),
-            sig(sfCounterpartySignature, broker),
-            loanServiceFee(IOU(100).value()),
-            paymentInterval(100),
-            fee(XRP(100)));
-        env.close();
-        // Ensure broker has sufficient cover so brokerPayee == brokerOwner
-        // We need coverAvailable >= (debtTotal * coverRateMinimum)
-        // Deposit enough cover to ensure the fee goes to broker owner
-        // The default coverRateMinimum is 10%, so for a 10,000 loan we need
-        // at least 1,000 cover. Default cover is 1,000, so we add more to be
-        // safe.
-        auto const additionalCover = IOU(50'000).value();
-        env(loanBroker::coverDeposit(
-            broker, brokerInfo.brokerID, STAmount{IOU, additionalCover}));
-        env.close();
-        // Verify broker owner has a trustline
-        auto const brokerTrustline = keylet::line(broker, IOU);
-        BEAST_EXPECT(env.le(brokerTrustline) != nullptr);
-        // Broker owner deletes their trustline
-        // First, pay any positive balance to issuer to zero it out
-        auto const brokerBalance = env.balance(broker, IOU);
-        env(pay(broker, issuer, brokerBalance));
-        env.close();
-        // Remove the trustline by setting limit to 0
-        env(trust(broker, IOU(0)));
-        env.close();
-        // Verify trustline is deleted
-        BEAST_EXPECT(env.le(brokerTrustline) == nullptr);
-        // Now borrower tries to make a payment
-        // We should get a tesSUCCESS instead of a tecNO_LINE.
-        env(pay(borrower, keylet.key, IOU(10'100)),
-            fee(XRP(100)),
-            ter(tesSUCCESS));
-        env.close();
-    }
-
-    void
-    testLoanPayBrokerOwnerUnauthorizedMPT()
-    {
-        testcase << "LoanPay Broker Owner MPT unauthorized";
-        using namespace jtx;
-        using namespace loan;
-
-        Account const issuer("issuer");
-        Account const borrower("borrower");
-        Account const broker("broker");
-
-        Env env(*this, all);
-        env.fund(XRP(20'000), issuer, broker, borrower);
-        env.close();
-
-        MPTTester mptt{env, issuer, mptInitNoFund};
-        mptt.create(
-            {.flags = tfMPTCanClawback | tfMPTCanTransfer | tfMPTCanLock});
-
-        PrettyAsset const MPT{mptt.issuanceID()};
-
-        // Authorize broker and borrower
-        mptt.authorize({.account = broker});
-        mptt.authorize({.account = borrower});
-
-        env.close();
-
-        // Fund accounts
-        env(pay(issuer, broker, MPT(10'000'000)));
-        env(pay(issuer, borrower, MPT(1'000)));
-        env.close();
-
-        // Create vault and broker
-        auto const brokerInfo = createVaultAndBroker(env, MPT, broker);
-        // Create a loan first (this creates debt)
-        auto const keylet = keylet::loan(brokerInfo.brokerID, 1);
-        env(set(borrower, brokerInfo.brokerID, 10'000),
-            sig(sfCounterpartySignature, broker),
-            loanServiceFee(MPT(100).value()),
-            paymentInterval(100),
-            fee(XRP(100)));
-        env.close();
-        // Ensure broker has sufficient cover so brokerPayee == brokerOwner
-        // We need coverAvailable >= (debtTotal * coverRateMinimum)
-        // Deposit enough cover to ensure the fee goes to broker owner
-        // The default coverRateMinimum is 10%, so for a 10,000 loan we need
-        // at least 1,000 cover. Default cover is 1,000, so we add more to be
-        // safe.
-        auto const additionalCover = MPT(50'000).value();
-        env(loanBroker::coverDeposit(
-            broker, brokerInfo.brokerID, STAmount{MPT, additionalCover}));
-        env.close();
-        // Verify broker owner is authorized
-        auto const brokerMpt = keylet::mptoken(mptt.issuanceID(), broker);
-        BEAST_EXPECT(env.le(brokerMpt) != nullptr);
-        // Broker owner unauthorizes.
-        // First, pay any positive balance to issuer to zero it out
-        auto const brokerBalance = env.balance(broker, MPT);
-        env(pay(broker, issuer, brokerBalance));
-        env.close();
-        // Then, unauthorize the MPT.
-        mptt.authorize({.account = broker, .flags = tfMPTUnauthorize});
-        env.close();
-        // Verify the MPT is unauthorized.
-        BEAST_EXPECT(env.le(brokerMpt) == nullptr);
-        // Now borrower tries to make a payment
-        // We should get a tesSUCCESS instead of a tecNO_AUTH.
-        auto const borrowerBalance = env.balance(borrower, MPT);
-        env(pay(borrower, keylet.key, MPT(10'100)),
-            fee(XRP(100)),
-            ter(tesSUCCESS));
-        env.close();
-    }
-
 public:
     void
     run() override
@@ -7210,8 +7077,6 @@ public:
         testBorrowerIsBroker();
         testIssuerIsBorrower();
         testLimitExceeded();
-        testLoanPayBrokerOwnerMissingTrustline();
-        testLoanPayBrokerOwnerUnauthorizedMPT();
     }
 };
 

src/xrpld/app/misc/LendingHelpers.h

@@ -84,46 +84,6 @@ struct LoanPaymentParts
     operator==(LoanPaymentParts const& other) const;
 };
 
-/* Describes the initial computed properties of a loan.
- *
- * This structure contains the fundamental calculated values that define a
- * loan's payment structure and amortization schedule. These properties are
- * computed:
- * - At loan creation (LoanSet transaction)
- * - When loan terms change (e.g., after an overpayment that reduces the loan
- * balance)
- */
-struct LoanProperties
-{
-    // The unrounded amount to be paid at each regular payment period.
-    // Calculated using the standard amortization formula based on principal,
-    // interest rate, and number of payments.
-    // The actual amount paid in the LoanPay transaction must be rounded up to
-    // the precision of the asset and loan.
-    Number periodicPayment;
-
-    // The total amount the borrower will pay over the life of the loan.
-    // Equal to periodicPayment * paymentsRemaining.
-    // This includes principal, interest, and management fees.
-    Number totalValueOutstanding;
-
-    // The total management fee that will be paid to the broker over the
-    // loan's lifetime. This is a percentage of the total interest (gross)
-    // as specified by the broker's management fee rate.
-    Number managementFeeOwedToBroker;
-
-    // The scale (decimal places) used for rounding all loan amounts.
-    // This is the maximum of:
-    // - The asset's native scale
-    // - A minimum scale required to represent the periodic payment accurately
-    // All loan state values (principal, interest, fees) are rounded to this
-    // scale.
-    std::int32_t loanScale;
-
-    // The principal portion of the first payment.
-    Number firstPaymentPrincipal;
-};
-
 /** This structure captures the parts of a loan state.
  *
  *  Whether the values are raw (unrounded) or rounded will depend on how it was
@@ -161,6 +121,39 @@ struct LoanState
     }
 };
 
+/* Describes the initial computed properties of a loan.
+ *
+ * This structure contains the fundamental calculated values that define a
+ * loan's payment structure and amortization schedule. These properties are
+ * computed:
+ * - At loan creation (LoanSet transaction)
+ * - When loan terms change (e.g., after an overpayment that reduces the loan
+ * balance)
+ */
+struct LoanProperties
+{
+    // The unrounded amount to be paid at each regular payment period.
+    // Calculated using the standard amortization formula based on principal,
+    // interest rate, and number of payments.
+    // The actual amount paid in the LoanPay transaction must be rounded up to
+    // the precision of the asset and loan.
+    Number periodicPayment;
+
+    // The loan's current state, with all values rounded to the loan's scale.
+    LoanState loanState;
+
+    // The scale (decimal places) used for rounding all loan amounts.
+    // This is the maximum of:
+    // - The asset's native scale
+    // - A minimum scale required to represent the periodic payment accurately
+    // All loan state values (principal, interest, fees) are rounded to this
+    // scale.
+    std::int32_t loanScale;
+
+    // The principal portion of the first payment.
+    Number firstPaymentPrincipal;
+};
+
 // Some values get re-rounded to the vault scale any time they are adjusted. In
 // addition, they are prevented from ever going below zero. This helps avoid
 // accumulated rounding errors and leftover dust amounts.
@@ -368,6 +361,18 @@ struct LoanStateDeltas
     nonNegative();
 };
 
+Expected<std::pair<LoanPaymentParts, LoanProperties>, TER>
+tryOverpayment(
+    Asset const& asset,
+    std::int32_t loanScale,
+    ExtendedPaymentComponents const& overpaymentComponents,
+    LoanState const& roundedLoanState,
+    Number const& periodicPayment,
+    Number const& periodicRate,
+    std::uint32_t paymentRemaining,
+    TenthBips16 const managementFeeRate,
+    beast::Journal j);
+
 Number
 computeRaisedRate(Number const& periodicRate, std::uint32_t paymentsRemaining);
 
@@ -446,13 +451,22 @@ operator+(LoanState const& lhs, detail::LoanStateDeltas const& rhs);
 LoanProperties
 computeLoanProperties(
     Asset const& asset,
-    Number principalOutstanding,
+    Number const& principalOutstanding,
     TenthBips32 interestRate,
     std::uint32_t paymentInterval,
     std::uint32_t paymentsRemaining,
     TenthBips32 managementFeeRate,
     std::int32_t minimumScale);
 
+LoanProperties
+computeLoanProperties(
+    Asset const& asset,
+    Number const& principalOutstanding,
+    Number const& periodicRate,
+    std::uint32_t paymentsRemaining,
+    TenthBips32 managementFeeRate,
+    std::int32_t minimumScale);
+
 bool
 isRounded(Asset const& asset, Number const& value, std::int32_t scale);
 

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,21 +384,15 @@ 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,
-    TenthBips32 interestRate,
-    std::uint32_t paymentInterval,
+    LoanState const& roundedOldState,
+    Number const& periodicPayment,
     Number const& periodicRate,
     std::uint32_t paymentRemaining,
-    std::uint32_t prevPaymentDate,
-    std::optional<std::uint32_t> nextDueDate,
     TenthBips16 const managementFeeRate,
     beast::Journal j)
 {
@@ -404,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.
@@ -426,8 +418,7 @@ tryOverpayment(
     auto newLoanProperties = computeLoanProperties(
         asset,
         newRawPrincipal,
-        interestRate,
-        paymentInterval,
+        periodicRate,
         paymentRemaining,
         managementFeeRate,
         loanScale);
@@ -435,7 +426,7 @@ tryOverpayment(
     JLOG(j.debug()) << "new periodic payment: "
                     << newLoanProperties.periodicPayment
                     << ", new total value: "
-                    << newLoanProperties.totalValueOutstanding
+                    << newLoanProperties.loanState.valueOutstanding
                     << ", first payment principal: "
                     << newLoanProperties.firstPaymentPrincipal;
 
@@ -454,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.totalValueOutstanding = 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(
@@ -494,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))
@@ -508,61 +497,64 @@ tryOverpayment(
     // Validate that all computed properties are reasonable. These checks should
     // never fail under normal circumstances, but we validate defensively.
     if (newLoanProperties.periodicPayment <= 0 ||
-        newLoanProperties.totalValueOutstanding <= 0 ||
-        newLoanProperties.managementFeeOwedToBroker < 0)
+        newLoanProperties.loanState.valueOutstanding <= 0 ||
+        newLoanProperties.loanState.managementFeeDue < 0)
     {
         // LCOV_EXCL_START
         JLOG(j.warn()) << "Overpayment not allowed: Computed loan "
                           "properties are invalid. Does "
                           "not compute. TotalValueOutstanding: "
-                       << newLoanProperties.totalValueOutstanding
+                       << newLoanProperties.loanState.valueOutstanding
                        << ", PeriodicPayment : "
                        << newLoanProperties.periodicPayment
                        << ", ManagementFeeOwedToBroker: "
-                       << newLoanProperties.managementFeeOwedToBroker;
+                       << newLoanProperties.loanState.managementFeeDue;
         return Unexpected(tesSUCCESS);
         // 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.
-    auto const valueChange =
-        newRounded.valueOutstanding - hypotheticalValueOutstanding;
+    auto const valueChange = -deltas.interest;
     if (valueChange > 0)
     {
         JLOG(j.warn()) << "Principal overpayment would increase the value of "
                           "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.
@@ -586,23 +578,16 @@ doOverpayment(
     NumberProxy& principalOutstandingProxy,
     NumberProxy& managementFeeOutstandingProxy,
     NumberProxy& periodicPaymentProxy,
-    TenthBips32 const interestRate,
-    std::uint32_t const paymentInterval,
     Number const& periodicRate,
     std::uint32_t const paymentRemaining,
-    std::uint32_t const prevPaymentDate,
-    std::optional<std::uint32_t> const nextDueDate,
     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
@@ -621,33 +606,28 @@ doOverpayment(
         asset,
         loanScale,
         overpaymentComponents,
-        totalValueOutstanding,
-        principalOutstanding,
-        managementFeeOutstanding,
+        loanState,
         periodicPayment,
-        interestRate,
-        paymentInterval,
         periodicRate,
         paymentRemaining,
-        prevPaymentDate,
-        nextDueDate,
         managementFeeRate,
         j);
     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
     }
@@ -658,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(),
@@ -694,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;
 }
@@ -1290,7 +1271,7 @@ checkLoanGuards(
     beast::Journal j)
 {
     auto const totalInterestOutstanding =
-        properties.totalValueOutstanding - principalRequested;
+        properties.loanState.valueOutstanding - principalRequested;
     // Guard 1: if there is no computed total interest over the life of the
     // loan for a non-zero interest rate, we cannot properly amortize the
     // loan
@@ -1345,13 +1326,13 @@ checkLoanGuards(
         NumberRoundModeGuard mg(Number::upward);
 
         if (std::int64_t const computedPayments{
-                properties.totalValueOutstanding / roundedPayment};
+                properties.loanState.valueOutstanding / roundedPayment};
             computedPayments != paymentTotal)
         {
             JLOG(j.warn()) << "Loan Periodic payment ("
                            << properties.periodicPayment << ") rounding ("
                            << roundedPayment << ") on a total value of "
-                           << properties.totalValueOutstanding
+                           << properties.loanState.valueOutstanding
                            << " can not complete the loan in the specified "
                               "number of payments ("
                            << computedPayments << " != " << paymentTotal << ")";
@@ -1534,7 +1515,7 @@ computeManagementFee(
 LoanProperties
 computeLoanProperties(
     Asset const& asset,
-    Number principalOutstanding,
+    Number const& principalOutstanding,
     TenthBips32 interestRate,
     std::uint32_t paymentInterval,
     std::uint32_t paymentsRemaining,
@@ -1545,7 +1526,24 @@ computeLoanProperties(
     XRPL_ASSERT(
         interestRate == 0 || periodicRate > 0,
         "ripple::computeLoanProperties : valid rate");
+    return computeLoanProperties(
+        asset,
+        principalOutstanding,
+        periodicRate,
+        paymentsRemaining,
+        managementFeeRate,
+        minimumScale);
+}
 
+LoanProperties
+computeLoanProperties(
+    Asset const& asset,
+    Number const& principalOutstanding,
+    Number const& periodicRate,
+    std::uint32_t paymentsRemaining,
+    TenthBips32 managementFeeRate,
+    std::int32_t minimumScale)
+{
     auto const periodicPayment = detail::loanPeriodicPayment(
         principalOutstanding, periodicRate, paymentsRemaining);
 
@@ -1579,12 +1577,12 @@ computeLoanProperties(
     // Since we just figured out the loan scale, we haven't been able to
     // validate that the principal fits in it, so to allow this function to
     // succeed, round it here, and let the caller do the validation.
-    principalOutstanding = roundToAsset(
+    auto const roundedPrincipalOutstanding = roundToAsset(
         asset, principalOutstanding, loanScale, Number::to_nearest);
 
     // E<quation (31) from XLS-66 spec, Section A-2 Equation Glossary
     auto const totalInterestOutstanding =
-        totalValueOutstanding - principalOutstanding;
+        totalValueOutstanding - roundedPrincipalOutstanding;
     auto const feeOwedToBroker = computeManagementFee(
         asset, totalInterestOutstanding, managementFeeRate, loanScale);
 
@@ -1614,10 +1612,13 @@ computeLoanProperties(
 
     return LoanProperties{
         .periodicPayment = periodicPayment,
-        .totalValueOutstanding = totalValueOutstanding,
-        .managementFeeOwedToBroker = feeOwedToBroker,
+        .loanState = constructLoanState(
+            totalValueOutstanding,
+            roundedPrincipalOutstanding,
+            feeOwedToBroker),
         .loanScale = loanScale,
-        .firstPaymentPrincipal = firstPaymentPrincipal};
+        .firstPaymentPrincipal = firstPaymentPrincipal,
+    };
 }
 
 /*
@@ -1946,12 +1947,8 @@ loanMakePayment(
                     principalOutstandingProxy,
                     managementFeeOutstandingProxy,
                     periodicPaymentProxy,
-                    interestRate,
-                    paymentInterval,
                     periodicRate,
                     paymentRemainingProxy,
-                    prevPaymentDateProxy,
-                    nextDueDateProxy,
                     managementFeeRate,
                     j))
                 totalParts += *overResult;

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

@@ -262,10 +262,9 @@ LoanPay::doApply()
     auto debtTotalProxy = brokerSle->at(sfDebtTotal);
 
     // Send the broker fee to the owner if they have sufficient cover available,
-    // _and_ if the owner can receive funds
-    // _and_ if the broker is authorized to hold funds. If not, so as not to
-    // block the payment, add it to the cover balance (send it to the broker
-    // pseudo account).
+    // _and_ if the owner can receive funds. If not, so as not to block the
+    // payment, add it to the cover balance (send it to the broker pseudo
+    // account).
     //
     // Normally freeze status is checked in preflight, but we do it here to
     // avoid duplicating the check. It'll claim a fee either way.
@@ -279,9 +278,7 @@ LoanPay::doApply()
                    asset,
                    tenthBipsOfValue(debtTotalProxy.value(), coverRateMinimum),
                    loanScale) &&
-            !isDeepFrozen(view, brokerOwner, asset) &&
-            requireAuth(view, asset, brokerOwner, AuthType::StrongAuth) ==
-            tesSUCCESS;
+            !isDeepFrozen(view, brokerOwner, asset);
     }();
 
     auto const brokerPayee =

src/xrpld/app/tx/detail/LoanSet.cpp

@@ -417,7 +417,7 @@ LoanSet::doApply()
                 JLOG(j_.warn())
                     << field.f->getName() << " (" << *value
                     << ") has too much precision. Total loan value is "
-                    << properties.totalValueOutstanding << " with a scale of "
+                    << properties.loanState.valueOutstanding << " with a scale of "
                     << properties.loanScale;
                 return tecPRECISION_LOSS;
             }
@@ -434,8 +434,8 @@ LoanSet::doApply()
         return ret;
 
     // Check that the other computed values are valid
-    if (properties.managementFeeOwedToBroker < 0 ||
-        properties.totalValueOutstanding <= 0 ||
+    if (properties.loanState.managementFeeDue < 0 ||
+        properties.loanState.valueOutstanding <= 0 ||
         properties.periodicPayment <= 0)
     {
         // LCOV_EXCL_START
@@ -446,9 +446,9 @@ LoanSet::doApply()
     }
 
     LoanState const state = constructLoanState(
-        properties.totalValueOutstanding,
+        properties.loanState.valueOutstanding,
         principalRequested,
-        properties.managementFeeOwedToBroker);
+        properties.loanState.managementFeeDue);
 
     auto const originationFee = tx[~sfLoanOriginationFee].value_or(Number{});
 
@@ -588,8 +588,8 @@ LoanSet::doApply()
     // Set dynamic / computed fields to their initial values
     loan->at(sfPrincipalOutstanding) = principalRequested;
     loan->at(sfPeriodicPayment) = properties.periodicPayment;
-    loan->at(sfTotalValueOutstanding) = properties.totalValueOutstanding;
-    loan->at(sfManagementFeeOutstanding) = properties.managementFeeOwedToBroker;
+    loan->at(sfTotalValueOutstanding) = properties.loanState.valueOutstanding;
+    loan->at(sfManagementFeeOutstanding) = properties.loanState.managementFeeDue;
     loan->at(sfPreviousPaymentDate) = 0;
     loan->at(sfNextPaymentDueDate) = startDate + paymentInterval;
     loan->at(sfPaymentRemaining) = paymentTotal;