refactor: Drop InvariantCheckAdapter, dispatch tx invariants directly

Address open PR #7404 review feedback:
- Rename CheckInvariants.h/.cpp to InvariantRunner.h/.cpp; the old name
  read as invariants for the Checks (ttCHECK*) feature.
- Remove the InvariantCheck interface and Transactor::InvariantCheckAdapter
  wrapper. The free checkInvariants runner now takes an optional
  Transactor& directly and calls visitInvariantEntry/finalizeInvariants on
  it, since the adapter only existed to bridge to a single concrete type.
This commit is contained in:
Vito
2026-07-23 15:28:11 +02:00
parent 1135dcc1e7
commit fd914e1182
6 changed files with 127 additions and 230 deletions

View File

@@ -20,7 +20,7 @@
#include <xrpl/protocol/XRPAmount.h>
#include <xrpl/tx/ApplyContext.h>
#include <xrpl/tx/applySteps.h>
#include <xrpl/tx/invariants/CheckInvariants.h>
#include <xrpl/tx/invariants/InvariantRunner.h>
#include <cstddef>
#include <cstdint>
@@ -197,8 +197,8 @@ public:
* Check all invariants for the current transaction.
*
* Delegates to the free @c xrpl::checkInvariants runner. When @p check is
* @c CheckTxInvariants::Yes, the transaction-specific adapter is passed so
* both layers share a single walk of the modified ledger entries.
* @c CheckTxInvariants::Yes, this transactor is passed so both layers
* share a single walk of the modified ledger entries.
* Protocol faults (tefINVARIANT_FAILED) take priority over transaction
* faults (tecINVARIANT_FAILED).
*
@@ -211,6 +211,51 @@ public:
[[nodiscard]] TER
checkInvariants(TER result, XRPAmount fee, CheckTxInvariants check);
/**
* Inspect a single ledger entry modified by this transaction.
*
* Called once for every SLE created, modified, or deleted by the
* transaction, before finalizeInvariants. Implementations should
* accumulate whatever state they need to verify transaction-specific
* post-conditions.
*
* @param isDelete true if the entry was erased from the ledger.
* @param before the entry's state before the transaction (nullptr
* for newly created entries).
* @param after the entry's state as supplied by the apply logic
* for this transaction. For deletions, this is the
* SLE being erased and is not guaranteed to be null;
* callers must use isDelete rather than after == nullptr
* to detect deletions.
*/
virtual void
visitInvariantEntry(bool isDelete, SLE::const_ref before, SLE::const_ref after) = 0;
/**
* Check transaction-specific post-conditions after all entries have
* been visited.
*
* Called once after every modified ledger entry has been passed to
* visitInvariantEntry. Returns true if all transaction-specific
* invariants hold, or false to fail the transaction with
* tecINVARIANT_FAILED.
*
* @param tx the transaction being applied.
* @param result the tentative TER result so far.
* @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 pass; false otherwise.
*/
[[nodiscard]] virtual bool
finalizeInvariants(
STTx const& tx,
TER result,
XRPAmount fee,
ReadView const& view,
beast::Journal const& j) = 0;
/////////////////////////////////////////////////////
/*
These static functions are called from invoke_preclaim<Tx>
@@ -361,51 +406,6 @@ protected:
virtual TER
doApply() = 0;
/**
* Inspect a single ledger entry modified by this transaction.
*
* Called once for every SLE created, modified, or deleted by the
* transaction, before finalizeInvariants. Implementations should
* accumulate whatever state they need to verify transaction-specific
* post-conditions.
*
* @param isDelete true if the entry was erased from the ledger.
* @param before the entry's state before the transaction (nullptr
* for newly created entries).
* @param after the entry's state as supplied by the apply logic
* for this transaction. For deletions, this is the
* SLE being erased and is not guaranteed to be null;
* callers must use isDelete rather than after == nullptr
* to detect deletions.
*/
virtual void
visitInvariantEntry(bool isDelete, SLE::const_ref before, SLE::const_ref after) = 0;
/**
* Check transaction-specific post-conditions after all entries have
* been visited.
*
* Called once after every modified ledger entry has been passed to
* visitInvariantEntry. Returns true if all transaction-specific
* invariants hold, or false to fail the transaction with
* tecINVARIANT_FAILED.
*
* @param tx the transaction being applied.
* @param result the tentative TER result so far.
* @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 pass; false otherwise.
*/
[[nodiscard]] virtual bool
finalizeInvariants(
STTx const& tx,
TER result,
XRPAmount fee,
ReadView const& view,
beast::Journal const& j) = 0;
/**
* Compute the minimum fee required to process a transaction
* with a given baseFee based on the current server load.
@@ -549,37 +549,6 @@ private:
*/
static NotTEC
preflightUniversal(PreflightContext const& ctx);
/**
* Bridges the transaction-specific two-phase invariant hooks
* (visitInvariantEntry + finalizeInvariants) into the InvariantCheck
* interface consumed by the free xrpl::checkInvariants runner.
*/
class InvariantCheckAdapter : public InvariantCheck
{
Transactor& self_;
public:
explicit InvariantCheckAdapter(Transactor& self) : self_(self)
{
}
void
visitEntry(bool isDelete, SLE::const_ref before, SLE::const_ref after) override
{
self_.visitInvariantEntry(isDelete, before, after);
}
[[nodiscard]] bool
finalize(
STTx const& tx,
TER result,
XRPAmount fee,
ReadView const& view,
beast::Journal const& j) const override;
};
InvariantCheckAdapter invariantCheck_{*this};
};
inline bool

