From 2dd239c59fc66b665667b95eae24c918a5abc7d3 Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Tue, 7 Oct 2025 21:48:43 -0400 Subject: [PATCH] Update payment component computation and rounding, and more tests - Tests not expected to pass. - Check in LoanSet if a loan with interest actually has interest. tecPRECISION_LOSS if not. - Add checks in LoanPay for deep froze broker owner and pseudoaccount. - Fix management fee calculations in LoanPay and associated LoanBroker and Vault data updates. - Make state tracking next payment due date optional. - Add a test case showing multiple payments combined. - Update more tests to work with the new fields. --- src/test/app/Loan_test.cpp | 288 +++++++++++++++---- src/xrpld/app/misc/LendingHelpers.h | 121 ++++---- src/xrpld/app/misc/detail/LendingHelpers.cpp | 8 +- src/xrpld/app/tx/detail/LoanPay.cpp | 111 +++++-- src/xrpld/app/tx/detail/LoanSet.cpp | 11 +- 5 files changed, 406 insertions(+), 133 deletions(-) diff --git a/src/test/app/Loan_test.cpp b/src/test/app/Loan_test.cpp index b992cf278d..3c1f022186 100644 --- a/src/test/app/Loan_test.cpp +++ b/src/test/app/Loan_test.cpp @@ -110,7 +110,7 @@ class Loan_test : public beast::unit_test::suite { std::uint32_t previousPaymentDate = 0; NetClock::time_point startDate = {}; - std::uint32_t nextPaymentDate = 0; + std::optional nextPaymentDate = 0; std::uint32_t paymentRemaining = 0; std::int32_t const loanScale = 0; Number totalValue = 0; @@ -210,7 +210,7 @@ class Loan_test : public beast::unit_test::suite void operator()( std::uint32_t previousPaymentDate, - std::uint32_t nextPaymentDate, + std::optional nextPaymentDate, std::uint32_t paymentRemaining, Number const& loanScale, Number const& totalValue, @@ -226,7 +226,7 @@ class Loan_test : public beast::unit_test::suite env.test.BEAST_EXPECT( loan->at(sfPreviousPaymentDate) == previousPaymentDate); env.test.BEAST_EXPECT( - loan->at(sfNextPaymentDueDate) == nextPaymentDate); + loan->at(~sfNextPaymentDueDate) == nextPaymentDate); env.test.BEAST_EXPECT( loan->at(sfPaymentRemaining) == paymentRemaining); env.test.BEAST_EXPECT(loan->at(sfLoanScale) == loanScale); @@ -360,7 +360,7 @@ class Loan_test : public beast::unit_test::suite LoanState state{ .previousPaymentDate = loan->at(sfPreviousPaymentDate), .startDate = tp{d{loan->at(sfStartDate)}}, - .nextPaymentDate = loan->at(sfNextPaymentDueDate), + .nextPaymentDate = loan->at(~sfNextPaymentDueDate), .paymentRemaining = loan->at(sfPaymentRemaining), .loanScale = loan->at(sfLoanScale), .totalValue = loan->at(sfTotalValueOutstanding), @@ -373,8 +373,9 @@ class Loan_test : public beast::unit_test::suite .interestRate = TenthBips32{loan->at(sfInterestRate)}, }; BEAST_EXPECT(state.previousPaymentDate == 0); - BEAST_EXPECT( - tp{d{state.nextPaymentDate}} == state.startDate + 600s); + if (BEAST_EXPECT(state.nextPaymentDate)) + BEAST_EXPECT( + tp{d{*state.nextPaymentDate}} == state.startDate + 600s); BEAST_EXPECT(state.paymentRemaining == 12); BEAST_EXPECT( state.principalOutstanding == broker.asset(1000).value()); @@ -1231,7 +1232,9 @@ class Loan_test : public beast::unit_test::suite verifyLoanStatus(state); } - auto const nextDueDate = tp{d{state.nextPaymentDate}}; + BEAST_EXPECT(state.nextPaymentDate); + auto const nextDueDate = + tp{d{state.nextPaymentDate.value_or(0)}}; // Can't default the loan yet. The grace period hasn't // expired @@ -1273,7 +1276,6 @@ class Loan_test : public beast::unit_test::suite // Can't make a payment on it either env(pay(borrower, loanKeylet.key, broker.asset(300)), ter(tecKILLED)); - }; }; @@ -1354,6 +1356,11 @@ class Loan_test : public beast::unit_test::suite payoffAmount == broker.asset(Number(1040000114155251, -12))); BEAST_EXPECT(payoffAmount > state.principalOutstanding); + // The terms of this loan actually make the early payoff more + // expensive than just making payments + BEAST_EXPECT( + payoffAmount > state.paymentRemaining * + (state.periodicPayment + broker.asset(2).value())); // Try to pay a little extra to show that it's _not_ // taken auto const transactionAmount = payoffAmount + broker.asset(10); @@ -1373,6 +1380,120 @@ class Loan_test : public beast::unit_test::suite state.paymentRemaining = 0; state.principalOutstanding = 0; + state.referencePrincipal = 0; + state.totalValue = 0; + state.interestOwed = 0; + if (BEAST_EXPECT(state.nextPaymentDate)) + state.previousPaymentDate = *state.nextPaymentDate; + state.nextPaymentDate.reset(); + verifyLoanStatus(state); + + STAmount const balanceChangeAmount{ + broker.asset, + roundToAsset(broker.asset, payoffAmount, state.loanScale)}; + { + auto const difference = + roundToScale( + env.balance(borrower, broker.asset), + state.loanScale) - + (borrowerBalanceBeforePayment - balanceChangeAmount - + adjustment); + BEAST_EXPECT(difference == beast::zero); + BEAST_EXPECT( + roundToScale(difference, state.loanScale) == + beast::zero); + } + + // Can't impair or default a paid off loan + env(manage(lender, loanKeylet.key, tfLoanImpair), + ter(tecNO_PERMISSION)); + env(manage(lender, loanKeylet.key, tfLoanDefault), + ter(tecNO_PERMISSION)); + }; + }; + + auto multiplePayoff = [&](std::uint32_t baseFlag) { + return [&, baseFlag]( + Keylet const& loanKeylet, + VerifyLoanStatus const& verifyLoanStatus) { + // toEndOfLife + // + auto state = + getCurrentState(env, broker, loanKeylet, verifyLoanStatus); + BEAST_EXPECT(state.flags == baseFlag); + env.close(state.startDate + 20s); + auto const loanAge = (env.now() - state.startDate).count(); + BEAST_EXPECT(loanAge == 30); + + verifyLoanStatus(state); + + // Send some bogus pay transactions + env(pay(borrower, + keylet::loan(uint256(0)).key, + broker.asset(10)), + ter(temINVALID)); + env(pay(borrower, loanKeylet.key, broker.asset(-100)), + ter(temBAD_AMOUNT)); + env(pay(borrower, broker.brokerID, broker.asset(100)), + ter(tecNO_ENTRY)); + env(pay(evan, loanKeylet.key, broker.asset(500)), + ter(tecNO_PERMISSION)); + + { + auto const otherAsset = + broker.asset.raw() == assets[0].raw() ? assets[1] + : assets[0]; + env(pay(borrower, loanKeylet.key, otherAsset(100)), + ter(tecWRONG_ASSET)); + } + + // Amount doesn't cover a single payment + env(pay(borrower, loanKeylet.key, STAmount{broker.asset, 1}), + ter(tecINSUFFICIENT_PAYMENT)); + + // Get the balance after these failed transactions take + // fees + auto const borrowerBalanceBeforePayment = + env.balance(borrower, broker.asset); + + // Make all the payments in one transaction + // service fee is 2 + auto const startingPayments = state.paymentRemaining; + auto const rawPayoff = startingPayments * + (state.periodicPayment + broker.asset(2).value()); + STAmount const payoffAmount{broker.asset, rawPayoff}; + BEAST_EXPECT( + payoffAmount == + broker.asset(Number(1024014840139457, -12))); + BEAST_EXPECT(payoffAmount > state.principalOutstanding); + + // Try to pay a little extra to show that it's _not_ + // taken + auto const transactionAmount = payoffAmount + broker.asset(10); + BEAST_EXPECT( + transactionAmount == + broker.asset(Number(1034014840139457, -12))); + env(pay(borrower, loanKeylet.key, transactionAmount)); + + env.close(); + + // Need to account for fees if the loan is in XRP + PrettyAmount adjustment = broker.asset(0); + if (broker.asset.raw().native()) + { + adjustment = env.current()->fees().base; + } + + state.paymentRemaining = 0; + state.principalOutstanding = 0; + state.referencePrincipal = 0; + state.totalValue = 0; + state.interestOwed = 0; + if (BEAST_EXPECT(state.nextPaymentDate)) + state.previousPaymentDate = *state.nextPaymentDate + + state.paymentInterval * + (startingPayments - 1); // 9280-2680=6600 + state.nextPaymentDate.reset(); verifyLoanStatus(state); STAmount const balanceChangeAmount{ @@ -1482,6 +1603,34 @@ class Loan_test : public beast::unit_test::suite tfLoanOverpayment, immediatePayoff(lsfLoanOverpayment)); + lifecycle( + caseLabel, + "Loan overpayment prohibited - Make multiple payments", + env, + loanAmount, + interestExponent, + lender, + borrower, + evan, + broker, + pseudoAcct, + 0, + multiplePayoff(0)); + + lifecycle( + caseLabel, + "Loan overpayment allowed - Make multiple payments", + env, + loanAmount, + interestExponent, + lender, + borrower, + evan, + broker, + pseudoAcct, + tfLoanOverpayment, + multiplePayoff(lsfLoanOverpayment)); + lifecycle( caseLabel, "Loan overpayment prohibited - Make payments", @@ -1529,20 +1678,24 @@ class Loan_test : public beast::unit_test::suite roundPeriodicPayment( broker.asset, state.periodicPayment, state.loanScale)}; + testcase << "\tPayment components: " + << "Payments remaining, rawInterest, rawPrincipal, " + "roundedInterest, " + "roundedPrincipal, roundedPayment, final, extra"; + while (state.paymentRemaining > 0) { - testcase << "Payments remaining: " << state.paymentRemaining - << ", computed payment amount: " - << state.periodicPayment; - + auto const serviceFee = broker.asset(2); // Only check the first payment since the rounding // may drift as payments are made BEAST_EXPECT( roundedPeriodicPayment == - broker.asset(Number(8333457001162141, -14))); + broker.asset( + Number(8333457001162141, -14), Number::upward)); + // 83334570.01162141 // Include the service fee STAmount const totalDue = roundToScale( - roundedPeriodicPayment + broker.asset(2), + roundedPeriodicPayment + serviceFee, state.loanScale, Number::upward); // Only check the first payment since the rounding @@ -1569,38 +1722,52 @@ class Loan_test : public beast::unit_test::suite state.loanScale, Number::upward)); - auto const totalDueAmount = - STAmount{broker.asset, totalDue}; - // Compute the expected principal amount - Number const rawInterest = state.paymentRemaining == 1 - ? state.periodicPayment - state.referencePrincipal - : state.referencePrincipal * periodicRate; - STAmount const interest{ - broker.asset, - roundToAsset( + auto const paymentComponents = + detail::computePaymentComponents( broker.asset, - rawInterest, state.loanScale, - Number::upward)}; + state.totalValue, + state.principalOutstanding, + state.referencePrincipal, + state.periodicPayment, + periodicRate, + state.paymentRemaining); + + testcase + << "\tPayment components: " << state.paymentRemaining + << ", " << paymentComponents.rawInterest << ", " + << paymentComponents.rawPrincipal << ", " + << paymentComponents.roundedInterest << ", " + << paymentComponents.roundedPrincipal << ", " + << paymentComponents.roundedPayment << ", " + << paymentComponents.final << ", " + << paymentComponents.extra; + + auto const totalDueAmount = STAmount{ + broker.asset, + paymentComponents.roundedPayment + serviceFee.number()}; + + BEAST_EXPECT( + paymentComponents.final || totalDue == totalDueAmount); + BEAST_EXPECT( state.paymentRemaining < 12 || - interest == + paymentComponents.roundedInterest == roundToScale( broker.asset( Number(2283105022831050, -18), Number::upward), state.loanScale, Number::upward)); - BEAST_EXPECT(interest >= Number(0)); + BEAST_EXPECT( + paymentComponents.roundedInterest >= Number(0)); - auto const rawPrincipal = - state.periodicPayment - rawInterest; BEAST_EXPECT( state.paymentRemaining < 12 || roundToAsset( broker.asset, - rawPrincipal, + paymentComponents.rawPrincipal, state.loanScale, Number::upward) == roundToScale( @@ -1610,25 +1777,27 @@ class Loan_test : public beast::unit_test::suite state.loanScale, Number::upward)); BEAST_EXPECT( - state.paymentRemaining > 1 || - rawPrincipal == state.principalOutstanding); - STAmount const principal{ - broker.asset, - roundToAsset( - broker.asset, - roundedPeriodicPayment - interest, - state.loanScale, - Number::downward)}; + !paymentComponents.final || + paymentComponents.rawPrincipal == + state.referencePrincipal); BEAST_EXPECT( - principal > Number(0) && - principal <= state.principalOutstanding); + paymentComponents.roundedPrincipal >= Number(0) && + paymentComponents.roundedPrincipal <= + state.principalOutstanding); BEAST_EXPECT( - state.paymentRemaining > 1 || - principal == state.principalOutstanding); + !paymentComponents.final || + paymentComponents.roundedPrincipal == + state.principalOutstanding); BEAST_EXPECT( - rawPrincipal + rawInterest == state.periodicPayment); + paymentComponents.final || + paymentComponents.rawPrincipal + + paymentComponents.rawInterest == + state.periodicPayment); BEAST_EXPECT( - principal + interest == roundedPeriodicPayment); + paymentComponents.final || + paymentComponents.roundedPrincipal + + paymentComponents.roundedInterest == + roundedPeriodicPayment); auto const borrowerBalanceBeforePayment = env.balance(borrower, broker.asset); @@ -1665,9 +1834,29 @@ class Loan_test : public beast::unit_test::suite Number(1, -4)))); --state.paymentRemaining; - state.previousPaymentDate = state.nextPaymentDate; - state.nextPaymentDate += state.paymentInterval; - state.principalOutstanding -= principal; + if (BEAST_EXPECT(state.nextPaymentDate)) + { + state.previousPaymentDate = *state.nextPaymentDate; + if (paymentComponents.final) + { + state.nextPaymentDate.reset(); + state.paymentRemaining = 0; + } + else + { + *state.nextPaymentDate += state.paymentInterval; + } + } + state.principalOutstanding -= + paymentComponents.roundedPrincipal; + state.referencePrincipal -= paymentComponents.rawPrincipal; + state.totalValue -= paymentComponents.roundedPrincipal + + paymentComponents.roundedInterest; + state.interestOwed -= valueMinusFee( + broker.asset.raw(), + paymentComponents.roundedInterest, + managementFeeRateParameter, + state.loanScale); verifyLoanStatus(state); } @@ -1725,7 +1914,8 @@ class Loan_test : public beast::unit_test::suite MPTTester mptt{env, issuer, mptInitNoFund}; mptt.create( {.flags = tfMPTCanClawback | tfMPTCanTransfer | tfMPTCanLock}); - PrettyAsset const mptAsset = mptt.issuanceID(); + // Scale the MPT asset a little bit so we can get some interest + PrettyAsset const mptAsset{mptt.issuanceID(), 100}; mptt.authorize({.account = lender}); mptt.authorize({.account = borrower}); mptt.authorize({.account = evan}); diff --git a/src/xrpld/app/misc/LendingHelpers.h b/src/xrpld/app/misc/LendingHelpers.h index e1ddf7e1b2..69b77bf470 100644 --- a/src/xrpld/app/misc/LendingHelpers.h +++ b/src/xrpld/app/misc/LendingHelpers.h @@ -150,14 +150,13 @@ computePaymentComponents( "Outstanding values are rounded"); auto const roundedPeriodicPayment = roundPeriodicPayment(asset, periodicPayment, scale); - if (paymentRemaining == 1 || totalValueOutstanding <= periodicPayment) + if (paymentRemaining == 1 || + totalValueOutstanding <= roundedPeriodicPayment) { // If there's only one payment left, we need to pay off each of the loan // parts. - // - // The totalValueOutstanding should never be less than the - // periodicPayment until the last scheduled payment, but if it ever is, - // make it the last payment. + // rawInterest could be < 0 because we're computing it with the rounded + // value outstanding, but for the last payment, we also don't care. Number rawInterest = totalValueOutstanding - referencePrincipal; Number roundedInterest = totalValueOutstanding - principalOutstanding; @@ -169,7 +168,7 @@ computePaymentComponents( "last payment is complete"); return { - .rawInterest = rawInterest, + .rawInterest = std::max(Number{}, rawInterest), .rawPrincipal = referencePrincipal, .roundedInterest = roundedInterest, .roundedPrincipal = principalOutstanding, @@ -190,54 +189,54 @@ computePaymentComponents( "ripple::detail::computePaymentComponents", "valid raw interest"); XRPL_ASSERT_PARTS( - rawPrincipal > 0 && rawPrincipal <= referencePrincipal, + rawPrincipal >= 0 && rawPrincipal <= referencePrincipal, "ripple::detail::computePaymentComponents", "valid raw principal"); - // if (count($A20), MIN(Z19, Z19 - FLOOR(AA19 - Y20, 1)), "") - // Z19 = outstanding principal - // AA19 = reference principal - // Y20 = raw principal - Number const roundedPrincipal = [&]() { - Number const p = std::max( - Number{}, - std::min( - principalOutstanding, - principalOutstanding - - roundToAsset( - asset, - referencePrincipal - rawPrincipal, - scale, - Number::downward))); - // if the estimated principal payment would leave the principal higher - // than the "total "after payment" value of the loan, make the principal - // payment also take the principal down to that same "after" value. - // This should mean that all interest is paid, or that the loan has some - // tricky parameters. - if (principalOutstanding - p > - totalValueOutstanding - roundedPeriodicPayment) - return roundedPeriodicPayment; - // Use the amount that will get principal outstanding as close to - // reference principal as possible, but don't pay more than the rounded - // periodic payment, or we'll end up with negative interest. - return std::min(p, roundedPeriodicPayment); + // Round the raw principal after payment + auto const roundedPrincipalTarget = + roundToAsset(asset, referencePrincipal - rawPrincipal, scale); + // Determine the total value after payment + auto const totalValueTarget = + totalValueOutstanding - roundedPeriodicPayment; + // We want to get the principal down to the smaller of the two targets + auto const principalTarget = + std::min(roundedPrincipalTarget, totalValueTarget); + // What will get us to our target? + auto const p = principalOutstanding - principalTarget; + + XRPL_ASSERT_PARTS( + p >= 0 && p <= roundedPeriodicPayment, + "rippled::detail::computePaymentComponents", + "principal part not larger than total payment"); + // Make sure nothing goes negative + if (p > roundedPeriodicPayment || p > principalOutstanding) + return std::min(roundedPeriodicPayment, principalOutstanding); + else if (p < 0) + return Number{}; + + return p; }(); - // if(count($A20), if(AB19 < $B$5, AB19 - Z19, CEILING($B$10-W20, 1)), "") - // AB19 = total loan value - // $B$5 = periodic payment (unrounded) - // Z19 = outstanding principal - // $B$10 = periodic payment (rounded up) - // W20 = rounded principal + Number const roundedInterest = [&]() { + // Zero interest means ZERO interest + if (periodicRate == 0) + return Number{}; + auto i = roundedPeriodicPayment - roundedPrincipal; + // No negative interest! + if (i < 0) + return Number{}; + return i; + }(); - Number const roundedInterest = roundedPeriodicPayment - roundedPrincipal; XRPL_ASSERT_PARTS( roundedInterest >= 0 && isRounded(asset, roundedInterest, scale), "ripple::detail::computePaymentComponents", "valid rounded interest"); XRPL_ASSERT_PARTS( roundedPrincipal >= 0 && roundedPrincipal <= principalOutstanding && + roundedPrincipal <= roundedPeriodicPayment && isRounded(asset, roundedPrincipal, scale), "ripple::detail::computePaymentComponents", "valid rounded principal"); @@ -280,11 +279,12 @@ doPayment( nextDueDateProxy, "ripple::detail::doPayment", "Next due date proxy set"); + auto const totalValueDelta = payment.roundedPrincipal + + payment.roundedInterest - payment.valueChange; if (!payment.extra) { if (payment.final) { - paymentRemainingProxy = 0; XRPL_ASSERT_PARTS( referencePrincipalProxy == payment.rawPrincipal, "ripple::detail::doPayment", @@ -294,11 +294,12 @@ doPayment( "ripple::detail::doPayment", "Full principal payment"); XRPL_ASSERT_PARTS( - totalValueOutstandingProxy == - payment.roundedPrincipal + payment.roundedInterest, + totalValueOutstandingProxy == totalValueDelta, "ripple::detail::doPayment", "Full value payment"); + paymentRemainingProxy = 0; + prevPaymentDateProxy = *nextDueDateProxy; // Remove the field. This is the only condition where nextDueDate is // allowed to be removed. @@ -306,6 +307,19 @@ doPayment( } else { + XRPL_ASSERT_PARTS( + referencePrincipalProxy > payment.rawPrincipal, + "ripple::detail::doPayment", + "Full reference principal payment"); + XRPL_ASSERT_PARTS( + principalOutstandingProxy > payment.roundedPrincipal, + "ripple::detail::doPayment", + "Full principal payment"); + XRPL_ASSERT_PARTS( + totalValueOutstandingProxy > totalValueDelta, + "ripple::detail::doPayment", + "Full value payment"); + paymentRemainingProxy -= 1; prevPaymentDateProxy = *nextDueDateProxy; @@ -317,8 +331,7 @@ doPayment( referencePrincipalProxy -= payment.rawPrincipal; principalOutstandingProxy -= payment.roundedPrincipal; - totalValueOutstandingProxy -= - payment.roundedPrincipal + payment.roundedInterest; + totalValueOutstandingProxy -= totalValueDelta; return LoanPaymentParts{ .principalPaid = payment.roundedPrincipal, @@ -1008,10 +1021,15 @@ loanMakePayment( nextDueDateProxy, paymentInterval); ++numPayments; + + if (nextPayment.final) + break; } XRPL_ASSERT_PARTS( - totalParts.principalPaid + totalParts.interestPaid == totalPaid, + totalParts.principalPaid + totalParts.interestPaid + + totalParts.feeToPay == + totalPaid, "ripple::loanMakePayment", "payment parts add up"); XRPL_ASSERT_PARTS( @@ -1025,17 +1043,16 @@ loanMakePayment( // ------------------------------------------------------------- // overpayment handling - if (loan->isFlag(lsfLoanOverpayment)) + if (loan->isFlag(lsfLoanOverpayment) && paymentRemainingProxy > 0 && + nextDueDateProxy && totalPaid < amount) { TenthBips32 const overpaymentInterestRate{ loan->at(sfOverpaymentInterestRate)}; TenthBips32 const overpaymentFeeRate{loan->at(sfOverpaymentFee)}; - Number const overpayment = amount - - (totalParts.principalPaid + totalParts.interestPaid + - totalParts.feeToPay); + Number const overpayment = amount - totalPaid; XRPL_ASSERT( - overpayment >= 0 && isRounded(asset, overpayment, loanScale), + overpayment > 0 && isRounded(asset, overpayment, loanScale), "ripple::loanMakePayment : valid overpayment amount"); Number const fee = roundToAsset( diff --git a/src/xrpld/app/misc/detail/LendingHelpers.cpp b/src/xrpld/app/misc/detail/LendingHelpers.cpp index eafb28cd34..d32413437f 100644 --- a/src/xrpld/app/misc/detail/LendingHelpers.cpp +++ b/src/xrpld/app/misc/detail/LendingHelpers.cpp @@ -61,13 +61,11 @@ loanPeriodicPayment( /* * This formula is from the XLS-66 spec, section 3.2.4.1.1 (Regular - * Payment), though the awkwardly-named "timeFactor" is computed only once - * and used twice. + * Payment), though "raisedRate" is computed only once and used twice. */ - // TODO: Need a better name - Number const timeFactor = power(1 + periodicRate, paymentsRemaining); + Number const raisedRate = power(1 + periodicRate, paymentsRemaining); - return principalOutstanding * periodicRate * timeFactor / (timeFactor - 1); + return principalOutstanding * periodicRate * raisedRate / (raisedRate - 1); } Number diff --git a/src/xrpld/app/tx/detail/LoanPay.cpp b/src/xrpld/app/tx/detail/LoanPay.cpp index 99bb05f9ec..e9c85af255 100644 --- a/src/xrpld/app/tx/detail/LoanPay.cpp +++ b/src/xrpld/app/tx/detail/LoanPay.cpp @@ -48,7 +48,7 @@ LoanPay::calculateBaseFee(ReadView const& view, STTx const& tx) auto const normalCost = Transactor::calculateBaseFee(view, tx); auto const paymentsPerFeeIncrement = 20; - // The fee is based on the number of potential payments, unless the loan is + // The fee is based on the potential number of payments, unless the loan is // being fully paid off. auto const amount = tx[sfAmount]; auto const loanID = tx[sfLoanID]; @@ -136,17 +136,17 @@ LoanPay::preclaim(PreclaimContext const& ctx) return tecNO_ENTRY; } - auto const principalOutstanding = loanSle->at(sfPrincipalOutstanding); - TenthBips32 const interestRate{loanSle->at(sfInterestRate)}; - auto const paymentRemaining = loanSle->at(sfPaymentRemaining); - TenthBips32 const lateInterestRate{loanSle->at(sfLateInterestRate)}; - if (loanSle->at(sfBorrower) != account) { JLOG(ctx.j.warn()) << "Loan does not belong to the account."; return tecNO_PERMISSION; } + auto const principalOutstanding = loanSle->at(sfPrincipalOutstanding); + TenthBips32 const interestRate{loanSle->at(sfInterestRate)}; + auto const paymentRemaining = loanSle->at(sfPaymentRemaining); + TenthBips32 const lateInterestRate{loanSle->at(sfLateInterestRate)}; + if (paymentRemaining == 0 || principalOutstanding == 0) { JLOG(ctx.j.warn()) << "Loan is already paid off."; @@ -164,6 +164,7 @@ LoanPay::preclaim(PreclaimContext const& ctx) // LCOV_EXCL_STOP } auto const brokerPseudoAccount = loanBrokerSle->at(sfAccount); + auto const brokerOwner = loanBrokerSle->at(sfOwner); auto const vaultID = loanBrokerSle->at(sfVaultID); auto const vaultSle = ctx.view.read(keylet::vault(vaultID)); if (!vaultSle) @@ -175,6 +176,7 @@ LoanPay::preclaim(PreclaimContext const& ctx) // LCOV_EXCL_STOP } auto const asset = vaultSle->at(sfAsset); + auto const vaultPseudoAccount = vaultSle->at(sfAccount); if (amount.asset() != asset) { @@ -193,6 +195,18 @@ LoanPay::preclaim(PreclaimContext const& ctx) "funds (deep frozen)."; return ret; } + if (auto const ret = checkDeepFrozen(ctx.view, brokerOwner, asset)) + { + JLOG(ctx.j.warn()) + << "Loan Broker can not receive funds (deep frozen)."; + return ret; + } + if (auto const ret = checkDeepFrozen(ctx.view, vaultPseudoAccount, asset)) + { + JLOG(ctx.j.warn()) + << "Vault pseudo-account can not receive funds (deep frozen)."; + return ret; + } return tesSUCCESS; } @@ -235,12 +249,27 @@ LoanPay::doApply() } TenthBips16 managementFeeRate{brokerSle->at(sfManagementFeeRate)}; + auto const managementFeeOutstanding = [&]() { + auto const m = loanSle->at(sfTotalValueOutstanding) - + loanSle->at(sfPrincipalOutstanding) - loanSle->at(sfInterestOwed); + // It shouldn't be possible for this to result in a negative number, but + // with overpayments, who knows? + if (m < 0) + return Number{}; + return m; + }(); Expected paymentParts = loanMakePayment(asset, view, loanSle, amount, managementFeeRate, j_); if (!paymentParts) + { + XRPL_ASSERT_PARTS( + paymentParts.error(), + "ripple::LoanPay::doApply", + "payment error is an error"); return paymentParts.error(); + } // If the payment computation completed without error, the loanSle object // has been modified. @@ -256,6 +285,11 @@ LoanPay::doApply() paymentParts->interestPaid >= 0, "ripple::LoanPay::doApply", "valid interest paid"); + XRPL_ASSERT_PARTS( + // It should not be possible to pay 0 total + paymentParts->principalPaid + paymentParts->interestPaid > 0, + "ripple::LoanPay::doApply", + "valid principal paid"); XRPL_ASSERT_PARTS( paymentParts->feeToPay >= 0, "ripple::LoanPay::doApply", @@ -277,27 +311,45 @@ LoanPay::doApply() auto interestOwedProxy = loanSle->at(sfInterestOwed); - auto const [managementFee, interestPaidToVault] = [&]() { - auto const managementFee = roundToAsset( + auto const [managementFee, interestPaidForDebt, interestPaidExtra] = [&]() { + auto const interestOwed = + paymentParts->interestPaid - paymentParts->valueChange; + auto const interestPaidExtra = paymentParts->valueChange; + + auto const managementFeeOwed = std::min( + managementFeeOutstanding, + roundToAsset( + asset, + tenthBipsOfValue(interestOwed, managementFeeRate), + loanScale)); + auto const managementFeeExtra = roundToAsset( asset, - tenthBipsOfValue(paymentParts->interestPaid, managementFeeRate), + tenthBipsOfValue(interestPaidExtra, managementFeeRate), loanScale); - auto const interest = paymentParts->interestPaid - managementFee; + auto const interestForDebt = interestOwed - managementFeeOwed; + auto const interestExtra = interestPaidExtra - managementFeeExtra; auto const owed = *interestOwedProxy; - if (interest > owed) - return std::make_pair(paymentParts->interestPaid - owed, owed); - return std::make_pair(managementFee, interest); + if (interestForDebt > owed) + return std::make_tuple( + interestOwed - owed + managementFeeExtra, owed, interestExtra); + return std::make_tuple( + managementFeeOwed + managementFeeExtra, + interestForDebt, + interestExtra); }(); XRPL_ASSERT_PARTS( - managementFee >= 0 && interestPaidToVault >= 0 && - (managementFee + interestPaidToVault == + managementFee >= 0 && interestPaidForDebt >= 0 && + interestPaidExtra >= 0 && + (managementFee + interestPaidForDebt + interestPaidExtra == paymentParts->interestPaid) && isRounded(asset, managementFee, loanScale) && - isRounded(asset, interestPaidToVault, loanScale), + isRounded(asset, interestPaidForDebt, loanScale) && + isRounded(asset, interestPaidExtra, loanScale), "ripple::LoanPay::doApply", "management fee computation is valid"); - auto const totalPaidToVault = - paymentParts->principalPaid + interestPaidToVault; + auto const totalPaidToVaultForDebt = + paymentParts->principalPaid + interestPaidForDebt; + auto const totalPaidToVault = totalPaidToVaultForDebt + interestPaidExtra; auto const totalPaidToBroker = paymentParts->feeToPay + managementFee; @@ -311,33 +363,40 @@ LoanPay::doApply() auto debtTotalProxy = brokerSle->at(sfDebtTotal); // Decrease LoanBroker Debt by the amount paid, add the Loan value change - // (which might be negative). debtDecrease may be negative, increasing the - // debt - auto const debtDecrease = totalPaidToVault - paymentParts->valueChange; + // (which might be negative). totalPaidToVaultForDebt may be negative, + // increasing the debt XRPL_ASSERT_PARTS( - isRounded(asset, debtDecrease, loanScale), + isRounded(asset, totalPaidToVaultForDebt, loanScale), "ripple::LoanPay::doApply", - "debtDecrease rounding good"); + "totalPaidToVaultForDebt rounding good"); // Despite our best efforts, it's possible for rounding errors to accumulate // in the loan broker's debt total. This is because the broker may have more // that one loan with significantly different scales. - if (debtDecrease >= debtTotalProxy) + if (totalPaidToVaultForDebt >= debtTotalProxy) debtTotalProxy = 0; else - debtTotalProxy -= debtDecrease; + debtTotalProxy -= totalPaidToVaultForDebt; //------------------------------------------------------ // Vault object state changes view.update(vaultSle); + // auto const available = *vaultSle->at(sfAssetsAvailable); + // auto const total = *vaultSle->at(sfAssetsTotal); + // auto const unavailable = total - available; + vaultSle->at(sfAssetsAvailable) += totalPaidToVault; - vaultSle->at(sfAssetsTotal) += paymentParts->valueChange; + vaultSle->at(sfAssetsTotal) += interestPaidExtra; interestOwedProxy -= interestPaidToVault; XRPL_ASSERT_PARTS( *vaultSle->at(sfAssetsAvailable) <= *vaultSle->at(sfAssetsTotal), "ripple::LoanPay::doApply", "assets available must not be greater than assets outstanding"); + // auto const available = *vaultSle->at(sfAssetsAvailable); + // auto const total = *vaultSle->at(sfAssetsTotal); + // auto const unavailable = total - available; + // Move funds STAmount const paidToVault(asset, totalPaidToVault); STAmount const paidToBroker(asset, totalPaidToBroker); diff --git a/src/xrpld/app/tx/detail/LoanSet.cpp b/src/xrpld/app/tx/detail/LoanSet.cpp index 7be4b9fe21..ee3cb507ab 100644 --- a/src/xrpld/app/tx/detail/LoanSet.cpp +++ b/src/xrpld/app/tx/detail/LoanSet.cpp @@ -317,7 +317,16 @@ LoanSet::doApply() // good. Note that the outstanding principal is rounded, and may not // change right away. JLOG(j_.warn()) << "Loan is unable to pay principal."; - return tecLIMIT_EXCEEDED; + return tecPRECISION_LOSS; + } + if (interestRate != 0 && + (properties.totalValueOutstanding - principalRequested) <= 0) + { + // Unless this is a zero-interst loan, there must be some interest due + // on the loan, even if it's (measurable) dust + JLOG(j_.warn()) << "Loan with " << interestRate + << "% interest has no interest due"; + return tecPRECISION_LOSS; } // Check that the other computed values are valid if (properties.interestOwedToVault < 0 ||