Skip to content

Commit

Permalink
Merge #1369: feat(chain): add get and range methods to CheckPoint
Browse files Browse the repository at this point in the history
53942cc chore(chain)!: rm `From<LocalChain> for BTreeMap<u32, BlockHash>` (志宇)
2d1d95a feat(chain): impl `PartialEq` on `CheckPoint` (志宇)
9a62d56 feat(chain): add `get` and `range` methods to `CheckPoint` (志宇)

Pull request description:

  Partially fixes #1354

  ### Description

  These methods allow us to query for checkpoints contained within the linked list by height and height range. This is useful to determine checkpoints to fetch for chain sources without having to refer back to the `LocalChain`.

  Currently this is not implemented efficiently, but in the future, we will change `CheckPoint` to use a skip list structure.

  ### Notes to the reviewers

  Please refer to the conversation in #1354 for more details. In summary, both `TxGraph::missing_heights` and `tx_graph::ChangeSet::missing_heights_from` are problematic in their own ways. Additionally, it's a better API for chain sources if we only need to lock our receiving structures twice (once to obtain initial state and once for applying the update). Instead of relying on methods such as `EsploraExt::update_local_chain` to get relevant checkpoints, we can use these query methods instead. This allows up to get rid of `::missing_heights`-esc methods and remove the need to lock receiving structures in the middle of the sync.

  ### Changelog notice

  * Added `get` and `range` methods to `CheckPoint` (and in turn, `LocalChain`). This simulates an API where we have implemented a skip list of checkpoints (to implement in the future). This is a better API because we can query for any height or height range with just a checkpoint tip instead of relying on a separate checkpoint index (which needs to live in `LocalChain`).
  * Changed `LocalChain` to have a faster `Eq` implementation. We now maintain an xor value of all checkpoint block hashes. We compare this xor value to determine whether two chains are equal.
  * Added `PartialEq` implementation for `CheckPoint` and `local_chain::Update`.

  ### 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 53942cc

Tree-SHA512: 90ef8047fe1265daa54c9dfe8a8c520685c898a50d18efd6e803707fcb529d0790d20373c9e439b9c7ff301db32b47453020cae7db4da2ea64eba895aa047f30
  • Loading branch information
evanlinjin committed Apr 6, 2024
2 parents 2bb6540 + 53942cc commit 53791eb
Show file tree
Hide file tree
Showing 7 changed files with 198 additions and 116 deletions.
10 changes: 3 additions & 7 deletions crates/bdk/src/wallet/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1127,18 +1127,14 @@ impl<D> Wallet<D> {
// anchor tx to checkpoint with lowest height that is >= position's height
let anchor = self
.chain
.blocks()
.range(height..)
.next()
.last()
.ok_or(InsertTxError::ConfirmationHeightCannotBeGreaterThanTip {
tip_height: self.chain.tip().height(),
tx_height: height,
})
.map(|(&anchor_height, &hash)| ConfirmationTimeHeightAnchor {
anchor_block: BlockId {
height: anchor_height,
hash,
},
.map(|anchor_cp| ConfirmationTimeHeightAnchor {
anchor_block: anchor_cp.block_id(),
confirmation_height: height,
confirmation_time: time,
})?;
Expand Down
18 changes: 12 additions & 6 deletions crates/bitcoind_rpc/tests/test_emitter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,15 @@ pub fn test_sync_local_chain() -> anyhow::Result<()> {
}

assert_eq!(
local_chain.blocks(),
&exp_hashes
local_chain
.iter_checkpoints()
.map(|cp| (cp.height(), cp.hash()))
.collect::<BTreeSet<_>>(),
exp_hashes
.iter()
.enumerate()
.map(|(i, hash)| (i as u32, *hash))
.collect(),
.collect::<BTreeSet<_>>(),
"final local_chain state is unexpected",
);

Expand Down Expand Up @@ -110,12 +113,15 @@ pub fn test_sync_local_chain() -> anyhow::Result<()> {
}

assert_eq!(
local_chain.blocks(),
&exp_hashes
local_chain
.iter_checkpoints()
.map(|cp| (cp.height(), cp.hash()))
.collect::<BTreeSet<_>>(),
exp_hashes
.iter()
.enumerate()
.map(|(i, hash)| (i as u32, *hash))
.collect(),
.collect::<BTreeSet<_>>(),
"final local_chain state is unexpected after reorg",
);

Expand Down
Loading

0 comments on commit 53791eb

Please sign in to comment.