diff --git a/include/xrpl/tx/invariants/CheckInvariants.h b/include/xrpl/tx/invariants/CheckInvariants.h index fd014e6055..33aab7d387 100644 --- a/include/xrpl/tx/invariants/CheckInvariants.h +++ b/include/xrpl/tx/invariants/CheckInvariants.h @@ -101,17 +101,11 @@ public: * `visitEntry` accumulates state on the same traversal that drives the * protocol checkers, then both layers' `finalize` run on the complete state. * - * 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 - * failed results gracefully, so passing the same @p txCheck after a fee-claim - * reset is safe. + * 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. diff --git a/src/libxrpl/tx/Transactor.cpp b/src/libxrpl/tx/Transactor.cpp index 9694d5cfa5..e87cccb3e5 100644 --- a/src/libxrpl/tx/Transactor.cpp +++ b/src/libxrpl/tx/Transactor.cpp @@ -45,7 +45,7 @@ #include #include -#include +#include #include #include #include diff --git a/src/libxrpl/tx/invariants/CheckInvariants.cpp b/src/libxrpl/tx/invariants/CheckInvariants.cpp index 92f7f75ee2..ca58641494 100644 --- a/src/libxrpl/tx/invariants/CheckInvariants.cpp +++ b/src/libxrpl/tx/invariants/CheckInvariants.cpp @@ -17,7 +17,6 @@ #include #include #include -#include #include #include @@ -43,133 +42,55 @@ checkInvariantsHelper( std::index_sequence) { auto checkers = getInvariantChecks(); + bool allOk = true; - // Phase 1 — state collection. - // One walk feeds both layers. Per-layer try-catch isolates faults: a throw - // in txCheck stops only txCheck from visiting further entries; the protocol - // fold keeps going (and vice-versa). A layer that threw skips finalize. - bool txCollectionOk = true; - bool protoCollectionOk = true; - std::string txCollectionEx; - std::string protoCollectionEx; - - ctx.visit([&](uint256 const&, bool isDelete, SLE::const_ref before, SLE::const_ref after) { - if (txCheck && txCollectionOk) - { - try - { + try + { + ctx.visit([&](uint256 const&, bool isDelete, SLE::const_ref before, SLE::const_ref after) { + if (txCheck) txCheck->get().visitEntry(isDelete, before, after); - } - catch (std::exception const& ex) - { - txCollectionOk = false; - txCollectionEx = ex.what(); - } - } + (..., std::get(checkers).visitEntry(isDelete, before, after)); + }); - if (protoCollectionOk) - { - try - { - (..., std::get(checkers).visitEntry(isDelete, before, after)); - } - catch (std::exception const& ex) - { - protoCollectionOk = false; - protoCollectionEx = ex.what(); - } - } - }); - - // Phase 2 — evaluate invariant conditions. - auto const txResult = [&]() -> TER { - if (!txCheck) - return result; - - if (!txCollectionOk) - { - JLOG(ctx.journal.fatal()) - << "Transaction caused an exception while collecting transaction invariant state" - << ", ex: " << txCollectionEx - << ", tx: " << to_string(ctx.tx.getJson(JsonOptions::Values::None)); - return tecINVARIANT_FAILED; - } - - try + 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)); - return tecINVARIANT_FAILED; + allOk = false; } } - catch (std::exception const& ex) + + // 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)...}}; // NOLINT(bugprone-unchecked-optional-access) + + if (!std::all_of(finalizers.cbegin(), finalizers.cend(), [](auto const& b) { return b; })) { - JLOG(ctx.journal.fatal()) - << "Transaction caused an exception in a transaction invariant" - << ", ex: " << ex.what() - << ", tx: " << to_string(ctx.tx.getJson(JsonOptions::Values::None)); - return tecINVARIANT_FAILED; + 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 result; - }(); - - auto const protoResult = [&]() -> TER { - if (!protoCollectionOk) - { - JLOG(ctx.journal.fatal()) - << "Transaction caused an exception while collecting global invariant state" - << ", ex: " << protoCollectionEx - << ", tx: " << to_string(ctx.tx.getJson(JsonOptions::Values::None)); - return failInvariantCheck(result); - } - - bool protoOk = true; - try - { - // 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)...}}; // NOLINT(bugprone-unchecked-optional-access) - - protoOk = std::all_of( - finalizers.cbegin(), finalizers.cend(), [](auto const& b) { return b; }); - if (!protoOk) - { - JLOG(ctx.journal.fatal()) - << "Transaction has failed one or more global invariants: " - << to_string(ctx.tx.getJson(JsonOptions::Values::None)); - } - } - catch (std::exception const& ex) - { - JLOG(ctx.journal.fatal()) - << "Transaction caused an exception in a global invariant" - << ", ex: " << ex.what() - << ", tx: " << to_string(ctx.tx.getJson(JsonOptions::Values::None)); - protoOk = false; - } - - return protoOk ? result : failInvariantCheck(result); - }(); - - // Fail if either check failed. tef (fatal) takes priority over tec. - if (protoResult == tefINVARIANT_FAILED) - return tefINVARIANT_FAILED; - if (txResult == tecINVARIANT_FAILED || protoResult == tecINVARIANT_FAILED) - return tecINVARIANT_FAILED; - - return result; + return allOk ? result : failInvariantCheck(result); } } // namespace