refactor: Validate visitEntry SLE nullability via InvariantEntry

include/xrpl/tx/Transactor.h

@@ -20,6 +20,7 @@
 #include <xrpl/protocol/XRPAmount.h>
 #include <xrpl/tx/ApplyContext.h>
 #include <xrpl/tx/applySteps.h>
+#include <xrpl/tx/invariants/InvariantEntry.h>
 #include <xrpl/tx/invariants/InvariantRunner.h>
 
 #include <cstddef>
@@ -578,9 +579,9 @@ class Transactor : public TxInvariantCheck
      * ordering is enforced.
      */
     void
-    visitEntry(bool isDelete, SLE::const_ref before, SLE::const_ref after) final
+    visitEntry(InvariantEntry const& entry) final
     {
-        visitInvariantEntry(isDelete, before, after);
+        visitInvariantEntry(entry.isDelete(), entry.before(), entry.after());
     }
 
     [[nodiscard]] bool

include/xrpl/tx/invariants/AMMInvariant.h

@@ -4,10 +4,10 @@
 #include <xrpl/ledger/ReadView.h>
 #include <xrpl/protocol/AccountID.h>
 #include <xrpl/protocol/STAmount.h>
-#include <xrpl/protocol/STLedgerEntry.h>
 #include <xrpl/protocol/STTx.h>
 #include <xrpl/protocol/TER.h>
 #include <xrpl/protocol/XRPAmount.h>
+#include <xrpl/tx/invariants/InvariantEntry.h>
 
 #include <optional>
 
@@ -27,7 +27,7 @@ class ValidAMM
 
     ValidAMM() = default;
     void
-    visitEntry(bool, SLE::const_ref, SLE::const_ref);
+    visitEntry(InvariantEntry const&);
 
     bool
     finalize(STTx const&, TER const, XRPAmount const, ReadView const&, beast::Journal const&);

include/xrpl/tx/invariants/DirectoryInvariant.h

@@ -4,12 +4,10 @@
 #include <xrpl/basics/base_uint.h>
 #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 <memory>
