From 9f4cdb80cd14e04678c070fa867500aaaaa311af Mon Sep 17 00:00:00 2001 From: Vito <5780819+Tapanito@users.noreply.github.com> Date: Tue, 11 Aug 2026 15:27:52 +0200 Subject: [PATCH] fix: Eliminate cross-scale arithmetic in LoanPay::doApply Gated behind fixCleanup3_4_0. Pre-amendment, the payment path used the same cross-scale pattern that `LoanManage::defaultLoan` addressed: `totalPaidToVault` was rounded down to `vaultScale` before being applied to `sfAssetsAvailable`, while `assetsTotalDelta` (`valueChange` for accrual, `interestPaid` for cash-basis) was applied at the finer loan scale. The `sfAssetsAvailable <= sfAssetsTotal` invariant was only defended by an assertion after the fact. Under the fix, drop the `roundToAsset(..., vaultScale, Downward)` step and apply the raw `totalPaidToVault` to both the ledger update and the cash transfer via `accountSendMulti`. The pseudo-account balance and `sfAssetsAvailable` therefore land on the same STAmount value without an intermediate scale reduction. For integral assets this is a no-op (the removed rounding was already a no-op there). For non-integral IOUs, the vault receives the full raw payment (up to 1 vaultScale ULP more than pre-amendment) and the existing `tecPRECISION_LOSS` / `tecINTERNAL` post-checks are no longer needed as safety nets against an arithmetic-induced Available > Total. `LoanPay_test` now runs the amendment-sensitive tests under both branches by adding `fixCleanup3_4_0` to the `amendmentCombinations` matrix, matching the sibling change to `LoanRounding_test`. --- .../tx/transactors/lending/LoanPay.cpp | 30 ++++++++++++++----- src/test/app/lending/LoanPay_test.cpp | 2 +- 2 files changed, 23 insertions(+), 9 deletions(-) diff --git a/src/libxrpl/tx/transactors/lending/LoanPay.cpp b/src/libxrpl/tx/transactors/lending/LoanPay.cpp index 4619540295..def96e66ce 100644 --- a/src/libxrpl/tx/transactors/lending/LoanPay.cpp +++ b/src/libxrpl/tx/transactors/lending/LoanPay.cpp @@ -436,10 +436,24 @@ LoanPay::doApply() auto assetsTotalProxy = vaultSle->at(sfAssetsTotal); auto const totalPaidToVaultRaw = paymentParts->principalPaid + paymentParts->interestPaid; - auto const totalPaidToVaultRounded = - roundToAsset(asset, totalPaidToVaultRaw, vaultScale, Number::RoundingMode::Downward); + // Under fixCleanup3_4_0, feed the same raw payment Number into both + // sfAssetsAvailable and the accountSendMulti transfer to the vault + // pseudo-account. Both sinks store the value as an asset-typed + // STAmount (the ledger field is normalized through STNumber via + // associateAsset(*vaultSle, asset) further below, and the pseudo + // account trust line stores an STAmount directly), so the IOU + // normalization is applied symmetrically and the two balances land on + // the same STAmount value -- which is what makes the debug invariant + // sfAssetsAvailable == pseudo-account balance hold. Pre-amendment, + // sfAssetsAvailable was fed the vaultScale-rounded value while the + // pseudo-account received the raw Number, creating the same + // cross-side asymmetry with `assetsTotalDelta` that + // `LoanManage::defaultLoan` addresses on the default path. + auto const totalPaidToVault = view.rules().enabled(fixCleanup3_4_0) + ? totalPaidToVaultRaw + : roundToAsset(asset, totalPaidToVaultRaw, vaultScale, Number::RoundingMode::Downward); XRPL_ASSERT_PARTS( - !asset.integral() || totalPaidToVaultRaw == totalPaidToVaultRounded, + !asset.integral() || totalPaidToVaultRaw == totalPaidToVault, "xrpl::LoanPay::doApply", "rounding does nothing for integral asset"); auto const totalPaidToBroker = paymentParts->feePaid; @@ -485,7 +499,7 @@ LoanPay::doApply() } #endif - assetsAvailableProxy += totalPaidToVaultRounded; + assetsAvailableProxy += totalPaidToVault; assetsTotalProxy += assetsTotalDelta; XRPL_ASSERT_PARTS( @@ -494,13 +508,13 @@ LoanPay::doApply() "assets available must not be greater than assets outstanding"); JLOG(j_.debug()) << "total paid to vault raw: " << totalPaidToVaultRaw - << ", total paid to vault rounded: " << totalPaidToVaultRounded + << ", total paid to vault: " << totalPaidToVault << ", total paid to broker: " << totalPaidToBroker << ", amount from transaction: " << amount; // Move funds XRPL_ASSERT_PARTS( - totalPaidToVaultRounded + totalPaidToBroker <= amount, + totalPaidToVault + totalPaidToBroker <= amount, "xrpl::LoanPay::doApply", "amount is sufficient"); @@ -610,7 +624,7 @@ LoanPay::doApply() j_, SpendableHandling::FullBalance); - if (totalPaidToVaultRounded != beast::kZero) + if (totalPaidToVault != beast::kZero) { if (auto const ter = requireAuth(view, asset, vaultPseudoAccount, AuthType::StrongAuth)) return ter; @@ -642,7 +656,7 @@ LoanPay::doApply() view, accountID_, asset, - {{vaultPseudoAccount, totalPaidToVaultRounded}, {brokerPayee, totalPaidToBroker}}, + {{vaultPseudoAccount, totalPaidToVault}, {brokerPayee, totalPaidToBroker}}, j_, WaiveTransferFee::Yes)) return ter; diff --git a/src/test/app/lending/LoanPay_test.cpp b/src/test/app/lending/LoanPay_test.cpp index 9d840fe1bf..d36a635b74 100644 --- a/src/test/app/lending/LoanPay_test.cpp +++ b/src/test/app/lending/LoanPay_test.cpp @@ -752,7 +752,7 @@ public: { runAmendmentIndependent(); for (auto const& features : jtx::amendmentCombinations( - {fixCleanup3_1_3, fixCleanup3_2_0, featureMPTokensV2}, all_)) + {fixCleanup3_1_3, fixCleanup3_2_0, fixCleanup3_4_0, featureMPTokensV2}, all_)) runAmendmentSensitive(features); } };