From c47ce561d679695e73c45d1979067217fbbb4f9b Mon Sep 17 00:00:00 2001 From: Torkel Rogstad Date: Wed, 9 Sep 2026 22:17:25 +0200 Subject: [PATCH] fix: 2WPD disconnect boundary skips the parent's event block Off-by-one left behind by 67ab45d. --- integration_tests/ibd.rs | 118 +++++++++++++++++++++++--- integration_tests/integration_test.rs | 7 +- lib/node/net_task.rs | 4 +- 3 files changed, 116 insertions(+), 13 deletions(-) diff --git a/integration_tests/ibd.rs b/integration_tests/ibd.rs index e4fec3e8..1bb2b89e 100644 --- a/integration_tests/ibd.rs +++ b/integration_tests/ibd.rs @@ -3,7 +3,9 @@ use std::net::SocketAddr; use bip300301_enforcer_integration_tests::{ - integration_test::{activate_sidechain, fund_enforcer, propose_sidechain}, + integration_test::{ + activate_sidechain, deposit, fund_enforcer, propose_sidechain, + }, setup::{ Mode, Network, PostSetup as EnforcerPostSetup, PreSetup as EnforcerPreSetup, SetupOpts as EnforcerSetupOpts, @@ -97,26 +99,83 @@ async fn check_peer_connection( } } +/// What the syncer holds before it meets the sender. +#[derive(Clone, Copy, Debug)] +enum SyncerStart { + /// Fresh node: plain IBD. + Empty, + /// The syncer already BMM'd its own chain of three blocks, with a deposit + /// that lands in the second one and nothing in the third. Adopting the + /// sender's chain then has to disconnect a tip whose parent is the most + /// recent deposit block, which is the shape that panicked the alphanet + /// seed in `State::disconnect` (`two_way_peg_data.rs`, deposit-height + /// assert) on 2026-09-09. + OwnChainWithDeposit, +} + +/// Number of blocks the syncer holds under [`SyncerStart::OwnChainWithDeposit`] +const OWN_CHAIN_BLOCKS: u32 = 3; + async fn initial_block_download_task( bin_paths: BinPaths, res_tx: mpsc::UnboundedSender>, + syncer_start: SyncerStart, ) -> anyhow::Result<()> { - let (mut enforcer_post_setup, thunder_nodes) = + use bitcoin::Amount; + const DEPOSIT_AMOUNT: Amount = Amount::from_sat(21_000_000); + const DEPOSIT_FEE: Amount = Amount::from_sat(1_000_000); + + let (mut enforcer_post_setup, mut thunder_nodes) = setup(bin_paths, res_tx).await?; + let expected_syncer_blocks = match syncer_start { + SyncerStart::Empty => 0, + SyncerStart::OwnChainWithDeposit => { + tracing::info!("Syncer: BMM block 1 (no deposit)"); + thunder_nodes + .syncer + .bmm(&mut enforcer_post_setup, 1) + .await?; + let deposit_address = + thunder_nodes.syncer.get_deposit_address().await?; + // `deposit` mines the mainchain deposit block, then + // `confirm_deposit` BMMs syncer block 2 to apply it. + tracing::info!("Syncer: deposit, applied by BMM block 2"); + let () = deposit( + &mut enforcer_post_setup, + &mut thunder_nodes.syncer, + &deposit_address, + DEPOSIT_AMOUNT, + DEPOSIT_FEE, + ) + .await?; + tracing::info!("Syncer: BMM block 3 (no deposit)"); + thunder_nodes + .syncer + .bmm(&mut enforcer_post_setup, 1) + .await?; + let syncer_blocks = + thunder_nodes.syncer.rpc_client.getblockcount().await?; + anyhow::ensure!( + syncer_blocks == OWN_CHAIN_BLOCKS, + "syncer should hold {OWN_CHAIN_BLOCKS} blocks, has {syncer_blocks}" + ); + OWN_CHAIN_BLOCKS + } + }; const BMM_BLOCKS: u32 = 16; tracing::info!(blocks = %BMM_BLOCKS, "Attempting BMM"); thunder_nodes .sender .bmm(&mut enforcer_post_setup, BMM_BLOCKS) .await?; - // Check that sender has all blocks, and syncer has 0 + // Check that sender has all blocks, and syncer only its own { let sender_blocks = thunder_nodes.sender.rpc_client.getblockcount().await?; anyhow::ensure!(sender_blocks == BMM_BLOCKS); let syncer_blocks = thunder_nodes.syncer.rpc_client.getblockcount().await?; - anyhow::ensure!(syncer_blocks == 0); + anyhow::ensure!(syncer_blocks == expected_syncer_blocks); } tracing::info!("Attempting sync"); tracing::debug!( @@ -153,14 +212,31 @@ async fn initial_block_download_task( ) .await?; tracing::debug!("Syncer still has connection to sender"); - // Check that sender and syncer have all blocks + // Check that sender and syncer have all blocks, on the same tip { let sender_blocks = thunder_nodes.sender.rpc_client.getblockcount().await?; anyhow::ensure!(sender_blocks == BMM_BLOCKS); let syncer_blocks = thunder_nodes.syncer.rpc_client.getblockcount().await?; - anyhow::ensure!(syncer_blocks == BMM_BLOCKS); + anyhow::ensure!( + syncer_blocks == BMM_BLOCKS, + "syncer stuck at {syncer_blocks} blocks, sender at {sender_blocks}" + ); + let sender_tip = thunder_nodes + .sender + .rpc_client + .get_best_sidechain_block_hash() + .await?; + let syncer_tip = thunder_nodes + .syncer + .rpc_client + .get_best_sidechain_block_hash() + .await?; + anyhow::ensure!( + sender_tip == syncer_tip, + "syncer tip {syncer_tip:?} != sender tip {sender_tip:?}" + ); } drop(thunder_nodes.syncer); drop(thunder_nodes.sender); @@ -175,13 +251,20 @@ async fn initial_block_download_task( Ok(()) } -async fn ibd(bin_paths: BinPaths) -> anyhow::Result<()> { +async fn ibd( + bin_paths: BinPaths, + syncer_start: SyncerStart, +) -> anyhow::Result<()> { let (res_tx, mut res_rx) = mpsc::unbounded(); let _test_task: AbortOnDrop<()> = tokio::task::spawn({ let res_tx = res_tx.clone(); async move { - let res = - initial_block_download_task(bin_paths, res_tx.clone()).await; + let res = initial_block_download_task( + bin_paths, + res_tx.clone(), + syncer_start, + ) + .await; let _send_err: Result<(), _> = res_tx.unbounded_send(res); } .in_current_span() @@ -199,7 +282,22 @@ pub fn ibd_trial( ) -> AsyncTrial>> { AsyncTrial::new( "initial_block_download", - ibd(bin_paths).boxed(), + ibd(bin_paths, SyncerStart::Empty).boxed(), + file_registry, + failure_collector, + ) +} + +/// IBD onto a node that must first reorg its own chain away, disconnecting a +/// tip whose parent carries the latest deposit. +pub fn reorg_across_deposit_trial( + bin_paths: BinPaths, + file_registry: TestFileRegistry, + failure_collector: TestFailureCollector, +) -> AsyncTrial>> { + AsyncTrial::new( + "reorg_across_deposit", + ibd(bin_paths, SyncerStart::OwnChainWithDeposit).boxed(), file_registry, failure_collector, ) diff --git a/integration_tests/integration_test.rs b/integration_tests/integration_test.rs index 5ff99bab..5a8e5b9d 100644 --- a/integration_tests/integration_test.rs +++ b/integration_tests/integration_test.rs @@ -13,7 +13,7 @@ use thunder_app_rpc_api::node::RpcClient as _; use crate::{ block_template::block_template_trial, - ibd::ibd_trial, + ibd::{ibd_trial, reorg_across_deposit_trial}, setup::{Init, PostSetup}, unknown_withdrawal::unknown_withdrawal_trial, util::BinPaths, @@ -178,6 +178,11 @@ pub fn tests( file_registry.clone(), failure_collector.clone(), ), + reorg_across_deposit_trial( + bin_paths.clone(), + file_registry.clone(), + failure_collector.clone(), + ), unknown_withdrawal_trial(bin_paths, file_registry, failure_collector), ] } diff --git a/lib/node/net_task.rs b/lib/node/net_task.rs index c675a7e7..df9dc41b 100644 --- a/lib/node/net_task.rs +++ b/lib/node/net_task.rs @@ -104,7 +104,7 @@ pub(in crate::node) fn disconnect_tip_( .rev_iter(rwtxn) .map_err(DbError::from)? .find_map(|(_, (block_hash, applied_height))| { - if applied_height < height - 1 { + if applied_height < height { Ok(Some((block_hash, applied_height))) } else { Ok(None) @@ -116,7 +116,7 @@ pub(in crate::node) fn disconnect_tip_( .rev_iter(rwtxn) .map_err(DbError::from)? .find_map(|(_, (block_hash, applied_height))| { - if applied_height < height - 1 { + if applied_height < height { Ok(Some((block_hash, applied_height))) } else { Ok(None)