Merge branch 'ximinez/lending-XLS-66' into tapanito/lending-delete-freeze

src/test/app/Loan_test.cpp

@@ -83,6 +83,7 @@ protected:
         TenthBips16 managementFeeRate{100};
         TenthBips32 coverRateLiquidation = percentageToTenthBips(25);
         std::string data{};
+        std::uint32_t flags = 0;
 
         Number
         maxCoveredLoanValue(Number const& currentDebt) const
@@ -223,6 +224,22 @@ protected:
         }
     };
 
+    struct PaymentParameters
+    {
+        Number overpaymentFactor = Number{1};
+        std::optional<Number> overpaymentExtra = std::nullopt;
+        std::uint32_t flags = 0;
+        bool showStepBalances = false;
+        bool validateBalances = true;
+
+        static PaymentParameters const&
+        defaults()
+        {
+            static PaymentParameters const result{};
+            return result;
+        }
+    };
+
     struct LoanState
     {
         std::uint32_t previousPaymentDate = 0;
@@ -465,7 +482,7 @@ protected:
         auto const keylet = keylet::loanbroker(lender.id(), env.seq(lender));
 
         using namespace loanBroker;
-        env(set(lender, vaultKeylet.key),
+        env(set(lender, vaultKeylet.key, params.flags),
             data(params.data),
             managementFeeRate(params.managementFeeRate),
             debtMaximum(debtMaximumValue),
@@ -700,10 +717,10 @@ protected:
             << std::endl;
 
         // checkGuards returns a TER, so success is 0
-        BEAST_EXPECT(!LoanSet::checkGuards(
+        BEAST_EXPECT(!checkLoanGuards(
             asset,
             asset(loanParams.principalRequest).number(),
-            loanParams.interest.value_or(TenthBips32{}),
+            loanParams.interest.value_or(TenthBips32{}) != beast::zero,
             loanParams.payTotal.value_or(LoanSet::defaultPaymentTotal),
             props,
             env.journal));
@@ -835,7 +852,7 @@ protected:
         jtx::Account const& issuer,
         jtx::Account const& lender,
         jtx::Account const& borrower,
-        bool showStepBalances = false)
+        PaymentParameters const& paymentParams = PaymentParameters::defaults())
     {
         // Make all the individual payments
         using namespace jtx;
@@ -846,6 +863,8 @@ protected:
         // Account const evan{"evan"};
         // Account const alice{"alice"};
 
+        bool const showStepBalances = paymentParams.showStepBalances;
+
         auto const currencyLabel = getCurrencyLabel(broker.asset);
 
         auto const baseFee = env.current()->fees().base;
@@ -891,24 +910,25 @@ protected:
             state.loanScale,
             Number::upward);
 
+        auto currentRoundedState = constructRoundedLoanState(
+            state.totalValue,
+            state.principalOutstanding,
+            state.managementFeeOutstanding);
         {
             auto const raw = calculateRawLoanState(
                 state.periodicPayment,
                 periodicRate,
                 state.paymentRemaining,
                 broker.params.managementFeeRate);
-            auto const rounded = constructRoundedLoanState(
-                state.totalValue,
-                state.principalOutstanding,
-                state.managementFeeOutstanding);
 
             if (showStepBalances)
             {
                 log << currencyLabel << " Starting loan balances: "
-                    << "\n\tTotal value: " << rounded.valueOutstanding
+                    << "\n\tTotal value: "
-                    << "\n\tPrincipal: " << rounded.principalOutstanding
+                    << currentRoundedState.valueOutstanding << "\n\tPrincipal: "
-                    << "\n\tInterest: " << rounded.interestDue
+                    << currentRoundedState.principalOutstanding
-                    << "\n\tMgmt fee: " << rounded.managementFeeDue
+                    << "\n\tInterest: " << currentRoundedState.interestDue
+                    << "\n\tMgmt fee: " << currentRoundedState.managementFeeDue
                     << "\n\tPayments remaining " << state.paymentRemaining
                     << std::endl;
             }
@@ -918,18 +938,24 @@ protected:
                     << " Loan starting state: " << state.paymentRemaining
                     << ", " << raw.interestDue << ", "
                     << raw.principalOutstanding << ", " << raw.managementFeeDue
-                    << ", " << rounded.valueOutstanding << ", "
+                    << ", " << currentRoundedState.valueOutstanding << ", "
-                    << rounded.principalOutstanding << ", "
+                    << currentRoundedState.principalOutstanding << ", "
-                    << rounded.interestDue << ", " << rounded.managementFeeDue
+                    << currentRoundedState.interestDue << ", "
-                    << std::endl;
+                    << currentRoundedState.managementFeeDue << std::endl;
             }
         }
 
         // Try to pay a little extra to show that it's _not_
         // taken
-        STAmount const transactionAmount = STAmount{broker.asset, totalDue} +
+        auto const extraAmount = paymentParams.overpaymentExtra
-            std::min(broker.asset(10).value(),
+            ? broker.asset(*paymentParams.overpaymentExtra).value()
-                     STAmount{broker.asset, totalDue / 20});
+            : std::min(
+                  broker.asset(10).value(),
+                  STAmount{broker.asset, totalDue / 20});
+
+        STAmount const transactionAmount =
+            STAmount{broker.asset, totalDue * paymentParams.overpaymentFactor} +
+            extraAmount;
 
         auto const borrowerInitialBalance =
             env.balance(borrower, broker.asset).number();
@@ -949,7 +975,7 @@ protected:
             broker.params.managementFeeRate);
 
         auto validateBorrowerBalance = [&]() {
-            if (borrower == issuer)
+            if (borrower == issuer || !paymentParams.validateBalances)
                 return;
             auto const totalSpent =
                 (totalPaid.trackedValueDelta + totalFeesPaid +
@@ -1035,54 +1061,64 @@ protected:
             auto const totalDueAmount = STAmount{
                 broker.asset, paymentComponents.trackedValueDelta + serviceFee};
 
-            // Due to the rounding algorithms to keep the interest and
+            if (paymentParams.validateBalances)
-            // principal in sync with "true" values, the computed amount
+            {
-            // may be a little less than the rounded fixed payment
+                // Due to the rounding algorithms to keep the interest and
-            // amount. For integral types, the difference should be < 3
+                // principal in sync with "true" values, the computed amount
-            // (1 unit for each of the interest and management fee). For
+                // may be a little less than the rounded fixed payment
-            // IOUs, the difference should be dust.
+                // amount. For integral types, the difference should be < 3
-            Number const diff = totalDue - totalDueAmount;
+                // (1 unit for each of the interest and management fee). For
-            BEAST_EXPECT(
+                // IOUs, the difference should be dust.
-                paymentComponents.specialCase ==
+                Number const diff = totalDue - totalDueAmount;
-                    detail::PaymentSpecialCase::final ||
+                BEAST_EXPECT(
-                diff == beast::zero ||
+                    paymentComponents.specialCase ==
-                (diff > beast::zero &&
+                        detail::PaymentSpecialCase::final ||
-                 ((broker.asset.integral() &&
+                    diff == beast::zero ||
-                   (static_cast<Number>(diff) < 3)) ||
+                    (diff > beast::zero &&
-                  (state.loanScale - diff.exponent() > 13))));
+                     ((broker.asset.integral() &&
+                       (static_cast<Number>(diff) < 3)) ||
+                      (state.loanScale - diff.exponent() > 13))));
 
-            BEAST_EXPECT(
+                BEAST_EXPECT(
-                paymentComponents.trackedPrincipalDelta >= beast::zero &&
+                    paymentComponents.trackedPrincipalDelta >= beast::zero &&
-                paymentComponents.trackedPrincipalDelta <=
+                    paymentComponents.trackedPrincipalDelta <=
-                    state.principalOutstanding);
+                        state.principalOutstanding);
-            BEAST_EXPECT(
+                BEAST_EXPECT(
-                paymentComponents.specialCase !=
+                    paymentComponents.specialCase !=
-                    detail::PaymentSpecialCase::final ||
+                        detail::PaymentSpecialCase::final ||
-                paymentComponents.trackedPrincipalDelta ==
+                    paymentComponents.trackedPrincipalDelta ==
-                    state.principalOutstanding);
+                        state.principalOutstanding);
+            }
 
             auto const borrowerBalanceBeforePayment =
                 env.balance(borrower, broker.asset);
 
             // Make the payment
-            env(pay(borrower, loanKeylet.key, transactionAmount));
+            env(
+                pay(borrower,
+                    loanKeylet.key,
+                    transactionAmount,
+                    paymentParams.flags));
 
             env.close(d{state.paymentInterval / 2});
 
-            // Need to account for fees if the loan is in XRP
+            if (paymentParams.validateBalances)
-            PrettyAmount adjustment = broker.asset(0);
-            if (broker.asset.native())
             {
-                adjustment = env.current()->fees().base;
+                // Need to account for fees if the loan is in XRP
-            }
+                PrettyAmount adjustment = broker.asset(0);
+                if (broker.asset.native())
+                {
+                    adjustment = env.current()->fees().base;
+                }
 
-            // Check the result
+                // Check the result
-            verifyLoanStatus.checkPayment(
+                verifyLoanStatus.checkPayment(
-                state.loanScale,
+                    state.loanScale,
-                borrower,
+                    borrower,
-                borrowerBalanceBeforePayment,
+                    borrowerBalanceBeforePayment,
-                totalDueAmount,
+                    totalDueAmount,
-                adjustment);
+                    adjustment);
+            }
 
             if (showStepBalances)
             {
@@ -1110,6 +1146,8 @@ protected:
                     << ", error: " << truncate(errors.managementFee)
                     << ")\n\tPayments remaining "
                     << loanSle->at(sfPaymentRemaining) << std::endl;
+
+                currentRoundedState = current;
             }
 
             --state.paymentRemaining;
@@ -1130,7 +1168,8 @@ protected:
                 paymentComponents.trackedManagementFeeDelta;
             state.totalValue -= paymentComponents.trackedValueDelta;
 
-            verifyLoanStatus(state);
+            if (paymentParams.validateBalances)
+                verifyLoanStatus(state);
 
             totalPaid.trackedValueDelta += paymentComponents.trackedValueDelta;
             totalPaid.trackedPrincipalDelta +=
@@ -1149,21 +1188,25 @@ protected:
         BEAST_EXPECT(state.paymentRemaining == 0);
         BEAST_EXPECT(state.principalOutstanding == 0);
 
-        // Make sure all the payments add up
-        BEAST_EXPECT(totalPaid.trackedValueDelta == initialState.totalValue);
-        BEAST_EXPECT(
-            totalPaid.trackedPrincipalDelta ==
-            initialState.principalOutstanding);
-        BEAST_EXPECT(
-            totalPaid.trackedManagementFeeDelta ==
-            initialState.managementFeeOutstanding);
-        // This is almost a tautology given the previous checks, but
-        // check it anyway for completeness.
         auto const initialInterestDue = initialState.totalValue -
             (initialState.principalOutstanding +
              initialState.managementFeeOutstanding);
-        BEAST_EXPECT(totalInterestPaid == initialInterestDue);
+        if (paymentParams.validateBalances)
-        BEAST_EXPECT(totalPaymentsMade == initialState.paymentRemaining);
+        {
+            // Make sure all the payments add up
+            BEAST_EXPECT(
+                totalPaid.trackedValueDelta == initialState.totalValue);
+            BEAST_EXPECT(
+                totalPaid.trackedPrincipalDelta ==
+                initialState.principalOutstanding);
+            BEAST_EXPECT(
+                totalPaid.trackedManagementFeeDelta ==
+                initialState.managementFeeOutstanding);
+            // This is almost a tautology given the previous checks, but
+            // check it anyway for completeness.
+            BEAST_EXPECT(totalInterestPaid == initialInterestDue);
+            BEAST_EXPECT(totalPaymentsMade == initialState.paymentRemaining);
+        }
 
         if (showStepBalances)
         {
@@ -6514,7 +6557,7 @@ protected:
             issuer,
             lender,
             borrower,
-            true);
+            PaymentParameters{.showStepBalances = true});
 
         if (auto const brokerSle = env.le(broker.brokerKeylet());
             BEAST_EXPECT(brokerSle))
@@ -6640,7 +6683,7 @@ protected:
                 env.le(keylet::loanbroker(brokerInfo.brokerID));
             BEAST_EXPECT(brokerSle))
         {
-            std::cout << *brokerSle << std::endl;
+            log << *brokerSle << std::endl;
             BEAST_EXPECT(brokerSle->at(sfDebtTotal) == Number(804));
         }
 
