From e09949ccd7bea3f39de0b4c137d5ef66aa2b5667 Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Tue, 14 Apr 2026 18:09:51 -0400 Subject: [PATCH] fix: Fix touchy "funds are conserved" assertion in LoanPay (#6231) --- include/xrpl/basics/Number.h | 6 +- include/xrpl/protocol/STAmount.h | 6 +- src/libxrpl/basics/Number.cpp | 21 +- src/libxrpl/protocol/STAmount.cpp | 2 +- src/libxrpl/protocol/STNumber.cpp | 8 +- src/test/app/Loan_test.cpp | 144 ++++++++++- src/xrpld/app/misc/detail/LendingHelpers.cpp | 16 +- src/xrpld/app/tx/detail/LoanPay.cpp | 246 ++++++++++++++++--- src/xrpld/app/tx/detail/Transactor.cpp | 2 +- src/xrpld/app/tx/detail/VaultClawback.cpp | 2 +- 10 files changed, 380 insertions(+), 73 deletions(-) diff --git a/include/xrpl/basics/Number.h b/include/xrpl/basics/Number.h index b311ff3cbf..709fb87f8b 100644 --- a/include/xrpl/basics/Number.h +++ b/include/xrpl/basics/Number.h @@ -594,7 +594,7 @@ Number::mantissa() const noexcept { XRPL_ASSERT_PARTS( !isnormal() || (m % 10 == 0 && m / 10 <= maxRep), - "xrpl::Number::mantissa", + "ripple::Number::mantissa", "large normalized mantissa has no remainder"); m /= 10; } @@ -615,7 +615,7 @@ Number::exponent() const noexcept { XRPL_ASSERT_PARTS( !isnormal() || (mantissa_ % 10 == 0 && mantissa_ / 10 <= maxRep), - "xrpl::Number::exponent", + "ripple::Number::exponent", "large normalized mantissa has no remainder"); ++e; } @@ -748,7 +748,7 @@ Number::normalizeToRange(T minMantissa, T maxMantissa) const if constexpr (std::is_unsigned_v) XRPL_ASSERT_PARTS( !negative, - "xrpl::Number::normalizeToRange", + "ripple::Number::normalizeToRange", "Number is non-negative for unsigned range."); Number::normalize(negative, mantissa, exponent, minMantissa, maxMantissa); diff --git a/include/xrpl/protocol/STAmount.h b/include/xrpl/protocol/STAmount.h index 26bbcaf82d..1398224d3c 100644 --- a/include/xrpl/protocol/STAmount.h +++ b/include/xrpl/protocol/STAmount.h @@ -62,8 +62,8 @@ private: public: using value_type = STAmount; - static int const cMinOffset = -96; - static int const cMaxOffset = 80; + static constexpr int cMinOffset = -96; + static constexpr int cMaxOffset = 80; // Maximum native value supported by the code constexpr static std::uint64_t cMinValue = 1'000'000'000'000'000ull; @@ -372,7 +372,7 @@ STAmount::STAmount( { XRPL_ASSERT( mValue <= std::numeric_limits::max(), - "xrpl::STAmount::STAmount(SField, A, std::uint64_t, int, bool) : " + "ripple::STAmount::STAmount(SField, A, std::uint64_t, int, bool) : " "maximum mantissa input"); } else diff --git a/src/libxrpl/basics/Number.cpp b/src/libxrpl/basics/Number.cpp index ebd93aef08..2c36e8fe85 100644 --- a/src/libxrpl/basics/Number.cpp +++ b/src/libxrpl/basics/Number.cpp @@ -470,7 +470,7 @@ doNormalize( // (maxRep * 10 > maxMantissa) XRPL_ASSERT_PARTS( m <= maxRep, - "xrpl::doNormalize", + "ripple::doNormalize", "intermediate mantissa fits in int64"); mantissa_ = m; @@ -483,7 +483,7 @@ doNormalize( "Number::normalize 2"); XRPL_ASSERT_PARTS( mantissa_ >= minMantissa && mantissa_ <= maxMantissa, - "xrpl::doNormalize", + "ripple::doNormalize", "final mantissa fits in range"); } @@ -536,7 +536,8 @@ Number::normalize() Number Number::shiftExponent(int exponentDelta) const { - XRPL_ASSERT_PARTS(isnormal(), "xrpl::Number::shiftExponent", "normalized"); + XRPL_ASSERT_PARTS( + isnormal(), "ripple::Number::shiftExponent", "normalized"); auto const newExponent = exponent_ + exponentDelta; if (newExponent >= maxExponent) throw std::overflow_error("Number::shiftExponent"); @@ -547,7 +548,7 @@ Number::shiftExponent(int exponentDelta) const Number const result{negative_, mantissa_, newExponent, unchecked{}}; XRPL_ASSERT_PARTS( result.isnormal(), - "xrpl::Number::shiftExponent", + "ripple::Number::shiftExponent", "result is normalized"); return result; } @@ -571,7 +572,7 @@ Number::operator+=(Number const& y) XRPL_ASSERT( isnormal() && y.isnormal(), - "xrpl::Number::operator+=(Number) : is normal"); + "ripple::Number::operator+=(Number) : is normal"); // *n = negative // *m = mantissa // *e = exponent @@ -837,7 +838,7 @@ Number::operator/=(Number const& y) mantissa_ = static_cast(zm); exponent_ = ze; XRPL_ASSERT_PARTS( - isnormal(), "xrpl::Number::operator/=", "result is normalized"); + isnormal(), "ripple::Number::operator/=", "result is normalized"); return *this; } @@ -1048,7 +1049,7 @@ root(Number f, unsigned d) f = f.shiftExponent(-e); // f /= 10^e; XRPL_ASSERT_PARTS( - f.isnormal(), "xrpl::root(Number, unsigned)", "f is normalized"); + f.isnormal(), "ripple::root(Number, unsigned)", "f is normalized"); bool neg = false; if (f < zero) { @@ -1083,7 +1084,7 @@ root(Number f, unsigned d) auto const result = r.shiftExponent(e / di); XRPL_ASSERT_PARTS( result.isnormal(), - "xrpl::root(Number, unsigned)", + "ripple::root(Number, unsigned)", "result is normalized"); return result; } @@ -1106,7 +1107,7 @@ root2(Number f) if (e % 2 != 0) ++e; f = f.shiftExponent(-e); // f /= 10^e; - XRPL_ASSERT_PARTS(f.isnormal(), "xrpl::root2(Number)", "f is normalized"); + XRPL_ASSERT_PARTS(f.isnormal(), "ripple::root2(Number)", "f is normalized"); // Quadratic least squares curve fit of f^(1/d) in the range [0, 1] auto const D = 105; @@ -1129,7 +1130,7 @@ root2(Number f) // return r * 10^(e/2) to reverse scaling auto const result = r.shiftExponent(e / 2); XRPL_ASSERT_PARTS( - result.isnormal(), "xrpl::root2(Number)", "result is normalized"); + result.isnormal(), "ripple::root2(Number)", "result is normalized"); return result; } diff --git a/src/libxrpl/protocol/STAmount.cpp b/src/libxrpl/protocol/STAmount.cpp index 5e8a56e950..86f16669b4 100644 --- a/src/libxrpl/protocol/STAmount.cpp +++ b/src/libxrpl/protocol/STAmount.cpp @@ -357,7 +357,7 @@ STAmount::operator=(IOUAmount const& iou) { XRPL_ASSERT( integral() == false, - "xrpl::STAmount::operator=(IOUAmount) : is not integral"); + "ripple::STAmount::operator=(IOUAmount) : is not integral"); mOffset = iou.exponent(); mIsNegative = iou < beast::zero; if (mIsNegative) diff --git a/src/libxrpl/protocol/STNumber.cpp b/src/libxrpl/protocol/STNumber.cpp index 698b6e2a2c..dbe1e015ee 100644 --- a/src/libxrpl/protocol/STNumber.cpp +++ b/src/libxrpl/protocol/STNumber.cpp @@ -85,7 +85,7 @@ STNumber::add(Serializer& s) const getFName().isBinary(), "ripple::STNumber::add : field is binary"); XRPL_ASSERT( getFName().fieldType == getSType(), - "xrpl::STNumber::add : field type match"); + "ripple::STNumber::add : field type match"); auto value = value_; auto const mantissa = value.mantissa(); @@ -102,7 +102,7 @@ STNumber::add(Serializer& s) const roundToAsset(*asset_, value); XRPL_ASSERT_PARTS( value_ == value, - "xrpl::STNumber::add", + "ripple::STNumber::add", "value is already rounded"); } else @@ -116,7 +116,7 @@ STNumber::add(Serializer& s) const // STNumber is when the scale is large. XRPL_ASSERT_PARTS( Number::getMantissaScale() == MantissaRange::large, - "xrpl::STNumber::add", + "ripple::STNumber::add", "STNumber only used with large mantissa scale"); #endif } @@ -125,7 +125,7 @@ STNumber::add(Serializer& s) const XRPL_ASSERT_PARTS( mantissa <= std::numeric_limits::max() && mantissa >= std::numeric_limits::min(), - "xrpl::STNumber::add", + "ripple::STNumber::add", "mantissa in valid range"); s.add64(mantissa); s.add32(exponent); diff --git a/src/test/app/Loan_test.cpp b/src/test/app/Loan_test.cpp index f8d9940803..e21a544352 100644 --- a/src/test/app/Loan_test.cpp +++ b/src/test/app/Loan_test.cpp @@ -314,17 +314,11 @@ protected: env.balance(vaultPseudo, broker.asset).number()); if (ownerCount == 0) { - // Allow some slop for rounding IOUs - - // TODO: This needs to be an exact match once all the - // other rounding issues are worked out. + // The Vault must be perfectly balanced if there + // are no loans outstanding auto const total = vaultSle->at(sfAssetsTotal); auto const available = vaultSle->at(sfAssetsAvailable); - env.test.BEAST_EXPECT( - total == available || - (!broker.asset.integral() && available != 0 && - ((total - available) / available < - Number(1, -6)))); + env.test.BEAST_EXPECT(total == available); env.test.BEAST_EXPECT( vaultSle->at(sfLossUnrealized) == 0); } @@ -7660,6 +7654,133 @@ protected: BEAST_EXPECT(afterSecondCoverAvailable == 0); } + void + testYieldTheftRounding(std::uint32_t flags) + { + testcase("Yield Theft via Rounding Manipulation"); + using namespace jtx; + using namespace loan; + + // 1. Setup Environment + Env env(*this, all); + Account const issuer{"issuer"}; + Account const lender{"lender"}; + Account const borrower{"borrower"}; + + env.fund(XRP(1000), issuer, lender, borrower); + env.close(); + + // 2. Asset Selection + PrettyAsset const iou = issuer["USD"]; + env(trust(lender, iou(100'000'000))); + env(trust(borrower, iou(100'000'000))); + env(pay(issuer, lender, iou(100'000'000))); + env(pay(issuer, borrower, iou(100'000'000))); + env.close(); + + // 3. Create Vault and Broker with High Debt Limit (100M) + auto const brokerInfo = createVaultAndBroker( + env, + iou, + lender, + { + .vaultDeposit = 5'000'000, + .debtMax = Number{100'000'000}, + .coverDeposit = 500'000, + }); + auto const [currentSeq, vaultId, vaultKeylet] = [&]() { + auto const brokerSle = + env.le(keylet::loanbroker(brokerInfo.brokerID)); + auto const currentSeq = brokerSle->at(sfLoanSequence); + auto const vaultKeylet = keylet::vault(brokerSle->at(sfVaultID)); + auto const vaultId = brokerSle->at(sfVaultID); + return std::make_tuple(currentSeq, vaultId, vaultKeylet); + }(); + + // 4. Loan Parameters (Attack Vector) + Number const principal = 1'000'000; + TenthBips32 const interestRate = TenthBips32{1}; // 0.001% + std::uint32_t const paymentInterval = 86400; + std::uint32_t const paymentTotal = 3650; + + auto const loanSetFee = fee(env.current()->fees().base * 2); + env(set(borrower, brokerInfo.brokerID, iou(principal).value(), flags), + sig(sfCounterpartySignature, lender), + loan::interestRate(interestRate), + loan::paymentInterval(paymentInterval), + loan::paymentTotal(paymentTotal), + fee(loanSetFee)); + env.close(); + + // --- RETRIEVE OBJECTS & SETUP ATTACK --- + + auto const loanKeylet = keylet::loan(brokerInfo.brokerID, currentSeq); + auto const [periodicPayment, loanScale] = [&]() { + auto const loanSle = env.le(loanKeylet); + // Construct Payment + return std::make_tuple( + STAmount{iou, loanSle->at(sfPeriodicPayment)}, + loanSle->at(sfLoanScale)); + }(); + auto const roundedPayment = + roundToScale(periodicPayment, loanScale, Number::upward); + + // ATTACK: Add dust buffer (1e-9) to force 'excess' logic execution + STAmount const paymentBuffer{iou, Number(1, -9)}; + STAmount const attackPayment = periodicPayment + paymentBuffer; + + auto const initialVaultAssets = env.le(vaultKeylet)->at(sfAssetsTotal); + + // 5. Execution Loop + int yieldTheftCount = 0; + auto previousAssetsTotal = initialVaultAssets; + + auto borrowerBalance = [&]() { return env.balance(borrower, iou); }; + + for (int i = 0; i < 100; ++i) + { + auto const balanceBefore = borrowerBalance(); + env(pay(borrower, loanKeylet.key, attackPayment, flags)); + env.close(); + auto const borrowerDelta = borrowerBalance() - balanceBefore; + + auto const loanSle = env.le(loanKeylet); + if (!BEAST_EXPECT(loanSle)) + break; + auto const updatedPayment = + STAmount{iou, loanSle->at(sfPeriodicPayment)}; + BEAST_EXPECT( + (roundToScale(updatedPayment, loanScale, Number::upward) == + roundedPayment)); + BEAST_EXPECT( + (updatedPayment == periodicPayment) || + (flags == tfLoanOverpayment && i >= 2 && + updatedPayment < periodicPayment)); + + auto const currentVaultSle = env.le(vaultKeylet); + if (!BEAST_EXPECT(currentVaultSle)) + break; + + auto const currentAssetsTotal = currentVaultSle->at(sfAssetsTotal); + auto const delta = currentAssetsTotal - previousAssetsTotal; + + BEAST_EXPECT( + (delta == beast::zero && borrowerDelta <= roundedPayment) || + (delta > beast::zero && borrowerDelta > roundedPayment)); + + // If tx succeeded but Assets Total didn't change, interest was + // stolen. + if (delta == beast::zero && borrowerDelta > roundedPayment) + { + yieldTheftCount++; + } + + previousAssetsTotal = currentAssetsTotal; + } + + BEAST_EXPECTS(yieldTheftCount == 0, std::to_string(yieldTheftCount)); + } + public: void run() override @@ -7668,6 +7789,11 @@ public: testLoanPayLateFullPaymentBypassesPenalties(); testLoanCoverMinimumRoundingExploit(); #endif + for (auto const flags : {0u, tfLoanOverpayment}) + { + testYieldTheftRounding(flags); + } + testInvalidLoanSet(); testCoverDepositWithdrawNonTransferableMPT(); diff --git a/src/xrpld/app/misc/detail/LendingHelpers.cpp b/src/xrpld/app/misc/detail/LendingHelpers.cpp index 1650dde157..8b8a31a54e 100644 --- a/src/xrpld/app/misc/detail/LendingHelpers.cpp +++ b/src/xrpld/app/misc/detail/LendingHelpers.cpp @@ -1927,10 +1927,20 @@ loanMakePayment( "no value change"); // ------------------------------------------------------------- - // overpayment handling + // + // 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 < amount && numPayments < loanMaximumPaymentsPerTransaction) + totalPaid < roundedAmount && + numPayments < loanMaximumPaymentsPerTransaction) { TenthBips32 const overpaymentInterestRate{ loan->at(sfOverpaymentInterestRate)}; @@ -1940,7 +1950,7 @@ loanMakePayment( // totalValueOutstanding, because that would have been processed as // another normal payment. But cap it just in case. Number const overpayment = - std::min(amount - totalPaid, *totalValueOutstandingProxy); + std::min(roundedAmount - totalPaid, *totalValueOutstandingProxy); detail::ExtendedPaymentComponents const overpaymentComponents = detail::computeOverpaymentComponents( diff --git a/src/xrpld/app/tx/detail/LoanPay.cpp b/src/xrpld/app/tx/detail/LoanPay.cpp index f606ab91cb..eea283c1d7 100644 --- a/src/xrpld/app/tx/detail/LoanPay.cpp +++ b/src/xrpld/app/tx/detail/LoanPay.cpp @@ -8,6 +8,7 @@ #include #include +#include #include namespace ripple { @@ -432,9 +433,10 @@ LoanPay::doApply() // Vault object state changes view.update(vaultSle); + Number const assetsAvailableBefore = *assetsAvailableProxy; + Number const assetsTotalBefore = *assetsTotalProxy; #if !NDEBUG { - Number const assetsAvailableBefore = *assetsAvailableProxy; Number const pseudoAccountBalanceBefore = accountHolds( view, vaultPseudoAccount, @@ -455,20 +457,9 @@ LoanPay::doApply() XRPL_ASSERT_PARTS( *assetsAvailableProxy <= *assetsTotalProxy, - "rippled::LoanPay::doApply", + "ripple::LoanPay::doApply", "assets available must not be greater than assets outstanding"); - if (*assetsAvailableProxy > *assetsTotalProxy) - { - // LCOV_EXCL_START - JLOG(j_.fatal()) << "Vault assets available must not be greater " - "than assets outstanding. Available: " - << *assetsAvailableProxy - << ", Total: " << *assetsTotalProxy; - return tecINTERNAL; - // LCOV_EXCL_STOP - } - JLOG(j_.debug()) << "total paid to vault raw: " << totalPaidToVaultRaw << ", total paid to vault rounded: " << totalPaidToVaultRounded @@ -495,12 +486,74 @@ LoanPay::doApply() associateAsset(*vaultSle, asset); // Duplicate some checks after rounding - XRPL_ASSERT_PARTS( - *assetsAvailableProxy <= *assetsTotalProxy, - "xrpl::LoanPay::doApply", - "assets available must not be greater than assets outstanding"); + Number const assetsAvailableAfter = *assetsAvailableProxy; + Number const assetsTotalAfter = *assetsTotalProxy; -#if !NDEBUG + XRPL_ASSERT_PARTS( + assetsAvailableAfter <= assetsTotalAfter, + "ripple::LoanPay::doApply", + "assets available must not be greater than assets outstanding"); + 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. + // + // LCOV_EXCL_START + JLOG(j_.warn()) + << "LoanPay: Vault assets available unchanged after rounding: " // + << "Before: " << assetsAvailableBefore // + << ", After: " << assetsAvailableAfter; + return tecPRECISION_LOSS; + // LCOV_EXCL_STOP + } + if (paymentParts->valueChange != beast::zero && + assetsTotalAfter == assetsTotalBefore) + { + // Non-zero valueChange with an unchanged assetsTotal indicates that the + // actual value change rounded to zero. That should be impossible, but I + // can't rule it out for extreme edge cases, so fail gracefully if it + // happens. + // + // LCOV_EXCL_START + JLOG(j_.warn()) << "LoanPay: Vault assets expected change, but " + "unchanged after rounding: " // + << "Before: " << assetsTotalBefore // + << ", After: " << assetsTotalAfter // + << ", ValueChange: " << paymentParts->valueChange; + return tecPRECISION_LOSS; + // LCOV_EXCL_STOP + } + if (paymentParts->valueChange == beast::zero && + assetsTotalAfter != assetsTotalBefore) + { + // A change in assetsTotal when there was no valueChange indicates that + // something really weird happened. That should be flat out impossible. + // + // LCOV_EXCL_START + JLOG(j_.fatal()) + << "LoanPay: Vault assets changed unexpectedly after rounding: " // + << "Before: " << assetsTotalBefore // + << ", After: " << assetsTotalAfter // + << ", ValueChange: " << paymentParts->valueChange; + return tecINTERNAL; + // LCOV_EXCL_STOP + } + if (assetsAvailableAfter > assetsTotalAfter) + { + // Assets available are not allowed to be larger than assets total. + // LCOV_EXCL_START + JLOG(j_.fatal()) + << "LoanPay: Vault assets available must not be greater " + "than assets outstanding. Available: " + << assetsAvailableAfter << ", Total: " << assetsTotalAfter; + return tecINTERNAL; + // LCOV_EXCL_STOP + } + + // These three values are used to check that funds are conserved after the + // transfers auto const accountBalanceBefore = accountHolds( view, account_, @@ -529,7 +582,6 @@ LoanPay::doApply() ahIGNORE_AUTH, j_, SpendableHandling::shFULL_BALANCE); -#endif if (totalPaidToVaultRounded != beast::zero) { @@ -570,19 +622,22 @@ LoanPay::doApply() return ter; #if !NDEBUG - Number const assetsAvailableAfter = *assetsAvailableProxy; - Number const pseudoAccountBalanceAfter = accountHolds( - view, - vaultPseudoAccount, - asset, - FreezeHandling::fhIGNORE_FREEZE, - AuthHandling::ahIGNORE_AUTH, - j_); - XRPL_ASSERT_PARTS( - assetsAvailableAfter == pseudoAccountBalanceAfter, - "ripple::LoanPay::doApply", - "vault pseudo balance agrees after"); + { + Number const pseudoAccountBalanceAfter = accountHolds( + view, + vaultPseudoAccount, + asset, + FreezeHandling::fhIGNORE_FREEZE, + AuthHandling::ahIGNORE_AUTH, + j_); + XRPL_ASSERT_PARTS( + assetsAvailableAfter == pseudoAccountBalanceAfter, + "ripple::LoanPay::doApply", + "vault pseudo balance agrees after"); + } +#endif + // Check that funds are conserved auto const accountBalanceAfter = accountHolds( view, account_, @@ -611,16 +666,132 @@ LoanPay::doApply() ahIGNORE_AUTH, j_, SpendableHandling::shFULL_BALANCE); + auto const balanceScale = [&]() { + // 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 : { + accountBalanceBefore, + vaultBalanceBefore, + brokerBalanceBefore, + accountBalanceAfter, + vaultBalanceAfter, + brokerBalanceAfter, + }) + { + // Exclude zeroes + if (a != beast::zero) + exponents.push_back(a.exponent()); + } + if (exponents.empty()) + { + UNREACHABLE("ripple::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; + JLOG(j_.trace()) << "Min scale: " << min << ", max scale: " << max; + // IOU rounding can be interesting. We want all the balance checks to + // agree, but don't want to round to such an extreme that it becomes + // meaningless. e.g. Everything rounds to one digit. So add 1 to the + // max (reducing the number of digits after the decimal point by 1) if + // the scales are not already all the same. + return std::min(min == max ? max : max + 1, STAmount::cMaxOffset); + }(); + + auto const accountBalanceBeforeRounded = + roundToScale(accountBalanceBefore, balanceScale); + auto const vaultBalanceBeforeRounded = + roundToScale(vaultBalanceBefore, balanceScale); + auto const brokerBalanceBeforeRounded = + roundToScale(brokerBalanceBefore, balanceScale); + + auto const totalBalanceBefore = + accountBalanceBefore + vaultBalanceBefore + brokerBalanceBefore; + auto const totalBalanceBeforeRounded = + roundToScale(totalBalanceBefore, balanceScale); + + JLOG(j_.trace()) << "Before: " // + << "account " << Number(accountBalanceBeforeRounded) + << " (" << Number(accountBalanceBefore) << ")" + << ", vault " << Number(vaultBalanceBeforeRounded) << " (" + << Number(vaultBalanceBefore) << ")" + << ", broker " << Number(brokerBalanceBeforeRounded) + << " (" << Number(brokerBalanceBefore) << ")" + << ", total " << Number(totalBalanceBeforeRounded) << " (" + << Number(totalBalanceBefore) << ")"; + + auto const accountBalanceAfterRounded = + roundToScale(accountBalanceAfter, balanceScale); + auto const vaultBalanceAfterRounded = + roundToScale(vaultBalanceAfter, balanceScale); + auto const brokerBalanceAfterRounded = + roundToScale(brokerBalanceAfter, balanceScale); + + auto const totalBalanceAfter = + accountBalanceAfter + vaultBalanceAfter + brokerBalanceAfter; + auto const totalBalanceAfterRounded = + roundToScale(totalBalanceAfter, balanceScale); + + JLOG(j_.trace()) << "After: " // + << "account " << Number(accountBalanceAfterRounded) << " (" + << Number(accountBalanceAfter) << ")" + << ", vault " << Number(vaultBalanceAfterRounded) << " (" + << Number(vaultBalanceAfter) << ")" + << ", broker " << Number(brokerBalanceAfterRounded) << " (" + << Number(brokerBalanceAfter) << ")" + << ", total " << Number(totalBalanceAfterRounded) << " (" + << Number(totalBalanceAfter) << ")"; + + auto const accountBalanceChange = + accountBalanceAfter - accountBalanceBefore; + auto const vaultBalanceChange = vaultBalanceAfter - vaultBalanceBefore; + auto const brokerBalanceChange = brokerBalanceAfter - brokerBalanceBefore; + + auto const totalBalanceChange = + accountBalanceChange + vaultBalanceChange + brokerBalanceChange; + auto const totalBalanceChangeRounded = + roundToScale(totalBalanceChange, balanceScale); + + JLOG(j_.trace()) << "Changes: " // + << "account " << to_string(accountBalanceChange) // + << ", vault " << to_string(vaultBalanceChange) // + << ", broker " << to_string(brokerBalanceChange) // + << ", total " << to_string(totalBalanceChangeRounded) + << " (" << Number(totalBalanceChange) << ")"; + + if (totalBalanceBeforeRounded != totalBalanceAfterRounded) + { + JLOG(j_.warn()) << "Total rounded balances don't match" + << (totalBalanceChangeRounded == beast::zero + ? ", but total changes do" + : ""); + } + if (totalBalanceChangeRounded != beast::zero) + { + JLOG(j_.warn()) << "Total balance changes don't match" + << (totalBalanceBeforeRounded == + totalBalanceAfterRounded + ? ", but total balances do" + : ""); + } + + // Rounding for IOUs can be weird, so check a few different ways to show + // that funds are conserved. XRPL_ASSERT_PARTS( - accountBalanceBefore + vaultBalanceBefore + brokerBalanceBefore == - accountBalanceAfter + vaultBalanceAfter + brokerBalanceAfter, + totalBalanceBeforeRounded == totalBalanceAfterRounded || + totalBalanceChangeRounded == beast::zero, "ripple::LoanPay::doApply", "funds are conserved (with rounding)"); - XRPL_ASSERT_PARTS( - accountBalanceAfter >= beast::zero, - "ripple::LoanPay::doApply", - "positive account balance"); + XRPL_ASSERT_PARTS( accountBalanceAfter < accountBalanceBefore || account_ == asset.getIssuer(), @@ -643,7 +814,6 @@ LoanPay::doApply() brokerBalanceAfter > brokerBalanceBefore, "ripple::LoanPay::doApply", "vault and/or broker balance increased"); -#endif return tesSUCCESS; } diff --git a/src/xrpld/app/tx/detail/Transactor.cpp b/src/xrpld/app/tx/detail/Transactor.cpp index fdc0c48cef..a866de8b2d 100644 --- a/src/xrpld/app/tx/detail/Transactor.cpp +++ b/src/xrpld/app/tx/detail/Transactor.cpp @@ -235,7 +235,7 @@ Transactor::preflight2(PreflightContext const& ctx) // featureBatch being enabled XRPL_ASSERT_PARTS( !ctx.tx.isFlag(tfInnerBatchTxn) || ctx.rules.enabled(featureBatch), - "xrpl::Transactor::preflight2", + "ripple::Transactor::preflight2", "InnerBatch flag only set if feature enabled"); // Skip signature check on batch inner transactions if (ctx.tx.isFlag(tfInnerBatchTxn) && ctx.rules.enabled(featureBatch)) diff --git a/src/xrpld/app/tx/detail/VaultClawback.cpp b/src/xrpld/app/tx/detail/VaultClawback.cpp index e2830fc90c..95f31fe41f 100644 --- a/src/xrpld/app/tx/detail/VaultClawback.cpp +++ b/src/xrpld/app/tx/detail/VaultClawback.cpp @@ -413,7 +413,7 @@ VaultClawback::doApply() { XRPL_ASSERT( amount.asset() == vaultAsset, - "xrpl::VaultClawback::doApply : matching asset"); + "ripple::VaultClawback::doApply : matching asset"); auto const clawbackParts = assetsToClawback(vault, sleIssuance, holder, amount);