fix: Improve rounding for IOU loans

- Scale the loan to the Vault, so that amounts moving to the vault are less likely to have rounding errors. - Similar to LoanPay, when LoanManage defaults a loan, round the amounts to the Vault scale (because the Vault scale can change) before applying them to the Vault.
2025-11-18 18:15:50 +00:00 · 2025-11-06 19:20:53 -05:00
parent d2f8e3817e
commit e001681ada
6 changed files with 120 additions and 68 deletions
--- a/src/test/app/Loan_test.cpp
+++ b/src/test/app/Loan_test.cpp
@@ -107,14 +107,43 @@ protected:
    {
        jtx::PrettyAsset asset;
        uint256 brokerID;
+        uint256 vaultID;
        BrokerParameters params;
        BrokerInfo(
            jtx::PrettyAsset const& asset_,
-            uint256 const& brokerID_,
+            Keylet const& brokerKeylet_,
+            Keylet const& vaultKeylet_,
            BrokerParameters const& p)
-            : asset(asset_), brokerID(brokerID_), params(p)
+            : asset(asset_)
+            , brokerID(brokerKeylet_.key)
+            , vaultID(vaultKeylet_.key)
+            , params(p)
        {
        }
+
+        Keylet
+        brokerKeylet() const
+        {
+            return keylet::loanbroker(brokerID);
+        }
+        Keylet
+        vaultKeylet() const
+        {
+            return keylet::vault(vaultID);
+        }
+
+        int
+        vaultScale(jtx::Env const& env) const
+        {
+            using namespace jtx;
+
+            auto const vaultSle = env.le(keylet::vault(vaultID));
+            if (!vaultSle)
+                // This function is not important enough to return an optional.
+                // Return an impossibly small number
+                return STAmount::cMinOffset - 1;
+            return vaultSle->at(sfAssetsTotal).exponent();
+        }
    };

    struct LoanParameters
@@ -448,7 +477,7 @@ protected:

        env.close();

-        return {asset, keylet.key, params};
+        return {asset, keylet, vaultKeylet, params};
    }

    /// Get the state without checking anything
@@ -502,9 +531,12 @@ protected:
        BEAST_EXPECT(state.paymentRemaining == 12);
        BEAST_EXPECT(state.principalOutstanding == broker.asset(1000).value());
        BEAST_EXPECT(
-            state.loanScale ==
-            (broker.asset.integral() ? 0
-                                     : state.principalOutstanding.exponent()));
+            state.loanScale >=
+            (broker.asset.integral()
+                 ? 0
+                 : std::max(
+                       broker.vaultScale(env),
+                       state.principalOutstanding.exponent())));
        BEAST_EXPECT(state.paymentInterval == 600);
        BEAST_EXPECT(
            state.totalValue ==
@@ -759,9 +791,12 @@ protected:
                startDate + *loanParams.payInterval);
            BEAST_EXPECT(loan->at(sfPaymentRemaining) == *loanParams.payTotal);
            BEAST_EXPECT(
-                loan->at(sfLoanScale) ==
-                (broker.asset.integral() ? 0
-                                         : principalRequestAmount.exponent()));
+                loan->at(sfLoanScale) >=
+                (broker.asset.integral()
+                     ? 0
+                     : std::max(
+                           broker.vaultScale(env),
+                           principalRequestAmount.exponent())));
            BEAST_EXPECT(
                loan->at(sfPrincipalOutstanding) == principalRequestAmount);
        }
@@ -774,13 +809,14 @@ protected:
            state.interestRate,
            state.paymentInterval,
            state.paymentRemaining,
