fix: Re-store nodes missing from both backends during online_delete rotation

[sophiax851]

High Level Overview of Change

Fixes a silent durability gap in online delete: a node that is still referenced by the validated state map, but whose only on-disk copy was deleted with a previously rotated-out backend, is never rewritten to disk. The fix has two small, independent layers:

1. Prevention — during the rotation window (cache freshening through the backend swap), an ordinary read served by the archive backend is copied forward into the writable backend (DatabaseRotatingImp::fetchNodeObject). Today only duplicate == true fetches do this; a node fetched from the soon-to-be-deleted archive after the freshen snapshot would otherwise survive only in RAM once the archive is dropped.
2. Repair — the rotation copy walk (SHAMapStoreImp::copyNode) detects a walked node that misses in both backends and re-serializes the in-memory body into the writable backend, restoring the invariant that every node reachable from the validated ledger exists on disk.

Both layers mirror existing store patterns (duplicate copy-forward and SHAMap::writeNode serialize-then-store respectively); each logs on occurrence so operators can see the recovery happen.

Context of Change

Symptom

Affected nodes kept dropping in and out of sync with the network. Log analysis tied the desyncs to two surfaces of one condition:

1. Ledger close: transactions failed to apply against a missing state-tree node (SHAMapMissingNode); the locally built ledger diverged from the network-validated one, taking minutes to recover, repeatedly.
2. Online delete: the rotation copy walk halted on the same missing nodes, producing a repeated rotation-abort loop — online delete never completed and disk usage grew, while the continued rotation retries kept the node under load pressure, flapping in and out of sync until a full resync or node reversion.

Root cause

Verified step by step against the code:

1. The NodeStore is a flat, content-addressed store keyed by node hash. An unchanged node has exactly one on-disk copy shared by every ledger that references it.
2. At ledger close, SHAMap::flushDirty writes only dirty nodes (cowid != 0); walkSubTree skips clean children by design.
3. A clean node can stay clean across many ledgers (copy-on-write sharing), living in TreeNodeCache while its only on-disk copy sits in the old backend.
4. Online delete rotates backends and deletes the old archive — including that only copy.
5. Nothing rewrites the node: the flush path skips it (clean), and cache hits return without a write-back (Database::fetchNodeNT returns on cacheLookup success).
6. When the cache eventually evicts the node, the next fetch misses both backends → SHAMapMissingNode, with the entire clean subtree beneath it unreachable from disk.

Ruled out (by production instrumentation, kept out of this PR)

• Rotation race (write interleaved with the backend swap) — missing nodes were written long before the rotation window.
• Network desync — the node was in sync; the affected ledgers were validated locally.
• Backend I/O failure — no failed writes or read errors observed.
• Confirmed: missing nodes were clean (cowid == 0), present in TreeNodeCache, absent from both backends.

Why two layers

Prevention closes the exposure window that the copy walk cannot see. freshenCaches() takes a getKeys() snapshot of each cache and re-fetches those keys with duplicate == true, copying their archive bodies forward. But a node fetched from the archive after that snapshot — and canonicalized into a cache — generates no further disk reads, so nothing copies it forward before the swap deletes the archive. The change: SHAMapStoreImp sets a rotation-in-flight flag (setRotationInFlight, RAII-cleared on every exit path) just before freshening; while set, DatabaseRotatingImp::fetchNodeObject extends the existing duplicate copy-forward to ordinary reads served by the archive. Zero cost outside the window (one atomic load on the archive-hit path only); a per-rotation counter is logged at swap (Rotating: copied forward N archive-served reads…).

Repair (copyNode) handles nodes already stranded by past rotations. It is the only point that simultaneously holds the live in-memory node and proof that disk lost it (the both-backends fetch miss, !obj). The re-store:

• produces byte-identical output to a normal flush write (same serialization, hash key, AccountNode type, and store path as SHAMap::writeNode);
• cannot mutate the tree — the walked map is an immutable snapshot and the node arrives as const&;
• costs one result check per visited node; the store executes only in the miss case.

AccountNode is hardcoded deliberately: the state-map walk is the only call site that can reach this code. The transaction map is not exposed — it is built fresh per ledger, shares no clean nodes across ledgers, and is never walked by rotation. Parameterizing the type for a hypothetical future tx-map path was considered and rejected as speculative.

Type of Change

• Bug fix (non-breaking change which fixes an issue)

API Impact

No public API, protocol, or on-disk format changes; the stored bytes are identical to a normal flush write. One addition to the internal NodeStore::DatabaseRotating interface (setRotationInFlight), implemented only by DatabaseRotatingImp and called only by SHAMapStore.

Test Plan

• A deterministic unit test requires a private-access hook into the rotation walk; open to adding a friend-based test hook if maintainers prefer. Until then, the logs below are the production observables.
• Live validation (confirmed): the fixed binary was deployed on a stock node tracking mainnet, previously exhibiting the symptom, and ran real online-delete rotation cycles against the pass/fail lines set before deployment. Results:
  • the copy-forward summary fired with non-zero counts — the prevention layer is persisting archive-served reads during the rotation window;
  • rotation aborts dropped to zero: online delete completes every cycle, aside from deliberate healthWait() pauses that defer rotation while the node is catching up
  • no SHAMapMissingNode ledger-close failures or resulting desyncs since deployment;
  • guardrails held: no regression in rotation duration, job-queue depth, or sync stability.

What to watch in the logs:

