From 3fe5847cfa335ab33840127033a9cbfaf4bff55d Mon Sep 17 00:00:00 2001 From: MeABzZz <73040345+MeABzZz@users.noreply.github.com> Date: Wed, 21 Oct 2026 17:25:20 -0300 Subject: [PATCH] fix(node): refuse a reorg over empty undo data and tell the operator at startup Parallel IBD before the undo fix stored empty BlockUndoLogs. A reorg over those blocks restored nothing and left the UTXO set wrong, with no message. Check the last 100 blocks at startup and log what happened and how to recover; refuse a reorg whose disconnected blocks have missing or empty undo data. --- src/node/mod.rs | 7 ++ src/node/reorg_executor.rs | 18 +++++ src/node/undo_check.rs | 119 ++++++++++++++++++++++++++++ tests/fork_reorg_regtest.rs | 106 +++++++++++++++++++------ tests/undo_startup_check.rs | 153 ++++++++++++++++++++++++++++++++++++ 5 files changed, 381 insertions(+), 22 deletions(-) create mode 100644 src/node/undo_check.rs create mode 100644 tests/undo_startup_check.rs diff --git a/src/node/mod.rs b/src/node/mod.rs index 91c51d6..8a47c92 100644 --- a/src/node/mod.rs +++ b/src/node/mod.rs @@ -21,6 +21,8 @@ pub mod reorg_executor; pub mod run_loop; pub mod subsystems; pub mod sync; +#[cfg(feature = "production")] +pub mod undo_check; use anyhow::Result; use hex; @@ -473,6 +475,11 @@ impl Node { } } + // Undo data written by the pre-fix parallel IBD is empty; tell the operator now + // rather than at the first reorg (which will be refused). + #[cfg(feature = "production")] + crate::node::undo_check::check_recent_undo_at_startup(&self.storage); + // Sequential startup in this task: wire RPC auth/events/modules, then start RPC and subsystems. // Create early event publisher for RPC (BlockchainRpc needs it for invalidateblock → BlockDisconnected) diff --git a/src/node/reorg_executor.rs b/src/node/reorg_executor.rs index 5d3f940..cdd2574 100644 --- a/src/node/reorg_executor.rs +++ b/src/node/reorg_executor.rs @@ -228,6 +228,24 @@ pub fn try_activate_heavier_fork( return Ok(false); } + // current_chain[0] is the common ancestor; every later block is disconnected and needs + // usable undo data (nodes synced before the parallel-IBD undo fix have empty logs). + let to_disconnect: Vec<(u64, Hash)> = current_chain + .iter() + .enumerate() + .skip(1) + .map(|(i, block)| { + let height = active_height - (current_chain.len() - 1 - i) as u64; + (height, blockstore.get_block_hash(block)) + }) + .collect(); + if let Err(e) = + crate::node::undo_check::ensure_disconnect_undo_usable(blockstore, &to_disconnect) + { + tracing::error!("{e}"); + return Err(e); + } + let mut new_chain = new_chain; let mut new_witnesses = witnesses_for_chain(blockstore, storage, &new_chain, protocol)?; for (block, witnesses) in new_chain.iter_mut().zip(new_witnesses.iter_mut()) { diff --git a/src/node/undo_check.rs b/src/node/undo_check.rs new file mode 100644 index 0000000..454eb6b --- /dev/null +++ b/src/node/undo_check.rs @@ -0,0 +1,119 @@ +//! Detect undo data a reorg cannot use. +//! +//! Before the parallel-IBD undo fix, blocks connected by parallel IBD were stored with an +//! empty `BlockUndoLog`. A correct undo log is never empty: the coinbase always creates at +//! least one output, and every created output is recorded. Disconnecting a block whose undo +//! log is missing or empty restores nothing, so the UTXO set silently goes wrong. +//! +//! This module checks the most recent blocks at startup and logs what the operator has to +//! do. The reorg executor uses the same check to refuse a reorg that would disconnect such +//! a block. + +use crate::storage::Storage; +use crate::storage::blockstore::BlockStore; +use blvm_consensus::reorganization::BlockUndoLog; +use blvm_protocol::Hash; +use tracing::{error, info}; + +/// Blocks below the tip checked at startup (coinbase maturity: deeper reorgs are not +/// expected in practice). +pub const STARTUP_UNDO_CHECK_DEPTH: u64 = 100; + +/// What the operator should do when undo data is unusable. +pub const RESYNC_ADVICE: &str = "This node was most likely synced by a blvm-node release whose \ +parallel IBD stored empty undo data. Stop the node, move its data directory aside, and resync \ +from scratch (blvm has no -reindex yet) to rebuild the undo data."; + +/// True when this undo log cannot undo a block. +pub fn undo_log_unusable(log: Option<&BlockUndoLog>) -> bool { + log.is_none_or(|l| l.entries.is_empty()) +} + +/// True when the stored undo log for `hash` is missing, unreadable, or empty. +pub fn block_undo_unusable(blockstore: &BlockStore, hash: &Hash) -> bool { + match blockstore.get_undo_log(hash) { + Ok(log) => undo_log_unusable(log.as_ref()), + Err(_) => true, + } +} + +/// Heights in `[tip - depth + 1, tip]` (never genesis) on the active chain whose undo data +/// is unusable, newest first. Heights with no hash in the height index are skipped. +pub fn recent_heights_with_unusable_undo( + blockstore: &BlockStore, + tip_height: u64, + depth: u64, +) -> Vec { + if depth == 0 || tip_height == 0 { + return Vec::new(); + } + let lowest = tip_height.saturating_sub(depth - 1).max(1); + (lowest..=tip_height) + .rev() + .filter(|&h| match blockstore.get_hash_by_height(h) { + Ok(Some(hash)) => block_undo_unusable(blockstore, &hash), + _ => false, + }) + .collect() +} + +/// Operator message for unusable undo data at `heights` among the last `depth` blocks. +pub fn startup_message(heights: &[u64], tip_height: u64, depth: u64) -> String { + let newest = heights.iter().max().copied().unwrap_or(tip_height); + let oldest = heights.iter().min().copied().unwrap_or(tip_height); + format!( + "Undo data is missing or empty for {} of the last {} blocks (heights {}..={}, tip {}). \ + blvm will refuse any reorg that disconnects one of these blocks instead of corrupting \ + the UTXO set, so this node can get stuck on a stale chain. {}", + heights.len(), + depth.min(tip_height), + oldest, + newest, + tip_height, + RESYNC_ADVICE + ) +} + +/// Startup check: log an error telling the operator to resync if recent undo data is unusable. +/// Returns the affected heights (empty when all is well). +pub fn check_recent_undo_at_startup(storage: &Storage) -> Vec { + let Ok((_, tip_height)) = storage.chain().get_tip_hash_and_height() else { + return Vec::new(); + }; + let heights = + recent_heights_with_unusable_undo(&storage.blocks(), tip_height, STARTUP_UNDO_CHECK_DEPTH); + if heights.is_empty() { + info!( + "Undo data check: last {} blocks have usable undo data", + STARTUP_UNDO_CHECK_DEPTH.min(tip_height) + ); + } else { + error!( + "{}", + startup_message(&heights, tip_height, STARTUP_UNDO_CHECK_DEPTH) + ); + } + heights +} + +/// Refuse a reorg that would disconnect a block with unusable undo data. +/// `to_disconnect` is `(height, hash)` for each block leaving the active chain. +pub fn ensure_disconnect_undo_usable( + blockstore: &BlockStore, + to_disconnect: &[(u64, Hash)], +) -> anyhow::Result<()> { + if let Some((height, hash)) = to_disconnect + .iter() + .find(|(_, hash)| block_undo_unusable(blockstore, hash)) + { + anyhow::bail!( + "Refusing reorg of depth {}: block at height {} ({}) has missing or empty undo \ + data, and disconnecting it would corrupt the UTXO set. {}", + to_disconnect.len(), + height, + hex::encode(hash.iter().rev().copied().collect::>()), + RESYNC_ADVICE + ); + } + Ok(()) +} diff --git a/tests/fork_reorg_regtest.rs b/tests/fork_reorg_regtest.rs index ea4742c..e51a595 100644 --- a/tests/fork_reorg_regtest.rs +++ b/tests/fork_reorg_regtest.rs @@ -178,9 +178,19 @@ async fn linear_chain_control() -> anyhow::Result<()> { Ok(()) } -/// Higher-work fork: after importing the fork branch, the active tip follows the heavier chain. -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn higher_work_fork_from_mid_chain() -> anyhow::Result<()> { +/// Main chain `1..=12` on `storage_main`, and the wire blocks of a heavier fork that +/// diverges at height 5 (not yet imported). +struct ForkFixture { + _dirs: (TempDir, TempDir), + protocol: BitcoinProtocolEngine, + storage_main: Arc, + main_utxo: UtxoSet, + main_tip: blvm_protocol::Hash, + fork_tip: blvm_protocol::Hash, + fork_wires: Vec<(u64, Vec)>, +} + +fn build_fork() -> anyhow::Result { let protocol = BitcoinProtocolEngine::new(ProtocolVersion::Regtest)?; let genesis_header = protocol.get_network_params().genesis_block.header.clone(); let consensus = ConsensusProof::new(); @@ -254,9 +264,7 @@ async fn higher_work_fork_from_mid_chain() -> anyhow::Result<()> { "fork must carry more cumulative chainwork (fork={fork_work:?}, main={main_work:?})" ); - // Import fork branch blocks onto the main node (side chain until the fork wins on work). - let mut replay_utxo = main_utxo.clone(); - let mut replay_coord = SyncCoordinator::new(); + let mut fork_wires = Vec::new(); for connect_height in FORK_DIVERGE_HEIGHT..=FORK_TIP_HEIGHT { let fork_hash = storage_fork .blocks() @@ -264,27 +272,81 @@ async fn higher_work_fork_from_mid_chain() -> anyhow::Result<()> { .unwrap(); let fork_block = storage_fork.blocks().get_block(&fork_hash)?.unwrap(); let wire = serialize_block_with_witnesses(&fork_block, &witnesses_for(&fork_block), true); - let accepted = replay_coord.process_block( - storage_main.blocks().as_ref(), - &protocol, - Some(&storage_main), - &wire, - connect_height, - &mut replay_utxo, - None, - None, - )?; - assert!( - accepted, - "fork block at height {connect_height} must be accepted" - ); + fork_wires.push((connect_height, wire)); } + Ok(ForkFixture { + _dirs: (main_dir, fork_dir), + protocol, + storage_main, + main_utxo, + main_tip, + fork_tip, + fork_wires, + }) +} + +/// Import the fork's blocks onto the main node; returns how many were accepted. +fn import_fork(f: &ForkFixture) -> usize { + let mut replay_utxo = f.main_utxo.clone(); + let mut replay_coord = SyncCoordinator::new(); + f.fork_wires + .iter() + .filter(|(connect_height, wire)| { + matches!( + replay_coord.process_block( + f.storage_main.blocks().as_ref(), + &f.protocol, + Some(&f.storage_main), + wire, + *connect_height, + &mut replay_utxo, + None, + None, + ), + Ok(true) + ) + }) + .count() +} +/// Higher-work fork: after importing the fork branch, the active tip follows the heavier chain. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn higher_work_fork_from_mid_chain() -> anyhow::Result<()> { + let f = build_fork()?; assert_eq!( - storage_main.chain().get_tip_hash()?.unwrap(), - fork_tip, + import_fork(&f), + f.fork_wires.len(), + "every fork block must be accepted" + ); + assert_eq!( + f.storage_main.chain().get_tip_hash()?.unwrap(), + f.fork_tip, "active tip must follow the heavier fork after reorg" ); + Ok(()) +} + +/// A node synced by the old parallel IBD has empty undo logs. A reorg over those blocks +/// would restore nothing and leave the UTXO set wrong, so it must be refused: the node +/// stays on its chain with its UTXO set intact. +#[cfg(feature = "production")] +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn heavier_fork_over_empty_undo_logs_is_refused() -> anyhow::Result<()> { + let f = build_fork()?; + let blocks = f.storage_main.blocks(); + for h in FORK_DIVERGE_HEIGHT..=MAIN_TIP_HEIGHT { + let hash = blocks.get_hash_by_height(h)?.unwrap(); + blocks.store_undo_log(&hash, &blvm_consensus::reorganization::BlockUndoLog::new())?; + } + let utxo_before = f.storage_main.utxos().get_all_utxos()?; + import_fork(&f); + + assert_eq!( + f.storage_main.chain().get_tip_hash()?.unwrap(), + f.main_tip, + "reorg over empty undo logs must be refused" + ); + assert_eq!(f.storage_main.utxos().get_all_utxos()?, utxo_before); Ok(()) } diff --git a/tests/undo_startup_check.rs b/tests/undo_startup_check.rs new file mode 100644 index 0000000..e7b64a9 --- /dev/null +++ b/tests/undo_startup_check.rs @@ -0,0 +1,153 @@ +//! Nodes synced before the parallel-IBD undo fix stored empty undo logs. Startup must say +//! so (with what to do), and a reorg that would disconnect such a block must be refused +//! rather than silently corrupting the UTXO set. +#![cfg(feature = "production")] + +use blvm_consensus::reorganization::{BlockUndoLog, UndoEntry}; +use blvm_node::node::undo_check::{ + STARTUP_UNDO_CHECK_DEPTH, check_recent_undo_at_startup, ensure_disconnect_undo_usable, + recent_heights_with_unusable_undo, startup_message, +}; +use blvm_node::storage::Storage; +use blvm_protocol::{BlockHeader, Hash, OutPoint, UTXO}; +use std::sync::Arc; +use tempfile::TempDir; + +fn hash_at(height: u64) -> Hash { + let mut h = [0u8; 32]; + h[..8].copy_from_slice(&height.to_le_bytes()); + h[31] = 0xab; + h +} + +/// What the sequential path writes for a coinbase-only block: one created output. +fn good_undo(height: u64) -> BlockUndoLog { + BlockUndoLog { + entries: vec![UndoEntry { + outpoint: OutPoint { + hash: hash_at(height), + index: 0, + }, + previous_utxo: None, + new_utxo: Some(Arc::new(UTXO { + value: 50_0000_0000, + height, + is_coinbase: true, + ..Default::default() + })), + }], + } +} + +enum Undo { + Good, + Empty, + Missing, +} + +/// Chain 0..=tip with the given undo state per height (genesis has no undo). +fn chain(dir: &TempDir, tip: u64, undo: impl Fn(u64) -> Undo) -> Storage { + let storage = Storage::new(dir.path()).unwrap(); + let blocks = storage.blocks(); + let header = |h: u64| BlockHeader { + version: 4, + prev_block_hash: if h == 0 { [0; 32] } else { hash_at(h - 1) }, + merkle_root: [0; 32], + timestamp: 1_300_000_000 + h, + bits: 0x207fffff, + nonce: 0, + }; + let headers: Vec<_> = (0..=tip).map(|h| (hash_at(h), header(h), h)).collect(); + blocks.store_headers_batch(&headers).unwrap(); + for h in 0..=tip { + storage + .chain() + .update_tip(&hash_at(h), &header(h), h) + .unwrap(); + } + for h in 1..=tip { + match undo(h) { + Undo::Good => blocks.store_undo_log(&hash_at(h), &good_undo(h)).unwrap(), + Undo::Empty => blocks + .store_undo_log(&hash_at(h), &BlockUndoLog::new()) + .unwrap(), + Undo::Missing => {} + } + } + storage +} + +#[test] +fn healthy_node_passes_startup_check() { + let dir = TempDir::new().unwrap(); + let storage = chain(&dir, 120, |_| Undo::Good); + assert!(check_recent_undo_at_startup(&storage).is_empty()); +} + +/// Synced by parallel IBD on the old code: every undo log is empty. +#[test] +fn old_parallel_ibd_node_is_flagged_at_startup() { + let dir = TempDir::new().unwrap(); + let storage = chain(&dir, 120, |_| Undo::Empty); + let bad = check_recent_undo_at_startup(&storage); + assert_eq!(bad.len() as u64, STARTUP_UNDO_CHECK_DEPTH); + assert_eq!(bad.first(), Some(&120)); + assert_eq!(bad.last(), Some(&21)); +} + +/// Old IBD, then a few blocks connected at the tip by the (fixed) sequential path. +#[test] +fn reports_only_the_affected_heights() { + let dir = TempDir::new().unwrap(); + let storage = chain(&dir, 30, |h| match h { + 26..=30 => Undo::Good, + 25 => Undo::Missing, + _ => Undo::Empty, + }); + let bad = recent_heights_with_unusable_undo(&storage.blocks(), 30, 10); + assert_eq!(bad, vec![25, 24, 23, 22, 21]); + // Genesis is never checked; a short chain checks what it has. + let all = recent_heights_with_unusable_undo(&storage.blocks(), 30, 1000); + assert_eq!(all.len(), 25); + assert!(!all.contains(&0)); +} + +#[test] +fn operator_message_says_what_happened_and_what_to_do() { + let msg = startup_message(&[25, 24, 21], 30, 100); + for needle in [ + "missing or empty for 3 of the last 30 blocks", + "heights 21..=25", + "refuse any reorg", + "resync", + "data directory", + ] { + assert!(msg.contains(needle), "message lacks {needle:?}: {msg}"); + } +} + +#[test] +fn reorg_over_empty_undo_is_refused() { + let dir = TempDir::new().unwrap(); + let storage = chain(&dir, 30, |h| if h >= 29 { Undo::Good } else { Undo::Empty }); + let blocks = storage.blocks(); + // Disconnecting 29..=30 is fine. + ensure_disconnect_undo_usable(&blocks, &[(29, hash_at(29)), (30, hash_at(30))]).unwrap(); + // Disconnecting 28..=30 hits an empty undo log at 28. + let err = ensure_disconnect_undo_usable( + &blocks, + &[(28, hash_at(28)), (29, hash_at(29)), (30, hash_at(30))], + ) + .unwrap_err() + .to_string(); + assert!(err.contains("Refusing reorg of depth 3"), "{err}"); + assert!(err.contains("height 28"), "{err}"); + assert!(err.contains("resync"), "{err}"); +} + +#[test] +fn reorg_over_missing_undo_is_refused() { + let dir = TempDir::new().unwrap(); + let storage = chain(&dir, 5, |h| if h == 5 { Undo::Missing } else { Undo::Good }); + assert!(ensure_disconnect_undo_usable(&storage.blocks(), &[(5, hash_at(5))]).is_err()); +}