From 89347cf091486c885c58a1d47e18e83cc5065383 Mon Sep 17 00:00:00 2001 From: Vito <5780819+Tapanito@users.noreply.github.com> Date: Wed, 15 Jul 2026 13:51:54 +0200 Subject: [PATCH] refactor: Pass Loan SLE directly into payment-computation helpers doPayment, doOverpayment, computeFullPayment, and computeLatePayment each took a long list of individually-fetched proxy/value parameters that were always sourced from the same Loan ledger object at their single call site. Take SLE::ref/const_ref directly instead and unwrap fields internally, cutting parameter counts and removing threading duplication in loanMakePayment. computePaymentComponents gains a thin SLE-unwrapping overload alongside its existing value-based form, which stays for direct exercise by the unit test model-based checks. Also renames constructRoundedLoanState to a constructLoanState overload for consistency with the other overload. --- include/xrpl/ledger/helpers/LendingHelpers.h | 2 +- src/libxrpl/ledger/helpers/LendingHelpers.cpp | 268 +++++++----------- src/test/app/Loan_test.cpp | 6 +- 3 files changed, 101 insertions(+), 175 deletions(-) diff --git a/include/xrpl/ledger/helpers/LendingHelpers.h b/include/xrpl/ledger/helpers/LendingHelpers.h index e2605e9ab7..5f146c52df 100644 --- a/include/xrpl/ledger/helpers/LendingHelpers.h +++ b/include/xrpl/ledger/helpers/LendingHelpers.h @@ -266,7 +266,7 @@ constructLoanState( // Constructs a valid LoanState object from a Loan object, which always has // rounded values LoanState -constructRoundedLoanState(SLE::const_ref loan); +constructLoanState(SLE::const_ref loan); Number computeManagementFee( diff --git a/src/libxrpl/ledger/helpers/LendingHelpers.cpp b/src/libxrpl/ledger/helpers/LendingHelpers.cpp index f7ec8a8bc3..a81cd5a716 100644 --- a/src/libxrpl/ledger/helpers/LendingHelpers.cpp +++ b/src/libxrpl/ledger/helpers/LendingHelpers.cpp @@ -387,22 +387,18 @@ loanAccruedInterest( * * This is the core function that updates the Loan ledger object fields based on * a computed payment. - - * The function is templated to work with both direct Number/uint32_t values - * (for testing/simulation) and ValueProxy types (for actual ledger updates). */ -template LoanPaymentParts -doPayment( - ExtendedPaymentComponents const& payment, - NumberProxy& totalValueOutstandingProxy, - NumberProxy& principalOutstandingProxy, - NumberProxy& managementFeeOutstandingProxy, - UInt32Proxy& paymentRemainingProxy, - UInt32Proxy& prevPaymentDateProxy, - UInt32OptionalProxy& nextDueDateProxy, - std::uint32_t paymentInterval) +doPayment(ExtendedPaymentComponents const& payment, SLE::ref loan) { + auto totalValueOutstandingProxy = loan->at(sfTotalValueOutstanding); + auto principalOutstandingProxy = loan->at(sfPrincipalOutstanding); + auto managementFeeOutstandingProxy = loan->at(sfManagementFeeOutstanding); + auto paymentRemainingProxy = loan->at(sfPaymentRemaining); + auto prevPaymentDateProxy = loan->at(sfPreviousPaymentDueDate); + auto nextDueDateProxy = loan->at(sfNextPaymentDueDate); + std::uint32_t const paymentInterval = loan->at(sfPaymentInterval); + XRPL_ASSERT_PARTS(nextDueDateProxy, "xrpl::detail::doPayment", "Next due date proxy set"); if (payment.specialCase == PaymentSpecialCase::Final) @@ -470,16 +466,12 @@ doPayment( // Principal can never exceed total value (principal is part of total value) XRPL_ASSERT_PARTS( - // Use an explicit cast because the template parameter can be - // ValueProxy or Number static_cast(principalOutstandingProxy) <= static_cast(totalValueOutstandingProxy), "xrpl::detail::doPayment", "principal does not exceed total"); XRPL_ASSERT_PARTS( - // Use an explicit cast because the template parameter can be - // ValueProxy or Number static_cast(managementFeeOutstandingProxy) >= beast::kZero, "xrpl::detail::doPayment", "fee outstanding stays valid"); @@ -717,22 +709,23 @@ tryOverpayment( * overpayment would leave the loan in an invalid state, we can reject it * gracefully without corrupting the ledger data. */ -template std::expected doOverpayment( Rules const& rules, Asset const& asset, std::int32_t loanScale, ExtendedPaymentComponents const& overpaymentComponents, - NumberProxy& totalValueOutstandingProxy, - NumberProxy& principalOutstandingProxy, - NumberProxy& managementFeeOutstandingProxy, - NumberProxy& periodicPaymentProxy, + SLE::ref loan, Number const& periodicRate, - std::uint32_t const paymentRemaining, TenthBips16 const managementFeeRate, beast::Journal j) { + auto totalValueOutstandingProxy = loan->at(sfTotalValueOutstanding); + auto principalOutstandingProxy = loan->at(sfPrincipalOutstanding); + auto managementFeeOutstandingProxy = loan->at(sfManagementFeeOutstanding); + auto periodicPaymentProxy = loan->at(sfPeriodicPayment); + auto const paymentsRemaining = loan->at(sfPaymentRemaining); + auto const loanState = constructLoanState( totalValueOutstandingProxy, principalOutstandingProxy, managementFeeOutstandingProxy); auto const periodicPayment = periodicPaymentProxy; @@ -744,7 +737,7 @@ doOverpayment( << ", interestPart: " << overpaymentComponents.trackedInterestPart() << ", untrackedInterest: " << overpaymentComponents.untrackedInterest << ", totalDue: " << overpaymentComponents.totalDue - << ", payments remaining :" << paymentRemaining; + << ", payments remaining :" << paymentsRemaining; // Attempt to re-amortize the loan with the overpayment applied. // This modifies the temporary copies, leaving the proxies unchanged. @@ -756,7 +749,7 @@ doOverpayment( loanState, periodicPayment, periodicRate, - paymentRemaining, + paymentsRemaining, managementFeeRate, j); if (!ret) @@ -864,16 +857,15 @@ std::expected computeLatePayment( Asset const& asset, ApplyView const& view, - Number const& principalOutstanding, - std::int32_t nextDueDate, + SLE::const_ref loan, ExtendedPaymentComponents const& periodic, - TenthBips32 lateInterestRate, - std::int32_t loanScale, - Number const& latePaymentFee, STAmount const& amount, TenthBips16 managementFeeRate, beast::Journal j) { + std::int32_t const nextDueDate = loan->at(sfNextPaymentDueDate); + std::int32_t const loanScale = loan->at(sfLoanScale); + // Check if the due date has passed. If not, reject the payment as // being too soon if (!hasExpired(view, nextDueDate)) @@ -881,7 +873,10 @@ computeLatePayment( // Calculate the penalty interest based on how long the payment is overdue. auto const latePaymentInterest = loanLatePaymentInterest( - principalOutstanding, lateInterestRate, view.parentCloseTime(), nextDueDate); + loan->at(sfPrincipalOutstanding), + TenthBips32{loan->at(sfLateInterestRate)}, + view.parentCloseTime(), + nextDueDate); // Round the late interest and split it between the vault (net interest) // and the broker (management fee portion). This lambda ensures we @@ -908,7 +903,7 @@ computeLatePayment( // 1. Regular service fee (from periodic.untrackedManagementFee) // 2. Late payment fee (fixed penalty) // 3. Management fee portion of late interest - periodic.untrackedManagementFee + latePaymentFee + roundedLateManagementFee, + periodic.untrackedManagementFee + loan->at(sfLatePaymentFee) + roundedLateManagementFee, // Untracked interest includes: // 1. Any untracked interest from the regular payment (usually 0) @@ -958,22 +953,15 @@ std::expected computeFullPayment( Asset const& asset, ApplyView& view, - Number const& principalOutstanding, - Number const& managementFeeOutstanding, - Number const& periodicPayment, - std::uint32_t paymentRemaining, - std::uint32_t prevPaymentDate, - std::uint32_t const startDate, - std::uint32_t const paymentInterval, - TenthBips32 const closeInterestRate, - std::int32_t loanScale, - Number const& totalInterestOutstanding, + SLE::const_ref loan, Number const& periodicRate, - Number const& closePaymentFee, STAmount const& amount, TenthBips16 managementFeeRate, beast::Journal j) { + std::uint32_t const paymentRemaining = loan->at(sfPaymentRemaining); + std::int32_t const loanScale = loan->at(sfLoanScale); + // Full payment must be made before the final scheduled payment. if (paymentRemaining <= 1) { @@ -986,7 +974,7 @@ computeFullPayment( // This theoretical (unrounded) value is used to compute interest and // penalties accurately. Number const theoreticalPrincipalOutstanding = loanPrincipalFromPeriodicPayment( - view.rules(), periodicPayment, periodicRate, paymentRemaining); + view.rules(), loan->at(sfPeriodicPayment), periodicRate, paymentRemaining); // Full payment interest includes both accrued interest (time since last // payment) and prepayment penalty (for closing early). @@ -994,10 +982,10 @@ computeFullPayment( theoreticalPrincipalOutstanding, periodicRate, view.parentCloseTime(), - paymentInterval, - prevPaymentDate, - startDate, - closeInterestRate); + loan->at(sfPaymentInterval), + loan->at(sfPreviousPaymentDueDate), + loan->at(sfStartDate), + TenthBips32{loan->at(sfCloseInterestRate)}); // Split the full payment interest into net interest (to vault) and // management fee (to broker), applying proper rounding. @@ -1007,6 +995,12 @@ computeFullPayment( return computeInterestAndFeeParts(asset, interest, managementFeeRate, loanScale); }(); + LoanState const loanState = constructLoanState(loan); + Number const principalOutstanding = loanState.principalOutstanding; + Number const managementFeeOutstanding = loanState.managementFeeDue; + Number const totalInterestOutstanding = loanState.interestDue; + Number const closePaymentFee = roundToAsset(asset, loan->at(sfClosePaymentFee), loanScale); + ExtendedPaymentComponents const full{ PaymentComponents{ // Pay off all tracked outstanding balances: principal, interest, @@ -1046,8 +1040,7 @@ computeFullPayment( "xrpl::detail::computeFullPayment", "total due is rounded"); - JLOG(j.trace()) << "computeFullPayment result: periodicPayment: " << periodicPayment - << ", periodicRate: " << periodicRate + JLOG(j.trace()) << "computeFullPayment result: periodicRate: " << periodicRate << ", paymentRemaining: " << paymentRemaining << ", theoreticalPrincipalOutstanding: " << theoreticalPrincipalOutstanding << ", fullPaymentInterest: " << fullPaymentInterest @@ -1298,6 +1291,34 @@ computePaymentComponents( }; } +/* Thin overload of computePaymentComponents() that unwraps the tracked + * fields directly from the Loan ledger object. `periodicRate` is derived + * rather than stored, and `managementFeeRate` comes from the LoanBroker, not + * the Loan, so both remain explicit parameters. Kept separate from the + * value-based overload above, which is exercised directly by unit tests + * against simulated (non-ledger) loan states. + */ +PaymentComponents +computePaymentComponents( + Rules const& rules, + Asset const& asset, + SLE::ref loan, + Number const& periodicRate, + TenthBips16 managementFeeRate) +{ + return computePaymentComponents( + rules, + asset, + loan->at(sfLoanScale), + loan->at(sfTotalValueOutstanding), + loan->at(sfPrincipalOutstanding), + loan->at(sfManagementFeeOutstanding), + loan->at(sfPeriodicPayment), + periodicRate, + loan->at(sfPaymentRemaining), + managementFeeRate); +} + /* Computes payment components for an overpayment scenario. * * An overpayment occurs when a borrower pays more than the scheduled periodic @@ -1632,8 +1653,10 @@ constructLoanState( } LoanState -constructRoundedLoanState(SLE::const_ref loan) +constructLoanState(SLE::const_ref loan) { + XRPL_ASSERT(loan && loan->getType() == ltLOAN, "xrpl::constructLoanState : valid loan SLE"); + return constructLoanState( loan->at(sfTotalValueOutstanding), loan->at(sfPrincipalOutstanding), @@ -1792,10 +1815,7 @@ loanMakePayment( { using namespace Lending; - auto principalOutstandingProxy = loan->at(sfPrincipalOutstanding); - auto paymentRemainingProxy = loan->at(sfPaymentRemaining); - - if (paymentRemainingProxy == 0 || principalOutstandingProxy == 0) + if (loan->at(sfPaymentRemaining) == 0 || loan->at(sfPrincipalOutstanding) == 0) { // Loan complete this is already checked in LoanPay::preclaim() // LCOV_EXCL_START @@ -1804,9 +1824,6 @@ loanMakePayment( // LCOV_EXCL_STOP } - auto totalValueOutstandingProxy = loan->at(sfTotalValueOutstanding); - auto managementFeeOutstandingProxy = loan->at(sfManagementFeeOutstanding); - // Next payment due date must be set unless the loan is complete auto nextDueDateProxy = loan->at(sfNextPaymentDueDate); if (*nextDueDateProxy == 0) @@ -1817,24 +1834,17 @@ loanMakePayment( std::int32_t const loanScale = loan->at(sfLoanScale); - TenthBips32 const interestRate{loan->at(sfInterestRate)}; - Number const serviceFee = loan->at(sfLoanServiceFee); TenthBips16 const managementFeeRate{brokerSle->at(sfManagementFeeRate)}; - Number const periodicPayment = loan->at(sfPeriodicPayment); - - auto prevPaymentDateProxy = loan->at(sfPreviousPaymentDueDate); - std::uint32_t const startDate = loan->at(sfStartDate); - - std::uint32_t const paymentInterval = loan->at(sfPaymentInterval); - // Compute the periodic rate that will be used for calculations // throughout - Number const periodicRate = loanPeriodicRate(interestRate, paymentInterval); + TenthBips32 const interestRate{loan->at(sfInterestRate)}; + Number const periodicRate = loanPeriodicRate(interestRate, loan->at(sfPaymentInterval)); XRPL_ASSERT(interestRate == 0 || periodicRate > 0, "xrpl::loanMakePayment : valid rate"); - XRPL_ASSERT(*totalValueOutstandingProxy > 0, "xrpl::loanMakePayment : valid total value"); + XRPL_ASSERT( + *loan->at(sfTotalValueOutstanding) > 0, "xrpl::loanMakePayment : valid total value"); view.update(loan); @@ -1844,11 +1854,11 @@ loanMakePayment( { // If the payment is late, and the late flag was not set, it's not // valid - JLOG(j.warn()) << "Loan payment is overdue. Use the tfLoanLatePayment " - "transaction " - "flag to make a late payment. Loan was created on " - << startDate << ", prev payment due date is " << prevPaymentDateProxy - << ", next payment due date is " << nextDueDateProxy << ", ledger time is " + JLOG(j.warn()) << "Loan payment is overdue. Use the tfLoanLatePayment transaction flag to " + "make a late payment. Loan was created on " + << loan->at(sfStartDate) << ", prev payment due date is " + << loan->at(sfPreviousPaymentDueDate) << ", next payment due date is " + << nextDueDateProxy << ", ledger time is " << view.parentCloseTime().time_since_epoch().count(); return std::unexpected(tecEXPIRED); } @@ -1857,42 +1867,12 @@ loanMakePayment( // full payment handling if (paymentType == LoanPaymentType::Full) { - TenthBips32 const closeInterestRate{loan->at(sfCloseInterestRate)}; - Number const closePaymentFee = roundToAsset(asset, loan->at(sfClosePaymentFee), loanScale); - - LoanState const roundedLoanState = constructLoanState( - totalValueOutstandingProxy, principalOutstandingProxy, managementFeeOutstandingProxy); - auto const fullPaymentComponents = detail::computeFullPayment( - asset, - view, - principalOutstandingProxy, - managementFeeOutstandingProxy, - periodicPayment, - paymentRemainingProxy, - prevPaymentDateProxy, - startDate, - paymentInterval, - closeInterestRate, - loanScale, - roundedLoanState.interestDue, - periodicRate, - closePaymentFee, - amount, - managementFeeRate, - j); + asset, view, loan, periodicRate, amount, managementFeeRate, j); if (fullPaymentComponents.has_value()) { - return doPayment( - *fullPaymentComponents, - totalValueOutstandingProxy, - principalOutstandingProxy, - managementFeeOutstandingProxy, - paymentRemainingProxy, - prevPaymentDateProxy, - nextDueDateProxy, - paymentInterval); + return doPayment(*fullPaymentComponents, loan); } if (fullPaymentComponents.error()) @@ -1915,16 +1895,7 @@ loanMakePayment( // payment is late or regular detail::ExtendedPaymentComponents periodic{ detail::computePaymentComponents( - view.rules(), - asset, - loanScale, - totalValueOutstandingProxy, - principalOutstandingProxy, - managementFeeOutstandingProxy, - periodicPayment, - periodicRate, - paymentRemainingProxy, - managementFeeRate), + view.rules(), asset, loan, periodicRate, managementFeeRate), serviceFee}; XRPL_ASSERT_PARTS( periodic.trackedPrincipalDelta >= 0, @@ -1935,33 +1906,12 @@ loanMakePayment( // late payment handling if (paymentType == LoanPaymentType::Late) { - TenthBips32 const lateInterestRate{loan->at(sfLateInterestRate)}; - Number const latePaymentFee = loan->at(sfLatePaymentFee); - - auto const latePaymentComponents = detail::computeLatePayment( - asset, - view, - principalOutstandingProxy, - nextDueDateProxy, - periodic, - lateInterestRate, - loanScale, - latePaymentFee, - amount, - managementFeeRate, - j); + auto const latePaymentComponents = + detail::computeLatePayment(asset, view, loan, periodic, amount, managementFeeRate, j); if (latePaymentComponents.has_value()) { - return doPayment( - *latePaymentComponents, - totalValueOutstandingProxy, - principalOutstandingProxy, - managementFeeOutstandingProxy, - paymentRemainingProxy, - prevPaymentDateProxy, - nextDueDateProxy, - paymentInterval); + return doPayment(*latePaymentComponents, loan); } if (latePaymentComponents.error()) @@ -1991,7 +1941,7 @@ loanMakePayment( Number totalPaid; std::size_t numPayments = 0; - while ((amount >= (totalPaid + periodic.totalDue)) && paymentRemainingProxy > 0 && + while ((amount >= (totalPaid + periodic.totalDue)) && loan->at(sfPaymentRemaining) > 0 && numPayments < kLoanMaximumPaymentsPerTransaction) { // Try to make more payments @@ -2001,20 +1951,12 @@ loanMakePayment( "payment pays non-negative principal"); totalPaid += periodic.totalDue; - totalParts += detail::doPayment( - periodic, - totalValueOutstandingProxy, - principalOutstandingProxy, - managementFeeOutstandingProxy, - paymentRemainingProxy, - prevPaymentDateProxy, - nextDueDateProxy, - paymentInterval); + totalParts += detail::doPayment(periodic, loan); ++numPayments; XRPL_ASSERT_PARTS( (periodic.specialCase == detail::PaymentSpecialCase::Final) == - (paymentRemainingProxy == 0), + (loan->at(sfPaymentRemaining) == 0), "xrpl::loanMakePayment", "final payment is the final payment"); @@ -2024,16 +1966,7 @@ loanMakePayment( periodic = detail::ExtendedPaymentComponents{ detail::computePaymentComponents( - view.rules(), - asset, - loanScale, - totalValueOutstandingProxy, - principalOutstandingProxy, - managementFeeOutstandingProxy, - periodicPayment, - periodicRate, - paymentRemainingProxy, - managementFeeRate), + view.rules(), asset, loan, periodicRate, managementFeeRate), serviceFee}; } @@ -2063,7 +1996,7 @@ loanMakePayment( ? roundToAsset(asset, amount, loanScale, Number::RoundingMode::TowardsZero) : amount; if (paymentType == LoanPaymentType::Overpayment && loan->isFlag(lsfLoanOverpayment) && - paymentRemainingProxy > 0 && totalPaid < roundedAmount && + loan->at(sfPaymentRemaining) > 0 && totalPaid < roundedAmount && numPayments < kLoanMaximumPaymentsPerTransaction) { TenthBips32 const overpaymentInterestRate{loan->at(sfOverpaymentInterestRate)}; @@ -2073,7 +2006,7 @@ loanMakePayment( // totalValueOutstanding, because that would have been processed as // another normal payment. But cap it just in case. Number const overpaymentRaw = - std::min(roundedAmount - totalPaid, *totalValueOutstandingProxy); + std::min(roundedAmount - totalPaid, *loan->at(sfTotalValueOutstanding)); bool const fixEnabled = view.rules().enabled(fixCleanup3_2_0); Number const overpayment = fixEnabled @@ -2102,20 +2035,13 @@ loanMakePayment( overpaymentComponents.untrackedInterest >= beast::kZero, "xrpl::loanMakePayment", "overpayment penalty did not reduce value of loan"); - // Can't just use `periodicPayment` here, because it might - // change - auto periodicPaymentProxy = loan->at(sfPeriodicPayment); if (auto const overResult = detail::doOverpayment( view.rules(), asset, loanScale, overpaymentComponents, - totalValueOutstandingProxy, - principalOutstandingProxy, - managementFeeOutstandingProxy, - periodicPaymentProxy, + loan, periodicRate, - paymentRemainingProxy, managementFeeRate, j)) { diff --git a/src/test/app/Loan_test.cpp b/src/test/app/Loan_test.cpp index 371fcae54f..231a3b405a 100644 --- a/src/test/app/Loan_test.cpp +++ b/src/test/app/Loan_test.cpp @@ -446,7 +446,7 @@ protected: env.test.BEAST_EXPECT(loan->at(sfPeriodicPayment) == periodicPayment); env.test.BEAST_EXPECT(loan->at(sfFlags) == flags); - auto const ls = constructRoundedLoanState(loan); + auto const ls = constructLoanState(loan); auto const interestRate = TenthBips32{loan->at(sfInterestRate)}; auto const paymentInterval = loan->at(sfPaymentInterval); @@ -1119,7 +1119,7 @@ protected: // No reason for this not to exist return; } - auto const current = constructRoundedLoanState(loanSle); + auto const current = constructLoanState(loanSle); auto const errors = nextTrueState - current; log << currencyLabel << " Loan balances: " << "\n\tAmount taken: " << paymentComponents.trackedValueDelta @@ -6056,7 +6056,7 @@ protected: auto const loanSle = env.le(loanKeylet); if (!BEAST_EXPECT(loanSle)) return; - auto const state = constructRoundedLoanState(loanSle); + auto const state = constructLoanState(loanSle); log << "Loan state:" << std::endl; log << " ValueOutstanding: " << state.valueOutstanding << std::endl;