[WIP] Fix touchy "funds are conserved" assertion in LoanPay

- Add poorly named "Yield Theft via Rounding Manipulation" test

src/test/app/Loan_test.cpp

@@ -7644,6 +7644,156 @@ protected:
         BEAST_EXPECT(afterSecondCoverAvailable == 0);
     }
 
+    void
+    testYieldTheftRounding()
+    {
+        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 = 1'000'000,
+                .debtMax = Number{100'000'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);
+        }();
+        Vault vault{env};
+        env(vault.deposit(
+            {.depositor = lender, .id = vaultId, .amount = iou(5'000'000)}));
+        env.close();
+        env(loanBroker::coverDeposit(
+            lender, brokerInfo.brokerID, iou(500'000)));
+        env.close();
+
+        // 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(),
+                tfLoanOverpayment),
+            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);
+
+        log << "Periodic Payment: " << periodicPayment.value() << std::endl;
+        log << "Attack Payment:   " << attackPayment.value() << std::endl;
+        log << "Initial Vault Assets: " << initialVaultAssets << std::endl;
+
+        // 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 /*, tfLoanOverpayment*/));
+            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);
+
+            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++;
+                // delta should be zero
+                log << "[ALERT] Iteration " << i
+                    << ": YIELD THEFT CONFIRMED. Vault Delta: " << delta
+                    << std::endl;
+            }
+            else
+            {
+                log << "[INFO]  Iteration " << i << ": Normal Yield: " << delta
+                    << std::endl;
+            }
+
+            previousAssetsTotal = currentAssetsTotal;
+        }
+
+        log << "[RESULT] Yield Theft Events: " << yieldTheftCount
+            << " / 50 payments." << std::endl;
+    }
+
 public:
     void
     run() override
@@ -7652,6 +7802,8 @@ public:
         testLoanPayLateFullPaymentBypassesPenalties();
         testLoanCoverMinimumRoundingExploit();
 #endif
+        testYieldTheftRounding();
+
         testInvalidLoanSet();
 
         testCoverDepositWithdrawNonTransferableMPT();

src/xrpld/app/misc/detail/LendingHelpers.cpp

