docs: Improve documentation for InvariantCheck (#6518)

Removed redundant check for account creating trustline to itself.

include/xrpl/tx/invariants/InvariantCheck.h

@@ -27,6 +27,33 @@ namespace xrpl {
  * communicate the interface required of any invariant checker. Any invariant
  * check implementation should implement the public methods documented here.
  *
+ * ## Rules for implementing `finalize`
+ *
+ * ### Invariants must run regardless of transaction result
+ *
+ * An invariant's `finalize` method MUST perform meaningful checks even when
+ * the transaction has failed (i.e., `!isTesSuccess(tec)`). The following
+ * pattern is almost certainly wrong and must never be used:
+ *
+ * @code
+ * // WRONG: skipping all checks on failure defeats the purpose of invariants
+ * if (!isTesSuccess(tec))
+ *     return true;
+ * @endcode
+ *
+ * The entire purpose of invariants is to detect and prevent the impossible.
+ * A bug or exploit could cause a failed transaction to mutate ledger state in
+ * unexpected ways. Invariants are the last line of defense against such
+ * scenarios.
+ *
+ * In general: an invariant that expects a domain-specific state change to
+ * occur (e.g., a new object being created) should only expect that change
+ * when the transaction succeeded. A failed VaultCreate must not have created
+ * a Vault. A failed LoanSet must not have created a Loan.
+ *
+ * Also be aware that failed transactions, regardless of type, carry no
+ * Privileges. Any privilege-gated checks must therefore also be applied to
+ * failed transactions.
  */
 class InvariantChecker_PROTOTYPE
 {
@@ -48,7 +75,11 @@ public:
 
     /**
      * @brief called after all ledger entries have been visited to determine
-     * the final status of the check
+     * the final status of the check.
+     *
+     * This method MUST perform meaningful checks even when `tec` indicates a
+     * failed transaction. See the class-level documentation for the rules
+     * governing how failed transactions must be handled.
      *
      * @param tx the transaction being applied
      * @param tec the current TER result of the transaction

src/libxrpl/tx/transactors/dex/AMMWithdraw.cpp

@@ -676,7 +676,7 @@ AMMWithdraw::equalWithdrawTokens(
     STAmount const& lpTokens,
     STAmount const& lpTokensWithdraw,
     std::uint16_t tfee,
-    FreezeHandling freezeHanding,
+    FreezeHandling freezeHandling,
     WithdrawAll withdrawAll,
     XRPAmount const& priorBalance,
     beast::Journal const& journal)
@@ -697,7 +697,7 @@ AMMWithdraw::equalWithdrawTokens(
                 lptAMMBalance,
                 lpTokensWithdraw,
                 tfee,
-                freezeHanding,
+                freezeHandling,
                 WithdrawAll::Yes,
                 priorBalance,
                 journal);
@@ -731,7 +731,7 @@ AMMWithdraw::equalWithdrawTokens(
             lptAMMBalance,
             tokensAdj,
             tfee,
-            freezeHanding,
+            freezeHandling,
             withdrawAll,
             priorBalance,
             journal);

src/libxrpl/tx/transactors/dex/CreateOffer.cpp

@@ -214,8 +214,10 @@ CreateOffer::checkAcceptAsset(
         return (flags & tapRETRY) ? TER{terNO_ACCOUNT} : TER{tecNO_ISSUER};
     }
 
+    // An account cannot create a trustline to itself, so no line can exist
+    // to be frozen. Additionally, an issuer can always accept its own
+    // issuance.
     if (issue.account == id)
-        // An account can always accept its own issuance.
         return tesSUCCESS;
 
     if ((*issuerAccount)[sfFlags] & lsfRequireAuth)
@@ -242,14 +244,6 @@ CreateOffer::checkAcceptAsset(
         }
     }
 
-    // An account can not create a trustline to itself, so no line can exist
-    // to be frozen. Additionally, an issuer can always accept its own
-    // issuance.
-    if (issue.account == id)
-    {
-        return tesSUCCESS;
-    }
-
     auto const trustLine = view.read(keylet::line(id, issue.account, issue.currency));
 
     if (!trustLine)