+#include <xrpl/tx/invariants/InvariantEntry.h>
 
 namespace xrpl {
 
@@ -20,7 +18,7 @@ class ValidBookDirectory
 
 public:
     void
-    visitEntry(bool, std::shared_ptr<SLE const> const&, std::shared_ptr<SLE const> const&);
+    visitEntry(InvariantEntry const&);
 
     bool
     finalize(STTx const&, TER const, XRPAmount const, ReadView const&, beast::Journal const&);

include/xrpl/tx/invariants/FreezeInvariant.h

@@ -10,6 +10,7 @@
 #include <xrpl/protocol/STTx.h>
 #include <xrpl/protocol/TER.h>
 #include <xrpl/protocol/XRPAmount.h>
+#include <xrpl/tx/invariants/InvariantEntry.h>
 
 #include <map>
 #include <optional>
@@ -44,7 +45,7 @@ class TransfersNotFrozen
 
 public:
     void
-    visitEntry(bool, SLE::const_ref, SLE::const_ref);
+    visitEntry(InvariantEntry const&);
 
     bool
     finalize(STTx const&, TER const, XRPAmount const, ReadView const&, beast::Journal const&);

include/xrpl/tx/invariants/InvariantCheck.h

@@ -9,6 +9,7 @@
 #include <xrpl/tx/invariants/AMMInvariant.h>
 #include <xrpl/tx/invariants/DirectoryInvariant.h>
 #include <xrpl/tx/invariants/FreezeInvariant.h>
+#include <xrpl/tx/invariants/InvariantEntry.h>
 #include <xrpl/tx/invariants/LoanBrokerInvariant.h>
 #include <xrpl/tx/invariants/LoanInvariant.h>
 #include <xrpl/tx/invariants/MPTInvariant.h>
@@ -71,21 +72,13 @@ class InvariantChecker_PROTOTYPE
     /**
      * @brief called for each ledger entry in the current transaction.
      *
-     * @param isDelete true if the SLE is being deleted.
-     * @param before ledger entry before modification by the transaction. `before` will be null if
-     *  the entry is new.
-     * @param after ledger entry after modification by the transaction. Always non-null. When
-     *  deleting, `after` may differ from `before`. Whether that is important is up to the
-     *  individual invariant check.
+     * @param entry validated, non-owning view of the modified ledger entry.
      *
-     * @note `after` IS NEVER NULL. `isDelete` is the only correct way to check for deletions.
-     *  Do not make logic or branching decisions on whether on `after` is set, because it will
-     *  always be set. Treat a null `after` as a programming error (with XRPL_ASSERT). An
-     *  invariant MAY check for null defensively, if it makes more sense, but an assertion is
-     *  preferred for new invariants.
+     * @note `entry.after()` IS NEVER NULL. `entry.isDelete()` is the only
+     * correct way to check for deletions.
      */
     void
-    visitEntry(bool isDelete, SLE::const_ref before, SLE::const_ref after);
+    visitEntry(InvariantEntry const& entry);
 
     /**
      * @brief called after all ledger entries have been visited to determine
@@ -123,7 +116,7 @@ class TransactionFeeCheck
 {
 public:
     void
-    visitEntry(bool, SLE::const_ref, SLE::const_ref);
+    visitEntry(InvariantEntry const&);
 
     static bool
     finalize(STTx const&, TER const, XRPAmount const, ReadView const&, beast::Journal const&);
@@ -143,7 +136,7 @@ class XRPNotCreated
 
 public:
     void
-    visitEntry(bool, SLE::const_ref, SLE::const_ref);
+    visitEntry(InvariantEntry const&);
 
     [[nodiscard]] bool
     finalize(STTx const&, TER const, XRPAmount const, ReadView const&, beast::Journal const&) const;
@@ -163,7 +156,7 @@ class AccountRootsNotDeleted
 
 public:
     void
-    visitEntry(bool, SLE::const_ref, SLE::const_ref);
+    visitEntry(InvariantEntry const&);
 
     [[nodiscard]] bool
     finalize(STTx const&, TER const, XRPAmount const, ReadView const&, beast::Journal const&) const;
@@ -190,7 +183,7 @@ class AccountRootsDeletedClean
 
 public:
     void
-    visitEntry(bool, SLE::const_ref, SLE::const_ref);
+    visitEntry(InvariantEntry const&);
 
     bool
     finalize(STTx const&, TER const, XRPAmount const, ReadView const&, beast::Journal const&);
@@ -209,7 +202,7 @@ class XRPBalanceChecks
 
 public:
     void
-    visitEntry(bool, SLE::const_ref, SLE::const_ref);
+    visitEntry(InvariantEntry const&);
 
     [[nodiscard]] bool
     finalize(STTx const&, TER const, XRPAmount const, ReadView const&, beast::Journal const&) const;
@@ -226,7 +219,7 @@ class LedgerEntryTypesMatch
 
 public:
     void
-    visitEntry(bool, SLE::const_ref, SLE::const_ref);
+    visitEntry(InvariantEntry const&);
 
     [[nodiscard]] bool
     finalize(STTx const&, TER const, XRPAmount const, ReadView const&, beast::Journal const&) const;
@@ -244,7 +237,7 @@ class NoXRPTrustLines
 
 public:
     void
-    visitEntry(bool, SLE::const_ref, SLE::const_ref);
+    visitEntry(InvariantEntry const&);
 
     [[nodiscard]] bool
     finalize(STTx const&, TER const, XRPAmount const, ReadView const&, beast::Journal const&) const;
@@ -263,7 +256,7 @@ class NoDeepFreezeTrustLinesWithoutFreeze
 
 public:
     void
-    visitEntry(bool, SLE::const_ref, SLE::const_ref);
+    visitEntry(InvariantEntry const&);
 
     [[nodiscard]] bool
     finalize(STTx const&, TER const, XRPAmount const, ReadView const&, beast::Journal const&) const;
@@ -282,7 +275,7 @@ class NoBadOffers
 
 public:
     void
-    visitEntry(bool, SLE::const_ref, SLE::const_ref);
+    visitEntry(InvariantEntry const&);
 
     [[nodiscard]] bool
     finalize(STTx const&, TER const, XRPAmount const, ReadView const&, beast::Journal const&) const;
@@ -298,7 +291,7 @@ class NoZeroEscrow
 
 public:
     void
-    visitEntry(bool, SLE::const_ref, SLE::const_ref);
+    visitEntry(InvariantEntry const&);
 
     [[nodiscard]] bool
     finalize(STTx const&, TER const, XRPAmount const, ReadView const&, beast::Journal const&) const;
@@ -318,7 +311,7 @@ class ValidNewAccountRoot
 
 public:
     void
-    visitEntry(bool, SLE::const_ref, SLE::const_ref);
+    visitEntry(InvariantEntry const&);
 
     [[nodiscard]] bool
     finalize(STTx const&, TER const, XRPAmount const, ReadView const&, beast::Journal const&) const;
@@ -348,7 +341,7 @@ class ValidClawback
 
 public:
     void
-    visitEntry(bool, SLE::const_ref, SLE::const_ref);
+    visitEntry(InvariantEntry const&);
 
     [[nodiscard]] bool
     finalize(STTx const&, TER const, XRPAmount const, ReadView const&, beast::Journal const&) const;
@@ -368,7 +361,7 @@ class ValidPseudoAccounts
 
 public:
     void
-    visitEntry(bool, SLE::const_ref, SLE::const_ref);
+    visitEntry(InvariantEntry const&);
 
     bool
     finalize(STTx const&, TER const, XRPAmount const, ReadView const&, beast::Journal const&);
@@ -388,7 +381,7 @@ class NoModifiedUnmodifiableFields
 
 public:
     void
-    visitEntry(bool, SLE::const_ref, SLE::const_ref);
+    visitEntry(InvariantEntry const&);
 
     bool
     finalize(STTx const&, TER const, XRPAmount const, ReadView const&, beast::Journal const&);
@@ -404,7 +397,7 @@ class ValidAmounts
 
 public:
     void
-    visitEntry(bool, SLE::const_ref, SLE::const_ref);
+    visitEntry(InvariantEntry const&);
 
     [[nodiscard]] bool
     finalize(STTx const&, TER const, XRPAmount const, ReadView const&, beast::Journal const&) const;
@@ -421,7 +414,7 @@ class ObjectHasPseudoAccount
 {
 public:
     void
-    visitEntry(bool, SLE::const_ref, SLE::const_ref);
+    visitEntry(InvariantEntry const&);
 
     [[nodiscard]] bool
     finalize(STTx const&, TER const, XRPAmount const, ReadView const&, beast::Journal const&) const;

include/xrpl/tx/invariants/InvariantEntry.h

@@ -0,0 +1,53 @@
+#pragma once
+
+#include <xrpl/basics/contract.h>
+#include <xrpl/protocol/STLedgerEntry.h>
+
+#include <stdexcept>
+#include <utility>
+
+namespace xrpl {
+
+/**
+ * A validated ledger entry visited by invariants.
+ */
+class InvariantEntry
+{
+    bool isDelete_;
+    SLE::const_pointer before_;
+    SLE::const_pointer after_;
+
+public:
+    InvariantEntry(bool isDelete, SLE::const_pointer before, SLE::const_pointer after)
+        : isDelete_(isDelete), before_(std::move(before)), after_(std::move(after))
+    {
+        if (after_ == nullptr)
+            Throw<std::logic_error>("InvariantEntry: after is never null");
+        if (isDelete_ && before_ == nullptr)
+            Throw<std::logic_error>("InvariantEntry: deleted entry missing before state");
+    }
+
+    InvariantEntry(InvariantEntry const&) = delete;
+    InvariantEntry&
+    operator=(InvariantEntry const&) = delete;
+
+    [[nodiscard]] bool
+    isDelete() const
+    {
+        return isDelete_;
+    }
+
+    [[nodiscard]] SLE::const_ref
+    before() const
+    {
+        return before_;
+    }
+
+    [[nodiscard]] SLE::const_ref
+    after() const
+    {
+        return after_;
+    }
+};
+
+}  // namespace xrpl

