Skip to content

Commit 333cc2a

Browse files
committed
Merge #2011: fix(electrum): fix stale anchor hash on reorg
fdec863 fix(electrum): fix stale anchor hash on reorg (Wei Chen) Pull request description: ### Description Fixes an issue in `batch_fetch_anchors()` in `bdk_electrum` where, if a stale block header was detected and a fresh header was fetched, the confirmation anchor inserted into `anchor_cache` still used the hash from the original stale header instead of the new one. ### Changelog notice - `batch_fetch_anchors()` no longer uses a potentially stale hash as the anchor. ### Checklists #### All Submissions: * [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md) #### Bugfixes: * [ ] This pull request breaks the existing API * [ ] I've added tests to reproduce the issue which are now passing * [ ] I'm linking the issue being fixed by this PR ACKs for top commit: evanlinjin: ACK fdec863 Tree-SHA512: be098569d8e038ca65b9c572f92642151679e36ed4138ca89d554c31bd97aaabfcb430ee055939cc37fcccb35a45bb6158a9b303241a6cf410caf7657d33a5f5
2 parents 6a4ada5 + fdec863 commit 333cc2a

File tree

1 file changed

+70
-6
lines changed

1 file changed

+70
-6
lines changed

crates/electrum/src/bdk_electrum_client.rs

Lines changed: 70 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -518,17 +518,16 @@ impl<E: ElectrumApi> BdkElectrumClient<E> {
518518
if let Some(anchor) = anchor_cache.get(&(txid, hash)) {
519519
results.push((txid, *anchor));
520520
} else {
521-
to_fetch.push((txid, height, hash));
521+
to_fetch.push((txid, height));
522522
}
523523
}
524524
}
525525

526526
// Fetch merkle proofs.
527-
let txids_and_heights = to_fetch.iter().map(|&(txid, height, _)| (txid, height));
528-
let proofs = self.inner.batch_transaction_get_merkle(txids_and_heights)?;
527+
let proofs = self.inner.batch_transaction_get_merkle(to_fetch.iter())?;
529528

530529
// Validate each proof, retrying once for each stale header.
531-
for ((txid, height, hash), proof) in to_fetch.into_iter().zip(proofs.into_iter()) {
530+
for ((txid, height), proof) in to_fetch.into_iter().zip(proofs.into_iter()) {
532531
let mut header = {
533532
let cache = self.block_header_cache.lock().unwrap();
534533
cache
@@ -553,6 +552,7 @@ impl<E: ElectrumApi> BdkElectrumClient<E> {
553552

554553
// Build and cache the anchor if merkle proof is valid.
555554
if valid {
555+
let hash = header.block_hash();
556556
let anchor = ConfirmationBlockTime {
557557
confirmation_time: header.time as u64,
558558
block_id: BlockId {
@@ -696,11 +696,13 @@ fn chain_update(
696696
#[cfg(test)]
697697
#[cfg_attr(coverage_nightly, coverage(off))]
698698
mod test {
699-
use crate::{bdk_electrum_client::TxUpdate, BdkElectrumClient};
699+
use crate::{bdk_electrum_client::TxUpdate, electrum_client::ElectrumApi, BdkElectrumClient};
700+
use bdk_chain::bitcoin::Amount;
700701
use bdk_chain::bitcoin::{constants, Network, OutPoint, ScriptBuf, Transaction, TxIn};
701702
use bdk_chain::{BlockId, CheckPoint};
702703
use bdk_core::{collections::BTreeMap, spk_client::SyncRequest};
703-
use bdk_testenv::{anyhow, utils::new_tx, TestEnv};
704+
use bdk_testenv::{anyhow, bitcoincore_rpc::RpcApi, utils::new_tx, TestEnv};
705+
use core::time::Duration;
704706
use electrum_client::Error as ElectrumError;
705707
use std::sync::Arc;
706708

@@ -764,4 +766,66 @@ mod test {
764766

765767
Ok(())
766768
}
769+
770+
/// This test checks that when a transaction is reorged into a different block
771+
/// at the same height, `batch_fetch_anchors()` updates its anchor correctly:
772+
///
773+
/// 1. A transaction is confirmed in a block, and that block header is cached.
774+
/// 2. A reorg happens, replacing that block with a new one at the same height.
775+
/// 3. When we call `batch_fetch_anchors()`, it should fetch the new block header and recreate
776+
/// the transaction’s anchor using the new block hash.
777+
///
778+
/// Reorgs should cause the anchor to point to the new block instead of the stale one.
779+
#[cfg(feature = "default")]
780+
#[test]
781+
fn test_batch_fetch_anchors_reorg_uses_new_hash() -> anyhow::Result<()> {
782+
let env = TestEnv::new()?;
783+
let client = electrum_client::Client::new(env.electrsd.electrum_url.as_str()).unwrap();
784+
let electrum_client = BdkElectrumClient::new(client);
785+
786+
env.mine_blocks(101, None)?;
787+
788+
let addr = env
789+
.rpc_client()
790+
.get_new_address(None, None)?
791+
.assume_checked();
792+
let txid = env.send(&addr, Amount::from_sat(50_000))?;
793+
794+
// Mine block that confirms transaction.
795+
env.mine_blocks(1, None)?;
796+
env.wait_until_electrum_sees_block(Duration::from_secs(6))?;
797+
let height: u32 = env.rpc_client().get_block_count()? as u32;
798+
799+
// Add the pre-reorg block that the tx is confirmed in to the header cache.
800+
let header = electrum_client.inner.block_header(height as usize)?;
801+
{
802+
electrum_client
803+
.block_header_cache
804+
.lock()
805+
.unwrap()
806+
.insert(height, header);
807+
}
808+
809+
// Reorg to create a new header and hash.
810+
env.reorg(1)?;
811+
env.wait_until_electrum_sees_block(Duration::from_secs(6))?;
812+
813+
// Calling `batch_fetch_anchors` should fetch new header, replacing the pre-reorg header.
814+
let anchors = electrum_client.batch_fetch_anchors(&[(txid, height as usize)])?;
815+
assert_eq!(anchors.len(), 1);
816+
817+
let new_header = electrum_client.inner.block_header(height as usize)?;
818+
let new_hash = new_header.block_hash();
819+
820+
// Anchor should contain new hash.
821+
let (_, anchor) = anchors[0];
822+
assert_eq!(anchor.block_id.height, height);
823+
assert_eq!(anchor.block_id.hash, new_hash);
824+
825+
// Anchor cache should also contain new hash.
826+
let cache = electrum_client.anchor_cache.lock().unwrap();
827+
assert!(cache.get(&(txid, new_hash)).is_some());
828+
829+
Ok(())
830+
}
767831
}

0 commit comments

Comments
 (0)