mirror of
https://github.com/XRPLF/rippled.git
synced 2026-08-21 14:20:56 +00:00
refactor: Have Transactor implement TxInvariantCheck directly
Restores a runtime interface (renamed InvariantCheck -> TxInvariantCheck per review feedback) that Transactor now inherits from, rather than the free checkInvariants runner depending on Transactor directly. Transactor implements visitEntry/finalize as private final overrides forwarding to its own protected visitInvariantEntry/finalizeInvariants, so the two-phase hooks stay unreachable through a bare Transactor&, only through the TxInvariantCheck& the runner holds. Adds a test exercising the runner's exception path via a throwing TxInvariantCheck, and fixes two inaccurate doc comments (checkInvariants' stale escalation description, and getInvariantChecks() construction moved back inside the try block).
This commit is contained in:
Vito
@@ -148,7 +148,7 @@ struct FeePayer
|
||||
FeePayerType type{FeePayerType::Account};
|
||||
};
|
||||
|
||||
class Transactor
|
||||
class Transactor : public TxInvariantCheck
|
||||
{
|
||||
protected:
|
||||
ApplyContext& ctx_;
|
||||
@@ -159,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;
|
||||
@@ -198,9 +198,10 @@ public:
|
||||
*
|
||||
* Delegates to the free @c xrpl::checkInvariants runner. When @p check is
|
||||
* @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).
|
||||
* 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, escalating to tefINVARIANT_FAILED if invariants are
|
||||
* checked again after a fee-claim reset.
|
||||
*
|
||||
* @param result the tentative TER from transaction processing.
|
||||
* @param fee the fee consumed by the transaction.
|
||||
@@ -211,51 +212,6 @@ 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>
|
||||
@@ -406,6 +362,51 @@ 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,6 +550,32 @@ private:
|
||||
*/
|
||||
static NotTEC
|
||||
preflightUniversal(PreflightContext const& ctx);
|
||||
|
||||
/**
|
||||
* 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
|
||||
* @c Transactor& — only through the @c TxInvariantCheck& that the free
|
||||
* @c xrpl::checkInvariants runner holds, which is where the two-phase
|
||||
* ordering is enforced.
|
||||
*/
|
||||
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
|
||||
|
||||
@@ -1,5 +1,9 @@
|
||||
#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>
|
||||
@@ -9,49 +13,117 @@
|
||||
|
||||
namespace xrpl {
|
||||
|
||||
class Transactor;
|
||||
|
||||
/**
|
||||
* @brief Run all protocol invariant checks plus the transaction-specific check
|
||||
* in a single pass over the modified entries.
|
||||
* @brief Runtime interface for a transaction-specific invariant check.
|
||||
*
|
||||
* Two layers of checks share one walk of the modified-entry set:
|
||||
* 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**, when @p txCheck is seated, is
|
||||
* dispatched at runtime through @c Transactor::visitInvariantEntry and
|
||||
* @c Transactor::finalizeInvariants.
|
||||
* - **The transaction-specific check** is injected at runtime through this
|
||||
* interface, so the runner can call it without depending on the concrete
|
||||
* transactor type. @c 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 @c TxInvariantCheck&
|
||||
* (all the runner ever holds) is public, but calling through a
|
||||
* @c 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` / `visitInvariantEntry`)
|
||||
* **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` / `finalizeInvariants`)
|
||||
* **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.
|
||||
*
|
||||
* `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.
|
||||
* ## Rules for implementing `finalize`
|
||||
*
|
||||
* 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.
|
||||
* ### 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 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
|
||||
* `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) = 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.
|
||||
*
|
||||
* 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 transactor whose transaction-specific invariants should
|
||||
* also be checked, or @c std::nullopt to run only the
|
||||
* protocol checks.
|
||||
* @param txCheck the transaction-specific invariant check.
|
||||
* @return the final TER after all invariant checks.
|
||||
*/
|
||||
[[nodiscard]] TER
|
||||
@@ -59,7 +131,7 @@ checkInvariants(
|
||||
ApplyContext& ctx,
|
||||
TER result,
|
||||
XRPAmount fee,
|
||||
std::optional<std::reference_wrapper<Transactor>> txCheck);
|
||||
std::optional<std::reference_wrapper<TxInvariantCheck>> txCheck);
|
||||
|
||||
[[nodiscard]] inline TER
|
||||
checkInvariants(ApplyContext& ctx, TER result, XRPAmount fee)
|
||||
|
||||
@@ -46,7 +46,6 @@
|
||||
#include <algorithm>
|
||||
#include <cstddef>
|
||||
#include <cstdint>
|
||||
#include <functional>
|
||||
#include <map>
|
||||
#include <optional>
|
||||
#include <stdexcept>
|
||||
@@ -1544,7 +1543,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(*this));
|
||||
return xrpl::checkInvariants(ctx_, result, fee, *this);
|
||||
}
|
||||
|
||||
//------------------------------------------------------------------------------
|
||||
|
||||
@@ -9,7 +9,6 @@
|
||||
#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>
|
||||
@@ -39,23 +38,24 @@ checkInvariantsHelper(
|
||||
ApplyContext& ctx,
|
||||
TER const result,
|
||||
XRPAmount const fee,
|
||||
std::optional<std::reference_wrapper<Transactor>> txCheck,
|
||||
std::optional<std::reference_wrapper<TxInvariantCheck>> txCheck,
|
||||
std::index_sequence<Is...>)
|
||||
{
|
||||
auto checkers = getInvariantChecks();
|
||||
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().visitInvariantEntry(isDelete, before, after);
|
||||
txCheck->get().visitEntry(isDelete, before, after);
|
||||
(..., std::get<Is>(checkers).visitEntry(isDelete, before, after));
|
||||
});
|
||||
|
||||
if (txCheck)
|
||||
{
|
||||
if (!txCheck->get().finalizeInvariants(ctx.tx, result, fee, ctx.view(), ctx.journal))
|
||||
if (!txCheck->get().finalize(ctx.tx, result, fee, ctx.view(), ctx.journal))
|
||||
{
|
||||
JLOG(ctx.journal.fatal())
|
||||
<< "Transaction has failed one or more transaction invariants: "
|
||||
@@ -69,12 +69,8 @@ checkInvariantsHelper(
|
||||
// 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<bool, sizeof...(Is)> const finalizers{{std::get<Is>(checkers).finalize(
|
||||
ctx.tx,
|
||||
result,
|
||||
fee,
|
||||
ctx.view(),
|
||||
ctx.journal)...}}; // NOLINT(bugprone-unchecked-optional-access)
|
||||
std::array<bool, sizeof...(Is)> const finalizers{
|
||||
{std::get<Is>(checkers).finalize(ctx.tx, result, fee, ctx.view(), ctx.journal)...}};
|
||||
|
||||
if (!std::all_of(finalizers.cbegin(), finalizers.cend(), [](auto const& b) { return b; }))
|
||||
{
|
||||
@@ -101,7 +97,7 @@ checkInvariants(
|
||||
ApplyContext& ctx,
|
||||
TER const result,
|
||||
XRPAmount const fee,
|
||||
std::optional<std::reference_wrapper<Transactor>> txCheck)
|
||||
std::optional<std::reference_wrapper<TxInvariantCheck>> txCheck)
|
||||
{
|
||||
XRPL_ASSERT(
|
||||
isTesSuccess(result) || isTecClaim(result),
|
||||
|
||||
@@ -21,6 +21,7 @@
|
||||
#include <xrpl/beast/utility/Zero.h>
|
||||
#include <xrpl/ledger/ApplyView.h>
|
||||
#include <xrpl/ledger/OpenView.h>
|
||||
#include <xrpl/ledger/ReadView.h>
|
||||
#include <xrpl/ledger/helpers/AccountRootHelpers.h>
|
||||
#include <xrpl/ledger/helpers/DirectoryHelpers.h>
|
||||
#include <xrpl/ledger/helpers/RippleStateHelpers.h>
|
||||
@@ -54,6 +55,7 @@
|
||||
#include <xrpl/tx/applySteps.h>
|
||||
#include <xrpl/tx/invariants/AMMInvariant.h>
|
||||
#include <xrpl/tx/invariants/DirectoryInvariant.h>
|
||||
#include <xrpl/tx/invariants/InvariantRunner.h>
|
||||
#include <xrpl/tx/invariants/VaultInvariant.h>
|
||||
|
||||
#include <algorithm>
|
||||
@@ -64,6 +66,7 @@
|
||||
#include <initializer_list>
|
||||
#include <memory>
|
||||
#include <optional>
|
||||
#include <stdexcept>
|
||||
#include <string>
|
||||
#include <tuple>
|
||||
#include <utility>
|
||||
@@ -5916,6 +5919,54 @@ class Invariants_test : public beast::unit_test::Suite
|
||||
}
|
||||
}
|
||||
|
||||
void
|
||||
testTxCheckException()
|
||||
{
|
||||
testcase << "txCheck exception";
|
||||
using namespace jtx;
|
||||
|
||||
// A TxInvariantCheck that always throws from finalize, 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).
|
||||
struct ThrowingTxInvariantCheck : TxInvariantCheck
|
||||
{
|
||||
void
|
||||
visitEntry(bool, SLE::const_ref, SLE::const_ref) override
|
||||
{
|
||||
}
|
||||
|
||||
[[nodiscard]] bool
|
||||
finalize(STTx const&, TER, XRPAmount, ReadView const&, beast::Journal const&) override
|
||||
{
|
||||
throw std::runtime_error("test-injected txCheck exception");
|
||||
}
|
||||
};
|
||||
|
||||
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());
|
||||
|
||||
ThrowingTxInvariantCheck throwing;
|
||||
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
|
||||
testConfidentialMPTTransfer()
|
||||
{
|
||||
@@ -6200,6 +6251,7 @@ public:
|
||||
testAMM();
|
||||
testObjectHasPseudoAccount();
|
||||
testSponsorship();
|
||||
testTxCheckException();
|
||||
}
|
||||
};
|
||||
|
||||
|
||||
Reference in New Issue
Block a user