refactor: Address review feedback on CheckInvariants (#7404)

- Document that `after` is never null in InvariantCheck::visitEntry
- Clarify tec/tef escalation in checkInvariants doc: both protocol and
  transaction faults return tecINVARIANT_FAILED on the first pass;
  tefINVARIANT_FAILED only occurs when the incoming result is already
  tecINVARIANT_FAILED (i.e. the fee-claim reset path checks again)
- Rename SkipTxInvariants -> CheckTxInvariants with Yes/No semantics
  to eliminate the double-negative at call sites

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);
 
     /////////////////////////////////////////////////////
     /*

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

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,

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));