@@ -6662,7 +6705,7 @@ protected:
                 env.le(keylet::loanbroker(brokerInfo.brokerID));
             BEAST_EXPECT(brokerSle))
         {
-            std::cout << *brokerSle << std::endl;
+            log << *brokerSle << std::endl;
             BEAST_EXPECT(
                 brokerSle->at(sfCoverAvailable) == xrpAsset(81).value());
             BEAST_EXPECT(brokerSle->at(sfDebtTotal) == Number(804));
@@ -6670,8 +6713,7 @@ protected:
             // Also demonstrate that the true minimum (804 * 10%) exceeds 80
             auto const theoreticalMin =
                 tenthBipsOfValue(Number(804), TenthBips32(10'000));
-            std::cout << "Theoretical min cover: " << theoreticalMin
+            log << "Theoretical min cover: " << theoreticalMin << std::endl;
-                      << std::endl;
             BEAST_EXPECT(Number(804, -1) == theoreticalMin);
         }
     }
@@ -6727,7 +6769,7 @@ protected:
             issuer,
             lender,
             borrower,
-            true);
+            PaymentParameters{.showStepBalances = true});
     }
 
     void
@@ -6901,7 +6943,85 @@ protected:
             issuer,
             lender,
             issuer,
-            true);
+            PaymentParameters{.showStepBalances = true});
+    }
+
+    void
+    testLimitExceeded()
+    {
+        testcase("RIPD-4125 - overpayment");
+
+        using namespace jtx;
+
+        Account const issuer("issuer");
+        Account const lender("lender");
+        Account const borrower("borrower");
+
+        BrokerParameters const brokerParams{
+            .vaultDeposit = 100'000,
+            .debtMax = 0,
+            .coverRateMin = TenthBips32{0},
+            .managementFeeRate = TenthBips16{0},
+            .coverRateLiquidation = TenthBips32{0}};
+        LoanParameters const loanParams{
+            .account = lender,
+            .counter = borrower,
+            .principalRequest = Number{200000, -6},
+            .interest = TenthBips32{50000},
+            .payTotal = 3,
+            .payInterval = 200,
+            .gracePd = 60,
+            .flags = tfLoanOverpayment,
+        };
+
+        auto const assetType = AssetType::XRP;
+
+        Env env(
+            *this,
+            makeConfig(),
+            all,
+            nullptr,
+            beast::severities::Severity::kWarning);
+
+        auto loanResult = createLoan(
+            env, assetType, brokerParams, loanParams, issuer, lender, borrower);
+
+        if (!BEAST_EXPECT(loanResult))
+            return;
+
+        auto broker = std::get<BrokerInfo>(*loanResult);
+        auto loanKeylet = std::get<Keylet>(*loanResult);
+        auto pseudoAcct = std::get<Account>(*loanResult);
+
+        VerifyLoanStatus verifyLoanStatus(env, broker, pseudoAcct, loanKeylet);
+
+        auto const state = getCurrentState(env, broker, loanKeylet);
+
+        env(loan::pay(
+            borrower,
+            loanKeylet.key,
+            STAmount{broker.asset, state.periodicPayment * 3 / 2 + 1},
+            tfLoanOverpayment));
+        env.close();
+
+        PaymentParameters paymentParams{
+            //.overpaymentFactor = Number{15, -1},
+            //.overpaymentExtra = Number{1, -6},
+            //.flags = tfLoanOverpayment,
+            .showStepBalances = true,
+            //.validateBalances = false,
+        };
+
+        makeLoanPayments(
+            env,
+            broker,
+            loanParams,
+            loanKeylet,
+            verifyLoanStatus,
+            issuer,
+            lender,
+            borrower,
+            paymentParams);
     }
 
 public:
