From 810ffde484d373f65e8200b9dd6708558f7744d2 Mon Sep 17 00:00:00 2001 From: Vito <5780819+Tapanito@users.noreply.github.com> Date: Tue, 17 Mar 2026 16:21:38 +0100 Subject: [PATCH] fix: Prevent early loan impairment and due-date manipulation Stop impairLoan and unimpairLoan from rewriting sfNextPaymentDueDate when the amendment is active. Previously a colluding broker could repeatedly impair and unimpair an overdue loan to keep pushing the due date forward, permanently blocking default eligibility and suppressing late-interest / late-fee accrual. --- .../tx/transactors/lending/LendingHelpers.h | 4 + .../tx/transactors/lending/LendingHelpers.cpp | 17 +- .../tx/transactors/lending/LoanManage.cpp | 51 +-- src/test/app/Loan_test.cpp | 308 +++++++++++++++++- 4 files changed, 338 insertions(+), 42 deletions(-) diff --git a/include/xrpl/tx/transactors/lending/LendingHelpers.h b/include/xrpl/tx/transactors/lending/LendingHelpers.h index 8dd6866ac34..963660d6114 100644 --- a/include/xrpl/tx/transactors/lending/LendingHelpers.h +++ b/include/xrpl/tx/transactors/lending/LendingHelpers.h @@ -219,6 +219,10 @@ computeFullPaymentInterest( std::uint32_t startDate, TenthBips32 closeInterestRate); +/// Returns true if the loan's next payment due date has passed. +[[nodiscard]] bool +isPaymentLate(ReadView const& view, SLE::const_ref loanSle); + namespace detail { // These classes and functions should only be accessed by LendingHelper // functions and unit tests diff --git a/src/libxrpl/tx/transactors/lending/LendingHelpers.cpp b/src/libxrpl/tx/transactors/lending/LendingHelpers.cpp index ad4cd8440d1..0a553588eab 100644 --- a/src/libxrpl/tx/transactors/lending/LendingHelpers.cpp +++ b/src/libxrpl/tx/transactors/lending/LendingHelpers.cpp @@ -64,6 +64,12 @@ isRounded(Asset const& asset, Number const& value, std::int32_t scale) roundToAsset(asset, value, scale, Number::upward); } +[[nodiscard]] bool +isPaymentLate(ReadView const& view, SLE::const_ref loanSle) +{ + return hasExpired(view, loanSle->at(sfNextPaymentDueDate)); +} + namespace detail { void @@ -675,11 +681,6 @@ computeLatePayment( TenthBips16 managementFeeRate, beast::Journal j) { - // Check if the due date has passed. If not, reject the payment as - // being too soon - if (!hasExpired(view, nextDueDate)) - return Unexpected(tecTOO_SOON); - // Calculate the penalty interest based on how long the payment is overdue. auto const latePaymentInterest = loanLatePaymentInterest( principalOutstanding, lateInterestRate, view.parentCloseTime(), nextDueDate); @@ -1614,7 +1615,7 @@ loanMakePayment( // ------------------------------------------------------------- // A late payment not flagged as late overrides all other options. - if (paymentType != LoanPaymentType::late && hasExpired(view, nextDueDateProxy)) + if (paymentType != LoanPaymentType::late && isPaymentLate(view, loan)) { // If the payment is late, and the late flag was not set, it's not // valid @@ -1708,6 +1709,10 @@ loanMakePayment( // late payment handling if (paymentType == LoanPaymentType::late) { + // Check if the due date has passed. If not, reject the payment as being too soon + if (!isPaymentLate(view, loan)) + return Unexpected(tecTOO_SOON); + TenthBips32 const lateInterestRate{loan->at(sfLateInterestRate)}; Number const latePaymentFee = loan->at(sfLatePaymentFee); diff --git a/src/libxrpl/tx/transactors/lending/LoanManage.cpp b/src/libxrpl/tx/transactors/lending/LoanManage.cpp index 6cc9df1aabc..30acccfe5b7 100644 --- a/src/libxrpl/tx/transactors/lending/LoanManage.cpp +++ b/src/libxrpl/tx/transactors/lending/LoanManage.cpp @@ -282,6 +282,12 @@ LoanManage::impairLoan( Asset const& vaultAsset, beast::Journal j) { + if (view.rules().enabled(featureLendingProtocolV1_1) && !isPaymentLate(view, loanSle)) + { + JLOG(j.warn()) << "Cannot impair a loan that is not late"; + return tecTOO_SOON; + } + Number const lossUnrealized = owedToVault(loanSle); // The vault may be at a different scale than the loan. Reduce rounding @@ -296,20 +302,22 @@ LoanManage::impairLoan( { // Having a loss greater than the vault's unavailable assets // will leave the vault in an invalid / inconsistent state. - JLOG(j.warn()) << "Vault unrealized loss is too large, and will " - "corrupt the vault."; + JLOG(j.warn()) << "Vault unrealized loss is too large, and will corrupt the vault."; return tecLIMIT_EXCEEDED; } view.update(vaultSle); // Update the Loan object loanSle->setFlag(lsfLoanImpaired); - auto loanNextDueProxy = loanSle->at(sfNextPaymentDueDate); - if (!hasExpired(view, loanNextDueProxy)) + + if (!view.rules().enabled(featureLendingProtocolV1_1)) { - // loan payment is not yet late - - // move the next payment due date to now - loanNextDueProxy = view.parentCloseTime().time_since_epoch().count(); + auto loanNextDueProxy = loanSle->at(sfNextPaymentDueDate); + if (!isPaymentLate(view, loanSle)) + { + // loan payment is not yet late move the next payment due date to now + loanNextDueProxy = view.parentCloseTime().time_since_epoch().count(); + } } view.update(loanSle); @@ -346,19 +354,24 @@ LoanManage::unimpairLoan( // Update the Loan object loanSle->clearFlag(lsfLoanImpaired); - auto const paymentInterval = loanSle->at(sfPaymentInterval); - auto const normalPaymentDueDate = - std::max(loanSle->at(sfPreviousPaymentDueDate), loanSle->at(sfStartDate)) + paymentInterval; - if (!hasExpired(view, normalPaymentDueDate)) + if (!view.rules().enabled(featureLendingProtocolV1_1)) { - // loan was unimpaired within the payment interval - loanSle->at(sfNextPaymentDueDate) = normalPaymentDueDate; - } - else - { - // loan was unimpaired after the original payment due date - loanSle->at(sfNextPaymentDueDate) = - view.parentCloseTime().time_since_epoch().count() + paymentInterval; + auto const paymentInterval = loanSle->at(sfPaymentInterval); + auto const normalPaymentDueDate = + std::max(loanSle->at(sfPreviousPaymentDueDate), loanSle->at(sfStartDate)) + + paymentInterval; + + if (!hasExpired(view, normalPaymentDueDate)) + { + // loan was unimpaired within the payment interval + loanSle->at(sfNextPaymentDueDate) = normalPaymentDueDate; + } + else + { + // loan was unimpaired after the original payment due date + loanSle->at(sfNextPaymentDueDate) = + view.parentCloseTime().time_since_epoch().count() + paymentInterval; + } } view.update(loanSle); diff --git a/src/test/app/Loan_test.cpp b/src/test/app/Loan_test.cpp index 4c25f9ead91..4d1adf481fc 100644 --- a/src/test/app/Loan_test.cpp +++ b/src/test/app/Loan_test.cpp @@ -1385,17 +1385,27 @@ class Loan_test : public beast::unit_test::suite // due env(manage(lender, keylet.key, tfLoanDefault), ter(tecTOO_SOON)); - // Check the vault - bool const canImpair = canImpairLoan(env, broker, state); - // Impair the loan, if possible - env(manage(lender, keylet.key, tfLoanImpair), - canImpair ? ter(tesSUCCESS) : ter(tecLIMIT_EXCEEDED)); - // Unimpair the loan - env(manage(lender, keylet.key, tfLoanUnimpair), - canImpair ? ter(tesSUCCESS) : ter(tecNO_PERMISSION)); - auto const nextDueDate = startDate + *loanParams.payInterval; + if (env.enabled(featureLendingProtocolV1_1)) + { + // With the amendment, impairment is only allowed when the + // payment is late. Impair/unimpair cycle is tested in the + // toEndOfLife callbacks. + env(manage(lender, keylet.key, tfLoanImpair), ter(tecTOO_SOON)); + } + else + { + // Check the vault + bool const canImpair = canImpairLoan(env, broker, state); + // Impair the loan, if possible + env(manage(lender, keylet.key, tfLoanImpair), + canImpair ? ter(tesSUCCESS) : ter(tecLIMIT_EXCEEDED)); + // Unimpair the loan + env(manage(lender, keylet.key, tfLoanUnimpair), + canImpair ? ter(tesSUCCESS) : ter(tecNO_PERMISSION)); + } + env.close(); verifyLoanStatus( @@ -1986,6 +1996,19 @@ class Loan_test : public beast::unit_test::suite if (impair) { + auto const paymentDue = tp{d{state.nextPaymentDate}}; + bool const alreadyLate = env.now() > paymentDue; + + if (env.enabled(featureLendingProtocolV1_1) && !alreadyLate) + { + // With the amendment, impairment requires the + // payment to be late + env(manage(lender, loanKeylet.key, tfLoanImpair), ter(tecTOO_SOON)); + + // Advance time past the payment due date + env.close(paymentDue + 1s); + } + // Check the vault bool const canImpair = canImpairLoan(env, broker, state); // Impair the loan, if possible @@ -1995,7 +2018,12 @@ class Loan_test : public beast::unit_test::suite if (canImpair) { state.flags |= tfLoanImpair; - state.nextPaymentDate = env.now().time_since_epoch().count(); + if (!env.enabled(featureLendingProtocolV1_1)) + { + // Without the amendment, impairment moves the + // due date to now + state.nextPaymentDate = env.now().time_since_epoch().count(); + } // Once the loan is impaired, it can't be impaired again env(manage(lender, loanKeylet.key, tfLoanImpair), ter(tecNO_PERMISSION)); @@ -2004,14 +2032,27 @@ class Loan_test : public beast::unit_test::suite } auto const nextDueDate = tp{d{state.nextPaymentDate}}; + auto const gracePeriod = [&]() { + auto const loanSle = env.le(loanKeylet); + return loanSle ? loanSle->at(sfGracePeriod) : 60; + }(); + auto const defaultableTime = nextDueDate + std::chrono::seconds{gracePeriod}; - // Can't default the loan yet. The grace period hasn't - // expired - env(manage(lender, loanKeylet.key, tfLoanDefault), ter(tecTOO_SOON)); + if (env.now() <= defaultableTime) + { + // Can't default the loan yet. The grace period hasn't + // expired + env(manage(lender, loanKeylet.key, tfLoanDefault), ter(tecTOO_SOON)); - // Let some time pass so that the loan can be - // defaulted - env.close(nextDueDate + 60s); + // Let some time pass so that the loan can be + // defaulted + env.close(defaultableTime); + } + else + { + // Grace period already expired + env.close(); + } auto const [amountToBeCovered, brokerAcct] = getDefaultInfo(state, broker); @@ -2587,7 +2628,10 @@ class Loan_test : public beast::unit_test::suite auto const borrowerBalanceBeforePayment = env.balance(borrower, broker.asset); - if (canImpairLoan(env, broker, state)) + // With the amendment, on-time payments can't be + // preceded by impairment (payment is not late) + if (!env.enabled(featureLendingProtocolV1_1) && + canImpairLoan(env, broker, state)) { // Making a payment will unimpair the loan env(manage(lender, loanKeylet.key, tfLoanImpair)); @@ -6983,6 +7027,225 @@ class Loan_test : public beast::unit_test::suite BEAST_EXPECT(afterSecondCoverAvailable == 0); } + // Verify that with featureLendingProtocolV1_1: + // 1. A loan cannot be impaired before its payment is late. + // 2. Impairing a late loan does not change sfNextPaymentDueDate. + // 3. The unimpair operation does not change sfNextPaymentDueDate. + void + testImpairmentPaymentDateUnchanged() + { + using namespace jtx; + using namespace loan; + using namespace std::chrono_literals; + + testcase("Impairment does not change payment due date"); + + Env env(*this, all); + BEAST_EXPECT(env.enabled(featureLendingProtocolV1_1)); + + Account const lender{"lender"}; + Account const borrower{"borrower"}; + + env.fund(XRP(1'000'000), lender, borrower); + env.close(); + + auto const broker = createVaultAndBroker(env, xrpIssue(), lender); + + // Create a loan + auto const sleBroker = env.le(keylet::loanbroker(broker.brokerID)); + if (!BEAST_EXPECT(sleBroker)) + return; + auto const loanKeylet = keylet::loan(broker.brokerID, sleBroker->at(sfLoanSequence)); + + env(set(borrower, broker.brokerID, Number(1000)), + sig(sfCounterpartySignature, lender), + paymentTotal(12), + paymentInterval(600), + fee(env.current()->fees().base * 2)); + env.close(); + + auto const loanSle = env.le(loanKeylet); + if (!BEAST_EXPECT(loanSle)) + return; + auto const originalNextDueDate = loanSle->at(sfNextPaymentDueDate); + BEAST_EXPECT(originalNextDueDate > 0); + + // 1. Impairment must fail when payment is not yet late + env(manage(lender, loanKeylet.key, tfLoanImpair), ter(tecTOO_SOON)); + + // Verify the due date was not changed by the failed impairment + { + auto const loan = env.le(loanKeylet); + BEAST_EXPECT(loan->at(sfNextPaymentDueDate) == originalNextDueDate); + } + + // Advance time past the payment due date + env.close(NetClock::time_point{NetClock::duration{originalNextDueDate}} + 1s); + + // 2. Impairment succeeds when payment is late + env(manage(lender, loanKeylet.key, tfLoanImpair), ter(tesSUCCESS)); + + // Verify sfNextPaymentDueDate is unchanged after impairment + { + auto const loan = env.le(loanKeylet); + if (!BEAST_EXPECT(loan)) + return; + BEAST_EXPECT(loan->isFlag(lsfLoanImpaired)); + BEAST_EXPECT(loan->at(sfNextPaymentDueDate) == originalNextDueDate); + } + + // 3. Unimpair also does not change sfNextPaymentDueDate + env(manage(lender, loanKeylet.key, tfLoanUnimpair), ter(tesSUCCESS)); + + { + auto const loan = env.le(loanKeylet); + if (!BEAST_EXPECT(loan)) + return; + BEAST_EXPECT(!loan->isFlag(lsfLoanImpaired)); + BEAST_EXPECT(loan->at(sfNextPaymentDueDate) == originalNextDueDate); + } + } + + // Verify that without featureLendingProtocolV1_1, the pre-amendment + // impair/unimpair behaviour is preserved: + // 1. Impairing a loan before its payment is late moves + // sfNextPaymentDueDate to "now". + // 2a. Unimpair within the original payment interval restores + // sfNextPaymentDueDate to StartDate + PaymentInterval. + // 2b. Unimpair after the original due date sets + // sfNextPaymentDueDate to now + PaymentInterval. + void + testImpairmentPaymentDatePreAmendment() + { + using namespace jtx; + using namespace loan; + using namespace std::chrono_literals; + + testcase("Pre-amendment impair/unimpair date restoration"); + + Env env(*this, all - featureLendingProtocolV1_1); + BEAST_EXPECT(!env.enabled(featureLendingProtocolV1_1)); + + Account const lender{"lender"}; + Account const borrower{"borrower"}; + + env.fund(XRP(1'000'000), lender, borrower); + env.close(); + + auto const broker = createVaultAndBroker(env, xrpIssue(), lender); + + // Helper to create a loan and return its keylet + auto createLoan = [&]() { + auto const sleBroker = env.le(keylet::loanbroker(broker.brokerID)); + BEAST_EXPECT(sleBroker); + auto const lk = keylet::loan(broker.brokerID, sleBroker->at(sfLoanSequence)); + env(set(borrower, broker.brokerID, Number(1000)), + sig(sfCounterpartySignature, lender), + paymentTotal(12), + paymentInterval(600), + fee(env.current()->fees().base * 2)); + env.close(); + return lk; + }; + + // Helper to default + delete a loan and replenish first-loss capital + // so the broker is ready for the next loan. + auto cleanupLoan = [&](Keylet const& loanKeylet, std::uint32_t dueDate) { + env.close(NetClock::time_point{NetClock::duration{dueDate + 60}} + 1s); + env(manage(lender, loanKeylet.key, tfLoanDefault), ter(tesSUCCESS)); + env.close(); + + auto const brokerSle = env.le(keylet::loanbroker(broker.brokerID)); + if (!BEAST_EXPECT(brokerSle)) + return; + auto const coverNeeded = + broker.asset(broker.params.coverDeposit).value() - brokerSle->at(sfCoverAvailable); + if (coverNeeded > beast::zero) + { + env(loanBroker::coverDeposit( + lender, broker.brokerID, STAmount{broker.asset, coverNeeded})); + env.close(); + } + env(del(lender, loanKeylet.key)); + env.close(); + }; + + // ---- Case A: impair before late, unimpair within original interval ---- + { + auto const loanKeylet = createLoan(); + auto const loanSle = env.le(loanKeylet); + if (!BEAST_EXPECT(loanSle)) + return; + auto const startDate = loanSle->at(sfStartDate); + auto const originalNextDueDate = loanSle->at(sfNextPaymentDueDate); + BEAST_EXPECT(originalNextDueDate == startDate + 600); + + // Payment is not late yet — impair succeeds and moves due date + // to now (pre-amendment allows immediate impairment) + env(manage(lender, loanKeylet.key, tfLoanImpair), ter(tesSUCCESS)); + + { + auto const loan = env.le(loanKeylet); + if (!BEAST_EXPECT(loan)) + return; + BEAST_EXPECT(loan->isFlag(lsfLoanImpaired)); + // Due date was moved to parentCloseTime + auto const movedDueDate = loan->at(sfNextPaymentDueDate); + BEAST_EXPECT(movedDueDate != originalNextDueDate); + BEAST_EXPECT(movedDueDate < originalNextDueDate); + } + + // Unimpair while still within the original payment interval. + // The normal due date (startDate + 600) has not yet expired, + // so it should be restored. + env(manage(lender, loanKeylet.key, tfLoanUnimpair), ter(tesSUCCESS)); + + { + auto const loan = env.le(loanKeylet); + if (!BEAST_EXPECT(loan)) + return; + BEAST_EXPECT(!loan->isFlag(lsfLoanImpaired)); + BEAST_EXPECT(loan->at(sfNextPaymentDueDate) == originalNextDueDate); + } + + cleanupLoan(loanKeylet, originalNextDueDate); + } + + // ---- Case B: impair before late, unimpair after original due date ---- + { + auto const loanKeylet = createLoan(); + auto const loanSle = env.le(loanKeylet); + if (!BEAST_EXPECT(loanSle)) + return; + auto const startDate = loanSle->at(sfStartDate); + auto const originalNextDueDate = loanSle->at(sfNextPaymentDueDate); + BEAST_EXPECT(originalNextDueDate == startDate + 600); + + // Payment is not late yet — impair moves due date to now + env(manage(lender, loanKeylet.key, tfLoanImpair), ter(tesSUCCESS)); + + // Advance time past the original due date before unimpair + env.close(NetClock::time_point{NetClock::duration{originalNextDueDate}} + 10s); + + auto const timeBeforeUnimpair = + env.current()->header().parentCloseTime.time_since_epoch().count(); + + env(manage(lender, loanKeylet.key, tfLoanUnimpair), ter(tesSUCCESS)); + + { + auto const loan = env.le(loanKeylet); + if (!BEAST_EXPECT(loan)) + return; + BEAST_EXPECT(!loan->isFlag(lsfLoanImpaired)); + // Unimpaired after the original due date, so the new due + // date should be approximately now + paymentInterval + auto const newDueDate = loan->at(sfNextPaymentDueDate); + BEAST_EXPECT(newDueDate > originalNextDueDate); + BEAST_EXPECT(newDueDate == timeBeforeUnimpair + 600); + } + } + } + // Tests that vault withdrawals work correctly when the vault has unrealized // loss from an impaired loan, ensuring the invariant check properly // accounts for the loss. @@ -7066,6 +7329,15 @@ class Loan_test : public beast::unit_test::suite ter(tesSUCCESS)); env.close(); + // Advance time past the payment due date so the loan can be impaired + { + auto const loanSle = env.le(loanKeylet); + if (!BEAST_EXPECT(loanSle)) + return; + auto const nextDueDate = loanSle->at(sfNextPaymentDueDate); + env.close(NetClock::time_point{NetClock::duration{nextDueDate}} + 1s); + } + // Impair the loan to create unrealized loss env(manage(lender, loanKeylet.key, tfLoanImpair), ter(tesSUCCESS)); env.close(); @@ -7121,6 +7393,8 @@ class Loan_test : public beast::unit_test::suite testLoanPayLateFullPaymentBypassesPenalties(); testLoanCoverMinimumRoundingExploit(); #endif + testImpairmentPaymentDateUnchanged(); + testImpairmentPaymentDatePreAmendment(); testWithdrawReflectsUnrealizedLoss(); testInvalidLoanSet();