From 0f5d71aba6c12be4b7a96c89bbc998ba3f4f597e Mon Sep 17 00:00:00 2001 From: Wei Chen Date: Sat, 31 Aug 2024 19:06:44 +0800 Subject: [PATCH 1/4] refactor(core): `CheckPoint` takes a generic --- crates/bitcoind_rpc/tests/test_emitter.rs | 5 +- crates/core/src/checkpoint.rs | 251 +++++++++++++++------- 2 files changed, 181 insertions(+), 75 deletions(-) diff --git a/crates/bitcoind_rpc/tests/test_emitter.rs b/crates/bitcoind_rpc/tests/test_emitter.rs index 14b0c9212..9133c2374 100644 --- a/crates/bitcoind_rpc/tests/test_emitter.rs +++ b/crates/bitcoind_rpc/tests/test_emitter.rs @@ -280,7 +280,10 @@ fn process_block( block: Block, block_height: u32, ) -> anyhow::Result<()> { - recv_chain.apply_update(CheckPoint::from_header(&block.header, block_height))?; + recv_chain.apply_update(CheckPoint::blockhash_checkpoint_from_header( + &block.header, + block_height, + ))?; let _ = recv_graph.apply_block(block, block_height); Ok(()) } diff --git a/crates/core/src/checkpoint.rs b/crates/core/src/checkpoint.rs index 9bfdf2eed..1943443f0 100644 --- a/crates/core/src/checkpoint.rs +++ b/crates/core/src/checkpoint.rs @@ -1,7 +1,7 @@ use core::ops::RangeBounds; use alloc::sync::Arc; -use bitcoin::BlockHash; +use bitcoin::{block::Header, BlockHash}; use crate::BlockId; @@ -9,16 +9,24 @@ use crate::BlockId; /// /// Checkpoints are cheaply cloneable and are useful to find the agreement point between two sparse /// block chains. -#[derive(Debug, Clone)] -pub struct CheckPoint(Arc); +#[derive(Debug)] +pub struct CheckPoint(Arc>); + +impl Clone for CheckPoint { + fn clone(&self) -> Self { + CheckPoint(Arc::clone(&self.0)) + } +} /// The internal contents of [`CheckPoint`]. -#[derive(Debug, Clone)] -struct CPInner { - /// Block id (hash and height). - block: BlockId, +#[derive(Debug)] +struct CPInner { + /// Block data. + block_id: BlockId, + /// Data. + data: B, /// Previous checkpoint (if any). - prev: Option>, + prev: Option>>, } /// When a `CPInner` is dropped we need to go back down the chain and manually remove any @@ -26,7 +34,7 @@ struct CPInner { /// leads to recursive logic and stack overflows /// /// https://github.com/bitcoindevkit/bdk/issues/1634 -impl Drop for CPInner { +impl Drop for CPInner { fn drop(&mut self) { // Take out `prev` so its `drop` won't be called when this drop is finished let mut current = self.prev.take(); @@ -49,40 +57,51 @@ impl Drop for CPInner { } } -impl PartialEq for CheckPoint { +/// Trait that converts [`CheckPoint`] `data` to [`BlockHash`]. +pub trait ToBlockHash { + /// Returns the [`BlockHash`] for the associated [`CheckPoint`] `data` type. + fn to_blockhash(&self) -> BlockHash; +} + +impl ToBlockHash for BlockHash { + fn to_blockhash(&self) -> BlockHash { + *self + } +} + +impl ToBlockHash for Header { + fn to_blockhash(&self) -> BlockHash { + self.block_hash() + } +} + +impl PartialEq for CheckPoint +where + B: core::cmp::PartialEq, +{ fn eq(&self, other: &Self) -> bool { - let self_cps = self.iter().map(|cp| cp.block_id()); - let other_cps = other.iter().map(|cp| cp.block_id()); + let self_cps = self.iter().map(|cp| cp.0.block_id); + let other_cps = other.iter().map(|cp| cp.0.block_id); self_cps.eq(other_cps) } } -impl CheckPoint { - /// Construct a new base block at the front of a linked list. +impl CheckPoint { + /// Construct a new base [`CheckPoint`] at the front of a linked list. pub fn new(block: BlockId) -> Self { - Self(Arc::new(CPInner { block, prev: None })) + CheckPoint::from_data(block.height, block.hash) } - /// Construct a checkpoint from a list of [`BlockId`]s in ascending height order. - /// - /// # Errors - /// - /// This method will error if any of the follow occurs: - /// - /// - The `blocks` iterator is empty, in which case, the error will be `None`. - /// - The `blocks` iterator is not in ascending height order. - /// - The `blocks` iterator contains multiple [`BlockId`]s of the same height. + /// Construct a checkpoint from the given `header` and block `height`. /// - /// The error type is the last successful checkpoint constructed (if any). - pub fn from_block_ids( - block_ids: impl IntoIterator, - ) -> Result> { - let mut blocks = block_ids.into_iter(); - let mut acc = CheckPoint::new(blocks.next().ok_or(None)?); - for id in blocks { - acc = acc.push(id).map_err(Some)?; - } - Ok(acc) + /// If `header` is of the genesis block, the checkpoint won't have a `prev` node. Otherwise, + /// we return a checkpoint linked with the previous block. + #[deprecated( + since = "0.4.1", + note = "Please use [`CheckPoint::blockhash_checkpoint_from_header`] instead. To create a CheckPoint
, please use [`CheckPoint::from_data`]." + )] + pub fn from_header(header: &bitcoin::block::Header, height: u32) -> Self { + CheckPoint::blockhash_checkpoint_from_header(header, height) } /// Construct a checkpoint from the given `header` and block `height`. @@ -91,7 +110,7 @@ impl CheckPoint { /// we return a checkpoint linked with the previous block. /// /// [`prev`]: CheckPoint::prev - pub fn from_header(header: &bitcoin::block::Header, height: u32) -> Self { + pub fn blockhash_checkpoint_from_header(header: &bitcoin::block::Header, height: u32) -> Self { let hash = header.block_hash(); let this_block_id = BlockId { height, hash }; @@ -110,55 +129,90 @@ impl CheckPoint { .expect("must construct checkpoint") } - /// Puts another checkpoint onto the linked list representing the blockchain. + /// Construct a checkpoint from a list of [`BlockId`]s in ascending height order. /// - /// Returns an `Err(self)` if the block you are pushing on is not at a greater height that the one you - /// are pushing on to. - pub fn push(self, block: BlockId) -> Result { - if self.height() < block.height { - Ok(Self(Arc::new(CPInner { - block, - prev: Some(self.0), - }))) - } else { - Err(self) + /// # Errors + /// + /// This method will error if any of the follow occurs: + /// + /// - The `blocks` iterator is empty, in which case, the error will be `None`. + /// - The `blocks` iterator is not in ascending height order. + /// - The `blocks` iterator contains multiple [`BlockId`]s of the same height. + /// + /// The error type is the last successful checkpoint constructed (if any). + pub fn from_block_ids( + block_ids: impl IntoIterator, + ) -> Result> { + let mut blocks = block_ids.into_iter(); + let block = blocks.next().ok_or(None)?; + let mut acc = CheckPoint::new(block); + for id in blocks { + acc = acc.push(id).map_err(Some)?; } + Ok(acc) } /// Extends the checkpoint linked list by a iterator of block ids. /// /// Returns an `Err(self)` if there is block which does not have a greater height than the /// previous one. - pub fn extend(self, blocks: impl IntoIterator) -> Result { - let mut curr = self.clone(); - for block in blocks { - curr = curr.push(block).map_err(|_| self.clone())?; - } - Ok(curr) + pub fn extend(self, blockdata: impl IntoIterator) -> Result { + self.extend_data( + blockdata + .into_iter() + .map(|block| (block.height, block.hash)), + ) + } + + /// Inserts `block_id` at its height within the chain. + /// + /// The effect of `insert` depends on whether a height already exists. If it doesn't the + /// `block_id` we inserted and all pre-existing blocks higher than it will be re-inserted after + /// it. If the height already existed and has a conflicting block hash then it will be purged + /// along with all blocks following it. The returned chain will have a tip of the `block_id` + /// passed in. Of course, if the `block_id` was already present then this just returns `self`. + #[must_use] + pub fn insert(self, block_id: BlockId) -> Self { + self.insert_data(block_id.height, block_id.hash) + } + + /// Puts another checkpoint onto the linked list representing the blockchain. + /// + /// Returns an `Err(self)` if the block you are pushing on is not at a greater height that the one you + /// are pushing on to. + pub fn push(self, block: BlockId) -> Result { + self.push_data(block.height, block.hash) + } +} + +impl CheckPoint { + /// Get the `data` of the checkpoint. + pub fn data(&self) -> &B { + &self.0.data } /// Get the [`BlockId`] of the checkpoint. pub fn block_id(&self) -> BlockId { - self.0.block + self.0.block_id } - /// Get the height of the checkpoint. + /// Get the `height` of the checkpoint. pub fn height(&self) -> u32 { - self.0.block.height + self.0.block_id.height } /// Get the block hash of the checkpoint. pub fn hash(&self) -> BlockHash { - self.0.block.hash + self.0.block_id.hash } - /// Get the previous checkpoint in the chain - pub fn prev(&self) -> Option { + /// Get the previous checkpoint in the chain. + pub fn prev(&self) -> Option> { self.0.prev.clone().map(CheckPoint) } /// Iterate from this checkpoint in descending height. - pub fn iter(&self) -> CheckPointIter { + pub fn iter(&self) -> CheckPointIter { self.clone().into_iter() } @@ -173,7 +227,7 @@ impl CheckPoint { /// /// Note that we always iterate checkpoints in reverse height order (iteration starts at tip /// height). - pub fn range(&self, range: R) -> impl Iterator + pub fn range(&self, range: R) -> impl Iterator> where R: RangeBounds, { @@ -191,8 +245,38 @@ impl CheckPoint { core::ops::Bound::Unbounded => true, }) } +} - /// Inserts `block_id` at its height within the chain. +impl CheckPoint +where + B: Copy + core::fmt::Debug + ToBlockHash, +{ + /// Construct a new base [`CheckPoint`] from given `height` and `data` at the front of a linked + /// list. + pub fn from_data(height: u32, data: B) -> Self { + Self(Arc::new(CPInner { + block_id: BlockId { + height, + hash: data.to_blockhash(), + }, + data, + prev: None, + })) + } + + /// Extends the checkpoint linked list by a iterator containing `height` and `data`. + /// + /// Returns an `Err(self)` if there is block which does not have a greater height than the + /// previous one. + pub fn extend_data(self, blockdata: impl IntoIterator) -> Result { + let mut curr = self.clone(); + for (height, data) in blockdata { + curr = curr.push_data(height, data).map_err(|_| self.clone())?; + } + Ok(curr) + } + + /// Inserts `data` at its `height` within the chain. /// /// The effect of `insert` depends on whether a height already exists. If it doesn't the /// `block_id` we inserted and all pre-existing blocks higher than it will be re-inserted after @@ -204,12 +288,12 @@ impl CheckPoint { /// /// This panics if called with a genesis block that differs from that of `self`. #[must_use] - pub fn insert(self, block_id: BlockId) -> Self { + pub fn insert_data(self, height: u32, data: B) -> Self { let mut cp = self.clone(); let mut tail = vec![]; let base = loop { - if cp.height() == block_id.height { - if cp.hash() == block_id.hash { + if cp.height() == height { + if cp.hash() == data.to_blockhash() { return self; } assert_ne!(cp.height(), 0, "cannot replace genesis block"); @@ -219,18 +303,37 @@ impl CheckPoint { break cp.prev().expect("can't be called on genesis block"); } - if cp.height() < block_id.height { + if cp.height() < height { break cp; } - tail.push(cp.block_id()); + tail.push((cp.height(), *cp.data())); cp = cp.prev().expect("will break before genesis block"); }; - base.extend(core::iter::once(block_id).chain(tail.into_iter().rev())) + base.extend_data(core::iter::once((height, data)).chain(tail.into_iter().rev())) .expect("tail is in order") } + /// Puts another checkpoint onto the linked list representing the blockchain. + /// + /// Returns an `Err(self)` if the block you are pushing on is not at a greater height that the one you + /// are pushing on to. + pub fn push_data(self, height: u32, data: B) -> Result { + if self.height() < height { + Ok(Self(Arc::new(CPInner { + block_id: BlockId { + height, + hash: data.to_blockhash(), + }, + data, + prev: Some(self.0), + }))) + } else { + Err(self) + } + } + /// This method tests for `self` and `other` to have equal internal pointers. pub fn eq_ptr(&self, other: &Self) -> bool { Arc::as_ptr(&self.0) == Arc::as_ptr(&other.0) @@ -238,12 +341,12 @@ impl CheckPoint { } /// Iterates over checkpoints backwards. -pub struct CheckPointIter { - current: Option>, +pub struct CheckPointIter { + current: Option>>, } -impl Iterator for CheckPointIter { - type Item = CheckPoint; +impl Iterator for CheckPointIter { + type Item = CheckPoint; fn next(&mut self) -> Option { let current = self.current.clone()?; @@ -252,9 +355,9 @@ impl Iterator for CheckPointIter { } } -impl IntoIterator for CheckPoint { - type Item = CheckPoint; - type IntoIter = CheckPointIter; +impl IntoIterator for CheckPoint { + type Item = CheckPoint; + type IntoIter = CheckPointIter; fn into_iter(self) -> Self::IntoIter { CheckPointIter { From 380cdac9a2b6180410f5ad89cc09800b082b5613 Mon Sep 17 00:00:00 2001 From: Wei Chen Date: Thu, 5 Sep 2024 11:41:33 +0800 Subject: [PATCH 2/4] refactor(chain): `LocalChain` takes a generic --- crates/chain/src/local_chain.rs | 296 ++++++++++++++++++++------------ 1 file changed, 183 insertions(+), 113 deletions(-) diff --git a/crates/chain/src/local_chain.rs b/crates/chain/src/local_chain.rs index b9a1b645c..9b5bc9755 100644 --- a/crates/chain/src/local_chain.rs +++ b/crates/chain/src/local_chain.rs @@ -10,29 +10,32 @@ use bitcoin::block::Header; use bitcoin::BlockHash; /// Apply `changeset` to the checkpoint. -fn apply_changeset_to_checkpoint( - mut init_cp: CheckPoint, - changeset: &ChangeSet, -) -> Result { +fn apply_changeset_to_checkpoint( + mut init_cp: CheckPoint, + changeset: &ChangeSet, +) -> Result, MissingGenesisError> +where + B: Copy + core::fmt::Debug + bdk_core::ToBlockHash, +{ if let Some(start_height) = changeset.blocks.keys().next().cloned() { // changes after point of agreement let mut extension = BTreeMap::default(); // point of agreement - let mut base: Option = None; + let mut base: Option> = None; for cp in init_cp.iter() { if cp.height() >= start_height { - extension.insert(cp.height(), cp.hash()); + extension.insert(cp.height(), *cp.data()); } else { base = Some(cp); break; } } - for (&height, &hash) in &changeset.blocks { - match hash { - Some(hash) => { - extension.insert(height, hash); + for (&height, &data) in &changeset.blocks { + match data { + Some(data) => { + extension.insert(height, data); } None => { extension.remove(&height); @@ -42,9 +45,9 @@ fn apply_changeset_to_checkpoint( let new_tip = match base { Some(base) => base - .extend(extension.into_iter().map(BlockId::from)) + .extend_data(extension) .expect("extension is strictly greater than base"), - None => LocalChain::from_blocks(extension)?.tip(), + None => LocalChain::from_data(extension)?.tip(), }; init_cp = new_tip; } @@ -53,12 +56,24 @@ fn apply_changeset_to_checkpoint( } /// This is a local implementation of [`ChainOracle`]. -#[derive(Debug, Clone, PartialEq)] -pub struct LocalChain { - tip: CheckPoint, +#[derive(Debug, Clone)] +pub struct LocalChain { + tip: CheckPoint, } -impl ChainOracle for LocalChain { +impl PartialEq for LocalChain +where + B: Copy + core::cmp::PartialEq, +{ + fn eq(&self, other: &Self) -> bool { + self.tip == other.tip + } +} + +impl ChainOracle for LocalChain +where + B: Copy, +{ type Error = Infallible; fn is_block_in_chain( @@ -83,18 +98,13 @@ impl ChainOracle for LocalChain { } } -impl LocalChain { - /// Get the genesis hash. - pub fn genesis_hash(&self) -> BlockHash { - self.tip.get(0).expect("genesis must exist").hash() - } - +impl LocalChain { /// Construct [`LocalChain`] from genesis `hash`. #[must_use] pub fn from_genesis_hash(hash: BlockHash) -> (Self, ChangeSet) { let height = 0; let chain = Self { - tip: CheckPoint::new(BlockId { height, hash }), + tip: CheckPoint::from_data(height, hash), }; let changeset = chain.initial_changeset(); (chain, changeset) @@ -116,45 +126,12 @@ impl LocalChain { Ok(chain) } - /// Construct a [`LocalChain`] from a given `checkpoint` tip. - pub fn from_tip(tip: CheckPoint) -> Result { - let genesis_cp = tip.iter().last().expect("must have at least one element"); - if genesis_cp.height() != 0 { - return Err(MissingGenesisError); - } - Ok(Self { tip }) - } - /// Constructs a [`LocalChain`] from a [`BTreeMap`] of height to [`BlockHash`]. /// /// The [`BTreeMap`] enforces the height order. However, the caller must ensure the blocks are /// all of the same chain. pub fn from_blocks(blocks: BTreeMap) -> Result { - if !blocks.contains_key(&0) { - return Err(MissingGenesisError); - } - - let mut tip: Option = None; - for block in &blocks { - match tip { - Some(curr) => { - tip = Some( - curr.push(BlockId::from(block)) - .expect("BTreeMap is ordered"), - ) - } - None => tip = Some(CheckPoint::new(BlockId::from(block))), - } - } - - Ok(Self { - tip: tip.expect("already checked to have genesis"), - }) - } - - /// Get the highest checkpoint. - pub fn tip(&self) -> CheckPoint { - self.tip.clone() + LocalChain::from_data(blocks) } /// Applies the given `update` to the chain. @@ -266,11 +243,7 @@ impl LocalChain { /// Apply the given `changeset`. pub fn apply_changeset(&mut self, changeset: &ChangeSet) -> Result<(), MissingGenesisError> { - let old_tip = self.tip.clone(); - let new_tip = apply_changeset_to_checkpoint(old_tip, changeset)?; - self.tip = new_tip; - debug_assert!(self._check_changeset_is_applied(changeset)); - Ok(()) + self.apply_data_changeset(changeset) } /// Insert a [`BlockId`]. @@ -279,29 +252,7 @@ impl LocalChain { /// /// Replacing the block hash of an existing checkpoint will result in an error. pub fn insert_block(&mut self, block_id: BlockId) -> Result { - if let Some(original_cp) = self.tip.get(block_id.height) { - let original_hash = original_cp.hash(); - if original_hash != block_id.hash { - return Err(AlterCheckPointError { - height: block_id.height, - original_hash, - update_hash: Some(block_id.hash), - }); - } - return Ok(ChangeSet::default()); - } - - let mut changeset = ChangeSet::default(); - changeset - .blocks - .insert(block_id.height, Some(block_id.hash)); - self.apply_changeset(&changeset) - .map_err(|_| AlterCheckPointError { - height: 0, - original_hash: self.genesis_hash(), - update_hash: changeset.blocks.get(&0).cloned().flatten(), - })?; - Ok(changeset) + self.insert_data(block_id.height, block_id.hash) } /// Removes blocks from (and inclusive of) the given `block_id`. @@ -360,23 +311,28 @@ impl LocalChain { } fn _check_changeset_is_applied(&self, changeset: &ChangeSet) -> bool { - let mut curr_cp = self.tip.clone(); - for (height, exp_hash) in changeset.blocks.iter().rev() { - match curr_cp.get(*height) { - Some(query_cp) => { - if query_cp.height() != *height || Some(query_cp.hash()) != *exp_hash { - return false; - } - curr_cp = query_cp; - } - None => { - if exp_hash.is_some() { - return false; - } - } - } + self._check_data_changeset_is_applied(changeset) + } +} + +impl LocalChain { + /// Get the highest checkpoint. + pub fn tip(&self) -> CheckPoint { + self.tip.clone() + } + + /// Get the genesis hash. + pub fn genesis_hash(&self) -> BlockHash { + self.tip.get(0).expect("genesis must exist").block_id().hash + } + + /// Construct a [`LocalChain`] from a given `checkpoint` tip. + pub fn from_tip(tip: CheckPoint) -> Result { + let genesis_cp = tip.iter().last().expect("must have at least one element"); + if genesis_cp.height() != 0 { + return Err(MissingGenesisError); } - true + Ok(Self { tip }) } /// Get checkpoint at given `height` (if it exists). @@ -384,7 +340,7 @@ impl LocalChain { /// This is a shorthand for calling [`CheckPoint::get`] on the [`tip`]. /// /// [`tip`]: LocalChain::tip - pub fn get(&self, height: u32) -> Option { + pub fn get(&self, height: u32) -> Option> { self.tip.get(height) } @@ -396,7 +352,7 @@ impl LocalChain { /// This is a shorthand for calling [`CheckPoint::range`] on the [`tip`]. /// /// [`tip`]: LocalChain::tip - pub fn range(&self, range: R) -> impl Iterator + pub fn range(&self, range: R) -> impl Iterator> where R: RangeBounds, { @@ -404,18 +360,132 @@ impl LocalChain { } } +impl LocalChain +where + B: Copy + core::fmt::Debug + bdk_core::ToBlockHash, +{ + /// Apply the given `changeset`. + pub fn apply_data_changeset( + &mut self, + changeset: &ChangeSet, + ) -> Result<(), MissingGenesisError> { + let old_tip = self.tip.clone(); + let new_tip = apply_changeset_to_checkpoint(old_tip, changeset)?; + self.tip = new_tip; + debug_assert!(self._check_data_changeset_is_applied(changeset)); + Ok(()) + } + + fn _check_data_changeset_is_applied(&self, changeset: &ChangeSet) -> bool { + let mut curr_cp = self.tip.clone(); + for (height, data) in changeset.blocks.iter().rev() { + match curr_cp.get(*height) { + Some(query_cp) => { + if let Some(data) = data { + if query_cp.height() != *height || query_cp.hash() != data.to_blockhash() { + return false; + } + } else { + return false; + } + curr_cp = query_cp; + } + None => { + if data.is_some() { + return false; + } + } + } + } + true + } + + /// Insert data into a [`LocalChain`]. + /// + /// # Errors + /// + /// Replacing the block hash of an existing checkpoint will result in an error. + pub fn insert_data( + &mut self, + height: u32, + data: B, + ) -> Result, AlterCheckPointError> { + if let Some(original_cp) = self.tip.get(height) { + let original_hash = original_cp.hash(); + if original_hash != data.to_blockhash() { + return Err(AlterCheckPointError { + height, + original_hash, + update_hash: Some(data.to_blockhash()), + }); + } + return Ok(ChangeSet::default()); + } + + let mut changeset = ChangeSet::::default(); + changeset.blocks.insert(height, Some(data)); + self.apply_data_changeset(&changeset) + .map_err(|_| AlterCheckPointError { + height: 0, + original_hash: self.genesis_hash(), + update_hash: Some( + changeset + .blocks + .get(&0) + .cloned() + .flatten() + .unwrap() + .to_blockhash(), + ), + })?; + Ok(changeset) + } + + /// Constructs a [`LocalChain`] from a [`BTreeMap`] of height and data. + /// + /// The [`BTreeMap`] enforces the height order. However, the caller must ensure the blocks are + /// all of the same chain. + pub fn from_data(blocks: BTreeMap) -> Result { + if !blocks.contains_key(&0) { + return Err(MissingGenesisError); + } + + let mut tip: Option> = None; + for (height, data) in &blocks { + match tip { + Some(curr) => { + tip = Some(curr.push_data(*height, *data).expect("BTreeMap is ordered")) + } + None => tip = Some(CheckPoint::from_data(*height, *data)), + } + } + + Ok(Self { + tip: tip.expect("already checked to have genesis"), + }) + } +} + /// The [`ChangeSet`] represents changes to [`LocalChain`]. -#[derive(Debug, Default, Clone, PartialEq)] +#[derive(Debug, Clone, PartialEq)] #[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))] -pub struct ChangeSet { +pub struct ChangeSet { /// Changes to the [`LocalChain`] blocks. /// /// The key represents the block height, and the value either represents added a new [`CheckPoint`] /// (if [`Some`]), or removing a [`CheckPoint`] (if [`None`]). - pub blocks: BTreeMap>, + pub blocks: BTreeMap>, +} + +impl Default for ChangeSet { + fn default() -> Self { + ChangeSet { + blocks: BTreeMap::default(), + } + } } -impl Merge for ChangeSet { +impl Merge for ChangeSet { fn merge(&mut self, other: Self) { Merge::merge(&mut self.blocks, other.blocks) } @@ -425,24 +495,24 @@ impl Merge for ChangeSet { } } -impl)>> From for ChangeSet { - fn from(blocks: B) -> Self { +impl)>> From for ChangeSet { + fn from(blocks: I) -> Self { Self { blocks: blocks.into_iter().collect(), } } } -impl FromIterator<(u32, Option)> for ChangeSet { - fn from_iter)>>(iter: T) -> Self { +impl FromIterator<(u32, Option)> for ChangeSet { + fn from_iter)>>(iter: T) -> Self { Self { blocks: iter.into_iter().collect(), } } } -impl FromIterator<(u32, BlockHash)> for ChangeSet { - fn from_iter>(iter: T) -> Self { +impl FromIterator<(u32, B)> for ChangeSet { + fn from_iter>(iter: T) -> Self { Self { blocks: iter .into_iter() From e23184bdd0e7dc05cb044bc1bcaec2a386907ad4 Mon Sep 17 00:00:00 2001 From: Wei Chen Date: Thu, 5 Sep 2024 11:42:59 +0800 Subject: [PATCH 3/4] refactor(core): `spk_client` types take generics --- crates/core/src/spk_client.rs | 100 ++++++++++++++++++---------------- 1 file changed, 52 insertions(+), 48 deletions(-) diff --git a/crates/core/src/spk_client.rs b/crates/core/src/spk_client.rs index a5ec813c9..407c9a50a 100644 --- a/crates/core/src/spk_client.rs +++ b/crates/core/src/spk_client.rs @@ -4,7 +4,7 @@ use crate::{ collections::BTreeMap, CheckPoint, ConfirmationBlockTime, Indexed, }; -use bitcoin::{OutPoint, Script, ScriptBuf, Txid}; +use bitcoin::{BlockHash, OutPoint, Script, ScriptBuf, Txid}; type InspectSync = dyn FnMut(SyncItem, SyncProgress) + Send + 'static; @@ -88,11 +88,11 @@ impl SyncProgress { /// Builds a [`SyncRequest`]. #[must_use] -pub struct SyncRequestBuilder { - inner: SyncRequest, +pub struct SyncRequestBuilder { + inner: SyncRequest, } -impl Default for SyncRequestBuilder { +impl Default for SyncRequestBuilder { fn default() -> Self { Self { inner: Default::default(), @@ -107,11 +107,11 @@ impl SyncRequestBuilder<()> { } } -impl SyncRequestBuilder { +impl SyncRequestBuilder { /// Set the initial chain tip for the sync request. /// /// This is used to update [`LocalChain`](../../bdk_chain/local_chain/struct.LocalChain.html). - pub fn chain_tip(mut self, cp: CheckPoint) -> Self { + pub fn chain_tip(mut self, cp: CheckPoint) -> Self { self.inner.chain_tip = Some(cp); self } @@ -124,6 +124,7 @@ impl SyncRequestBuilder { /// [`KeychainTxOutIndex`](../../bdk_chain/indexer/keychain_txout/struct.KeychainTxOutIndex.html). /// /// ```rust + /// # use bdk_chain::bitcoin::BlockHash; /// # use bdk_chain::spk_client::SyncRequest; /// # use bdk_chain::indexer::keychain_txout::KeychainTxOutIndex; /// # use bdk_chain::miniscript::{Descriptor, DescriptorPublicKey}; @@ -141,7 +142,7 @@ impl SyncRequestBuilder { /// let (newly_revealed_spks, _changeset) = indexer /// .reveal_to_target("descriptor_a", 21) /// .expect("keychain must exist"); - /// let _request = SyncRequest::builder() + /// let _request: SyncRequest = SyncRequest::builder() /// .spks_with_indexes(newly_revealed_spks) /// .build(); /// @@ -149,7 +150,7 @@ impl SyncRequestBuilder { /// // keychains. Each spk will be indexed with `(&'static str, u32)` where `&'static str` is /// // the keychain identifier and `u32` is the derivation index. /// let all_revealed_spks = indexer.revealed_spks(..); - /// let _request = SyncRequest::builder() + /// let _request: SyncRequest<(&str, u32), BlockHash> = SyncRequest::builder() /// .spks_with_indexes(all_revealed_spks) /// .build(); /// # Ok::<_, bdk_chain::keychain_txout::InsertDescriptorError<_>>(()) @@ -181,7 +182,7 @@ impl SyncRequestBuilder { } /// Build the [`SyncRequest`]. - pub fn build(self) -> SyncRequest { + pub fn build(self) -> SyncRequest { self.inner } } @@ -209,8 +210,8 @@ impl SyncRequestBuilder { /// .build(); /// ``` #[must_use] -pub struct SyncRequest { - chain_tip: Option, +pub struct SyncRequest { + chain_tip: Option>, spks: VecDeque<(I, ScriptBuf)>, spks_consumed: usize, txids: VecDeque, @@ -220,7 +221,7 @@ pub struct SyncRequest { inspect: Box>, } -impl Default for SyncRequest { +impl Default for SyncRequest { fn default() -> Self { Self { chain_tip: None, @@ -235,15 +236,15 @@ impl Default for SyncRequest { } } -impl From> for SyncRequest { - fn from(builder: SyncRequestBuilder) -> Self { +impl From> for SyncRequest { + fn from(builder: SyncRequestBuilder) -> Self { builder.inner } } -impl SyncRequest { +impl SyncRequest { /// Start building a [`SyncRequest`]. - pub fn builder() -> SyncRequestBuilder { + pub fn builder() -> SyncRequestBuilder { SyncRequestBuilder { inner: Default::default(), } @@ -262,7 +263,7 @@ impl SyncRequest { } /// Get the chain tip [`CheckPoint`] of this request (if any). - pub fn chain_tip(&self) -> Option { + pub fn chain_tip(&self) -> Option> { self.chain_tip.clone() } @@ -298,17 +299,17 @@ impl SyncRequest { /// Iterate over [`ScriptBuf`]s contained in this request. pub fn iter_spks(&mut self) -> impl ExactSizeIterator + '_ { - SyncIter::::new(self) + SyncIter::::new(self) } /// Iterate over [`Txid`]s contained in this request. pub fn iter_txids(&mut self) -> impl ExactSizeIterator + '_ { - SyncIter::::new(self) + SyncIter::::new(self) } /// Iterate over [`OutPoint`]s contained in this request. pub fn iter_outpoints(&mut self) -> impl ExactSizeIterator + '_ { - SyncIter::::new(self) + SyncIter::::new(self) } fn _call_inspect(&mut self, item: SyncItem) { @@ -322,14 +323,14 @@ impl SyncRequest { /// See also [`SyncRequest`]. #[must_use] #[derive(Debug)] -pub struct SyncResponse { +pub struct SyncResponse { /// Relevant transaction data discovered during the scan. pub tx_update: crate::TxUpdate, /// Changes to the chain discovered during the scan. - pub chain_update: Option, + pub chain_update: Option>, } -impl Default for SyncResponse { +impl Default for SyncResponse { fn default() -> Self { Self { tx_update: Default::default(), @@ -340,11 +341,11 @@ impl Default for SyncResponse { /// Builds a [`FullScanRequest`]. #[must_use] -pub struct FullScanRequestBuilder { - inner: FullScanRequest, +pub struct FullScanRequestBuilder { + inner: FullScanRequest, } -impl Default for FullScanRequestBuilder { +impl Default for FullScanRequestBuilder { fn default() -> Self { Self { inner: Default::default(), @@ -352,11 +353,11 @@ impl Default for FullScanRequestBuilder { } } -impl FullScanRequestBuilder { +impl FullScanRequestBuilder { /// Set the initial chain tip for the full scan request. /// /// This is used to update [`LocalChain`](../../bdk_chain/local_chain/struct.LocalChain.html). - pub fn chain_tip(mut self, tip: CheckPoint) -> Self { + pub fn chain_tip(mut self, tip: CheckPoint) -> Self { self.inner.chain_tip = Some(tip); self } @@ -383,7 +384,7 @@ impl FullScanRequestBuilder { } /// Build the [`FullScanRequest`]. - pub fn build(self) -> FullScanRequest { + pub fn build(self) -> FullScanRequest { self.inner } } @@ -396,19 +397,19 @@ impl FullScanRequestBuilder { /// used scripts is not known. The full scan process also updates the chain from the given /// [`chain_tip`](FullScanRequestBuilder::chain_tip) (if provided). #[must_use] -pub struct FullScanRequest { - chain_tip: Option, +pub struct FullScanRequest { + chain_tip: Option>, spks_by_keychain: BTreeMap> + Send>>, inspect: Box>, } -impl From> for FullScanRequest { - fn from(builder: FullScanRequestBuilder) -> Self { +impl From> for FullScanRequest { + fn from(builder: FullScanRequestBuilder) -> Self { builder.inner } } -impl Default for FullScanRequest { +impl Default for FullScanRequest { fn default() -> Self { Self { chain_tip: None, @@ -418,16 +419,16 @@ impl Default for FullScanRequest { } } -impl FullScanRequest { +impl FullScanRequest { /// Start building a [`FullScanRequest`]. - pub fn builder() -> FullScanRequestBuilder { + pub fn builder() -> FullScanRequestBuilder { FullScanRequestBuilder { inner: Self::default(), } } /// Get the chain tip [`CheckPoint`] of this request (if any). - pub fn chain_tip(&self) -> Option { + pub fn chain_tip(&self) -> Option> { self.chain_tip.clone() } @@ -459,17 +460,17 @@ impl FullScanRequest { /// See also [`FullScanRequest`]. #[must_use] #[derive(Debug)] -pub struct FullScanResponse { +pub struct FullScanResponse { /// Relevant transaction data discovered during the scan. pub tx_update: crate::TxUpdate, /// Last active indices for the corresponding keychains (`K`). An index is active if it had a /// transaction associated with the script pubkey at that index. pub last_active_indices: BTreeMap, /// Changes to the chain discovered during the scan. - pub chain_update: Option, + pub chain_update: Option>, } -impl Default for FullScanResponse { +impl Default for FullScanResponse { fn default() -> Self { Self { tx_update: Default::default(), @@ -495,13 +496,13 @@ impl Iterator for KeychainSpkIter<'_, K> { } } -struct SyncIter<'r, I, Item> { - request: &'r mut SyncRequest, +struct SyncIter<'r, I, B, Item> { + request: &'r mut SyncRequest, marker: core::marker::PhantomData, } -impl<'r, I, Item> SyncIter<'r, I, Item> { - fn new(request: &'r mut SyncRequest) -> Self { +impl<'r, I, B, Item> SyncIter<'r, I, B, Item> { + fn new(request: &'r mut SyncRequest) -> Self { Self { request, marker: core::marker::PhantomData, @@ -509,9 +510,12 @@ impl<'r, I, Item> SyncIter<'r, I, Item> { } } -impl<'r, I, Item> ExactSizeIterator for SyncIter<'r, I, Item> where SyncIter<'r, I, Item>: Iterator {} +impl<'r, I, B, Item> ExactSizeIterator for SyncIter<'r, I, B, Item> where + SyncIter<'r, I, B, Item>: Iterator +{ +} -impl Iterator for SyncIter<'_, I, ScriptBuf> { +impl Iterator for SyncIter<'_, I, B, ScriptBuf> { type Item = ScriptBuf; fn next(&mut self) -> Option { @@ -524,7 +528,7 @@ impl Iterator for SyncIter<'_, I, ScriptBuf> { } } -impl Iterator for SyncIter<'_, I, Txid> { +impl Iterator for SyncIter<'_, I, B, Txid> { type Item = Txid; fn next(&mut self) -> Option { @@ -537,7 +541,7 @@ impl Iterator for SyncIter<'_, I, Txid> { } } -impl Iterator for SyncIter<'_, I, OutPoint> { +impl Iterator for SyncIter<'_, I, B, OutPoint> { type Item = OutPoint; fn next(&mut self) -> Option { From 179ec5e4709a08a1f6a252ed3c42e656fd10a2d8 Mon Sep 17 00:00:00 2001 From: Wei Chen Date: Tue, 10 Sep 2024 23:36:34 +0800 Subject: [PATCH 4/4] test(chain): Add test for inserting `Header` into `LocalChain` --- crates/chain/tests/test_local_chain.rs | 86 ++++++++++++++++++++++++++ 1 file changed, 86 insertions(+) diff --git a/crates/chain/tests/test_local_chain.rs b/crates/chain/tests/test_local_chain.rs index a0b8220ee..e845c9250 100644 --- a/crates/chain/tests/test_local_chain.rs +++ b/crates/chain/tests/test_local_chain.rs @@ -1,5 +1,6 @@ #![cfg(feature = "miniscript")] +use std::collections::BTreeMap; use std::ops::{Bound, RangeBounds}; use bdk_chain::{ @@ -378,6 +379,91 @@ fn local_chain_insert_block() { } } +#[test] +fn local_chain_insert_header() { + fn header(prev_blockhash: BlockHash) -> Header { + Header { + version: bitcoin::block::Version::default(), + prev_blockhash, + merkle_root: bitcoin::hash_types::TxMerkleNode::all_zeros(), + time: 0, + bits: bitcoin::CompactTarget::default(), + nonce: 0, + } + } + + fn local_chain(data: Vec<(u32, Header)>) -> LocalChain
{ + bdk_chain::local_chain::LocalChain::from_data(data.into_iter().collect::>()) + .expect("chain must have genesis block") + } + + struct TestCase { + original: LocalChain
, + insert: (u32, Header), + expected_result: Result, AlterCheckPointError>, + expected_final: LocalChain
, + } + + let test_cases = [ + TestCase { + original: local_chain(vec![(0, header(hash!("_")))]), + insert: (5, header(hash!("block5"))), + expected_result: Ok([(5, Some(header(hash!("block5"))))].into()), + expected_final: local_chain(vec![ + (0, header(hash!("_"))), + (5, header(hash!("block5"))), + ]), + }, + TestCase { + original: local_chain(vec![(0, header(hash!("_"))), (3, header(hash!("A")))]), + insert: (4, header(hash!("B"))), + expected_result: Ok([(4, Some(header(hash!("B"))))].into()), + expected_final: local_chain(vec![ + (0, header(hash!("_"))), + (3, header(hash!("A"))), + (4, header(hash!("B"))), + ]), + }, + TestCase { + original: local_chain(vec![(0, header(hash!("_"))), (4, header(hash!("B")))]), + insert: (3, header(hash!("A"))), + expected_result: Ok([(3, Some(header(hash!("A"))))].into()), + expected_final: local_chain(vec![ + (0, header(hash!("_"))), + (3, header(hash!("A"))), + (4, header(hash!("B"))), + ]), + }, + TestCase { + original: local_chain(vec![(0, header(hash!("_"))), (2, header(hash!("K")))]), + insert: (2, header(hash!("K"))), + expected_result: Ok([].into()), + expected_final: local_chain(vec![(0, header(hash!("_"))), (2, header(hash!("K")))]), + }, + TestCase { + original: local_chain(vec![(0, header(hash!("_"))), (2, header(hash!("K")))]), + insert: (2, header(hash!("J"))), + expected_result: Err(AlterCheckPointError { + height: 2, + original_hash: header(hash!("K")).block_hash(), + update_hash: Some(header(hash!("J")).block_hash()), + }), + expected_final: local_chain(vec![(0, header(hash!("_"))), (2, header(hash!("K")))]), + }, + ]; + + for (i, t) in test_cases.into_iter().enumerate() { + let mut chain = t.original; + assert_eq!( + chain.insert_data(t.insert.0, t.insert.1), + t.expected_result, + "[{}] unexpected result when inserting block", + i, + ); + assert_eq!(chain, t.expected_final, "[{}] unexpected final chain", i,); + } +} + #[test] fn local_chain_disconnect_from() { struct TestCase {