-            broker.params.managementFeeRate);
+            broker.params.managementFeeRate,
+            state.loanScale);

        verifyLoanStatus(
            0,
            startDate + *loanParams.payInterval,
            *loanParams.payTotal,
-            broker.asset.integral() ? 0 : principalRequestAmount.exponent(),
+            state.loanScale,
            loanProperties.totalValueOutstanding,
            principalRequestAmount,
            loanProperties.managementFeeOwedToBroker,
@@ -837,7 +873,7 @@ protected:
            0,
            nextDueDate,
            *loanParams.payTotal,
-            broker.asset.integral() ? 0 : principalRequestAmount.exponent(),
+            loanProperties.loanScale,
            loanProperties.totalValueOutstanding,
            principalRequestAmount,
            loanProperties.managementFeeOwedToBroker,
@@ -1394,10 +1430,12 @@ protected:
                BEAST_EXPECT(brokerSle))
            {
                BEAST_EXPECT(
-                    state.loanScale ==
+                    state.loanScale >=
                    (broker.asset.integral()
                         ? 0
-                         : state.principalOutstanding.exponent()));
+                         : std::max(
+                               broker.vaultScale(env),
+                               state.principalOutstanding.exponent())));
                auto const defaultAmount = roundToAsset(
                    broker.asset,
                    std::min(
@@ -1688,7 +1726,10 @@ protected:
                    state.loanScale);
                BEAST_EXPECT(
                    payoffAmount ==
-                    broker.asset(Number(1040000114155251, -12)));
+                    roundToAsset(
+                        broker.asset,
+                        broker.asset(Number(1040000114155251, -12)).number(),
+                        state.loanScale));

                // The terms of this loan actually make the early payoff
                // more expensive than just making payments
@@ -5687,15 +5728,7 @@ protected:
        if (!loanKeyletOpt)
            return;

-        auto const vaultKeyletOpt = [&]() -> std::optional<Keylet> {
-            auto const brokerSle = env.le(keylet::loanbroker(broker.brokerID));
-            if (!BEAST_EXPECT(brokerSle))
-                return std::nullopt;
-            return keylet::vault(brokerSle->at(sfVaultID));
-        }();
-        if (!BEAST_EXPECT(vaultKeyletOpt))
-            return;
-        auto const& vaultKeylet = *vaultKeyletOpt;
+        auto const& vaultKeylet = broker.vaultKeylet();

        {
            auto const vaultSle = env.le(vaultKeylet);
@@ -6096,7 +6129,8 @@ failed with assertion error: Both principal and interest rounded are zero 0 + 0
                loanParams.payInterval.value_or(
                    LoanSet::defaultPaymentInterval),
                loanParams.payTotal.value_or(LoanSet::defaultPaymentTotal),
-                brokerParams.managementFeeRate);
+                brokerParams.managementFeeRate,
+                asset(brokerParams.vaultDeposit).number().exponent());
            log << "Loan properties:\n"
                << "\tPeriodic Payment: " << props.periodicPayment << std::endl
                << "\tTotal Value: " << props.totalValueOutstanding << std::endl
--- a/src/xrpld/app/misc/LendingHelpers.h
+++ b/src/xrpld/app/misc/LendingHelpers.h
@@ -234,7 +234,8 @@ computeLoanProperties(
    TenthBips32 interestRate,
    std::uint32_t paymentInterval,
    std::uint32_t paymentsRemaining,
-    TenthBips32 managementFeeRate);
+    TenthBips32 managementFeeRate,
+    std::int32_t minimumScale);

 bool
 isRounded(Asset const& asset, Number const& value, std::int32_t scale);
