diff --git a/include/xrpl/tx/invariants/LoanBrokerInvariant.h b/include/xrpl/tx/invariants/LoanBrokerInvariant.h index 979f57de35..8a37855947 100644 --- a/include/xrpl/tx/invariants/LoanBrokerInvariant.h +++ b/include/xrpl/tx/invariants/LoanBrokerInvariant.h @@ -19,6 +19,9 @@ namespace xrpl { * 1. If `LoanBroker.OwnerCount = 0` the `DirectoryNode` will have at most one * node (the root), which will only hold entries for `RippleState` or * `MPToken` objects. + * 2. Under featureLendingProtocolV1_1, an `ltLOAN_BROKER` may only be deleted + * by a `ttLOAN_BROKER_DELETE` transaction, and only when its pre-state + * `DebtTotal` and `OwnerCount` are both zero. * */ class ValidLoanBroker @@ -36,6 +39,10 @@ class ValidLoanBroker // pseudo-accounts. Key is the brokerID / index. It will be used to find the // LoanBroker object if brokerBefore and brokerAfter are nullptr std::map brokers_; + // Brokers whose ledger entry was deleted by this transaction. The final + // pre-deletion state is captured so the deletion invariants can inspect + // DebtTotal and OwnerCount. + std::vector deletedBrokers_; // Collect all the modified trust lines. Their high and low accounts will be // loaded to look for LoanBroker pseudo-accounts. std::vector lines_; diff --git a/src/libxrpl/tx/invariants/LoanBrokerInvariant.cpp b/src/libxrpl/tx/invariants/LoanBrokerInvariant.cpp index b70c02947f..96dadf7d60 100644 --- a/src/libxrpl/tx/invariants/LoanBrokerInvariant.cpp +++ b/src/libxrpl/tx/invariants/LoanBrokerInvariant.cpp @@ -22,6 +22,16 @@ namespace xrpl { void ValidLoanBroker::visitEntry(bool isDelete, SLE::const_ref before, SLE::const_ref after) { + // Track LoanBroker deletions so the finalize can enforce (a) that only + // ttLOAN_BROKER_DELETE removes a broker and (b) that the broker's + // pre-state DebtTotal and OwnerCount were zero at deletion. `before` + // holds the entry's state prior to erasure. + if (isDelete && before && before->getType() == ltLOAN_BROKER) + { + deletedBrokers_.emplace_back(before); + return; + } + if (after) { if (after->getType() == ltLOAN_BROKER) @@ -99,6 +109,38 @@ ValidLoanBroker::finalize( // Loan Brokers will not exist on ledger if the Lending Protocol amendment // is not enabled, so there's no need to check it. + // Deletion invariants (featureLendingProtocolV1_1). A LoanBroker may + // only be removed by ttLOAN_BROKER_DELETE, and only when its pre-state + // DebtTotal and OwnerCount are both zero. The DebtTotal check + // complements ValidLoan's LoanBrokerDelete-must-not-touch-any-loan + // rule: even a broker that has finished paying off every loan may + // still hold non-zero exposure until its LoanBrokerCoverWithdraw + // settles, and neither state is safe to delete. + if (view.rules().enabled(featureLendingProtocolV1_1)) + { + for (auto const& broker : deletedBrokers_) + { + if (tx.getTxnType() != ttLOAN_BROKER_DELETE) + { + JLOG(j.fatal()) << "Invariant failed: Loan Broker deleted by a " + "transaction other than LoanBrokerDelete"; + return false; + } + if (broker->at(sfDebtTotal) != beast::kZero) + { + JLOG(j.fatal()) << "Invariant failed: Loan Broker deleted with " + "non-zero debt total"; + return false; + } + if (broker->at(sfOwnerCount) != 0) + { + JLOG(j.fatal()) << "Invariant failed: Loan Broker deleted with " + "non-zero owner count"; + return false; + } + } + } + for (auto const& line : lines_) { for (auto const& field : {&sfLowLimit, &sfHighLimit}) diff --git a/src/libxrpl/tx/invariants/LoanInvariant.cpp b/src/libxrpl/tx/invariants/LoanInvariant.cpp index 7cd1aeefae..9e4a6aa2aa 100644 --- a/src/libxrpl/tx/invariants/LoanInvariant.cpp +++ b/src/libxrpl/tx/invariants/LoanInvariant.cpp @@ -117,6 +117,35 @@ ValidLoan::finalize( // set-once (never cleared) live in NoModifiedUnmodifiableFields, next // to the other loan-object constant-field immutability checks. + // Flag-transition scoping: lsfLoanImpaired may only move under + // ttLOAN_MANAGE (impair/unimpair) or ttLOAN_PAY (LoanPay::doApply + // calls LoanManage::unimpairLoan before applying the payment when + // the loan was impaired). lsfLoanDefault may only be set under + // ttLOAN_MANAGE (its set-once immutability is separately enforced + // by NoModifiedUnmodifiableFields). Any other transaction that + // moves these flags is manufacturing state. + if (before) + { + bool const wasImpaired = before->isFlag(lsfLoanImpaired); + bool const isImpaired = after->isFlag(lsfLoanImpaired); + if (wasImpaired != isImpaired && txType != ttLOAN_MANAGE && + txType != ttLOAN_PAY) + { + JLOG(j.fatal()) << "Invariant failed: lsfLoanImpaired changed " + "outside LoanManage or LoanPay"; + return false; + } + + bool const wasDefaulted = before->isFlag(lsfLoanDefault); + bool const isDefaulted = after->isFlag(lsfLoanDefault); + if (wasDefaulted != isDefaulted && txType != ttLOAN_MANAGE) + { + JLOG(j.fatal()) << "Invariant failed: lsfLoanDefault changed " + "outside LoanManage"; + return false; + } + } + // LoanManage sub-operation flag preconditions. These are transaction // post-conditions, so they only apply on a successful apply: after a // reset the ledger flags revert and the checks would spuriously fail. diff --git a/src/libxrpl/tx/invariants/VaultInvariant.cpp b/src/libxrpl/tx/invariants/VaultInvariant.cpp index 3c7e22b023..7fef4fd87f 100644 --- a/src/libxrpl/tx/invariants/VaultInvariant.cpp +++ b/src/libxrpl/tx/invariants/VaultInvariant.cpp @@ -1477,6 +1477,13 @@ ValidVault::finalize( // (deposit, withdraw, clawback, and every loan-* sub-operation); // vault set is excluded because it may never change either, and // vault create has nothing to compare against. + // + // The identity is written as `Δ (AssetsAvailable + reserved) == + // Δ pseudo-account balance`, with `reserved == 0` today. When + // Vault.AssetsReserved is introduced, the identity extends to + // `Δ AssetsAvailable + Δ AssetsReserved == Δ pseudo-account balance` + // by taking the sum of the two vault-side fields on both sides of the + // delta. See PR #7732 discussion r3756798430. if (txnType != ttVAULT_CREATE && txnType != ttVAULT_SET) { auto const& beforeVault = beforeVault_[0]; diff --git a/src/test/app/Invariants_test.cpp b/src/test/app/Invariants_test.cpp index e455ea570e..a33e1be72d 100644 --- a/src/test/app/Invariants_test.cpp +++ b/src/test/app/Invariants_test.cpp @@ -3091,6 +3091,128 @@ class Invariants_test : public beast::unit_test::Suite STTx{ttLOAN_BROKER_SET, [](STObject& tx) {}}, {tecINVARIANT_FAILED, tefINVARIANT_FAILED}, createLoanBroker); + + // A LoanBroker may only be removed by ttLOAN_BROKER_DELETE. Erase + // the broker in the apply view under a non-delete tx type and + // expect the deletion-tx invariant to fire. + doInvariantCheck( + {{"Loan Broker deleted by a transaction other than LoanBrokerDelete"}}, + [&](Account const&, Account const&, ApplyContext& ac) { + if (loanBrokerKeylet.type != ltLOAN_BROKER) + return false; + auto sleBroker = ac.view().peek(loanBrokerKeylet); + if (!BEAST_EXPECT(sleBroker)) + return false; + ac.view().erase(sleBroker); + return true; + }, + XRPAmount{}, + STTx{ttACCOUNT_SET, [](STObject&) {}}, + {tecINVARIANT_FAILED, tefINVARIANT_FAILED}, + createLoanBroker); + } + + // A LoanBrokerDelete must not remove a broker whose pre-transaction + // DebtTotal is non-zero. visitEntry captures `before` from the parent + // view, so the DebtTotal must be seeded in the OpenView before the + // ApplyContext is constructed; a Precheck modification would only + // land in the applyView (visible as `after`) and would leave `before` + // at the createLoanBroker-produced zero. + { + Env env{*this}; + Account const a1{"A1"}; + Account const a2{"A2"}; + env.fund(XRP(1000), a1, a2); + env.close(); + + PrettyAsset const xrpAsset{xrpIssue(), 1'000'000}; + auto const brokerKeylet = createLoanBroker(a1, env, xrpAsset); + if (!BEAST_EXPECT(env.le(brokerKeylet))) + return; + env.close(); + + OpenView ov{*env.current()}; + + // Seed a non-zero DebtTotal in the base view so `before` at + // visitEntry time reports it. + { + auto const sleBrokerRead = ov.read(brokerKeylet); + if (!BEAST_EXPECT(sleBrokerRead)) + return; + auto sleBroker = std::make_shared(*sleBrokerRead); + sleBroker->at(sfDebtTotal) = Number(1); + ov.rawReplace(sleBroker); + } + + STTx const tx{ttLOAN_BROKER_DELETE, [](STObject&) {}}; + test::StreamSink sink{beast::Severity::Warning}; + beast::Journal const jlog{sink}; + ApplyContext ac{ + env.app(), ov, tx, tesSUCCESS, env.current()->fees().base, TapNone, jlog}; + CurrentTransactionRulesGuard const rulesGuard(ov.rules()); + + auto sleBroker = ac.view().peek(brokerKeylet); + if (!BEAST_EXPECT(sleBroker)) + return; + ac.view().erase(sleBroker); + + auto transactor = makeTransactor(ac); + if (!BEAST_EXPECT(transactor)) + return; + TER const result = transactor->checkInvariants( + tesSUCCESS, XRPAmount{}, Transactor::InvariantScope::Full); + BEAST_EXPECT(result == tecINVARIANT_FAILED); + BEAST_EXPECT(sink.messages().str().contains( + "Loan Broker deleted with non-zero debt total")); + } + + // A LoanBrokerDelete must not remove a broker whose pre-transaction + // OwnerCount is non-zero. DebtTotal is left at zero so the earlier + // check passes and the OwnerCount check is what fires. + { + Env env{*this}; + Account const a1{"A1"}; + Account const a2{"A2"}; + env.fund(XRP(1000), a1, a2); + env.close(); + + PrettyAsset const xrpAsset{xrpIssue(), 1'000'000}; + auto const brokerKeylet = createLoanBroker(a1, env, xrpAsset); + if (!BEAST_EXPECT(env.le(brokerKeylet))) + return; + env.close(); + + OpenView ov{*env.current()}; + + { + auto const sleBrokerRead = ov.read(brokerKeylet); + if (!BEAST_EXPECT(sleBrokerRead)) + return; + auto sleBroker = std::make_shared(*sleBrokerRead); + sleBroker->at(sfOwnerCount) = 1; + ov.rawReplace(sleBroker); + } + + STTx const tx{ttLOAN_BROKER_DELETE, [](STObject&) {}}; + test::StreamSink sink{beast::Severity::Warning}; + beast::Journal const jlog{sink}; + ApplyContext ac{ + env.app(), ov, tx, tesSUCCESS, env.current()->fees().base, TapNone, jlog}; + CurrentTransactionRulesGuard const rulesGuard(ov.rules()); + + auto sleBroker = ac.view().peek(brokerKeylet); + if (!BEAST_EXPECT(sleBroker)) + return; + ac.view().erase(sleBroker); + + auto transactor = makeTransactor(ac); + if (!BEAST_EXPECT(transactor)) + return; + TER const result = transactor->checkInvariants( + tesSUCCESS, XRPAmount{}, Transactor::InvariantScope::Full); + BEAST_EXPECT(result == tecINVARIANT_FAILED); + BEAST_EXPECT(sink.messages().str().contains( + "Loan Broker deleted with non-zero owner count")); } } @@ -4754,6 +4876,23 @@ class Invariants_test : public beast::unit_test::Suite {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, precloseXrp); + // ttLOAN_MANAGE (default): the object-level identity + // `Delta Vault.AssetsAvailable == Delta Vault.pseudo-account balance` + // must hold. Bump assets available by +50 while leaving the vault + // pseudo-account (and the source of the first-loss capital) untouched: + // the shared identity fires because the two ledgers no longer add up. + // Sourced sibling: PR #7732 review discussion r3756829578. + doInvariantCheck( + {"vault balance and assets available must add up"}, + [&](Account const& a1, Account const& a2, ApplyContext& ac) { + auto const keylet = keylet::vault(a1.id(), SeqProxy::rawSequence(ac.view().seq())); + return kAdjust(ac.view(), keylet, Adjustments{.assetsAvailable = 50}); + }, + XRPAmount{}, + STTx{ttLOAN_MANAGE, [](STObject& tx) { tx.setFieldU32(sfFlags, tfLoanDefault); }}, + {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + precloseXrp); + // ttLOAN_MANAGE (unimpair): loss unrealized must not increase. Bumping // loss unrealized upward is the wrong direction for unimpair, which // reverses a paper loss. @@ -5000,6 +5139,86 @@ class Invariants_test : public beast::unit_test::Suite } } + // Flag-transition scoping (featureLendingProtocolV1_1). + // lsfLoanImpaired may only change under ttLOAN_MANAGE or ttLOAN_PAY + // (LoanPay unimpairs before applying the payment when the loan was + // impaired); lsfLoanDefault may only change under ttLOAN_MANAGE. + // Any other transaction that moves either flag is manufacturing + // state. Setup mirrors the impair/unimpair blocks: seed a + // pre-existing loan with a specific flag, then flip the flag under + // an out-of-scope transaction type. The out-of-scope tx is + // ttACCOUNT_SET, chosen because it is a valid transaction type + // that lives outside the loan-manage / loan-pay switch. + { + struct Case + { + std::uint32_t before; + std::uint32_t after; + std::string expected; + }; + auto const cases = std::to_array({ + {.before = 0, + .after = lsfLoanImpaired, + .expected = "lsfLoanImpaired changed outside LoanManage or LoanPay"}, + {.before = lsfLoanImpaired, + .after = 0, + .expected = "lsfLoanImpaired changed outside LoanManage or LoanPay"}, + // lsfLoanDefault: only the unset->set direction is exercised + // here because the reverse (set->unset) is separately blocked + // by NoModifiedUnmodifiableFields, whose fatal log fires first + // and would mask the ValidLoan message under test. + {.before = 0, + .after = lsfLoanDefault, + .expected = "lsfLoanDefault changed outside LoanManage"}, + }); + + for (auto const& c : cases) + { + Env env{*this, defaultAmendments()}; + Account const a1{"A1"}; + Account const a2{"A2"}; + env.fund(XRP(1000), a1, a2); + BEAST_EXPECT(precloseXrp(a1, a2, env)); + env.close(); + + OpenView ov{*env.current()}; + + auto const vaultKeylet = keylet::vault(a1.id(), SeqProxy::rawSequence(ov.seq())); + auto const loanKeylet = keylet::loan(vaultKeylet.key, SeqProxy::rawSequence(1)); + { + auto sleLoan = std::make_shared(loanKeylet); + sleLoan->at(sfPrincipalOutstanding) = Number(100); + sleLoan->at(sfTotalValueOutstanding) = Number(150); + sleLoan->at(sfManagementFeeOutstanding) = Number(0); + sleLoan->at(sfPeriodicPayment) = Number(1); + sleLoan->setFieldU32(sfPaymentRemaining, 1); + sleLoan->setFieldU32(sfFlags, c.before); + ov.rawInsert(sleLoan); + } + + STTx const tx{ttACCOUNT_SET, [](STObject&) {}}; + test::StreamSink sink{beast::Severity::Warning}; + beast::Journal const jlog{sink}; + ApplyContext ac{ + env.app(), ov, tx, tesSUCCESS, env.current()->fees().base, TapNone, jlog}; + CurrentTransactionRulesGuard const rulesGuard(ov.rules()); + + auto sleLoan = ac.view().peek(loanKeylet); + if (!BEAST_EXPECT(sleLoan)) + continue; + sleLoan->setFieldU32(sfFlags, c.after); + ac.view().update(sleLoan); + + auto transactor = makeTransactor(ac); + if (!BEAST_EXPECT(transactor)) + continue; + TER const result = transactor->checkInvariants( + tesSUCCESS, XRPAmount{}, Transactor::InvariantScope::Full); + BEAST_EXPECT(result == tecINVARIANT_FAILED); + BEAST_EXPECT(sink.messages().str().contains(c.expected)); + } + } + // ttLOAN_MANAGE (unimpair): the mirror of the impair check. A // successful unimpair must leave the loan without lsfLoanImpaired and // must not target a non-impaired loan. Setup mirrors the impair block