@@ -1928,9 +1928,12 @@ loanMakePayment(
 
     // -------------------------------------------------------------
     // overpayment handling
+    auto const roundedAmount =
+        roundToAsset(asset, amount, loanScale, Number::towards_zero);
     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 +1943,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(

src/xrpld/app/tx/detail/LoanPay.cpp

@@ -8,6 +8,7 @@
 #include <xrpl/protocol/STTakesAsset.h>
 #include <xrpl/protocol/TxFlags.h>
 
+#include <algorithm>
 #include <bit>
 
 namespace ripple {
@@ -430,9 +431,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,
@@ -493,10 +495,48 @@ LoanPay::doApply()
     associateAsset(*vaultSle, asset);
 
     // Duplicate some checks after rounding
+    Number const assetsAvailableAfter = *assetsAvailableProxy;
+    Number const assetsTotalAfter = *assetsTotalProxy;
+
     XRPL_ASSERT_PARTS(
-        *assetsAvailableProxy <= *assetsTotalProxy,
+        assetsAvailableAfter <= *assetsTotalProxy,
         "xrpl::LoanPay::doApply",
         "assets available must not be greater than assets outstanding");
+    if (assetsAvailableAfter == assetsAvailableBefore)
+    {
+        JLOG(j_.warn()) << "LoanPay: Vault assets available unchanged after "
+                           "rounding: Before: "
+                        << assetsAvailableBefore
+                        << ", After: " << assetsAvailableAfter;
+        return tecPRECISION_LOSS;
+    }
+    if (paymentParts->valueChange != beast::zero &&
+        assetsTotalAfter == assetsTotalBefore)
+    {
+        JLOG(j_.warn())
+            << "LoanPay: Vault assets expected change, but unchanged after "
+               "rounding: Before: "
+            << assetsTotalBefore << ", After: " << assetsTotalAfter
+            << ", ValueChange: " << paymentParts->valueChange;
+        return tecPRECISION_LOSS;
+    }
+    if (paymentParts->valueChange == beast::zero &&
+        assetsTotalAfter != assetsTotalBefore)
+    {
+        JLOG(j_.warn()) << "LoanPay: Vault assets changed unexpectedly after "
+                           "rounding: Before: "
+                        << assetsTotalBefore << ", After: " << assetsTotalAfter
+                        << ", ValueChange: " << paymentParts->valueChange;
+        return tecPRECISION_LOSS;
+    }
+    if (assetsAvailableAfter > *assetsTotalProxy)
+    {
+        JLOG(j_.warn())
+            << "LoanPay: Vault assets available must not be greater "
+               "than assets outstanding. Available: "
+            << assetsAvailableAfter << ", Total: " << *assetsTotalProxy;
+        return tecINTERNAL;
+    }
 
 #if !NDEBUG
     auto const accountBalanceBefore = accountHolds(
@@ -568,7 +608,6 @@ LoanPay::doApply()
         return ter;
 
 #if !NDEBUG
-    Number const assetsAvailableAfter = *assetsAvailableProxy;
     Number const pseudoAccountBalanceAfter = accountHolds(
         view,
         vaultPseudoAccount,
@@ -609,12 +648,99 @@ LoanPay::doApply()
               ahIGNORE_AUTH,
               j_,
               SpendableHandling::shFULL_BALANCE);
+    auto const balanceScale = [&]() {
+        // This is so ugly.
+        std::vector<int> exponents;
+
+        for (auto const& a : {
+                 accountBalanceBefore,
+                 vaultBalanceBefore,
+                 brokerBalanceBefore,
+                 accountBalanceAfter,
+                 vaultBalanceAfter,
+                 brokerBalanceAfter,
+             })
+        {
+            // Exclude zeroes
+            if (a != beast::zero)
+                exponents.push_back(a.exponent());
+        }
+        auto [min, max] =
+            std::minmax_element(exponents.begin(), exponents.end());
+        // IOU rounding can be interesting. Give a margin of error that reflects
+        // the orders of magnitude between the extremes.
+        if (!asset.integral() && *max < STAmount::cMaxOffset * 3 / 4)
+            *max += *max - *min;
+        return std::min(*max, 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);
+
+    std::cout << "Before:\n\taccount " << Number(accountBalanceBeforeRounded)
+              << " (" << Number(accountBalanceBefore) << ")"
+              << ",\n\tvault " << Number(vaultBalanceBeforeRounded) << " ("
+              << Number(vaultBalanceBefore) << ")"
+              << ",\n\tbroker " << Number(brokerBalanceBeforeRounded) << " ("
+              << Number(brokerBalanceBefore) << ")"
+              << ",\n\ttotal " << Number(totalBalanceBefore) << " ("
+              << Number(
+                     accountBalanceBefore + vaultBalanceBefore +
+                     brokerBalanceBefore)
+              << ")" << std::endl;
+
+    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);
+
+    std::cout << "After:\n\taccount " << Number(accountBalanceAfterRounded)
+              << " (" << Number(accountBalanceAfter) << ")"
+              << ",\n\tvault " << Number(vaultBalanceAfterRounded) << " ("
+              << Number(vaultBalanceAfter) << ")"
+              << ",\n\tbroker " << Number(brokerBalanceAfterRounded) << " ("
+              << Number(brokerBalanceAfter) << ")"
+              << ",\n\ttotal " << Number(totalBalanceAfterRounded) << " ("
+              << Number(totalBalanceAfter) << ")" << std::endl;
+
+    auto const accountBalanceChange =
+        accountBalanceAfter - accountBalanceBefore;
+    auto const vaultBalanceChange = vaultBalanceAfter - vaultBalanceBefore;
+    auto const brokerBalanceChange = brokerBalanceAfter - brokerBalanceBefore;
+
+    auto const totalBalanceChange = roundToScale(
+        accountBalanceChange + vaultBalanceChange + brokerBalanceChange,
+        balanceScale);
+
+    std::cout << "Changes:\n\taccount " << to_string(accountBalanceChange)
+              << ",\n\tvault " << to_string(vaultBalanceChange)
+              << ",\n\tbroker " << to_string(brokerBalanceChange)
+              << ",\n\ttotal " << to_string(totalBalanceChange) << std::endl
+              << std::endl;
 
     XRPL_ASSERT_PARTS(
-        accountBalanceBefore + vaultBalanceBefore + brokerBalanceBefore ==
-            accountBalanceAfter + vaultBalanceAfter + brokerBalanceAfter,
+        totalBalanceBefore == totalBalanceAfter ||
+            totalBalanceBeforeRounded == totalBalanceAfterRounded ||
+            totalBalanceChange == beast::zero,
         "ripple::LoanPay::doApply",
         "funds are conserved (with rounding)");
+
     XRPL_ASSERT_PARTS(
         accountBalanceAfter >= beast::zero,
         "ripple::LoanPay::doApply",