diff --git a/src/test/app/LendingHelpers_test.cpp b/src/test/app/LendingHelpers_test.cpp index ec2d4e99a2..9506c94562 100644 --- a/src/test/app/LendingHelpers_test.cpp +++ b/src/test/app/LendingHelpers_test.cpp @@ -668,23 +668,14 @@ class LendingHelpers_test : public beast::unit_test::suite std::cout << loanProperites.loanState.interestOutstanding() << std::endl; - Number totalValueOutstanding = - loanProperites.loanState.valueOutstanding; - Number principalOutstanding = - loanProperites.loanState.principalOutstanding; - Number managementFeeOutstanding = - loanProperites.loanState.managementFeeDue; Number periodicPayment = loanProperites.periodicPayment; auto const ret = tryOverpayment( asset, loanScale, overpaymentComponents, - totalValueOutstanding, - principalOutstanding, - managementFeeOutstanding, + loanProperites.loanState, periodicPayment, - paymentInterval, periodicRate, paymentsRemaining, managementFeeRate, @@ -692,50 +683,45 @@ class LendingHelpers_test : public beast::unit_test::suite BEAST_EXPECT(ret); - auto const& actualPaymentParts = *ret; + auto const& [actualPaymentParts, newLoanProperties] = *ret; + auto const newState = newLoanProperties.loanState; BEAST_EXPECTS( actualPaymentParts.valueChange == - ((totalValueOutstanding - principalOutstanding - - managementFeeOutstanding)) - - loanProperites.loanState.interestDue, + newState.interestDue - loanProperites.loanState.interestDue, " valueChange mismatch: expected " + to_string( - (totalValueOutstanding - principalOutstanding - - managementFeeOutstanding) - + newState.interestDue - loanProperites.loanState.interestDue) + ", got " + to_string(actualPaymentParts.valueChange)); BEAST_EXPECTS( actualPaymentParts.feePaid == loanProperites.loanState.managementFeeDue - - managementFeeOutstanding, + newState.managementFeeDue, " feePaid mismatch: expected " + to_string( loanProperites.loanState.managementFeeDue - - managementFeeOutstanding) + + newState.managementFeeDue) + ", got " + to_string(actualPaymentParts.feePaid)); BEAST_EXPECTS( actualPaymentParts.principalPaid == loanProperites.loanState.principalOutstanding - - principalOutstanding, + newState.principalOutstanding, " principalPaid mismatch: expected " + to_string( loanProperites.loanState.principalOutstanding - - principalOutstanding) + + newState.principalOutstanding) + ", got " + to_string(actualPaymentParts.principalPaid)); BEAST_EXPECTS( actualPaymentParts.interestPaid == - loanProperites.loanState.interestDue - - (totalValueOutstanding - principalOutstanding - - managementFeeOutstanding), + loanProperites.loanState.interestDue - newState.interestDue, " interestPaid mismatch: expected " + to_string( loanProperites.loanState.interestDue - - (totalValueOutstanding - principalOutstanding - - managementFeeOutstanding)) + + newState.interestDue) + ", got " + to_string(actualPaymentParts.interestPaid)); } diff --git a/src/xrpld/app/misc/LendingHelpers.h b/src/xrpld/app/misc/LendingHelpers.h index e2a100d1c4..84c190081b 100644 --- a/src/xrpld/app/misc/LendingHelpers.h +++ b/src/xrpld/app/misc/LendingHelpers.h @@ -361,16 +361,13 @@ struct LoanStateDeltas nonNegative(); }; -Expected +Expected, TER> tryOverpayment( Asset const& asset, std::int32_t loanScale, ExtendedPaymentComponents const& overpaymentComponents, - Number& totalValueOutstanding, - Number& principalOutstanding, - Number& managementFeeOutstanding, - Number& periodicPayment, - std::uint32_t paymentInterval, + LoanState const& roundedLoanState, + Number const& periodicPayment, Number const& periodicRate, std::uint32_t paymentRemaining, TenthBips16 const managementFeeRate, diff --git a/src/xrpld/app/misc/detail/LendingHelpers.cpp b/src/xrpld/app/misc/detail/LendingHelpers.cpp index e620a30061..74773731b5 100644 --- a/src/xrpld/app/misc/detail/LendingHelpers.cpp +++ b/src/xrpld/app/misc/detail/LendingHelpers.cpp @@ -2,6 +2,8 @@ // DO NOT REMOVE forces header file include to sort first #include +#include + namespace ripple { bool @@ -382,16 +384,13 @@ doPayment( * The function preserves accumulated rounding errors across the re-amortization * to ensure the loan state remains consistent with its payment history. */ -Expected +Expected, TER> tryOverpayment( Asset const& asset, std::int32_t loanScale, ExtendedPaymentComponents const& overpaymentComponents, - Number& totalValueOutstanding, - Number& principalOutstanding, - Number& managementFeeOutstanding, - Number& periodicPayment, - std::uint32_t paymentInterval, + LoanState const& roundedOldState, + Number const& periodicPayment, Number const& periodicRate, std::uint32_t paymentRemaining, TenthBips16 const managementFeeRate, @@ -401,15 +400,11 @@ tryOverpayment( auto const raw = computeRawLoanState( periodicPayment, periodicRate, paymentRemaining, managementFeeRate); - // Get the actual loan state (with accumulated rounding from past payments) - auto const rounded = constructLoanState( - totalValueOutstanding, principalOutstanding, managementFeeOutstanding); - // Calculate the accumulated rounding errors. These need to be preserved // across the re-amortization to maintain consistency with the loan's // payment history. Without preserving these errors, the loan could end // up with a different total value than what the borrower has actually paid. - auto const errors = rounded - raw; + auto const errors = roundedOldState - raw; // Compute the new principal by applying the overpayment to the raw // (theoretical) principal. Use max with 0 to ensure we never go negative. @@ -450,37 +445,35 @@ tryOverpayment( // rounding errors. This ensures the loan's tracked state remains // consistent with its payment history. - principalOutstanding = std::clamp( + auto const principalOutstanding = std::clamp( roundToAsset( asset, newRaw.principalOutstanding, loanScale, Number::upward), numZero, - rounded.principalOutstanding); - totalValueOutstanding = std::clamp( + roundedOldState.principalOutstanding); + auto const totalValueOutstanding = std::clamp( roundToAsset( asset, principalOutstanding + newRaw.interestOutstanding(), loanScale, Number::upward), numZero, - rounded.valueOutstanding); - managementFeeOutstanding = std::clamp( + roundedOldState.valueOutstanding); + auto const managementFeeOutstanding = std::clamp( roundToAsset(asset, newRaw.managementFeeDue, loanScale), numZero, - rounded.managementFeeDue); + roundedOldState.managementFeeDue); - auto const newRounded = constructLoanState( + auto const roundedNewState = constructLoanState( totalValueOutstanding, principalOutstanding, managementFeeOutstanding); // Update newLoanProperties so that checkLoanGuards can make an accurate // evaluation. - newLoanProperties.loanState.valueOutstanding = newRounded.valueOutstanding; + newLoanProperties.loanState = roundedNewState; - JLOG(j.debug()) << "new rounded value: " << newRounded.valueOutstanding - << ", principal: " << newRounded.principalOutstanding - << ", interest gross: " << newRounded.interestOutstanding(); - - // Update the periodic payment to reflect the re-amortized schedule - periodicPayment = newLoanProperties.periodicPayment; + JLOG(j.debug()) << "new rounded value: " << roundedNewState.valueOutstanding + << ", principal: " << roundedNewState.principalOutstanding + << ", interest gross: " + << roundedNewState.interestOutstanding(); // check that the loan is still valid if (auto const ter = checkLoanGuards( @@ -490,7 +483,7 @@ tryOverpayment( // small interest amounts, that may have already been paid // off. Check what's still outstanding. This should // guarantee that the interest checks pass. - newRounded.interestOutstanding() != beast::zero, + roundedNewState.interestOutstanding() != beast::zero, paymentRemaining, newLoanProperties, j)) @@ -520,19 +513,16 @@ tryOverpayment( // LCOV_EXCL_STOP } - auto const deltas = rounded - newRounded; + auto const deltas = roundedOldState - roundedNewState; // The change in loan management fee is equal to the change between the old // and the new outstanding management fees XRPL_ASSERT_PARTS( deltas.managementFee == - rounded.managementFeeDue - managementFeeOutstanding, + roundedOldState.managementFeeDue - managementFeeOutstanding, "ripple::detail::tryOverpayment", "no fee change"); - auto const hypotheticalValueOutstanding = - rounded.valueOutstanding - deltas.principal; - // Calculate how the loan's value changed due to the overpayment. // This should be negative (value decreased) or zero. A principal // overpayment should never increase the loan's value. @@ -543,21 +533,28 @@ tryOverpayment( "the loan. Ignore the overpayment"; return Unexpected(tesSUCCESS); } - return LoanPaymentParts{ - // Principal paid is the reduction in principal outstanding - .principalPaid = deltas.principal, - // Interest paid is the reduction in interest due - .interestPaid = - deltas.interest + overpaymentComponents.untrackedInterest, - // Value change includes both the reduction from paying down principal - // (negative) and any untracked interest penalties (positive, e.g., if - // the overpayment itself incurs a fee) - .valueChange = - valueChange + overpaymentComponents.trackedInterestPart(), - // Fee paid includes both the reduction in tracked management fees and - // any untracked fees on the overpayment itself - .feePaid = deltas.managementFee + - overpaymentComponents.untrackedManagementFee}; + + return std::make_pair( + LoanPaymentParts{ + // Principal paid is the reduction in principal outstanding + .principalPaid = deltas.principal, + // Interest paid is the reduction in interest due + .interestPaid = + deltas.interest + overpaymentComponents.untrackedInterest, + // Value change includes both the reduction from paying down + // principal + // (negative) and any untracked interest penalties (positive, e.g., + // if + // the overpayment itself incurs a fee) + .valueChange = + valueChange + overpaymentComponents.trackedInterestPart(), + // Fee paid includes both the reduction in tracked management fees + // and + // any untracked fees on the overpayment itself + .feePaid = deltas.managementFee + + overpaymentComponents.untrackedManagementFee, + }, + newLoanProperties); } /* Validates and applies an overpayment to the loan state. @@ -581,20 +578,16 @@ doOverpayment( NumberProxy& principalOutstandingProxy, NumberProxy& managementFeeOutstandingProxy, NumberProxy& periodicPaymentProxy, - std::uint32_t const paymentInterval, Number const& periodicRate, std::uint32_t const paymentRemaining, TenthBips16 const managementFeeRate, beast::Journal j) { - // Create temporary copies of the loan state that can be safely modified - // and discarded if the overpayment doesn't work out. This prevents - // corrupting the actual ledger data if validation fails. - Number totalValueOutstanding = totalValueOutstandingProxy; - Number principalOutstanding = principalOutstandingProxy; - Number managementFeeOutstanding = managementFeeOutstandingProxy; - Number periodicPayment = periodicPaymentProxy; - + auto const loanState = constructLoanState( + totalValueOutstandingProxy, + principalOutstandingProxy, + managementFeeOutstandingProxy); + auto const periodicPayment = periodicPaymentProxy; JLOG(j.debug()) << "overpayment components:" << ", totalValue before: " << *totalValueOutstandingProxy @@ -613,11 +606,8 @@ doOverpayment( asset, loanScale, overpaymentComponents, - totalValueOutstanding, - principalOutstanding, - managementFeeOutstanding, + loanState, periodicPayment, - paymentInterval, periodicRate, paymentRemaining, managementFeeRate, @@ -625,18 +615,19 @@ doOverpayment( if (!ret) return Unexpected(ret.error()); - auto const& loanPaymentParts = *ret; + auto const& [loanPaymentParts, newLoanProperties] = *ret; + auto const newRoundedLoanState = newLoanProperties.loanState; // Safety check: the principal must have decreased. If it didn't (or // increased!), something went wrong in the calculation and we should // reject the overpayment. - if (principalOutstandingProxy <= principalOutstanding) + if (principalOutstandingProxy <= newRoundedLoanState.principalOutstanding) { // LCOV_EXCL_START JLOG(j.warn()) << "Overpayment not allowed: principal " << "outstanding did not decrease. Before: " - << *principalOutstandingProxy - << ". After: " << principalOutstanding; + << *principalOutstandingProxy << ". After: " + << newRoundedLoanState.principalOutstanding; return Unexpected(tesSUCCESS); // LCOV_EXCL_STOP } @@ -647,28 +638,29 @@ doOverpayment( XRPL_ASSERT_PARTS( overpaymentComponents.trackedPrincipalDelta == - principalOutstandingProxy - principalOutstanding, + principalOutstandingProxy - + newRoundedLoanState.principalOutstanding, "ripple::detail::doOverpayment", "principal change agrees"); // I'm not 100% sure the following asserts are correct. If in doubt, and // everything else works, remove any that cause trouble. - JLOG(j.debug()) << "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); + JLOG(j.debug()) + << "valueChange: " << loanPaymentParts.valueChange + << ", totalValue before: " << *totalValueOutstandingProxy + << ", totalValue after: " << newRoundedLoanState.valueOutstanding + << ", totalValue delta: " + << (totalValueOutstandingProxy - newRoundedLoanState.valueOutstanding) + << ", principalDelta: " << overpaymentComponents.trackedPrincipalDelta + << ", principalPaid: " << loanPaymentParts.principalPaid + << ", Computed difference: " + << overpaymentComponents.trackedPrincipalDelta - + (totalValueOutstandingProxy - newRoundedLoanState.valueOutstanding); XRPL_ASSERT_PARTS( loanPaymentParts.valueChange == - totalValueOutstanding - + newRoundedLoanState.valueOutstanding - (totalValueOutstandingProxy - overpaymentComponents.trackedPrincipalDelta) + overpaymentComponents.trackedInterestPart(), @@ -683,10 +675,10 @@ doOverpayment( // All validations passed, so update the proxy objects (which will // modify the actual Loan ledger object) - totalValueOutstandingProxy = totalValueOutstanding; - principalOutstandingProxy = principalOutstanding; - managementFeeOutstandingProxy = managementFeeOutstanding; - periodicPaymentProxy = periodicPayment; + totalValueOutstandingProxy = newRoundedLoanState.valueOutstanding; + principalOutstandingProxy = newRoundedLoanState.principalOutstanding; + managementFeeOutstandingProxy = newRoundedLoanState.managementFeeDue; + periodicPaymentProxy = newLoanProperties.periodicPayment; return loanPaymentParts; } @@ -1955,7 +1947,6 @@ loanMakePayment( principalOutstandingProxy, managementFeeOutstandingProxy, periodicPaymentProxy, - paymentInterval, periodicRate, paymentRemainingProxy, managementFeeRate,