View File

@@ -1,132 +0,0 @@
#pragma once
#include <xrpl/beast/utility/Journal.h>
#include <xrpl/ledger/ReadView.h>
#include <xrpl/protocol/STLedgerEntry.h>
#include <xrpl/protocol/STTx.h>
#include <xrpl/protocol/TER.h>
#include <xrpl/protocol/XRPAmount.h>
#include <xrpl/tx/ApplyContext.h>
#include <functional>
#include <optional>
namespace xrpl {
/**
* @brief Runtime interface for a transaction-specific invariant check.
*
* The free @c checkInvariants runner drives two layers of checks over a single
* walk of the modified ledger entries:
*
* - **Protocol checks** are the concrete types in @c InvariantChecks, held in
* a @c 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 @c 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.
*
* 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.
*
* ## Rules for implementing `finalize`
*
* ### Invariants must run regardless of transaction result
*
* `finalize` MUST perform meaningful checks even when the transaction has
* failed (`!isTesSuccess(result)`). 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.
*
* ### 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 InvariantCheck
{
public:
virtual ~InvariantCheck() = 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
* `after == nullptr` 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) const = 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 @c failInvariantCheck(result). On the first pass that yields
* @c tecINVARIANT_FAILED. If that triggers a fee-claim reset and invariants
* are checked again, a second failure escalates to @c tefINVARIANT_FAILED,
* which excludes the transaction from the ledger entirely.
*
* @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<std::reference_wrapper<InvariantCheck>> txCheck);
[[nodiscard]] inline TER
checkInvariants(ApplyContext& ctx, TER result, XRPAmount fee)
{
return checkInvariants(ctx, result, fee, std::nullopt);
}
} // namespace xrpl

View File

@@ -0,0 +1,70 @@
#pragma once
#include <xrpl/protocol/TER.h>
#include <xrpl/protocol/XRPAmount.h>
#include <xrpl/tx/ApplyContext.h>
#include <functional>
#include <optional>
namespace xrpl {
class Transactor;
/**
* @brief Run all protocol invariant checks plus the transaction-specific check
* in a single pass over the modified entries.
*
* Two layers of checks share one walk of the modified-entry set:
*
* - **Protocol checks** are the concrete types in @c InvariantChecks, held in
* a @c 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 @c InvariantChecker_PROTOTYPE in InvariantCheck.h.
* - **The transaction-specific check**, when @p txCheck is seated, is
* dispatched at runtime through @c Transactor::visitInvariantEntry and
* @c Transactor::finalizeInvariants.
*
* Both layers honour the same two-phase protocol:
*
* **Phase 1 — state collection** (`visitEntry` / `visitInvariantEntry`)
* 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` / `finalizeInvariants`)
* Called once after every modified entry has been visited. Returns true if
* all post-conditions hold, false to fail the transaction.
*
* `txCheck`'s phase 1 accumulates state on the same traversal that drives the
* protocol checkers, then both layers' phase 2 run on the complete state.
*
* Any failure (a finalize step returning false or an exception anywhere in
* the check) returns @c failInvariantCheck(result). On the first pass that
* yields @c tecINVARIANT_FAILED. If that triggers a fee-claim reset and
* invariants are checked again, a second failure escalates to
* @c tefINVARIANT_FAILED, which excludes the transaction from the ledger
* entirely.
*
* @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 transactor whose transaction-specific invariants should
* also be checked, or @c std::nullopt to run only the
* protocol checks.
* @return the final TER after all invariant checks.
*/
[[nodiscard]] TER
checkInvariants(
ApplyContext& ctx,
TER result,
XRPAmount fee,
std::optional<std::reference_wrapper<Transactor>> txCheck);
[[nodiscard]] inline TER
checkInvariants(ApplyContext& ctx, TER result, XRPAmount fee)
{
return checkInvariants(ctx, result, fee, std::nullopt);
}
} // namespace xrpl

