refactor: Payment component calculation will target next true state

- Compute the next "true" state, round the values off, then compute the deltas needed to get the current state to that state. Plus some data integrity checks. - Add `Number::zero`, which is longer to type, but more readable than `Number{}`. - Prepare to improve Loan unit tests: track managementFeeRate in BrokerInfo, define a LoanParameters object for creation options and start adding support for it, track and verify loan state while making payments.
2025-12-01 08:25:51 +00:00 · 2025-10-28 14:49:15 -04:00
parent f4404eafbd
commit 310852ba2d
6 changed files with 437 additions and 54 deletions
--- a/include/xrpl/basics/Number.h
+++ b/include/xrpl/basics/Number.h
@@ -58,6 +58,8 @@ 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
--- a/src/libxrpl/basics/Number.cpp
+++ b/src/libxrpl/basics/Number.cpp
@@ -43,6 +43,8 @@ namespace ripple {

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

+Number const Number::zero{};
+
 Number::rounding_mode
 Number::getround()
 {
--- a/src/test/app/Loan_test.cpp
+++ b/src/test/app/Loan_test.cpp
@@ -102,12 +102,84 @@ class Loan_test : public beast::unit_test::suite
    {
        jtx::PrettyAsset asset;
        uint256 brokerID;
-        BrokerInfo(jtx::PrettyAsset const& asset_, uint256 const& brokerID_)
-            : asset(asset_), brokerID(brokerID_)
+        TenthBips16 managementFeeRate;
+        BrokerInfo(
+            jtx::PrettyAsset const& asset_,
+            uint256 const& brokerID_,
+            TenthBips16 mgmtRate)
+            : asset(asset_), brokerID(brokerID_), managementFeeRate(mgmtRate)
        {
        }
    };

+    struct LoanParameters
+    {
+        BrokerInfo broker;
+        // The account submitting the transaction. May be borrower or broker.
+        jtx::Account account;
+        // The counterparty. Should be the other of borrower or broker.
+        jtx::Account counter;
+        // Whether the counterparty is specified in the `counterparty` field, or
+        // only signs.
+        bool counterpartyExplicit = true;
+        Number principalRequest;
+        STAmount setFee;
+        std::optional<Number> originationFee;
+        std::optional<Number> serviceFee;
+        std::optional<Number> lateFee;
+        std::optional<Number> closeFee;
+        std::optional<TenthBips32> overFee;
+        std::optional<TenthBips32> interest;
+        std::optional<TenthBips32> lateInterest;
+        std::optional<TenthBips32> closeInterest;
+        std::optional<TenthBips32> overpaymentInterest;
+        std::optional<std::uint32_t> payTotal;
+        std::optional<std::uint32_t> payInterval;
+        std::optional<std::uint32_t> gracePd;
+        std::optional<std::uint32_t> flags;
+
+        template <class... FN>
+        jtx::JTx
+        operator()(jtx::Env& env, FN const&... fN) const
+        {
+            using namespace jtx;
+            using namespace jtx::loan;
+
+            JTx jt{loan::set(
+                account, broker.brokerID, principalRequest, flags.value_or(0))};
+            sig(sfCounterpartySignature, counter)(env, jt);
+            fee{setFee}(env, jt);
+            if (counterpartyExplicit)
+                counterparty(counter)(env, jt);
+            if (originationFee)
+                loanOriginationFee (*originationFee)(env, jt);
+            if (serviceFee)
+                loanServiceFee (*serviceFee)(env, jt);
+            if (lateFee)
+                latePaymentFee (*lateFee)(env, jt);
+            if (closeFee)
+                closePaymentFee (*closeFee)(env, jt);
+            if (overFee)
+                overpaymentFee (*overFee)(env, jt);
+            if (interest)
+                interestRate (*interest)(env, jt);
+            if (lateInterest)
+                lateInterestRate (*lateInterest)(env, jt);
+            if (closeInterest)
+                closeInterestRate (*closeInterest)(env, jt);
+            if (overpaymentInterest)
+                overpaymentInterestRate (*overpaymentInterest)(env, jt);
+            if (payTotal)
+                paymentTotal (*payTotal)(env, jt);
+            if (payInterval)
+                paymentInterval (*payInterval)(env, jt);
+            if (gracePd)
+                gracePeriod (*gracePd)(env, jt);
+
+            return env.jt(jt, fN...);
+        }
+    };
+
    struct LoanState
    {
        std::uint32_t previousPaymentDate = 0;
@@ -373,7 +445,7 @@ class Loan_test : public beast::unit_test::suite

        env.close();

-        return {asset, keylet.key};
+        return {asset, keylet.key, managementFeeRateParameter};
    }

    /// Get the state without checking anything
@@ -541,7 +613,7 @@ class Loan_test : public beast::unit_test::suite

        auto const borrowerOwnerCount = env.ownerCount(borrower);

-        auto const loanSetFee = fee(env.current()->fees().base * 2);
+        auto const loanSetFee = env.current()->fees().base * 2;
        Number const principalRequest = broker.asset(loanAmount).value();
        auto const originationFee = broker.asset(1).value();
        auto const serviceFee = broker.asset(2).value();
@@ -583,7 +655,31 @@ class Loan_test : public beast::unit_test::suite
        auto const borrowerStartbalance = env.balance(borrower, broker.asset);

        // Use the defined values
-        auto createJtx = env.jt(
+        LoanParameters const loanParams{
+            .broker = broker,
+            .account = borrower,
+            .counter = lender,
+            .counterpartyExplicit = false,
+            .principalRequest = principalRequest,
+            .setFee = loanSetFee,
+            .originationFee = originationFee,
+            .serviceFee = serviceFee,
+            .lateFee = lateFee,
+            .closeFee = closeFee,
+            .overFee = overFee,
+            .interest = interest,
+            .lateInterest = lateInterest,
+            .closeInterest = closeInterest,
+            .overpaymentInterest = overpaymentInterest,
+            .payTotal = total,
+            .payInterval = interval,
+            .gracePd = grace,
+            .flags = flags,
+        };
+        auto createJtx = loanParams(env);
+#if LOANCOMPLETE
+        {
+            auto createJtxOld = env.jt(
                set(borrower, broker.brokerID, principalRequest, flags),
                sig(sfCounterpartySignature, lender),
                loanOriginationFee(originationFee),
@@ -599,6 +695,10 @@ class Loan_test : public beast::unit_test::suite
                paymentInterval(interval),
                gracePeriod(grace),
                fee(loanSetFee));
+            BEAST_EXPECT(
+                createJtx.stx->getJson(0) == createJtxOld.stx->getJson(0));
+        }
+#endif
        // Successfully create a Loan
        env(createJtx);

@@ -1777,7 +1877,7 @@ class Loan_test : public beast::unit_test::suite
                    << "\tPayment components: "
                    << "Payments remaining, rawInterest, rawPrincipal, "
                       "rawMFee, trackedValueDelta, trackedPrincipalDelta, "
-                       "trackedMgmtFeeDelta, special";
+                       "trackedInterestDelta, trackedMgmtFeeDelta, special";

                auto const serviceFee = broker.asset(2);

@@ -1821,6 +1921,7 @@ class Loan_test : public beast::unit_test::suite
                        << raw.managementFeeDue << ", "
                        << rounded.valueOutstanding << ", "
                        << rounded.principalOutstanding << ", "
+                        << rounded.interestOutstanding << ", "
                        << rounded.managementFeeDue;
                }

@@ -1838,6 +1939,19 @@ class Loan_test : public beast::unit_test::suite
                        state.loanScale,
                        Number::upward));

