diff --git a/crates/electrum/benches/test_sync.rs b/crates/electrum/benches/test_sync.rs index 39dd10d9a..655ac139a 100644 --- a/crates/electrum/benches/test_sync.rs +++ b/crates/electrum/benches/test_sync.rs @@ -69,7 +69,7 @@ pub fn test_sync_performance(c: &mut Criterion) { spks.push(spk); } - let _ = env.wait_until_electrum_sees_block(Duration::from_secs(6)); + let _ = env.wait_until_electrum_tip_syncs_with_bitcoind(Duration::from_secs(6)); assert_eq!( spks.iter().cloned().collect::>().len(), spks.len(), diff --git a/crates/electrum/src/bdk_electrum_client.rs b/crates/electrum/src/bdk_electrum_client.rs index a7c943150..5126006ef 100644 --- a/crates/electrum/src/bdk_electrum_client.rs +++ b/crates/electrum/src/bdk_electrum_client.rs @@ -836,7 +836,7 @@ mod test { // Mine block that confirms 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 height: u32 = env.rpc_client().get_block_count()?.into_model().0 as u32; // Add the pre-reorg block that the tx is confirmed in to the header cache. @@ -851,7 +851,7 @@ mod test { // Reorg to create a new header and hash. env.reorg(1)?; - env.wait_until_electrum_sees_block(Duration::from_secs(6))?; + env.wait_until_electrum_tip_syncs_with_bitcoind(Duration::from_secs(6))?; // Calling `batch_fetch_anchors` should fetch new header, replacing the pre-reorg header. let anchors = electrum_client.batch_fetch_anchors(&[(txid, height as usize)])?; diff --git a/crates/electrum/tests/test_electrum.rs b/crates/electrum/tests/test_electrum.rs index ef94c62f3..653668aff 100644 --- a/crates/electrum/tests/test_electrum.rs +++ b/crates/electrum/tests/test_electrum.rs @@ -323,7 +323,7 @@ pub fn test_update_tx_graph_without_keychain() -> anyhow::Result<()> { .send_to_address(&receive_address0, Amount::from_sat(20000))? .txid()?; 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(); @@ -415,7 +415,7 @@ pub fn test_update_tx_graph_stop_gap() -> anyhow::Result<()> { .send_to_address(&addresses[3], Amount::from_sat(10000))? .txid()?; 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(); @@ -454,7 +454,7 @@ pub fn test_update_tx_graph_stop_gap() -> anyhow::Result<()> { .send_to_address(&addresses[addresses.len() - 1], Amount::from_sat(10000))? .txid()?; 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. @@ -519,7 +519,7 @@ pub fn test_stop_gap_past_last_revealed() -> anyhow::Result<()> { .send_to_address(&addresses[9], Amount::from_sat(10000))? .txid()?; 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 cp_tip = env.make_checkpoint_tip(); @@ -575,7 +575,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)?; @@ -600,7 +600,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, @@ -621,7 +621,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, @@ -641,7 +641,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)?; @@ -725,7 +725,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()], @@ -756,7 +756,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()], @@ -801,7 +801,7 @@ fn test_sync_with_coinbase() -> anyhow::Result<()> { // Mine some blocks. env.mine_blocks(101, Some(addr_to_track))?; - env.wait_until_electrum_sees_block(Duration::from_secs(6))?; + env.wait_until_electrum_tip_syncs_with_bitcoind(Duration::from_secs(6))?; // Check to see if electrum syncs properly. assert!(sync_with_electrum( @@ -877,7 +877,7 @@ fn test_check_fee_calculation() -> anyhow::Result<()> { .expect("should've successfully found the existing `TxOut`"); // 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 _ = sync_with_electrum( &client, [spk_to_track.clone()], diff --git a/crates/testenv/src/lib.rs b/crates/testenv/src/lib.rs index 3c3f8a6f4..15408a181 100644 --- a/crates/testenv/src/lib.rs +++ b/crates/testenv/src/lib.rs @@ -291,9 +291,21 @@ impl TestEnv { Err(anyhow::anyhow!("Cannot find nonce that meets the target")) } - /// This method waits for the Electrum notification indicating that a new block has been mined. + /// Wait until Electrum is aware of a block at the given `block_height` (and optionally that the + /// block at that height matches the given `block_hash`). + /// /// `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<()> { + pub fn wait_until_electrum_sees_block( + &self, + block_height: usize, + block_hash: Option, + timeout: Duration, + ) -> anyhow::Result<()> { + // NOTE: We use the subscribe endpoint because polling Electrs for a block header at + // `block_height` and verifying the `block_hash` does not ensure that the spk histories are + // current. In contrast, getting a notification for a new block tip ensures that the + // confirmed spk histories are current, including those of the newly notified tip. This is a + // result of the internal workings of Electrs. self.electrsd.client.block_headers_subscribe()?; let delay = Duration::from_millis(200); let start = std::time::Instant::now(); @@ -301,19 +313,44 @@ impl TestEnv { while start.elapsed() < timeout { self.electrsd.trigger()?; self.electrsd.client.ping()?; - if self.electrsd.client.block_headers_pop()?.is_some() { - return Ok(()); + if let Some(header_notif) = self.electrsd.client.block_headers_pop()? { + if header_notif.height >= block_height { + // If the notified tip already advanced past `block_height`, fetch the header at + // `block_height` explicitly so we can verify the expected `block_hash`. + let header = if header_notif.height == block_height { + header_notif.header + } else { + self.electrsd.client.block_header(block_height)? + }; + match block_hash { + None => return Ok(()), + Some(exp_hash) if exp_hash == header.block_hash() => return Ok(()), + Some(_) => {} + } + } } std::thread::sleep(delay); } Err(anyhow::Error::msg(format!( - "Timed out waiting for Electrsd to get transaction, took: {:?}", + "Timed out waiting for Electrsd to sync to block {block_height}, took: {:?}", start.elapsed() ))) } + /// Wait until Electrum's tip syncs with bitcoind's chain tip. + /// + /// `timeout` is the maximum [`Duration`] we want to wait for a response from Electrsd. + pub fn wait_until_electrum_tip_syncs_with_bitcoind( + &self, + timeout: Duration, + ) -> anyhow::Result<()> { + let chain_height = self.rpc_client().get_block_count()?.into_model().0; + let chain_hash = self.get_block_hash(chain_height)?; + self.wait_until_electrum_sees_block(chain_height as usize, 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( @@ -438,7 +475,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()?.into_model().0; let blocks = (0..=height) .map(|i| env.bitcoind.client.get_block_hash(i)) @@ -446,7 +483,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()?.into_model().0; let reorged_blocks = (0..=height) .map(|i| env.bitcoind.client.get_block_hash(i))