From f68b3326dfd431014646085b2d14a70234666ad8 Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Tue, 31 Mar 2026 21:02:28 -0400 Subject: [PATCH] Review feedback from @tapanito - Return a default value in LoanPay balanceScale if the exponent list is empty. - Amendment gate the "roundedAmount" change in overpayment. - Improve comments and logging. documentation --- .../tx/transactors/lending/LendingHelpers.cpp | 11 +++++++++- .../tx/transactors/lending/LoanPay.cpp | 21 +++++++++++++------ 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/src/libxrpl/tx/transactors/lending/LendingHelpers.cpp b/src/libxrpl/tx/transactors/lending/LendingHelpers.cpp index 3fa42037fa..2405cee21d 100644 --- a/src/libxrpl/tx/transactors/lending/LendingHelpers.cpp +++ b/src/libxrpl/tx/transactors/lending/LendingHelpers.cpp @@ -1824,7 +1824,16 @@ loanMakePayment( // ------------------------------------------------------------- // overpayment handling - auto const roundedAmount = roundToAsset(asset, amount, loanScale, Number::towards_zero); + // + // If the "fixSecurity3_1_3" amendment is enabled, truncate "amount", + // at the loan scale. If the raw value is used, the overpayment + // amount could be meaningless dust. Trying to process such a small + // amount will, at best, waste time when all the result values round + // to zero. At worst, it can cause logical errors with tiny amounts + // of interest that don't add up correctly. + auto const roundedAmount = view.rules().enabled(fixSecurity3_1_3) + ? roundToAsset(asset, amount, loanScale, Number::towards_zero) + : amount; if (paymentType == LoanPaymentType::overpayment && loan->isFlag(lsfLoanOverpayment) && paymentRemainingProxy > 0 && totalPaid < roundedAmount && numPayments < loanMaximumPaymentsPerTransaction) diff --git a/src/libxrpl/tx/transactors/lending/LoanPay.cpp b/src/libxrpl/tx/transactors/lending/LoanPay.cpp index a59c95066e..a45448bdf1 100644 --- a/src/libxrpl/tx/transactors/lending/LoanPay.cpp +++ b/src/libxrpl/tx/transactors/lending/LoanPay.cpp @@ -515,10 +515,10 @@ LoanPay::doApply() // something really weird happened. That should be flat out impossible. // // LCOV_EXCL_START - JLOG(j_.warn()) << "LoanPay: Vault assets changed unexpectedly after rounding: " // - << "Before: " << assetsTotalBefore // - << ", After: " << assetsTotalAfter // - << ", ValueChange: " << paymentParts->valueChange; + JLOG(j_.fatal()) << "LoanPay: Vault assets changed unexpectedly after rounding: " // + << "Before: " << assetsTotalBefore // + << ", After: " << assetsTotalAfter // + << ", ValueChange: " << paymentParts->valueChange; return tecINTERNAL; // LCOV_EXCL_STOP } @@ -642,8 +642,12 @@ LoanPay::doApply() j_, SpendableHandling::shFULL_BALANCE); auto const balanceScale = [&]() { - // Find the maximum exponent of all the non-zero balances, before and after. - // This is so ugly. + // Find a reasonable scale to use for the balance comparisons. + // + // First find the minimum and maximum exponent of all the non-zero balances, before and + // after. If min and max are equal, use that value. If they are not, use "max + 1" to reduce + // rounding discrepancies without making the result meaningless. Cap the scale at + // STAmount::cMaxOffset, just in case the numbers are all very large. std::vector exponents; for (auto const& a : { @@ -659,6 +663,11 @@ LoanPay::doApply() if (a != beast::zero) exponents.push_back(a.exponent()); } + if (exponents.empty()) + { + UNREACHABLE("xrpl::LoanPay::doApply : all zeroes"); + return 0; + } auto const [minItr, maxItr] = std::minmax_element(exponents.begin(), exponents.end()); auto const min = *minItr; auto const max = *maxItr;