• Rotating: copied forward N archive-served reads into the writable backend during the rotation window — one summary warn per rotation (suppressed when N=0); prevention layer working.
• copyNode: re-stored node missing from both backends, hash=... type=... — one warn per recovered node, emitted only in the miss branch, so it cannot flood; repair layer working. Expected to be rare and to trend to zero once nodes stranded by past rotations have been repaired.

Status: confirmed on a mainnet-tracking node that previously reproduced the issue — rotations complete cleanly and the missing-node symptom has not recurred.

[github-actions]

⚠️ This PR contains unsigned commits. To get your PR merged, please sign them. ⚠️

If only the most recent commit is unsigned, you can run:

1. Amend the commit: git commit --amend --no-edit -n -S
2. Overwrite the commit: git push --force-with-lease

If multiple commits are unsigned, you can run:

1. Go into interactive rebase mode: git rebase --interactive HEAD~<NUM_OF_COMMITS>, where NUM_OF_COMMITS is the number of most recent commits that will be available to edit.
2. Change "pick" to "edit" for the commits you need to sign, and then save and exit.
3. For each commit, run: git commit --amend --no-edit -n -S
4. Continue the rebase: git rebase --continue
5. Overwrite the commit(s): git push --force-with-lease

If you're new to commit signing, there are different ways to set it up:

Sign commits with gpg

Follow the steps below to set up commit signing with gpg:

1. Generate a GPG key
2. Add the GPG key to your GitHub account
3. Configure git to use your GPG key for commit signing

Sign commits with ssh-agent

Follow the steps below to set up commit signing with ssh-agent:

1. Generate an SSH key and add it to ssh-agent
2. Add the SSH key to your GitHub account
3. Configure git to use your SSH key for commit signing

Sign commits with 1Password

You can also sign commits using 1Password, which lets you sign commits with biometrics without the signing key leaving the local 1Password process.
See use 1Password to sign your commits.

[Copilot]

Pull request overview

This PR addresses a durability gap in online-delete backend rotation: when a node is reachable from the validated in-memory state map but has been deleted from disk due to an earlier backend rotation (and never rewritten because it remained “clean”), rotation could later fail and/or the node could surface as an unresolvable SHAMapMissingNode. The change makes the rotation copy walk detect a miss in both backends and re-store the in-memory node bytes into the writable backend, restoring the invariant that all nodes reachable from the validated ledger exist on disk.

Changes:

• In SHAMapStoreImp::copyNode, detect when DatabaseRotatingImp::fetchNodeObject(..., duplicate=true) returns null (missing from both writable and archive backends).
• Re-serialize the provided SHAMapTreeNode (with prefix) and store it as a NodeObjectType::AccountNode into the rotating database’s writable backend.
• Emit a warn log when this recovery path is taken to make the repair observable operationally.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[codecov]

Codecov Report

❌ Patch coverage is 60.00000% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.8%. Comparing base (cda63d0) to head (2eed1cc).

Files with missing lines	Patch %	Lines
src/xrpld/app/misc/SHAMapStoreImp.cpp	46.2%	7 Missing ⚠️
src/libxrpl/nodestore/DatabaseRotatingImp.cpp	75.0%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff            @@
##           develop   #7763     +/-   ##
=========================================
- Coverage     82.8%   82.8%   -0.0%     
=========================================
  Files         1036    1036             
  Lines        80067   80090     +23     
  Branches      8920    8928      +8     
=========================================
+ Hits         66282   66292     +10     
- Misses       13776   13789     +13     
  Partials         9       9             

Files with missing lines	Coverage Δ
include/xrpl/nodestore/DatabaseRotating.h	100.0% <ø> (ø)
...nclude/xrpl/nodestore/detail/DatabaseRotatingImp.h	66.7% <ø> (ø)
src/libxrpl/nodestore/DatabaseRotatingImp.cpp	64.6% <75.0%> (+1.1%)	⬆️
src/xrpld/app/misc/SHAMapStoreImp.cpp	69.5% <46.2%> (-1.0%)	⬇️

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[github-actions]

This PR has conflicts, please resolve them in order for the PR to be reviewed.

[github-actions]

All conflicts have been resolved. Assigned reviewers can now start or resume their review.

[ximinez]

This is good as-is. I think any further enhancements can come later.

[vlntb]

[nit] copyForwardCount_ can use std::memory_order_relaxed on both the increment (fetch_add) and the exchange(0). This counter is stats-only — its value feeds a single log line and gates no other memory access — so atomicity alone is sufficient. operator++ in particular currently defaults to seq_cst, which is the strongest (and most expensive) ordering. Switching to explicit fetch_add(1, std::memory_order_relaxed) is a small performance win.

[sophiax851]

Thanks. I made the change though AI says: "Performance: real but negligible here. On x86-64, a seq_cst and a relaxed fetch_add compile to the identical lock xadd instruction — zero difference. On ARM (Apple Silicon, Graviton) there is a genuine difference ( ldadd vs ldaddal with acquire-release semantics). But context matters: this increment executes only on archive-backend hits during a rotation window, on a path that just completed a backend disk fetch and is about to do a backend store. A saved fence is unmeasurable next to the I/O on either side of it. The exchange(0) runs once per rotation (roughly hourly) — irrelevant either way."

[vlntb]

Suggested change

	}
	XRPL_ASSERT(!dbRotating_->getRotationInFlight(),
	"SHAMapStoreImp::run : rotationInFlight_ must be false "
	"outside rotation window");

[vlntb]

The change makes sense and allows us to catch the gap earlier than we do right now.
I left a couple of suggestions to add runtime asserts so we can use Antithesis to verify the theory and the change - those are nits and only valuable once we start testing db rotation in Antithesis, so they can be added later.