From d1dc7a6ccf7541212ee7fc15298ab9fa91aea27c Mon Sep 17 00:00:00 2001 From: Vito Tumas <5780819+Tapanito@users.noreply.github.com> Date: Wed, 19 Aug 2026 14:10:11 +0000 Subject: [PATCH] refactor: Extract invariant invocation into free checkInvariants runner (#7404) Co-authored-by: Cursor --- include/xrpl/tx/ApplyContext.h | 18 --- include/xrpl/tx/Transactor.h | 79 +++++++--- include/xrpl/tx/invariants/InvariantRunner.h | 140 ++++++++++++++++++ src/libxrpl/tx/ApplyContext.cpp | 79 ---------- src/libxrpl/tx/Transactor.cpp | 74 +++------ src/libxrpl/tx/invariants/InvariantRunner.cpp | 110 ++++++++++++++ src/test/app/Invariants_test.cpp | 135 ++++++++++++++++- src/test/app/NFTokenBurn_test.cpp | 5 +- 8 files changed, 465 insertions(+), 175 deletions(-) create mode 100644 include/xrpl/tx/invariants/InvariantRunner.h create mode 100644 src/libxrpl/tx/invariants/InvariantRunner.cpp diff --git a/include/xrpl/tx/ApplyContext.h b/include/xrpl/tx/ApplyContext.h index 472afdf624..e827e69f01 100644 --- a/include/xrpl/tx/ApplyContext.h +++ b/include/xrpl/tx/ApplyContext.h @@ -17,7 +17,6 @@ #include #include #include -#include namespace xrpl { @@ -130,16 +129,6 @@ public: view_->rawDestroyXRP(fee); } - /** - * Applies all invariant checkers one by one. - * - * @param result the result generated by processing this transaction. - * @param fee the fee charged for this transaction - * @return the result code that should be returned for this transaction. - */ - TER - checkInvariants(TER const result, XRPAmount const fee); - ApplyViewContext getApplyViewContext() { @@ -150,13 +139,6 @@ public: } private: - static TER - failInvariantCheck(TER const result); - - template - TER - checkInvariantsHelper(TER const result, XRPAmount const fee, std::index_sequence); - OpenView& base_; ApplyFlags flags_; std::optional view_; diff --git a/include/xrpl/tx/Transactor.h b/include/xrpl/tx/Transactor.h index a71285f70e..96ad7e00bc 100644 --- a/include/xrpl/tx/Transactor.h +++ b/include/xrpl/tx/Transactor.h @@ -20,6 +20,7 @@ #include #include #include +#include #include #include @@ -147,7 +148,7 @@ struct FeePayer FeePayerType type{FeePayerType::Account}; }; -class Transactor +class Transactor : public TxInvariantCheck { protected: ApplyContext& ctx_; @@ -158,7 +159,7 @@ protected: XRPAmount preFeeBalance_{}; // Balance before fees. public: - virtual ~Transactor() = default; + ~Transactor() override = default; Transactor(Transactor const&) = delete; Transactor& operator=(Transactor const&) = delete; @@ -183,20 +184,50 @@ public: return ctx_.view(); } + /** + * Which invariant layers to check. + * + * Full runs the protocol invariants plus the transaction-specific + * check. This is always the scope of the initial pass, even when the + * tentative TER is a tec: a bug or exploit could still mutate ledger + * state, so transaction-specific invariants must run for failed + * transactions too. + * + * ProtocolOnly runs only the protocol invariants and is used + * exclusively for the second invariant pass that follows a + * fee-claim reset — specifically, the reset that + * Transactor::operator() performs when the initial invariant pass + * returns tecINVARIANT_FAILED, rolling the transaction's effects back + * to a fee-claim-only state. In that reduced state the + * transaction-specific post-conditions no longer apply, but the + * protocol invariants must still hold against the fee claim itself. + * ProtocolOnly is not intended for other context discards (e.g. the + * reset used to handle tecOVERSIZE/tecKILLED/etc. in + * processPersistentChanges, or the ctx_.discard() done under + * TapFailHard); those paths do not re-run invariants at all. + */ + enum class InvariantScope { Full, ProtocolOnly }; + /** * Check all invariants for the current transaction. * - * Runs transaction-specific invariants first (visitInvariantEntry + - * finalizeInvariants), then protocol-level invariants. Both layers - * always run; the worst failure code is returned. + * Delegates to the free xrpl::checkInvariants runner. When @p scope is + * InvariantScope::Full, this transactor is passed so both layers + * share a single walk of the modified ledger entries. A failure in + * either layer fails the transaction the same way: tecINVARIANT_FAILED + * on the first pass, which the caller may respond to by rolling the + * transaction back to a fee-claim state and re-invoking this with + * InvariantScope::ProtocolOnly; a failure on that post-reset pass + * escalates to tefINVARIANT_FAILED. * * @param result the tentative TER from transaction processing. * @param fee the fee consumed by the transaction. + * @param scope which invariant layers to check. * * @return the final TER after all invariant checks. */ [[nodiscard]] TER - checkInvariants(TER result, XRPAmount fee); + checkInvariants(TER result, XRPAmount fee, InvariantScope scope); ///////////////////////////////////////////////////// /* @@ -538,20 +569,30 @@ private: preflightUniversal(PreflightContext const& ctx); /** - * Check transaction-specific invariants only. - * - * Walks every modified ledger entry via visitInvariantEntry, then - * calls finalizeInvariants on the derived transactor. Returns - * tecINVARIANT_FAILED if any transaction invariant is violated. - * - * @param result the tentative TER from transaction processing. - * @param fee the fee consumed by the transaction. - * - * @return the original result if all invariants pass, or - * tecINVARIANT_FAILED otherwise. + * Bridges the two-phase TxInvariantCheck interface to this transactor's + * visitInvariantEntry/finalizeInvariants hooks. Declared private (rather + * than protected, like the hooks they forward to) so that neither this + * transactor nor any subclass can call them directly through a + * Transactor& — only through the TxInvariantCheck& that the free + * xrpl::checkInvariants runner holds, which is where the two-phase + * ordering is enforced. */ - [[nodiscard]] TER - checkTransactionInvariants(TER result, XRPAmount fee); + void + visitEntry(bool isDelete, SLE::const_ref before, SLE::const_ref after) final + { + visitInvariantEntry(isDelete, before, after); + } + + [[nodiscard]] bool + finalize( + STTx const& tx, + TER result, + XRPAmount fee, + ReadView const& view, + beast::Journal const& j) final + { + return finalizeInvariants(tx, result, fee, view, j); + } }; inline bool diff --git a/include/xrpl/tx/invariants/InvariantRunner.h b/include/xrpl/tx/invariants/InvariantRunner.h new file mode 100644 index 0000000000..29a9dc09b2 --- /dev/null +++ b/include/xrpl/tx/invariants/InvariantRunner.h @@ -0,0 +1,140 @@ +#pragma once + +#include +#include +#include +#include +#include +#include +#include + +#include +#include + +namespace xrpl { + +/** + * @brief Runtime interface for a transaction-specific invariant check. + * + * The free checkInvariants runner drives two layers of checks over a single + * walk of the modified ledger entries: + * + * - Protocol checks are the concrete types in InvariantChecks, held in a + * std::tuple and dispatched statically by a compile-time fold (no + * virtual calls). They are duck-typed against the two-phase contract + * described below; see InvariantChecker_PROTOTYPE in InvariantCheck.h. + * - The transaction-specific check is injected at runtime through this + * interface, so the runner can call it without depending on the concrete + * transactor type. Transactor implements this interface directly (see + * Transactor.h) so that the interface's access can stay narrower than + * Transactor's own public surface: calling through a TxInvariantCheck& + * (all the runner ever holds) is public, but calling through a + * Transactor& is not, since Transactor overrides these as private + * (forwarding to its own protected visitInvariantEntry/finalizeInvariants). + * + * Both layers honour the same two-phase protocol: + * + * Phase 1 — state collection (visitEntry). Called once for each ledger + * entry created, modified, or deleted by the transaction. Implementations + * accumulate whatever state they need to evaluate their post-conditions. + * Must not throw. + * + * Phase 2 — condition evaluation (finalize). Called once after every + * modified entry has been visited. Returns true if all post-conditions + * hold, false to fail the transaction. + * + * Rule: invariants must run regardless of transaction result. finalize + * MUST perform meaningful checks even when the transaction has failed + * (when result is not tesSUCCESS). A bug or exploit could cause a failed + * transaction to mutate ledger state in unexpected ways; invariants are the + * last line of defense. + * + * The typical pattern: an invariant that expects a domain-specific state + * change (e.g. a Vault being created) should expect that change only when + * the transaction succeeded. A failed VaultCreate must not have created a + * Vault. + * + * Rule: privilege-gated checks apply to failed transactions too. Failed + * transactions carry no privileges. Any privilege-gated assertion must + * therefore also be enforced for failed transactions. + */ +class TxInvariantCheck +{ +public: + virtual ~TxInvariantCheck() = default; + + /** + * @brief Called for each ledger entry modified by the transaction. + * + * @param isDelete true if the SLE is being deleted. + * @param before the entry's state before the transaction (nullptr for + * newly created entries). + * @param after the entry's state after the transaction. For deletions + * this is the SLE being erased; use @p isDelete rather than + * a null @p after to detect deletions. @p after is + * never null. + */ + virtual void + visitEntry(bool isDelete, SLE::const_ref before, SLE::const_ref after) = 0; + + /** + * @brief Called after all entries have been visited. + * + * @param tx the transaction being applied. + * @param result the tentative TER result of the transaction. + * @param fee the fee consumed by the transaction. + * @param view read-only view of the ledger after the transaction. + * @param j journal for logging invariant failures. + * @return true if all invariants hold; false to fail with + * tecINVARIANT_FAILED / tefINVARIANT_FAILED. + */ + [[nodiscard]] virtual bool + finalize( + STTx const& tx, + TER result, + XRPAmount fee, + ReadView const& view, + beast::Journal const& j) = 0; +}; + +/** + * @brief Run all protocol invariant checks plus the transaction-specific check + * in a single pass over the modified entries. + * + * Both layers share one walk of the modified-entry set: @p txCheck's + * visitEntry accumulates state on the same traversal that drives the + * protocol checkers, then both layers' finalize run on the complete state. + * + * Any failure (a finalize returning false or an exception anywhere in the + * check) returns failInvariantCheck(result). On the first pass that yields + * tecINVARIANT_FAILED, which the transactor treats as a signal to roll the + * transaction's effects back to a fee-claim-only state and re-run this + * runner against the reduced state (see Transactor::InvariantScope). If + * that second pass also fails, the result escalates to tefINVARIANT_FAILED, + * which excludes the transaction from the ledger entirely. + * + * The whole traversal — both layers' visitEntry calls and both layers' + * finalize calls — runs under a single try/catch. There is no per-layer + * isolation: an exception anywhere aborts the remaining traversal and + * finalize calls and fails the transaction. + * + * @param ctx the apply context for the current transaction. + * @param result the tentative TER from transaction processing. + * @param fee the fee consumed by the transaction. + * @param txCheck the transaction-specific invariant check. + * @return the final TER after all invariant checks. + */ +[[nodiscard]] TER +checkInvariants( + ApplyContext& ctx, + TER result, + XRPAmount fee, + std::optional> txCheck); + +[[nodiscard]] inline TER +checkInvariants(ApplyContext& ctx, TER result, XRPAmount fee) +{ + return checkInvariants(ctx, result, fee, std::nullopt); +} + +} // namespace xrpl diff --git a/src/libxrpl/tx/ApplyContext.cpp b/src/libxrpl/tx/ApplyContext.cpp index 5e5ab90441..50f46fceef 100644 --- a/src/libxrpl/tx/ApplyContext.cpp +++ b/src/libxrpl/tx/ApplyContext.cpp @@ -1,27 +1,19 @@ #include -#include #include #include #include #include -#include #include #include #include #include #include #include -#include -#include -#include #include -#include #include #include -#include -#include namespace xrpl { @@ -75,75 +67,4 @@ ApplyContext::visit( view_->visit(base_, func); // NOLINT(bugprone-unchecked-optional-access) } -TER -ApplyContext::failInvariantCheck(TER const result) -{ - // If we already failed invariant checks before and we are now attempting to - // only charge a fee, and even that fails the invariant checks something is - // very wrong. We switch to tefINVARIANT_FAILED, which does NOT get included - // in a ledger. - - return (result == tecINVARIANT_FAILED || result == tefINVARIANT_FAILED) - ? TER{tefINVARIANT_FAILED} - : TER{tecINVARIANT_FAILED}; -} - -template -TER -ApplyContext::checkInvariantsHelper( - TER const result, - XRPAmount const fee, - std::index_sequence) -{ - try - { - auto checkers = getInvariantChecks(); - - // call each check's per-entry method - visit( - [&checkers]( - uint256 const& index, bool isDelete, SLE::const_ref before, SLE::const_ref after) { - (..., std::get(checkers).visitEntry(isDelete, before, after)); - }); - - // Note: do not replace this logic with a `...&&` fold expression. - // The fold expression will only run until the first check fails (it - // short-circuits). While the logic is still correct, the log - // message won't be. Every failed invariant should write to the log, - // not just the first one. - std::array const finalizers{{std::get(checkers).finalize( - tx, result, fee, *view_, journal)...}}; // NOLINT(bugprone-unchecked-optional-access) - - // call each check's finalizer to see that it passes - if (!std::ranges::all_of(finalizers, [](auto const& b) { return b; })) - { - JLOG(journal.fatal()) << "Transaction has failed one or more global invariants: " - << to_string(tx.getJson(JsonOptions::Values::None)); - - return failInvariantCheck(result); - } - } - catch (std::exception const& ex) - { - JLOG(journal.fatal()) << "Transaction caused an exception in a global invariant" - << ", ex: " << ex.what() - << ", tx: " << to_string(tx.getJson(JsonOptions::Values::None)); - - return failInvariantCheck(result); - } - - return result; -} - -TER -ApplyContext::checkInvariants(TER const result, XRPAmount const fee) -{ - XRPL_ASSERT( - isTesSuccess(result) || isTecClaim(result), - "xrpl::ApplyContext::checkInvariants : is tesSUCCESS or tecCLAIM"); - - return checkInvariantsHelper( - result, fee, std::make_index_sequence>{}); -} - } // namespace xrpl diff --git a/src/libxrpl/tx/Transactor.cpp b/src/libxrpl/tx/Transactor.cpp index 594aa24940..6bf99e567d 100644 --- a/src/libxrpl/tx/Transactor.cpp +++ b/src/libxrpl/tx/Transactor.cpp @@ -41,11 +41,11 @@ #include #include #include +#include #include #include #include -#include #include #include #include @@ -1540,53 +1540,12 @@ Transactor::processPersistentChanges(TER result, XRPAmount fee) } [[nodiscard]] TER -Transactor::checkTransactionInvariants(TER result, XRPAmount fee) +Transactor::checkInvariants(TER result, XRPAmount fee, InvariantScope scope) { - try - { - // Phase 1: visit modified entries - ctx_.visit( - [this](uint256 const&, bool isDelete, SLE::const_ref before, SLE::const_ref after) { - this->visitInvariantEntry(isDelete, before, after); - }); + if (scope == InvariantScope::Full) + return xrpl::checkInvariants(ctx_, result, fee, *this); - // Phase 2: finalize - if (!this->finalizeInvariants(ctx_.tx, result, fee, ctx_.view(), ctx_.journal)) - { - JLOG(ctx_.journal.fatal()) << // - "Transaction has failed one or more transaction invariants, tx: " << // - to_string(ctx_.tx.getJson(JsonOptions::Values::None)); - return tecINVARIANT_FAILED; - } - } - catch (std::exception const& ex) - { - JLOG(ctx_.journal.fatal()) << // - "Exception while checking transaction invariants: " << // - ex.what() << // - ", tx: " << // - to_string(ctx_.tx.getJson(JsonOptions::Values::None)); - - return tecINVARIANT_FAILED; - } - - return result; -} - -[[nodiscard]] TER -Transactor::checkInvariants(TER result, XRPAmount fee) -{ - /* - * DISABLED for 3.2.0 — Must be re-introduced for 3.3.0 - * - * Transaction invariants are disabled due to a performance regression: - * the two-pass design (transaction-specific invariants + protocol invariants) - * iterates over modified ledger entries twice per transaction. - * - * Until resolved, only protocol invariants are checked (delegated to ctx_). - * This is safe because all transaction invariants in 3.2.0 are no-ops. - */ - return ctx_.checkInvariants(result, fee); + return xrpl::checkInvariants(ctx_, result, fee); } //------------------------------------------------------------------------------ @@ -1674,24 +1633,29 @@ Transactor::operator()() if (!canApply) return logger(result, canApply); - // Check invariants: if `tecINVARIANT_FAILED` is not returned, we can - // proceed to apply the tx - result = checkInvariants(result, fee); + // First invariant pass: both protocol and transaction-specific + // checks run against the transaction's tentative outcome. If it + // does not return tecINVARIANT_FAILED, we can proceed to apply the + // tx. + result = checkInvariants(result, fee, InvariantScope::Full); if (result == tecINVARIANT_FAILED) { - // Reset to fee-claim only + // Fee-claim reset: roll the transaction's effects back so that + // only the fee deduction remains. This is the reset referenced + // by InvariantScope::ProtocolOnly. auto const resetResult = reset(fee); if (!isTesSuccess(resetResult.first)) result = resetResult.first; fee = resetResult.second; - // Check invariants again to ensure the fee claiming doesn't violate - // invariants. After reset, only protocol invariants are re-checked. - // Transaction invariants are not meaningful here — the transaction's - // effects have been rolled back. + // Re-check invariants against the post-reset (fee-claim only) + // state. The transaction's effects are gone, so the + // transaction-specific invariants no longer apply and only the + // protocol invariants are re-run. A failure here escalates to + // tefINVARIANT_FAILED and excludes the tx from the ledger. if (isTesSuccess(result) || isTecClaim(result)) - result = ctx_.checkInvariants(result, fee); + result = checkInvariants(result, fee, InvariantScope::ProtocolOnly); } // We ran through the invariant checker, which can, in some cases, diff --git a/src/libxrpl/tx/invariants/InvariantRunner.cpp b/src/libxrpl/tx/invariants/InvariantRunner.cpp new file mode 100644 index 0000000000..55bff2d693 --- /dev/null +++ b/src/libxrpl/tx/invariants/InvariantRunner.cpp @@ -0,0 +1,110 @@ +#include + +#include +#include +#include +#include +#include +#include // IWYU pragma: keep +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include +#include + +namespace xrpl { + +namespace { + +TER +failInvariantCheck(TER const result) +{ + return (result == tecINVARIANT_FAILED || result == tefINVARIANT_FAILED) + ? TER{tefINVARIANT_FAILED} + : TER{tecINVARIANT_FAILED}; +} + +template +TER +checkInvariantsHelper( + ApplyContext& ctx, + TER const result, + XRPAmount const fee, + std::optional> txCheck, + std::index_sequence) +{ + bool allOk = true; + + try + { + auto checkers = getInvariantChecks(); + + ctx.visit([&](uint256 const&, bool isDelete, SLE::const_ref before, SLE::const_ref after) { + if (txCheck) + txCheck->get().visitEntry(isDelete, before, after); + (..., std::get(checkers).visitEntry(isDelete, before, after)); + }); + + if (txCheck) + { + if (!txCheck->get().finalize(ctx.tx, result, fee, ctx.view(), ctx.journal)) + { + JLOG(ctx.journal.fatal()) + << "Transaction has failed one or more transaction invariants: " + << to_string(ctx.tx.getJson(JsonOptions::Values::None)); + allOk = false; + } + } + + // Note: do not replace this logic with a `...&&` fold expression. + // The fold expression will only run until the first check fails (it + // short-circuits). While the logic is still correct, the log + // message won't be. Every failed invariant should write to the log, + // not just the first one. + std::array const finalizers{ + {std::get(checkers).finalize(ctx.tx, result, fee, ctx.view(), ctx.journal)...}}; + + if (!std::all_of(finalizers.cbegin(), finalizers.cend(), [](auto const& b) { return b; })) + { + JLOG(ctx.journal.fatal()) << "Transaction has failed one or more global invariants: " + << to_string(ctx.tx.getJson(JsonOptions::Values::None)); + allOk = false; + } + } + catch (std::exception const& ex) + { + JLOG(ctx.journal.fatal()) << "Transaction caused an exception during invariant checks" + << ", ex: " << ex.what() << ", tx: " + << to_string(ctx.tx.getJson(JsonOptions::Values::None)); + return failInvariantCheck(result); + } + + return allOk ? result : failInvariantCheck(result); +} + +} // namespace + +TER +checkInvariants( + ApplyContext& ctx, + TER const result, + XRPAmount const fee, + std::optional> txCheck) +{ + XRPL_ASSERT( + isTesSuccess(result) || isTecClaim(result), + "xrpl::checkInvariants : is tesSUCCESS or tecCLAIM"); + + return checkInvariantsHelper( + ctx, result, fee, txCheck, std::make_index_sequence>{}); +} + +} // namespace xrpl diff --git a/src/test/app/Invariants_test.cpp b/src/test/app/Invariants_test.cpp index 70eaadbe17..ced2dea9bb 100644 --- a/src/test/app/Invariants_test.cpp +++ b/src/test/app/Invariants_test.cpp @@ -22,6 +22,7 @@ #include #include #include +#include #include #include #include @@ -56,6 +57,7 @@ #include #include #include +#include #include #include @@ -68,6 +70,7 @@ #include #include #include +#include #include #include #include @@ -217,7 +220,8 @@ class Invariants_test : public beast::unit_test::Suite TER terActual = tesSUCCESS; for (TER const& terExpect : ters) { - terActual = transactor->checkInvariants(terActual, fee); + terActual = + transactor->checkInvariants(terActual, fee, Transactor::InvariantScope::Full); expect( terExpect == terActual, "expected: " + transToken(terExpect) + " got: " + transToken(terActual), @@ -6379,12 +6383,137 @@ class Invariants_test : public beast::unit_test::Suite auto transactor = makeTransactor(ac); if (!BEAST_EXPECT(transactor)) return; - TER const result = transactor->checkInvariants(tesSUCCESS, XRPAmount{}); + TER const result = transactor->checkInvariants( + tesSUCCESS, XRPAmount{}, Transactor::InvariantScope::Full); BEAST_EXPECT(result == tecINVARIANT_FAILED); BEAST_EXPECT(sink.messages().str().contains("is missing pseudo-account field")); } } + void + testTxCheckException() + { + testcase << "txCheck exception"; + using namespace jtx; + + // A TxInvariantCheck that throws from the requested hook, so we can + // exercise checkInvariantsHelper's catch block via the + // transaction-specific layer (as opposed to the protocol layer, + // which testObjectHasPseudoAccount's last case already covers via a + // real Transactor's finalizeInvariants). + enum class ThrowFrom { VisitEntry, Finalize }; + + struct ThrowingTxInvariantCheck : TxInvariantCheck + { + ThrowFrom const throwFrom; + + explicit ThrowingTxInvariantCheck(ThrowFrom throwFrom) : throwFrom(throwFrom) + { + } + + void + visitEntry(bool, SLE::const_ref, SLE::const_ref) override + { + if (throwFrom == ThrowFrom::VisitEntry) + throw std::runtime_error("test-injected visitEntry exception"); + } + + [[nodiscard]] bool + finalize(STTx const&, TER, XRPAmount, ReadView const&, beast::Journal const&) override + { + if (throwFrom == ThrowFrom::Finalize) + throw std::runtime_error("test-injected finalize exception"); + return true; + } + }; + + for (auto const throwFrom : {ThrowFrom::VisitEntry, ThrowFrom::Finalize}) + { + Env env{*this}; + Account const alice{"alice"}; + env.fund(XRP(1000), alice); + env.close(); + + OpenView ov{*env.current()}; + 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()); + + // visitEntry only runs for entries the transaction touched, so + // make a modification for the traversal to report. + auto sle = ac.view().peek(keylet::account(alice.id())); + if (!BEAST_EXPECT(sle)) + return; + sle->at(sfSequence) = sle->at(sfSequence) + 1; + ac.view().update(sle); + + ThrowingTxInvariantCheck throwing{throwFrom}; + TER terActual = tesSUCCESS; + for (TER const& terExpect : {TER(tecINVARIANT_FAILED), TER(tefINVARIANT_FAILED)}) + { + terActual = checkInvariants(ac, terActual, XRPAmount{}, throwing); + BEAST_EXPECT(terExpect == terActual); + BEAST_EXPECT(sink.messages().str().contains( + "Transaction caused an exception during invariant checks")); + } + } + } + + void + testTxCheckFinalizeFalse() + { + testcase << "txCheck finalize returns false"; + using namespace jtx; + + // A TxInvariantCheck whose finalize returns false, so we can exercise + // the "Transaction has failed one or more transaction invariants" + // log path in checkInvariantsHelper independently of any real + // transactor. This is the transaction-layer analogue of the + // protocol-layer coverage in testObjectHasPseudoAccount / others. + struct FailingTxInvariantCheck : TxInvariantCheck + { + void + visitEntry(bool, SLE::const_ref, SLE::const_ref) override + { + } + + [[nodiscard]] bool + finalize(STTx const&, TER, XRPAmount, ReadView const&, beast::Journal const&) override + { + return false; + } + }; + + Env env{*this}; + Account const alice{"alice"}; + env.fund(XRP(1000), alice); + env.close(); + + OpenView ov{*env.current()}; + 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()); + + FailingTxInvariantCheck failing; + TER terActual = tesSUCCESS; + for (TER const& terExpect : {TER(tecINVARIANT_FAILED), TER(tefINVARIANT_FAILED)}) + { + terActual = checkInvariants(ac, terActual, XRPAmount{}, failing); + BEAST_EXPECT(terExpect == terActual); + BEAST_EXPECT(sink.messages().str().contains( + "Transaction has failed one or more transaction invariants")); + // The protocol-layer log must not appear: only the tx-layer + // finalize failed here. + BEAST_EXPECT(!sink.messages().str().contains( + "Transaction has failed one or more global invariants")); + } + } + void testConfidentialMPTTransfer() { @@ -6670,6 +6799,8 @@ public: testAMM(); testObjectHasPseudoAccount(); testSponsorship(); + testTxCheckException(); + testTxCheckFinalizeFalse(); } }; diff --git a/src/test/app/NFTokenBurn_test.cpp b/src/test/app/NFTokenBurn_test.cpp index 52565432a9..ae1d557bb9 100644 --- a/src/test/app/NFTokenBurn_test.cpp +++ b/src/test/app/NFTokenBurn_test.cpp @@ -32,6 +32,7 @@ #include #include #include +#include #include #include @@ -794,7 +795,7 @@ class NFTokenBurn_test : public beast::unit_test::Suite TER terActual = tesSUCCESS; for (TER const& terExpect : {TER(tecINVARIANT_FAILED), TER(tefINVARIANT_FAILED)}) { - terActual = ac.checkInvariants(terActual, XRPAmount{}); + terActual = xrpl::checkInvariants(ac, terActual, XRPAmount{}); BEAST_EXPECT(terExpect == terActual); BEAST_EXPECT(sink.messages().str().starts_with("Invariant failed:")); // uncomment to log the invariant failure message @@ -830,7 +831,7 @@ class NFTokenBurn_test : public beast::unit_test::Suite TER terActual = tesSUCCESS; for (TER const& terExpect : {TER(tecINVARIANT_FAILED), TER(tefINVARIANT_FAILED)}) { - terActual = ac.checkInvariants(terActual, XRPAmount{}); + terActual = xrpl::checkInvariants(ac, terActual, XRPAmount{}); BEAST_EXPECT(terExpect == terActual); BEAST_EXPECT(sink.messages().str().starts_with("Invariant failed:")); // uncomment to log the invariant failure message