+                auto const initialState = state;
+                detail::PaymentComponents totalPaid{
+                    .trackedValueDelta = 0,
+                    .trackedPrincipalDelta = 0,
+                    .trackedManagementFeeDelta = 0};
+                Number totalInterestPaid = 0;
+
+                ripple::LoanState currentTrueState = calculateRawLoanState(
+                    state.periodicPayment,
+                    periodicRate,
+                    state.paymentRemaining,
+                    managementFeeRateParameter);
+
                while (state.paymentRemaining > 0)
                {
                    // Compute the expected principal amount
@@ -1853,13 +1967,27 @@ class Loan_test : public beast::unit_test::suite
                            state.paymentRemaining,
                            managementFeeRateParameter);

+                    BEAST_EXPECT(
+                        paymentComponents.trackedValueDelta <=
+                        roundedPeriodicPayment);
+
+                    ripple::LoanState const nextTrueState =
+                        calculateRawLoanState(
+                            state.periodicPayment,
+                            periodicRate,
+                            state.paymentRemaining - 1,
+                            managementFeeRateParameter);
+                    detail::LoanDeltas const deltas =
+                        currentTrueState - nextTrueState;
+
                    testcase
                        << "\tPayment components: " << state.paymentRemaining
-                        << ", " << paymentComponents.rawInterest << ", "
-                        << paymentComponents.rawPrincipal << ", "
-                        << paymentComponents.rawManagementFee << ", "
+                        << ", " << deltas.interestDueDelta << ", "
+                        << deltas.principalDelta << ", "
+                        << deltas.managementFeeDueDelta << ", "
                        << paymentComponents.trackedValueDelta << ", "
                        << paymentComponents.trackedPrincipalDelta << ", "
