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

@@ -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 {

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<BlockHash>,
+        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,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))