refactor: Make ApplyContext hold ApplyViewImpl directly

ApplyContext kept its view in a std::optional only so that discard() could
re-emplace it, since ApplyViewImpl is not assignable. Every access to the
view therefore needed a bugprone-unchecked-optional-access suppression and
getApplyViewContext() asserted that the optional was engaged.

ApplyViewImpl gains reset(), which clears the buffered state table (entries
and destroyed drops) through a new ApplyStateTable::clear() and disengages
the delivered amount, leaving the view as if freshly constructed over the
same base and flags. ApplyContext now holds a plain ApplyViewImpl member
initialized in the constructor and discard() calls reset(). The apply()
docstring now states that reset() is valid afterwards, since apply() does
not consume the buffered changes.

Gtests cover reset() for inserted, modified and erased entries, destroyed
drops and the delivered amount, that the view is reusable afterwards, that
reset() after apply() leaves nothing for a second apply(), and that
ApplyContext::discard() has the same effect through the context.

.clang-tidy

@@ -140,9 +140,5 @@ CheckOptions:
   readability-identifier-naming.GlobalFunctionIgnoredRegexp: "^(to_string|hash_append|tuple_hash)$"
 
 HeaderFilterRegex: '^.*/(tests?|xrpl|xrpld)/.*\.(h|hpp|ipp)$'
-# Skip headers reached via add_module's symlinks (<build-dir>/modules/xrpl.lib*/...).
-# verify_headers already checks each one under its real path,
-# so reporting it twice would also apply its fixes twice.
-# .ipp files are kept: they are only ever reached via the symlink.
-ExcludeHeaderFilterRegex: '^.*/protocol_autogen/.*\.(h|hpp)$|^.*\.pb\.h$|^.*/modules/xrpl\.lib[^/]+/.*\.(h|hpp)$'
+ExcludeHeaderFilterRegex: '^.*/protocol_autogen/.*\.(h|hpp)$|^.*\.pb\.h$'
 WarningsAsErrors: "*"

CONTRIBUTING.md

@@ -365,15 +365,14 @@ With this set, the hook will run as part of `git commit` alongside the other pre
 Then run clang-tidy on your local changes:
 
 ```
-run-clang-tidy -p build -allow-no-checks include src tests
+run-clang-tidy -p build -allow-no-checks src tests
 ```
 
-This will check all source files in the `include`, `src` and `tests` directories using the compile commands from your `build` directory.
-Headers are checked on their own, which requires the `verify_headers` CMake option (ON by default).
+This will check all source files in the `src`, `include` and `tests` directories using the compile commands from your `build` directory.
 If you wish to automatically fix whatever clang-tidy finds _and_ is capable of fixing, add `-fix -format` to the above command:
 
 ```
-run-clang-tidy -p build -quiet -fix -format -allow-no-checks include src tests
+run-clang-tidy -p build -quiet -fix -format -allow-no-checks src tests
 ```
 
 `-format` reformats the fixed code with [`.clang-format`](./.clang-format); without it the fixes are inserted in LLVM style and the `clang-format` hook rewrites them afterwards.

bin/pre-commit/clang_tidy_check.py

@@ -5,6 +5,15 @@ The script determines the staged files itself (see `pass_filenames: false` in
 .pre-commit-config.yaml) so run-clang-tidy is run once and handles parallelism
 internally: pre-commit would otherwise split the files across parallel hook
 invocations that race when fixes edit a shared header.
