From ce565acc8e49f1662f884293e3dbba2263ca9a26 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Fri, 4 Oct 2024 21:13:52 +0000 Subject: [PATCH] refactor(testenv)!: `TestEnv::wait_until_electrum_sees_block` Add inputs `block_height` and `block_hash` so that it is more concrete what exactly we are waiting for. Introduce `TestEnv::wait_until_electrum_tip_syncs_with_bitcoind`. --- crates/electrum/tests/test_electrum.rs | 22 +++++----- crates/testenv/src/lib.rs | 57 +++++++++++++++++++++----- 2 files changed, 59 insertions(+), 20 deletions(-) diff --git a/crates/electrum/tests/test_electrum.rs b/crates/electrum/tests/test_electrum.rs index 5f032ba6c..b4b625351 100644 --- a/crates/electrum/tests/test_electrum.rs +++ b/crates/electrum/tests/test_electrum.rs @@ -92,7 +92,7 @@ pub fn test_update_tx_graph_without_keychain() -> anyhow::Result<()> { None, )?; env.mine_blocks(1, None)?; - env.wait_until_electrum_sees_block(Duration::from_secs(6))?; + env.wait_until_electrum_tip_syncs_with_bitcoind(Duration::from_secs(6))?; // use a full checkpoint linked list (since this is not what we are testing) let cp_tip = env.make_checkpoint_tip(); @@ -204,7 +204,7 @@ pub fn test_update_tx_graph_stop_gap() -> anyhow::Result<()> { None, )?; env.mine_blocks(1, None)?; - env.wait_until_electrum_sees_block(Duration::from_secs(6))?; + env.wait_until_electrum_tip_syncs_with_bitcoind(Duration::from_secs(6))?; // use a full checkpoint linked list (since this is not what we are testing) let cp_tip = env.make_checkpoint_tip(); @@ -248,7 +248,7 @@ pub fn test_update_tx_graph_stop_gap() -> anyhow::Result<()> { None, )?; env.mine_blocks(1, None)?; - env.wait_until_electrum_sees_block(Duration::from_secs(6))?; + env.wait_until_electrum_tip_syncs_with_bitcoind(Duration::from_secs(6))?; // A scan with gap limit 5 won't find the second transaction, but a scan with gap limit 6 will. // The last active indice won't be updated in the first case but will in the second one. @@ -316,7 +316,7 @@ fn test_sync() -> anyhow::Result<()> { // Mine some blocks. env.mine_blocks(101, Some(addr_to_mine))?; - env.wait_until_electrum_sees_block(Duration::from_secs(6))?; + env.wait_until_electrum_tip_syncs_with_bitcoind(Duration::from_secs(6))?; // Broadcast transaction to mempool. let txid = env.send(&addr_to_track, SEND_AMOUNT)?; @@ -341,7 +341,7 @@ fn test_sync() -> anyhow::Result<()> { // Mine block to confirm transaction. env.mine_blocks(1, None)?; - env.wait_until_electrum_sees_block(Duration::from_secs(6))?; + env.wait_until_electrum_tip_syncs_with_bitcoind(Duration::from_secs(6))?; let _ = sync_with_electrum( &client, @@ -362,7 +362,7 @@ fn test_sync() -> anyhow::Result<()> { // Perform reorg on block with confirmed transaction. env.reorg_empty_blocks(1)?; - env.wait_until_electrum_sees_block(Duration::from_secs(6))?; + env.wait_until_electrum_tip_syncs_with_bitcoind(Duration::from_secs(6))?; let _ = sync_with_electrum( &client, @@ -382,7 +382,7 @@ fn test_sync() -> anyhow::Result<()> { // Mine block to confirm transaction again. env.mine_blocks(1, None)?; - env.wait_until_electrum_sees_block(Duration::from_secs(6))?; + env.wait_until_electrum_tip_syncs_with_bitcoind(Duration::from_secs(6))?; let _ = sync_with_electrum(&client, [spk_to_track], &mut recv_chain, &mut recv_graph)?; @@ -425,7 +425,8 @@ fn test_sync() -> anyhow::Result<()> { Ok(()) } -/// Ensure that confirmed txs that are reorged become unconfirmed. +/// Ensure transactions can become unconfirmed during reorg. +/// ~Ensure that confirmed txs that are reorged become unconfirmed.~ /// /// 1. Mine 101 blocks. /// 2. Mine 8 blocks with a confirmed tx in each. @@ -469,7 +470,7 @@ fn tx_can_become_unconfirmed_after_reorg() -> anyhow::Result<()> { } // Sync up to tip. - env.wait_until_electrum_sees_block(Duration::from_secs(6))?; + env.wait_until_electrum_tip_syncs_with_bitcoind(Duration::from_secs(6))?; let update = sync_with_electrum( &client, [spk_to_track.clone()], @@ -500,7 +501,7 @@ fn tx_can_become_unconfirmed_after_reorg() -> anyhow::Result<()> { for depth in 1..=REORG_COUNT { env.reorg_empty_blocks(depth)?; - env.wait_until_electrum_sees_block(Duration::from_secs(6))?; + env.wait_until_electrum_tip_syncs_with_bitcoind(Duration::from_secs(6))?; let update = sync_with_electrum( &client, [spk_to_track.clone()], @@ -511,6 +512,7 @@ fn tx_can_become_unconfirmed_after_reorg() -> anyhow::Result<()> { // Check that no new anchors are added during current reorg. assert!(initial_anchors.is_superset(&update.tx_update.anchors)); + // TODO: Fails here. assert_eq!( get_balance(&recv_chain, &recv_graph)?, Balance { diff --git a/crates/testenv/src/lib.rs b/crates/testenv/src/lib.rs index 6d169bdce..034d8b688 100644 --- a/crates/testenv/src/lib.rs +++ b/crates/testenv/src/lib.rs @@ -186,18 +186,45 @@ impl TestEnv { Ok((bt.height as usize, block.block_hash())) } - /// This method waits for the Electrum notification indicating that a new block has been mined. - /// `timeout` is the maximum [`Duration`] we want to wait for a response from Electrsd. - pub fn wait_until_electrum_sees_block(&self, timeout: Duration) -> anyhow::Result<()> { - self.electrsd.client.block_headers_subscribe()?; + /// Wait until Electrum is aware of a block of a given `block_height` (and optionally, matches + /// the given `block_hash`). + pub fn wait_until_electrum_sees_block( + &self, + block_height: usize, + block_hash: Option, + timeout: Duration, + ) -> anyhow::Result<()> { + self.electrsd.trigger()?; + // NOTE: There is a reason why we use the subscribe endpoint specifically. You may think + // just polling Electrs via Electrum for a block header at height `block_height` and + // checking whether `block_hash` matches is enough. However, having the header polling call + // up to date does not necessarily mean the spk histories are up to date. On the other hand, + // getting a notification for a new block tip does mean that the confirmed spk histories + // are up to date up to and including the new notified tip. This is all due to the internal + // workings of Electrs. + self.electrum_client().block_headers_subscribe()?; + let delay = Duration::from_millis(200); let start = std::time::Instant::now(); while start.elapsed() < timeout { - self.electrsd.trigger()?; - self.electrsd.client.ping()?; - if self.electrsd.client.block_headers_pop()?.is_some() { - return Ok(()); + self.electrum_client().ping()?; + if let Some(header_notif) = self.electrum_client().block_headers_pop()? { + if header_notif.height >= block_height { + let header = if header_notif.height == block_height { + header_notif.header + } else { + self.electrum_client().block_header(block_height)? + }; + match block_hash { + None => return Ok(()), + Some(exp_hash) => { + if exp_hash == header.block_hash() { + return Ok(()); + } + } + } + } } std::thread::sleep(delay); @@ -208,6 +235,16 @@ impl TestEnv { )) } + /// Wait until Electrum is aware of bitcoind's chain tip. + pub fn wait_until_electrum_tip_syncs_with_bitcoind( + &self, + timeout: Duration, + ) -> anyhow::Result<()> { + let chain_height = self.rpc_client().get_block_count()?; + let chain_hash = self.rpc_client().get_block_hash(chain_height)?; + self.wait_until_electrum_sees_block(chain_height as _, Some(chain_hash), timeout) + } + /// This method waits for Electrsd to see a transaction with given `txid`. `timeout` is the /// maximum [`Duration`] we want to wait for a response from Electrsd. pub fn wait_until_electrum_sees_txid( @@ -321,7 +358,7 @@ mod test { // Mine some blocks. env.mine_blocks(101, None)?; - env.wait_until_electrum_sees_block(Duration::from_secs(6))?; + env.wait_until_electrum_tip_syncs_with_bitcoind(Duration::from_secs(6))?; let height = env.bitcoind.client.get_block_count()?; let blocks = (0..=height) .map(|i| env.bitcoind.client.get_block_hash(i)) @@ -329,7 +366,7 @@ mod test { // Perform reorg on six blocks. env.reorg(6)?; - env.wait_until_electrum_sees_block(Duration::from_secs(6))?; + env.wait_until_electrum_tip_syncs_with_bitcoind(Duration::from_secs(6))?; let reorged_height = env.bitcoind.client.get_block_count()?; let reorged_blocks = (0..=height) .map(|i| env.bitcoind.client.get_block_hash(i))