From b8b9e7a51ed7c6f34e8d510667bd86554553f6e8 Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Sun, 26 Oct 2025 15:05:51 -0400 Subject: [PATCH] Add repro unit tests, with modifications, from audit - FIND-005, FIND-009, and FIND-010. - Add the finding number to existing tests - FIND-001, FIND-003, FIND-012, FIND-007. - Tweak the interest rate failure log messages in LoanSet. --- src/test/app/Loan_test.cpp | 280 +++++++++++++++++++++++++++- src/xrpld/app/tx/detail/LoanSet.cpp | 15 +- 2 files changed, 284 insertions(+), 11 deletions(-) diff --git a/src/test/app/Loan_test.cpp b/src/test/app/Loan_test.cpp index e69c0e22ce..8abc53ebca 100644 --- a/src/test/app/Loan_test.cpp +++ b/src/test/app/Loan_test.cpp @@ -2832,6 +2832,7 @@ class Loan_test : public beast::unit_test::suite void testBatchBypassCounterparty() { + // From FIND-001 testcase << "Batch Bypass Counterparty"; using namespace jtx; @@ -2880,7 +2881,7 @@ class Loan_test : public beast::unit_test::suite ter(temBAD_SIGNATURE)); env.close(); - // ? Check that the loan was created + // ? Check that the loan was NOT created { Json::Value params(Json::objectValue); params[jss::account] = borrower.human(); @@ -2904,6 +2905,7 @@ class Loan_test : public beast::unit_test::suite void testWrongMaxDebtBehavior() { + // From FIND-003 testcase << "Wrong Max Debt Behavior"; using namespace jtx; @@ -2950,6 +2952,7 @@ class Loan_test : public beast::unit_test::suite void testLoanPayComputePeriodicPaymentValidRateInvariant() { + // From FIND-012 testcase << "LoanPay ripple::detail::computePeriodicPayment : " "valid rate"; @@ -3775,6 +3778,7 @@ class Loan_test : public beast::unit_test::suite void testLoanPayDebtDecreaseInvariant() { + // From FIND-007 testcase << "LoanPay ripple::LoanPay::doApply : debtDecrease rounding good"; @@ -3876,6 +3880,271 @@ class Loan_test : public beast::unit_test::suite originalState.loanScale)); } + void + testLoanPayComputePeriodicPaymentValidTotalInterestInvariant() + { + // From FIND-010 + testcase << "ripple::loanComputePaymentParts : valid total interest"; + + using namespace jtx; + using namespace std::chrono_literals; + Env env(*this, all); + + Account const issuer{"issuer"}; + Account const lender{"lender"}; + Account const borrower{"borrower"}; + + env.fund(XRP(1'000'000), issuer, lender, borrower); + env.close(); + + PrettyAsset const iouAsset = issuer[iouCurrency]; + auto trustLenderTx = env.json(trust(lender, iouAsset(1'000'000'000))); + env(trustLenderTx); + auto trustBorrowerTx = + env.json(trust(borrower, iouAsset(1'000'000'000))); + env(trustBorrowerTx); + auto payLenderTx = pay(issuer, lender, iouAsset(100'000'000)); + env(payLenderTx); + auto payIssuerTx = pay(issuer, borrower, iouAsset(1'000'000)); + env(payIssuerTx); + env.close(); + + BrokerInfo broker{createVaultAndBroker(env, iouAsset, lender)}; + + using namespace loan; + + auto const loanSetFee = fee(env.current()->fees().base * 2); + Number const principalRequest{1, 3}; + auto const startDate = env.now() + 60s; + + auto createJson = env.json( + set(borrower, broker.brokerID, principalRequest), + fee(loanSetFee), + json(sfCounterpartySignature, Json::objectValue)); + + createJson["CloseInterestRate"] = 47299; + createJson["ClosePaymentFee"] = "3985819770"; + createJson["GracePeriod"] = 0; + createJson["InterestRate"] = 92; + createJson["LatePaymentFee"] = "3866894865"; + createJson["LoanOriginationFee"] = "0"; + createJson["LoanServiceFee"] = "2348810240"; + createJson["OverpaymentFee"] = 58545; + createJson["PaymentInterval"] = 60; + createJson["PaymentTotal"] = 1; + createJson["PrincipalRequested"] = "0.000763058"; + + auto const brokerStateBefore = + env.le(keylet::loanbroker(broker.brokerID)); + auto const loanSequence = brokerStateBefore->at(sfLoanSequence); + auto const keylet = keylet::loan(broker.brokerID, loanSequence); + + createJson = env.json(createJson, sig(sfCounterpartySignature, lender)); + env(createJson, ter(tecPRECISION_LOSS)); + env.close(startDate); + + auto loanPayTx = env.json( + pay(borrower, keylet.key, STAmount{broker.asset, Number{}})); + loanPayTx["Amount"]["value"] = "0.000281284125490196"; + env(loanPayTx, ter(tecNO_ENTRY)); + env.close(); + } + + void + testDosLoanPay() + { + // From FIND-005 + testcase << "DoS LoanPay"; + + using namespace jtx; + using namespace std::chrono_literals; + Env env(*this, all); + + Account const issuer{"issuer"}; + Account const lender{"lender"}; + Account const borrower{"borrower"}; + + env.fund(XRP(1'000'000), issuer, lender, borrower); + env.close(); + + PrettyAsset const iouAsset = issuer[iouCurrency]; + env(trust(lender, iouAsset(100'000'000))); + env(trust(borrower, iouAsset(100'000'000))); + env(pay(issuer, lender, iouAsset(10'000'000))); + env(pay(issuer, borrower, iouAsset(1'000))); + env.close(); + + BrokerInfo broker{createVaultAndBroker(env, iouAsset, lender)}; + + using namespace loan; + + auto const loanSetFee = fee(env.current()->fees().base * 2); + Number const principalRequest{1, 3}; + auto const baseFee = env.current()->fees().base; + + auto createJson = env.json( + set(borrower, broker.brokerID, principalRequest), + fee(loanSetFee), + json(sfCounterpartySignature, Json::objectValue)); + + createJson["ClosePaymentFee"] = "0"; + createJson["GracePeriod"] = 60; + createJson["InterestRate"] = 20930; + createJson["LateInterestRate"] = 77049; + createJson["LatePaymentFee"] = "0"; + createJson["LoanServiceFee"] = "0"; + createJson["OverpaymentFee"] = 7; + createJson["OverpaymentInterestRate"] = 66653; + createJson["PaymentInterval"] = 60; + createJson["PaymentTotal"] = 3239184; + createJson["PrincipalRequested"] = "3959.37"; + + auto const brokerStateBefore = + env.le(keylet::loanbroker(broker.brokerID)); + auto const loanSequence = brokerStateBefore->at(sfLoanSequence); + auto const keylet = keylet::loan(broker.brokerID, loanSequence); + + createJson = env.json(createJson, sig(sfCounterpartySignature, lender)); + env(createJson, ter(tesSUCCESS)); + env.close(); + + auto const stateBefore = getCurrentState(env, broker, keylet); + BEAST_EXPECT(stateBefore.paymentRemaining == 3239184); + BEAST_EXPECT( + stateBefore.paymentRemaining > loanMaximumPaymentsPerTransaction); + + auto loanPayTx = env.json( + pay(borrower, keylet.key, STAmount{broker.asset, Number{}})); + Number const amount{395937, -2}; + loanPayTx["Amount"]["value"] = to_string(amount); + XRPAmount const payFee{ + baseFee * + std::int64_t( + amount / stateBefore.periodicPayment / + loanPaymentsPerFeeIncrement + + 1)}; + env(loanPayTx, ter(tesSUCCESS), fee(payFee)); + env.close(); + + auto const stateAfter = getCurrentState(env, broker, keylet); + BEAST_EXPECT( + stateAfter.paymentRemaining == + stateBefore.paymentRemaining - loanMaximumPaymentsPerTransaction); + } + + void + testLoanPayComputePeriodicPaymentValidTotalPrincipalPaidInvariant() + { + // From FIND-009 + testcase + << "ripple::loanComputePaymentParts : totalPrincipalPaid rounded"; + + using namespace jtx; + using namespace std::chrono_literals; + Env env(*this, all); + + Account const issuer{"issuer"}; + Account const lender{"lender"}; + Account const borrower{"borrower"}; + + env.fund(XRP(1'000'000), issuer, lender, borrower); + env.close(); + + PrettyAsset const iouAsset = issuer[iouCurrency]; + auto trustLenderTx = env.json(trust(lender, iouAsset(1'000'000'000))); + env(trustLenderTx); + auto trustBorrowerTx = + env.json(trust(borrower, iouAsset(1'000'000'000))); + env(trustBorrowerTx); + auto payLenderTx = pay(issuer, lender, iouAsset(100'000'000)); + env(payLenderTx); + auto payIssuerTx = pay(issuer, borrower, iouAsset(1'000'000)); + env(payIssuerTx); + env.close(); + + BrokerInfo broker{createVaultAndBroker(env, iouAsset, lender)}; + + using namespace loan; + + auto const loanSetFee = fee(env.current()->fees().base * 2); + Number const principalRequest{1, 3}; + + auto createJson = env.json( + set(borrower, broker.brokerID, principalRequest), + fee(loanSetFee), + json(sfCounterpartySignature, Json::objectValue)); + + createJson["ClosePaymentFee"] = "0"; + createJson["GracePeriod"] = 0; + createJson["InterestRate"] = 24346; + createJson["LateInterestRate"] = 65535; + createJson["LatePaymentFee"] = "0"; + createJson["LoanOriginationFee"] = "218"; + createJson["LoanServiceFee"] = "0"; + createJson["PaymentInterval"] = 60; + createJson["PaymentTotal"] = 5678; + createJson["PrincipalRequested"] = "9924.81"; + + auto const brokerStateBefore = + env.le(keylet::loanbroker(broker.brokerID)); + auto const loanSequence = brokerStateBefore->at(sfLoanSequence); + auto const keylet = keylet::loan(broker.brokerID, loanSequence); + + createJson = env.json(createJson, sig(sfCounterpartySignature, lender)); + env(createJson, ter(tesSUCCESS)); + env.close(); + + auto const baseFee = env.current()->fees().base; + + auto const stateBefore = getCurrentState(env, broker, keylet); + + { + auto loanPayTx = env.json( + pay(borrower, keylet.key, STAmount{broker.asset, Number{}})); + Number const amount{3074'745'058'823'529, -12}; + BEAST_EXPECT(to_string(amount) == "3074.745058823529"); + XRPAmount const payFee{ + baseFee * + (amount / stateBefore.periodicPayment / + loanPaymentsPerFeeIncrement + + 1)}; + loanPayTx["Amount"]["value"] = to_string(amount); + env(loanPayTx, fee(payFee), ter(tesSUCCESS)); + env.close(); + } + + { + auto loanPayTx = env.json( + pay(borrower, keylet.key, STAmount{broker.asset, Number{}})); + Number const amount{6732'118'170'944'051, -12}; + BEAST_EXPECT(to_string(amount) == "6732.118170944051"); + XRPAmount const payFee{ + baseFee * + (amount / stateBefore.periodicPayment / + loanPaymentsPerFeeIncrement + + 1)}; + loanPayTx["Amount"]["value"] = to_string(amount); + env(loanPayTx, fee(payFee), ter(tesSUCCESS)); + env.close(); + } + + auto const stateAfter = getCurrentState(env, broker, keylet); + // Total interest outstanding is non-negative + BEAST_EXPECT(stateAfter.totalValue >= stateAfter.principalOutstanding); + // Principal paid is non-negative + BEAST_EXPECT( + stateBefore.principalOutstanding >= + stateAfter.principalOutstanding); + // Total value change is non-negative + BEAST_EXPECT(stateBefore.totalValue >= stateAfter.totalValue); + // Value delta is larger or same as principal delta (meaning + // non-negative interest paid) + BEAST_EXPECT( + (stateBefore.totalValue - stateAfter.totalValue) >= + (stateBefore.principalOutstanding - + stateAfter.principalOutstanding)); + } + public: void run() override @@ -3885,9 +4154,6 @@ public: testSelfLoan(); testLoanSet(); testLifecycle(); - testBatchBypassCounterparty(); - testWrongMaxDebtBehavior(); - testLoanPayComputePeriodicPaymentValidRateInvariant(); testServiceFeeOnBrokerDeepFreeze(); testRPC(); @@ -3898,8 +4164,14 @@ public: testInvalidLoanPay(); testInvalidLoanSet(); + testBatchBypassCounterparty(); + testLoanPayComputePeriodicPaymentValidRateInvariant(); testAccountSendMptMinAmountInvariant(); testLoanPayDebtDecreaseInvariant(); + testWrongMaxDebtBehavior(); + testLoanPayComputePeriodicPaymentValidTotalInterestInvariant(); + testDosLoanPay(); + testLoanPayComputePeriodicPaymentValidTotalPrincipalPaidInvariant(); } }; diff --git a/src/xrpld/app/tx/detail/LoanSet.cpp b/src/xrpld/app/tx/detail/LoanSet.cpp index 41e4b7d4ba..681031777e 100644 --- a/src/xrpld/app/tx/detail/LoanSet.cpp +++ b/src/xrpld/app/tx/detail/LoanSet.cpp @@ -362,24 +362,25 @@ LoanSet::doApply() } } + auto const totalInterestOutstanding = + properties.totalValueOutstanding - principalRequested; // Guard 1: if there is no computed total interest over the life of the loan // for a non-zero interest rate, we cannot properly amortize the loan - if (interestRate > TenthBips32{0} && - (properties.totalValueOutstanding - principalRequested) <= 0) + if (interestRate > TenthBips32{0} && totalInterestOutstanding <= 0) { // Unless this is a zero-interest loan, there must be some interest due // on the loan, even if it's (measurable) dust - JLOG(j_.warn()) << "Loan with " << interestRate - << "% interest has no interest due"; + JLOG(j_.warn()) << "Loan for " << principalRequested << " with " + << interestRate << "% interest has no interest due"; return tecPRECISION_LOSS; } // Guard 1a: If there is any interest computed over the life of the loan, // for a zero interest rate, something went sideways. - if (interestRate == TenthBips32{0} && - (properties.totalValueOutstanding - principalRequested) > 0) + if (interestRate == TenthBips32{0} && totalInterestOutstanding > 0) { // LCOV_EXCL_START - JLOG(j_.warn()) << "Loan with 0% interest has interest due"; + JLOG(j_.warn()) << "Loan for " << principalRequested + << " with 0% interest has interest due"; return tecINTERNAL; // LCOV_EXCL_STOP }