Skip to content

Commit

Permalink
refactor(esplora): better variable naming and docs
Browse files Browse the repository at this point in the history
  • Loading branch information
evanlinjin committed Jan 31, 2024
1 parent 216648b commit 929b5dd
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 43 deletions.
7 changes: 4 additions & 3 deletions crates/esplora/src/async_ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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;
Expand Down
7 changes: 4 additions & 3 deletions crates/esplora/src/blocking_ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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;
Expand Down
93 changes: 56 additions & 37 deletions crates/esplora/tests/blocking_ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -238,95 +238,113 @@ 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::<BTreeMap<_, _>>()
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::<BTreeMap<_, _>>()
};
// so new blocks can be seen by Electrs
let env = env.reset_electrsd()?;

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],
},
];
Expand All @@ -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)
})?;
Expand All @@ -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::<BTreeSet<_>>();
Expand All @@ -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),
Expand Down

0 comments on commit 929b5dd

Please sign in to comment.