From 19519c6b19017426f10dc45a29ee38650a3cf4e4 Mon Sep 17 00:00:00 2001 From: Vito <5780819+Tapanito@users.noreply.github.com> Date: Wed, 23 Sep 2026 18:04:05 +0200 Subject: [PATCH] refactor: Extract FixedPrecision LoanPay deltas and tighten coverage Move interest-first AssetsTotal, DebtTotal, and vault-credit rounding into fixed_precision::loanPaymentDeltas, warn before clamping YieldUnrealized, and cover management fees plus early full payoff. --- include/xrpl/ledger/helpers/LendingHelpers.h | 16 ++++++ include/xrpl/ledger/helpers/VaultHelpers.h | 3 +- src/libxrpl/ledger/helpers/LendingHelpers.cpp | 34 ++++++++++++ .../tx/transactors/lending/LoanPay.cpp | 53 +++++++++---------- src/test/app/lending/LendingHelpers_test.cpp | 27 ++++++++++ src/test/app/lending/LoanPay_test.cpp | 41 +++++++++++--- 6 files changed, 140 insertions(+), 34 deletions(-) diff --git a/include/xrpl/ledger/helpers/LendingHelpers.h b/include/xrpl/ledger/helpers/LendingHelpers.h index 0112892ab5..81c5213ff0 100644 --- a/include/xrpl/ledger/helpers/LendingHelpers.h +++ b/include/xrpl/ledger/helpers/LendingHelpers.h @@ -414,6 +414,22 @@ loanPaymentDeltas(LoanPaymentParts const& parts); } // namespace cash_basis +// FixedPrecision payment accounting records interest into AssetsTotal before +// deriving the cash credit sent to the Vault pseudo-account. +namespace fixed_precision { + +struct PaymentDeltas +{ + Number assetsTotalDelta; + Number debtTotalDelta; + Number vaultCredit; +}; + +PaymentDeltas +loanPaymentDeltas(SLE::const_ref vaultSle, LoanPaymentParts const& parts); + +} // namespace fixed_precision + // Public dispatchers: pick cash_basis:: if featureLendingProtocolV1_1 is // enabled AND the Vault's LEVersion (VaultHelpers::getVaultVersion) is // VaultVersion::CashBasis, else instant_recognition::. These are the only entry points diff --git a/include/xrpl/ledger/helpers/VaultHelpers.h b/include/xrpl/ledger/helpers/VaultHelpers.h index 3fc72e8675..1bb307f30c 100644 --- a/include/xrpl/ledger/helpers/VaultHelpers.h +++ b/include/xrpl/ledger/helpers/VaultHelpers.h @@ -60,7 +60,8 @@ roundToPosteriorVaultScale( Number::RoundingMode roundingMode); /** - * Round an amount at the posterior live exponent of AssetsAvailable. + * Round the LoanPay cash-credit delta at the posterior live exponent of + * AssetsAvailable. The reference is AssetsAvailable, not AssetsTotal. */ [[nodiscard]] STAmount roundToPosteriorAvailableScale( diff --git a/src/libxrpl/ledger/helpers/LendingHelpers.cpp b/src/libxrpl/ledger/helpers/LendingHelpers.cpp index 05da0b8323..9e9708eca1 100644 --- a/src/libxrpl/ledger/helpers/LendingHelpers.cpp +++ b/src/libxrpl/ledger/helpers/LendingHelpers.cpp @@ -378,6 +378,40 @@ loanPaymentDeltas(LoanPaymentParts const& parts) } // namespace cash_basis +namespace fixed_precision { + +PaymentDeltas +loanPaymentDeltas(SLE::const_ref vaultSle, LoanPaymentParts const& parts) +{ + XRPL_ASSERT( + vaultSle && vaultSle->getType() == ltVAULT, + "xrpl::fixed_precision::loanPaymentDeltas : valid Vault sle"); + XRPL_ASSERT( + getVaultVersion(vaultSle) == VaultVersion::FixedPrecision, + "xrpl::fixed_precision::loanPaymentDeltas : FixedPrecision Vault"); + + Asset const asset = vaultSle->at(sfAsset); + Number const assetsTotalBefore = vaultSle->at(sfAssetsTotal); + Number const assetsTotalAfter = [&] { + NumberRoundModeGuard const rg(Number::RoundingMode::Downward); + // Floor the posterior rather than the interest delta because a prior + // AssetsTotal may be off the posterior grid when this payment coarsens + // the Vault. The STAmount conversion also clamps integral assets. + return Number{STAmount{asset, assetsTotalBefore + parts.interestPaid}}; + }(); + Number const assetsTotalDelta = assetsTotalAfter - assetsTotalBefore; + Number const creditRaw = parts.principalPaid + assetsTotalDelta; + Number const vaultCredit = roundToPosteriorAvailableScale( + vaultSle, STAmount{asset, creditRaw}, Number::RoundingMode::Downward); + + return { + .assetsTotalDelta = assetsTotalDelta, + .debtTotalDelta = parts.principalPaid, + .vaultCredit = vaultCredit}; +} + +} // namespace fixed_precision + namespace { // Cash-basis accounting applies to Vaults created under diff --git a/src/libxrpl/tx/transactors/lending/LoanPay.cpp b/src/libxrpl/tx/transactors/lending/LoanPay.cpp index ee3da77fe2..92173e2130 100644 --- a/src/libxrpl/tx/transactors/lending/LoanPay.cpp +++ b/src/libxrpl/tx/transactors/lending/LoanPay.cpp @@ -491,30 +491,24 @@ LoanPay::doApply() Number const assetsTotalBefore = *assetsTotalProxy; auto const totalPaidToVaultRaw = paymentParts->principalPaid + paymentParts->interestPaid; - auto const [assetsTotalDelta, debtTotalDelta] = [&] { - if (!fixedPrecision) - return loanPaymentDeltas(vaultSle, *paymentParts); - - Number const assetsTotalAfter = [&] { - NumberRoundModeGuard const rg(Number::RoundingMode::Downward); - // The STAmount conversion also clamps integral assets. - return Number{STAmount{asset, assetsTotalBefore + paymentParts->interestPaid}}; - }(); - return AccountingDeltas{ - .assetsTotalDelta = assetsTotalAfter - assetsTotalBefore, - .debtTotalDelta = paymentParts->principalPaid}; - }(); - Number const totalPaidToVaultRounded = [&] { - if (!fixedPrecision) - { - return roundToAsset( - asset, totalPaidToVaultRaw, vaultScale, Number::RoundingMode::Downward); - } - - Number const creditRaw = paymentParts->principalPaid + assetsTotalDelta; - return Number{roundToPosteriorAvailableScale( - vaultSle, STAmount{asset, creditRaw}, Number::RoundingMode::Downward)}; - }(); + Number assetsTotalDelta; + Number debtTotalDelta; + Number totalPaidToVaultRounded; + if (fixedPrecision) + { + auto const deltas = fixed_precision::loanPaymentDeltas(vaultSle, *paymentParts); + assetsTotalDelta = deltas.assetsTotalDelta; + debtTotalDelta = deltas.debtTotalDelta; + totalPaidToVaultRounded = deltas.vaultCredit; + } + else + { + auto const deltas = loanPaymentDeltas(vaultSle, *paymentParts); + assetsTotalDelta = deltas.assetsTotalDelta; + debtTotalDelta = deltas.debtTotalDelta; + totalPaidToVaultRounded = + roundToAsset(asset, totalPaidToVaultRaw, vaultScale, Number::RoundingMode::Downward); + } XRPL_ASSERT_PARTS( !asset.integral() || totalPaidToVaultRaw == totalPaidToVaultRounded, "xrpl::LoanPay::doApply", @@ -588,7 +582,11 @@ LoanPay::doApply() auto yieldUnrealizedProxy = vaultSle->at(sfYieldUnrealized); yieldUnrealizedProxy += scheduledInterestDelta; if (*yieldUnrealizedProxy < beast::kZero) + { + JLOG(j_.warn()) << "LoanPay: YieldUnrealized became negative before clamping: " + << *yieldUnrealizedProxy; yieldUnrealizedProxy = kNumZero; + } } XRPL_ASSERT_PARTS( @@ -632,9 +630,10 @@ LoanPay::doApply() if (assetsAvailableAfter == assetsAvailableBefore) { // An unchanged assetsAvailable indicates that the amount paid to the - // vault was zero, or rounded to zero. That should be impossible, but I - // can't rule it out for extreme edge cases, so fail gracefully if it - // happens. + // vault was zero, or rounded to zero. FixedPrecision LoanSet requires + // positive first-payment principal, and no transaction-generated + // schedule currently produces a non-terminal zero-credit payment. + // Fail gracefully if an extreme edge case still reaches this branch. // // LCOV_EXCL_START JLOG(j_.warn()) << "LoanPay: Vault assets available unchanged after rounding: " // diff --git a/src/test/app/lending/LendingHelpers_test.cpp b/src/test/app/lending/LendingHelpers_test.cpp index 642986035c..aefc2e0d98 100644 --- a/src/test/app/lending/LendingHelpers_test.cpp +++ b/src/test/app/lending/LendingHelpers_test.cpp @@ -1667,6 +1667,32 @@ class LendingHelpers_test : public beast::unit_test::Suite } } + void + testFixedPrecisionLoanPaymentDeltas() + { + testcase("fixed_precision::loanPaymentDeltas floors posterior AssetsTotal"); + + using namespace jtx; + + Env const env{*this}; + Account const issuer{"issuer"}; + PrettyAsset const asset = issuer["USD"]; + auto vault = std::make_shared(ltVAULT, uint256{2u}); + vault->setFieldIssue(sfAsset, STIssue{sfAsset, asset}); + vault->at(sfAssetsTotal) = Number{9'999'999'999'999'999, -6}; + vault->at(sfAssetsAvailable) = Number{9'999'999'999'999'999, -6}; + vault->at(sfScale) = 6; + vault->at(sfLEVersion) = std::to_underlying(VaultVersion::FixedPrecision); + associateAsset(*vault, asset); + + LoanPaymentParts const parts{.principalPaid = Number{1, -5}, .interestPaid = Number{5, -6}}; + auto const deltas = fixed_precision::loanPaymentDeltas(vault, parts); + + BEAST_EXPECT((deltas.assetsTotalDelta == Number{1, -6})); + BEAST_EXPECT(deltas.debtTotalDelta == parts.principalPaid); + BEAST_EXPECT((deltas.vaultCredit == Number{1, -5})); + } + void testLoanOriginationDeltasDispatcher() { @@ -2050,6 +2076,7 @@ public: testInstantRecognitionLoanVaultExposure(); testCashBasisLoanVaultExposure(); testLoanPaymentDeltas(); + testFixedPrecisionLoanPaymentDeltas(); testLoanOriginationDeltasDispatcher(); testLoanOriginationExceedsVaultMaximumDispatcher(); testLoanVaultExposureDispatcher(); diff --git a/src/test/app/lending/LoanPay_test.cpp b/src/test/app/lending/LoanPay_test.cpp index c3660e523a..25587545c4 100644 --- a/src/test/app/lending/LoanPay_test.cpp +++ b/src/test/app/lending/LoanPay_test.cpp @@ -1510,18 +1510,19 @@ private: env(trust(lender, asset(10'000'000))); env(trust(borrower, asset(10'000'000))); env(pay(issuer, lender, asset(2'000'000))); + env(pay(issuer, borrower, asset(100))); env.close(); BrokerParameters brokerParams; brokerParams.vaultScale = 6; - brokerParams.managementFeeRate = TenthBips16{0}; + brokerParams.managementFeeRate = TenthBips16{100}; auto const broker = createVaultAndBroker(env, asset, lender, brokerParams); auto const loanKeylet = nextLoanKeylet(env, broker); env(set(borrower, broker.brokerID, asset(1'000).value()), Sig(sfCounterpartySignature, lender), kInterestRate(percentageToTenthBips(12)), - kPaymentTotal(2), + kPaymentTotal(3), kPaymentInterval(24 * 60 * 60), Fee(env.current()->fees().base * 2)); env.close(); @@ -1541,8 +1542,11 @@ private: Number const yieldBefore = vaultBefore->at(sfYieldUnrealized); Number const debtBefore = brokerBefore->at(sfDebtTotal); Number const principalBefore = loanBefore->at(sfPrincipalOutstanding); - Number const scheduledInterestBefore = loanBefore->at(sfTotalValueOutstanding) - - principalBefore - loanBefore->at(sfManagementFeeOutstanding); + Number const managementFeeBefore = loanBefore->at(sfManagementFeeOutstanding); + Number const scheduledInterestBefore = + loanBefore->at(sfTotalValueOutstanding) - principalBefore - managementFeeBefore; + Number const lenderBalanceBefore = env.balance(lender, asset).number(); + BEAST_EXPECT(managementFeeBefore > beast::kZero); env(pay(borrower, loanKeylet.key, payment)); env.close(); @@ -1556,8 +1560,9 @@ private: Number const principalAfter = loanAfter->at(sfPrincipalOutstanding); Number const principalPaid = principalBefore - principalAfter; Number const credit = vaultAfter->at(sfAssetsAvailable) - assetsAvailableBefore; - Number const scheduledInterestAfter = loanAfter->at(sfTotalValueOutstanding) - - principalAfter - loanAfter->at(sfManagementFeeOutstanding); + Number const managementFeeAfter = loanAfter->at(sfManagementFeeOutstanding); + Number const scheduledInterestAfter = + loanAfter->at(sfTotalValueOutstanding) - principalAfter - managementFeeAfter; Number const interestPaid = scheduledInterestBefore - scheduledInterestAfter; Number const expectedAssetsTotal = [&] { NumberRoundModeGuard const rg(Number::RoundingMode::Downward); @@ -1570,7 +1575,11 @@ private: vaultAfter->at(sfAssetsTotal) == expectedAssetsTotal, "AssetsTotal expected " + to_string(expectedAssetsTotal) + ", got " + to_string(vaultAfter->at(sfAssetsTotal))); + BEAST_EXPECT(expectedAssetsTotal == assetsTotalBefore + interestPaid); BEAST_EXPECT(credit == principalPaid + interestActual); + BEAST_EXPECT( + env.balance(lender, asset).number() - lenderBalanceBefore == + managementFeeBefore - managementFeeAfter); BEAST_EXPECT( vaultAfter->at(sfYieldUnrealized) == yieldBefore + scheduledInterestAfter - scheduledInterestBefore); @@ -1579,6 +1588,24 @@ private: assetsTotalBefore + yieldBefore, "capacity before " + to_string(assetsTotalBefore + yieldBefore) + ", after " + to_string(vaultAfter->at(sfAssetsTotal) + vaultAfter->at(sfYieldUnrealized))); + + Number const yieldBeforeFull = vaultAfter->at(sfYieldUnrealized); + BEAST_EXPECT(scheduledInterestAfter > beast::kZero); + BEAST_EXPECT(yieldBeforeFull > beast::kZero); + + Number const fullPaymentMaximum = env.balance(borrower, asset).number(); + env(pay(borrower, loanKeylet.key, asset(fullPaymentMaximum), tfLoanFullPayment)); + env.close(); + + auto const vaultAfterFull = env.le(broker.vaultKeylet()); + auto const loanAfterFull = env.le(loanKeylet); + if (!BEAST_EXPECT(vaultAfterFull && loanAfterFull)) + return; + BEAST_EXPECT(loanAfterFull->at(sfPaymentRemaining) == 0); + BEAST_EXPECT(loanAfterFull->at(sfTotalValueOutstanding) == beast::kZero); + BEAST_EXPECT(loanAfterFull->at(sfPrincipalOutstanding) == beast::kZero); + BEAST_EXPECT(loanAfterFull->at(sfManagementFeeOutstanding) == beast::kZero); + BEAST_EXPECT(vaultAfterFull->at(sfYieldUnrealized) == beast::kZero); } void @@ -2262,6 +2289,8 @@ private: testFixedPrecisionSpecialPayments(); testFixedPrecisionIntegralPayments(); testFixedPrecisionYieldAcrossLoans(); + testOverpaymentManagementFee( + all_ | featureLendingProtocolV1_1 | featureLendingProtocolV1_2); } // Tests run under each entry in amendmentCombinations().