refactor(testenv)!: make wait_until_electrum_sees_block concrete

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::<BTreeSet<_>>().len(),
         spks.len(),

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)])?;

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()],

crates/testenv/src/lib.rs

@@ -291,29 +291,66 @@ 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<BlockHash>,
+        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();
 
         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,15 +475,15 @@ 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))
             .collect::<Result<Vec<_>, _>>()?;
 
         // 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))