View File

@@ -41,7 +41,7 @@
#include <xrpl/tx/SignerEntries.h>
#include <xrpl/tx/apply.h>
#include <xrpl/tx/applySteps.h>
#include <xrpl/tx/invariants/CheckInvariants.h>
#include <xrpl/tx/invariants/InvariantRunner.h>
#include <algorithm>
#include <cstddef>
@@ -1426,17 +1426,6 @@ Transactor::trapTransaction(uint256 txHash) const
JLOG(j_.debug()) << "Transaction trapped: " << txHash;
}
[[nodiscard]] bool
Transactor::InvariantCheckAdapter::finalize(
STTx const& tx,
TER const result,
XRPAmount const fee,
ReadView const& view,
beast::Journal const& j) const
{
return self_.finalizeInvariants(tx, result, fee, view, j);
}
std::tuple<TER, XRPAmount, bool>
Transactor::processPersistentChanges(TER result, XRPAmount fee)
{
@@ -1555,7 +1544,7 @@ Transactor::checkInvariants(TER result, XRPAmount fee, CheckTxInvariants check)
if (check == CheckTxInvariants::No)
return xrpl::checkInvariants(ctx_, result, fee);
return xrpl::checkInvariants(ctx_, result, fee, std::ref(invariantCheck_));
return xrpl::checkInvariants(ctx_, result, fee, std::ref(*this));
}
//------------------------------------------------------------------------------

View File

@@ -1,4 +1,4 @@
#include <xrpl/tx/invariants/CheckInvariants.h>
#include <xrpl/tx/invariants/InvariantRunner.h>
#include <xrpl/basics/Log.h>
#include <xrpl/basics/base_uint.h>
@@ -9,6 +9,7 @@
#include <xrpl/protocol/TER.h>
#include <xrpl/protocol/XRPAmount.h>
#include <xrpl/tx/ApplyContext.h>
#include <xrpl/tx/Transactor.h>
#include <xrpl/tx/invariants/InvariantCheck.h>
#include <algorithm>
@@ -38,7 +39,7 @@ checkInvariantsHelper(
ApplyContext& ctx,
TER const result,
XRPAmount const fee,
std::optional<std::reference_wrapper<InvariantCheck>> txCheck,
std::optional<std::reference_wrapper<Transactor>> txCheck,
std::index_sequence<Is...>)
{
auto checkers = getInvariantChecks();
@@ -48,13 +49,13 @@ checkInvariantsHelper(
{
ctx.visit([&](uint256 const&, bool isDelete, SLE::const_ref before, SLE::const_ref after) {
if (txCheck)
txCheck->get().visitEntry(isDelete, before, after);
txCheck->get().visitInvariantEntry(isDelete, before, after);
(..., std::get<Is>(checkers).visitEntry(isDelete, before, after));
});
if (txCheck)
{
if (!txCheck->get().finalize(ctx.tx, result, fee, ctx.view(), ctx.journal))
if (!txCheck->get().finalizeInvariants(ctx.tx, result, fee, ctx.view(), ctx.journal))
{
JLOG(ctx.journal.fatal())
<< "Transaction has failed one or more transaction invariants: "
@@ -100,7 +101,7 @@ checkInvariants(
ApplyContext& ctx,
TER const result,
XRPAmount const fee,
std::optional<std::reference_wrapper<InvariantCheck>> txCheck)
std::optional<std::reference_wrapper<Transactor>> txCheck)
{
XRPL_ASSERT(
isTesSuccess(result) || isTecClaim(result),

View File

@@ -31,7 +31,7 @@
#include <xrpl/protocol/jss.h>
#include <xrpl/protocol/nft.h>
#include <xrpl/tx/ApplyContext.h>
#include <xrpl/tx/invariants/CheckInvariants.h>
#include <xrpl/tx/invariants/InvariantRunner.h>
#include <algorithm>
#include <cstddef>