Stop on a coin read error instead of treating the coin as missing

[bkeroack]

Problem

RocksDbStore answered a failed coin read with "no such coin":

• get_coin did self.db.get_cf(&cf, key).ok()??
• get_coins_batch did result.ok().flatten() for each multi_get_cf result
• has_coin matched only Ok(Some(_)), so an Err read as false
• a row that was present but did not decode was logged ("corrupt coin") and returned as None

CoinCache passes the inner store's answer straight through, and connect_block takes a missing coin as a verdict on the block that spends it: MissingOrSpentInput, bad-txns-inputs-missingorspent (chain/connect.rs). is_verdict_on_block() is true for it, so accept_block and the reorg path (reorg_to_naming_invalid) record the block as Invalid. That record outlives a restart. An I/O error or a checksum mismatch while connecting a valid block therefore left the node refusing that block, and every block built on it, until an operator ran reconsiderblock.

Bitcoin Core reads coins through CCoinsViewErrorCatcher. CDBWrapper::ReadImpl throws on any status other than NotFound (dbwrapper.cpp:305-316, HandleError), and ExecuteBackedWrapper catches it, logs "Error reading from database", runs the registered callback ("Error reading from database, shutting down.", init.cpp:1393) and calls std::abort() (coins.cpp:398-412). Core's comment gives the reason: returning to the caller "would be interpreted as 'entry not found' (as opposed to unable to read data), and could lead to invalid interpretation". GetCoin, HaveCoin and PeekCoin all go through it (coins.cpp:415-428).

Fix

All three coin reads in RocksDbStore (get_coin, get_coins_batch, has_coin) now go through one rule:

• no row: None (or false), as before;
• a read error, or a row that does not decode: stop_on_unreadable_coin, which logs Error reading from database, shutting down with the outpoint and the error (to the log and to stderr), then calls std::process::abort().

It stops with abort() as Core does, and as satd's other fail-stops do (fail_stop_on_unsafe_discard in chain/state.rs, the stall watchdog). No shutdown flush runs. The chainstate on disk is the one the last flush left, and the next start replays the blocks after it from the block files, as after any crash. As in fail_stop_on_unsafe_discard, a test build panics instead of aborting so the path can be tested.

The fix sits at the lowest layer, so every reader is covered: block connect, reorg, the IBD connector, prefetch, the AssumeUTXO background chainstate, the mempool, and the RPC, Electrum, Esplora and silent-payment lookups. In Core those all read through the same error catcher too. CoinCache and SplitStore delegate to this store, so they need no change.

The Operator Manual (ibd.md) gets a short section on what the stop looks like and what to do if it repeats after a restart.

Tests

New file node/src/storage/rocksdb_store_storefault_tests.rs. The read errors are real: the coin's SST file is flushed, then damaged in place on disk, and RocksDB fails the read with Corruption: block checksum mismatch.

• a_coin_read_error_stops_the_node_and_is_never_read_as_missing: on a damaged store, get_coin, get_coins_batch and has_coin each stop. An undamaged copy of the same store returns the coin from all three, so the reads do reach the damaged block.
  Fails without the fix: yes ("get_coin: a coin that cannot be read must stop the node, but the read returned None").
• an_undecodable_coin_row_stops_the_node_and_is_never_read_as_missing: a present row that does not decode stops get_coin and get_coins_batch. The coin next to it still reads.
  Fails without the fix: yes (same assertion, None).
• a_block_spending_a_coin_that_cannot_be_read_is_not_marked_invalid: ChainState over CoinCache over RocksDbStore, as the node runs. A block at height 1 spends the coin. Over an undamaged database it connects. Over the damaged one, accept_block stops, the block's index entry is not Invalid, and the tip does not move.
  Fails without the fix: yes (left: Some(Invalid), "a read error is not a verdict on the block (accept_block: Err(Connect(MissingOrSpentInput { .. })))").

Perturbations, one guard at a time, each restored afterwards:

• get_coin back to .ok()??: a_coin_read_error_… and an_undecodable_coin_row_… fail ("get_coin: … returned None").
• has_coin back to false on Err: a_coin_read_error_… fails ("has_coin: … returned false").
• get_coins_batch back to .ok().flatten(): both store-level tests fail ("get_coins_batch: … returned [None]").
• an undecodable row back to None: an_undecodable_coin_row_… fails.

The chain-level test fails only when both get_coins_batch and get_coin are reverted (the unfixed tree, above). connect_block falls back to get_coin for a coin the batch did not return, so either guard alone still stops the node there.

Notes

• Stacked on Bound the unknown-parent block buffer #941 → Check a block's merkle root first, and don't keep a body-check failure as an invalid mark #936 (merge those first).
• A row that is present but does not decode is treated as damage and stops the node. Core's generic CDBWrapper::Read returns "not found" when a value fails to deserialize (dbwrapper.h:207-224). In Core that cannot come from disk damage, though, because LevelDB verifies block checksums on every read (dbwrapper.cpp:221) and damage arrives as a Corruption status, which aborts. satd's own decoder already calls a short coin row corruption, not absence (storage/coinview.rs, deserialize_compact_stream), and the schema check at open refuses a datadir in a layout this binary cannot read. Reading such a row as "spent" would bring back the invalid mark this PR removes.
• Scope is coin reads, which is what Core's error catcher covers. Other reads in RocksDbStore still turn an error into None (get_block_index, get_undo, get_tip, get_block_hash_by_height and others). None of them feeds a missing-coin verdict. They are listed in the report for a follow-up and not changed here.
• The offline satd-chainstate-audit looks coins up through the same store. It now stops at the first unreadable coin, naming the outpoint and the RocksDB error, where it used to list that coin as missing. The manual says so beside the tool's exit statuses. No live-node path runs that audit: the verifychain RPC reads block bodies, not coins.
• handle_block_ibd's peer scoring of store errors is not touched (a separate PR covers it).

Release notes

satd now stops, as Bitcoin Core does, when its chainstate database cannot read a coin: an I/O error, a checksum mismatch, or a coin record that does not decode. It logs Error reading from database, shutting down. Before, satd treated such a coin as spent. If that coin was being spent by a valid block, satd refused the block and recorded it as invalid, and the record survived restarts. After a stop, the next start replays the blocks after the last flush. If the error comes back, check the disk and rebuild with -reindex-chainstate.

🤖 Generated with Claude Code