include/xrpl/tx/invariants/InvariantRunner.h

@@ -2,11 +2,11 @@
 
 #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>
+#include <xrpl/tx/invariants/InvariantEntry.h>
 
 #include <functional>
 #include <optional>
@@ -66,16 +66,10 @@ class TxInvariantCheck
     /**
      * @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
-     *                 a null @p after to detect deletions.  @p after is
-     *                 never null.
+     * @param entry a validated, non-owning view of the modified entry.
      */
     virtual void
-    visitEntry(bool isDelete, SLE::const_ref before, SLE::const_ref after) = 0;
+    visitEntry(InvariantEntry const& entry) = 0;
 
     /**
      * @brief Called after all entries have been visited.

include/xrpl/tx/invariants/LoanBrokerInvariant.h

@@ -7,6 +7,7 @@
 #include <xrpl/protocol/STTx.h>
 #include <xrpl/protocol/TER.h>
 #include <xrpl/protocol/XRPAmount.h>
+#include <xrpl/tx/invariants/InvariantEntry.h>
 
 #include <map>
 #include <vector>
@@ -48,7 +49,7 @@ class ValidLoanBroker
 
 public:
     void
-    visitEntry(bool, SLE::const_ref, SLE::const_ref);
+    visitEntry(InvariantEntry const&);
 
     bool
     finalize(STTx const&, TER const, XRPAmount const, ReadView const&, beast::Journal const&);

include/xrpl/tx/invariants/LoanInvariant.h

@@ -6,6 +6,7 @@
 #include <xrpl/protocol/STTx.h>
 #include <xrpl/protocol/TER.h>
 #include <xrpl/protocol/XRPAmount.h>
+#include <xrpl/tx/invariants/InvariantEntry.h>
 
 #include <utility>
 #include <vector>
@@ -28,7 +29,7 @@ class ValidLoan
 
 public:
     void
-    visitEntry(bool, SLE::const_ref, SLE::const_ref);
+    visitEntry(InvariantEntry const&);
 
     bool
     finalize(STTx const&, TER const, XRPAmount const, ReadView const&, beast::Journal const&);