From 7f3a0cbb8a29492060e4243bbedadabb0672480d Mon Sep 17 00:00:00 2001 From: JCW Date: Mon, 13 Jul 2026 15:08:28 +0100 Subject: [PATCH] Add more invariants --- include/xrpl/tx/invariants/LoanInvariant.h | 30 +++- src/libxrpl/tx/invariants/LoanInvariant.cpp | 71 +++++++- src/test/app/Invariants_test.cpp | 181 +++++++++++++++++++- 3 files changed, 266 insertions(+), 16 deletions(-) diff --git a/include/xrpl/tx/invariants/LoanInvariant.h b/include/xrpl/tx/invariants/LoanInvariant.h index 325e5c97d0..3f7d33f13e 100644 --- a/include/xrpl/tx/invariants/LoanInvariant.h +++ b/include/xrpl/tx/invariants/LoanInvariant.h @@ -24,26 +24,40 @@ namespace xrpl { * existing loan by `LoanAccept`. * 3. A loan's `lsfLoanPending` flag may only be cleared (never set) on an * existing loan, and only by `LoanAccept`. - * 4. A `LoanSet` that creates a loan must set `lsfLoanPending` if and only if + * 4. A `LoanAccept` may only modify an existing loan when: + * a. the loan was pending (`lsfLoanPending` set); + * b. the submitting account (`Account`) is the loan's `Borrower`; + * c. the loan's `StartDate` is still in the future. + * 5. A `LoanSet` that creates a loan must use exactly one of two mutually + * exclusive creation paths: it either names a `Borrower`, or it carries a + * `Counterparty` together with a `CounterpartySignature`. Specifically: + * a. If `Borrower` is present, `Counterparty` and `CounterpartySignature` + * must be absent. + * b. If `Counterparty` and `CounterpartySignature` are present, `Borrower` + * must be absent. + * c. Either `Borrower`, or both `Counterparty` and `CounterpartySignature`, + * must be present. + * 6. A `LoanSet` that creates a loan must set `lsfLoanPending` if and only if * `Borrower` is present and `CounterpartySignature` is absent. - * 5. A pending loan (`lsfLoanPending` set) must not be linked into the + * 7. A `LoanSet` that creates a loan must set the loan's `Borrower`. + * 8. A pending loan (`lsfLoanPending` set) must not be linked into the * borrower's directory (`OwnerNode` absent), and a non-pending loan must * be linked (`OwnerNode` present). * * While the `LendingProtocolV1_1` amendment is not enabled, a `LoanSet` that * creates a loan must not use any of the two-step flow's inputs: * - * 6. It must not create a pending loan (`lsfLoanPending` must be clear). - * 7. It must not be given a `Borrower`. - * 8. It must always carry a `CounterpartySignature`. + * 9. It must not create a pending loan (`lsfLoanPending` must be clear). + * 10. It must not be given a `Borrower`. + * 11. It must always carry a `CounterpartySignature`. * * A loan may only be deleted once it is fully paid off (no payments remaining) * or, once the `LendingProtocolV1_1` amendment is enabled, while it is still * pending: * - * 9. A loan may only be deleted by a `LoanDelete` transaction. - * 10. A pending loan must not be deleted while the amendment is not enabled. - * 11. A loan that is neither fully paid off nor pending must not be deleted. + * 12. A loan may only be deleted by a `LoanDelete` transaction. + * 13. A pending loan must not be deleted while the amendment is not enabled. + * 14. A loan that is neither fully paid off nor pending must not be deleted. * */ class ValidLoan diff --git a/src/libxrpl/tx/invariants/LoanInvariant.cpp b/src/libxrpl/tx/invariants/LoanInvariant.cpp index dbaa0fd4c2..4fd0e28949 100644 --- a/src/libxrpl/tx/invariants/LoanInvariant.cpp +++ b/src/libxrpl/tx/invariants/LoanInvariant.cpp @@ -124,21 +124,80 @@ ValidLoan::finalize( "by an unauthorized transaction"; return false; } + + // LoanAccept must be submitted by the loan's Borrower, and may only + // finalise a loan whose StartDate is still in the future. + if (lendingV11Enabled && txType == ttLOAN_ACCEPT) + { + if (after->getAccountID(sfBorrower) != tx.getAccountID(sfAccount)) + { + JLOG(j.fatal()) << "Invariant failed: LoanAccept submitted " + "by an account other than the Borrower"; + return false; + } + if (after->getFieldU32(sfStartDate) <= + view.parentCloseTime().time_since_epoch().count()) + { + JLOG(j.fatal()) << "Invariant failed: LoanAccept processed a " + "Loan whose StartDate is not in the future"; + return false; + } + } } - // On creation, LoanSet chooses the loan's initial state from its - // inputs: a Borrower with no CounterpartySignature starts the two-step - // flow and must create a pending loan; any other LoanSet must create an - // active (non-pending) loan. + // On creation, LoanSet must use exactly one of two mutually exclusive + // paths: it either names a Borrower (starting the two-step flow) or + // carries a Counterparty together with its CounterpartySignature. A + // Borrower with no CounterpartySignature starts the two-step flow and + // must create a pending loan; any other LoanSet must create an active + // (non-pending) loan. if (lendingV11Enabled && !before && txType == ttLOAN_SET) { - bool const shouldPend = - tx.isFieldPresent(sfBorrower) && !tx.isFieldPresent(sfCounterpartySignature); + bool const hasBorrower = tx.isFieldPresent(sfBorrower); + bool const hasCounterparty = tx.isFieldPresent(sfCounterparty); + bool const hasCounterpartySig = tx.isFieldPresent(sfCounterpartySignature); + + // If a LoanSet carries a Counterparty and a CounterpartySignature it + // must not also name a Borrower. + if (hasCounterparty && hasCounterpartySig && hasBorrower) + { + JLOG(j.fatal()) << "Invariant failed: LoanSet specified a " + "Counterparty and CounterpartySignature " + "together with a Borrower"; + return false; + } + // If a LoanSet names a Borrower it must not also carry a + // Counterparty or a CounterpartySignature. + if (hasBorrower && (hasCounterparty || hasCounterpartySig)) + { + JLOG(j.fatal()) << "Invariant failed: LoanSet specified a " + "Borrower together with a Counterparty or " + "CounterpartySignature"; + return false; + } + // A LoanSet must use one of the two creation paths. + if (!hasBorrower && !(hasCounterparty && hasCounterpartySig)) + { + JLOG(j.fatal()) << "Invariant failed: LoanSet specified neither " + "a Borrower nor a Counterparty with " + "CounterpartySignature"; + return false; + } + + bool const shouldPend = hasBorrower && !hasCounterpartySig; if (shouldPend != after->isFlag(lsfLoanPending)) { JLOG(j.fatal()) << "Invariant failed: LoanSet pending flag does " "not match Borrower and CounterpartySignature"; return false; } + + // A created loan must always record its Borrower. + if (!after->isFieldPresent(sfBorrower)) + { + JLOG(j.fatal()) << "Invariant failed: LoanSet did not set the " + "Loan Borrower"; + return false; + } } // Without the two-step flow amendment, LoanSet must not make use of any // of its inputs: it must not create a pending loan, must not be given a diff --git a/src/test/app/Invariants_test.cpp b/src/test/app/Invariants_test.cpp index 0b433d6e8a..9d9f36456b 100644 --- a/src/test/app/Invariants_test.cpp +++ b/src/test/app/Invariants_test.cpp @@ -3942,12 +3942,20 @@ class Invariants_test : public beast::unit_test::Suite // need a loan that already exists in the base ledger, hence the same // bespoke view construction as above. { + // The loan's Borrower is the account that a valid LoanAccept must be + // submitted by; stranger stands in for any other account. + Account const borrower{"borrower"}; + Account const stranger{"stranger"}; + + // startDate defaults to the far future so the "StartDate must be in + // the future" check passes unless a test overrides it. auto const testLoanUpdate = [&, this]( STTx const& tx, std::uint32_t baseFlags, std::optional baseNode, auto&& mutate, - std::optional const& expected) { + std::optional const& expected, + std::uint32_t startDate = 0xFFFFFFFFu) { Env env{*this, defaultAmendments() | featureLendingProtocolV1_1}; Account const a1{"A1"}; Account const a2{"A2"}; @@ -3965,6 +3973,8 @@ class Invariants_test : public beast::unit_test::Suite sleLoan->at(sfManagementFeeOutstanding) = Number(0); sleLoan->at(sfPeriodicPayment) = Number(1); sleLoan->setFieldU32(sfPaymentRemaining, 2); + sleLoan->setAccountID(sfBorrower, borrower.id()); + sleLoan->setFieldU32(sfStartDate, startDate); if (baseFlags != 0) sleLoan->setFieldU32(sfFlags, baseFlags); if (baseNode) @@ -3999,7 +4009,8 @@ class Invariants_test : public beast::unit_test::Suite } }; - STTx const acceptTx{ttLOAN_ACCEPT, [](STObject&) {}}; + STTx const acceptTx{ + ttLOAN_ACCEPT, [&](STObject& tx) { tx.setAccountID(sfAccount, borrower.id()); }}; // ttLOAN_ACCEPT: modifying an active (non-pending) loan fails, // even if the modification is otherwise harmless. @@ -4051,6 +4062,172 @@ class Invariants_test : public beast::unit_test::Suite sle->setFieldU64(sfOwnerNode, 0); }, std::nullopt); + + // ttLOAN_ACCEPT: an account other than the loan's Borrower must not + // accept the loan, even for an otherwise legitimate transition. + testLoanUpdate( + STTx{ + ttLOAN_ACCEPT, + [&](STObject& tx) { tx.setAccountID(sfAccount, stranger.id()); }}, + lsfLoanPending, + std::nullopt, + [](SLE::pointer const& sle) { + sle->clearFlag(lsfLoanPending); + sle->setFieldU64(sfOwnerNode, 0); + }, + "LoanAccept submitted by an account other than the Borrower"); + + // ttLOAN_ACCEPT: the loan's StartDate must still be in the future; + // accepting a loan whose StartDate has passed is a violation. + testLoanUpdate( + acceptTx, + lsfLoanPending, + std::nullopt, + [](SLE::pointer const& sle) { + sle->clearFlag(lsfLoanPending); + sle->setFieldU64(sfOwnerNode, 0); + }, + "LoanAccept processed a Loan whose StartDate is not in the future", + 1u); + + // The pending flag may only be cleared by LoanAccept: any other + // transaction clearing it is a violation. + testLoanUpdate( + STTx{ttACCOUNT_SET, [](STObject&) {}}, + lsfLoanPending, + std::nullopt, + [](SLE::pointer const& sle) { sle->clearFlag(lsfLoanPending); }, + "Loan Pending flag changed by an unauthorized transaction"); + + // The pending flag may never be set on an existing loan (it is only + // set at creation by LoanSet), not even by LoanAccept. + testLoanUpdate( + acceptTx, + 0, + 0, + [](SLE::pointer const& sle) { sle->setFlag(lsfLoanPending); }, + "Loan Pending flag changed by an unauthorized transaction"); + } + + // LoanSet malformedness: with the two-step flow enabled a LoanSet that + // creates a loan must use exactly one of two mutually exclusive paths - + // it either names a Borrower, or it carries a Counterparty together with + // a CounterpartySignature. A LoanSet that breaks these rules must never + // be applied. Each case creates a fully paid-off loan directly (so the + // earlier loan checks pass) under a ttLOAN_SET whose fields break one of + // the rules. + { + Account const borrower{"borrower"}; + Account const counterparty{"counterparty"}; + + // Creates a fully paid-off loan, optionally flagged pending, so the + // earlier loan checks pass and only the LoanSet creation checks + // fire. + auto const createLoan = [](bool pending) { + return [pending](Account const& a1, Account const&, ApplyContext& ac) { + auto const vaultKeylet = keylet::vault(a1.id(), ac.view().seq()); + auto const loanKeylet = keylet::loan(vaultKeylet.key, 1); + auto sleLoan = std::make_shared(loanKeylet); + sleLoan->at(sfPrincipalOutstanding) = Number(0); + sleLoan->at(sfTotalValueOutstanding) = Number(0); + sleLoan->at(sfManagementFeeOutstanding) = Number(0); + sleLoan->at(sfPeriodicPayment) = Number(1); + sleLoan->setFieldU32(sfPaymentRemaining, 0); + if (pending) + sleLoan->setFlag(lsfLoanPending); + ac.view().insert(sleLoan); + return true; + }; + }; + + // A Counterparty and CounterpartySignature must not be accompanied + // by a Borrower. + doInvariantCheck( + Env{*this, defaultAmendments() | featureLendingProtocolV1_1}, + {"LoanSet specified a Counterparty and CounterpartySignature " + "together with a Borrower"}, + createLoan(false), + XRPAmount{}, + STTx{ + ttLOAN_SET, + [&](STObject& tx) { + tx.setAccountID(sfBorrower, borrower.id()); + tx.setAccountID(sfCounterparty, counterparty.id()); + tx.makeFieldPresent(sfCounterpartySignature); + }}, + {tecINVARIANT_FAILED, tefINVARIANT_FAILED}); + + // A Borrower must not be accompanied by a Counterparty or a + // CounterpartySignature. + doInvariantCheck( + Env{*this, defaultAmendments() | featureLendingProtocolV1_1}, + {"LoanSet specified a Borrower together with a Counterparty or " + "CounterpartySignature"}, + createLoan(false), + XRPAmount{}, + STTx{ + ttLOAN_SET, + [&](STObject& tx) { + tx.setAccountID(sfBorrower, borrower.id()); + tx.setAccountID(sfCounterparty, counterparty.id()); + }}, + {tecINVARIANT_FAILED, tefINVARIANT_FAILED}); + + // A LoanSet must use one of the two creation paths. + doInvariantCheck( + Env{*this, defaultAmendments() | featureLendingProtocolV1_1}, + {"LoanSet specified neither a Borrower nor a Counterparty with " + "CounterpartySignature"}, + createLoan(false), + XRPAmount{}, + STTx{ttLOAN_SET, [](STObject&) {}}, + {tecINVARIANT_FAILED, tefINVARIANT_FAILED}); + + // A Borrower-only LoanSet (no CounterpartySignature) must create a + // pending loan; creating a non-pending loan instead is a violation. + doInvariantCheck( + Env{*this, defaultAmendments() | featureLendingProtocolV1_1}, + {"LoanSet pending flag does not match Borrower and " + "CounterpartySignature"}, + createLoan(false), + XRPAmount{}, + STTx{ttLOAN_SET, [&](STObject& tx) { tx.setAccountID(sfBorrower, borrower.id()); }}, + {tecINVARIANT_FAILED, tefINVARIANT_FAILED}); + + // A Counterparty + CounterpartySignature LoanSet must create an + // active (non-pending) loan; creating a pending loan instead is a + // violation. + doInvariantCheck( + Env{*this, defaultAmendments() | featureLendingProtocolV1_1}, + {"LoanSet pending flag does not match Borrower and " + "CounterpartySignature"}, + createLoan(true), + XRPAmount{}, + STTx{ + ttLOAN_SET, + [&](STObject& tx) { + tx.setAccountID(sfCounterparty, counterparty.id()); + tx.makeFieldPresent(sfCounterpartySignature); + }}, + {tecINVARIANT_FAILED, tefINVARIANT_FAILED}); + + // A created loan must record its Borrower: an otherwise well-formed + // LoanSet that leaves the loan without a Borrower is a violation. + // createLoan never sets sfBorrower, so a Counterparty + + // CounterpartySignature LoanSet (which passes the mutual-exclusion + // and pending-flag checks) reaches and trips this check. + doInvariantCheck( + Env{*this, defaultAmendments() | featureLendingProtocolV1_1}, + {"LoanSet did not set the Loan Borrower"}, + createLoan(false), + XRPAmount{}, + STTx{ + ttLOAN_SET, + [&](STObject& tx) { + tx.setAccountID(sfCounterparty, counterparty.id()); + tx.makeFieldPresent(sfCounterpartySignature); + }}, + {tecINVARIANT_FAILED, tefINVARIANT_FAILED}); } // Loan interest due (total value less principal and management fee)