Merge branch 'ximinez/lending-XLS-66-ongoing' into tapanito/lending-check-error

- Originally defined as uint64_t, but the testIssuerLoan() test called
  it with a negative number, causing an overflow to a very large number
  that in some circumstances could be silently cast back to an int64_t,
  but might not be. I believe this is UB, and we don't want to rely on
  that.

src/test/app/Loan_test.cpp

@@ -2728,12 +2728,8 @@ protected:
                             broker.params.managementFeeRate);
 
                     BEAST_EXPECT(
-                        paymentComponents.trackedValueDelta ==
-                            roundedPeriodicPayment ||
-                        (paymentComponents.specialCase ==
-                             detail::PaymentSpecialCase::final &&
-                         paymentComponents.trackedValueDelta <
-                             roundedPeriodicPayment));
+                        paymentComponents.trackedValueDelta <=
+                        roundedPeriodicPayment);
 
                     ripple::LoanState const nextTrueState = computeRawLoanState(
                         state.periodicPayment,

src/test/rpc/Feature_test.cpp

@@ -187,9 +187,9 @@ class Feature_test : public beast::unit_test::suite
         BEAST_EXPECTS(jrr[jss::status] == jss::success, "status");
         jrr.removeMember(jss::status);
         BEAST_EXPECT(jrr.size() == 1);
-        BEAST_EXPECT(jrr.isMember(
-            "740352F2412A9909880C23A559FCECEDA3BE2126FED62FC7660D6"
-            "28A06927F11"));
+        BEAST_EXPECT(
+            jrr.isMember("12523DF04B553A0B1AD74F42DDB741DE8DC06A03FC089A0EF197E"
+                         "2A87F1D8107"));
         auto feature = *(jrr.begin());
 
         BEAST_EXPECTS(feature[jss::name] == "fixAMMOverflowOffer", "name");

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

@@ -1095,23 +1095,6 @@ 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");
-        };
     // Helper to reduce deltas when they collectively exceed a limit.
     // Order matters: we prefer to reduce interest first (most flexible),
     // then management fee, then principal (least flexible).
@@ -1121,19 +1104,6 @@ computePaymentComponents(
         takeFrom(deltas.managementFee, excess);
         takeFrom(deltas.principal, excess);
     };
-    // Helper to increase deltas when they collectively do not reach an
-    // expected value.
-    // Order matters: we prefer to increase interest first (most flexible),
-    // then management fee, then principal (least flexible).
-    auto addressShortage = [&giveTo](
-                               LoanDeltas& deltas,
-                               Number& shortage,
-                               LoanState const& current) {
-        giveTo(deltas.interestDueDelta, shortage, current.interestDue);
-        giveTo(
-            deltas.managementFeeDueDelta, shortage, current.managementFeeDue);
-        giveTo(deltas.principalDelta, shortage, current.principalOutstanding);
-    };
 
     // Check if deltas exceed the total outstanding value. This should never
     // happen due to earlier caps, but handle it defensively.
@@ -1165,19 +1135,12 @@ computePaymentComponents(
         addressExcess(deltas, excess);
         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);
-    }
 
     // At this point, shortage >= 0 means we're paying less than the full
     // periodic payment (due to rounding or component caps).
-    // shortage < 0 means mean we're trying to pay more than allowed (bug).
+    // shortage < 0 would mean we're trying to pay more than allowed (bug).
     XRPL_ASSERT_PARTS(
-        shortage == beast::zero,
+        shortage >= beast::zero,
         "ripple::detail::computePaymentComponents",
         "no shortage or excess");
 

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

@@ -305,7 +305,13 @@ LoanPay::doApply()
     // change will be discarded.
     if (loanSle->isFlag(lsfLoanImpaired))
     {
-        LoanManage::unimpairLoan(view, loanSle, vaultSle, j_);
+        if (auto const ret =
+                LoanManage::unimpairLoan(view, loanSle, vaultSle, j_);
+            ret != tesSUCCESS)
+        {
+            JLOG(j_.fatal()) << "Failed to unimpair loan before payment.";
+            return ret;  // LCOV_EXCL_LINE
+        }
     }
 
     LoanPaymentType const paymentType = [&tx]() {