--- a/src/xrpld/app/misc/detail/LendingHelpers.cpp
+++ b/src/xrpld/app/misc/detail/LendingHelpers.cpp
@@ -601,7 +601,8 @@ tryOverpayment(
        interestRate,
        paymentInterval,
        paymentRemaining,
-        managementFeeRate);
+        managementFeeRate,
+        loanScale);

    auto const newRaw = calculateRawLoanState(
        newLoanProperties.periodicPayment,
@@ -1547,7 +1548,8 @@ computeLoanProperties(
    TenthBips32 interestRate,
    std::uint32_t paymentInterval,
    std::uint32_t paymentsRemaining,
-    TenthBips32 managementFeeRate)
+    TenthBips32 managementFeeRate,
+    std::int32_t minimumScale)
 {
    auto const periodicRate = loanPeriodicRate(interestRate, paymentInterval);
    XRPL_ASSERT(
@@ -1556,12 +1558,12 @@ computeLoanProperties(

    auto const periodicPayment = detail::loanPeriodicPayment(
        principalOutstanding, periodicRate, paymentsRemaining);
-    STAmount const totalValueOutstanding = [&]() {
+    auto const [totalValueOutstanding, loanScale] = [&]() {
        NumberRoundModeGuard mg(Number::to_nearest);
        // Use STAmount's internal rounding instead of roundToAsset, because
        // we're going to use this result to determine the scale for all the
        // other rounding.
-        return STAmount{
+        STAmount amount{
            asset,
            /*
             * This formula is from the XLS-66 spec, section 3.2.4.2 (Total
@@ -1569,18 +1571,23 @@ computeLoanProperties(
             * = ..."
             */
            periodicPayment * paymentsRemaining};
+
+        // Base the loan scale on the total value, since that's going to be the
+        // biggest number involved (barring unusual parameters for late, full,
+        // or over payments)
+        auto const loanScale = std::max(minimumScale, amount.exponent());
+        XRPL_ASSERT_PARTS(
+            (amount.integral() && loanScale == 0) ||
+                (!amount.integral() &&
+                 loanScale >= static_cast<Number>(amount).exponent()),
+            "ripple::computeLoanProperties",
+            "loanScale value fits expectations");
+
+        // We may need to truncate the total value because of the minimum scale
+        amount = roundToAsset(asset, amount, loanScale, Number::to_nearest);
+
+        return std::make_pair(amount, loanScale);
    }();
-    // Base the loan scale on the total value, since that's going to be the
-    // biggest number involved (barring unusual parameters for late, full, or
-    // over payments)
-    auto const loanScale = totalValueOutstanding.exponent();
-    XRPL_ASSERT_PARTS(
-        (totalValueOutstanding.integral() && loanScale == 0) ||
-            (!totalValueOutstanding.integral() &&
-             loanScale ==
-                 static_cast<Number>(totalValueOutstanding).exponent()),
-        "ripple::computeLoanProperties",
-        "loanScale value fits expectations");

    // Since we just figured out the loan scale, we haven't been able to
    // validate that the principal fits in it, so to allow this function to
--- a/src/xrpld/app/tx/detail/LoanManage.cpp
+++ b/src/xrpld/app/tx/detail/LoanManage.cpp
@@ -169,9 +169,15 @@ LoanManage::defaultLoan(

    {
        // Decrease the Total Value of the Vault:
-        auto vaultAssetsTotalProxy = vaultSle->at(sfAssetsTotal);
-        auto vaultAssetsAvailableProxy = vaultSle->at(sfAssetsAvailable);
-        if (vaultAssetsTotalProxy < vaultDefaultAmount)
+        auto vaultTotalProxy = vaultSle->at(sfAssetsTotal);
+        auto vaultAvailableProxy = vaultSle->at(sfAssetsAvailable);
+
+        // The vault may be at a different scale than the loan. Reduce rounding
+        // errors during the accounting by rounding some of the values to that
+        // scale.
+        auto const vaultScale = vaultTotalProxy->value().exponent();
+
+        if (vaultTotalProxy < vaultDefaultAmount)
        {
            // LCOV_EXCL_START
            JLOG(j.warn())
@@ -179,23 +185,24 @@ LoanManage::defaultLoan(
            return tefBAD_LEDGER;
            // LCOV_EXCL_STOP
        }
-        vaultAssetsTotalProxy -= vaultDefaultAmount;
+
+        auto const vaultDefaultRounded = roundToAsset(
+            vaultAsset, vaultDefaultAmount, vaultScale, Number::downward);
+        vaultTotalProxy -= vaultDefaultRounded;
        // Increase the Asset Available of the Vault by liquidated First-Loss
        // Capital and any unclaimed funds amount:
-        vaultAssetsAvailableProxy += defaultCovered;
-        if (*vaultAssetsAvailableProxy > *vaultAssetsTotalProxy &&
-            !vaultAsset.integral())
+        vaultAvailableProxy += defaultCovered;
+        if (*vaultAvailableProxy > *vaultTotalProxy && !vaultAsset.integral())
        {
-            auto const difference =
-                vaultAssetsAvailableProxy - vaultAssetsTotalProxy;
+            auto const difference = vaultAvailableProxy - vaultTotalProxy;
            JLOG(j.debug())
-                << "Vault assets available: " << *vaultAssetsAvailableProxy
-                << "(" << vaultAssetsAvailableProxy->value().exponent()
-                << "), Total: " << *vaultAssetsTotalProxy << "("
-                << vaultAssetsTotalProxy->value().exponent()
+                << "Vault assets available: " << *vaultAvailableProxy << "("
+                << vaultAvailableProxy->value().exponent()
+                << "), Total: " << *vaultTotalProxy << "("
+                << vaultTotalProxy->value().exponent()
                << "), Difference: " << difference << "("
                << difference.exponent() << ")";
-            if (vaultAssetsAvailableProxy->value().exponent() -
+            if (vaultAvailableProxy->value().exponent() -
                    difference.exponent() >
                13)
            {
@@ -204,15 +211,15 @@ LoanManage::defaultLoan(
                JLOG(j.debug())
                    << "Difference between vault assets available and total is "
                       "dust. Set both to the larger value.";
-                vaultAssetsTotalProxy = vaultAssetsAvailableProxy;
+                vaultTotalProxy = vaultAvailableProxy;
            }
        }
-        if (*vaultAssetsAvailableProxy > *vaultAssetsTotalProxy)
+        if (*vaultAvailableProxy > *vaultTotalProxy)
        {
            JLOG(j.warn()) << "Vault assets available must not be greater "
                              "than assets outstanding. Available: "
-                           << *vaultAssetsAvailableProxy
-                           << ", Total: " << *vaultAssetsTotalProxy;
+                           << *vaultAvailableProxy
+                           << ", Total: " << *vaultTotalProxy;
            return tecLIMIT_EXCEEDED;
        }

--- a/src/xrpld/app/tx/detail/LoanPay.cpp
+++ b/src/xrpld/app/tx/detail/LoanPay.cpp
@@ -347,9 +347,11 @@ LoanPay::doApply()
    view.update(brokerSle);

    auto assetsAvailableProxy = vaultSle->at(sfAssetsAvailable);
+    auto assetsTotalProxy = vaultSle->at(sfAssetsTotal);
+
    // The vault may be at a different scale than the loan. Reduce rounding
    // errors during the payment by rounding some of the values to that scale.
-    auto const vaultScale = assetsAvailableProxy->value().exponent();
+    auto const vaultScale = assetsTotalProxy->value().exponent();

    auto const totalPaidToVaultRaw =
        paymentParts->principalPaid + paymentParts->interestPaid;
@@ -405,8 +407,6 @@ LoanPay::doApply()
            "ripple::LoanPay::doApply",
            "vault pseudo balance agrees before");

-        auto assetsTotalProxy = vaultSle->at(sfAssetsTotal);
-
        assetsAvailableProxy += totalPaidToVaultRounded;
        assetsTotalProxy += paymentParts->valueChange;

--- a/src/xrpld/app/tx/detail/LoanSet.cpp
+++ b/src/xrpld/app/tx/detail/LoanSet.cpp
@@ -418,8 +418,10 @@ LoanSet::doApply()
    }
    auto const principalRequested = tx[sfPrincipalRequested];

-    if (auto const assetsAvailable = vaultSle->at(sfAssetsAvailable);
-        assetsAvailable < principalRequested)
+    auto vaultAvailableProxy = vaultSle->at(sfAssetsAvailable);
+    auto vaultTotalProxy = vaultSle->at(sfAssetsTotal);
+    auto const vaultScale = vaultTotalProxy->value().exponent();
+    if (vaultAvailableProxy < principalRequested)
    {
        JLOG(j_.warn())
            << "Insufficient assets available in the Vault to fund the loan.";
@@ -438,7 +440,8 @@ LoanSet::doApply()
        interestRate,
        paymentInterval,
        paymentTotal,
-        TenthBips16{brokerSle->at(sfManagementFeeRate)});
+        TenthBips16{brokerSle->at(sfManagementFeeRate)},
+        vaultScale);

    // Check that relevant values won't lose precision. This is mostly only
    // relevant for IOU assets.
@@ -623,10 +626,10 @@ LoanSet::doApply()
    view.insert(loan);

    // Update the balances in the vault
-    vaultSle->at(sfAssetsAvailable) -= principalRequested;
-    vaultSle->at(sfAssetsTotal) += state.interestDue;
+    vaultAvailableProxy -= principalRequested;
+    vaultTotalProxy += state.interestDue;
    XRPL_ASSERT_PARTS(
-        *vaultSle->at(sfAssetsAvailable) <= *vaultSle->at(sfAssetsTotal),
+        *vaultAvailableProxy <= *vaultTotalProxy,
        "ripple::LoanSet::doApply",
        "assets available must not be greater than assets outstanding");
    view.update(vaultSle);