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
This commit is contained in:
Ed Hennis
2026-03-31 21:02:28 -04:00
parent da1cb2d7ce
commit f68b3326df
2 changed files with 25 additions and 7 deletions

View File

@@ -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)

View File

@@ -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<int> 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;