diff --git a/include/xrpl/tx/Transactor.h b/include/xrpl/tx/Transactor.h index e1797b0ffc..ed71c55672 100644 --- a/include/xrpl/tx/Transactor.h +++ b/include/xrpl/tx/Transactor.h @@ -147,27 +147,27 @@ public: /** Whether to run the transaction-specific invariant check. * * After a fee-claim reset the transaction's effects have been rolled back, - * so only the protocol invariants are meaningful; pass @c Yes to skip the + * so only the protocol invariants are meaningful; pass @c No to omit the * transaction-specific check in that case. */ - enum class SkipTxInvariants : bool { No = false, Yes = true }; + enum class CheckTxInvariants : bool { No = false, Yes = true }; /** Check all invariants for the current transaction. * - * Delegates to the free @c xrpl::checkInvariants runner. Unless @p skip is - * @c SkipTxInvariants::Yes, the transaction-specific adapter is passed so + * 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. * Protocol faults (tefINVARIANT_FAILED) take priority over transaction * faults (tecINVARIANT_FAILED). * * @param result the tentative TER from transaction processing. * @param fee the fee consumed by the transaction. - * @param skip whether to skip the transaction-specific invariant check. + * @param check whether to include the transaction-specific invariant check. * * @return the final TER after all invariant checks. */ [[nodiscard]] TER - checkInvariants(TER result, XRPAmount fee, SkipTxInvariants skip); + checkInvariants(TER result, XRPAmount fee, CheckTxInvariants check); ///////////////////////////////////////////////////// /* diff --git a/include/xrpl/tx/invariants/CheckInvariants.h b/include/xrpl/tx/invariants/CheckInvariants.h index dd3db92188..fd014e6055 100644 --- a/include/xrpl/tx/invariants/CheckInvariants.h +++ b/include/xrpl/tx/invariants/CheckInvariants.h @@ -67,7 +67,8 @@ public: * 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. + * `after == nullptr` to detect deletions. @p after is + * never null. */ virtual void visitEntry(bool isDelete, SLE::const_ref before, SLE::const_ref after) = 0; @@ -99,8 +100,13 @@ public: * 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. - * Protocol faults (tefINVARIANT_FAILED) take precedence over transaction - * faults (tecINVARIANT_FAILED). + * + * On the first invariant pass both a protocol fault and a transaction fault + * return @c tecINVARIANT_FAILED. If that result triggers a fee-claim reset + * and invariants are checked again, the incoming @p result is already + * @c tecINVARIANT_FAILED; a second failure then escalates to + * @c tefINVARIANT_FAILED (via @c failInvariantCheck), which excludes the + * transaction from the ledger entirely. * * Every transaction is guaranteed to supply a @p txCheck (either a real check * or a no-op stub). The invariant contract requires @c finalize to handle diff --git a/src/libxrpl/tx/Transactor.cpp b/src/libxrpl/tx/Transactor.cpp index 56e032f03e..9694d5cfa5 100644 --- a/src/libxrpl/tx/Transactor.cpp +++ b/src/libxrpl/tx/Transactor.cpp @@ -1267,9 +1267,9 @@ Transactor::processPersistentChanges(TER result, XRPAmount fee) } [[nodiscard]] TER -Transactor::checkInvariants(TER result, XRPAmount fee, SkipTxInvariants skip) +Transactor::checkInvariants(TER result, XRPAmount fee, CheckTxInvariants check) { - if (skip == SkipTxInvariants::Yes) + if (check == CheckTxInvariants::No) return xrpl::checkInvariants(ctx_, result, fee); return xrpl::checkInvariants(ctx_, result, fee, std::ref(invariantCheck_)); @@ -1348,7 +1348,7 @@ Transactor::operator()() { // Check invariants: if `tecINVARIANT_FAILED` is not returned, we can // proceed to apply the tx - result = checkInvariants(result, fee, SkipTxInvariants::No); + result = checkInvariants(result, fee, CheckTxInvariants::Yes); if (result == tecINVARIANT_FAILED) { // Reset to fee-claim only @@ -1363,7 +1363,7 @@ Transactor::operator()() // the transaction's effects have been rolled back, so the // transaction-specific invariants are no longer meaningful. if (isTesSuccess(result) || isTecClaim(result)) - result = checkInvariants(result, fee, SkipTxInvariants::Yes); + result = checkInvariants(result, fee, CheckTxInvariants::No); } // We ran through the invariant checker, which can, in some cases, diff --git a/src/test/app/Invariants_test.cpp b/src/test/app/Invariants_test.cpp index 1ce325764d..6cc9f67023 100644 --- a/src/test/app/Invariants_test.cpp +++ b/src/test/app/Invariants_test.cpp @@ -201,7 +201,7 @@ class Invariants_test : public beast::unit_test::Suite for (TER const& terExpect : ters) { terActual = - transactor->checkInvariants(terActual, fee, Transactor::SkipTxInvariants::No); + transactor->checkInvariants(terActual, fee, Transactor::CheckTxInvariants::Yes); BEAST_EXPECTS( terExpect == terActual, "expected: " + transToken(terExpect) + " got: " + transToken(terActual));