Skip to content

Commit 8e73998

Browse files
committed
Merge #1380: Simplified EsploraExt API
96a9aa6 feat(chain): refactor `merge_chains` (志宇) 2f22987 chore(chain): fix comment (志宇) daf588f feat(chain): optimize `merge_chains` (志宇) 77d3595 feat(chain)!: rm `local_chain::Update` (志宇) 1269b06 test(chain): fix incorrect test case (志宇) 72fe65b feat(esplora)!: simplify chain update logic (志宇) eded1a7 feat(chain): introduce `CheckPoint::insert` (志宇) 519cd75 test(esplora): move esplora tests into src files (志宇) a6e613e test(esplora): add `test_finalize_chain_update` (志宇) 494d253 feat(testenv): add `genesis_hash` method (志宇) 886d72e chore(chain)!: rm `missing_heights` and `missing_heights_from` methods (志宇) bd62aa0 feat(esplora)!: remove `EsploraExt::update_local_chain` (志宇) 1e99793 feat(testenv): add `make_checkpoint_tip` (志宇) Pull request description: Fixes #1354 ### Description Built on top of both #1369 and #1373, we simplify the `EsploraExt` API by removing the `update_local_chain` method and having `full_scan` and `sync` update the local chain in the same call. The `full_scan` and `sync` methods now takes in an additional input (`local_tip`) which provides us with the view of the `LocalChain` before the update. These methods now return structs `FullScanUpdate` and `SyncUpdate`. The examples are updated to use this new API. `TxGraph::missing_heights` and `tx_graph::ChangeSet::missing_heights_from` are no longer needed, therefore they are removed. Additionally, we used this opportunity to simplify the logic which updates `LocalChain`. We got rid of the `local_chain::Update` struct (which contained the update `CheckPoint` tip and a `bool` which signaled whether we want to introduce blocks below point of agreement). It turns out we can use something like `CheckPoint::insert` so the chain source can craft an update based on the old tip. This way, we can make better use of `merge_chains`' optimization that compares the `Arc` pointers of the local and update chain (before we were crafting the update chain NOT based on top of the previous local chain). With this, we no longer need the `Update::introduce_older_block` field since the logic will naturally break when we reach a matching `Arc` pointer. ### Notes to the reviewers * Obtaining the `LocalChain`'s update now happens within `EsploraExt::full_scan` and `EsploraExt::sync`. Creating the `LocalChain` update is now split into two methods (`fetch_latest_blocks` and `chain_update`) that are called before and after fetching transactions and anchors. * We need to duplicate code for `bdk_esplora`. One for blocking and one for async. ### Changelog notice * Changed `EsploraExt` API so that sync only requires one round of fetching data. The `local_chain_update` method is removed and the `local_tip` parameter is added to the `full_scan` and `sync` methods. * Removed `TxGraph::missing_heights` and `tx_graph::ChangeSet::missing_heights_from` methods. * Introduced `CheckPoint::insert` which allows convenient checkpoint-insertion. This is intended for use by chain-sources when crafting an update. * Refactored `merge_chains` to also return the resultant `CheckPoint` tip. * Optimized the update `LocalChain` logic - use the update `CheckPoint` as the new `CheckPoint` tip when possible. ### Checklists #### All Submissions: * [x] I've signed all my commits * [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md) * [x] I ran `cargo fmt` and `cargo clippy` before committing #### New Features: * [x] I've added tests for the new feature * [x] I've added docs for the new feature ACKs for top commit: LLFourn: ACK 96a9aa6 Tree-SHA512: 3d4f2eab08a1fe94eb578c594126e99679f72e231680b2edd4bfb018ba1d998ca123b07acb2d19c644d5887fc36b8e42badba91cd09853df421ded04de45bf69
2 parents 9800f8d + 96a9aa6 commit 8e73998

File tree

19 files changed

+1606
-1117
lines changed

19 files changed

+1606
-1117
lines changed

crates/bdk/src/wallet/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ pub struct Update {
107107
/// Update for the wallet's internal [`LocalChain`].
108108
///
109109
/// [`LocalChain`]: local_chain::LocalChain
110-
pub chain: Option<local_chain::Update>,
110+
pub chain: Option<CheckPoint>,
111111
}
112112

