Polish the code

Signed-off-by: JCW <a1q123456@users.noreply.github.com>

src/test/app/LendingHelpers_test.cpp

@@ -620,116 +620,10 @@ 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,9 +705,8 @@ protected:
             << "\tManagement Fee Rate: " << feeRate << std::endl
             << "\tTotal Payments: " << total << std::endl
             << "\tPeriodic Payment: " << props.periodicPayment << std::endl
-            << "\tTotal Value: " << props.loanState.valueOutstanding
+            << "\tTotal Value: " << props.totalValueOutstanding << std::endl
-            << std::endl
+            << "\tManagement Fee: " << props.managementFeeOwedToBroker
-            << "\tManagement Fee: " << props.loanState.managementFeeDue
             << std::endl
             << "\tLoan Scale: " << props.loanScale << std::endl
             << "\tFirst payment principal: " << props.firstPaymentPrincipal
@@ -1486,9 +1485,9 @@ protected:
             startDate + *loanParams.payInterval,
             *loanParams.payTotal,
             state.loanScale,
-            loanProperties.loanState.valueOutstanding,
+            loanProperties.totalValueOutstanding,
             principalRequestAmount,
-            loanProperties.loanState.managementFeeDue,
+            loanProperties.managementFeeOwedToBroker,
             loanProperties.periodicPayment,
             loanFlags | 0);
 
@@ -1543,9 +1542,9 @@ protected:
             nextDueDate,
             *loanParams.payTotal,
             loanProperties.loanScale,
-            loanProperties.loanState.valueOutstanding,
+            loanProperties.totalValueOutstanding,
             principalRequestAmount,
-            loanProperties.loanState.managementFeeDue,
+            loanProperties.managementFeeOwedToBroker,
             loanProperties.periodicPayment,
             loanFlags | 0);
 
@@ -7029,6 +7028,140 @@ 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
@@ -7077,6 +7210,8 @@ public:
         testBorrowerIsBroker();
         testIssuerIsBorrower();
         testLimitExceeded();
+        testLoanPayBrokerOwnerMissingTrustline();
+        testLoanPayBrokerOwnerUnauthorizedMPT();
     }
 };
 

src/xrpld/app/misc/LendingHelpers.h

@@ -84,6 +84,46 @@ 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
@@ -121,39 +161,6 @@ 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.
@@ -361,18 +368,6 @@ 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);
 
