feat(node): drop the RPC block record on disconnect

Connection pushes a BlockRecord that RPC serves getblock from. Leaving it
after a disconnect means answering for a block the chain no longer
contains.

Removal is opportunistic on purpose. The vector is a best-effort
in-process cache, not authoritative state: it starts empty on every boot
while applied_tip resumes from a checkpoint at height N, and the prune
service removes records from it. An earlier version of the comment called
a missing tail a bug; making it one would refuse the first disconnect
after any restart. The hash check is what stops the pop from taking a
record that is not ours, and the tail can only be ours or gone, since
disconnect runs on the applied tip and connection pushed that tip's
record last.

Two entries leave the owed list. The transactions map never needed one:
connection does not populate it, which an existing test already pins.

Also records that retry is not a recovery strategy for the partial
failure window. Each UTXO operation looks idempotent on the set, but
undo_block runs through commit_adds_and_removes, which fires UtxoSet's
listener, and coin_stats is one. A second pass re-emits callbacks for
operations that changed nothing, so a cumulative listener double-counts
even where the set converges.

Author: metaphorics

crates/node/src/apply.rs

@@ -541,7 +541,8 @@ impl ApplyHandles {
 /// | `utxo`, `tx_index`, `applied_tip` | restored here |
 /// | `coin_stats` | **owed** — cumulative, so it drifts on every disconnect |
 /// | `filter_index`, `filter_header_cache` | **owed** — BIP157 headers chain, so a stale link corrupts the chain |
-/// | `blocks`, `transactions` | **owed** — RPC would keep serving the disconnected block |
+/// | `blocks` | restored here — RPC would otherwise keep serving the disconnected block |
+/// | `transactions` | nothing owed: connection never populates it |
 /// | `mempool` | **owed** once transaction relay exists; disconnected transactions belong back in it |
 /// | `block_tree` | retained deliberately — the header stays valid and known |
 /// | `block_body_store` | retained deliberately — the body is still a real block |
@@ -557,10 +558,14 @@ impl ApplyHandles {
 /// path can return an error after other shards already committed, so the set
 /// can be left partly undone.
 ///
-/// Retrying converges only if both rollbacks are idempotent. Each individual
-/// UTXO operation is (restoring a live output and removing an absent one are
-/// both no-ops), but neither that nor index rollback idempotence is proven, so
-/// the window is listed with the owed work above rather than claimed away.
+/// Retrying does not obviously converge. Each individual UTXO operation looks
+/// idempotent, since restoring a live output and removing an absent one are
+/// both no-ops on the set. The set is not the whole contract: `undo_block`
+/// runs through `commit_adds_and_removes`, which fires `UtxoSet`'s listener,
+/// and `coin_stats` is one. A second pass can re-emit callbacks for operations
+/// that changed nothing, so a cumulative listener double-counts even though the
+/// set converged. Retry is therefore not a recovery strategy until listener
+/// behaviour is settled, and the window stays with the owed work above.
 ///
 /// 1. Read and decode the undo record. Nothing is mutated until this succeeds,
 ///    so a missing or corrupt record costs nothing.
@@ -655,6 +660,28 @@ pub fn disconnect_block(
         .undo_block(&undo)
         .map_err(ApplyError::UtxoCommit)?;
 
+    // RPC serves blocks from this vector, so the disconnected block's record
+    // must go or `getblock` keeps answering for it.
+    //
+    // Absence is legitimate and must never be an error. This is a best-effort
+    // in-process cache, not authoritative state: it starts empty on every boot
+    // while `applied_tip` resumes from a checkpoint at height N, and pruning
+    // removes records from it. Failing a consensus rollback because an optional
+    // cache is empty would refuse the first disconnect after any restart.
+    //
+    // The hash check is what stops the pop from truncating a record that is not
+    // ours. The tail can only be ours or gone, because disconnect runs on the
+    // applied tip and connection pushed that tip's record last.
+    {
+        let mut blocks = handles.blocks.write();
+        if blocks
+            .last()
+            .is_some_and(|record| record.hash == block_hash)
+        {
+            blocks.pop();
+        }
+    }
+
     handles
         .applied_tip
         .store(Some(Arc::new(parent_tip.clone())));
@@ -4626,6 +4653,52 @@ mod consensus_rule_tests {
         Ok(())
     }
 
+    /// RPC reads blocks from `handles.blocks`. Leaving the entry there would let
+    /// `getblock` keep answering for a block the chain no longer contains.
+    #[test]
+    #[allow(clippy::arc_with_non_send_sync)]
+    fn disconnect_drops_the_rpc_block_record() -> Result<(), Box<dyn std::error::Error>> {
+        let genesis = bitcoin::blockdata::constants::genesis_block(bitcoin::Network::Regtest);
+        let utxo = Arc::new(UtxoSet::new());
+        let handles = apply_handles_without_tx_index(Network::Regtest, Arc::clone(&utxo));
+        let genesis_hash = Hash256::from_le_bytes(genesis.block_hash().as_byte_array());
+        let genesis_tip = applied_header_tip(&handles, genesis_hash, &genesis, 0)?;
+        handles.applied_tip.store(Some(Arc::new(genesis_tip)));
+        let records_before = handles.blocks.read().len();
+
+        let block = mined_block_with_prev_hash_and_transactions(
+            genesis.block_hash(),
+            vec![coinbase_transaction(1)],
+        )?;
+        let block_hash = Hash256::from_le_bytes(block.block_hash().as_byte_array());
+        apply_block(&handles, &block)?;
+        assert!(
+            handles
+                .blocks
+                .read()
+                .iter()
+                .any(|record| record.hash == block_hash),
+            "connection must publish the record this test then removes"
+        );
+
+        disconnect_block(&handles, &block)?;
+
+        assert!(
+            !handles
+                .blocks
+                .read()
+                .iter()
+                .any(|record| record.hash == block_hash),
+            "RPC must not keep serving a disconnected block"
+        );
+        assert_eq!(
+            handles.blocks.read().len(),
+            records_before,
+            "exactly the one record must go"
+        );
+        Ok(())
+    }
+
     /// An indexer that cannot roll back must stop the disconnect, not be
     /// skipped. Finishing would leave index rows describing a block the chain
     /// no longer contains, and queries would answer from them.

docs/solutions/architecture-patterns/node-reorg-execution-design.md

@@ -153,7 +153,7 @@ Open, and prerequisites for giving `disconnect_block` a caller:
 | `coin_stats` inverse feed | cumulative, so it drifts on every disconnect |
 | `filter_index` rollback | BIP157 headers chain, so a stale link corrupts the chain; `filter_header_cache` must be reset with it |
 | RPC caches | `blocks` and `transactions` would keep serving the disconnected block |
-| Rollback idempotence, both stores | `undo_block` is fallible and runs after the index rolls back. Worse, it is not all-or-nothing: `commit_adds_and_removes` walks shards and both its serial and parallel paths can fail after other shards committed, so the UTXO set can be left partly undone. Retry converges only if both rollbacks are idempotent. Prove it, make the UTXO commit atomic, or add compensation |
+| Rollback idempotence, both stores | `undo_block` is fallible and runs after the index rolls back. Worse, it is not all-or-nothing: `commit_adds_and_removes` walks shards and both its serial and parallel paths can fail after other shards committed, so the UTXO set can be left partly undone. Retry is not obviously a fix: each UTXO operation looks idempotent on the set, but `commit_adds_and_removes` fires `UtxoSet`'s listener, and `coin_stats` is one, so a second pass can re-emit callbacks for operations that changed nothing and a cumulative listener double-counts. Prove listener-level idempotence, make the UTXO commit atomic, or add compensation |
 
 Open, layer 4: