Fill in payment computation shortages (#5941)

- Ensures a consistent fixed payment amount for the entire life of the loan, except the final payment, which is guaranteed to be the same or smaller. - Convert some Loan structs to compute values that had need manual updates to stay consistent. - Fail the transaction in `LoanPay` if it violates the Vault `assetsAvailable <= assetsTotal` invariant. - Use constexpr to check that min mantissa value for Number and STAmount is a power of 10, and compute the max in terms of the min. - Improve unit tests: - Use BrokerParameters and Loan Parameters instead of semi-global class values - In tests, check that the expected number of loan payments are made. - Add LoanBatch manual test to generate a set number of random loans, set them up, and pay them off. - Add LoanArbitrary manual test to run a single test with specific (hard-coded for now) parameters. - Add Number support to XRP_t.
2025-11-20 02:55:50 +00:00 · 2025-11-04 17:56:16 -05:00
parent 7925cc4052
commit aed8e2b166
12 changed files with 1768 additions and 382 deletions
--- a/include/xrpl/basics/Number.h
+++ b/include/xrpl/basics/Number.h
@@ -32,6 +32,15 @@ class Number;
 std::string
 to_string(Number const& amount);

+template <typename T>
+constexpr bool
+isPowerOfTen(T value)
+{
+    while (value >= 10 && value % 10 == 0)
+        value /= 10;
+    return value == 1;
+}
+
 class Number
 {
    using rep = std::int64_t;
@@ -41,7 +50,9 @@ class Number
 public:
    // The range for the mantissa when normalized
    constexpr static std::int64_t minMantissa = 1'000'000'000'000'000LL;
-    constexpr static std::int64_t maxMantissa = 9'999'999'999'999'999LL;
+    static_assert(isPowerOfTen(minMantissa));
+    constexpr static std::int64_t maxMantissa = minMantissa * 10 - 1;
+    static_assert(maxMantissa == 9'999'999'999'999'999LL);

    // The range for the exponent when normalized
    constexpr static int minExponent = -32768;
@@ -58,8 +69,6 @@ public:
    explicit Number(rep mantissa, int exponent);
    explicit constexpr Number(rep mantissa, int exponent, unchecked) noexcept;

-    static Number const zero;
-
    constexpr rep
    mantissa() const noexcept;
    constexpr int
@@ -153,22 +162,7 @@ public:
    }

    Number
-    truncate() const noexcept
-    {
-        if (exponent_ >= 0 || mantissa_ == 0)
-            return *this;
-
-        Number ret = *this;
-        while (ret.exponent_ < 0 && ret.mantissa_ != 0)
-        {
-            ret.exponent_ += 1;
-            ret.mantissa_ /= rep(10);
-        }
-        // We are guaranteed that normalize() will never throw an exception
-        // because exponent is either negative or zero at this point.
-        ret.normalize();
-        return ret;
-    }
+    truncate() const noexcept;

    friend constexpr bool
    operator>(Number const& x, Number const& y) noexcept
@@ -213,6 +207,8 @@ private:
    class Guard;
 };

+constexpr static Number numZero{};
+
 inline constexpr Number::Number(rep mantissa, int exponent, unchecked) noexcept
    : mantissa_{mantissa}, exponent_{exponent}
 {
--- a/include/xrpl/protocol/Protocol.h
+++ b/include/xrpl/protocol/Protocol.h
@@ -102,7 +102,9 @@ std::uint16_t constexpr maxTransferFee = 50000;
 * Example: 50% is 0.50 * bipsPerUnity = 5,000 bps.
 */
 Bips32 constexpr bipsPerUnity(100 * 100);
+static_assert(bipsPerUnity == Bips32{10'000});
 TenthBips32 constexpr tenthBipsPerUnity(bipsPerUnity.value() * 10);
+static_assert(tenthBipsPerUnity == TenthBips32(100'000));

 constexpr Bips32
 percentageToBips(std::uint32_t percentage)
--- a/include/xrpl/protocol/STAmount.h
+++ b/include/xrpl/protocol/STAmount.h
@@ -66,16 +66,18 @@ public:
    static int const cMaxOffset = 80;

    // Maximum native value supported by the code
-    static std::uint64_t const cMinValue = 1'000'000'000'000'000ull;
-    static std::uint64_t const cMaxValue = 9'999'999'999'999'999ull;
-    static std::uint64_t const cMaxNative = 9'000'000'000'000'000'000ull;
+    constexpr static std::uint64_t cMinValue = 1'000'000'000'000'000ull;
+    static_assert(isPowerOfTen(cMinValue));
+    constexpr static std::uint64_t cMaxValue = cMinValue * 10 - 1;
+    static_assert(cMaxValue == 9'999'999'999'999'999ull);
+    constexpr static std::uint64_t cMaxNative = 9'000'000'000'000'000'000ull;

    // Max native value on network.
-    static std::uint64_t const cMaxNativeN = 100'000'000'000'000'000ull;
-    static std::uint64_t const cIssuedCurrency = 0x8'000'000'000'000'000ull;
-    static std::uint64_t const cPositive = 0x4'000'000'000'000'000ull;
-    static std::uint64_t const cMPToken = 0x2'000'000'000'000'000ull;
-    static std::uint64_t const cValueMask = ~(cPositive | cMPToken);
+    constexpr static std::uint64_t cMaxNativeN = 100'000'000'000'000'000ull;
+    constexpr static std::uint64_t cIssuedCurrency = 0x8'000'000'000'000'000ull;
+    constexpr static std::uint64_t cPositive = 0x4'000'000'000'000'000ull;
+    constexpr static std::uint64_t cMPToken = 0x2'000'000'000'000'000ull;
+    constexpr static std::uint64_t cValueMask = ~(cPositive | cMPToken);

    static std::uint64_t const uRateOne;

--- a/src/libxrpl/basics/Number.cpp
+++ b/src/libxrpl/basics/Number.cpp
@@ -43,8 +43,6 @@ namespace ripple {

 thread_local Number::rounding_mode Number::mode_ = Number::to_nearest;

-Number const Number::zero{};
-
 Number::rounding_mode
 Number::getround()
 {
@@ -523,6 +521,24 @@ Number::operator rep() const
    return drops;
 }

+Number
+Number::truncate() const noexcept
+{
+    if (exponent_ >= 0 || mantissa_ == 0)
+        return *this;
+
+    Number ret = *this;
+    while (ret.exponent_ < 0 && ret.mantissa_ != 0)
+    {
+        ret.exponent_ += 1;
+        ret.mantissa_ /= rep(10);
+    }
+    // We are guaranteed that normalize() will never throw an exception
+    // because exponent is either negative or zero at this point.
+    ret.normalize();
+    return ret;
+}
+
 std::string
 to_string(Number const& amount)
 {
--- a/src/test/app/Loan_test.cpp
+++ b/src/test/app/Loan_test.cpp
--- a/src/test/jtx/Env.h
+++ b/src/test/jtx/Env.h
@@ -251,7 +251,9 @@ public:
     *
     * @param suite_ the current unit_test::suite
     */
-    Env(beast::unit_test::suite& suite_) : Env(suite_, envconfig())
+    Env(beast::unit_test::suite& suite_,
+        beast::severities::Severity thresh = beast::severities::kError)
+        : Env(suite_, envconfig(), nullptr, thresh)
    {
    }

--- a/src/test/jtx/amount.h
+++ b/src/test/jtx/amount.h
@@ -295,6 +295,21 @@ struct XRP_t
        return {TOut{v} * dropsPerXRP};
    }

+    /** Returns an amount of XRP as PrettyAmount,
+        which is trivially convertable to STAmount
+
+        @param v The Number of XRP (not drops). May be fractional.
+    */
+    PrettyAmount
+    operator()(Number v) const
+    {
+        auto const c = dropsPerXRP.drops();
+        auto const d = std::int64_t(v * c);
+        if (Number(d) / c != v)
+            Throw<std::domain_error>("unrepresentable");
+        return {d};
+    }
+
    PrettyAmount
    operator()(double v) const
    {
--- a/src/xrpld/app/misc/LendingHelpers.h
+++ b/src/xrpld/app/misc/LendingHelpers.h
@@ -98,14 +98,24 @@ struct LoanState
    Number valueOutstanding;
    /// Prinicipal still due to be paid by the borrower.
    Number principalOutstanding;
-    /// Interest still due to be paid by the borrower.
-    Number interestOutstanding;
    /// Interest still due to be paid TO the Vault.
    // This is a portion of interestOutstanding
    Number interestDue;
    /// Management fee still due to be paid TO the broker.
    // This is a portion of interestOutstanding
    Number managementFeeDue;
+
+    /// Interest still due to be paid by the borrower.
+    Number
+    interestOutstanding() const
+    {
+        XRPL_ASSERT_PARTS(
+            interestDue + managementFeeDue ==
+                valueOutstanding - principalOutstanding,
+            "ripple::LoanState::interestOutstanding",
+            "other values add up correctly");
+        return interestDue + managementFeeDue;
+    }
 };

 LoanState
@@ -113,7 +123,7 @@ calculateRawLoanState(
    Number const& periodicPayment,
    Number const& periodicRate,
    std::uint32_t const paymentRemaining,
-    TenthBips16 const managementFeeRate);
+    TenthBips32 const managementFeeRate);

 LoanState
 calculateRawLoanState(
@@ -121,7 +131,7 @@ calculateRawLoanState(
    TenthBips32 interestRate,
    std::uint32_t paymentInterval,
    std::uint32_t const paymentRemaining,
-    TenthBips16 const managementFeeRate);
+    TenthBips32 const managementFeeRate);

 LoanState
 calculateRoundedLoanState(
@@ -136,7 +146,7 @@ Number
 computeFee(
    Asset const& asset,
    Number const& value,
-    TenthBips16 managementFeeRate,
+    TenthBips32 managementFeeRate,
    std::int32_t scale);

 Number
@@ -195,10 +205,18 @@ struct PaymentComponents

 struct LoanDeltas
 {
-    Number valueDelta;
    Number principalDelta;
    Number interestDueDelta;
    Number managementFeeDueDelta;
+
+    Number
+    valueDelta() const
+    {
+        return principalDelta + interestDueDelta + managementFeeDueDelta;
+    }
+
+    void
+    nonNegative();
 };

 PaymentComponents
@@ -218,6 +236,9 @@ computePaymentComponents(
 detail::LoanDeltas
 operator-(LoanState const& lhs, LoanState const& rhs);

+LoanState
+operator-(LoanState const& lhs, detail::LoanDeltas const& rhs);
+
 Number
 valueMinusFee(
    Asset const& asset,
@@ -232,7 +253,7 @@ computeLoanProperties(
    TenthBips32 interestRate,
    std::uint32_t paymentInterval,
    std::uint32_t paymentsRemaining,
-    TenthBips16 managementFeeRate);
+    TenthBips32 managementFeeRate);

 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
@@ -467,7 +467,7 @@ struct PaymentComponentsPlus : public PaymentComponents
    PaymentComponentsPlus(
        PaymentComponents const& p,
        Number f,
-        Number v = Number{})
+        Number v = numZero)
        : PaymentComponents(p)
        , untrackedManagementFee(f)
        , untrackedInterest(v)
@@ -673,7 +673,7 @@ tryOverpayment(
    auto const newRounded = calculateRoundedLoanState(
        totalValueOutstanding, principalOutstanding, managementFeeOutstanding);
    auto const valueChange =
-        newRounded.interestOutstanding - rounded.interestOutstanding;
+        newRounded.interestOutstanding() - rounded.interestOutstanding();
    XRPL_ASSERT_PARTS(
        valueChange < beast::zero,
        "ripple::detail::tryOverpayment",
@@ -999,6 +999,17 @@ PaymentComponents::trackedInterestPart() const
        (trackedPrincipalDelta + trackedManagementFeeDelta);
 }

+void
+LoanDeltas::nonNegative()
+{
+    if (principalDelta < beast::zero)
+        principalDelta = numZero;
+    if (interestDueDelta < beast::zero)
+        interestDueDelta = numZero;
+    if (managementFeeDueDelta < beast::zero)
+        managementFeeDueDelta = numZero;
+}
+
 PaymentComponents
 computePaymentComponents(
    Asset const& asset,
@@ -1035,8 +1046,6 @@ computePaymentComponents(
            roundToAsset(asset, trueTarget.valueOutstanding, scale),
        .principalOutstanding =
            roundToAsset(asset, trueTarget.principalOutstanding, scale),
-        .interestOutstanding =
-            roundToAsset(asset, trueTarget.interestOutstanding, scale),
        .interestDue = roundToAsset(asset, trueTarget.interestDue, scale),
        .managementFeeDue =
            roundToAsset(asset, trueTarget.managementFeeDue, scale)};
@@ -1044,67 +1053,38 @@ computePaymentComponents(
        totalValueOutstanding, principalOutstanding, managementFeeOutstanding);

    LoanDeltas deltas = currentLedgerState - roundedTarget;
-
-    // It should be impossible for any of the deltas to be negative, but do
-    // defensive checks
-    if (deltas.principalDelta < beast::zero)
-    {
-        // LCOV_EXCL_START
-        UNREACHABLE(
-            "ripple::detail::computePaymentComponents : negative principal "
-            "delta");
-        deltas.principalDelta = Number::zero;
-        // LCOV_EXCL_STOP
-    }
-    if (deltas.interestDueDelta < beast::zero)
-    {
-        // LCOV_EXCL_START
-        UNREACHABLE(
-            "ripple::detail::computePaymentComponents : negative interest "
-            "delta");
-        deltas.interestDueDelta = Number::zero;
-        // LCOV_EXCL_STOP
-    }
-    if (deltas.managementFeeDueDelta < beast::zero)
-    {
-        // LCOV_EXCL_START
-        UNREACHABLE(
-            "ripple::detail::computePaymentComponents : negative management "
-            "fee delta");
-        deltas.managementFeeDueDelta = Number::zero;
-        // LCOV_EXCL_STOP
-    }
+    deltas.nonNegative();

    // Adjust the deltas if necessary for data integrity
    XRPL_ASSERT_PARTS(
        deltas.principalDelta <= currentLedgerState.principalOutstanding,
        "ripple::detail::computePaymentComponents",
        "principal delta not greater than outstanding");
+
    deltas.principalDelta = std::min(
        deltas.principalDelta, currentLedgerState.principalOutstanding);
+
    XRPL_ASSERT_PARTS(
        deltas.interestDueDelta <= currentLedgerState.interestDue,
        "ripple::detail::computePaymentComponents",
        "interest due delta not greater than outstanding");
+
    deltas.interestDueDelta = std::min(
        {deltas.interestDueDelta,
-         std::max(Number::zero, roundedPeriodicPayment - deltas.principalDelta),
+         std::max(numZero, roundedPeriodicPayment - deltas.principalDelta),
         currentLedgerState.interestDue});
+
    XRPL_ASSERT_PARTS(
        deltas.managementFeeDueDelta <= currentLedgerState.managementFeeDue,
        "ripple::detail::computePaymentComponents",
        "management fee due delta not greater than outstanding");
+
    deltas.managementFeeDueDelta = std::min(
        {deltas.managementFeeDueDelta,
         roundedPeriodicPayment -
             (deltas.principalDelta + deltas.interestDueDelta),
         currentLedgerState.managementFeeDue});

-    // In case any adjustments were made (or if the original rounding didn't
-    // quite add up right), recompute the total value delta
-    deltas.valueDelta = deltas.principalDelta + deltas.interestDueDelta +
-        deltas.managementFeeDueDelta;
-
    if (paymentRemaining == 1 ||
        totalValueOutstanding <= roundedPeriodicPayment)
    {
@@ -1112,15 +1092,15 @@ computePaymentComponents(
        // parts.

        XRPL_ASSERT_PARTS(
-            deltas.valueDelta == totalValueOutstanding,
+            deltas.valueDelta() <= totalValueOutstanding,
            "ripple::detail::computePaymentComponents",
            "last payment total value agrees");
        XRPL_ASSERT_PARTS(
-            deltas.principalDelta == principalOutstanding,
+            deltas.principalDelta <= principalOutstanding,
            "ripple::detail::computePaymentComponents",
            "last payment principal agrees");
        XRPL_ASSERT_PARTS(
-            deltas.managementFeeDueDelta == managementFeeOutstanding,
+            deltas.managementFeeDueDelta <= managementFeeOutstanding,
            "ripple::detail::computePaymentComponents",
            "last payment management fee agrees");

@@ -1231,13 +1211,12 @@ computePaymentComponents(
    // trying to take more than the whole payment. The excess can be positive,
    // which indicates that we're not going to take the whole payment amount,
    // but if so, it must be small.
-    auto takeFrom = [](Number& total, Number& component, Number& excess) {
+    auto takeFrom = [](Number& component, Number& excess) {
        if (excess > beast::zero)
        {
            // Take as much of the excess as we can out of the provided part and
            // the total
            auto part = std::min(component, excess);
-            total -= part;
            component -= part;
            excess -= part;
        }
@@ -1248,14 +1227,41 @@ computePaymentComponents(
            "ripple::detail::computePaymentComponents",
            "excess non-negative");
    };
+    auto giveTo =
+        [](Number& component, Number& shortage, Number const& maximum) {
+            if (shortage > beast::zero)
+            {
+                // Put as much of the shortage as we can into the provided part
+                // and the total
+                auto part = std::min(maximum - component, shortage);
+                component += part;
+                shortage -= part;
+            }
+            // If the shortage goes negative, we put too much, which should be
+            // impossible
+            XRPL_ASSERT_PARTS(
+                shortage >= beast::zero,
+                "ripple::detail::computePaymentComponents",
+                "excess non-negative");
+        };
    auto addressExcess = [&takeFrom](LoanDeltas& deltas, Number& excess) {
-        takeFrom(deltas.valueDelta, deltas.interestDueDelta, excess);
-        takeFrom(deltas.valueDelta, deltas.managementFeeDueDelta, excess);
-        takeFrom(deltas.valueDelta, deltas.principalDelta, excess);
+        // This order is based on where errors are the least problematic
+        takeFrom(deltas.interestDueDelta, excess);
+        takeFrom(deltas.managementFeeDueDelta, excess);
+        takeFrom(deltas.principalDelta, excess);
+    };
+    auto addressShortage = [&giveTo](
+                               LoanDeltas& deltas,
+                               Number& shortage,
+                               LoanState const& current) {
+        giveTo(deltas.interestDueDelta, shortage, current.interestDue);
+        giveTo(deltas.principalDelta, shortage, current.principalOutstanding);
+        giveTo(
+            deltas.managementFeeDueDelta, shortage, current.managementFeeDue);
    };
    Number totalOverpayment =
-        deltas.valueDelta - currentLedgerState.valueOutstanding;
-    if (totalOverpayment > 0)
+        deltas.valueDelta() - currentLedgerState.valueOutstanding;
+    if (totalOverpayment > beast::zero)
    {
        // LCOV_EXCL_START
        UNREACHABLE(
@@ -1264,8 +1270,9 @@ computePaymentComponents(
        addressExcess(deltas, totalOverpayment);
        // LCOV_EXCL_STOP
    }
+
    // Make sure the parts don't add up to too much
-    Number shortage = roundedPeriodicPayment - deltas.valueDelta;
+    Number shortage = roundedPeriodicPayment - deltas.valueDelta();

    XRPL_ASSERT_PARTS(
        isRounded(asset, shortage, scale),
@@ -1280,38 +1287,63 @@ computePaymentComponents(

        shortage = -excess;
    }
+    else if (shortage > beast::zero && totalOverpayment < beast::zero)
+    {
+        // If there's a shortage, and there's room in the loan itself, we can
+        // top up the parts to make the payment correct.
+        shortage = std::min(-totalOverpayment, shortage);
+        addressShortage(deltas, shortage, currentLedgerState);
+    }

    // The shortage should never be negative, which indicates that the parts are
-    // trying to take more than the whole payment. The shortage can be positive,
-    // which indicates that we're not going to take the whole payment amount,
-    // but if so, it must be small.
+    // trying to take more than the whole payment. The shortage should not be
+    // positive, either, which indicates that we're not going to take the whole
+    // payment amount. Only the last payment should be allowed to have a
+    // shortage, and that's handled in a special case above.
    XRPL_ASSERT_PARTS(
-        shortage == beast::zero ||
-            (shortage > beast::zero &&
-             ((asset.integral() && shortage < 3) ||
-              (scale - shortage.exponent() > 14))),
+        shortage == beast::zero,
        "ripple::detail::computePaymentComponents",
-        "excess is extremely small");
+        "no shortage or excess");
+#if LOANCOMPLETE
+    /*
+    // This used to be part of the above assert. It will eventually be removed
+    // if proved accurate
+    ||
+        (shortage > beast::zero &&
+         ((asset.integral() && shortage < 3) ||
+          (scale - shortage.exponent() > 14)))
+          */
+#endif

    XRPL_ASSERT_PARTS(
-        deltas.valueDelta ==
+        deltas.valueDelta() ==
            deltas.principalDelta + deltas.interestDueDelta +
                deltas.managementFeeDueDelta,
        "ripple::detail::computePaymentComponents",
        "total value adds up");

    XRPL_ASSERT_PARTS(
-        deltas.principalDelta >= beast::zero,
+        deltas.principalDelta >= beast::zero &&
+            deltas.principalDelta <= currentLedgerState.principalOutstanding,
        "ripple::detail::computePaymentComponents",
-        "non-negative principal");
+        "valid principal result");
    XRPL_ASSERT_PARTS(
-        deltas.interestDueDelta >= beast::zero,
+        deltas.interestDueDelta >= beast::zero &&
+            deltas.interestDueDelta <= currentLedgerState.interestDue,
        "ripple::detail::computePaymentComponents",
-        "non-negative interest");
+        "valid interest result");
    XRPL_ASSERT_PARTS(
-        deltas.managementFeeDueDelta >= beast::zero,
+        deltas.managementFeeDueDelta >= beast::zero &&
+            deltas.managementFeeDueDelta <= currentLedgerState.managementFeeDue,
        "ripple::detail::computePaymentComponents",
-        "non-negative fee");
+        "valid fee result");
+
+    XRPL_ASSERT_PARTS(
+        deltas.principalDelta + deltas.interestDueDelta +
+                deltas.managementFeeDueDelta >
+            beast::zero,
+        "ripple::detail::computePaymentComponents",
+        "payment parts add to payment");

    return PaymentComponents{
 #if LOANCOMPLETE
@@ -1319,11 +1351,18 @@ computePaymentComponents(
        .rawPrincipal = rawPrincipal,
        .rawManagementFee = rawFee,
 #endif
-        // As a final safety check, don't return any negative values
-        .trackedValueDelta = std::max(deltas.valueDelta, Number::zero),
-        .trackedPrincipalDelta = std::max(deltas.principalDelta, Number::zero),
-        .trackedManagementFeeDelta =
-            std::max(deltas.managementFeeDueDelta, Number::zero),
+        // As a final safety check, ensure the value is non-negative, and won't
+        // make the corresponding item negative
+        .trackedValueDelta = std::clamp(
+            deltas.valueDelta(), numZero, currentLedgerState.valueOutstanding),
+        .trackedPrincipalDelta = std::clamp(
+            deltas.principalDelta,
+            numZero,
+            currentLedgerState.principalOutstanding),
+        .trackedManagementFeeDelta = std::clamp(
+            deltas.managementFeeDueDelta,
+            numZero,
+            currentLedgerState.managementFeeDue),
    };
 }

@@ -1381,28 +1420,23 @@ detail::LoanDeltas
 operator-(LoanState const& lhs, LoanState const& rhs)
 {
    detail::LoanDeltas result{
-        .valueDelta = lhs.valueOutstanding - rhs.valueOutstanding,
        .principalDelta = lhs.principalOutstanding - rhs.principalOutstanding,
        .interestDueDelta = lhs.interestDue - rhs.interestDue,
        .managementFeeDueDelta = lhs.managementFeeDue - rhs.managementFeeDue,
    };

-    XRPL_ASSERT_PARTS(
-        result.valueDelta >= 0,
-        "ripple::operator-(LoanState,LoanState)",
-        "valueDelta difference non-negative");
-    XRPL_ASSERT_PARTS(
-        result.principalDelta >= 0,
-        "ripple::operator-(LoanState,LoanState)",
-        "principalDelta difference non-negative");
-    XRPL_ASSERT_PARTS(
-        result.interestDueDelta >= 0,
-        "ripple::operator-(LoanState,LoanState)",
-        "interestDueDelta difference non-negative");
-    XRPL_ASSERT_PARTS(
-        result.managementFeeDueDelta >= 0,
-        "ripple::operator-(LoanState,LoanState)",
-        "managementFeeDueDelta difference non-negative");
+    return result;
+}
+
+LoanState
+operator-(LoanState const& lhs, detail::LoanDeltas const& rhs)
+{
+    LoanState result{
+        .valueOutstanding = lhs.valueOutstanding - rhs.valueDelta(),
+        .principalOutstanding = lhs.principalOutstanding - rhs.principalDelta,
+        .interestDue = lhs.interestDue - rhs.interestDueDelta,
+        .managementFeeDue = lhs.managementFeeDue - rhs.managementFeeDueDelta,
+    };

    return result;
 }
@@ -1470,14 +1504,13 @@ calculateRawLoanState(
    Number const& periodicPayment,
    Number const& periodicRate,
    std::uint32_t const paymentRemaining,
-    TenthBips16 const managementFeeRate)
+    TenthBips32 const managementFeeRate)
 {
    if (paymentRemaining == 0)
    {
        return LoanState{
            .valueOutstanding = 0,
            .principalOutstanding = 0,
-            .interestOutstanding = 0,
            .interestDue = 0,
            .managementFeeDue = 0};
    }
@@ -1493,7 +1526,6 @@ calculateRawLoanState(
    return LoanState{
        .valueOutstanding = rawValueOutstanding,
        .principalOutstanding = rawPrincipalOutstanding,
-        .interestOutstanding = rawInterestOutstanding,
        .interestDue = rawInterestOutstanding - rawManagementFeeOutstanding,
        .managementFeeDue = rawManagementFeeOutstanding};
 };
@@ -1504,7 +1536,7 @@ calculateRawLoanState(
    TenthBips32 interestRate,
    std::uint32_t paymentInterval,
    std::uint32_t const paymentRemaining,
-    TenthBips16 const managementFeeRate)
+    TenthBips32 const managementFeeRate)
 {
    return calculateRawLoanState(
        periodicPayment,
@@ -1521,13 +1553,11 @@ calculateRoundedLoanState(
 {
    // This implementation is pretty trivial, but ensures the calculations are
    // consistent everywhere, and reduces copy/paste errors.
-    Number const interestOutstanding =
-        totalValueOutstanding - principalOutstanding;
    return {
        .valueOutstanding = totalValueOutstanding,
        .principalOutstanding = principalOutstanding,
-        .interestOutstanding = interestOutstanding,
-        .interestDue = interestOutstanding - managementFeeOutstanding,
+        .interestDue = totalValueOutstanding - principalOutstanding -
+            managementFeeOutstanding,
        .managementFeeDue = managementFeeOutstanding};
 }

@@ -1544,7 +1574,7 @@ Number
 computeFee(
    Asset const& asset,
    Number const& value,
-    TenthBips16 managementFeeRate,
+    TenthBips32 managementFeeRate,
    std::int32_t scale)
 {
    return roundToAsset(
@@ -1571,7 +1601,7 @@ computeLoanProperties(
    TenthBips32 interestRate,
    std::uint32_t paymentInterval,
    std::uint32_t paymentsRemaining,
-    TenthBips16 managementFeeRate)
+    TenthBips32 managementFeeRate)
 {
    auto const periodicRate = loanPeriodicRate(interestRate, paymentInterval);
    XRPL_ASSERT(
--- a/src/xrpld/app/tx/detail/LoanPay.cpp
+++ b/src/xrpld/app/tx/detail/LoanPay.cpp
@@ -374,6 +374,10 @@ LoanPay::doApply()
        paymentParts->principalPaid + paymentParts->interestPaid;
    auto const totalPaidToVaultRounded =
        roundToAsset(asset, totalPaidToVaultRaw, vaultScale, Number::downward);
+    XRPL_ASSERT_PARTS(
+        !asset.integral() || totalPaidToVaultRaw == totalPaidToVaultRounded,
+        "ripple::LoanPay::doApply",
+        "rounding does nothing for integral asset");
    auto const totalPaidToVaultForDebt =
        totalPaidToVaultRaw - paymentParts->valueChange;

@@ -405,7 +409,21 @@ LoanPay::doApply()
    // Vault object state changes
    view.update(vaultSle);

+    Number const assetsAvailableBefore = *assetsAvailableProxy;
+    Number const pseudoAccountBalanceBefore = accountHolds(
+        view,
+        vaultPseudoAccount,
+        asset,
+        FreezeHandling::fhIGNORE_FREEZE,
+        AuthHandling::ahIGNORE_AUTH,
+        j_);
+
    {
+        XRPL_ASSERT_PARTS(
+            assetsAvailableBefore == pseudoAccountBalanceBefore,
+            "ripple::LoanPay::doApply",
+            "vault pseudo balance agrees before");
+
        auto assetsTotalProxy = vaultSle->at(sfAssetsTotal);

        assetsAvailableProxy += totalPaidToVaultRounded;
@@ -415,6 +433,13 @@ LoanPay::doApply()
            *assetsAvailableProxy <= *assetsTotalProxy,
            "ripple::LoanPay::doApply",
            "assets available must not be greater than assets outstanding");
+
+        if (*assetsAvailableProxy > *assetsTotalProxy)
+        {
+            // LCOV_EXCL_START
+            return tecINTERNAL;
+            // LCOV_EXCL_STOP
+        }
    }

    // Move funds
@@ -488,6 +513,19 @@ LoanPay::doApply()
            WaiveTransferFee::Yes))
        return ter;

+    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");
+
 #if !NDEBUG
    auto const accountBalanceAfter = accountCanSend(
        view, account_, asset, fhIGNORE_FREEZE, ahIGNORE_AUTH, j_);
--- a/src/xrpld/app/tx/detail/LoanSet.cpp
+++ b/src/xrpld/app/tx/detail/LoanSet.cpp
@@ -318,6 +318,87 @@ LoanSet::preclaim(PreclaimContext const& ctx)
    return tesSUCCESS;
 }

+TER
+LoanSet::checkGuards(
+    Asset const& vaultAsset,
+    Number const& principalRequested,
+    TenthBips32 interestRate,
+    std::uint32_t paymentTotal,
+    LoanProperties const& properties,
+    beast::Journal j)
+{
+    auto const totalInterestOutstanding =
+        properties.totalValueOutstanding - principalRequested;
+    // Guard 1: if there is no computed total interest over the life of the
+    // loan for a non-zero interest rate, we cannot properly amortize the
+    // loan
+    if (interestRate > TenthBips32{0} && totalInterestOutstanding <= 0)
+    {
+        // Unless this is a zero-interest loan, there must be some interest
+        // due on the loan, even if it's (measurable) dust
+        JLOG(j.warn()) << "Loan for " << principalRequested << " with "
+                       << interestRate << "% interest has no interest due";
+        return tecPRECISION_LOSS;
+    }
+    // Guard 1a: If there is any interest computed over the life of the
+    // loan, for a zero interest rate, something went sideways.
+    if (interestRate == TenthBips32{0} && totalInterestOutstanding > 0)
+    {
+        // LCOV_EXCL_START
+        JLOG(j.warn()) << "Loan for " << principalRequested
+                       << " with 0% interest has interest due";
+        return tecINTERNAL;
+        // LCOV_EXCL_STOP
+    }
+
+    // Guard 2: if the principal portion of the first periodic payment is
+    // too small to be accurately represented with the given rounding mode,
+    // raise an error
+    if (properties.firstPaymentPrincipal <= 0)
+    {
+        // Check that some true (unrounded) principal is paid each period.
+        // Since the first payment pays the least principal, if it's good,
+        // they'll all be good. Note that the outstanding principal is
+        // rounded, and may not change right away.
+        JLOG(j.warn()) << "Loan is unable to pay principal.";
+        return tecPRECISION_LOSS;
+    }
+
+    // Guard 3: If the periodic payment is so small that it can't even be
+    // rounded to a representable value, then the loan can't be paid. Also,
+    // avoids dividing by 0.
+    auto const roundedPayment = roundPeriodicPayment(
+        vaultAsset, properties.periodicPayment, properties.loanScale);
+    if (roundedPayment == beast::zero)
+    {
+        JLOG(j.warn()) << "Loan Periodic payment ("
+                       << properties.periodicPayment << ") rounds to 0. ";
+        return tecPRECISION_LOSS;
+    }
+
+    // Guard 4: if the rounded periodic payment is large enough that the
+    // loan can't be amortized in the specified number of payments, raise an
+    // error
+    {
+        NumberRoundModeGuard mg(Number::upward);
+
+        if (std::int64_t const computedPayments{
+                properties.totalValueOutstanding / roundedPayment};
+            computedPayments != paymentTotal)
+        {
+            JLOG(j.warn()) << "Loan Periodic payment ("
+                           << properties.periodicPayment << ") rounding ("
+                           << roundedPayment << ") on a total value of "
+                           << properties.totalValueOutstanding
+                           << " can not complete the loan in the specified "
+                              "number of payments ("
+                           << computedPayments << " != " << paymentTotal << ")";
+            return tecPRECISION_LOSS;
+        }
+    }
+    return tesSUCCESS;
+}
+
 TER
 LoanSet::doApply()
 {
@@ -396,72 +477,14 @@ LoanSet::doApply()
        }
    }

-    auto const totalInterestOutstanding =
-        properties.totalValueOutstanding - principalRequested;
-    // Guard 1: if there is no computed total interest over the life of the loan
-    // for a non-zero interest rate, we cannot properly amortize the loan
-    if (interestRate > TenthBips32{0} && totalInterestOutstanding <= 0)
-    {
-        // Unless this is a zero-interest loan, there must be some interest due
-        // on the loan, even if it's (measurable) dust
-        JLOG(j_.warn()) << "Loan for " << principalRequested << " with "
-                        << interestRate << "% interest has no interest due";
-        return tecPRECISION_LOSS;
-    }
-    // Guard 1a: If there is any interest computed over the life of the loan,
-    // for a zero interest rate, something went sideways.
-    if (interestRate == TenthBips32{0} && totalInterestOutstanding > 0)
-    {
-        // LCOV_EXCL_START
-        JLOG(j_.warn()) << "Loan for " << principalRequested
-                        << " with 0% interest has interest due";
-        return tecINTERNAL;
-        // LCOV_EXCL_STOP
-    }
-
-    // Guard 2: if the principal portion of the first periodic payment is too
-    // small to be accurately represented with the given rounding mode, raise an
-    // error
-    if (properties.firstPaymentPrincipal <= 0)
-    {
-        // Check that some true (unrounded) principal is paid each period. Since
-        // the first payment pays the least principal, if it's good, they'll
-        // all be good. Note that the outstanding principal is rounded, and
-        // may not change right away.
-        JLOG(j_.warn()) << "Loan is unable to pay principal.";
-        return tecPRECISION_LOSS;
-    }
-
-    // Guard 3: If the periodic payment is so small that it can't even be
-    // rounded to a representable value, then the loan can't be paid. Also,
-    // avoids dividing by 0.
-    auto const roundedPayment = roundPeriodicPayment(
-        vaultAsset, properties.periodicPayment, properties.loanScale);
-    if (roundedPayment == beast::zero)
-    {
-        JLOG(j_.warn()) << "Loan Periodic payment ("
-                        << properties.periodicPayment << ") rounds to 0. ";
-        return tecPRECISION_LOSS;
-    }
-
-    // Guard 4: if the rounded periodic payment is large enough that the loan
-    // can't be amortized in the specified number of payments, raise an error
-    {
-        NumberRoundModeGuard mg(Number::upward);
-
-        if (std::int64_t const computedPayments{
-                properties.totalValueOutstanding / roundedPayment};
-            computedPayments != paymentTotal)
-        {
-            JLOG(j_.warn())
-                << "Loan Periodic payment (" << properties.periodicPayment
-                << ") rounding (" << roundedPayment
-                << ") will complete the "
-                   "loan in less than the specified number of payments ("
-                << computedPayments << " < " << paymentTotal << ")";
-            return tecPRECISION_LOSS;
-        }
-    }
+    if (auto const ret = checkGuards(
+            vaultAsset,
+            principalRequested,
+            interestRate,
+            paymentTotal,
+            properties,
+            j_))
+        return ret;

    // Check that the other computed values are valid
    if (properties.managementFeeOwedToBroker < 0 ||
--- a/src/xrpld/app/tx/detail/LoanSet.h
+++ b/src/xrpld/app/tx/detail/LoanSet.h
@@ -20,6 +20,7 @@
 #ifndef RIPPLE_TX_LOANSET_H_INCLUDED
 #define RIPPLE_TX_LOANSET_H_INCLUDED

+#include <xrpld/app/misc/LendingHelpers.h>
 #include <xrpld/app/tx/detail/Transactor.h>

 namespace ripple {
@@ -54,6 +55,15 @@ public:
    static TER
    preclaim(PreclaimContext const& ctx);

+    static TER
+    checkGuards(
+        Asset const& vaultAsset,
+        Number const& principalRequested,
+        TenthBips32 interestRate,
+        std::uint32_t paymentTotal,
+        LoanProperties const& properties,
+        beast::Journal j);
+
    TER
    doApply() override;