+
+Fixes are collected with `-export-fixes` and applied by clang-apply-replacements
+in a separate step rather than with run-clang-tidy's `-fix`. The `add_module`
+build isolates each module's headers behind a per-module symlink directory
+(build/modules/<module>/...), so a header reachable from several translation
+units is referenced through different paths that all resolve to the same source
+file. clang-apply-replacements deduplicates identical replacements by their
+literal path, so those paths must be canonicalised to the real source path
+first; otherwise the same fix is applied once per path and corrupts the header.
 """
 
 from __future__ import annotations
@@ -14,6 +23,7 @@ import re
 import shutil
 import subprocess
 import sys
+import tempfile
 from pathlib import Path
 
 # Extensions run-clang-tidy can analyse: `.cpp` translation units and, thanks to
@@ -21,6 +31,11 @@ from pathlib import Path
 # compile_commands.json entry). `.ipp` fragments have no entry and are skipped.
 TIDY_EXTENSIONS = {".cpp", ".h", ".hpp"}
 
+# A single-quoted `FilePath:` entry in an -export-fixes YAML file, allowing the
+# `- ` marker that precedes it inside a `Replacements:` sequence. clang-tidy
+# emits paths single-quoted and doubles any embedded quote per YAML rules.
+FILEPATH_RE = re.compile(r"^(\s*(?:-\s+)?FilePath:\s*)'((?:[^']|'')*)'\s*$")
+
 
 def clang_tidy_major() -> str | None:
     """Major version of the `clang-tidy` on PATH, which run-clang-tidy invokes."""
@@ -64,6 +79,25 @@ def staged_files(repo_root: Path) -> list[Path]:
     return [repo_root / rel for rel in output.splitlines() if rel]
 
 
+def canonicalize_fix_paths(fixes_dir: Path) -> None:
+    """Rewrite every `FilePath` in the exported fixes to its real source path.
+
+    A header included through a module's isolation symlink is recorded under that
+    symlink's path; collapsing all paths to the same real file lets
+    clang-apply-replacements recognise the per-translation-unit duplicates and
+    apply each fix once.
+    """
+    for yaml in fixes_dir.glob("*.yaml"):
+        lines = []
+        for line in yaml.read_text().splitlines():
+            if m := FILEPATH_RE.match(line):
+                path = m.group(2).replace("''", "'")
+                real = os.path.realpath(path).replace("'", "''")
+                line = f"{m.group(1)}'{real}'"
+            lines.append(line)
+        yaml.write_text("\n".join(lines) + "\n")
+
+
 def main():
     if not os.environ.get("TIDY"):
         return 0
@@ -82,10 +116,19 @@ def main():
 
     version = clang_tidy_major()
     run_clang_tidy = find_tool("run-clang-tidy", version)
-    if not run_clang_tidy:
+    clang_apply_replacements = find_tool("clang-apply-replacements", version)
+    missing = [
+        name
+        for name, path in (
+            ("run-clang-tidy", run_clang_tidy),
+            ("clang-apply-replacements", clang_apply_replacements),
+        )
+        if not path
+    ]
+    if missing:
         tried = f" (tried the '-{version}' suffix too)" if version else ""
         print(
-            f"clang-tidy check failed: TIDY is enabled but run-clang-tidy "
+            f"clang-tidy check failed: TIDY is enabled but {' and '.join(missing)} "
             f"was not found in PATH{tried}.",
             file=sys.stderr,
         )
@@ -100,19 +143,27 @@ def main():
         )
         return 1
 
-    result = subprocess.run(
-        [
-            run_clang_tidy,
-            "-quiet",
-            "-p",
-            build_dir,
-            "-fix",
-            "-format",
-            "-allow-no-checks",
-        ]
-        + files
-    )
-    return result.returncode
+    with tempfile.TemporaryDirectory() as fixes_dir:
+        result = subprocess.run(
+            [
+                run_clang_tidy,
+                "-quiet",
+                "-p",
+                build_dir,
+                "-export-fixes",
+                fixes_dir,
+                "-allow-no-checks",
+            ]
+            + files
+        )
+        canonicalize_fix_paths(Path(fixes_dir))
+        # `FormatStyle` in .clang-tidy does not reach this path,
+        # so ask for the repository style here.
+        applied = subprocess.run(
+            [clang_apply_replacements, "--format", "--style=file", fixes_dir]
+        )
+
+    return result.returncode or applied.returncode
 
 
 if __name__ == "__main__":

cmake/isolate_headers.cmake

@@ -34,12 +34,11 @@ include(create_symbolic_link)
 # sometimes `PUBLIC` and sometimes `PRIVATE`.
 # The Cs are all guaranteed to be unique.
 # We can guarantee a unique X per target by using
-# `${CMAKE_CURRENT_BINARY_DIR}/modules/${target}`.
+# `${CMAKE_CURRENT_BINARY_DIR}/include/${target}`.
 #
 # isolate_headers(target A B scope)
 function(isolate_headers target A B scope)
     file(RELATIVE_PATH C "${A}" "${B}")
-    # Keep in sync with ExcludeHeaderFilterRegex in .clang-tidy.
     set(X "${CMAKE_CURRENT_BINARY_DIR}/modules/${target}")
     set(Y "${X}/${C}")
     cmake_path(GET Y PARENT_PATH parent)

include/xrpl/ledger/ApplyViewImpl.h

@@ -41,9 +41,9 @@ public:
     /**
      * Apply the transaction.
      *
-     * After a call to `apply`, the only valid
-     * operation on this object is to call the
-     * destructor.
+     * The buffered changes stay in the view afterwards. The only valid
+     * operations on this object after `apply` are `reset()`, which makes it
+     * clean again, and the destructor.
      */
     std::optional<TxMeta>
     apply(
@@ -68,6 +68,15 @@ public:
         deliver_ = amount;
     }
 
+    /**
+     * Discard every buffered change and the delivered amount.
+     *
+     * Afterwards the view is indistinguishable from one freshly constructed
+     * over the same base with the same flags.
+     */
+    void
+    reset();
+
     /**
      * Get the number of modified entries
      */

include/xrpl/ledger/detail/ApplyStateTable.h

@@ -107,6 +107,15 @@ public:
     void
     destroyXRP(XRPAmount const& fee);
 
+    /**
+     * Drop every buffered modification and the destroyed drops.
+     *
+     * Afterwards the table is indistinguishable from a default-constructed
+     * one.
+     */
+    void
+    clear();
+
     // For debugging
     [[nodiscard]] XRPAmount const&
     dropsDestroyed() const

include/xrpl/ledger/detail/ApplyViewBase.h

@@ -106,6 +106,15 @@ public:
     rawDestroyXRP(XRPAmount const& feeDrops) override;
 
 protected:
+    /**
+     * Drop every buffered modification.
+     *
+     * Afterwards the view is indistinguishable from one freshly constructed
+     * over the same base with the same flags.
+     */
+    void
+    resetItems();
+
     ApplyFlags flags_;
     ReadView const* base_;
     detail::ApplyStateTable items_;

include/xrpl/tx/ApplyContext.h

@@ -63,20 +63,20 @@ public:
     ApplyView&
     view()
     {
-        return *view_;  // NOLINT(bugprone-unchecked-optional-access) view_ emplaced in constructor
+        return view_;
     }
 
     [[nodiscard]] ApplyView const&
     view() const
     {
-        return *view_;  // NOLINT(bugprone-unchecked-optional-access) view_ emplaced in constructor
+        return view_;
     }
 
     // VFALCO Unfortunately this is necessary
     RawView&
     rawView()
     {
-        return *view_;  // NOLINT(bugprone-unchecked-optional-access) view_ emplaced in constructor
+        return view_;
     }
 
     [[nodiscard]] ApplyFlags const&
@@ -91,8 +91,7 @@ public:
     void
     deliver(STAmount const& amount)
     {
-        // NOLINTNEXTLINE(bugprone-unchecked-optional-access) view_ emplaced in constructor
-        view_->deliver(amount);
+        view_.deliver(amount);
     }
 
     /**
@@ -126,8 +125,7 @@ public:
     void
     destroyXRP(XRPAmount const& fee)
     {
-        // NOLINTNEXTLINE(bugprone-unchecked-optional-access) view_ emplaced in constructor
-        view_->rawDestroyXRP(fee);
+        view_.rawDestroyXRP(fee);
     }
 
     /**
@@ -141,16 +139,14 @@ public:
     ApplyViewContext
     getApplyViewContext()
     {
-        XRPL_ASSERT(
-            view_.has_value(),
-            "xrpl::ApplyContext::getApplyViewContext : view_ emplaced in constructor");
-        return {.view = *view_, .tx = tx};
+        return {.view = view_, .tx = tx};
     }
 
 private:
     OpenView& base_;
     ApplyFlags flags_;
-    std::optional<ApplyViewImpl> view_;
+    // Declared after base_ and flags_, which its initializer uses.
+    ApplyViewImpl view_;
 
     // The ID of the batch transaction we are executing under, if set.
     std::optional<UInt256 const> parentBatchId_;

nix/packages.nix

@@ -62,8 +62,8 @@ let
   # version-suffixed name first and fall back to a system binary on the PATH
   # when Nix doesn't provide it:
   #   - Conan's Boost recipe looks up `g++-<major>` before plain `g++`.
-  #   - bin/pre-commit/clang_tidy_check.py looks up `run-clang-tidy-<v>`
-  #     before the unsuffixed name.
+  #   - bin/pre-commit/clang_tidy_check.py looks up `run-clang-tidy-<v>` and
+  #     `clang-apply-replacements-<v>` before the unsuffixed names.
   # On a host that also has the matching system binary
   # (e.g. Ubuntu's `/usr/bin/g++-<v>` or `clang-tidy-<v>`),
   # the probe escapes Nix and mixes a system tool into the Nix environment.

src/libxrpl/ledger/ApplyStateTable.cpp

@@ -530,6 +530,13 @@ ApplyStateTable::destroyXRP(XRPAmount const& fee)
     dropsDestroyed_ += fee;
 }
 
+void
+ApplyStateTable::clear()
+{
+    items_.clear();
+    dropsDestroyed_ = XRPAmount{0};
+}
+
 //------------------------------------------------------------------------------
 
 // Insert this transaction to the SLE's threading list

src/libxrpl/ledger/ApplyViewBase.cpp

@@ -19,6 +19,12 @@ ApplyViewBase::ApplyViewBase(ReadView const* base, ApplyFlags flags) : flags_(fl
 {
 }
 
+void
+ApplyViewBase::resetItems()
+{
+    items_.clear();
+}
+
 //---
 
 bool

src/libxrpl/ledger/ApplyViewImpl.cpp

@@ -33,6 +33,13 @@ ApplyViewImpl::apply(
     return items_.apply(to, tx, ter, deliver_, parentBatchId, isDryRun, j);
 }
 
+void
+ApplyViewImpl::reset()
+{
+    resetItems();
+    deliver_.reset();
+}
+
 std::size_t
 ApplyViewImpl::size()
 {

src/libxrpl/tx/ApplyContext.cpp

@@ -35,25 +35,24 @@ ApplyContext::ApplyContext(
     , journal(journal)
     , base_(base)
     , flags_(flags)
+    , view_(&base_, flags_)
     , parentBatchId_(parentBatchId)
 {
     XRPL_ASSERT(
         parentBatchId.has_value() == ((flags_ & TapBatch) == TapBatch),
         "Parent Batch ID should be set if batch apply flag is set");
-    view_.emplace(&base_, flags_);
 }
 
 void
 ApplyContext::discard()
 {
-    view_.emplace(&base_, flags_);
+    view_.reset();
 }
 
 std::optional<TxMeta>
 ApplyContext::apply(TER ter)
 {
-    // NOLINTNEXTLINE(bugprone-unchecked-optional-access) view_ emplaced in constructor
-    return view_->apply(base_, tx, ter, parentBatchId_, (flags_ & TapDryRun) != 0u, journal);
+    return view_.apply(base_, tx, ter, parentBatchId_, (flags_ & TapDryRun) != 0u, journal);
 }
 
 void
@@ -66,14 +65,14 @@ ApplyContext::addOrderBook(Book const& book)
 std::size_t
 ApplyContext::size()
 {
-    return view_->size();  // NOLINT(bugprone-unchecked-optional-access)
+    return view_.size();
 }
 
 void
 ApplyContext::visit(
     std::function<void(UInt256 const&, bool, SLE::ConstRef, SLE::ConstRef)> const& func)
 {
-    view_->visit(base_, func);  // NOLINT(bugprone-unchecked-optional-access)
+    view_.visit(base_, func);
 }
 
 }  // namespace xrpl

src/tests/libxrpl/ledger/ApplyViewImpl.cpp

@@ -0,0 +1,305 @@
+#include <xrpl/ledger/ApplyViewImpl.h>
+
+#include <xrpl/basics/base_uint.h>
+#include <xrpl/beast/utility/Journal.h>
+#include <xrpl/ledger/ApplyView.h>
+#include <xrpl/ledger/OpenView.h>
+#include <xrpl/ledger/RawView.h>
+#include <xrpl/ledger/ReadView.h>
+#include <xrpl/protocol/Indexes.h>
+#include <xrpl/protocol/SField.h>
+#include <xrpl/protocol/STAmount.h>
+#include <xrpl/protocol/STLedgerEntry.h>
+#include <xrpl/protocol/STTx.h>
+#include <xrpl/protocol/SeqProxy.h>
+#include <xrpl/protocol/Serializer.h>
+#include <xrpl/protocol/TER.h>
+#include <xrpl/protocol/TxMeta.h>
+#include <xrpl/protocol/XRPAmount.h>
+#include <xrpl/protocol_autogen/transactions/AccountSet.h>
+
+#include <gtest/gtest.h>
+#include <helpers/Account.h>
+#include <helpers/TxTest.h>
+
+#include <algorithm>
+#include <cstdint>
+#include <memory>
+#include <optional>
+#include <utility>
+#include <vector>
+
+namespace xrpl::test {
+
+namespace {
+
+/**
+ * Target that records what an OpenView hands down when it is applied, so a
+ * test can see which entries an ApplyViewImpl changed, how, and the drops it
+ * destroyed, without reaching into private state.
+ */
+class RecordingRawView final : public TxsRawView
+{
+public:
+    enum class Action { Erase, Insert, Replace };
+
+    XRPAmount destroyed{0};
+    std::vector<std::pair<Action, UInt256>> entries;
+
+    /**
+     * Whether an entry with @p key was recorded with @p action.
+     *
+     * @param action The raw operation to look for.
+     * @param key The entry's key.
+     * @return True if that exact operation on that key was recorded.
+     */
+    [[nodiscard]] bool
+    recorded(Action action, UInt256 const& key) const
+    {
+        return std::ranges::find(entries, std::pair{action, key}) != entries.end();
+    }
+
+    /**
+     * Whether any operation on @p key was recorded.
+     *
+     * @param key The entry's key.
+     * @return True if the key appears with any action.
+     */
+    [[nodiscard]] bool
+    touched(UInt256 const& key) const
+    {
+        return std::ranges::any_of(entries, [&](auto const& e) { return e.second == key; });
+    }
+
+    void
+    rawErase(SLE::Ref sle) override
+    {
+        entries.emplace_back(Action::Erase, sle->key());
+    }
+
+    void
+    rawInsert(SLE::Ref sle) override
+    {
+        entries.emplace_back(Action::Insert, sle->key());
+    }
+
+    void
+    rawReplace(SLE::Ref sle) override
+    {
+        entries.emplace_back(Action::Replace, sle->key());
+    }
+
+    void
+    rawDestroyXRP(XRPAmount const& fee) override
+    {
+        destroyed += fee;
+    }
+
+    void
+    rawTxInsert(
+        ReadView::key_type const&,
+        std::shared_ptr<Serializer const> const&,
+        std::shared_ptr<Serializer const> const&) override
+    {
+    }
+};
+
+}  // namespace
+
+/**
+ * A funded alice and bob over a closed ledger, plus the pieces needed to
+ * drive ApplyViewImpl::apply(): a signed transaction and a null journal.
+ */
+class ApplyViewImplTests : public ::testing::Test
+{
+protected:
+    TxTest env_;
+    Account const alice_{"alice"};
+    Account const bob_{"bob"};
+    beast::Journal const j_{beast::Journal::getNullSink()};
+
+    ApplyViewImplTests()
+    {
+        env_.createAccount(alice_, XRP(10'000));
+        env_.createAccount(bob_, XRP(10'000));
+    }
+
+    /**
+     * Build a signed AccountSet from alice, good enough for metadata.
+     *
+     * @return The transaction.
+     */
+    std::shared_ptr<STTx const>
+    makeTx()
+    {
+        transactions::AccountSetBuilder builder{alice_.id()};
+        builder.setSequence(env_.getAccountRoot(alice_.id()).getSequence());
+        builder.setFee(XRPAmount(10));
+        return builder.build(alice_.pk(), alice_.sk()).getSTTx();
+    }
+
+    /**
+     * Build a ticket entry that is not in the closed ledger.
+     *
+     * @param seq The raw ticket sequence.
+     * @return The new entry.
+     */
+    SLE::pointer
+    makeTicket(std::uint32_t seq) const
+    {
+        auto sle = std::make_shared<SLE>(keylet::ticket(alice_.id(), SeqProxy::rawTicket(seq)));
+        sle->setAccountID(sfAccount, alice_.id());
+        sle->setFieldU32(sfTicketSequence, seq);
+        sle->setFieldU64(sfOwnerNode, 0);
+        return sle;
+    }
+
+    /**
+     * Apply the view to a fresh OpenView over the closed ledger and then
+     * flush that OpenView into a recorder.
+     *
+     * @param av The view to apply.
+     * @param rec Receives what the OpenView applied.
+     * @return The metadata produced by the view.
+     */
+    std::optional<TxMeta>
+    applyAndRecord(ApplyViewImpl& av, RecordingRawView& rec)
+    {
+        auto const tx = makeTx();
+        if (!tx)
+        {
+            ADD_FAILURE() << "tx build failed";
+            return std::nullopt;
+        }
+        OpenView open(&env_.getClosedLedger());
+        auto meta = av.apply(open, *tx, tesSUCCESS, std::nullopt, false, j_);
+        open.apply(rec);
+        return meta;
+    }
+};
+
+TEST_F(ApplyViewImplTests, reset_drops_pending_state)
+{
+    ReadView const& base = env_.getClosedLedger();
+    ApplyViewImpl av(&base, TapRetry);
+
+    // Touch every kind of state the view buffers: an inserted entry, a
+    // modified entry, an erased entry, destroyed drops and a delivered amount.
+    auto const ticket = makeTicket(1);
+    av.rawInsert(ticket);
+    auto const account = av.peek(keylet::account(alice_.id()));
+    ASSERT_NE(account, nullptr);
+    auto const baseSeq = account->getFieldU32(sfSequence);
+    account->setFieldU32(sfSequence, baseSeq + 1);
+    av.update(account);
+    auto const bob = av.peek(keylet::account(bob_.id()));
+    ASSERT_NE(bob, nullptr);
+    av.erase(bob);
+    av.rawDestroyXRP(XRPAmount{10});
+    av.deliver(STAmount{XRPAmount{5}});
+
+    ASSERT_EQ(av.size(), 3);
+    ASSERT_TRUE(av.exists(keylet::unchecked(ticket->key())));
+    ASSERT_FALSE(av.exists(keylet::account(bob_.id())));
+
+    av.reset();
+
+    // Buffered changes are gone and reads fall through to the base again.
+    EXPECT_EQ(av.size(), 0);
+    EXPECT_FALSE(av.exists(keylet::unchecked(ticket->key())));
+    EXPECT_TRUE(av.exists(keylet::account(bob_.id())));
+    auto const reread = av.read(keylet::account(alice_.id()));
+    ASSERT_NE(reread, nullptr);
+    EXPECT_EQ(reread->getFieldU32(sfSequence), baseSeq);
+
+    // Base and flags are untouched.
+    EXPECT_EQ(av.flags(), TapRetry);
+    EXPECT_EQ(av.open(), base.open());
+    EXPECT_EQ(&av.header(), &base.header());
+    EXPECT_EQ(&av.fees(), &base.fees());
+
+    // Applying the reset view produces empty metadata, no delivered amount
+    // and no destroyed drops.
+    RecordingRawView rec;
+    auto const meta = applyAndRecord(av, rec);
+    ASSERT_TRUE(meta.has_value());
+    EXPECT_EQ(meta->getNodes().size(), 0);
+    EXPECT_FALSE(meta->getDeliveredAmount().has_value());
+    EXPECT_TRUE(rec.entries.empty());
+    EXPECT_EQ(rec.destroyed, XRPAmount{0});
+}
+
+TEST_F(ApplyViewImplTests, reset_after_apply_restores_a_clean_view)
+{
+    ApplyViewImpl av(&env_.getClosedLedger(), TapNone);
+
+    auto const ticket = makeTicket(1);
+    av.rawInsert(ticket);
+    av.rawDestroyXRP(XRPAmount{10});
+    STAmount const delivered{XRPAmount{5}};
+    av.deliver(delivered);
+
+    // apply() forwards the buffered changes but does not consume them.
+    RecordingRawView first;
+    auto const applied = applyAndRecord(av, first);
+    ASSERT_TRUE(applied.has_value());
+    ASSERT_TRUE(applied->getDeliveredAmount().has_value());
+    EXPECT_EQ(*applied->getDeliveredAmount(), delivered);
+    EXPECT_TRUE(first.recorded(RecordingRawView::Action::Insert, ticket->key()));
+    EXPECT_EQ(first.destroyed, XRPAmount{10});
+    EXPECT_EQ(av.size(), 1);
+
+    // reset() is the one operation besides destruction that is valid after
+    // apply(), and it leaves nothing behind for a second apply().
+    av.reset();
+    EXPECT_EQ(av.size(), 0);
+    EXPECT_FALSE(av.exists(keylet::unchecked(ticket->key())));
+
+    RecordingRawView second;
+    auto const meta = applyAndRecord(av, second);
+    ASSERT_TRUE(meta.has_value());
+    EXPECT_EQ(meta->getNodes().size(), 0);
+    EXPECT_FALSE(meta->getDeliveredAmount().has_value());
+    EXPECT_TRUE(second.entries.empty());
+    EXPECT_EQ(second.destroyed, XRPAmount{0});
+}
+
+TEST_F(ApplyViewImplTests, usable_again_after_reset)
+{
+    ApplyViewImpl av(&env_.getClosedLedger(), TapNone);
+
+    auto const stale = makeTicket(1);
+    av.rawInsert(stale);
+    av.rawDestroyXRP(XRPAmount{10});
+    av.deliver(STAmount{XRPAmount{5}});
+    av.reset();
+
+    // Only what happens after the reset is visible.
+    auto const fresh = makeTicket(2);
+    av.rawInsert(fresh);
+    av.rawDestroyXRP(XRPAmount{3});
+    STAmount const delivered{XRPAmount{7}};
+    av.deliver(delivered);
+
+    EXPECT_EQ(av.size(), 1);
+    EXPECT_FALSE(av.exists(keylet::unchecked(stale->key())));
+    EXPECT_TRUE(av.exists(keylet::unchecked(fresh->key())));
+
+    std::vector<UInt256> visited;
+    OpenView open(&env_.getClosedLedger());
+    av.visit(open, [&](UInt256 const& key, bool, SLE::ConstRef, SLE::ConstRef) {
+        visited.push_back(key);
+    });
+    EXPECT_EQ(visited, std::vector<UInt256>{fresh->key()});
+
+    RecordingRawView rec;
+    auto const meta = applyAndRecord(av, rec);
+    ASSERT_TRUE(meta.has_value());
+    ASSERT_TRUE(meta->getDeliveredAmount().has_value());
+    EXPECT_EQ(*meta->getDeliveredAmount(), delivered);
+    EXPECT_EQ(rec.destroyed, XRPAmount{3});
+    EXPECT_TRUE(rec.recorded(RecordingRawView::Action::Insert, fresh->key()));
+    EXPECT_FALSE(rec.touched(stale->key()));
+}
+
+}  // namespace xrpl::test

src/tests/libxrpl/tx/ApplyContext.cpp

@@ -0,0 +1,106 @@
+#include <xrpl/tx/ApplyContext.h>
+
+#include <xrpl/ledger/ApplyView.h>
+#include <xrpl/ledger/OpenView.h>
+#include <xrpl/protocol/Indexes.h>
+#include <xrpl/protocol/SField.h>
+#include <xrpl/protocol/STAmount.h>
+#include <xrpl/protocol/STLedgerEntry.h>
+#include <xrpl/protocol/STTx.h>
+#include <xrpl/protocol/SeqProxy.h>
+#include <xrpl/protocol/TER.h>
+#include <xrpl/protocol/TxMeta.h>
+#include <xrpl/protocol/XRPAmount.h>
+#include <xrpl/protocol_autogen/transactions/AccountSet.h>
+
+#include <gtest/gtest.h>
+#include <helpers/Account.h>
+#include <helpers/TxTest.h>
+
+#include <cstdint>
+#include <memory>
+
+namespace xrpl::test {
+
+/**
+ * A funded alice over an open ledger, plus a signed transaction to build an
+ * ApplyContext around.
+ */
+class ApplyContextTests : public ::testing::Test
+{
+protected:
+    TxTest env_;
+    Account const alice_{"alice"};
+    std::shared_ptr<STTx const> tx_;
+
+    ApplyContextTests()
+    {
+        env_.createAccount(alice_, XRP(10'000));
+        transactions::AccountSetBuilder builder{alice_.id()};
+        builder.setSequence(env_.getAccountRoot(alice_.id()).getSequence());
+        builder.setFee(XRPAmount(10));
+        tx_ = builder.build(alice_.pk(), alice_.sk()).getSTTx();
+    }
+
+    /**
+     * Build a ticket entry that is not in the ledger.
+     *
+     * @param seq The raw ticket sequence.
+     * @return The new entry.
+     */
+    SLE::pointer
+    makeTicket(std::uint32_t seq) const
+    {
+        auto sle = std::make_shared<SLE>(keylet::ticket(alice_.id(), SeqProxy::rawTicket(seq)));
+        sle->setAccountID(sfAccount, alice_.id());
+        sle->setFieldU32(sfTicketSequence, seq);
+        sle->setFieldU64(sfOwnerNode, 0);
+        return sle;
+    }
+};
+
+TEST_F(ApplyContextTests, discard_drops_pending_state_and_keeps_the_context_usable)
+{
+    ASSERT_NE(tx_, nullptr);
+    // A closed base makes apply() produce metadata, which an open ledger
+    // does not.
+    OpenView open(&env_.getClosedLedger());
+    ApplyContext ctx(env_.getServiceRegistry(), open, *tx_, tesSUCCESS, XRPAmount{10}, TapRetry);
+
+    // Buffer an inserted entry, a modified entry, destroyed drops and a
+    // delivered amount through the context's own surface.
+    auto const stale = makeTicket(1);
+    ctx.view().insert(stale);
+    auto const account = ctx.view().peek(keylet::account(alice_.id()));
+    ASSERT_NE(account, nullptr);
+    auto const baseSeq = account->getFieldU32(sfSequence);
+    account->setFieldU32(sfSequence, baseSeq + 1);
+    ctx.view().update(account);
+    ctx.destroyXRP(XRPAmount{10});
+    ctx.deliver(STAmount{XRPAmount{5}});
+    ASSERT_EQ(ctx.size(), 2);
+
+    ctx.discard();
+
+    // Reads fall through to the open ledger again and the flags survive.
+    EXPECT_EQ(ctx.size(), 0);
+    EXPECT_FALSE(ctx.view().exists(keylet::unchecked(stale->key())));
+    auto const reread = ctx.view().read(keylet::account(alice_.id()));
+    ASSERT_NE(reread, nullptr);
+    EXPECT_EQ(reread->getFieldU32(sfSequence), baseSeq);
+    EXPECT_EQ(ctx.view().flags(), TapRetry);
+
+    // Only changes made after the discard reach the open ledger.
+    auto const fresh = makeTicket(2);
+    ctx.view().insert(fresh);
+    auto const meta = ctx.apply(tesSUCCESS);
+    ASSERT_TRUE(meta.has_value());
+    EXPECT_FALSE(meta->getDeliveredAmount().has_value());
+    EXPECT_FALSE(open.exists(keylet::unchecked(stale->key())));
+    EXPECT_TRUE(open.exists(keylet::unchecked(fresh->key())));
+    auto const applied = open.read(keylet::account(alice_.id()));
+    ASSERT_NE(applied, nullptr);
+    EXPECT_EQ(applied->getFieldU32(sfSequence), baseSeq);
+}
+
+}  // namespace xrpl::test