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`.

Author: evanlinjin

crates/electrum/tests/test_electrum.rs

@@ -297,7 +297,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();
@@ -409,7 +409,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();
@@ -453,7 +453,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.
@@ -521,7 +521,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)?;
@@ -546,7 +546,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,
@@ -567,7 +567,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,
@@ -587,7 +587,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)?;
 
@@ -630,7 +630,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.
@@ -674,7 +675,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()],
@@ -705,7 +706,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()],
@@ -716,6 +717,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 {
@@ -751,7 +753,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(
@@ -829,7 +831,7 @@ fn test_check_fee_calculation() -> 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 _ = sync_with_electrum(
         &client,
         [spk_to_track.clone()],

crates/testenv/src/lib.rs

@@ -188,18 +188,43 @@ 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<BlockHash>,
+        timeout: Duration,
+    ) -> anyhow::Result<()> {
+        self.electrsd.trigger()?;
+        // 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 the new notified tip. This is a result of
+        // 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);
@@ -210,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(
@@ -323,15 +358,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()?;
         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()?;
         let reorged_blocks = (0..=height)
             .map(|i| env.bitcoind.client.get_block_hash(i))