Handle overpayment calculations

src/xrpld/app/misc/LendingHelpers.h

@@ -45,6 +45,7 @@ struct PaymentComponents
     // We may not need roundedPayment
     Number roundedPayment;
     bool final = false;
+    bool extra = false;
 };
 
 // This structure is explained in the XLS-66 spec, section 3.2.4.4 (Failure
@@ -279,39 +280,41 @@ doPayment(
         nextDueDateProxy,
         "ripple::detail::doPayment",
         "Next due date proxy set");
-    if (payment.final)
+    if (!payment.extra)
     {
-        paymentRemainingProxy = 0;
+        if (payment.final)
-        XRPL_ASSERT_PARTS(
+        {
-            referencePrincipalProxy == payment.rawPrincipal,
+            paymentRemainingProxy = 0;
-            "ripple::detail::doPayment",
+            XRPL_ASSERT_PARTS(
-            "Full reference principal payment");
+                referencePrincipalProxy == payment.rawPrincipal,
-        XRPL_ASSERT_PARTS(
+                "ripple::detail::doPayment",
-            principalOutstandingProxy == payment.roundedPrincipal,
+                "Full reference principal payment");
-            "ripple::detail::doPayment",
+            XRPL_ASSERT_PARTS(
-            "Full principal payment");
+                principalOutstandingProxy == payment.roundedPrincipal,
-        XRPL_ASSERT_PARTS(
+                "ripple::detail::doPayment",
-            totalValueOutstandingProxy ==
+                "Full principal payment");
-                payment.roundedPrincipal + payment.roundedInterest,
+            XRPL_ASSERT_PARTS(
-            "ripple::detail::doPayment",
+                totalValueOutstandingProxy ==
-            "Full value payment");
+                    payment.roundedPrincipal + payment.roundedInterest,
+                "ripple::detail::doPayment",
+                "Full value payment");
 
-        prevPaymentDateProxy = *nextDueDateProxy;
+            prevPaymentDateProxy = *nextDueDateProxy;
-        // Remove the field. This is the only condition where nextDueDate is
+            // Remove the field. This is the only condition where nextDueDate is
-        // allowed to be removed.
+            // allowed to be removed.
-        nextDueDateProxy = std::nullopt;
+            nextDueDateProxy = std::nullopt;
-    }
+        }
-    else
+        else
-    {
+        {
-        paymentRemainingProxy -= 1;
+            paymentRemainingProxy -= 1;
 
-        prevPaymentDateProxy = *nextDueDateProxy;
+            prevPaymentDateProxy = *nextDueDateProxy;
-        // STObject::OptionalField does not define operator+=, and I don't want
+            // STObject::OptionalField does not define operator+=, and I don't
-        // to add one right now.
+            // want to add one right now.
-        nextDueDateProxy = *nextDueDateProxy + paymentInterval;
+            nextDueDateProxy = *nextDueDateProxy + paymentInterval;
+        }
     }
-    // A single payment always pays the same amount of principal. Only the
+
-    // interest and fees are extra for a late payment
     referencePrincipalProxy -= payment.rawPrincipal;
     principalOutstandingProxy -= payment.roundedPrincipal;
     totalValueOutstandingProxy -=
