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

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