@@ -6951,6 +7071,7 @@ public:
         testRoundingAllowsUndercoverage();
         testBorrowerIsBroker();
         testIssuerIsBorrower();
+        testLimitExceeded();
     }
 };
 

src/xrpld/app/misc/LendingHelpers.h

@@ -102,6 +102,15 @@ struct LoanState
     }
 };
 
+TER
+checkLoanGuards(
+    Asset const& vaultAsset,
+    Number const& principalRequested,
+    bool expectInterest,
+    std::uint32_t paymentTotal,
+    LoanProperties const& properties,
+    beast::Journal j);
+
 LoanState
 calculateRawLoanState(
     Number const& periodicPayment,
@@ -217,6 +226,9 @@ operator-(LoanState const& lhs, LoanState const& rhs);
 LoanState
 operator-(LoanState const& lhs, detail::LoanDeltas const& rhs);
 
+LoanState
+operator+(LoanState const& lhs, detail::LoanDeltas const& rhs);
+
 LoanProperties
 computeLoanProperties(
     Asset const& asset,

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

@@ -372,9 +372,7 @@ tryOverpayment(
     auto const rounded = constructRoundedLoanState(
         totalValueOutstanding, principalOutstanding, managementFeeOutstanding);
 
-    auto const totalValueError = totalValueOutstanding - raw.valueOutstanding;
+    auto const errors = rounded - raw;
-    auto const principalError = principalOutstanding - raw.principalOutstanding;
-    auto const feeError = managementFeeOutstanding - raw.managementFeeDue;
 
     auto const newRawPrincipal = std::max(
         raw.principalOutstanding - overpaymentComponents.trackedPrincipalDelta,
@@ -389,33 +387,68 @@ tryOverpayment(
         managementFeeRate,
         loanScale);
 
-    auto const newRaw = calculateRawLoanState(
+    JLOG(j.debug()) << "new periodic payment: "
-        newLoanProperties.periodicPayment,
+                    << newLoanProperties.periodicPayment
-        periodicRate,
+                    << ", new total value: "
-        paymentRemaining,
+                    << newLoanProperties.totalValueOutstanding
-        managementFeeRate);
+                    << ", first payment principal: "
+                    << newLoanProperties.firstPaymentPrincipal;
 
-    totalValueOutstanding = roundToAsset(
+    auto const newRaw = calculateRawLoanState(
-        asset, newRaw.valueOutstanding + totalValueError, loanScale);
+                            newLoanProperties.periodicPayment,
-    principalOutstanding = roundToAsset(
+                            periodicRate,
-        asset,
+                            paymentRemaining,
-        newRaw.principalOutstanding + principalError,
+                            managementFeeRate) +
-        loanScale,
+        errors;
-        Number::downward);
+
-    managementFeeOutstanding =
+    JLOG(j.debug()) << "new raw value: " << newRaw.valueOutstanding
-        roundToAsset(asset, newRaw.managementFeeDue + feeError, loanScale);
+                    << ", principal: " << newRaw.principalOutstanding
+                    << ", interest gross: " << newRaw.interestOutstanding();
+
+    principalOutstanding = std::clamp(
+        roundToAsset(
+            asset, newRaw.principalOutstanding, loanScale, Number::upward),
+        numZero,
+        rounded.principalOutstanding);
+    totalValueOutstanding = std::clamp(
+        roundToAsset(
+            asset,
+            principalOutstanding + newRaw.interestOutstanding(),
+            loanScale,
+            Number::upward),
+        numZero,
+        rounded.valueOutstanding);
+    managementFeeOutstanding = std::clamp(
+        roundToAsset(asset, newRaw.managementFeeDue, loanScale),
+        numZero,
+        rounded.managementFeeDue);
+
+    auto const newRounded = constructRoundedLoanState(
+        totalValueOutstanding, principalOutstanding, managementFeeOutstanding);
+
+    newLoanProperties.totalValueOutstanding = newRounded.valueOutstanding;
+
+    JLOG(j.debug()) << "new rounded value: " << newRounded.valueOutstanding
+                    << ", principal: " << newRounded.principalOutstanding
+                    << ", interest gross: " << newRounded.interestOutstanding();
 
     periodicPayment = newLoanProperties.periodicPayment;
 
     // check that the loan is still valid
-    if (newLoanProperties.firstPaymentPrincipal <= 0 &&
+    if (auto const ter = checkLoanGuards(
-        principalOutstanding > 0)
+            asset,
+            principalOutstanding,
+            // The loan may have been created with interest, but for
+            // 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,
+            paymentRemaining,
+            newLoanProperties,
+            j))
     {
-        // The overpayment has caused the loan to be in a state
+        JLOG(j.warn()) << "Principal overpayment would cause the loan to be in "
-        // where no further principal can be paid.
+                          "an invalid state. Ignore the overpayment";
-        JLOG(j.warn())
-            << "Loan overpayment would cause loan to be stuck. "
-               "Rejecting overpayment, but normal payments are unaffected.";
         return Unexpected(tesSUCCESS);
     }
 
@@ -437,21 +470,27 @@ tryOverpayment(
         // LCOV_EXCL_STOP
     }
 
-    auto const newRounded = constructRoundedLoanState(
+    auto const deltas = rounded - newRounded;
-        totalValueOutstanding, principalOutstanding, managementFeeOutstanding);
+
+    auto const hypotheticalValueOutstanding =
+        rounded.valueOutstanding - deltas.principal;
+
     auto const valueChange =
-        newRounded.interestOutstanding() - rounded.interestOutstanding();
+        newRounded.valueOutstanding - hypotheticalValueOutstanding;
-    XRPL_ASSERT_PARTS(
+    if (valueChange > 0)
-        valueChange <= beast::zero,
+    {
-        "ripple::detail::tryOverpayment",
+        JLOG(j.warn()) << "Principal overpayment would increase the value of "
-        "principal overpayment did not increase value of loan");
+                          "the loan. Ignore the overpayment";
+        return Unexpected(tesSUCCESS);
+    }
 
     return LoanPaymentParts{
-        .principalPaid =
+        .principalPaid = deltas.principal,
-            rounded.principalOutstanding - newRounded.principalOutstanding,
+        .interestPaid =
-        .interestPaid = rounded.interestDue - newRounded.interestDue,
+            deltas.interest + overpaymentComponents.untrackedInterest,
-        .valueChange = valueChange + overpaymentComponents.untrackedInterest,
+        .valueChange =
-        .feePaid = rounded.managementFeeDue - newRounded.managementFeeDue +
+            valueChange + overpaymentComponents.trackedInterestPart(),
+        .feePaid = deltas.managementFee +
             overpaymentComponents.untrackedManagementFee};
 }
 
@@ -481,6 +520,17 @@ doOverpayment(
     Number managementFeeOutstanding = managementFeeOutstandingProxy;
     Number periodicPayment = periodicPaymentProxy;
 
+    JLOG(j.debug())
+        << "overpayment components:"
+        << ", totalValue before: " << *totalValueOutstandingProxy
+        << ", valueDelta: " << overpaymentComponents.trackedValueDelta
+        << ", principalDelta: " << overpaymentComponents.trackedPrincipalDelta
+        << ", managementFeeDelta: "
+        << overpaymentComponents.trackedManagementFeeDelta
+        << ", interestPart: " << overpaymentComponents.trackedInterestPart()
+        << ", untrackedInterest: " << overpaymentComponents.untrackedInterest
+        << ", totalDue: " << overpaymentComponents.totalDue
+        << ", payments remaining :" << paymentRemaining;
     auto const ret = tryOverpayment(
         asset,
         loanScale,
@@ -527,12 +577,29 @@ doOverpayment(
         "ripple::detail::doOverpayment",
         "no fee change");
 
+    // 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);
+
     XRPL_ASSERT_PARTS(
-        overpaymentComponents.untrackedInterest ==
+        loanPaymentParts.valueChange ==
-            totalValueOutstandingProxy - totalValueOutstanding -
+            totalValueOutstanding -
-                overpaymentComponents.trackedPrincipalDelta,
+                (totalValueOutstandingProxy -
+                 overpaymentComponents.trackedPrincipalDelta) +
+                overpaymentComponents.trackedInterestPart(),
         "ripple::detail::doOverpayment",
-        "value change agrees");
+        "interest paid agrees");
 
     XRPL_ASSERT_PARTS(
         overpaymentComponents.trackedPrincipalDelta ==
@@ -995,11 +1062,9 @@ computeOverpaymentComponents(
     Number const fee = roundToAsset(
         asset, tenthBipsOfValue(overpayment, overpaymentFeeRate), loanScale);
 
-    Number const payment = overpayment - fee;
+    auto const [rawOverpaymentInterest, _] = [&]() {
-
-    auto const [rawOverpaymentInterest, rawOverpaymentManagementFee] = [&]() {
         Number const interest =
-            tenthBipsOfValue(payment, overpaymentInterestRate);
+            tenthBipsOfValue(overpayment, overpaymentInterestRate);
         return detail::computeInterestAndFeeParts(interest, managementFeeRate);
     }();
     auto const [roundedOverpaymentInterest, roundedOverpaymentManagementFee] =
@@ -1010,15 +1075,20 @@ computeOverpaymentComponents(
                 asset, interest, managementFeeRate, loanScale);
         }();
 
-    return detail::ExtendedPaymentComponents{
+    auto const result = detail::ExtendedPaymentComponents{
         detail::PaymentComponents{
-            .trackedValueDelta = payment,
+            .trackedValueDelta = overpayment - fee,
-            .trackedPrincipalDelta = payment - roundedOverpaymentInterest -
+            .trackedPrincipalDelta = overpayment - roundedOverpaymentInterest -
-                roundedOverpaymentManagementFee,
+                roundedOverpaymentManagementFee - fee,
             .trackedManagementFeeDelta = roundedOverpaymentManagementFee,
             .specialCase = detail::PaymentSpecialCase::extra},
         fee,
         roundedOverpaymentInterest};
+    XRPL_ASSERT_PARTS(
+        result.trackedInterestPart() == roundedOverpaymentInterest,
+        "ripple::detail::computeOverpaymentComponents",
+        "valid interest computation");
+    return result;
 }
 
 }  // namespace detail
@@ -1048,6 +1118,100 @@ operator-(LoanState const& lhs, detail::LoanDeltas const& rhs)
     return result;
 }
 
+LoanState
+operator+(LoanState const& lhs, detail::LoanDeltas const& rhs)
+{
+    LoanState result{
+        .valueOutstanding = lhs.valueOutstanding + rhs.total(),
+        .principalOutstanding = lhs.principalOutstanding + rhs.principal,
+        .interestDue = lhs.interestDue + rhs.interest,
+        .managementFeeDue = lhs.managementFeeDue + rhs.managementFee,
+    };
+
+    return result;
+}
+
+TER
+checkLoanGuards(
+    Asset const& vaultAsset,
+    Number const& principalRequested,
+    bool expectInterest,
+    std::uint32_t paymentTotal,
+    LoanProperties const& properties,
+    beast::Journal j)
+{
+    auto const totalInterestOutstanding =
+        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
+    if (expectInterest && totalInterestOutstanding <= 0)
+    {
+        // Unless this is a zero-interest loan, there must be some interest
+        // due on the loan, even if it's (measurable) dust
+        JLOG(j.warn()) << "Loan for " << principalRequested
+                       << " with interest has no interest due";
+        return tecPRECISION_LOSS;
+    }
+    // Guard 1a: If there is any interest computed over the life of the
+    // loan, for a zero interest rate, something went sideways.
+    if (!expectInterest && totalInterestOutstanding > 0)
+    {
+        // LCOV_EXCL_START
+        JLOG(j.warn()) << "Loan for " << principalRequested
+                       << " with no interest has interest due";
+        return tecINTERNAL;
+        // LCOV_EXCL_STOP
+    }
+
+    // Guard 2: if the principal portion of the first periodic payment is
+    // too small to be accurately represented with the given rounding mode,
+    // raise an error
+    if (properties.firstPaymentPrincipal <= 0)
+    {
+        // Check that some true (unrounded) principal is paid each period.
+        // Since the first payment pays the least principal, if it's good,
+        // they'll all be good. Note that the outstanding principal is
+        // rounded, and may not change right away.
+        JLOG(j.warn()) << "Loan is unable to pay principal.";
+        return tecPRECISION_LOSS;
+    }
+
+    // Guard 3: If the periodic payment is so small that it can't even be
+    // rounded to a representable value, then the loan can't be paid. Also,
+    // avoids dividing by 0.
+    auto const roundedPayment = roundPeriodicPayment(
+        vaultAsset, properties.periodicPayment, properties.loanScale);
+    if (roundedPayment == beast::zero)
+    {
+        JLOG(j.warn()) << "Loan Periodic payment ("
+                       << properties.periodicPayment << ") rounds to 0. ";
+        return tecPRECISION_LOSS;
+    }
+
+    // Guard 4: if the rounded periodic payment is large enough that the
+    // loan can't be amortized in the specified number of payments, raise an
+    // error
+    {
+        NumberRoundModeGuard mg(Number::upward);
+
+        if (std::int64_t const computedPayments{
+                properties.totalValueOutstanding / roundedPayment};
+            computedPayments != paymentTotal)
+        {
+            JLOG(j.warn()) << "Loan Periodic payment ("
+                           << properties.periodicPayment << ") rounding ("
+                           << roundedPayment << ") on a total value of "
+                           << properties.totalValueOutstanding
+                           << " can not complete the loan in the specified "
+                              "number of payments ("
+                           << computedPayments << " != " << paymentTotal << ")";
+            return tecPRECISION_LOSS;
+        }
+    }
+    return tesSUCCESS;
+}
+
 Number
 calculateFullPaymentInterest(
     Number const& rawPrincipalOutstanding,

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

@@ -361,6 +361,12 @@ LoanPay::doApply()
         // LCOV_EXCL_STOP
     }
 
+    JLOG(j_.debug()) << "Loan Pay: principal paid: "
+                     << paymentParts->principalPaid
+                     << ", interest paid: " << paymentParts->interestPaid
+                     << ", fee paid: " << paymentParts->feePaid
+                     << ", value change: " << paymentParts->valueChange;
+
     //------------------------------------------------------
     // LoanBroker object state changes
     view.update(brokerSle);
@@ -442,6 +448,12 @@ LoanPay::doApply()
         }
     }
 
+    JLOG(j_.debug()) << "total paid to vault raw: " << totalPaidToVaultRaw
+                     << ", total paid to vault rounded: "
+                     << totalPaidToVaultRounded
+                     << ", total paid to broker: " << totalPaidToBroker
+                     << ", amount from transaction: " << amount;
+
     // Move funds
     XRPL_ASSERT_PARTS(
         totalPaidToVaultRounded + totalPaidToBroker <= amount,

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

@@ -312,87 +312,6 @@ LoanSet::preclaim(PreclaimContext const& ctx)
     return tesSUCCESS;
 }
 
-TER
-LoanSet::checkGuards(
-    Asset const& vaultAsset,
-    Number const& principalRequested,
-    TenthBips32 interestRate,
-    std::uint32_t paymentTotal,
-    LoanProperties const& properties,
-    beast::Journal j)
-{
-    auto const totalInterestOutstanding =
-        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
-    if (interestRate > TenthBips32{0} && totalInterestOutstanding <= 0)
-    {
-        // Unless this is a zero-interest loan, there must be some interest
-        // due on the loan, even if it's (measurable) dust
-        JLOG(j.warn()) << "Loan for " << principalRequested << " with "
-                       << interestRate << "% interest has no interest due";
-        return tecPRECISION_LOSS;
-    }
-    // Guard 1a: If there is any interest computed over the life of the
-    // loan, for a zero interest rate, something went sideways.
-    if (interestRate == TenthBips32{0} && totalInterestOutstanding > 0)
-    {
-        // LCOV_EXCL_START
-        JLOG(j.warn()) << "Loan for " << principalRequested
-                       << " with 0% interest has interest due";
-        return tecINTERNAL;
-        // LCOV_EXCL_STOP
-    }
-
-    // Guard 2: if the principal portion of the first periodic payment is
-    // too small to be accurately represented with the given rounding mode,
-    // raise an error
-    if (properties.firstPaymentPrincipal <= 0)
-    {
-        // Check that some true (unrounded) principal is paid each period.
-        // Since the first payment pays the least principal, if it's good,
-        // they'll all be good. Note that the outstanding principal is
-        // rounded, and may not change right away.
-        JLOG(j.warn()) << "Loan is unable to pay principal.";
-        return tecPRECISION_LOSS;
-    }
-
-    // Guard 3: If the periodic payment is so small that it can't even be
-    // rounded to a representable value, then the loan can't be paid. Also,
-    // avoids dividing by 0.
-    auto const roundedPayment = roundPeriodicPayment(
-        vaultAsset, properties.periodicPayment, properties.loanScale);
-    if (roundedPayment == beast::zero)
-    {
-        JLOG(j.warn()) << "Loan Periodic payment ("
-                       << properties.periodicPayment << ") rounds to 0. ";
-        return tecPRECISION_LOSS;
-    }
-
-    // Guard 4: if the rounded periodic payment is large enough that the
-    // loan can't be amortized in the specified number of payments, raise an
-    // error
-    {
-        NumberRoundModeGuard mg(Number::upward);
-
-        if (std::int64_t const computedPayments{
-                properties.totalValueOutstanding / roundedPayment};
-            computedPayments != paymentTotal)
-        {
-            JLOG(j.warn()) << "Loan Periodic payment ("
-                           << properties.periodicPayment << ") rounding ("
-                           << roundedPayment << ") on a total value of "
-                           << properties.totalValueOutstanding
-                           << " can not complete the loan in the specified "
-                              "number of payments ("
-                           << computedPayments << " != " << paymentTotal << ")";
-            return tecPRECISION_LOSS;
-        }
-    }
-    return tesSUCCESS;
-}
-
 TER
 LoanSet::doApply()
 {
@@ -474,10 +393,10 @@ LoanSet::doApply()
         }
     }
 
-    if (auto const ret = checkGuards(
+    if (auto const ret = checkLoanGuards(
             vaultAsset,
             principalRequested,
-            interestRate,
+            interestRate != beast::zero,
             paymentTotal,
             properties,
             j_))

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

@@ -36,15 +36,6 @@ public:
     static TER
     preclaim(PreclaimContext const& ctx);
 
-    static TER
-    checkGuards(
-        Asset const& vaultAsset,
-        Number const& principalRequested,
-        TenthBips32 interestRate,
-        std::uint32_t paymentTotal,
-        LoanProperties const& properties,
-        beast::Journal j);
-
     TER
     doApply() override;