@@ -800,6 +803,7 @@ loanMakePayment(
     ApplyView& view,
     SLE::ref loan,
     STAmount const& amount,
+    TenthBips16 managementFeeRate,
     beast::Journal j)
 {
     /*
@@ -826,20 +830,16 @@ loanMakePayment(
 
     std::int32_t const loanScale = loan->at(sfLoanScale);
     auto totalValueOutstandingProxy = loan->at(sfTotalValueOutstanding);
-    auto interestOwedProxy = loan->at(sfInterestOwed);
     auto referencePrincipalProxy = loan->at(sfReferencePrincipal);
 
     TenthBips32 const interestRate{loan->at(sfInterestRate)};
     TenthBips32 const lateInterestRate{loan->at(sfLateInterestRate)};
     TenthBips32 const closeInterestRate{loan->at(sfCloseInterestRate)};
-    TenthBips32 const overpaymentInterestRate{
-        loan->at(sfOverpaymentInterestRate)};
 
     Number const serviceFee = loan->at(sfLoanServiceFee);
     Number const latePaymentFee = loan->at(sfLatePaymentFee);
     Number const closePaymentFee =
         roundToAsset(asset, loan->at(sfClosePaymentFee), loanScale);
-    TenthBips32 const overpaymentFee{loan->at(sfOverpaymentFee)};
 
     auto const periodicPayment = loan->at(sfPeriodicPayment);
 
@@ -857,10 +857,6 @@ loanMakePayment(
     XRPL_ASSERT(
         *totalValueOutstandingProxy > 0,
         "ripple::loanMakePayment : valid total value");
-    XRPL_ASSERT_PARTS(
-        *interestOwedProxy >= 0,
-        "ripple::loanMakePayment",
-        "valid interest owed");
 
     view.update(loan);
 
@@ -1027,68 +1023,138 @@ loanMakePayment(
         "ripple::loanMakePayment",
         "fee parts add up");
 
-    return Unexpected(temDISABLED);
-#if LOANCOMPLETE
     // -------------------------------------------------------------
     // overpayment handling
-    Number overpaymentInterestPortion = 0;
     if (loan->isFlag(lsfLoanOverpayment))
     {
-        Number const overpayment = std::min(
+        TenthBips32 const overpaymentInterestRate{
-            principalOutstandingProxy.value(),
+            loan->at(sfOverpaymentInterestRate)};
-            amount - (totalPrincipalPaid + totalInterestPaid + totalfeeToPay));
+        TenthBips32 const overpaymentFeeRate{loan->at(sfOverpaymentFee)};
 
-        if (roundToAsset(asset, overpayment, loanScale) > 0)
+        Number const overpayment = amount -
+            (totalParts.principalPaid + totalParts.interestPaid +
+             totalParts.feeToPay);
+        XRPL_ASSERT(
+            overpayment >= 0 && isRounded(asset, overpayment, loanScale),
+            "ripple::loanMakePayment : valid overpayment amount");
+
+        Number const fee = roundToAsset(
+            asset,
+            tenthBipsOfValue(overpayment, overpaymentFeeRate),
+            loanScale);
+
+        Number const payment = overpayment - fee;
+
+        Number const interest =
+            tenthBipsOfValue(payment, overpaymentInterestRate);
+        Number const roundedInterest = roundToAsset(asset, interest, loanScale);
+
+        detail::PaymentComponentsPlus overpaymentComponents{
+            PaymentComponents{
+                .rawInterest = interest,
+                .rawPrincipal = payment - interest,
+                .roundedInterest = roundedInterest,
+                .roundedPrincipal = payment - roundedInterest,
+                .roundedPayment = payment,
+                .extra = true},
+            fee};
+
+        // Don't process an overpayment if the whole amount (or more!)
+        // gets eaten by fees and interest.
+        if (overpaymentComponents.rawPrincipal > 0 &&
+            overpaymentComponents.roundedPrincipal > 0)
         {
-            Number const interestPortion = roundToAsset(
+            Number const totalInterestOutstandingBefore =
-                asset,
+                totalValueOutstandingProxy - principalOutstandingProxy;
-                tenthBipsOfValue(overpayment, overpaymentInterestRate),
-                loanScale);
-            Number const feePortion = roundToAsset(
-                asset,
-                tenthBipsOfValue(overpayment, overpaymentFee),
-                loanScale);
-            Number const remainder = roundToAsset(
-                asset, overpayment - interestPortion - feePortion, loanScale);
 
-            // Don't process an overpayment if the whole amount (or more!)
+            auto const oldLoanProperties = computeLoanProperties(
-            // gets eaten by fees
+                asset,
-            if (remainder > 0)
+                principalOutstandingProxy,
+                referencePrincipalProxy,
+                interestRate,
+                paymentInterval,
+                paymentRemainingProxy,
+                managementFeeRate);
+
+            auto const accumulatedError =
+                oldLoanProperties.totalValueOutstanding -
+                totalValueOutstandingProxy;
+
+            totalParts += detail::doPayment(
+                overpaymentComponents,
+                totalValueOutstandingProxy,
+                principalOutstandingProxy,
+                referencePrincipalProxy,
+                paymentRemainingProxy,
+                prevPaymentDateProxy,
+                nextDueDateProxy,
+                paymentInterval);
+
+            auto const newLoanProperties = computeLoanProperties(
+                asset,
+                principalOutstandingProxy,
+                referencePrincipalProxy,
+                interestRate,
+                paymentInterval,
+                paymentRemainingProxy,
+                managementFeeRate);
+
+            if (newLoanProperties.firstPaymentPrincipal <= 0 &&
+                *principalOutstandingProxy > 0)
             {
-                overpaymentInterestPortion = interestPortion;
+                // The overpayment has caused the loan to be in a state where
-                totalPrincipalPaid += remainder;
+                // no further principal can be paid.
-                totalInterestPaid += interestPortion;
+                JLOG(j.warn())
-                totalfeeToPay += feePortion;
+                    << "Loan overpayment would cause loan to be stuck. "
-
+                       "Rejecting overpayment.";
-                principalOutstandingProxy -= remainder;
+                return Unexpected(tecLIMIT_EXCEEDED);
             }
+            // Check that the other computed values are valid
+            if (newLoanProperties.interestOwedToVault < 0 ||
+                newLoanProperties.totalValueOutstanding <= 0 ||
+                newLoanProperties.periodicPayment <= 0)
+            {
+                // LCOV_EXCL_START
+                JLOG(j.warn()) << "Computed loan properties are invalid. Does "
+                                  "not compute. TotalValueOutstanding: "
+                               << newLoanProperties.totalValueOutstanding
+                               << ", PeriodicPayment: "
+                               << newLoanProperties.periodicPayment
+                               << ", InterestOwedToVault: "
+                               << newLoanProperties.interestOwedToVault;
+                return Unexpected(tecINTERNAL);
+                // LCOV_EXCL_STOP
+            }
+
+            totalValueOutstandingProxy =
+                newLoanProperties.totalValueOutstanding;
+            loan->at(sfPeriodicPayment) = newLoanProperties.periodicPayment;
+            loan->at(sfInterestOwed) = newLoanProperties.interestOwedToVault;
+
+            auto const totalInterestOutstandingAfter =
+                totalValueOutstandingProxy - principalOutstandingProxy;
+
+            totalParts.valueChange += totalInterestOutstandingBefore -
+                totalInterestOutstandingAfter +
+                overpaymentComponents.roundedInterest + accumulatedError;
         }
     }
 
-    totalParts.valueChange += (newTotalInterest - totalInterestOutstanding) +
-        overpaymentInterestPortion;
-
     // Check the final results are rounded, to double-check that the
     // intermediate steps were rounded.
     XRPL_ASSERT(
-        roundToAsset(asset, totalPrincipalPaid, loanScale) ==
+        isRounded(asset, totalParts.principalPaid, loanScale),
-            totalPrincipalPaid,
+        "ripple::loanMakePayment : total principal paid rounded");
-        "ripple::loanMakePayment : totalPrincipalPaid rounded");
     XRPL_ASSERT(
-        roundToAsset(asset, totalInterestPaid, loanScale) == totalInterestPaid,
+        isRounded(asset, totalParts.interestPaid, loanScale),
-        "ripple::loanMakePayment : totalInterestPaid rounded");
+        "ripple::loanMakePayment : total interest paid rounded");
     XRPL_ASSERT(
-        roundToAsset(asset, loanValueChange, loanScale) == loanValueChange,
+        isRounded(asset, totalParts.valueChange, loanScale),
-        "ripple::loanMakePayment : loanValueChange rounded");
+        "ripple::loanMakePayment : loan value change rounded");
     XRPL_ASSERT(
-        roundToAsset(asset, totalfeeToPay, loanScale) == totalfeeToPay,
+        isRounded(asset, totalParts.feeToPay, loanScale),
-        "ripple::loanMakePayment : totalfeeToPay rounded");
+        "ripple::loanMakePayment : total fee to pay rounded");
-    return LoanPaymentParts{
+    return totalParts;
-        .principalPaid = totalPrincipalPaid,
-        .interestPaid = totalInterestPaid,
-        .valueChange = loanValueChange,
-        .feeToPay = totalfeeToPay};
-#endif
 }
 
 }  // namespace ripple

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

@@ -234,8 +234,10 @@ LoanPay::doApply()
         LoanManage::unimpairLoan(view, loanSle, vaultSle, j_);
     }
 
+    TenthBips16 managementFeeRate{brokerSle->at(sfManagementFeeRate)};
+
     Expected<LoanPaymentParts, TER> paymentParts =
-        loanMakePayment(asset, view, loanSle, amount, j_);
+        loanMakePayment(asset, view, loanSle, amount, managementFeeRate, j_);
 
     if (!paymentParts)
         return paymentParts.error();
@@ -245,11 +247,12 @@ LoanPay::doApply()
     view.update(loanSle);
 
     XRPL_ASSERT_PARTS(
-        // It is possible to pay 0 interest
+        // It is possible to pay 0 principal
         paymentParts->principalPaid >= 0,
         "ripple::LoanPay::doApply",
         "valid principal paid");
     XRPL_ASSERT_PARTS(
+        // It is possible to pay 0 interest
         paymentParts->interestPaid >= 0,
         "ripple::LoanPay::doApply",
         "valid interest paid");
@@ -272,7 +275,6 @@ LoanPay::doApply()
     // LoanBroker object state changes
     view.update(brokerSle);
 
-    TenthBips32 managementFeeRate{brokerSle->at(sfManagementFeeRate)};
     auto interestOwedProxy = loanSle->at(sfInterestOwed);
 
     auto const [managementFee, interestPaidToVault] = [&]() {
@@ -283,7 +285,7 @@ LoanPay::doApply()
         auto const interest = paymentParts->interestPaid - managementFee;
         auto const owed = *interestOwedProxy;
         if (interest > owed)
-            return std::make_pair(interest - owed, owed);
+            return std::make_pair(paymentParts->interestPaid - owed, owed);
         return std::make_pair(managementFee, interest);
     }();
     XRPL_ASSERT_PARTS(