From 8a1f244682d7bb4554d7cc0b4801ecb2d4ea9a9f Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Fri, 14 Nov 2025 17:34:53 -0500 Subject: [PATCH] Updated overpayment processing - Added a test for RIPD-4125 --- src/test/app/Loan_test.cpp | 275 +++++++++++++------ src/xrpld/app/misc/LendingHelpers.h | 12 + src/xrpld/app/misc/detail/LendingHelpers.cpp | 260 ++++++++++++++---- src/xrpld/app/tx/detail/LoanPay.cpp | 12 + src/xrpld/app/tx/detail/LoanSet.cpp | 85 +----- src/xrpld/app/tx/detail/LoanSet.h | 9 - 6 files changed, 436 insertions(+), 217 deletions(-) diff --git a/src/test/app/Loan_test.cpp b/src/test/app/Loan_test.cpp index 1db46cf91d..fa4ed233b4 100644 --- a/src/test/app/Loan_test.cpp +++ b/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 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\tPrincipal: " << rounded.principalOutstanding - << "\n\tInterest: " << rounded.interestDue - << "\n\tMgmt fee: " << rounded.managementFeeDue + << "\n\tTotal value: " + << currentRoundedState.valueOutstanding << "\n\tPrincipal: " + << currentRoundedState.principalOutstanding + << "\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 << ", " - << rounded.principalOutstanding << ", " - << rounded.interestDue << ", " << rounded.managementFeeDue - << std::endl; + << ", " << currentRoundedState.valueOutstanding << ", " + << currentRoundedState.principalOutstanding << ", " + << currentRoundedState.interestDue << ", " + << currentRoundedState.managementFeeDue << std::endl; } } // Try to pay a little extra to show that it's _not_ // taken - STAmount const transactionAmount = STAmount{broker.asset, totalDue} + - std::min(broker.asset(10).value(), - STAmount{broker.asset, totalDue / 20}); + auto const extraAmount = paymentParams.overpaymentExtra + ? broker.asset(*paymentParams.overpaymentExtra).value() + : 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 - // principal in sync with "true" values, the computed amount - // may be a little less than the rounded fixed payment - // amount. For integral types, the difference should be < 3 - // (1 unit for each of the interest and management fee). For - // IOUs, the difference should be dust. - Number const diff = totalDue - totalDueAmount; - BEAST_EXPECT( - paymentComponents.specialCase == - detail::PaymentSpecialCase::final || - diff == beast::zero || - (diff > beast::zero && - ((broker.asset.integral() && - (static_cast(diff) < 3)) || - (state.loanScale - diff.exponent() > 13)))); + if (paymentParams.validateBalances) + { + // Due to the rounding algorithms to keep the interest and + // principal in sync with "true" values, the computed amount + // may be a little less than the rounded fixed payment + // amount. For integral types, the difference should be < 3 + // (1 unit for each of the interest and management fee). For + // IOUs, the difference should be dust. + Number const diff = totalDue - totalDueAmount; + BEAST_EXPECT( + paymentComponents.specialCase == + detail::PaymentSpecialCase::final || + diff == beast::zero || + (diff > beast::zero && + ((broker.asset.integral() && + (static_cast(diff) < 3)) || + (state.loanScale - diff.exponent() > 13)))); - BEAST_EXPECT( - paymentComponents.trackedPrincipalDelta >= beast::zero && - paymentComponents.trackedPrincipalDelta <= - state.principalOutstanding); - BEAST_EXPECT( - paymentComponents.specialCase != - detail::PaymentSpecialCase::final || - paymentComponents.trackedPrincipalDelta == - state.principalOutstanding); + BEAST_EXPECT( + paymentComponents.trackedPrincipalDelta >= beast::zero && + paymentComponents.trackedPrincipalDelta <= + state.principalOutstanding); + BEAST_EXPECT( + paymentComponents.specialCase != + detail::PaymentSpecialCase::final || + paymentComponents.trackedPrincipalDelta == + 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 - PrettyAmount adjustment = broker.asset(0); - if (broker.asset.native()) + if (paymentParams.validateBalances) { - 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 - verifyLoanStatus.checkPayment( - state.loanScale, - borrower, - borrowerBalanceBeforePayment, - totalDueAmount, - adjustment); + // Check the result + verifyLoanStatus.checkPayment( + state.loanScale, + borrower, + borrowerBalanceBeforePayment, + totalDueAmount, + 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); - BEAST_EXPECT(totalPaymentsMade == initialState.paymentRemaining); + if (paymentParams.validateBalances) + { + // 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 - << std::endl; + log << "Theoretical min cover: " << theoreticalMin << 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(*loanResult); + auto loanKeylet = std::get(*loanResult); + auto pseudoAcct = std::get(*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(); } }; diff --git a/src/xrpld/app/misc/LendingHelpers.h b/src/xrpld/app/misc/LendingHelpers.h index f430a0f0d5..28a8ad0829 100644 --- a/src/xrpld/app/misc/LendingHelpers.h +++ b/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, diff --git a/src/xrpld/app/misc/detail/LendingHelpers.cpp b/src/xrpld/app/misc/detail/LendingHelpers.cpp index 62f567e7c0..e5eac77c1b 100644 --- a/src/xrpld/app/misc/detail/LendingHelpers.cpp +++ b/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 principalError = principalOutstanding - raw.principalOutstanding; - auto const feeError = managementFeeOutstanding - raw.managementFeeDue; + auto const errors = rounded - raw; auto const newRawPrincipal = std::max( raw.principalOutstanding - overpaymentComponents.trackedPrincipalDelta, @@ -389,33 +387,68 @@ tryOverpayment( managementFeeRate, loanScale); - auto const newRaw = calculateRawLoanState( - newLoanProperties.periodicPayment, - periodicRate, - paymentRemaining, - managementFeeRate); + JLOG(j.debug()) << "new periodic payment: " + << newLoanProperties.periodicPayment + << ", new total value: " + << newLoanProperties.totalValueOutstanding + << ", first payment principal: " + << newLoanProperties.firstPaymentPrincipal; - totalValueOutstanding = roundToAsset( - asset, newRaw.valueOutstanding + totalValueError, loanScale); - principalOutstanding = roundToAsset( - asset, - newRaw.principalOutstanding + principalError, - loanScale, - Number::downward); - managementFeeOutstanding = - roundToAsset(asset, newRaw.managementFeeDue + feeError, loanScale); + auto const newRaw = calculateRawLoanState( + newLoanProperties.periodicPayment, + periodicRate, + paymentRemaining, + managementFeeRate) + + errors; + + JLOG(j.debug()) << "new raw value: " << newRaw.valueOutstanding + << ", 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 && - principalOutstanding > 0) + if (auto const ter = checkLoanGuards( + 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 - // where no further principal can be paid. - JLOG(j.warn()) - << "Loan overpayment would cause loan to be stuck. " - "Rejecting overpayment, but normal payments are unaffected."; + JLOG(j.warn()) << "Principal overpayment would cause the loan to be in " + "an invalid state. Ignore the overpayment"; return Unexpected(tesSUCCESS); } @@ -437,21 +470,27 @@ tryOverpayment( // LCOV_EXCL_STOP } - auto const newRounded = constructRoundedLoanState( - totalValueOutstanding, principalOutstanding, managementFeeOutstanding); + auto const deltas = rounded - newRounded; + + auto const hypotheticalValueOutstanding = + rounded.valueOutstanding - deltas.principal; + auto const valueChange = - newRounded.interestOutstanding() - rounded.interestOutstanding(); - XRPL_ASSERT_PARTS( - valueChange <= beast::zero, - "ripple::detail::tryOverpayment", - "principal overpayment did not increase value of loan"); + 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{ - .principalPaid = - rounded.principalOutstanding - newRounded.principalOutstanding, - .interestPaid = rounded.interestDue - newRounded.interestDue, - .valueChange = valueChange + overpaymentComponents.untrackedInterest, - .feePaid = rounded.managementFeeDue - newRounded.managementFeeDue + + .principalPaid = deltas.principal, + .interestPaid = + deltas.interest + overpaymentComponents.untrackedInterest, + .valueChange = + 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 == - totalValueOutstandingProxy - totalValueOutstanding - - overpaymentComponents.trackedPrincipalDelta, + loanPaymentParts.valueChange == + totalValueOutstanding - + (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, rawOverpaymentManagementFee] = [&]() { + auto const [rawOverpaymentInterest, _] = [&]() { 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, - .trackedPrincipalDelta = payment - roundedOverpaymentInterest - - roundedOverpaymentManagementFee, + .trackedValueDelta = overpayment - fee, + .trackedPrincipalDelta = overpayment - roundedOverpaymentInterest - + 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, diff --git a/src/xrpld/app/tx/detail/LoanPay.cpp b/src/xrpld/app/tx/detail/LoanPay.cpp index 0f3a6a3819..39a93f634a 100644 --- a/src/xrpld/app/tx/detail/LoanPay.cpp +++ b/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, diff --git a/src/xrpld/app/tx/detail/LoanSet.cpp b/src/xrpld/app/tx/detail/LoanSet.cpp index f72ad33cee..d9e5f8b981 100644 --- a/src/xrpld/app/tx/detail/LoanSet.cpp +++ b/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_)) diff --git a/src/xrpld/app/tx/detail/LoanSet.h b/src/xrpld/app/tx/detail/LoanSet.h index e7f4ef5503..91f3960891 100644 --- a/src/xrpld/app/tx/detail/LoanSet.h +++ b/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;