diff --git a/crates/esplora/src/async_ext.rs b/crates/esplora/src/async_ext.rs index 53edd25c6..d5ba4a0e0 100644 --- a/crates/esplora/src/async_ext.rs +++ b/crates/esplora/src/async_ext.rs @@ -105,8 +105,8 @@ impl EsploraAsyncExt for esplora_client::AsyncClient { .copied() .expect("must have atleast one block"); - // fetch blocks of heights that the caller is interested in, reusing latest blocks that are - // already fetched. + // Fetch blocks of heights that the caller is interested in, skipping blocks that are + // already fetched when constructing `fetched_blocks`. for height in request_heights { // do not fetch blocks higher than remote tip if height > new_tip_height { @@ -122,7 +122,8 @@ impl EsploraAsyncExt for esplora_client::AsyncClient { } } - // Ensure `fetched_blocks` can create an update that connects with the original chain. + // Ensure `fetched_blocks` can create an update that connects with the original chain by + // finding a "Point of Agreement". for (height, local_hash) in local_tip.iter().map(|cp| (cp.height(), cp.hash())) { if height > new_tip_height { continue; diff --git a/crates/esplora/src/blocking_ext.rs b/crates/esplora/src/blocking_ext.rs index 81599fd5b..8f9fcd59c 100644 --- a/crates/esplora/src/blocking_ext.rs +++ b/crates/esplora/src/blocking_ext.rs @@ -97,8 +97,8 @@ impl EsploraExt for esplora_client::BlockingClient { .copied() .expect("must atleast have one block"); - // fetch blocks of heights that the caller is interested in, reusing latest blocks that are - // already fetched. + // Fetch blocks of heights that the caller is interested in, skipping blocks that are + // already fetched when constructing `fetched_blocks`. for height in request_heights { // do not fetch blocks higher than remote tip if height > new_tip_height { @@ -114,7 +114,8 @@ impl EsploraExt for esplora_client::BlockingClient { } } - // Ensure `fetched_blocks` can create an update that connects with the original chain. + // Ensure `fetched_blocks` can create an update that connects with the original chain by + // finding a "Point of Agreement". for (height, local_hash) in local_tip.iter().map(|cp| (cp.height(), cp.hash())) { if height > new_tip_height { continue; diff --git a/crates/esplora/tests/blocking_ext.rs b/crates/esplora/tests/blocking_ext.rs index 0959136cf..b91231d1d 100644 --- a/crates/esplora/tests/blocking_ext.rs +++ b/crates/esplora/tests/blocking_ext.rs @@ -238,13 +238,16 @@ fn update_local_chain() -> anyhow::Result<()> { const TIP_HEIGHT: u32 = 50; let env = TestEnv::new()?; - let b = { - let bdc = &env.bitcoind.client; - assert_eq!(bdc.get_block_count()?, 1); - [(0, bdc.get_block_hash(0)?), (1, bdc.get_block_hash(1)?)] - .into_iter() - .chain((2..).zip(env.mine_blocks((TIP_HEIGHT - 1) as usize, None)?)) - .collect::>() + let blocks = { + let bitcoind_client = &env.bitcoind.client; + assert_eq!(bitcoind_client.get_block_count()?, 1); + [ + (0, bitcoind_client.get_block_hash(0)?), + (1, bitcoind_client.get_block_hash(1)?), + ] + .into_iter() + .chain((2..).zip(env.mine_blocks((TIP_HEIGHT - 1) as usize, None)?)) + .collect::>() }; // so new blocks can be seen by Electrs let env = env.reset_electrsd()?; @@ -252,81 +255,96 @@ fn update_local_chain() -> anyhow::Result<()> { struct TestCase { name: &'static str, chain: LocalChain, - heights: &'static [u32], + request_heights: &'static [u32], exp_update_heights: &'static [u32], } let test_cases = [ TestCase { name: "request_later_blocks", - chain: local_chain![(0, b[&0]), (21, b[&21])], - heights: &[22, 25, 28], + chain: local_chain![(0, blocks[&0]), (21, blocks[&21])], + request_heights: &[22, 25, 28], exp_update_heights: &[21, 22, 25, 28], }, TestCase { name: "request_prev_blocks", - chain: local_chain![(0, b[&0]), (1, b[&1]), (5, b[&5])], - heights: &[4], + chain: local_chain![(0, blocks[&0]), (1, blocks[&1]), (5, blocks[&5])], + request_heights: &[4], exp_update_heights: &[4, 5], }, TestCase { name: "request_prev_blocks_2", - chain: local_chain![(0, b[&0]), (1, b[&1]), (10, b[&10])], - heights: &[4, 6], + chain: local_chain![(0, blocks[&0]), (1, blocks[&1]), (10, blocks[&10])], + request_heights: &[4, 6], exp_update_heights: &[4, 6, 10], }, TestCase { name: "request_later_and_prev_blocks", - chain: local_chain![(0, b[&0]), (7, b[&7]), (11, b[&11])], - heights: &[8, 9, 15], + chain: local_chain![(0, blocks[&0]), (7, blocks[&7]), (11, blocks[&11])], + request_heights: &[8, 9, 15], exp_update_heights: &[8, 9, 11, 15], }, TestCase { name: "request_tip_only", - chain: local_chain![(0, b[&0]), (5, b[&5]), (49, b[&49])], - heights: &[TIP_HEIGHT], + chain: local_chain![(0, blocks[&0]), (5, blocks[&5]), (49, blocks[&49])], + request_heights: &[TIP_HEIGHT], exp_update_heights: &[49], }, TestCase { name: "request_nothing", - chain: local_chain![(0, b[&0]), (13, b[&13]), (23, b[&23])], - heights: &[], + chain: local_chain![(0, blocks[&0]), (13, blocks[&13]), (23, blocks[&23])], + request_heights: &[], exp_update_heights: &[23], }, TestCase { name: "request_nothing_during_reorg", - chain: local_chain![(0, b[&0]), (13, b[&13]), (23, h!("23"))], - heights: &[], + chain: local_chain![(0, blocks[&0]), (13, blocks[&13]), (23, h!("23"))], + request_heights: &[], exp_update_heights: &[13, 23], }, TestCase { name: "request_nothing_during_reorg_2", - chain: local_chain![(0, b[&0]), (21, b[&21]), (22, h!("22")), (23, h!("23"))], - heights: &[], + chain: local_chain![ + (0, blocks[&0]), + (21, blocks[&21]), + (22, h!("22")), + (23, h!("23")) + ], + request_heights: &[], exp_update_heights: &[21, 22, 23], }, TestCase { name: "request_prev_blocks_during_reorg", - chain: local_chain![(0, b[&0]), (21, b[&21]), (22, h!("22")), (23, h!("23"))], - heights: &[17, 20], + chain: local_chain![ + (0, blocks[&0]), + (21, blocks[&21]), + (22, h!("22")), + (23, h!("23")) + ], + request_heights: &[17, 20], exp_update_heights: &[17, 20, 21, 22, 23], }, TestCase { name: "request_later_blocks_during_reorg", - chain: local_chain![(0, b[&0]), (9, b[&9]), (22, h!("22")), (23, h!("23"))], - heights: &[25, 27], + chain: local_chain![ + (0, blocks[&0]), + (9, blocks[&9]), + (22, h!("22")), + (23, h!("23")) + ], + request_heights: &[25, 27], exp_update_heights: &[9, 22, 23, 25, 27], }, TestCase { name: "request_later_blocks_during_reorg_2", - chain: local_chain![(0, b[&0]), (9, h!("9"))], - heights: &[10], + chain: local_chain![(0, blocks[&0]), (9, h!("9"))], + request_heights: &[10], exp_update_heights: &[0, 9, 10], }, TestCase { name: "request_later_and_prev_blocks_during_reorg", - chain: local_chain![(0, b[&0]), (1, b[&1]), (9, h!("9"))], - heights: &[8, 11], + chain: local_chain![(0, blocks[&0]), (1, blocks[&1]), (9, h!("9"))], + request_heights: &[8, 11], exp_update_heights: &[1, 8, 9, 11], }, ]; @@ -337,7 +355,7 @@ fn update_local_chain() -> anyhow::Result<()> { let update = env .client - .update_local_chain(chain.tip(), t.heights.iter().copied()) + .update_local_chain(chain.tip(), t.request_heights.iter().copied()) .map_err(|err| { anyhow::format_err!("[{}:{}] `update_local_chain` failed: {}", i, t.name, err) })?; @@ -352,13 +370,14 @@ fn update_local_chain() -> anyhow::Result<()> { .exp_update_heights .iter() .map(|&height| { - let hash = b[&height]; + let hash = blocks[&height]; BlockId { height, hash } }) .chain( // Electrs Esplora `get_block` call fetches 10 blocks which is included in the // update - b.range(TIP_HEIGHT - 9..) + blocks + .range(TIP_HEIGHT - 9..) .map(|(&height, &hash)| BlockId { height, hash }), ) .collect::>(); @@ -374,8 +393,8 @@ fn update_local_chain() -> anyhow::Result<()> { .unwrap_or_else(|err| panic!("[{}:{}] update failed to apply: {}", i, t.name, err)); // all requested heights must exist in the final chain - for height in t.heights { - let exp_blockhash = b.get(height).expect("block must exist in bitcoind"); + for height in t.request_heights { + let exp_blockhash = blocks.get(height).expect("block must exist in bitcoind"); assert_eq!( chain.blocks().get(height), Some(exp_blockhash),