+                        << paymentComponents.trackedInterestPart() << ", "
                        << paymentComponents.trackedManagementFeeDelta << ", "
                        << (paymentComponents.specialCase ==
                                    detail::PaymentSpecialCase::final
@@ -1895,17 +2023,20 @@ class Loan_test : public beast::unit_test::suite
                        paymentComponents.trackedPrincipalDelta +
                            paymentComponents.trackedInterestPart() +
                            paymentComponents.trackedManagementFeeDelta);
+                    BEAST_EXPECT(
+                        paymentComponents.trackedValueDelta <=
+                        state.periodicPayment);

                    BEAST_EXPECT(
                        state.paymentRemaining < 12 ||
                        roundToAsset(
                            broker.asset,
-                            paymentComponents.rawPrincipal,
+                            deltas.principalDelta,
                            state.loanScale,
                            Number::upward) ==
                            roundToScale(
                                broker.asset(
-                                    Number(8333228690659858, -14),
+                                    Number(8333228695260180, -14),
                                    Number::upward),
                                state.loanScale,
                                Number::upward));
@@ -1923,10 +2054,8 @@ class Loan_test : public beast::unit_test::suite
                        paymentComponents.specialCase ==
                            detail::PaymentSpecialCase::final ||
                        (state.periodicPayment.exponent() -
-                         (paymentComponents.rawPrincipal +
-                          paymentComponents.rawInterest +
-                          paymentComponents.rawManagementFee -
-                          state.periodicPayment)
+                         (deltas.principalDelta + deltas.interestDueDelta +
+                          deltas.managementFeeDueDelta - state.periodicPayment)
                             .exponent()) > 14);

                    auto const borrowerBalanceBeforePayment =
@@ -1976,12 +2105,40 @@ class Loan_test : public beast::unit_test::suite
                    state.totalValue -= paymentComponents.trackedValueDelta;

                    verifyLoanStatus(state);
+
+                    totalPaid.trackedValueDelta +=
+                        paymentComponents.trackedValueDelta;
+                    totalPaid.trackedPrincipalDelta +=
+                        paymentComponents.trackedPrincipalDelta;
+                    totalPaid.trackedManagementFeeDelta +=
+                        paymentComponents.trackedManagementFeeDelta;
+                    totalInterestPaid +=
+                        paymentComponents.trackedInterestPart();
+
+                    currentTrueState = nextTrueState;
                }

                // Loan is paid off
                BEAST_EXPECT(state.paymentRemaining == 0);
                BEAST_EXPECT(state.principalOutstanding == 0);

+                // Make sure all the payments add up
+                BEAST_EXPECT(
+                    totalPaid.trackedValueDelta == initialState.totalValue);
+                BEAST_EXPECT(
+                    totalPaid.trackedPrincipalDelta ==
+                    initialState.principalOutstanding);
+                BEAST_EXPECT(
+                    totalPaid.trackedManagementFeeDelta ==
+                    initialState.managementFeeOutstanding);
+                // This is almost a tautology given the previous checks, but
+                // check it anyway for completeness.
+                BEAST_EXPECT(
+                    totalInterestPaid ==
+                    initialState.totalValue -
+                        (initialState.principalOutstanding +
+                         initialState.managementFeeOutstanding));
+
                // Can't impair or default a paid off loan
                env(manage(lender, loanKeylet.key, tfLoanImpair),
                    ter(tecNO_PERMISSION));