113113
/// The changes made to a wallet by applying an [`Update`].

crates/bitcoind_rpc/tests/test_emitter.rs

Lines changed: 6 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use bdk_bitcoind_rpc::Emitter;
44
use bdk_chain::{
55
bitcoin::{Address, Amount, Txid},
66
keychain::Balance,
7-
local_chain::{self, CheckPoint, LocalChain},
7+
local_chain::{CheckPoint, LocalChain},
88
Append, BlockId, IndexedTxGraph, SpkTxOutIndex,
99
};
1010
use bdk_testenv::TestEnv;
@@ -47,10 +47,7 @@ pub fn test_sync_local_chain() -> anyhow::Result<()> {
4747
);
4848

4949
assert_eq!(
50-
local_chain.apply_update(local_chain::Update {
51-
tip: emission.checkpoint,
52-
introduce_older_blocks: false,
53-
})?,
50+
local_chain.apply_update(emission.checkpoint,)?,
5451
BTreeMap::from([(height, Some(hash))]),
5552
"chain update changeset is unexpected",
5653
);
@@ -95,10 +92,7 @@ pub fn test_sync_local_chain() -> anyhow::Result<()> {
9592
);
9693

9794
assert_eq!(
98-
local_chain.apply_update(local_chain::Update {
99-
tip: emission.checkpoint,
100-
introduce_older_blocks: false,
101-
})?,
95+
local_chain.apply_update(emission.checkpoint,)?,
10296
if exp_height == exp_hashes.len() - reorged_blocks.len() {
10397
core::iter::once((height, Some(hash)))
10498
.chain((height + 1..exp_hashes.len() as u32).map(|h| (h, None)))
@@ -168,10 +162,7 @@ fn test_into_tx_graph() -> anyhow::Result<()> {
168162

169163
while let Some(emission) = emitter.next_block()? {
170164
let height = emission.block_height();
171-
let _ = chain.apply_update(local_chain::Update {
172-
tip: emission.checkpoint,
173-
introduce_older_blocks: false,
174-
})?;
165+
let _ = chain.apply_update(emission.checkpoint)?;
175166
let indexed_additions = indexed_tx_graph.apply_block_relevant(&emission.block, height);
176167
assert!(indexed_additions.is_empty());
177168
}
@@ -232,10 +223,7 @@ fn test_into_tx_graph() -> anyhow::Result<()> {
232223
{
233224
let emission = emitter.next_block()?.expect("must get mined block");
234225
let height = emission.block_height();
235-
let _ = chain.apply_update(local_chain::Update {
236-
tip: emission.checkpoint,
237-
introduce_older_blocks: false,
238-
})?;
226+
let _ = chain.apply_update(emission.checkpoint)?;
239227
let indexed_additions = indexed_tx_graph.apply_block_relevant(&emission.block, height);
240228
assert!(indexed_additions.graph.txs.is_empty());
241229
assert!(indexed_additions.graph.txouts.is_empty());
@@ -294,8 +282,7 @@ fn process_block(
294282
block: Block,
295283
block_height: u32,
296284
) -> anyhow::Result<()> {
297-
recv_chain
298-
.apply_update(CheckPoint::from_header(&block.header, block_height).into_update(false))?;
285+
recv_chain.apply_update(CheckPoint::from_header(&block.header, block_height))?;
299286
let _ = recv_graph.apply_block(block, block_height);
300287
Ok(())
301288
}

crates/chain/src/local_chain.rs

Lines changed: 120 additions & 105 deletions
Original file line numberDiff line numberDiff line change
@@ -96,16 +96,6 @@ impl CheckPoint {
9696
.expect("must construct checkpoint")
9797
}
9898

99-
/// Convenience method to convert the [`CheckPoint`] into an [`Update`].
100-
///
101-
/// For more information, refer to [`Update`].
102-
pub fn into_update(self, introduce_older_blocks: bool) -> Update {
103-
Update {
104-
tip: self,
105-
introduce_older_blocks,
106-
}
107-
}
108-
10999
/// Puts another checkpoint onto the linked list representing the blockchain.
110100
///
111101
/// Returns an `Err(self)` if the block you are pushing on is not at a greater height that the one you
@@ -187,6 +177,82 @@ impl CheckPoint {
187177
core::ops::Bound::Unbounded => true,
188178
})
189179
}
180+
181+
/// Inserts `block_id` at its height within the chain.
182+
///
183+
/// The effect of `insert` depends on whether a height already exists. If it doesn't the
184+
/// `block_id` we inserted and all pre-existing blocks higher than it will be re-inserted after
185+
/// it. If the height already existed and has a conflicting block hash then it will be purged
186+
/// along with all block followin it. The returned chain will have a tip of the `block_id`
187+
/// passed in. Of course, if the `block_id` was already present then this just returns `self`.
188+
#[must_use]
189+
pub fn insert(self, block_id: BlockId) -> Self {
190+
assert_ne!(block_id.height, 0, "cannot insert the genesis block");
191+
192+
let mut cp = self.clone();
193+
let mut tail = vec![];
194+
let base = loop {
195+
if cp.height() == block_id.height {
196+
if cp.hash() == block_id.hash {
197+
return self;
198+
}
199+
// if we have a conflict we just return the inserted block because the tail is by
200+
// implication invalid.
201+
tail = vec![];
202+
break cp.prev().expect("can't be called on genesis block");
203+
}
204+
205+
if cp.height() < block_id.height {
206+
break cp;
207+
}
208+
209+
tail.push(cp.block_id());
210+
cp = cp.prev().expect("will break before genesis block");
211+
};
212+
213+
base.extend(core::iter::once(block_id).chain(tail.into_iter().rev()))
214+
.expect("tail is in order")
215+
}
216+
217+
/// Apply `changeset` to the checkpoint.
218+
fn apply_changeset(mut self, changeset: &ChangeSet) -> Result<CheckPoint, MissingGenesisError> {
219+
if let Some(start_height) = changeset.keys().next().cloned() {
220+
// changes after point of agreement
221+
let mut extension = BTreeMap::default();
222+
// point of agreement
223+
let mut base: Option<CheckPoint> = None;
224+
225+
for cp in self.iter() {
226+
if cp.height() >= start_height {
227+
extension.insert(cp.height(), cp.hash());
228+
} else {
229+
base = Some(cp);
230+
break;
231+
}
232+
}
233+
234+
for (&height, &hash) in changeset {
235+
match hash {
236+
Some(hash) => {
237+
extension.insert(height, hash);
238+
}
239+
None => {
240+
extension.remove(&height);
241+
}
242+
};
243+
}
244+
245+
let new_tip = match base {
246+
Some(base) => base
247+
.extend(extension.into_iter().map(BlockId::from))
248+
.expect("extension is strictly greater than base"),
249+
None => LocalChain::from_blocks(extension)?.tip(),
250+
};
251+
self = new_tip;
252+
}
253+
254+
Ok(self)
255+
}
190256
}
191257

192258
/// Iterates over checkpoints backwards.
@@ -215,31 +281,6 @@ impl IntoIterator for CheckPoint {
215281
}
216282
}
217283

218-
/// Used to update [`LocalChain`].
219-
///
220-
/// This is used as input for [`LocalChain::apply_update`]. It contains the update's chain `tip` and
221-
/// a flag `introduce_older_blocks` which signals whether this update intends to introduce missing
222-
/// blocks to the original chain.
223-
///
224-
/// Block-by-block syncing mechanisms would typically create updates that builds upon the previous
225-
/// tip. In this case, `introduce_older_blocks` would be `false`.
226-
///
227-
/// Script-pubkey based syncing mechanisms may not introduce transactions in a chronological order
228-
/// so some updates require introducing older blocks (to anchor older transactions). For
229-
/// script-pubkey based syncing, `introduce_older_blocks` would typically be `true`.
230-
#[derive(Debug, Clone, PartialEq)]
231-
pub struct Update {
232-
/// The update chain's new tip.
233-
pub tip: CheckPoint,
234-
235-
/// Whether the update allows for introducing older blocks.
236-
///
237-
/// Refer to [struct-level documentation] for more.
238-
///
239-
/// [struct-level documentation]: Update
240-
pub introduce_older_blocks: bool,
241-
}
242-
243284
/// This is a local implementation of [`ChainOracle`].
244285
#[derive(Debug, Clone, PartialEq)]
245286
pub struct LocalChain {
@@ -347,36 +388,22 @@ impl LocalChain {
347388

348389
/// Applies the given `update` to the chain.
349390
///
350-
/// The method returns [`ChangeSet`] on success. This represents the applied changes to `self`.
391+
/// The method returns [`ChangeSet`] on success. This represents the changes applied to `self`.
351392
///
352393
/// There must be no ambiguity about which of the existing chain's blocks are still valid and
353394
/// which are now invalid. That is, the new chain must implicitly connect to a definite block in
354395
/// the existing chain and invalidate the block after it (if it exists) by including a block at
355396
/// the same height but with a different hash to explicitly exclude it as a connection point.
356397
///
357-
/// Additionally, an empty chain can be updated with any chain, and a chain with a single block
358-
/// can have it's block invalidated by an update chain with a block at the same height but
359-
/// different hash.
360-
///
361398
/// # Errors
362399
///
363400
/// An error will occur if the update does not correctly connect with `self`.
364401
///
365-
/// Refer to [`Update`] for more about the update struct.
366-
///
367402
/// [module-level documentation]: crate::local_chain
368-
pub fn apply_update(&mut self, update: Update) -> Result<ChangeSet, CannotConnectError> {
369-
let changeset = merge_chains(
370-
self.tip.clone(),
371-
update.tip.clone(),
372-
update.introduce_older_blocks,
373-
)?;
374-
// `._check_index_is_consistent_with_tip` and `._check_changeset_is_applied` is called in
375-
// `.apply_changeset`
376-
self.apply_changeset(&changeset)
377-
.map_err(|_| CannotConnectError {
378-
try_include_height: 0,
379-
})?;
403+
pub fn apply_update(&mut self, update: CheckPoint) -> Result<ChangeSet, CannotConnectError> {
404+
let (new_tip, changeset) = merge_chains(self.tip.clone(), update)?;
405+
self.tip = new_tip;
406+
self._check_changeset_is_applied(&changeset);
380407
Ok(changeset)
381408
}
382409

@@ -428,11 +455,8 @@ impl LocalChain {
428455
conn => Some(conn),
429456
};
430457

431-
let update = Update {
432-
tip: CheckPoint::from_block_ids([conn, prev, Some(this)].into_iter().flatten())
433-
.expect("block ids must be in order"),
434-
introduce_older_blocks: false,
435-
};
458+
let update = CheckPoint::from_block_ids([conn, prev, Some(this)].into_iter().flatten())
459+
.expect("block ids must be in order");
436460

437461
self.apply_update(update)
438462
.map_err(ApplyHeaderError::CannotConnect)
@@ -471,43 +495,10 @@ impl LocalChain {
471495

472496
/// Apply the given `changeset`.
473497
pub fn apply_changeset(&mut self, changeset: &ChangeSet) -> Result<(), MissingGenesisError> {
474-
if let Some(start_height) = changeset.keys().next().cloned() {
475-
// changes after point of agreement
476-
let mut extension = BTreeMap::default();
477-
// point of agreement
478-
let mut base: Option<CheckPoint> = None;
479-
480-
for cp in self.iter_checkpoints() {
481-
if cp.height() >= start_height {
482-
extension.insert(cp.height(), cp.hash());
483-
} else {
484-
base = Some(cp);
485-
break;
486-
}
487-
}
488-
489-
for (&height, &hash) in changeset {
490-
match hash {
491-
Some(hash) => {
492-
extension.insert(height, hash);
493-
}
494-
None => {
495-
extension.remove(&height);
496-
}
497-
};
498-
}
499-
500-
let new_tip = match base {
501-
Some(base) => base
502-
.extend(extension.into_iter().map(BlockId::from))
503-
.expect("extension is strictly greater than base"),
504-
None => LocalChain::from_blocks(extension)?.tip(),
505-
};
506-
self.tip = new_tip;
507-
508-
debug_assert!(self._check_changeset_is_applied(changeset));
509-
}
510-
498+
let old_tip = self.tip.clone();
499+
let new_tip = old_tip.apply_changeset(changeset)?;
500+
self.tip = new_tip;
501+
debug_assert!(self._check_changeset_is_applied(changeset));
511502
Ok(())
512503
}
513504

@@ -730,14 +721,17 @@ impl core::fmt::Display for ApplyHeaderError {
730721
#[cfg(feature = "std")]
731722
impl std::error::Error for ApplyHeaderError {}
732723

724+
/// Applies `update_tip` onto `original_tip`.
725+
///
726+
/// On success, a tuple is returned `(changeset, can_replace)`. If `can_replace` is true, then the
727+
/// `update_tip` can replace the `original_tip`.
733728
fn merge_chains(
734729
original_tip: CheckPoint,
735730
update_tip: CheckPoint,
736-
introduce_older_blocks: bool,
737-
) -> Result<ChangeSet, CannotConnectError> {
731+
) -> Result<(CheckPoint, ChangeSet), CannotConnectError> {
738732
let mut changeset = ChangeSet::default();
739-
let mut orig = original_tip.into_iter();
740-
let mut update = update_tip.into_iter();
733+
let mut orig = original_tip.iter();
734+
let mut update = update_tip.iter();
741735
let mut curr_orig = None;
742736
let mut curr_update = None;
743737
let mut prev_orig: Option<CheckPoint> = None;
@@ -746,6 +740,12 @@ fn merge_chains(
746740
let mut prev_orig_was_invalidated = false;
747741
let mut potentially_invalidated_heights = vec![];
748742

743+
// If we can, we want to return the update tip as the new tip because this allows checkpoints
744+
// in multiple locations to keep the same `Arc` pointers when they are being updated from each
745+
// other using this function. We can do this as long as long as the update contains every
746+
// block's height of the original chain.
747+
let mut is_update_height_superset_of_original = true;
748+
749749
// To find the difference between the new chain and the original we iterate over both of them
750750
// from the tip backwards in tandem. We always dealing with the highest one from either chain
751751
// first and move to the next highest. The crucial logic is applied when they have blocks at the
@@ -771,6 +771,8 @@ fn merge_chains(
771771
prev_orig_was_invalidated = false;
772772
prev_orig = curr_orig.take();
773773

774+
is_update_height_superset_of_original = false;
775+
774776
// OPTIMIZATION: we have run out of update blocks so we don't need to continue
775777
// iterating because there's no possibility of adding anything to changeset.
776778
if u.is_none() {
@@ -793,12 +795,20 @@ fn merge_chains(
793795
}
794796
point_of_agreement_found = true;
795797
prev_orig_was_invalidated = false;
796-
// OPTIMIZATION 1 -- If we know that older blocks cannot be introduced without
797-
// invalidation, we can break after finding the point of agreement.
798798
// OPTIMIZATION 2 -- if we have the same underlying pointer at this point, we
799799
// can guarantee that no older blocks are introduced.
800-
if !introduce_older_blocks || Arc::as_ptr(&o.0) == Arc::as_ptr(&u.0) {
801-
return Ok(changeset);
800+
if Arc::as_ptr(&o.0) == Arc::as_ptr(&u.0) {
801+
if is_update_height_superset_of_original {
802+
return Ok((update_tip, changeset));
803+
} else {
804+
let new_tip =
805+
original_tip.apply_changeset(&changeset).map_err(|_| {
806+
CannotConnectError {
807+
try_include_height: 0,
808+
}
809+
})?;
810+
return Ok((new_tip, changeset));
811+
}
802812
}
803813
} else {
804814
// We have an invalidation height so we set the height to the updated hash and
@@ -832,5 +842,10 @@ fn merge_chains(
832842
}
833843
}
834844

835-
Ok(changeset)
845+
let new_tip = original_tip
846+
.apply_changeset(&changeset)
847+
.map_err(|_| CannotConnectError {
848+
try_include_height: 0,
849+
})?;
850+
Ok((new_tip, changeset))
836851
}

0 commit comments

Comments
 (0)