@@ -451,22 +446,13 @@ operator+(LoanState const& lhs, detail::LoanStateDeltas const& rhs);
 LoanProperties
 computeLoanProperties(
     Asset const& asset,
-    Number const& principalOutstanding,
+    Number 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,8 +2,6 @@
 // DO NOT REMOVE forces header file include to sort first
 #include <xrpld/app/tx/detail/VaultCreate.h>
 
-#include <utility>
-
 namespace ripple {
 
 bool
@@ -384,15 +382,21 @@ doPayment(
  * The function preserves accumulated rounding errors across the re-amortization
  * to ensure the loan state remains consistent with its payment history.
  */
-Expected<std::pair<LoanPaymentParts, LoanProperties>, TER>
+Expected<LoanPaymentParts, TER>
 tryOverpayment(
     Asset const& asset,
     std::int32_t loanScale,
     ExtendedPaymentComponents const& overpaymentComponents,
-    LoanState const& roundedOldState,
+    Number& totalValueOutstanding,
-    Number const& periodicPayment,
+    Number& principalOutstanding,
+    Number& managementFeeOutstanding,
+    Number& periodicPayment,
+    TenthBips32 interestRate,
+    std::uint32_t paymentInterval,
     Number const& periodicRate,
     std::uint32_t paymentRemaining,
+    std::uint32_t prevPaymentDate,
+    std::optional<std::uint32_t> nextDueDate,
     TenthBips16 const managementFeeRate,
     beast::Journal j)
 {
@@ -400,11 +404,15 @@ 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 = roundedOldState - raw;
+    auto const errors = rounded - raw;
 
     // Compute the new principal by applying the overpayment to the raw
     // (theoretical) principal. Use max with 0 to ensure we never go negative.
@@ -418,7 +426,8 @@ tryOverpayment(
     auto newLoanProperties = computeLoanProperties(
         asset,
         newRawPrincipal,
-        periodicRate,
+        interestRate,
+        paymentInterval,
         paymentRemaining,
         managementFeeRate,
         loanScale);
@@ -426,7 +435,7 @@ tryOverpayment(
     JLOG(j.debug()) << "new periodic payment: "
                     << newLoanProperties.periodicPayment
                     << ", new total value: "
-                    << newLoanProperties.loanState.valueOutstanding
+                    << newLoanProperties.totalValueOutstanding
                     << ", first payment principal: "
                     << newLoanProperties.firstPaymentPrincipal;
 
@@ -445,35 +454,37 @@ tryOverpayment(
     // rounding errors. This ensures the loan's tracked state remains
     // consistent with its payment history.
 
-    auto const principalOutstanding = std::clamp(
+    principalOutstanding = std::clamp(
         roundToAsset(
             asset, newRaw.principalOutstanding, loanScale, Number::upward),
         numZero,
-        roundedOldState.principalOutstanding);
+        rounded.principalOutstanding);
-    auto const totalValueOutstanding = std::clamp(
+    totalValueOutstanding = std::clamp(
         roundToAsset(
             asset,
             principalOutstanding + newRaw.interestOutstanding(),
             loanScale,
             Number::upward),
         numZero,
-        roundedOldState.valueOutstanding);
+        rounded.valueOutstanding);
-    auto const managementFeeOutstanding = std::clamp(
+    managementFeeOutstanding = std::clamp(
         roundToAsset(asset, newRaw.managementFeeDue, loanScale),
         numZero,
-        roundedOldState.managementFeeDue);
+        rounded.managementFeeDue);
 
-    auto const roundedNewState = constructLoanState(
+    auto const newRounded = constructLoanState(
         totalValueOutstanding, principalOutstanding, managementFeeOutstanding);
 
     // Update newLoanProperties so that checkLoanGuards can make an accurate
     // evaluation.
-    newLoanProperties.loanState = roundedNewState;
+    newLoanProperties.totalValueOutstanding = newRounded.valueOutstanding;
 
-    JLOG(j.debug()) << "new rounded value: " << roundedNewState.valueOutstanding
+    JLOG(j.debug()) << "new rounded value: " << newRounded.valueOutstanding
-                    << ", principal: " << roundedNewState.principalOutstanding
+                    << ", principal: " << newRounded.principalOutstanding
-                    << ", interest gross: "
+                    << ", interest gross: " << newRounded.interestOutstanding();
-                    << roundedNewState.interestOutstanding();
+
+    // Update the periodic payment to reflect the re-amortized schedule
+    periodicPayment = newLoanProperties.periodicPayment;
 
     // check that the loan is still valid
     if (auto const ter = checkLoanGuards(
@@ -483,7 +494,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.
-            roundedNewState.interestOutstanding() != beast::zero,
+            newRounded.interestOutstanding() != beast::zero,
             paymentRemaining,
             newLoanProperties,
             j))
@@ -497,64 +508,61 @@ 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.loanState.valueOutstanding <= 0 ||
+        newLoanProperties.totalValueOutstanding <= 0 ||
-        newLoanProperties.loanState.managementFeeDue < 0)
+        newLoanProperties.managementFeeOwedToBroker < 0)
     {
         // LCOV_EXCL_START
         JLOG(j.warn()) << "Overpayment not allowed: Computed loan "
                           "properties are invalid. Does "
                           "not compute. TotalValueOutstanding: "
-                       << newLoanProperties.loanState.valueOutstanding
+                       << newLoanProperties.totalValueOutstanding
                        << ", PeriodicPayment : "
                        << newLoanProperties.periodicPayment
                        << ", ManagementFeeOwedToBroker: "
-                       << newLoanProperties.loanState.managementFeeDue;
+                       << newLoanProperties.managementFeeOwedToBroker;
         return Unexpected(tesSUCCESS);
         // LCOV_EXCL_STOP
     }
 
-    auto const deltas = roundedOldState - roundedNewState;
+    auto const deltas = rounded - newRounded;
 
     // 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 ==
-            roundedOldState.managementFeeDue - managementFeeOutstanding,
+            rounded.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 = -deltas.interest;
+    auto const valueChange =
+        newRounded.valueOutstanding - hypotheticalValueOutstanding;
     if (valueChange > 0)
     {
         JLOG(j.warn()) << "Principal overpayment would increase the value of "
                           "the loan. Ignore the overpayment";
         return Unexpected(tesSUCCESS);
     }
-
+    return LoanPaymentParts{
-    return std::make_pair(
+        // Principal paid is the reduction in principal outstanding
-        LoanPaymentParts{
+        .principalPaid = deltas.principal,
-            // Principal paid is the reduction in principal outstanding
+        // Interest paid is the reduction in interest due
-            .principalPaid = deltas.principal,
+        .interestPaid =
-            // Interest paid is the reduction in interest due
+            deltas.interest + overpaymentComponents.untrackedInterest,
-            .interestPaid =
+        // Value change includes both the reduction from paying down principal
-                deltas.interest + overpaymentComponents.untrackedInterest,
+        // (negative) and any untracked interest penalties (positive, e.g., if
-            // Value change includes both the reduction from paying down
+        // the overpayment itself incurs a fee)
-            // principal
+        .valueChange =
-            // (negative) and any untracked interest penalties (positive, e.g.,
+            valueChange + overpaymentComponents.trackedInterestPart(),
-            // if
+        // Fee paid includes both the reduction in tracked management fees and
-            // the overpayment itself incurs a fee)
+        // any untracked fees on the overpayment itself
-            .valueChange =
+        .feePaid = deltas.managementFee +
-                valueChange + overpaymentComponents.trackedInterestPart(),
+            overpaymentComponents.untrackedManagementFee};
-            // 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.
@@ -578,16 +586,23 @@ 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)
 {
-    auto const loanState = constructLoanState(
+    // Create temporary copies of the loan state that can be safely modified
-        totalValueOutstandingProxy,
+    // and discarded if the overpayment doesn't work out. This prevents
-        principalOutstandingProxy,
+    // corrupting the actual ledger data if validation fails.
-        managementFeeOutstandingProxy);
+    Number totalValueOutstanding = totalValueOutstandingProxy;
-    auto const periodicPayment = periodicPaymentProxy;
+    Number principalOutstanding = principalOutstandingProxy;
+    Number managementFeeOutstanding = managementFeeOutstandingProxy;
+    Number periodicPayment = periodicPaymentProxy;
+
     JLOG(j.debug())
         << "overpayment components:"
         << ", totalValue before: " << *totalValueOutstandingProxy
@@ -606,28 +621,33 @@ doOverpayment(
         asset,
         loanScale,
         overpaymentComponents,
-        loanState,
+        totalValueOutstanding,
+        principalOutstanding,
+        managementFeeOutstanding,
         periodicPayment,
+        interestRate,
+        paymentInterval,
         periodicRate,
         paymentRemaining,
+        prevPaymentDate,
+        nextDueDate,
         managementFeeRate,
         j);
     if (!ret)
         return Unexpected(ret.error());
 
-    auto const& [loanPaymentParts, newLoanProperties] = *ret;
+    auto const& loanPaymentParts = *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 <= newRoundedLoanState.principalOutstanding)
+    if (principalOutstandingProxy <= principalOutstanding)
     {
         // LCOV_EXCL_START
         JLOG(j.warn()) << "Overpayment not allowed: principal "
                        << "outstanding did not decrease. Before: "
-                       << *principalOutstandingProxy << ". After: "
+                       << *principalOutstandingProxy
-                       << newRoundedLoanState.principalOutstanding;
+                       << ". After: " << principalOutstanding;
         return Unexpected(tesSUCCESS);
         // LCOV_EXCL_STOP
     }
@@ -638,29 +658,28 @@ doOverpayment(
 
     XRPL_ASSERT_PARTS(
         overpaymentComponents.trackedPrincipalDelta ==
-            principalOutstandingProxy -
+            principalOutstandingProxy - principalOutstanding,
-                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())
+    JLOG(j.debug()) << "valueChange: " << loanPaymentParts.valueChange
-        << "valueChange: " << loanPaymentParts.valueChange
+                    << ", totalValue before: " << *totalValueOutstandingProxy
-        << ", totalValue before: " << *totalValueOutstandingProxy
+                    << ", totalValue after: " << totalValueOutstanding
-        << ", totalValue after: " << newRoundedLoanState.valueOutstanding
+                    << ", totalValue delta: "
-        << ", totalValue delta: "
+                    << (totalValueOutstandingProxy - totalValueOutstanding)
-        << (totalValueOutstandingProxy - newRoundedLoanState.valueOutstanding)
+                    << ", principalDelta: "
-        << ", principalDelta: " << overpaymentComponents.trackedPrincipalDelta
+                    << overpaymentComponents.trackedPrincipalDelta
-        << ", principalPaid: " << loanPaymentParts.principalPaid
+                    << ", principalPaid: " << loanPaymentParts.principalPaid
-        << ", Computed difference: "
+                    << ", Computed difference: "
-        << overpaymentComponents.trackedPrincipalDelta -
+                    << overpaymentComponents.trackedPrincipalDelta -
-            (totalValueOutstandingProxy - newRoundedLoanState.valueOutstanding);
+            (totalValueOutstandingProxy - totalValueOutstanding);
 
     XRPL_ASSERT_PARTS(
         loanPaymentParts.valueChange ==
-            newRoundedLoanState.valueOutstanding -
+            totalValueOutstanding -
                 (totalValueOutstandingProxy -
                  overpaymentComponents.trackedPrincipalDelta) +
                 overpaymentComponents.trackedInterestPart(),
@@ -675,10 +694,10 @@ doOverpayment(
 
     // All validations passed, so update the proxy objects (which will
     // modify the actual Loan ledger object)
-    totalValueOutstandingProxy = newRoundedLoanState.valueOutstanding;
+    totalValueOutstandingProxy = totalValueOutstanding;
-    principalOutstandingProxy = newRoundedLoanState.principalOutstanding;
+    principalOutstandingProxy = principalOutstanding;
-    managementFeeOutstandingProxy = newRoundedLoanState.managementFeeDue;
+    managementFeeOutstandingProxy = managementFeeOutstanding;
-    periodicPaymentProxy = newLoanProperties.periodicPayment;
+    periodicPaymentProxy = periodicPayment;
 
     return loanPaymentParts;
 }
@@ -1271,7 +1290,7 @@ checkLoanGuards(
     beast::Journal j)
 {
     auto const totalInterestOutstanding =
-        properties.loanState.valueOutstanding - principalRequested;
+        properties.totalValueOutstanding - 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
@@ -1326,13 +1345,13 @@ checkLoanGuards(
         NumberRoundModeGuard mg(Number::upward);
 
         if (std::int64_t const computedPayments{
-                properties.loanState.valueOutstanding / roundedPayment};
+                properties.totalValueOutstanding / roundedPayment};
             computedPayments != paymentTotal)
         {
             JLOG(j.warn()) << "Loan Periodic payment ("
                            << properties.periodicPayment << ") rounding ("
                            << roundedPayment << ") on a total value of "
-                           << properties.loanState.valueOutstanding
+                           << properties.totalValueOutstanding
                            << " can not complete the loan in the specified "
                               "number of payments ("
                            << computedPayments << " != " << paymentTotal << ")";
@@ -1515,7 +1534,7 @@ computeManagementFee(
 LoanProperties
 computeLoanProperties(
     Asset const& asset,
-    Number const& principalOutstanding,
+    Number principalOutstanding,
     TenthBips32 interestRate,
     std::uint32_t paymentInterval,
     std::uint32_t paymentsRemaining,
@@ -1526,24 +1545,7 @@ 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);
 
@@ -1577,12 +1579,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.
-    auto const roundedPrincipalOutstanding = roundToAsset(
+    principalOutstanding = roundToAsset(
         asset, principalOutstanding, loanScale, Number::to_nearest);
 
     // E<quation (31) from XLS-66 spec, Section A-2 Equation Glossary
     auto const totalInterestOutstanding =
-        totalValueOutstanding - roundedPrincipalOutstanding;
+        totalValueOutstanding - principalOutstanding;
     auto const feeOwedToBroker = computeManagementFee(
         asset, totalInterestOutstanding, managementFeeRate, loanScale);
 
@@ -1612,13 +1614,10 @@ computeLoanProperties(
 
     return LoanProperties{
         .periodicPayment = periodicPayment,
-        .loanState = constructLoanState(
+        .totalValueOutstanding = totalValueOutstanding,
-            totalValueOutstanding,
+        .managementFeeOwedToBroker = feeOwedToBroker,
-            roundedPrincipalOutstanding,
-            feeOwedToBroker),
         .loanScale = loanScale,
-        .firstPaymentPrincipal = firstPaymentPrincipal,
+        .firstPaymentPrincipal = firstPaymentPrincipal};
-    };
 }
 
 /*
@@ -1947,8 +1946,12 @@ loanMakePayment(
                     principalOutstandingProxy,
                     managementFeeOutstandingProxy,
                     periodicPaymentProxy,
+                    interestRate,
+                    paymentInterval,
                     periodicRate,
                     paymentRemainingProxy,
+                    prevPaymentDateProxy,
+                    nextDueDateProxy,
                     managementFeeRate,
                     j))
                 totalParts += *overResult;

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

@@ -262,9 +262,10 @@ 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. If not, so as not to block the
+    // _and_ if the owner can receive funds
-    // payment, add it to the cover balance (send it to the broker pseudo
+    // _and_ if the broker is authorized to hold funds. If not, so as not to
-    // account).
+    // 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.
@@ -278,7 +279,9 @@ LoanPay::doApply()
                    asset,
                    tenthBipsOfValue(debtTotalProxy.value(), coverRateMinimum),
                    loanScale) &&
-            !isDeepFrozen(view, brokerOwner, asset);
+            !isDeepFrozen(view, brokerOwner, asset) &&
+            requireAuth(view, asset, brokerOwner, AuthType::StrongAuth) ==
+            tesSUCCESS;
     }();
 
     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.loanState.valueOutstanding << " with a scale of "
+                    << properties.totalValueOutstanding << " 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.loanState.managementFeeDue < 0 ||
+    if (properties.managementFeeOwedToBroker < 0 ||
-        properties.loanState.valueOutstanding <= 0 ||
+        properties.totalValueOutstanding <= 0 ||
         properties.periodicPayment <= 0)
     {
         // LCOV_EXCL_START
@@ -446,9 +446,9 @@ LoanSet::doApply()
     }
 
     LoanState const state = constructLoanState(
-        properties.loanState.valueOutstanding,
+        properties.totalValueOutstanding,
         principalRequested,
-        properties.loanState.managementFeeDue);
+        properties.managementFeeOwedToBroker);
 
     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.loanState.valueOutstanding;
+    loan->at(sfTotalValueOutstanding) = properties.totalValueOutstanding;
-    loan->at(sfManagementFeeOutstanding) = properties.loanState.managementFeeDue;
+    loan->at(sfManagementFeeOutstanding) = properties.managementFeeOwedToBroker;
     loan->at(sfPreviousPaymentDate) = 0;
     loan->at(sfNextPaymentDueDate) = startDate + paymentInterval;
     loan->at(sfPaymentRemaining) = paymentTotal;