--- a/src/xrpld/app/misc/LendingHelpers.h
+++ b/src/xrpld/app/misc/LendingHelpers.h
@@ -170,10 +170,12 @@ enum class PaymentSpecialCase { none, final, extra };
 /// single loan payment
 struct PaymentComponents
 {
+#if LOANCOMPLETE
    // raw values are unrounded, and are based on pure math
    Number rawInterest;
    Number rawPrincipal;
    Number rawManagementFee;
+#endif
    // tracked values are rounded to the asset and loan scale, and correspond to
    // fields in the Loan ledger object.
    // trackedValueDelta modifies sfTotalValueOutstanding.
@@ -191,6 +193,14 @@ struct PaymentComponents
    trackedInterestPart() const;
 };

+struct LoanDeltas
+{
+    Number valueDelta;
+    Number principalDelta;
+    Number interestDueDelta;
+    Number managementFeeDueDelta;
+};
+
 PaymentComponents
 computePaymentComponents(
    Asset const& asset,
@@ -205,6 +215,9 @@ computePaymentComponents(

 }  // namespace detail

+detail::LoanDeltas
+operator-(LoanState const& lhs, LoanState const& rhs);
+
 Number
 valueMinusFee(
    Asset const& asset,
--- a/src/xrpld/app/misc/detail/LendingHelpers.cpp
+++ b/src/xrpld/app/misc/detail/LendingHelpers.cpp
@@ -213,6 +213,7 @@ loanAccruedInterest(
        paymentInterval;
 }

+#if LOANCOMPLETE
 Number
 computeRoundedPrincipalComponent(
    Asset const& asset,
@@ -448,6 +449,7 @@ computeRoundedInterestAndFeeComponents(
    return std::make_pair(
        std::max(Number{}, roundedInterest), std::max(Number{}, roundedFee));
 }
+#endif

 struct PaymentComponentsPlus : public PaymentComponents
 {
@@ -839,8 +841,10 @@ computeLatePayment(
        view.parentCloseTime(),
        nextDueDate);

+#if LOANCOMPLETE
    auto const [rawLateInterest, rawLateManagementFee] =
        computeInterestAndFeeParts(latePaymentInterest, managementFeeRate);
+#endif
    auto const [roundedLateInterest, roundedLateManagementFee] = [&]() {
        auto const interest =
            roundToAsset(asset, latePaymentInterest, loanScale);
@@ -859,7 +863,9 @@ computeLatePayment(
    // This preserves all the other fields without having to enumerate them.
    PaymentComponentsPlus const late = [&]() {
        auto inner = periodic;
+#if LOANCOMPLETE
        inner.rawInterest += rawLateInterest;
+#endif

        return PaymentComponentsPlus{
            inner,
@@ -932,8 +938,10 @@ computeFullPayment(
        startDate,
        closeInterestRate);

+#if LOANCOMPLETE
    auto const [rawFullInterest, rawFullManagementFee] =
        computeInterestAndFeeParts(fullPaymentInterest, managementFeeRate);
+#endif

    auto const [roundedFullInterest, roundedFullManagementFee] = [&]() {
        auto const interest =
@@ -947,9 +955,11 @@ computeFullPayment(

    PaymentComponentsPlus const full{
        PaymentComponents{
+#if LOANCOMPLETE
            .rawInterest = rawFullInterest,
            .rawPrincipal = rawPrincipalOutstanding,
            .rawManagementFee = rawFullManagementFee,
+#endif
            .trackedValueDelta = principalOutstanding +
                totalInterestOutstanding + managementFeeOutstanding,
            .trackedPrincipalDelta = principalOutstanding,
@@ -1011,35 +1021,123 @@ computePaymentComponents(
            isRounded(asset, managementFeeOutstanding, scale),
        "ripple::detail::computePaymentComponents",
        "Outstanding values are rounded");
+    XRPL_ASSERT_PARTS(
+        paymentRemaining > 0,
+        "ripple::detail::computePaymentComponents",
+        "some payments remaining");
    auto const roundedPeriodicPayment =
        roundPeriodicPayment(asset, periodicPayment, scale);

-    LoanState const raw = calculateRawLoanState(
-        periodicPayment, periodicRate, paymentRemaining, managementFeeRate);
+    LoanState const trueTarget = calculateRawLoanState(
+        periodicPayment, periodicRate, paymentRemaining - 1, managementFeeRate);
+    LoanState const roundedTarget = LoanState{
+        .valueOutstanding =
+            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)};
+    LoanState const currentLedgerState = calculateRoundedLoanState(
+        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
+    }
+
+    // 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),
+         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)
    {
        // If there's only one payment left, we need to pay off each of the loan
-        // parts. It's probably impossible for the subtraction to result in a
-        // negative value, but don't leave anything to chance.
-        Number interest = std::max(
-            Number{},
-            totalValueOutstanding - principalOutstanding -
-                managementFeeOutstanding);
+        // parts.
+
+        XRPL_ASSERT(
+            deltas.valueDelta == totalValueOutstanding,
+            "ripple::detail::computePaymentComponents",
+            "last payment total value agrees");
+        XRPL_ASSERT(
+            deltas.principalDelta == principalOutstanding,
+            "ripple::detail::computePaymentComponents",
+            "last payment principal agrees");
+        XRPL_ASSERT(
+            deltas.managementFeeDueDelta == managementFeeOutstanding,
+            "ripple::detail::computePaymentComponents",
+            "last payment management fee agrees");

        // Pay everything off
        return PaymentComponents{
+#if LOANCOMPLETE
            .rawInterest = raw.interestOutstanding,
            .rawPrincipal = raw.principalOutstanding,
            .rawManagementFee = raw.managementFeeDue,
-            .trackedValueDelta =
-                interest + principalOutstanding + managementFeeOutstanding,
+#endif
+            .trackedValueDelta = totalValueOutstanding,
            .trackedPrincipalDelta = principalOutstanding,
            .trackedManagementFeeDelta = managementFeeOutstanding,
            .specialCase = PaymentSpecialCase::final};
    }

+#if LOANCOMPLETE
    /*
     * From the spec, once the periodicPayment is computed:
     *
@@ -1127,14 +1225,91 @@ computePaymentComponents(
            roundedPeriodicPayment,
        "ripple::detail::computePaymentComponents",
        "payment parts fit within payment limit");
+#endif
+
+    // Make sure the parts don't add up to too much
+    Number shortage = roundedPeriodicPayment - deltas.valueDelta;
+
+    XRPL_ASSERT_PARTS(
+        isRounded(asset, shortage, scale),
+        "ripple::detail::computePaymentComponents",
+        "excess is rounded");
+
+    // The shortage must never be negative, which indicates that the parts are
+    // 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) {
+        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;
+        }
+        // If the excess goes negative, we took too much, which should be
+        // impossible
+        XRPL_ASSERT_PARTS(
+            excess >= beast::zero,
+            "ripple::detail::computePaymentComponents",
+            "excess non-negative");
+    };
+    if (shortage < beast::zero)
+    {
+        Number excess = -shortage;
+
+        takeFrom(deltas.valueDelta, deltas.principalDelta, excess);
+        takeFrom(deltas.valueDelta, deltas.interestDueDelta, excess);
+        takeFrom(deltas.valueDelta, deltas.managementFeeDueDelta, excess);
+
+        shortage = -excess;
+    }
+
+    // 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.
+    XRPL_ASSERT_PARTS(
+        shortage == beast::zero ||
+            (shortage > beast::zero &&
+             ((asset.integral() && shortage < 3) ||
+              (scale - shortage.exponent() > 14))),
+        "ripple::detail::computePaymentComponents",
+        "excess is extremely small");
+
+    XRPL_ASSERT(
+        deltas.valueDelta ==
+            deltas.principalDelta + deltas.interestDueDelta +
+                deltas.managementFeeDueDelta,
+        "ripple::detail::computePaymentComponents",
+        "total value adds up");
+
+    XRPL_ASSERT_PARTS(
+        deltas.principalDelta >= beast::zero,
+        "ripple::detail::computePaymentComponents",
+        "non-negative principal");
+    XRPL_ASSERT_PARTS(
+        deltas.interestDueDelta >= beast::zero,
+        "ripple::detail::computePaymentComponents",
+        "non-negative interest");
+    XRPL_ASSERT_PARTS(
+        deltas.managementFeeDueDelta >= beast::zero,
+        "ripple::detail::computePaymentComponents",
+        "non-negative fee");

    return PaymentComponents{
+#if LOANCOMPLETE
        .rawInterest = rawInterest - rawFee,
        .rawPrincipal = rawPrincipal,
        .rawManagementFee = rawFee,
-        .trackedValueDelta = roundedInterest + roundedPrincipal + roundedFee,
-        .trackedPrincipalDelta = roundedPrincipal,
-        .trackedManagementFeeDelta = roundedFee,
+#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),
    };
 }

@@ -1172,9 +1347,11 @@ computeOverpaymentComponents(

    return detail::PaymentComponentsPlus{
        detail::PaymentComponents{
+#if LOANCOMPLETE
            .rawInterest = rawOverpaymentInterest,
            .rawPrincipal = payment - rawOverpaymentInterest,
            .rawManagementFee = 0,
+#endif
            .trackedValueDelta = payment,
            .trackedPrincipalDelta = payment - roundedOverpaymentInterest -
                roundedOverpaymentManagementFee,
@@ -1186,6 +1363,36 @@ computeOverpaymentComponents(

 }  // namespace detail

+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;
+}
+
 Number
 calculateFullPaymentInterest(
    Number const& rawPrincipalOutstanding,
@@ -1404,20 +1611,21 @@ computeLoanProperties(
    auto const firstPaymentPrincipal = [&]() {
        // Compute the parts for the first payment. Ensure that the
        // principal payment will actually change the principal.
-        auto const paymentComponents = detail::computePaymentComponents(
-            asset,
-            loanScale,
-            totalValueOutstanding,
-            principalOutstanding,
-            feeOwedToBroker,
+        auto const startingState = calculateRawLoanState(
            periodicPayment,
            periodicRate,
            paymentsRemaining,
            managementFeeRate);
+        auto const firstPaymentState = calculateRawLoanState(
+            periodicPayment,
+            periodicRate,
+            paymentsRemaining - 1,
+            managementFeeRate);

        // The unrounded principal part needs to be large enough to affect the
        // principal. What to do if not is left to the caller
-        return paymentComponents.rawPrincipal;
+        return startingState.principalOutstanding -
+            firstPaymentState.principalOutstanding;
    }();

    return LoanProperties{
@@ -1696,12 +1904,14 @@ loanMakePayment(
            nextPayment.trackedPrincipalDelta >= 0,
            "ripple::loanMakePayment",
            "additional payment pays non-negative principal");
+#if LOANCOMPLETE
        XRPL_ASSERT(
            nextPayment.rawInterest <= periodic.rawInterest,
            "ripple::loanMakePayment : decreasing interest");
        XRPL_ASSERT(
-            nextPayment.rawPrincipal >= periodic.rawPrincipal,
+            nextPayment.rawPrinicpal >= periodic.rawPrincipal,
            "ripple::loanMakePayment : increasing principal");
+#endif

        if (amount < totalPaid + nextPayment.totalDue)
            // We're done making payments.
@@ -1764,8 +1974,7 @@ loanMakePayment(

        // Don't process an overpayment if the whole amount (or more!)
        // gets eaten by fees and interest.
-        if (overpaymentComponents.rawPrincipal > 0 &&
-            overpaymentComponents.trackedPrincipalDelta > 0)
+        if (overpaymentComponents.trackedPrincipalDelta > 0)
        {
            XRPL_ASSERT_PARTS(
                overpaymentComponents.untrackedInterest >= beast::zero,
--- a/src/xrpld/app/tx/detail/LoanSet.cpp
+++ b/src/xrpld/app/tx/detail/LoanSet.cpp
@@ -451,7 +451,7 @@ LoanSet::doApply()

        if (std::int64_t const computedPayments{
                properties.totalValueOutstanding / roundedPayment};
-            computedPayments < paymentTotal)
+            computedPayments != paymentTotal)
        {
            JLOG(j_.warn())
                << "Loan Periodic payment (" << properties.periodicPayment