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

@@ -1271,7 +1271,8 @@ protected:
             verifyLoanStatus,
             issuer,
             lender,
-            borrower);
+            borrower,
+            PaymentParameters{.showStepBalances = true});
     }
 
     /** Runs through the complete lifecycle of a loan
@@ -7193,15 +7194,15 @@ class LoanArbitrary_test : public LoanBatch_test
             .vaultDeposit = 10000,
             .debtMax = 0,
             .coverRateMin = TenthBips32{0},
-            // .managementFeeRate = TenthBips16{5919},
+            .managementFeeRate = TenthBips16{0},
             .coverRateLiquidation = TenthBips32{0}};
         LoanParameters const loanParams{
             .account = Account("lender"),
             .counter = Account("borrower"),
-            .principalRequest = Number{10000, 0},
-            // .interest = TenthBips32{0},
-            // .payTotal = 5816,
-            .payInterval = 150};
+            .principalRequest = Number{200000, -6},
+            .interest = TenthBips32{50000},
+            .payTotal = 2,
+            .payInterval = 200};
 
         runLoan(AssetType::XRP, brokerParams, loanParams);
     }

src/test/jtx/impl/mpt.cpp

@@ -644,7 +644,7 @@ MPTTester::operator[](std::string const& name) const
 }
 
 PrettyAmount
-MPTTester::operator()(std::uint64_t amount) const
+MPTTester::operator()(std::int64_t amount) const
 {
     return MPT("", issuanceID())(amount);
 }

src/test/jtx/mpt.h

@@ -272,7 +272,7 @@ public:
     operator[](std::string const& name) const;
 
     PrettyAmount
-    operator()(std::uint64_t amount) const;
+    operator()(std::int64_t amount) const;
 
     operator Asset() const;
 

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

@@ -547,6 +547,14 @@ tryOverpayment(
 
     auto const deltas = rounded - newRounded;
 
+    // The change in loan management fee is equal to the change between the old
+    // and the new outstanding management fees
+    XRPL_ASSERT_PARTS(
+        deltas.managementFee ==
+            rounded.managementFeeDue - managementFeeOutstanding,
+        "ripple::detail::tryOverpayment",
+        "no fee change");
+
     auto const hypotheticalValueOutstanding =
         rounded.valueOutstanding - deltas.principal;
 
@@ -561,7 +569,6 @@ tryOverpayment(
                           "the loan. Ignore the overpayment";
         return Unexpected(tesSUCCESS);
     }
-
     return LoanPaymentParts{
         // Principal paid is the reduction in principal outstanding
         .principalPaid = deltas.principal,
@@ -676,12 +683,6 @@ doOverpayment(
         "ripple::detail::doOverpayment",
         "principal change agrees");
 
-    XRPL_ASSERT_PARTS(
-        overpaymentComponents.trackedManagementFeeDelta ==
-            managementFeeOutstandingProxy - managementFeeOutstanding,
-        "ripple::detail::doOverpayment",
-        "no fee change");
-
     // I'm not 100% sure the following asserts are correct. If in doubt, and
     // everything else works, remove any that cause trouble.
 
@@ -712,13 +713,6 @@ doOverpayment(
         "ripple::detail::doOverpayment",
         "principal payment matches");
 
-    XRPL_ASSERT_PARTS(
-        loanPaymentParts.feePaid ==
-            overpaymentComponents.untrackedManagementFee +
-                overpaymentComponents.trackedManagementFeeDelta,
-        "ripple::detail::doOverpayment",
-        "fee payment matches");
-
     // All validations passed, so update the proxy objects (which will
     // modify the actual Loan ledger object)
     totalValueOutstandingProxy = totalValueOutstanding;

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]() {