From 5998a228191caccfaeac2b38ed4f319994b944c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Thu, 12 Oct 2023 16:55:32 +0800 Subject: [PATCH 01/13] feat!: `LocalChain` with hardwired genesis checkpoint This ensures that `LocalChain` will always have a tip. The `ChainOracle` trait's `get_chain_tip` method no longer needs to return an option. --- crates/bdk/src/error.rs | 9 + crates/bdk/src/wallet/mod.rs | 135 ++++++++---- crates/bdk/tests/wallet.rs | 18 +- crates/bitcoind_rpc/src/lib.rs | 123 +++++------ crates/bitcoind_rpc/tests/test_emitter.rs | 68 ++++-- crates/chain/src/chain_oracle.rs | 2 +- crates/chain/src/local_chain.rs | 196 +++++++++++------- crates/chain/tests/common/mod.rs | 5 +- crates/chain/tests/test_indexed_tx_graph.rs | 13 +- crates/chain/tests/test_local_chain.rs | 152 +++++++------- crates/chain/tests/test_tx_graph.rs | 26 ++- crates/chain/tests/test_tx_graph_conflicts.rs | 5 +- crates/electrum/src/electrum_ext.rs | 16 +- crates/esplora/src/async_ext.rs | 66 +++--- crates/esplora/src/blocking_ext.rs | 66 +++--- .../example_bitcoind_rpc_polling/src/main.rs | 80 ++++--- example-crates/example_cli/src/lib.rs | 12 +- example-crates/example_electrum/src/main.rs | 4 +- example-crates/example_esplora/src/main.rs | 14 +- 19 files changed, 560 insertions(+), 450 deletions(-) diff --git a/crates/bdk/src/error.rs b/crates/bdk/src/error.rs index fcb5a6f7b..3bd3dc6d3 100644 --- a/crates/bdk/src/error.rs +++ b/crates/bdk/src/error.rs @@ -199,3 +199,12 @@ impl_error!(miniscript::Error, Miniscript); impl_error!(MiniscriptPsbtError, MiniscriptPsbt); impl_error!(bitcoin::bip32::Error, Bip32); impl_error!(bitcoin::psbt::Error, Psbt); + +impl From for Error { + fn from(e: crate::wallet::NewNoPersistError) -> Self { + match e { + wallet::NewNoPersistError::Descriptor(e) => Error::Descriptor(e), + unknown_network_err => Error::Generic(format!("{}", unknown_network_err)), + } + } +} diff --git a/crates/bdk/src/wallet/mod.rs b/crates/bdk/src/wallet/mod.rs index bc5093ce3..275487a58 100644 --- a/crates/bdk/src/wallet/mod.rs +++ b/crates/bdk/src/wallet/mod.rs @@ -28,14 +28,14 @@ use bdk_chain::{ Append, BlockId, ChainPosition, ConfirmationTime, ConfirmationTimeHeightAnchor, FullTxOut, IndexedTxGraph, Persist, PersistBackend, }; -use bitcoin::consensus::encode::serialize; -use bitcoin::psbt; use bitcoin::secp256k1::Secp256k1; use bitcoin::sighash::{EcdsaSighashType, TapSighashType}; use bitcoin::{ absolute, Address, Network, OutPoint, Script, ScriptBuf, Sequence, Transaction, TxOut, Txid, Weight, Witness, }; +use bitcoin::{consensus::encode::serialize, BlockHash}; +use bitcoin::{constants::genesis_block, psbt}; use core::fmt; use core::ops::Deref; use miniscript::psbt::{PsbtExt, PsbtInputExt, PsbtInputSatisfier}; @@ -225,26 +225,57 @@ impl Wallet { descriptor: E, change_descriptor: Option, network: Network, - ) -> Result { + ) -> Result { Self::new(descriptor, change_descriptor, (), network).map_err(|e| match e { - NewError::Descriptor(e) => e, - NewError::Persist(_) => unreachable!("no persistence so it can't fail"), + NewError::Descriptor(e) => NewNoPersistError::Descriptor(e), + NewError::Persist(_) | NewError::InvalidPersistenceGenesis => { + unreachable!("no persistence so it can't fail") + } + NewError::UnknownNetwork => NewNoPersistError::UnknownNetwork, }) } } +/// Error returned from [`Wallet::new_no_persist`] +#[derive(Debug)] +pub enum NewNoPersistError { + /// There was problem with the descriptors passed in + Descriptor(crate::descriptor::DescriptorError), + /// We cannot determine the genesis hash from the network. + UnknownNetwork, +} + +impl fmt::Display for NewNoPersistError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + NewNoPersistError::Descriptor(e) => e.fmt(f), + NewNoPersistError::UnknownNetwork => write!( + f, + "unknown network - genesis block hash needs to be provided explicitly" + ), + } + } +} + +#[cfg(feature = "std")] +impl std::error::Error for NewNoPersistError {} + #[derive(Debug)] /// Error returned from [`Wallet::new`] -pub enum NewError

{ +pub enum NewError { /// There was problem with the descriptors passed in Descriptor(crate::descriptor::DescriptorError), /// We were unable to load the wallet's data from the persistence backend - Persist(P), + Persist(PE), + /// We cannot determine the genesis hash from the network + UnknownNetwork, + /// The genesis block hash is either missing from persistence or has an unexpected value + InvalidPersistenceGenesis, } -impl

fmt::Display for NewError

+impl fmt::Display for NewError where - P: fmt::Display, + PE: fmt::Display, { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { @@ -252,10 +283,18 @@ where NewError::Persist(e) => { write!(f, "failed to load wallet from persistence backend: {}", e) } + NewError::UnknownNetwork => write!( + f, + "unknown network - genesis block hash needs to be provided explicitly" + ), + NewError::InvalidPersistenceGenesis => write!(f, "the genesis block hash is either missing from persistence or has an unexpected value"), } } } +#[cfg(feature = "std")] +impl std::error::Error for NewError where PE: core::fmt::Display + core::fmt::Debug {} + /// An error that may occur when inserting a transaction into [`Wallet`]. #[derive(Debug)] pub enum InsertTxError { @@ -263,29 +302,44 @@ pub enum InsertTxError { /// confirmation height that is greater than the internal chain tip. ConfirmationHeightCannotBeGreaterThanTip { /// The internal chain's tip height. - tip_height: Option, + tip_height: u32, /// The introduced transaction's confirmation height. tx_height: u32, }, } -#[cfg(feature = "std")] -impl std::error::Error for NewError

{} - impl Wallet { /// Create a wallet from a `descriptor` (and an optional `change_descriptor`) and load related /// transaction data from `db`. pub fn new( + descriptor: E, + change_descriptor: Option, + db: D, + network: Network, + ) -> Result> + where + D: PersistBackend, + { + Self::with_custom_genesis_hash(descriptor, change_descriptor, db, network, None) + } + + /// Create a new [`Wallet`] with a custom genesis hash. + /// + /// This is like [`Wallet::new`] with an additional `custom_genesis_hash` parameter. + pub fn with_custom_genesis_hash( descriptor: E, change_descriptor: Option, mut db: D, network: Network, + custom_genesis_hash: Option, ) -> Result> where D: PersistBackend, { let secp = Secp256k1::new(); - let mut chain = LocalChain::default(); + let genesis_hash = + custom_genesis_hash.unwrap_or_else(|| genesis_block(network).block_hash()); + let (mut chain, _) = LocalChain::from_genesis_hash(genesis_hash); let mut indexed_graph = IndexedTxGraph::< ConfirmationTimeHeightAnchor, KeychainTxOutIndex, @@ -319,7 +373,9 @@ impl Wallet { }; let changeset = db.load_from_persistence().map_err(NewError::Persist)?; - chain.apply_changeset(&changeset.chain); + chain + .apply_changeset(&changeset.chain) + .map_err(|_| NewError::InvalidPersistenceGenesis)?; indexed_graph.apply_changeset(changeset.indexed_tx_graph); let persist = Persist::new(db); @@ -446,7 +502,7 @@ impl Wallet { .graph() .filter_chain_unspents( &self.chain, - self.chain.tip().map(|cp| cp.block_id()).unwrap_or_default(), + self.chain.tip().block_id(), self.indexed_graph.index.outpoints().iter().cloned(), ) .map(|((k, i), full_txo)| new_local_utxo(k, i, full_txo)) @@ -458,7 +514,7 @@ impl Wallet { } /// Returns the latest checkpoint. - pub fn latest_checkpoint(&self) -> Option { + pub fn latest_checkpoint(&self) -> CheckPoint { self.chain.tip() } @@ -496,7 +552,7 @@ impl Wallet { .graph() .filter_chain_unspents( &self.chain, - self.chain.tip().map(|cp| cp.block_id()).unwrap_or_default(), + self.chain.tip().block_id(), core::iter::once((spk_i, op)), ) .map(|((k, i), full_txo)| new_local_utxo(k, i, full_txo)) @@ -669,7 +725,7 @@ impl Wallet { Some(CanonicalTx { chain_position: graph.get_chain_position( &self.chain, - self.chain.tip().map(|cp| cp.block_id()).unwrap_or_default(), + self.chain.tip().block_id(), txid, )?, tx_node: graph.get_tx_node(txid)?, @@ -686,7 +742,7 @@ impl Wallet { pub fn insert_checkpoint( &mut self, block_id: BlockId, - ) -> Result + ) -> Result where D: PersistBackend, { @@ -730,7 +786,7 @@ impl Wallet { .range(height..) .next() .ok_or(InsertTxError::ConfirmationHeightCannotBeGreaterThanTip { - tip_height: self.chain.tip().map(|b| b.height()), + tip_height: self.chain.tip().height(), tx_height: height, }) .map(|(&anchor_height, &hash)| ConfirmationTimeHeightAnchor { @@ -766,10 +822,9 @@ impl Wallet { pub fn transactions( &self, ) -> impl Iterator> + '_ { - self.indexed_graph.graph().list_chain_txs( - &self.chain, - self.chain.tip().map(|cp| cp.block_id()).unwrap_or_default(), - ) + self.indexed_graph + .graph() + .list_chain_txs(&self.chain, self.chain.tip().block_id()) } /// Return the balance, separated into available, trusted-pending, untrusted-pending and immature @@ -777,7 +832,7 @@ impl Wallet { pub fn get_balance(&self) -> Balance { self.indexed_graph.graph().balance( &self.chain, - self.chain.tip().map(|cp| cp.block_id()).unwrap_or_default(), + self.chain.tip().block_id(), self.indexed_graph.index.outpoints().iter().cloned(), |&(k, _), _| k == KeychainKind::Internal, ) @@ -945,14 +1000,14 @@ impl Wallet { _ => 1, }; - // We use a match here instead of a map_or_else as it's way more readable :) + // We use a match here instead of a unwrap_or_else as it's way more readable :) let current_height = match params.current_height { // If they didn't tell us the current height, we assume it's the latest sync height. - None => self - .chain - .tip() - .map(|cp| absolute::LockTime::from_height(cp.height()).expect("Invalid height")), - h => h, + None => { + let tip_height = self.chain.tip().height(); + absolute::LockTime::from_height(tip_height).expect("invalid height") + } + Some(h) => h, }; let lock_time = match params.locktime { @@ -961,7 +1016,7 @@ impl Wallet { // Fee sniping can be partially prevented by setting the timelock // to current_height. If we don't know the current_height, // we default to 0. - let fee_sniping_height = current_height.unwrap_or(absolute::LockTime::ZERO); + let fee_sniping_height = current_height; // We choose the biggest between the required nlocktime and the fee sniping // height @@ -1115,7 +1170,7 @@ impl Wallet { params.drain_wallet, params.manually_selected_only, params.bumping_fee.is_some(), // we mandate confirmed transactions if we're bumping the fee - current_height.map(absolute::LockTime::to_consensus_u32), + Some(current_height.to_consensus_u32()), ); // get drain script @@ -1257,7 +1312,7 @@ impl Wallet { ) -> Result, Error> { let graph = self.indexed_graph.graph(); let txout_index = &self.indexed_graph.index; - let chain_tip = self.chain.tip().map(|cp| cp.block_id()).unwrap_or_default(); + let chain_tip = self.chain.tip().block_id(); let mut tx = graph .get_tx(txid) @@ -1492,7 +1547,7 @@ impl Wallet { psbt: &mut psbt::PartiallySignedTransaction, sign_options: SignOptions, ) -> Result { - let chain_tip = self.chain.tip().map(|cp| cp.block_id()).unwrap_or_default(); + let chain_tip = self.chain.tip().block_id(); let tx = &psbt.unsigned_tx; let mut finished = true; @@ -1515,7 +1570,7 @@ impl Wallet { }); let current_height = sign_options .assume_height - .or(self.chain.tip().map(|b| b.height())); + .unwrap_or_else(|| self.chain.tip().height()); debug!( "Input #{} - {}, using `confirmation_height` = {:?}, `current_height` = {:?}", @@ -1552,8 +1607,8 @@ impl Wallet { &mut tmp_input, ( PsbtInputSatisfier::new(psbt, n), - After::new(current_height, false), - Older::new(current_height, confirmation_height, false), + After::new(Some(current_height), false), + Older::new(Some(current_height), confirmation_height, false), ), ) { Ok(_) => { @@ -1661,7 +1716,7 @@ impl Wallet { must_only_use_confirmed_tx: bool, current_height: Option, ) -> (Vec, Vec) { - let chain_tip = self.chain.tip().map(|cp| cp.block_id()).unwrap_or_default(); + let chain_tip = self.chain.tip().block_id(); // must_spend <- manually selected utxos // may_spend <- all other available utxos let mut may_spend = self.get_available_utxos(); diff --git a/crates/bdk/tests/wallet.rs b/crates/bdk/tests/wallet.rs index aad8c2db2..3aab7016d 100644 --- a/crates/bdk/tests/wallet.rs +++ b/crates/bdk/tests/wallet.rs @@ -42,14 +42,14 @@ fn receive_output(wallet: &mut Wallet, value: u64, height: ConfirmationTime) -> } fn receive_output_in_latest_block(wallet: &mut Wallet, value: u64) -> OutPoint { - let height = match wallet.latest_checkpoint() { - Some(cp) => ConfirmationTime::Confirmed { - height: cp.height(), - time: 0, - }, - None => ConfirmationTime::Unconfirmed { last_seen: 0 }, + let latest_cp = wallet.latest_checkpoint(); + let height = latest_cp.height(); + let anchor = if height == 0 { + ConfirmationTime::Unconfirmed { last_seen: 0 } + } else { + ConfirmationTime::Confirmed { height, time: 0 } }; - receive_output(wallet, value, height) + receive_output(wallet, value, anchor) } // The satisfaction size of a P2WPKH is 112 WU = @@ -277,7 +277,7 @@ fn test_create_tx_fee_sniping_locktime_last_sync() { // If there's no current_height we're left with using the last sync height assert_eq!( psbt.unsigned_tx.lock_time.to_consensus_u32(), - wallet.latest_checkpoint().unwrap().height() + wallet.latest_checkpoint().height() ); } @@ -1615,7 +1615,7 @@ fn test_bump_fee_drain_wallet() { .insert_tx( tx.clone(), ConfirmationTime::Confirmed { - height: wallet.latest_checkpoint().unwrap().height(), + height: wallet.latest_checkpoint().height(), time: 42_000, }, ) diff --git a/crates/bitcoind_rpc/src/lib.rs b/crates/bitcoind_rpc/src/lib.rs index a5016ce6f..07f6ea3d4 100644 --- a/crates/bitcoind_rpc/src/lib.rs +++ b/crates/bitcoind_rpc/src/lib.rs @@ -25,7 +25,7 @@ pub struct Emitter<'c, C> { /// The checkpoint of the last-emitted block that is in the best chain. If it is later found /// that the block is no longer in the best chain, it will be popped off from here. - last_cp: Option, + last_cp: CheckPoint, /// The block result returned from rpc of the last-emitted block. As this result contains the /// next block's block hash (which we use to fetch the next block), we set this to `None` @@ -43,29 +43,12 @@ pub struct Emitter<'c, C> { } impl<'c, C: bitcoincore_rpc::RpcApi> Emitter<'c, C> { - /// Construct a new [`Emitter`] with the given RPC `client` and `start_height`. - /// - /// `start_height` is the block height to start emitting blocks from. - pub fn from_height(client: &'c C, start_height: u32) -> Self { + /// TODO + pub fn new(client: &'c C, last_cp: CheckPoint, start_height: u32) -> Self { Self { client, start_height, - last_cp: None, - last_block: None, - last_mempool_time: 0, - last_mempool_tip: None, - } - } - - /// Construct a new [`Emitter`] with the given RPC `client` and `checkpoint`. - /// - /// `checkpoint` is used to find the latest block which is still part of the best chain. The - /// [`Emitter`] will emit blocks starting right above this block. - pub fn from_checkpoint(client: &'c C, checkpoint: CheckPoint) -> Self { - Self { - client, - start_height: 0, - last_cp: Some(checkpoint), + last_cp, last_block: None, last_mempool_time: 0, last_mempool_tip: None, @@ -134,7 +117,7 @@ impl<'c, C: bitcoincore_rpc::RpcApi> Emitter<'c, C> { .collect::, _>>()?; self.last_mempool_time = latest_time; - self.last_mempool_tip = self.last_cp.as_ref().map(|cp| cp.height()); + self.last_mempool_tip = Some(self.last_cp.height()); Ok(txs_to_emit) } @@ -156,7 +139,8 @@ enum PollResponse { /// Fetched block is not in the best chain. BlockNotInBestChain, AgreementFound(bitcoincore_rpc_json::GetBlockResult, CheckPoint), - AgreementPointNotFound, + /// Force the genesis checkpoint down the receiver's throat. + AgreementPointNotFound(BlockHash), } fn poll_once(emitter: &Emitter) -> Result @@ -166,45 +150,50 @@ where let client = emitter.client; if let Some(last_res) = &emitter.last_block { - assert!( - emitter.last_cp.is_some(), - "must not have block result without last cp" - ); - - let next_hash = match last_res.nextblockhash { - None => return Ok(PollResponse::NoMoreBlocks), - Some(next_hash) => next_hash, + let next_hash = if last_res.height < emitter.start_height as _ { + // enforce start height + let next_hash = client.get_block_hash(emitter.start_height as _)?; + // make sure last emission is still in best chain + if client.get_block_hash(last_res.height as _)? != last_res.hash { + return Ok(PollResponse::BlockNotInBestChain); + } + next_hash + } else { + match last_res.nextblockhash { + None => return Ok(PollResponse::NoMoreBlocks), + Some(next_hash) => next_hash, + } }; let res = client.get_block_info(&next_hash)?; if res.confirmations < 0 { return Ok(PollResponse::BlockNotInBestChain); } - return Ok(PollResponse::Block(res)); - } - - if emitter.last_cp.is_none() { - let hash = client.get_block_hash(emitter.start_height as _)?; - let res = client.get_block_info(&hash)?; - if res.confirmations < 0 { - return Ok(PollResponse::BlockNotInBestChain); - } return Ok(PollResponse::Block(res)); } - for cp in emitter.last_cp.iter().flat_map(CheckPoint::iter) { - let res = client.get_block_info(&cp.hash())?; - if res.confirmations < 0 { - // block is not in best chain - continue; - } + for cp in emitter.last_cp.iter() { + let res = match client.get_block_info(&cp.hash()) { + // block not in best chain + Ok(res) if res.confirmations < 0 => continue, + Ok(res) => res, + Err(e) if e.is_not_found_error() => { + if cp.height() > 0 { + continue; + } + // if we can't find genesis block, we can't create an update that connects + break; + } + Err(e) => return Err(e), + }; // agreement point found return Ok(PollResponse::AgreementFound(res, cp)); } - Ok(PollResponse::AgreementPointNotFound) + let genesis_hash = client.get_block_hash(0)?; + Ok(PollResponse::AgreementPointNotFound(genesis_hash)) } fn poll( @@ -222,25 +211,12 @@ where let hash = res.hash; let item = get_item(&hash)?; - let this_id = BlockId { height, hash }; - let prev_id = res.previousblockhash.map(|prev_hash| BlockId { - height: height - 1, - hash: prev_hash, - }); - - match (&mut emitter.last_cp, prev_id) { - (Some(cp), _) => *cp = cp.clone().push(this_id).expect("must push"), - (last_cp, None) => *last_cp = Some(CheckPoint::new(this_id)), - // When the receiver constructs a local_chain update from a block, the previous - // checkpoint is also included in the update. We need to reflect this state in - // `Emitter::last_cp` as well. - (last_cp, Some(prev_id)) => { - *last_cp = Some(CheckPoint::new(prev_id).push(this_id).expect("must push")) - } - } - + emitter.last_cp = emitter + .last_cp + .clone() + .push(BlockId { height, hash }) + .expect("must push"); emitter.last_block = Some(res); - return Ok(Some((height, item))); } PollResponse::NoMoreBlocks => { @@ -254,9 +230,6 @@ where PollResponse::AgreementFound(res, cp) => { let agreement_h = res.height as u32; - // get rid of evicted blocks - emitter.last_cp = Some(cp); - // The tip during the last mempool emission needs to in the best chain, we reduce // it if it is not. if let Some(h) = emitter.last_mempool_tip.as_mut() { @@ -264,15 +237,17 @@ where *h = agreement_h; } } + + // get rid of evicted blocks + emitter.last_cp = cp; emitter.last_block = Some(res); continue; } - PollResponse::AgreementPointNotFound => { - // We want to clear `last_cp` and set `start_height` to the first checkpoint's - // height. This way, the first checkpoint in `LocalChain` can be replaced. - if let Some(last_cp) = emitter.last_cp.take() { - emitter.start_height = last_cp.height(); - } + PollResponse::AgreementPointNotFound(genesis_hash) => { + emitter.last_cp = CheckPoint::new(BlockId { + height: 0, + hash: genesis_hash, + }); emitter.last_block = None; continue; } diff --git a/crates/bitcoind_rpc/tests/test_emitter.rs b/crates/bitcoind_rpc/tests/test_emitter.rs index f0bbd3d15..d3c9e36bd 100644 --- a/crates/bitcoind_rpc/tests/test_emitter.rs +++ b/crates/bitcoind_rpc/tests/test_emitter.rs @@ -188,8 +188,8 @@ fn block_to_chain_update(block: &bitcoin::Block, height: u32) -> local_chain::Up #[test] pub fn test_sync_local_chain() -> anyhow::Result<()> { let env = TestEnv::new()?; - let mut local_chain = LocalChain::default(); - let mut emitter = Emitter::from_height(&env.client, 0); + let (mut local_chain, _) = LocalChain::from_genesis_hash(env.client.get_block_hash(0)?); + let mut emitter = Emitter::new(&env.client, local_chain.tip(), 0); // mine some blocks and returned the actual block hashes let exp_hashes = { @@ -296,7 +296,7 @@ fn test_into_tx_graph() -> anyhow::Result<()> { env.mine_blocks(101, None)?; println!("mined blocks!"); - let mut chain = LocalChain::default(); + let (mut chain, _) = LocalChain::from_genesis_hash(env.client.get_block_hash(0)?); let mut indexed_tx_graph = IndexedTxGraph::::new({ let mut index = SpkTxOutIndex::::default(); index.insert_spk(0, addr_0.script_pubkey()); @@ -305,7 +305,7 @@ fn test_into_tx_graph() -> anyhow::Result<()> { index }); - let emitter = &mut Emitter::from_height(&env.client, 0); + let emitter = &mut Emitter::new(&env.client, chain.tip(), 0); while let Some((height, block)) = emitter.next_block()? { let _ = chain.apply_update(block_to_chain_update(&block, height))?; @@ -393,7 +393,14 @@ fn ensure_block_emitted_after_reorg_is_at_reorg_height() -> anyhow::Result<()> { const CHAIN_TIP_HEIGHT: usize = 110; let env = TestEnv::new()?; - let mut emitter = Emitter::from_height(&env.client, EMITTER_START_HEIGHT as _); + let mut emitter = Emitter::new( + &env.client, + CheckPoint::new(BlockId { + height: 0, + hash: env.client.get_block_hash(0)?, + }), + EMITTER_START_HEIGHT as _, + ); env.mine_blocks(CHAIN_TIP_HEIGHT, None)?; while emitter.next_header()?.is_some() {} @@ -442,9 +449,7 @@ fn get_balance( recv_chain: &LocalChain, recv_graph: &IndexedTxGraph>, ) -> anyhow::Result { - let chain_tip = recv_chain - .tip() - .map_or(BlockId::default(), |cp| cp.block_id()); + let chain_tip = recv_chain.tip().block_id(); let outpoints = recv_graph.index.outpoints().clone(); let balance = recv_graph .graph() @@ -461,7 +466,14 @@ fn tx_can_become_unconfirmed_after_reorg() -> anyhow::Result<()> { const SEND_AMOUNT: Amount = Amount::from_sat(10_000); let env = TestEnv::new()?; - let mut emitter = Emitter::from_height(&env.client, 0); + let mut emitter = Emitter::new( + &env.client, + CheckPoint::new(BlockId { + height: 0, + hash: env.client.get_block_hash(0)?, + }), + 0, + ); // setup addresses let addr_to_mine = env.client.get_new_address(None, None)?.assume_checked(); @@ -469,7 +481,7 @@ fn tx_can_become_unconfirmed_after_reorg() -> anyhow::Result<()> { let addr_to_track = Address::from_script(&spk_to_track, bitcoin::Network::Regtest)?; // setup receiver - let mut recv_chain = LocalChain::default(); + let (mut recv_chain, _) = LocalChain::from_genesis_hash(env.client.get_block_hash(0)?); let mut recv_graph = IndexedTxGraph::::new({ let mut recv_index = SpkTxOutIndex::default(); recv_index.insert_spk((), spk_to_track.clone()); @@ -542,7 +554,14 @@ fn mempool_avoids_re_emission() -> anyhow::Result<()> { const MEMPOOL_TX_COUNT: usize = 2; let env = TestEnv::new()?; - let mut emitter = Emitter::from_height(&env.client, 0); + let mut emitter = Emitter::new( + &env.client, + CheckPoint::new(BlockId { + height: 0, + hash: env.client.get_block_hash(0)?, + }), + 0, + ); // mine blocks and sync up emitter let addr = env.client.get_new_address(None, None)?.assume_checked(); @@ -597,7 +616,14 @@ fn mempool_re_emits_if_tx_introduction_height_not_reached() -> anyhow::Result<() const MEMPOOL_TX_COUNT: usize = 21; let env = TestEnv::new()?; - let mut emitter = Emitter::from_height(&env.client, 0); + let mut emitter = Emitter::new( + &env.client, + CheckPoint::new(BlockId { + height: 0, + hash: env.client.get_block_hash(0)?, + }), + 0, + ); // mine blocks to get initial balance, sync emitter up to tip let addr = env.client.get_new_address(None, None)?.assume_checked(); @@ -674,7 +700,14 @@ fn mempool_during_reorg() -> anyhow::Result<()> { const PREMINE_COUNT: usize = 101; let env = TestEnv::new()?; - let mut emitter = Emitter::from_height(&env.client, 0); + let mut emitter = Emitter::new( + &env.client, + CheckPoint::new(BlockId { + height: 0, + hash: env.client.get_block_hash(0)?, + }), + 0, + ); // mine blocks to get initial balance let addr = env.client.get_new_address(None, None)?.assume_checked(); @@ -789,7 +822,14 @@ fn no_agreement_point() -> anyhow::Result<()> { let env = TestEnv::new()?; // start height is 99 - let mut emitter = Emitter::from_height(&env.client, (PREMINE_COUNT - 2) as u32); + let mut emitter = Emitter::new( + &env.client, + CheckPoint::new(BlockId { + height: 0, + hash: env.client.get_block_hash(0)?, + }), + (PREMINE_COUNT - 2) as u32, + ); // mine 101 blocks env.mine_blocks(PREMINE_COUNT, None)?; diff --git a/crates/chain/src/chain_oracle.rs b/crates/chain/src/chain_oracle.rs index e736be035..038025619 100644 --- a/crates/chain/src/chain_oracle.rs +++ b/crates/chain/src/chain_oracle.rs @@ -21,5 +21,5 @@ pub trait ChainOracle { ) -> Result, Self::Error>; /// Get the best chain's chain tip. - fn get_chain_tip(&self) -> Result, Self::Error>; + fn get_chain_tip(&self) -> Result; } diff --git a/crates/chain/src/local_chain.rs b/crates/chain/src/local_chain.rs index 094b77424..1538cdfda 100644 --- a/crates/chain/src/local_chain.rs +++ b/crates/chain/src/local_chain.rs @@ -179,9 +179,9 @@ pub struct Update { } /// This is a local implementation of [`ChainOracle`]. -#[derive(Debug, Default, Clone)] +#[derive(Debug, Clone)] pub struct LocalChain { - tip: Option, + tip: CheckPoint, index: BTreeMap, } @@ -197,12 +197,6 @@ impl From for BTreeMap { } } -impl From> for LocalChain { - fn from(value: BTreeMap) -> Self { - Self::from_blocks(value) - } -} - impl ChainOracle for LocalChain { type Error = Infallible; @@ -225,39 +219,71 @@ impl ChainOracle for LocalChain { ) } - fn get_chain_tip(&self) -> Result, Self::Error> { - Ok(self.tip.as_ref().map(|tip| tip.block_id())) + fn get_chain_tip(&self) -> Result { + Ok(self.tip.block_id()) } } impl LocalChain { + /// Get the genesis hash. + pub fn genesis_hash(&self) -> BlockHash { + self.index.get(&0).copied().expect("must have genesis hash") + } + + /// 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 }), + index: core::iter::once((height, hash)).collect(), + }; + let changeset = chain.initial_changeset(); + (chain, changeset) + } + /// Construct a [`LocalChain`] from an initial `changeset`. - pub fn from_changeset(changeset: ChangeSet) -> Self { - let mut chain = Self::default(); - chain.apply_changeset(&changeset); + pub fn from_changeset(changeset: ChangeSet) -> Result { + let genesis_entry = changeset.get(&0).copied().flatten(); + let genesis_hash = match genesis_entry { + Some(hash) => hash, + None => return Err(MissingGenesisError), + }; + + let (mut chain, _) = Self::from_genesis_hash(genesis_hash); + chain.apply_changeset(&changeset)?; debug_assert!(chain._check_index_is_consistent_with_tip()); debug_assert!(chain._check_changeset_is_applied(&changeset)); - chain + Ok(chain) } /// Construct a [`LocalChain`] from a given `checkpoint` tip. - pub fn from_tip(tip: CheckPoint) -> Self { + pub fn from_tip(tip: CheckPoint) -> Result { let mut chain = Self { - tip: Some(tip), - ..Default::default() + tip, + index: BTreeMap::new(), }; chain.reindex(0); + + if chain.index.get(&0).copied().is_none() { + return Err(MissingGenesisError); + } + debug_assert!(chain._check_index_is_consistent_with_tip()); - chain + Ok(chain) } /// 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) -> Self { + pub fn from_blocks(blocks: BTreeMap) -> Result { + if !blocks.contains_key(&0) { + return Err(MissingGenesisError); + } + let mut tip: Option = None; for block in &blocks { @@ -272,25 +298,20 @@ impl LocalChain { } } - let chain = Self { index: blocks, tip }; + let chain = Self { + index: blocks, + tip: tip.expect("already checked to have genesis"), + }; debug_assert!(chain._check_index_is_consistent_with_tip()); - - chain + Ok(chain) } /// Get the highest checkpoint. - pub fn tip(&self) -> Option { + pub fn tip(&self) -> CheckPoint { self.tip.clone() } - /// Returns whether the [`LocalChain`] is empty (has no checkpoints). - pub fn is_empty(&self) -> bool { - let res = self.tip.is_none(); - debug_assert_eq!(res, self.index.is_empty()); - res - } - /// Applies the given `update` to the chain. /// /// The method returns [`ChangeSet`] on success. This represents the applied changes to `self`. @@ -312,34 +333,28 @@ impl LocalChain { /// /// [module-level documentation]: crate::local_chain pub fn apply_update(&mut self, update: Update) -> Result { - match self.tip() { - Some(original_tip) => { - let changeset = merge_chains( - original_tip, - update.tip.clone(), - update.introduce_older_blocks, - )?; - self.apply_changeset(&changeset); - - // return early as `apply_changeset` already calls `check_consistency` - Ok(changeset) - } - None => { - *self = Self::from_tip(update.tip); - let changeset = self.initial_changeset(); - - debug_assert!(self._check_index_is_consistent_with_tip()); - debug_assert!(self._check_changeset_is_applied(&changeset)); - Ok(changeset) - } - } + let changeset = merge_chains( + self.tip.clone(), + update.tip.clone(), + update.introduce_older_blocks, + )?; + // `._check_index_is_consistent_with_tip` and `._check_changeset_is_applied` is called in + // `.apply_changeset` + self.apply_changeset(&changeset) + .map_err(|_| CannotConnectError { + try_include_height: 0, + })?; + Ok(changeset) } /// Apply the given `changeset`. - pub fn apply_changeset(&mut self, changeset: &ChangeSet) { + pub fn apply_changeset(&mut self, changeset: &ChangeSet) -> Result<(), MissingGenesisError> { if let Some(start_height) = changeset.keys().next().cloned() { + // changes after point of agreement let mut extension = BTreeMap::default(); + // point of agreement let mut base: Option = None; + for cp in self.iter_checkpoints() { if cp.height() >= start_height { extension.insert(cp.height(), cp.hash()); @@ -359,12 +374,12 @@ impl LocalChain { } }; } + let new_tip = match base { - Some(base) => Some( - base.extend(extension.into_iter().map(BlockId::from)) - .expect("extension is strictly greater than base"), - ), - None => LocalChain::from_blocks(extension).tip(), + Some(base) => base + .extend(extension.into_iter().map(BlockId::from)) + .expect("extension is strictly greater than base"), + None => LocalChain::from_blocks(extension)?.tip(), }; self.tip = new_tip; self.reindex(start_height); @@ -372,6 +387,8 @@ impl LocalChain { debug_assert!(self._check_index_is_consistent_with_tip()); debug_assert!(self._check_changeset_is_applied(changeset)); } + + Ok(()) } /// Insert a [`BlockId`]. @@ -379,13 +396,13 @@ impl LocalChain { /// # Errors /// /// Replacing the block hash of an existing checkpoint will result in an error. - pub fn insert_block(&mut self, block_id: BlockId) -> Result { + pub fn insert_block(&mut self, block_id: BlockId) -> Result { if let Some(&original_hash) = self.index.get(&block_id.height) { if original_hash != block_id.hash { - return Err(InsertBlockError { + return Err(AlterCheckPointError { height: block_id.height, original_hash, - update_hash: block_id.hash, + update_hash: Some(block_id.hash), }); } else { return Ok(ChangeSet::default()); @@ -394,7 +411,12 @@ impl LocalChain { let mut changeset = ChangeSet::default(); changeset.insert(block_id.height, Some(block_id.hash)); - self.apply_changeset(&changeset); + self.apply_changeset(&changeset) + .map_err(|_| AlterCheckPointError { + height: 0, + original_hash: self.genesis_hash(), + update_hash: changeset.get(&0).cloned().flatten(), + })?; Ok(changeset) } @@ -418,7 +440,7 @@ impl LocalChain { /// Iterate over checkpoints in descending height order. pub fn iter_checkpoints(&self) -> CheckPointIter { CheckPointIter { - current: self.tip.as_ref().map(|tip| tip.0.clone()), + current: Some(self.tip.0.clone()), } } @@ -431,7 +453,6 @@ impl LocalChain { let tip_history = self .tip .iter() - .flat_map(CheckPoint::iter) .map(|cp| (cp.height(), cp.hash())) .collect::>(); self.index == tip_history @@ -447,29 +468,52 @@ impl LocalChain { } } -/// Represents a failure when trying to insert a checkpoint into [`LocalChain`]. +/// An error which occurs when a [`LocalChain`] is constructed without a genesis checkpoint. #[derive(Clone, Debug, PartialEq)] -pub struct InsertBlockError { - /// The checkpoints' height. - pub height: u32, - /// Original checkpoint's block hash. - pub original_hash: BlockHash, - /// Update checkpoint's block hash. - pub update_hash: BlockHash, -} +pub struct MissingGenesisError; -impl core::fmt::Display for InsertBlockError { +impl core::fmt::Display for MissingGenesisError { fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { write!( f, - "failed to insert block at height {} as block hashes conflict: original={}, update={}", - self.height, self.original_hash, self.update_hash + "cannot construct `LocalChain` without a genesis checkpoint" ) } } #[cfg(feature = "std")] -impl std::error::Error for InsertBlockError {} +impl std::error::Error for MissingGenesisError {} + +/// Represents a failure when trying to insert/remove a checkpoint to/from [`LocalChain`]. +#[derive(Clone, Debug, PartialEq)] +pub struct AlterCheckPointError { + /// The checkpoint's height. + pub height: u32, + /// The original checkpoint's block hash which cannot be replaced/removed. + pub original_hash: BlockHash, + /// The attempted update to the `original_block` hash. + pub update_hash: Option, +} + +impl core::fmt::Display for AlterCheckPointError { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + match self.update_hash { + Some(update_hash) => write!( + f, + "failed to insert block at height {}: original={} update={}", + self.height, self.original_hash, update_hash + ), + None => write!( + f, + "failed to remove block at height {}: original={}", + self.height, self.original_hash + ), + } + } +} + +#[cfg(feature = "std")] +impl std::error::Error for AlterCheckPointError {} /// Occurs when an update does not have a common checkpoint with the original chain. #[derive(Clone, Debug, PartialEq)] diff --git a/crates/chain/tests/common/mod.rs b/crates/chain/tests/common/mod.rs index d3db81910..cfb8b7c44 100644 --- a/crates/chain/tests/common/mod.rs +++ b/crates/chain/tests/common/mod.rs @@ -23,6 +23,7 @@ macro_rules! local_chain { [ $(($height:expr, $block_hash:expr)), * ] => {{ #[allow(unused_mut)] bdk_chain::local_chain::LocalChain::from_blocks([$(($height, $block_hash).into()),*].into_iter().collect()) + .expect("chain must have genesis block") }}; } @@ -32,8 +33,8 @@ macro_rules! chain_update { #[allow(unused_mut)] bdk_chain::local_chain::Update { tip: bdk_chain::local_chain::LocalChain::from_blocks([$(($height, $hash).into()),*].into_iter().collect()) - .tip() - .expect("must have tip"), + .expect("chain must have genesis block") + .tip(), introduce_older_blocks: true, } }}; diff --git a/crates/chain/tests/test_indexed_tx_graph.rs b/crates/chain/tests/test_indexed_tx_graph.rs index 3dc22ef5b..ec250c95c 100644 --- a/crates/chain/tests/test_indexed_tx_graph.rs +++ b/crates/chain/tests/test_indexed_tx_graph.rs @@ -1,7 +1,7 @@ #[macro_use] mod common; -use std::collections::{BTreeMap, BTreeSet}; +use std::collections::BTreeSet; use bdk_chain::{ indexed_tx_graph::{self, IndexedTxGraph}, @@ -9,9 +9,7 @@ use bdk_chain::{ local_chain::LocalChain, tx_graph, BlockId, ChainPosition, ConfirmationHeightAnchor, }; -use bitcoin::{ - secp256k1::Secp256k1, BlockHash, OutPoint, Script, ScriptBuf, Transaction, TxIn, TxOut, -}; +use bitcoin::{secp256k1::Secp256k1, OutPoint, Script, ScriptBuf, Transaction, TxIn, TxOut}; use miniscript::Descriptor; /// Ensure [`IndexedTxGraph::insert_relevant_txs`] can successfully index transactions NOT presented @@ -112,11 +110,8 @@ fn insert_relevant_txs() { fn test_list_owned_txouts() { // Create Local chains - let local_chain = LocalChain::from( - (0..150) - .map(|i| (i as u32, h!("random"))) - .collect::>(), - ); + let local_chain = LocalChain::from_blocks((0..150).map(|i| (i as u32, h!("random"))).collect()) + .expect("must have genesis hash"); // Initiate IndexedTxGraph diff --git a/crates/chain/tests/test_local_chain.rs b/crates/chain/tests/test_local_chain.rs index 9ea8b7f3a..d09325bd9 100644 --- a/crates/chain/tests/test_local_chain.rs +++ b/crates/chain/tests/test_local_chain.rs @@ -1,4 +1,6 @@ -use bdk_chain::local_chain::{CannotConnectError, ChangeSet, InsertBlockError, LocalChain, Update}; +use bdk_chain::local_chain::{ + AlterCheckPointError, CannotConnectError, ChangeSet, LocalChain, Update, +}; use bitcoin::BlockHash; #[macro_use] @@ -68,10 +70,10 @@ fn update_local_chain() { [ TestLocalChain { name: "add first tip", - chain: local_chain![], + chain: local_chain![(0, h!("A"))], update: chain_update![(0, h!("A"))], exp: ExpectedResult::Ok { - changeset: &[(0, Some(h!("A")))], + changeset: &[], init_changeset: &[(0, Some(h!("A")))], }, }, @@ -86,18 +88,18 @@ fn update_local_chain() { }, TestLocalChain { name: "two disjoint chains cannot merge", - chain: local_chain![(0, h!("A"))], - update: chain_update![(1, h!("B"))], + chain: local_chain![(0, h!("_")), (1, h!("A"))], + update: chain_update![(0, h!("_")), (2, h!("B"))], exp: ExpectedResult::Err(CannotConnectError { - try_include_height: 0, + try_include_height: 1, }), }, TestLocalChain { name: "two disjoint chains cannot merge (existing chain longer)", - chain: local_chain![(1, h!("A"))], - update: chain_update![(0, h!("B"))], + chain: local_chain![(0, h!("_")), (2, h!("A"))], + update: chain_update![(0, h!("_")), (1, h!("B"))], exp: ExpectedResult::Err(CannotConnectError { - try_include_height: 1, + try_include_height: 2, }), }, TestLocalChain { @@ -111,54 +113,54 @@ fn update_local_chain() { }, // Introduce an older checkpoint (B) // | 0 | 1 | 2 | 3 - // chain | C D - // update | B C + // chain | _ C D + // update | _ B C TestLocalChain { name: "can introduce older checkpoint", - chain: local_chain![(2, h!("C")), (3, h!("D"))], - update: chain_update![(1, h!("B")), (2, h!("C"))], + chain: local_chain![(0, h!("_")), (2, h!("C")), (3, h!("D"))], + update: chain_update![(0, h!("_")), (1, h!("B")), (2, h!("C"))], exp: ExpectedResult::Ok { changeset: &[(1, Some(h!("B")))], - init_changeset: &[(1, Some(h!("B"))), (2, Some(h!("C"))), (3, Some(h!("D")))], + init_changeset: &[(0, Some(h!("_"))), (1, Some(h!("B"))), (2, Some(h!("C"))), (3, Some(h!("D")))], }, }, // Introduce an older checkpoint (A) that is not directly behind PoA - // | 2 | 3 | 4 - // chain | B C - // update | A C + // | 0 | 2 | 3 | 4 + // chain | _ B C + // update | _ A C TestLocalChain { name: "can introduce older checkpoint 2", - chain: local_chain![(3, h!("B")), (4, h!("C"))], - update: chain_update![(2, h!("A")), (4, h!("C"))], + chain: local_chain![(0, h!("_")), (3, h!("B")), (4, h!("C"))], + update: chain_update![(0, h!("_")), (2, h!("A")), (4, h!("C"))], exp: ExpectedResult::Ok { changeset: &[(2, Some(h!("A")))], - init_changeset: &[(2, Some(h!("A"))), (3, Some(h!("B"))), (4, Some(h!("C")))], + init_changeset: &[(0, Some(h!("_"))), (2, Some(h!("A"))), (3, Some(h!("B"))), (4, Some(h!("C")))], } }, // Introduce an older checkpoint (B) that is not the oldest checkpoint - // | 1 | 2 | 3 - // chain | A C - // update | B C + // | 0 | 1 | 2 | 3 + // chain | _ A C + // update | _ B C TestLocalChain { name: "can introduce older checkpoint 3", - chain: local_chain![(1, h!("A")), (3, h!("C"))], - update: chain_update![(2, h!("B")), (3, h!("C"))], + chain: local_chain![(0, h!("_")), (1, h!("A")), (3, h!("C"))], + update: chain_update![(0, h!("_")), (2, h!("B")), (3, h!("C"))], exp: ExpectedResult::Ok { changeset: &[(2, Some(h!("B")))], - init_changeset: &[(1, Some(h!("A"))), (2, Some(h!("B"))), (3, Some(h!("C")))], + init_changeset: &[(0, Some(h!("_"))), (1, Some(h!("A"))), (2, Some(h!("B"))), (3, Some(h!("C")))], } }, // Introduce two older checkpoints below the PoA - // | 1 | 2 | 3 - // chain | C - // update | A B C + // | 0 | 1 | 2 | 3 + // chain | _ C + // update | _ A B C TestLocalChain { name: "introduce two older checkpoints below PoA", - chain: local_chain![(3, h!("C"))], - update: chain_update![(1, h!("A")), (2, h!("B")), (3, h!("C"))], + chain: local_chain![(0, h!("_")), (3, h!("C"))], + update: chain_update![(0, h!("_")), (1, h!("A")), (2, h!("B")), (3, h!("C"))], exp: ExpectedResult::Ok { changeset: &[(1, Some(h!("A"))), (2, Some(h!("B")))], - init_changeset: &[(1, Some(h!("A"))), (2, Some(h!("B"))), (3, Some(h!("C")))], + init_changeset: &[(0, Some(h!("_"))), (1, Some(h!("A"))), (2, Some(h!("B"))), (3, Some(h!("C")))], }, }, TestLocalChain { @@ -172,45 +174,46 @@ fn update_local_chain() { }, // B and C are in both chain and update // | 0 | 1 | 2 | 3 | 4 - // chain | B C - // update | A B C D + // chain | _ B C + // update | _ A B C D // This should succeed with the point of agreement being C and A should be added in addition. TestLocalChain { name: "two points of agreement", - chain: local_chain![(1, h!("B")), (2, h!("C"))], - update: chain_update![(0, h!("A")), (1, h!("B")), (2, h!("C")), (3, h!("D"))], + chain: local_chain![(0, h!("_")), (2, h!("B")), (3, h!("C"))], + update: chain_update![(0, h!("_")), (1, h!("A")), (2, h!("B")), (3, h!("C")), (4, h!("D"))], exp: ExpectedResult::Ok { - changeset: &[(0, Some(h!("A"))), (3, Some(h!("D")))], + changeset: &[(1, Some(h!("A"))), (4, Some(h!("D")))], init_changeset: &[ - (0, Some(h!("A"))), - (1, Some(h!("B"))), - (2, Some(h!("C"))), - (3, Some(h!("D"))), + (0, Some(h!("_"))), + (1, Some(h!("A"))), + (2, Some(h!("B"))), + (3, Some(h!("C"))), + (4, Some(h!("D"))), ], }, }, // Update and chain does not connect: // | 0 | 1 | 2 | 3 | 4 - // chain | B C - // update | A B D + // chain | _ B C + // update | _ A B D // This should fail as we cannot figure out whether C & D are on the same chain TestLocalChain { name: "update and chain does not connect", - chain: local_chain![(1, h!("B")), (2, h!("C"))], - update: chain_update![(0, h!("A")), (1, h!("B")), (3, h!("D"))], + chain: local_chain![(0, h!("_")), (2, h!("B")), (3, h!("C"))], + update: chain_update![(0, h!("_")), (1, h!("A")), (2, h!("B")), (4, h!("D"))], exp: ExpectedResult::Err(CannotConnectError { - try_include_height: 2, + try_include_height: 3, }), }, // Transient invalidation: // | 0 | 1 | 2 | 3 | 4 | 5 - // chain | A B C E - // update | A B' C' D + // chain | _ B C E + // update | _ B' C' D // This should succeed and invalidate B,C and E with point of agreement being A. TestLocalChain { name: "transitive invalidation applies to checkpoints higher than invalidation", - chain: local_chain![(0, h!("A")), (2, h!("B")), (3, h!("C")), (5, h!("E"))], - update: chain_update![(0, h!("A")), (2, h!("B'")), (3, h!("C'")), (4, h!("D"))], + chain: local_chain![(0, h!("_")), (2, h!("B")), (3, h!("C")), (5, h!("E"))], + update: chain_update![(0, h!("_")), (2, h!("B'")), (3, h!("C'")), (4, h!("D"))], exp: ExpectedResult::Ok { changeset: &[ (2, Some(h!("B'"))), @@ -219,7 +222,7 @@ fn update_local_chain() { (5, None), ], init_changeset: &[ - (0, Some(h!("A"))), + (0, Some(h!("_"))), (2, Some(h!("B'"))), (3, Some(h!("C'"))), (4, Some(h!("D"))), @@ -228,13 +231,13 @@ fn update_local_chain() { }, // Transient invalidation: // | 0 | 1 | 2 | 3 | 4 - // chain | B C E - // update | B' C' D + // chain | _ B C E + // update | _ B' C' D // This should succeed and invalidate B, C and E with no point of agreement TestLocalChain { name: "transitive invalidation applies to checkpoints higher than invalidation no point of agreement", - chain: local_chain![(1, h!("B")), (2, h!("C")), (4, h!("E"))], - update: chain_update![(1, h!("B'")), (2, h!("C'")), (3, h!("D"))], + chain: local_chain![(0, h!("_")), (1, h!("B")), (2, h!("C")), (4, h!("E"))], + update: chain_update![(0, h!("_")), (1, h!("B'")), (2, h!("C'")), (3, h!("D"))], exp: ExpectedResult::Ok { changeset: &[ (1, Some(h!("B'"))), @@ -243,6 +246,7 @@ fn update_local_chain() { (4, None) ], init_changeset: &[ + (0, Some(h!("_"))), (1, Some(h!("B'"))), (2, Some(h!("C'"))), (3, Some(h!("D"))), @@ -250,16 +254,16 @@ fn update_local_chain() { }, }, // Transient invalidation: - // | 0 | 1 | 2 | 3 | 4 - // chain | A B C E - // update | B' C' D + // | 0 | 1 | 2 | 3 | 4 | 5 + // chain | _ A B C E + // update | _ B' C' D // This should fail since although it tells us that B and C are invalid it doesn't tell us whether // A was invalid. TestLocalChain { name: "invalidation but no connection", - chain: local_chain![(0, h!("A")), (1, h!("B")), (2, h!("C")), (4, h!("E"))], - update: chain_update![(1, h!("B'")), (2, h!("C'")), (3, h!("D"))], - exp: ExpectedResult::Err(CannotConnectError { try_include_height: 0 }), + chain: local_chain![(0, h!("_")), (1, h!("A")), (2, h!("B")), (3, h!("C")), (5, h!("E"))], + update: chain_update![(0, h!("_")), (2, h!("B'")), (3, h!("C'")), (4, h!("D"))], + exp: ExpectedResult::Err(CannotConnectError { try_include_height: 1 }), }, // Introduce blocks between two points of agreement // | 0 | 1 | 2 | 3 | 4 | 5 @@ -294,44 +298,44 @@ fn local_chain_insert_block() { struct TestCase { original: LocalChain, insert: (u32, BlockHash), - expected_result: Result, + expected_result: Result, expected_final: LocalChain, } let test_cases = [ TestCase { - original: local_chain![], + original: local_chain![(0, h!("_"))], insert: (5, h!("block5")), expected_result: Ok([(5, Some(h!("block5")))].into()), - expected_final: local_chain![(5, h!("block5"))], + expected_final: local_chain![(0, h!("_")), (5, h!("block5"))], }, TestCase { - original: local_chain![(3, h!("A"))], + original: local_chain![(0, h!("_")), (3, h!("A"))], insert: (4, h!("B")), expected_result: Ok([(4, Some(h!("B")))].into()), - expected_final: local_chain![(3, h!("A")), (4, h!("B"))], + expected_final: local_chain![(0, h!("_")), (3, h!("A")), (4, h!("B"))], }, TestCase { - original: local_chain![(4, h!("B"))], + original: local_chain![(0, h!("_")), (4, h!("B"))], insert: (3, h!("A")), expected_result: Ok([(3, Some(h!("A")))].into()), - expected_final: local_chain![(3, h!("A")), (4, h!("B"))], + expected_final: local_chain![(0, h!("_")), (3, h!("A")), (4, h!("B"))], }, TestCase { - original: local_chain![(2, h!("K"))], + original: local_chain![(0, h!("_")), (2, h!("K"))], insert: (2, h!("K")), expected_result: Ok([].into()), - expected_final: local_chain![(2, h!("K"))], + expected_final: local_chain![(0, h!("_")), (2, h!("K"))], }, TestCase { - original: local_chain![(2, h!("K"))], + original: local_chain![(0, h!("_")), (2, h!("K"))], insert: (2, h!("J")), - expected_result: Err(InsertBlockError { + expected_result: Err(AlterCheckPointError { height: 2, original_hash: h!("K"), - update_hash: h!("J"), + update_hash: Some(h!("J")), }), - expected_final: local_chain![(2, h!("K"))], + expected_final: local_chain![(0, h!("_")), (2, h!("K"))], }, ]; diff --git a/crates/chain/tests/test_tx_graph.rs b/crates/chain/tests/test_tx_graph.rs index a0efd1004..b5311ea0b 100644 --- a/crates/chain/tests/test_tx_graph.rs +++ b/crates/chain/tests/test_tx_graph.rs @@ -511,11 +511,13 @@ fn test_calculate_fee_on_coinbase() { // where b0 and b1 spend a0, c0 and c1 spend b0, d0 spends c1, etc. #[test] fn test_walk_ancestors() { - let local_chain: LocalChain = (0..=20) - .map(|ht| (ht, BlockHash::hash(format!("Block Hash {}", ht).as_bytes()))) - .collect::>() - .into(); - let tip = local_chain.tip().expect("must have tip"); + let local_chain = LocalChain::from_blocks( + (0..=20) + .map(|ht| (ht, BlockHash::hash(format!("Block Hash {}", ht).as_bytes()))) + .collect(), + ) + .expect("must contain genesis hash"); + let tip = local_chain.tip(); let tx_a0 = Transaction { input: vec![TxIn { @@ -839,11 +841,13 @@ fn test_descendants_no_repeat() { #[test] fn test_chain_spends() { - let local_chain: LocalChain = (0..=100) - .map(|ht| (ht, BlockHash::hash(format!("Block Hash {}", ht).as_bytes()))) - .collect::>() - .into(); - let tip = local_chain.tip().expect("must have tip"); + let local_chain = LocalChain::from_blocks( + (0..=100) + .map(|ht| (ht, BlockHash::hash(format!("Block Hash {}", ht).as_bytes()))) + .collect(), + ) + .expect("must have genesis hash"); + let tip = local_chain.tip(); // The parent tx contains 2 outputs. Which are spent by one confirmed and one unconfirmed tx. // The parent tx is confirmed at block 95. @@ -1078,7 +1082,7 @@ fn test_missing_blocks() { g }, chain: { - let mut c = LocalChain::default(); + let (mut c, _) = LocalChain::from_genesis_hash(h!("genesis")); for (height, hash) in chain { let _ = c.insert_block(BlockId { height: *height, diff --git a/crates/chain/tests/test_tx_graph_conflicts.rs b/crates/chain/tests/test_tx_graph_conflicts.rs index 9660e3cb1..f2a161b43 100644 --- a/crates/chain/tests/test_tx_graph_conflicts.rs +++ b/crates/chain/tests/test_tx_graph_conflicts.rs @@ -39,10 +39,7 @@ fn test_tx_conflict_handling() { (5, h!("F")), (6, h!("G")) ); - let chain_tip = local_chain - .tip() - .map(|cp| cp.block_id()) - .unwrap_or_default(); + let chain_tip = local_chain.tip().block_id(); let scenarios = [ Scenario { diff --git a/crates/electrum/src/electrum_ext.rs b/crates/electrum/src/electrum_ext.rs index e0d2a7565..e54007cd9 100644 --- a/crates/electrum/src/electrum_ext.rs +++ b/crates/electrum/src/electrum_ext.rs @@ -148,7 +148,7 @@ pub trait ElectrumExt { /// single batch request. fn scan( &self, - prev_tip: Option, + prev_tip: CheckPoint, keychain_spks: BTreeMap>, txids: impl IntoIterator, outpoints: impl IntoIterator, @@ -161,7 +161,7 @@ pub trait ElectrumExt { /// [`scan`]: ElectrumExt::scan fn scan_without_keychain( &self, - prev_tip: Option, + prev_tip: CheckPoint, misc_spks: impl IntoIterator, txids: impl IntoIterator, outpoints: impl IntoIterator, @@ -188,7 +188,7 @@ pub trait ElectrumExt { impl ElectrumExt for Client { fn scan( &self, - prev_tip: Option, + prev_tip: CheckPoint, keychain_spks: BTreeMap>, txids: impl IntoIterator, outpoints: impl IntoIterator, @@ -289,17 +289,15 @@ impl ElectrumExt for Client { /// Return a [`CheckPoint`] of the latest tip, that connects with `prev_tip`. fn construct_update_tip( client: &Client, - prev_tip: Option, + prev_tip: CheckPoint, ) -> Result<(CheckPoint, Option), Error> { let HeaderNotification { height, .. } = client.block_headers_subscribe()?; let new_tip_height = height as u32; // If electrum returns a tip height that is lower than our previous tip, then checkpoints do // not need updating. We just return the previous tip and use that as the point of agreement. - if let Some(prev_tip) = prev_tip.as_ref() { - if new_tip_height < prev_tip.height() { - return Ok((prev_tip.clone(), Some(prev_tip.height()))); - } + if new_tip_height < prev_tip.height() { + return Ok((prev_tip.clone(), Some(prev_tip.height()))); } // Atomically fetch the latest `CHAIN_SUFFIX_LENGTH` count of blocks from Electrum. We use this @@ -317,7 +315,7 @@ fn construct_update_tip( // Find the "point of agreement" (if any). let agreement_cp = { let mut agreement_cp = Option::::None; - for cp in prev_tip.iter().flat_map(CheckPoint::iter) { + for cp in prev_tip.iter() { let cp_block = cp.block_id(); let hash = match new_blocks.get(&cp_block.height) { Some(&hash) => hash, diff --git a/crates/esplora/src/async_ext.rs b/crates/esplora/src/async_ext.rs index 10df02d44..653a80f03 100644 --- a/crates/esplora/src/async_ext.rs +++ b/crates/esplora/src/async_ext.rs @@ -32,7 +32,7 @@ pub trait EsploraAsyncExt { #[allow(clippy::result_large_err)] async fn update_local_chain( &self, - local_tip: Option, + local_tip: CheckPoint, request_heights: impl IntoIterator + Send> + Send, ) -> Result; @@ -95,7 +95,7 @@ pub trait EsploraAsyncExt { impl EsploraAsyncExt for esplora_client::AsyncClient { async fn update_local_chain( &self, - local_tip: Option, + local_tip: CheckPoint, request_heights: impl IntoIterator + Send> + Send, ) -> Result { let request_heights = request_heights.into_iter().collect::>(); @@ -129,41 +129,39 @@ impl EsploraAsyncExt for esplora_client::AsyncClient { let earliest_agreement_cp = { let mut earliest_agreement_cp = Option::::None; - if let Some(local_tip) = local_tip { - let local_tip_height = local_tip.height(); - for local_cp in local_tip.iter() { - let local_block = local_cp.block_id(); + let local_tip_height = local_tip.height(); + for local_cp in local_tip.iter() { + let local_block = local_cp.block_id(); - // the updated hash (block hash at this height after the update), can either be: - // 1. a block that already existed in `fetched_blocks` - // 2. a block that exists locally and at least has a depth of ASSUME_FINAL_DEPTH - // 3. otherwise we can freshly fetch the block from remote, which is safe as it - // is guaranteed that this would be at or below ASSUME_FINAL_DEPTH from the - // remote tip - let updated_hash = match fetched_blocks.entry(local_block.height) { - btree_map::Entry::Occupied(entry) => *entry.get(), - btree_map::Entry::Vacant(entry) => *entry.insert( - if local_tip_height - local_block.height >= ASSUME_FINAL_DEPTH { - local_block.hash - } else { - self.get_block_hash(local_block.height).await? - }, - ), - }; + // the updated hash (block hash at this height after the update), can either be: + // 1. a block that already existed in `fetched_blocks` + // 2. a block that exists locally and at least has a depth of ASSUME_FINAL_DEPTH + // 3. otherwise we can freshly fetch the block from remote, which is safe as it + // is guaranteed that this would be at or below ASSUME_FINAL_DEPTH from the + // remote tip + let updated_hash = match fetched_blocks.entry(local_block.height) { + btree_map::Entry::Occupied(entry) => *entry.get(), + btree_map::Entry::Vacant(entry) => *entry.insert( + if local_tip_height - local_block.height >= ASSUME_FINAL_DEPTH { + local_block.hash + } else { + self.get_block_hash(local_block.height).await? + }, + ), + }; - // since we may introduce blocks below the point of agreement, we cannot break - // here unconditionally - we only break if we guarantee there are no new heights - // below our current local checkpoint - if local_block.hash == updated_hash { - earliest_agreement_cp = Some(local_cp); + // since we may introduce blocks below the point of agreement, we cannot break + // here unconditionally - we only break if we guarantee there are no new heights + // below our current local checkpoint + if local_block.hash == updated_hash { + earliest_agreement_cp = Some(local_cp); - let first_new_height = *fetched_blocks - .keys() - .next() - .expect("must have at least one new block"); - if first_new_height >= local_block.height { - break; - } + let first_new_height = *fetched_blocks + .keys() + .next() + .expect("must have at least one new block"); + if first_new_height >= local_block.height { + break; } } } diff --git a/crates/esplora/src/blocking_ext.rs b/crates/esplora/src/blocking_ext.rs index 84c7fd4b6..9c259d583 100644 --- a/crates/esplora/src/blocking_ext.rs +++ b/crates/esplora/src/blocking_ext.rs @@ -30,7 +30,7 @@ pub trait EsploraExt { #[allow(clippy::result_large_err)] fn update_local_chain( &self, - local_tip: Option, + local_tip: CheckPoint, request_heights: impl IntoIterator, ) -> Result; @@ -87,7 +87,7 @@ pub trait EsploraExt { impl EsploraExt for esplora_client::BlockingClient { fn update_local_chain( &self, - local_tip: Option, + local_tip: CheckPoint, request_heights: impl IntoIterator, ) -> Result { let request_heights = request_heights.into_iter().collect::>(); @@ -120,41 +120,39 @@ impl EsploraExt for esplora_client::BlockingClient { let earliest_agreement_cp = { let mut earliest_agreement_cp = Option::::None; - if let Some(local_tip) = local_tip { - let local_tip_height = local_tip.height(); - for local_cp in local_tip.iter() { - let local_block = local_cp.block_id(); + let local_tip_height = local_tip.height(); + for local_cp in local_tip.iter() { + let local_block = local_cp.block_id(); - // the updated hash (block hash at this height after the update), can either be: - // 1. a block that already existed in `fetched_blocks` - // 2. a block that exists locally and at least has a depth of ASSUME_FINAL_DEPTH - // 3. otherwise we can freshly fetch the block from remote, which is safe as it - // is guaranteed that this would be at or below ASSUME_FINAL_DEPTH from the - // remote tip - let updated_hash = match fetched_blocks.entry(local_block.height) { - btree_map::Entry::Occupied(entry) => *entry.get(), - btree_map::Entry::Vacant(entry) => *entry.insert( - if local_tip_height - local_block.height >= ASSUME_FINAL_DEPTH { - local_block.hash - } else { - self.get_block_hash(local_block.height)? - }, - ), - }; + // the updated hash (block hash at this height after the update), can either be: + // 1. a block that already existed in `fetched_blocks` + // 2. a block that exists locally and at least has a depth of ASSUME_FINAL_DEPTH + // 3. otherwise we can freshly fetch the block from remote, which is safe as it + // is guaranteed that this would be at or below ASSUME_FINAL_DEPTH from the + // remote tip + let updated_hash = match fetched_blocks.entry(local_block.height) { + btree_map::Entry::Occupied(entry) => *entry.get(), + btree_map::Entry::Vacant(entry) => *entry.insert( + if local_tip_height - local_block.height >= ASSUME_FINAL_DEPTH { + local_block.hash + } else { + self.get_block_hash(local_block.height)? + }, + ), + }; - // since we may introduce blocks below the point of agreement, we cannot break - // here unconditionally - we only break if we guarantee there are no new heights - // below our current local checkpoint - if local_block.hash == updated_hash { - earliest_agreement_cp = Some(local_cp); + // since we may introduce blocks below the point of agreement, we cannot break + // here unconditionally - we only break if we guarantee there are no new heights + // below our current local checkpoint + if local_block.hash == updated_hash { + earliest_agreement_cp = Some(local_cp); - let first_new_height = *fetched_blocks - .keys() - .next() - .expect("must have at least one new block"); - if first_new_height >= local_block.height { - break; - } + let first_new_height = *fetched_blocks + .keys() + .next() + .expect("must have at least one new block"); + if first_new_height >= local_block.height { + break; } } } diff --git a/example-crates/example_bitcoind_rpc_polling/src/main.rs b/example-crates/example_bitcoind_rpc_polling/src/main.rs index f0b9996d3..6b30f01f6 100644 --- a/example-crates/example_bitcoind_rpc_polling/src/main.rs +++ b/example-crates/example_bitcoind_rpc_polling/src/main.rs @@ -131,7 +131,7 @@ fn main() -> anyhow::Result<()> { start.elapsed().as_secs_f32() ); - let chain = Mutex::new(LocalChain::from_changeset(init_changeset.0)); + let chain = Mutex::new(LocalChain::from_changeset(init_changeset.0)?); println!( "[{:>10}s] loaded local chain from changeset", start.elapsed().as_secs_f32() @@ -170,10 +170,7 @@ fn main() -> anyhow::Result<()> { let chain_tip = chain.lock().unwrap().tip(); let rpc_client = rpc_args.new_client()?; - let mut emitter = match chain_tip { - Some(cp) => Emitter::from_checkpoint(&rpc_client, cp), - None => Emitter::from_height(&rpc_client, fallback_height), - }; + let mut emitter = Emitter::new(&rpc_client, chain_tip, fallback_height); let mut last_db_commit = Instant::now(); let mut last_print = Instant::now(); @@ -205,23 +202,22 @@ fn main() -> anyhow::Result<()> { // print synced-to height and current balance in intervals if last_print.elapsed() >= STDOUT_PRINT_DELAY { last_print = Instant::now(); - if let Some(synced_to) = chain.tip() { - let balance = { - graph.graph().balance( - &*chain, - synced_to.block_id(), - graph.index.outpoints().iter().cloned(), - |(k, _), _| k == &Keychain::Internal, - ) - }; - println!( - "[{:>10}s] synced to {} @ {} | total: {} sats", - start.elapsed().as_secs_f32(), - synced_to.hash(), - synced_to.height(), - balance.total() - ); - } + let synced_to = chain.tip(); + let balance = { + graph.graph().balance( + &*chain, + synced_to.block_id(), + graph.index.outpoints().iter().cloned(), + |(k, _), _| k == &Keychain::Internal, + ) + }; + println!( + "[{:>10}s] synced to {} @ {} | total: {} sats", + start.elapsed().as_secs_f32(), + synced_to.hash(), + synced_to.height(), + balance.total() + ); } } @@ -253,10 +249,7 @@ fn main() -> anyhow::Result<()> { let (tx, rx) = std::sync::mpsc::sync_channel::(CHANNEL_BOUND); let emission_jh = std::thread::spawn(move || -> anyhow::Result<()> { let rpc_client = rpc_args.new_client()?; - let mut emitter = match last_cp { - Some(cp) => Emitter::from_checkpoint(&rpc_client, cp), - None => Emitter::from_height(&rpc_client, fallback_height), - }; + let mut emitter = Emitter::new(&rpc_client, last_cp, fallback_height); let mut block_count = rpc_client.get_block_count()? as u32; tx.send(Emission::Tip(block_count))?; @@ -335,24 +328,23 @@ fn main() -> anyhow::Result<()> { if last_print.map_or(Duration::MAX, |i| i.elapsed()) >= STDOUT_PRINT_DELAY { last_print = Some(Instant::now()); - if let Some(synced_to) = chain.tip() { - let balance = { - graph.graph().balance( - &*chain, - synced_to.block_id(), - graph.index.outpoints().iter().cloned(), - |(k, _), _| k == &Keychain::Internal, - ) - }; - println!( - "[{:>10}s] synced to {} @ {} / {} | total: {} sats", - start.elapsed().as_secs_f32(), - synced_to.hash(), - synced_to.height(), - tip_height, - balance.total() - ); - } + let synced_to = chain.tip(); + let balance = { + graph.graph().balance( + &*chain, + synced_to.block_id(), + graph.index.outpoints().iter().cloned(), + |(k, _), _| k == &Keychain::Internal, + ) + }; + println!( + "[{:>10}s] synced to {} @ {} / {} | total: {} sats", + start.elapsed().as_secs_f32(), + synced_to.hash(), + synced_to.height(), + tip_height, + balance.total() + ); } } diff --git a/example-crates/example_cli/src/lib.rs b/example-crates/example_cli/src/lib.rs index 9e572a892..364998034 100644 --- a/example-crates/example_cli/src/lib.rs +++ b/example-crates/example_cli/src/lib.rs @@ -315,10 +315,8 @@ where version: 0x02, // because the temporary planning module does not support timelocks, we can use the chain // tip as the `lock_time` for anti-fee-sniping purposes - lock_time: chain - .get_chain_tip()? - .and_then(|block_id| absolute::LockTime::from_height(block_id.height).ok()) - .unwrap_or(absolute::LockTime::ZERO), + lock_time: absolute::LockTime::from_height(chain.get_chain_tip()?.height) + .expect("invalid height"), input: selected_txos .iter() .map(|(_, utxo)| TxIn { @@ -404,7 +402,7 @@ pub fn planned_utxos, ) -> Result, FullTxOut)>, O::Error> { - let chain_tip = chain.get_chain_tip()?.unwrap_or_default(); + let chain_tip = chain.get_chain_tip()?; let outpoints = graph.index.outpoints().iter().cloned(); graph .graph() @@ -509,7 +507,7 @@ where let balance = graph.graph().try_balance( chain, - chain.get_chain_tip()?.unwrap_or_default(), + chain.get_chain_tip()?, graph.index.outpoints().iter().cloned(), |(k, _), _| k == &Keychain::Internal, )?; @@ -539,7 +537,7 @@ where Commands::TxOut { txout_cmd } => { let graph = &*graph.lock().unwrap(); let chain = &*chain.lock().unwrap(); - let chain_tip = chain.get_chain_tip()?.unwrap_or_default(); + let chain_tip = chain.get_chain_tip()?; let outpoints = graph.index.outpoints().iter().cloned(); match txout_cmd { diff --git a/example-crates/example_electrum/src/main.rs b/example-crates/example_electrum/src/main.rs index be5ffc7bc..a96378f64 100644 --- a/example-crates/example_electrum/src/main.rs +++ b/example-crates/example_electrum/src/main.rs @@ -112,7 +112,7 @@ fn main() -> anyhow::Result<()> { graph }); - let chain = Mutex::new(LocalChain::from_changeset(disk_local_chain)); + let chain = Mutex::new(LocalChain::from_changeset(disk_local_chain)?); let electrum_cmd = match &args.command { example_cli::Commands::ChainSpecific(electrum_cmd) => electrum_cmd, @@ -193,7 +193,7 @@ fn main() -> anyhow::Result<()> { // Get a short lock on the tracker to get the spks we're interested in let graph = graph.lock().unwrap(); let chain = chain.lock().unwrap(); - let chain_tip = chain.tip().map(|cp| cp.block_id()).unwrap_or_default(); + let chain_tip = chain.tip().block_id(); if !(all_spks || unused_spks || utxos || unconfirmed) { unused_spks = true; diff --git a/example-crates/example_esplora/src/main.rs b/example-crates/example_esplora/src/main.rs index df669d36f..c294986ad 100644 --- a/example-crates/example_esplora/src/main.rs +++ b/example-crates/example_esplora/src/main.rs @@ -5,10 +5,10 @@ use std::{ }; use bdk_chain::{ - bitcoin::{Address, Network, OutPoint, ScriptBuf, Txid}, + bitcoin::{constants::genesis_block, Address, Network, OutPoint, ScriptBuf, Txid}, indexed_tx_graph::{self, IndexedTxGraph}, keychain, - local_chain::{self, CheckPoint, LocalChain}, + local_chain::{self, LocalChain}, Append, ConfirmationTimeHeightAnchor, }; @@ -102,6 +102,8 @@ fn main() -> anyhow::Result<()> { let (args, keymap, index, db, init_changeset) = example_cli::init::(DB_MAGIC, DB_PATH)?; + let genesis_hash = genesis_block(args.network).block_hash(); + let (init_chain_changeset, init_indexed_tx_graph_changeset) = init_changeset; // Contruct `IndexedTxGraph` and `LocalChain` with our initial changeset. They are wrapped in @@ -113,8 +115,8 @@ fn main() -> anyhow::Result<()> { graph }); let chain = Mutex::new({ - let mut chain = LocalChain::default(); - chain.apply_changeset(&init_chain_changeset); + let (mut chain, _) = LocalChain::from_genesis_hash(genesis_hash); + chain.apply_changeset(&init_chain_changeset)?; chain }); @@ -234,7 +236,7 @@ fn main() -> anyhow::Result<()> { { let graph = graph.lock().unwrap(); let chain = chain.lock().unwrap(); - let chain_tip = chain.tip().map(|cp| cp.block_id()).unwrap_or_default(); + let chain_tip = chain.tip().block_id(); if *all_spks { let all_spks = graph @@ -332,7 +334,7 @@ fn main() -> anyhow::Result<()> { (missing_block_heights, tip) }; - println!("prev tip: {}", tip.as_ref().map_or(0, CheckPoint::height)); + println!("prev tip: {}", tip.height()); println!("missing block heights: {:?}", missing_block_heights); // Here, we actually fetch the missing blocks and create a `local_chain::Update`. From 7d5f31f6cc323241e866c136656f9674e0bf7c53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Thu, 26 Oct 2023 07:47:29 +0800 Subject: [PATCH 02/13] feat(chain, file_store): add `is_empty` method to `PersistBackend` trait --- crates/chain/src/persist.rs | 13 +++++++++++++ crates/file_store/src/store.rs | 21 +++++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/crates/chain/src/persist.rs b/crates/chain/src/persist.rs index 2ecc23c13..634e369e9 100644 --- a/crates/chain/src/persist.rs +++ b/crates/chain/src/persist.rs @@ -80,6 +80,15 @@ pub trait PersistBackend { /// Return the aggregate changeset `C` from persistence. fn load_from_persistence(&mut self) -> Result; + + /// Returns whether the persistence backend contains no data. + fn is_empty(&mut self) -> Result + where + C: Append, + { + self.load_from_persistence() + .map(|changeset| changeset.is_empty()) + } } impl PersistBackend for () { @@ -94,4 +103,8 @@ impl PersistBackend for () { fn load_from_persistence(&mut self) -> Result { Ok(C::default()) } + + fn is_empty(&mut self) -> Result { + Ok(true) + } } diff --git a/crates/file_store/src/store.rs b/crates/file_store/src/store.rs index a4aa2963c..fa73480e5 100644 --- a/crates/file_store/src/store.rs +++ b/crates/file_store/src/store.rs @@ -37,6 +37,14 @@ where let (changeset, result) = self.aggregate_changesets(); result.map(|_| changeset) } + + fn is_empty(&mut self) -> Result { + let init_pos = self.db_file.stream_position()?; + let stream_len = self.db_file.seek(io::SeekFrom::End(0))?; + let magic_len = self.magic.len() as u64; + self.db_file.seek(io::SeekFrom::Start(init_pos))?; + Ok(stream_len == magic_len) + } } impl<'a, C> Store<'a, C> @@ -182,6 +190,19 @@ mod test { #[derive(Debug)] struct TestTracker; + #[test] + fn is_empty() { + let mut file = NamedTempFile::new().unwrap(); + file.write_all(&TEST_MAGIC_BYTES).expect("should write"); + + let mut db = Store::::new(&TEST_MAGIC_BYTES, file.reopen().unwrap()) + .expect("must open"); + assert!(db.is_empty().expect("must read")); + db.write_changes(&vec!["hello".to_string(), "world".to_string()]) + .expect("must write"); + assert!(!db.is_empty().expect("must read")); + } + #[test] fn new_fails_if_file_is_too_short() { let mut file = NamedTempFile::new().unwrap(); From 6cf3963c6cfeacc8cd9d59bdb0bafded5022baaf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Thu, 26 Oct 2023 06:20:37 +0800 Subject: [PATCH 03/13] feat(bdk)!: have separate methods for creating and loading `Wallet` `Wallet::new` now creates a new wallet. `Wallet::load` loads an existing wallet. The network type is now recoverable from persistence. Error types have been simplified. --- crates/bdk/src/error.rs | 9 - crates/bdk/src/wallet/mod.rs | 194 +++++++++++------- example-crates/wallet_electrum/src/main.rs | 18 +- .../wallet_esplora_async/src/main.rs | 18 +- .../wallet_esplora_blocking/src/main.rs | 18 +- 5 files changed, 157 insertions(+), 100 deletions(-) diff --git a/crates/bdk/src/error.rs b/crates/bdk/src/error.rs index 3bd3dc6d3..fcb5a6f7b 100644 --- a/crates/bdk/src/error.rs +++ b/crates/bdk/src/error.rs @@ -199,12 +199,3 @@ impl_error!(miniscript::Error, Miniscript); impl_error!(MiniscriptPsbtError, MiniscriptPsbt); impl_error!(bitcoin::bip32::Error, Bip32); impl_error!(bitcoin::psbt::Error, Psbt); - -impl From for Error { - fn from(e: crate::wallet::NewNoPersistError) -> Self { - match e { - wallet::NewNoPersistError::Descriptor(e) => Error::Descriptor(e), - unknown_network_err => Error::Generic(format!("{}", unknown_network_err)), - } - } -} diff --git a/crates/bdk/src/wallet/mod.rs b/crates/bdk/src/wallet/mod.rs index 275487a58..05a834266 100644 --- a/crates/bdk/src/wallet/mod.rs +++ b/crates/bdk/src/wallet/mod.rs @@ -128,12 +128,18 @@ pub struct ChangeSet { ConfirmationTimeHeightAnchor, keychain::ChangeSet, >, + + /// Stores the network type of the wallet. + pub network: Option, } impl Append for ChangeSet { fn append(&mut self, other: Self) { Append::append(&mut self.chain, other.chain); Append::append(&mut self.indexed_tx_graph, other.indexed_tx_graph); + if other.network.is_some() { + self.network = other.network; + } } fn is_empty(&self) -> bool { @@ -225,75 +231,81 @@ impl Wallet { descriptor: E, change_descriptor: Option, network: Network, - ) -> Result { + ) -> Result { Self::new(descriptor, change_descriptor, (), network).map_err(|e| match e { - NewError::Descriptor(e) => NewNoPersistError::Descriptor(e), - NewError::Persist(_) | NewError::InvalidPersistenceGenesis => { - unreachable!("no persistence so it can't fail") - } - NewError::UnknownNetwork => NewNoPersistError::UnknownNetwork, + NewError::Descriptor(e) => e, + NewError::Write(_) => unreachable!("mock-write must always succeed"), }) } + + /// Creates a wallet that does not persist data, with a custom genesis hash. + pub fn new_no_persist_with_genesis_hash( + descriptor: E, + change_descriptor: Option, + network: Network, + genesis_hash: BlockHash, + ) -> Result { + Self::new_with_genesis_hash(descriptor, change_descriptor, (), network, genesis_hash) + .map_err(|e| match e { + NewError::Descriptor(e) => e, + NewError::Write(_) => unreachable!("mock-write must always succeed"), + }) + } } -/// Error returned from [`Wallet::new_no_persist`] #[derive(Debug)] -pub enum NewNoPersistError { - /// There was problem with the descriptors passed in +/// Error returned from [`Wallet::new`] +pub enum NewError { + /// There was problem with the passed-in descriptor(s). Descriptor(crate::descriptor::DescriptorError), - /// We cannot determine the genesis hash from the network. - UnknownNetwork, + /// We were unable to write the wallet's data to the persistence backend. + Write(W), } -impl fmt::Display for NewNoPersistError { +impl fmt::Display for NewError +where + W: fmt::Display, +{ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { - NewNoPersistError::Descriptor(e) => e.fmt(f), - NewNoPersistError::UnknownNetwork => write!( - f, - "unknown network - genesis block hash needs to be provided explicitly" - ), + NewError::Descriptor(e) => e.fmt(f), + NewError::Write(e) => e.fmt(f), } } } #[cfg(feature = "std")] -impl std::error::Error for NewNoPersistError {} +impl std::error::Error for NewError where W: core::fmt::Display + core::fmt::Debug {} +/// An error that may occur when loading a [`Wallet`] from persistence. #[derive(Debug)] -/// Error returned from [`Wallet::new`] -pub enum NewError { - /// There was problem with the descriptors passed in +pub enum LoadError { + /// There was a problem with the passed-in descriptor(s). Descriptor(crate::descriptor::DescriptorError), - /// We were unable to load the wallet's data from the persistence backend - Persist(PE), - /// We cannot determine the genesis hash from the network - UnknownNetwork, - /// The genesis block hash is either missing from persistence or has an unexpected value - InvalidPersistenceGenesis, + /// Loading data from the persistence backend failed. + Load(L), + /// Data loaded from persistence is missing network type. + MissingNetwork, + /// Data loaded from persistence is missing genesis hash. + MissingGenesis, } -impl fmt::Display for NewError +impl fmt::Display for LoadError where - PE: fmt::Display, + L: fmt::Display, { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { - NewError::Descriptor(e) => e.fmt(f), - NewError::Persist(e) => { - write!(f, "failed to load wallet from persistence backend: {}", e) - } - NewError::UnknownNetwork => write!( - f, - "unknown network - genesis block hash needs to be provided explicitly" - ), - NewError::InvalidPersistenceGenesis => write!(f, "the genesis block hash is either missing from persistence or has an unexpected value"), + LoadError::Descriptor(e) => e.fmt(f), + LoadError::Load(e) => e.fmt(f), + LoadError::MissingNetwork => write!(f, "loaded data is missing network type"), + LoadError::MissingGenesis => write!(f, "loaded data is missing genesis hash"), } } } #[cfg(feature = "std")] -impl std::error::Error for NewError where PE: core::fmt::Display + core::fmt::Debug {} +impl std::error::Error for LoadError where L: core::fmt::Display + core::fmt::Debug {} /// An error that may occur when inserting a transaction into [`Wallet`]. #[derive(Debug)] @@ -316,30 +328,29 @@ impl Wallet { change_descriptor: Option, db: D, network: Network, - ) -> Result> + ) -> Result> where D: PersistBackend, { - Self::with_custom_genesis_hash(descriptor, change_descriptor, db, network, None) + let genesis_hash = genesis_block(network).block_hash(); + Self::new_with_genesis_hash(descriptor, change_descriptor, db, network, genesis_hash) } /// Create a new [`Wallet`] with a custom genesis hash. /// /// This is like [`Wallet::new`] with an additional `custom_genesis_hash` parameter. - pub fn with_custom_genesis_hash( + pub fn new_with_genesis_hash( descriptor: E, change_descriptor: Option, - mut db: D, + db: D, network: Network, - custom_genesis_hash: Option, - ) -> Result> + genesis_hash: BlockHash, + ) -> Result> where D: PersistBackend, { let secp = Secp256k1::new(); - let genesis_hash = - custom_genesis_hash.unwrap_or_else(|| genesis_block(network).block_hash()); - let (mut chain, _) = LocalChain::from_genesis_hash(genesis_hash); + let (chain, _) = LocalChain::from_genesis_hash(genesis_hash); let mut indexed_graph = IndexedTxGraph::< ConfirmationTimeHeightAnchor, KeychainTxOutIndex, @@ -351,42 +362,85 @@ impl Wallet { .index .add_keychain(KeychainKind::External, descriptor.clone()); let signers = Arc::new(SignersContainer::build(keymap, &descriptor, &secp)); - let change_signers = match change_descriptor { - Some(desc) => { - let (change_descriptor, change_keymap) = - into_wallet_descriptor_checked(desc, &secp, network) - .map_err(NewError::Descriptor)?; - - let change_signers = Arc::new(SignersContainer::build( - change_keymap, - &change_descriptor, - &secp, - )); + let change_signers = Arc::new(match change_descriptor { + Some(desc) => { + let (descriptor, keymap) = into_wallet_descriptor_checked(desc, &secp, network) + .map_err(NewError::Descriptor)?; + let signers = SignersContainer::build(keymap, &descriptor, &secp); indexed_graph .index - .add_keychain(KeychainKind::Internal, change_descriptor); - - change_signers + .add_keychain(KeychainKind::Internal, descriptor); + signers } - None => Arc::new(SignersContainer::new()), - }; + None => SignersContainer::new(), + }); + + let mut persist = Persist::new(db); + persist.stage(ChangeSet { + chain: chain.initial_changeset(), + indexed_tx_graph: indexed_graph.initial_changeset(), + network: Some(network), + }); + persist.commit().map_err(NewError::Write)?; + + Ok(Wallet { + signers, + change_signers, + network, + chain, + indexed_graph, + persist, + secp, + }) + } - let changeset = db.load_from_persistence().map_err(NewError::Persist)?; - chain - .apply_changeset(&changeset.chain) - .map_err(|_| NewError::InvalidPersistenceGenesis)?; - indexed_graph.apply_changeset(changeset.indexed_tx_graph); + /// Load [`Wallet`] from persistence. + pub fn load( + descriptor: E, + change_descriptor: Option, + mut db: D, + ) -> Result> + where + D: PersistBackend, + { + let secp = Secp256k1::new(); + + let changeset = db.load_from_persistence().map_err(LoadError::Load)?; + let network = changeset.network.ok_or(LoadError::MissingNetwork)?; + let chain = + LocalChain::from_changeset(changeset.chain).map_err(|_| LoadError::MissingGenesis)?; + + let mut index = KeychainTxOutIndex::::default(); + + let (descriptor, keymap) = into_wallet_descriptor_checked(descriptor, &secp, network) + .map_err(LoadError::Descriptor)?; + let signers = Arc::new(SignersContainer::build(keymap, &descriptor, &secp)); + index.add_keychain(KeychainKind::External, descriptor); + + let change_signers = Arc::new(match change_descriptor { + Some(descriptor) => { + let (descriptor, keymap) = + into_wallet_descriptor_checked(descriptor, &secp, network) + .map_err(LoadError::Descriptor)?; + let signers = SignersContainer::build(keymap, &descriptor, &secp); + index.add_keychain(KeychainKind::Internal, descriptor); + signers + } + None => SignersContainer::new(), + }); + + let indexed_graph = IndexedTxGraph::new(index); let persist = Persist::new(db); Ok(Wallet { signers, change_signers, - network, chain, indexed_graph, persist, + network, secp, }) } diff --git a/example-crates/wallet_electrum/src/main.rs b/example-crates/wallet_electrum/src/main.rs index a6d7ca520..da0c5e5dd 100644 --- a/example-crates/wallet_electrum/src/main.rs +++ b/example-crates/wallet_electrum/src/main.rs @@ -18,16 +18,20 @@ use bdk_file_store::Store; fn main() -> Result<(), Box> { let db_path = std::env::temp_dir().join("bdk-electrum-example"); - let db = Store::::new_from_path(DB_MAGIC.as_bytes(), db_path)?; + let mut db = Store::::new_from_path(DB_MAGIC.as_bytes(), db_path)?; let external_descriptor = "wpkh(tprv8ZgxMBicQKsPdy6LMhUtFHAgpocR8GC6QmwMSFpZs7h6Eziw3SpThFfczTDh5rW2krkqffa11UpX3XkeTTB2FvzZKWXqPY54Y6Rq4AQ5R8L/84'/1'/0'/0/*)"; let internal_descriptor = "wpkh(tprv8ZgxMBicQKsPdy6LMhUtFHAgpocR8GC6QmwMSFpZs7h6Eziw3SpThFfczTDh5rW2krkqffa11UpX3XkeTTB2FvzZKWXqPY54Y6Rq4AQ5R8L/84'/1'/0'/1/*)"; - let mut wallet = Wallet::new( - external_descriptor, - Some(internal_descriptor), - db, - Network::Testnet, - )?; + let mut wallet = if db.is_empty()? { + Wallet::new( + external_descriptor, + Some(internal_descriptor), + db, + Network::Testnet, + )? + } else { + Wallet::load(external_descriptor, Some(internal_descriptor), db)? + }; let address = wallet.get_address(bdk::wallet::AddressIndex::New); println!("Generated Address: {}", address); diff --git a/example-crates/wallet_esplora_async/src/main.rs b/example-crates/wallet_esplora_async/src/main.rs index ff1bbfb6d..5c7d09d59 100644 --- a/example-crates/wallet_esplora_async/src/main.rs +++ b/example-crates/wallet_esplora_async/src/main.rs @@ -16,16 +16,20 @@ const PARALLEL_REQUESTS: usize = 5; #[tokio::main] async fn main() -> Result<(), Box> { let db_path = std::env::temp_dir().join("bdk-esplora-async-example"); - let db = Store::::new_from_path(DB_MAGIC.as_bytes(), db_path)?; + let mut db = Store::::new_from_path(DB_MAGIC.as_bytes(), db_path)?; let external_descriptor = "wpkh(tprv8ZgxMBicQKsPdy6LMhUtFHAgpocR8GC6QmwMSFpZs7h6Eziw3SpThFfczTDh5rW2krkqffa11UpX3XkeTTB2FvzZKWXqPY54Y6Rq4AQ5R8L/84'/1'/0'/0/*)"; let internal_descriptor = "wpkh(tprv8ZgxMBicQKsPdy6LMhUtFHAgpocR8GC6QmwMSFpZs7h6Eziw3SpThFfczTDh5rW2krkqffa11UpX3XkeTTB2FvzZKWXqPY54Y6Rq4AQ5R8L/84'/1'/0'/1/*)"; - let mut wallet = Wallet::new( - external_descriptor, - Some(internal_descriptor), - db, - Network::Testnet, - )?; + let mut wallet = if db.is_empty()? { + Wallet::new( + external_descriptor, + Some(internal_descriptor), + db, + Network::Testnet, + )? + } else { + Wallet::load(external_descriptor, Some(internal_descriptor), db)? + }; let address = wallet.get_address(AddressIndex::New); println!("Generated Address: {}", address); diff --git a/example-crates/wallet_esplora_blocking/src/main.rs b/example-crates/wallet_esplora_blocking/src/main.rs index 71554b0a8..f4de498cd 100644 --- a/example-crates/wallet_esplora_blocking/src/main.rs +++ b/example-crates/wallet_esplora_blocking/src/main.rs @@ -15,16 +15,20 @@ use bdk_file_store::Store; fn main() -> Result<(), Box> { let db_path = std::env::temp_dir().join("bdk-esplora-example"); - let db = Store::::new_from_path(DB_MAGIC.as_bytes(), db_path)?; + let mut db = Store::::new_from_path(DB_MAGIC.as_bytes(), db_path)?; let external_descriptor = "wpkh(tprv8ZgxMBicQKsPdy6LMhUtFHAgpocR8GC6QmwMSFpZs7h6Eziw3SpThFfczTDh5rW2krkqffa11UpX3XkeTTB2FvzZKWXqPY54Y6Rq4AQ5R8L/84'/1'/0'/0/*)"; let internal_descriptor = "wpkh(tprv8ZgxMBicQKsPdy6LMhUtFHAgpocR8GC6QmwMSFpZs7h6Eziw3SpThFfczTDh5rW2krkqffa11UpX3XkeTTB2FvzZKWXqPY54Y6Rq4AQ5R8L/84'/1'/0'/1/*)"; - let mut wallet = Wallet::new( - external_descriptor, - Some(internal_descriptor), - db, - Network::Testnet, - )?; + let mut wallet = if db.is_empty()? { + Wallet::new( + external_descriptor, + Some(internal_descriptor), + db, + Network::Testnet, + )? + } else { + Wallet::load(external_descriptor, Some(internal_descriptor), db)? + }; let address = wallet.get_address(AddressIndex::New); println!("Generated Address: {}", address); From 7c6cbc4d9f7349e769594151936b3e2947b79d00 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Thu, 26 Oct 2023 08:12:09 +0800 Subject: [PATCH 04/13] chore(file_store): rm empty test file --- crates/file_store/tests/test_file_store.rs | 1 - 1 file changed, 1 deletion(-) delete mode 100644 crates/file_store/tests/test_file_store.rs diff --git a/crates/file_store/tests/test_file_store.rs b/crates/file_store/tests/test_file_store.rs deleted file mode 100644 index 8b1378917..000000000 --- a/crates/file_store/tests/test_file_store.rs +++ /dev/null @@ -1 +0,0 @@ - From d294e2e3189dc14efe5b9916cf83f5e7f8592c64 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Fri, 27 Oct 2023 14:14:25 +0800 Subject: [PATCH 05/13] feat(wallet)!: add `new_or_load` methods These methods try to load wallet from persistence and initializes the wallet instead if non-existant. An internal helper method `create_signers` is added to reuse code. Documentation is also improved. --- crates/bdk/src/wallet/mod.rs | 241 ++++++++++++++---- .../wallet_esplora_async/src/main.rs | 1 + .../wallet_esplora_blocking/src/main.rs | 1 + 3 files changed, 191 insertions(+), 52 deletions(-) diff --git a/crates/bdk/src/wallet/mod.rs b/crates/bdk/src/wallet/mod.rs index 05a834266..ea76ad656 100644 --- a/crates/bdk/src/wallet/mod.rs +++ b/crates/bdk/src/wallet/mod.rs @@ -28,7 +28,7 @@ use bdk_chain::{ Append, BlockId, ChainPosition, ConfirmationTime, ConfirmationTimeHeightAnchor, FullTxOut, IndexedTxGraph, Persist, PersistBackend, }; -use bitcoin::secp256k1::Secp256k1; +use bitcoin::secp256k1::{All, Secp256k1}; use bitcoin::sighash::{EcdsaSighashType, TapSighashType}; use bitcoin::{ absolute, Address, Network, OutPoint, Script, ScriptBuf, Sequence, Transaction, TxOut, Txid, @@ -253,8 +253,13 @@ impl Wallet { } } +/// The error type when constructing a fresh [`Wallet`]. +/// +/// Methods [`new`] and [`new_with_genesis_hash`] may return this error. +/// +/// [`new`]: Wallet::new +/// [`new_with_genesis_hash`]: Wallet::new_with_genesis_hash #[derive(Debug)] -/// Error returned from [`Wallet::new`] pub enum NewError { /// There was problem with the passed-in descriptor(s). Descriptor(crate::descriptor::DescriptorError), @@ -277,7 +282,11 @@ where #[cfg(feature = "std")] impl std::error::Error for NewError where W: core::fmt::Display + core::fmt::Debug {} -/// An error that may occur when loading a [`Wallet`] from persistence. +/// The error type when loading a [`Wallet`] from persistence. +/// +/// Method [`load`] may return this error. +/// +/// [`load`]: Wallet::load #[derive(Debug)] pub enum LoadError { /// There was a problem with the passed-in descriptor(s). @@ -307,6 +316,64 @@ where #[cfg(feature = "std")] impl std::error::Error for LoadError where L: core::fmt::Display + core::fmt::Debug {} +/// Error type for when we try load a [`Wallet`] from persistence and creating it if non-existant. +/// +/// Methods [`new_or_load`] and [`new_or_load_with_genesis_hash`] may return this error. +/// +/// [`new_or_load`]: Wallet::new_or_load +/// [`new_or_load_with_genesis_hash`]: Wallet::new_or_load_with_genesis_hash +#[derive(Debug)] +pub enum NewOrLoadError { + /// There is a problem with the passed-in descriptor. + Descriptor(crate::descriptor::DescriptorError), + /// Writing to the persistence backend failed. + Write(W), + /// Loading from the persistence backend failed. + Load(L), + /// The loaded genesis hash does not match what was provided. + LoadedGenesisDoesNotMatch { + /// The expected genesis block hash. + expected: BlockHash, + /// The block hash loaded from persistence. + got: Option, + }, + /// The loaded network type does not match what was provided. + LoadedNetworkDoesNotMatch { + /// The expected network type. + expected: Network, + /// The network type loaded from persistence. + got: Option, + }, +} + +impl fmt::Display for NewOrLoadError +where + W: fmt::Display, + L: fmt::Display, +{ + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + NewOrLoadError::Descriptor(e) => e.fmt(f), + NewOrLoadError::Write(e) => write!(f, "failed to write to persistence: {}", e), + NewOrLoadError::Load(e) => write!(f, "failed to load from persistence: {}", e), + NewOrLoadError::LoadedGenesisDoesNotMatch { expected, got } => { + write!(f, "loaded genesis hash is not {}, got {:?}", expected, got) + } + NewOrLoadError::LoadedNetworkDoesNotMatch { expected, got } => { + write!(f, "loaded network type is not {}, got {:?}", expected, got) + } + } + } +} + +#[cfg(feature = "std")] +impl std::error::Error for NewOrLoadError +where + W: core::fmt::Display + core::fmt::Debug, + L: core::fmt::Display + core::fmt::Debug, +{ +} + /// An error that may occur when inserting a transaction into [`Wallet`]. #[derive(Debug)] pub enum InsertTxError { @@ -321,8 +388,7 @@ pub enum InsertTxError { } impl Wallet { - /// Create a wallet from a `descriptor` (and an optional `change_descriptor`) and load related - /// transaction data from `db`. + /// Initialize an empty [`Wallet`]. pub fn new( descriptor: E, change_descriptor: Option, @@ -336,9 +402,10 @@ impl Wallet { Self::new_with_genesis_hash(descriptor, change_descriptor, db, network, genesis_hash) } - /// Create a new [`Wallet`] with a custom genesis hash. + /// Initialize an empty [`Wallet`] with a custom genesis hash. /// - /// This is like [`Wallet::new`] with an additional `custom_genesis_hash` parameter. + /// This is like [`Wallet::new`] with an additional `genesis_hash` parameter. This is useful + /// for syncing from alternative networks. pub fn new_with_genesis_hash( descriptor: E, change_descriptor: Option, @@ -350,35 +417,18 @@ impl Wallet { D: PersistBackend, { let secp = Secp256k1::new(); - let (chain, _) = LocalChain::from_genesis_hash(genesis_hash); - let mut indexed_graph = IndexedTxGraph::< - ConfirmationTimeHeightAnchor, - KeychainTxOutIndex, - >::default(); + let (chain, chain_changeset) = LocalChain::from_genesis_hash(genesis_hash); + let mut index = KeychainTxOutIndex::::default(); - let (descriptor, keymap) = into_wallet_descriptor_checked(descriptor, &secp, network) - .map_err(NewError::Descriptor)?; - indexed_graph - .index - .add_keychain(KeychainKind::External, descriptor.clone()); - let signers = Arc::new(SignersContainer::build(keymap, &descriptor, &secp)); - - let change_signers = Arc::new(match change_descriptor { - Some(desc) => { - let (descriptor, keymap) = into_wallet_descriptor_checked(desc, &secp, network) - .map_err(NewError::Descriptor)?; - let signers = SignersContainer::build(keymap, &descriptor, &secp); - indexed_graph - .index - .add_keychain(KeychainKind::Internal, descriptor); - signers - } - None => SignersContainer::new(), - }); + let (signers, change_signers) = + create_signers(&mut index, &secp, descriptor, change_descriptor, network) + .map_err(NewError::Descriptor)?; + + let indexed_graph = IndexedTxGraph::new(index); let mut persist = Persist::new(db); persist.stage(ChangeSet { - chain: chain.initial_changeset(), + chain: chain_changeset, indexed_tx_graph: indexed_graph.initial_changeset(), network: Some(network), }); @@ -395,7 +445,7 @@ impl Wallet { }) } - /// Load [`Wallet`] from persistence. + /// Load [`Wallet`] from the given persistence backend. pub fn load( descriptor: E, change_descriptor: Option, @@ -405,31 +455,15 @@ impl Wallet { D: PersistBackend, { let secp = Secp256k1::new(); - let changeset = db.load_from_persistence().map_err(LoadError::Load)?; let network = changeset.network.ok_or(LoadError::MissingNetwork)?; - let chain = LocalChain::from_changeset(changeset.chain).map_err(|_| LoadError::MissingGenesis)?; - let mut index = KeychainTxOutIndex::::default(); - let (descriptor, keymap) = into_wallet_descriptor_checked(descriptor, &secp, network) - .map_err(LoadError::Descriptor)?; - let signers = Arc::new(SignersContainer::build(keymap, &descriptor, &secp)); - index.add_keychain(KeychainKind::External, descriptor); - - let change_signers = Arc::new(match change_descriptor { - Some(descriptor) => { - let (descriptor, keymap) = - into_wallet_descriptor_checked(descriptor, &secp, network) - .map_err(LoadError::Descriptor)?; - let signers = SignersContainer::build(keymap, &descriptor, &secp); - index.add_keychain(KeychainKind::Internal, descriptor); - signers - } - None => SignersContainer::new(), - }); + let (signers, change_signers) = + create_signers(&mut index, &secp, descriptor, change_descriptor, network) + .map_err(LoadError::Descriptor)?; let indexed_graph = IndexedTxGraph::new(index); let persist = Persist::new(db); @@ -445,6 +479,85 @@ impl Wallet { }) } + /// Either loads [`Wallet`] from persistence, or initializes it if it does not exist. + /// + /// This method will fail if the loaded [`Wallet`] has different parameters to those provided. + pub fn new_or_load( + descriptor: E, + change_descriptor: Option, + db: D, + network: Network, + ) -> Result> + where + D: PersistBackend, + { + let genesis_hash = genesis_block(network).block_hash(); + Self::new_or_load_with_genesis_hash( + descriptor, + change_descriptor, + db, + network, + genesis_hash, + ) + } + + /// Either loads [`Wallet`] from persistence, or initializes it if it does not exist (with a + /// custom genesis hash). + /// + /// This method will fail if the loaded [`Wallet`] has different parameters to those provided. + /// This is like [`Wallet::new_or_load`] with an additional `genesis_hash` parameter. This is + /// useful for syncing from alternative networks. + pub fn new_or_load_with_genesis_hash( + descriptor: E, + change_descriptor: Option, + mut db: D, + network: Network, + genesis_hash: BlockHash, + ) -> Result> + where + D: PersistBackend, + { + if db.is_empty().map_err(NewOrLoadError::Load)? { + return Self::new_with_genesis_hash( + descriptor, + change_descriptor, + db, + network, + genesis_hash, + ) + .map_err(|e| match e { + NewError::Descriptor(e) => NewOrLoadError::Descriptor(e), + NewError::Write(e) => NewOrLoadError::Write(e), + }); + } + + let wallet = Self::load(descriptor, change_descriptor, db).map_err(|e| match e { + LoadError::Descriptor(e) => NewOrLoadError::Descriptor(e), + LoadError::Load(e) => NewOrLoadError::Load(e), + LoadError::MissingNetwork => NewOrLoadError::LoadedNetworkDoesNotMatch { + expected: network, + got: None, + }, + LoadError::MissingGenesis => NewOrLoadError::LoadedGenesisDoesNotMatch { + expected: genesis_hash, + got: None, + }, + })?; + if wallet.network != network { + return Err(NewOrLoadError::LoadedNetworkDoesNotMatch { + expected: network, + got: Some(wallet.network), + }); + } + if wallet.chain.genesis_hash() != genesis_hash { + return Err(NewOrLoadError::LoadedGenesisDoesNotMatch { + expected: genesis_hash, + got: Some(wallet.chain.genesis_hash()), + }); + } + Ok(wallet) + } + /// Get the Bitcoin network the wallet is using. pub fn network(&self) -> Network { self.network @@ -2158,6 +2271,30 @@ fn new_local_utxo( } } +fn create_signers( + index: &mut KeychainTxOutIndex, + secp: &Secp256k1, + descriptor: E, + change_descriptor: Option, + network: Network, +) -> Result<(Arc, Arc), crate::descriptor::error::Error> { + let (descriptor, keymap) = into_wallet_descriptor_checked(descriptor, secp, network)?; + let signers = Arc::new(SignersContainer::build(keymap, &descriptor, secp)); + index.add_keychain(KeychainKind::External, descriptor); + + let change_signers = match change_descriptor { + Some(descriptor) => { + let (descriptor, keymap) = into_wallet_descriptor_checked(descriptor, secp, network)?; + let signers = Arc::new(SignersContainer::build(keymap, &descriptor, secp)); + index.add_keychain(KeychainKind::Internal, descriptor); + signers + } + None => Arc::new(SignersContainer::new()), + }; + + Ok((signers, change_signers)) +} + #[macro_export] #[doc(hidden)] /// Macro for getting a wallet for use in a doctest diff --git a/example-crates/wallet_esplora_async/src/main.rs b/example-crates/wallet_esplora_async/src/main.rs index 5c7d09d59..e44db9d8d 100644 --- a/example-crates/wallet_esplora_async/src/main.rs +++ b/example-crates/wallet_esplora_async/src/main.rs @@ -2,6 +2,7 @@ use std::{io::Write, str::FromStr}; use bdk::{ bitcoin::{Address, Network}, + chain::PersistBackend, wallet::{AddressIndex, Update}, SignOptions, Wallet, }; diff --git a/example-crates/wallet_esplora_blocking/src/main.rs b/example-crates/wallet_esplora_blocking/src/main.rs index f4de498cd..ec1076597 100644 --- a/example-crates/wallet_esplora_blocking/src/main.rs +++ b/example-crates/wallet_esplora_blocking/src/main.rs @@ -7,6 +7,7 @@ use std::{io::Write, str::FromStr}; use bdk::{ bitcoin::{Address, Network}, + chain::PersistBackend, wallet::{AddressIndex, Update}, SignOptions, Wallet, }; From 24994a3ed47aacf203ca0b456eb22f4b93ec58a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Mon, 30 Oct 2023 11:02:50 +0800 Subject: [PATCH 06/13] feat(file_store)!: have separate methods for creating and opening Store --- crates/file_store/src/store.rs | 94 ++++++++++++------- example-crates/example_cli/src/lib.rs | 2 +- example-crates/wallet_electrum/src/main.rs | 2 +- .../wallet_esplora_async/src/main.rs | 2 +- .../wallet_esplora_blocking/src/main.rs | 2 +- 5 files changed, 64 insertions(+), 38 deletions(-) diff --git a/crates/file_store/src/store.rs b/crates/file_store/src/store.rs index fa73480e5..22e8b39d6 100644 --- a/crates/file_store/src/store.rs +++ b/crates/file_store/src/store.rs @@ -51,20 +51,55 @@ impl<'a, C> Store<'a, C> where C: Default + Append + serde::Serialize + serde::de::DeserializeOwned, { - /// Creates a new store from a [`File`]. + /// Create a new [`Store`] file in write-only mode; error if the file exists. /// - /// The file must have been opened with read and write permissions. + /// `magic` is the prefixed bytes to write to the new file. This will be checked when opening + /// the `Store` in the future with [`open`]. /// - /// `magic` is the expected prefixed bytes of the file. If this does not match, an error will be - /// returned. + /// [`open`]: Store::open + pub fn create_new

(magic: &'a [u8], file_path: P) -> Result + where + P: AsRef, + { + if file_path.as_ref().exists() { + // `io::Error` is used instead of a variant on `FileError` because there is already a + // nightly-only `File::create_new` method + return Err(FileError::Io(io::Error::new( + io::ErrorKind::Other, + "file already exists", + ))); + } + let mut f = OpenOptions::new() + .create(true) + .read(true) + .write(true) + .open(file_path)?; + f.write_all(magic)?; + Ok(Self { + magic, + db_file: f, + marker: Default::default(), + }) + } + + /// Open an existing [`Store`]. + /// + /// Use [`create_new`] to create a new `Store`. /// - /// [`File`]: std::fs::File - pub fn new(magic: &'a [u8], mut db_file: File) -> Result { - db_file.rewind()?; + /// # Errors + /// + /// If the prefixed bytes of the opened file does not match the provided `magic`, the + /// [`FileError::InvalidMagicBytes`] error variant will be returned. + /// + /// [`create_new`]: Store::create_new + pub fn open

(magic: &'a [u8], file_path: P) -> Result + where + P: AsRef, + { + let mut f = OpenOptions::new().read(true).write(true).open(file_path)?; let mut magic_buf = vec![0_u8; magic.len()]; - db_file.read_exact(magic_buf.as_mut())?; - + f.read_exact(&mut magic_buf)?; if magic_buf != magic { return Err(FileError::InvalidMagicBytes { got: magic_buf, @@ -74,35 +109,26 @@ where Ok(Self { magic, - db_file, + db_file: f, marker: Default::default(), }) } - /// Creates or loads a store from `db_path`. - /// - /// If no file exists there, it will be created. + /// Attempt to open existing [`Store`] file; create it if the file is non-existant. /// - /// Refer to [`new`] for documentation on the `magic` input. + /// Internally, this calls either [`open`] or [`create_new`]. /// - /// [`new`]: Self::new - pub fn new_from_path

(magic: &'a [u8], db_path: P) -> Result + /// [`open`]: Store::open + /// [`create_new`]: Store::create_new + pub fn open_or_create_new

(magic: &'a [u8], file_path: P) -> Result where P: AsRef, { - let already_exists = db_path.as_ref().exists(); - - let mut db_file = OpenOptions::new() - .read(true) - .write(true) - .create(true) - .open(db_path)?; - - if !already_exists { - db_file.write_all(magic)?; + if file_path.as_ref().exists() { + Self::open(magic, file_path) + } else { + Self::create_new(magic, file_path) } - - Self::new(magic, db_file) } /// Iterates over the stored changeset from first to last, changing the seek position at each @@ -195,8 +221,8 @@ mod test { let mut file = NamedTempFile::new().unwrap(); file.write_all(&TEST_MAGIC_BYTES).expect("should write"); - let mut db = Store::::new(&TEST_MAGIC_BYTES, file.reopen().unwrap()) - .expect("must open"); + let mut db = + Store::::open(&TEST_MAGIC_BYTES, file.path()).expect("must open"); assert!(db.is_empty().expect("must read")); db.write_changes(&vec!["hello".to_string(), "world".to_string()]) .expect("must write"); @@ -209,7 +235,7 @@ mod test { file.write_all(&TEST_MAGIC_BYTES[..TEST_MAGIC_BYTES_LEN - 1]) .expect("should write"); - match Store::::new(&TEST_MAGIC_BYTES, file.reopen().unwrap()) { + match Store::::open(&TEST_MAGIC_BYTES, file.path()) { Err(FileError::Io(e)) => assert_eq!(e.kind(), std::io::ErrorKind::UnexpectedEof), unexpected => panic!("unexpected result: {:?}", unexpected), }; @@ -223,7 +249,7 @@ mod test { file.write_all(invalid_magic_bytes.as_bytes()) .expect("should write"); - match Store::::new(&TEST_MAGIC_BYTES, file.reopen().unwrap()) { + match Store::::open(&TEST_MAGIC_BYTES, file.path()) { Err(FileError::InvalidMagicBytes { got, .. }) => { assert_eq!(got, invalid_magic_bytes.as_bytes()) } @@ -242,8 +268,8 @@ mod test { let mut file = NamedTempFile::new().unwrap(); file.write_all(&data).expect("should write"); - let mut store = Store::::new(&TEST_MAGIC_BYTES, file.reopen().unwrap()) - .expect("should open"); + let mut store = + Store::::open(&TEST_MAGIC_BYTES, file.path()).expect("should open"); match store.iter_changesets().next() { Some(Err(IterError::Bincode(_))) => {} unexpected_res => panic!("unexpected result: {:?}", unexpected_res), diff --git a/example-crates/example_cli/src/lib.rs b/example-crates/example_cli/src/lib.rs index 364998034..0b5d9cd37 100644 --- a/example-crates/example_cli/src/lib.rs +++ b/example-crates/example_cli/src/lib.rs @@ -681,7 +681,7 @@ where index.add_keychain(Keychain::Internal, internal_descriptor); } - let mut db_backend = match Store::<'m, C>::new_from_path(db_magic, &args.db_path) { + let mut db_backend = match Store::<'m, C>::open_or_create_new(db_magic, &args.db_path) { Ok(db_backend) => db_backend, // we cannot return `err` directly as it has lifetime `'m` Err(err) => return Err(anyhow::anyhow!("failed to init db backend: {:?}", err)), diff --git a/example-crates/wallet_electrum/src/main.rs b/example-crates/wallet_electrum/src/main.rs index da0c5e5dd..471caccf7 100644 --- a/example-crates/wallet_electrum/src/main.rs +++ b/example-crates/wallet_electrum/src/main.rs @@ -18,7 +18,7 @@ use bdk_file_store::Store; fn main() -> Result<(), Box> { let db_path = std::env::temp_dir().join("bdk-electrum-example"); - let mut db = Store::::new_from_path(DB_MAGIC.as_bytes(), db_path)?; + let mut db = Store::::open_or_create_new(DB_MAGIC.as_bytes(), db_path)?; let external_descriptor = "wpkh(tprv8ZgxMBicQKsPdy6LMhUtFHAgpocR8GC6QmwMSFpZs7h6Eziw3SpThFfczTDh5rW2krkqffa11UpX3XkeTTB2FvzZKWXqPY54Y6Rq4AQ5R8L/84'/1'/0'/0/*)"; let internal_descriptor = "wpkh(tprv8ZgxMBicQKsPdy6LMhUtFHAgpocR8GC6QmwMSFpZs7h6Eziw3SpThFfczTDh5rW2krkqffa11UpX3XkeTTB2FvzZKWXqPY54Y6Rq4AQ5R8L/84'/1'/0'/1/*)"; diff --git a/example-crates/wallet_esplora_async/src/main.rs b/example-crates/wallet_esplora_async/src/main.rs index e44db9d8d..72a9911d9 100644 --- a/example-crates/wallet_esplora_async/src/main.rs +++ b/example-crates/wallet_esplora_async/src/main.rs @@ -17,7 +17,7 @@ const PARALLEL_REQUESTS: usize = 5; #[tokio::main] async fn main() -> Result<(), Box> { let db_path = std::env::temp_dir().join("bdk-esplora-async-example"); - let mut db = Store::::new_from_path(DB_MAGIC.as_bytes(), db_path)?; + let mut db = Store::::open_or_create_new(DB_MAGIC.as_bytes(), db_path)?; let external_descriptor = "wpkh(tprv8ZgxMBicQKsPdy6LMhUtFHAgpocR8GC6QmwMSFpZs7h6Eziw3SpThFfczTDh5rW2krkqffa11UpX3XkeTTB2FvzZKWXqPY54Y6Rq4AQ5R8L/84'/1'/0'/0/*)"; let internal_descriptor = "wpkh(tprv8ZgxMBicQKsPdy6LMhUtFHAgpocR8GC6QmwMSFpZs7h6Eziw3SpThFfczTDh5rW2krkqffa11UpX3XkeTTB2FvzZKWXqPY54Y6Rq4AQ5R8L/84'/1'/0'/1/*)"; diff --git a/example-crates/wallet_esplora_blocking/src/main.rs b/example-crates/wallet_esplora_blocking/src/main.rs index ec1076597..aa434b639 100644 --- a/example-crates/wallet_esplora_blocking/src/main.rs +++ b/example-crates/wallet_esplora_blocking/src/main.rs @@ -16,7 +16,7 @@ use bdk_file_store::Store; fn main() -> Result<(), Box> { let db_path = std::env::temp_dir().join("bdk-esplora-example"); - let mut db = Store::::new_from_path(DB_MAGIC.as_bytes(), db_path)?; + let mut db = Store::::open_or_create_new(DB_MAGIC.as_bytes(), db_path)?; let external_descriptor = "wpkh(tprv8ZgxMBicQKsPdy6LMhUtFHAgpocR8GC6QmwMSFpZs7h6Eziw3SpThFfczTDh5rW2krkqffa11UpX3XkeTTB2FvzZKWXqPY54Y6Rq4AQ5R8L/84'/1'/0'/0/*)"; let internal_descriptor = "wpkh(tprv8ZgxMBicQKsPdy6LMhUtFHAgpocR8GC6QmwMSFpZs7h6Eziw3SpThFfczTDh5rW2krkqffa11UpX3XkeTTB2FvzZKWXqPY54Y6Rq4AQ5R8L/84'/1'/0'/1/*)"; From 1886dc4fe743408132c3257afc1cacbb4d964105 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Mon, 30 Oct 2023 11:38:20 +0800 Subject: [PATCH 07/13] chore(examples): use `Wallet::new_or_load` method where appropriate --- example-crates/wallet_electrum/src/main.rs | 18 +++++++----------- .../wallet_esplora_async/src/main.rs | 19 +++++++------------ .../wallet_esplora_blocking/src/main.rs | 19 +++++++------------ 3 files changed, 21 insertions(+), 35 deletions(-) diff --git a/example-crates/wallet_electrum/src/main.rs b/example-crates/wallet_electrum/src/main.rs index 471caccf7..9c77d5df0 100644 --- a/example-crates/wallet_electrum/src/main.rs +++ b/example-crates/wallet_electrum/src/main.rs @@ -18,20 +18,16 @@ use bdk_file_store::Store; fn main() -> Result<(), Box> { let db_path = std::env::temp_dir().join("bdk-electrum-example"); - let mut db = Store::::open_or_create_new(DB_MAGIC.as_bytes(), db_path)?; + let db = Store::::open_or_create_new(DB_MAGIC.as_bytes(), db_path)?; let external_descriptor = "wpkh(tprv8ZgxMBicQKsPdy6LMhUtFHAgpocR8GC6QmwMSFpZs7h6Eziw3SpThFfczTDh5rW2krkqffa11UpX3XkeTTB2FvzZKWXqPY54Y6Rq4AQ5R8L/84'/1'/0'/0/*)"; let internal_descriptor = "wpkh(tprv8ZgxMBicQKsPdy6LMhUtFHAgpocR8GC6QmwMSFpZs7h6Eziw3SpThFfczTDh5rW2krkqffa11UpX3XkeTTB2FvzZKWXqPY54Y6Rq4AQ5R8L/84'/1'/0'/1/*)"; - let mut wallet = if db.is_empty()? { - Wallet::new( - external_descriptor, - Some(internal_descriptor), - db, - Network::Testnet, - )? - } else { - Wallet::load(external_descriptor, Some(internal_descriptor), db)? - }; + let mut wallet = Wallet::new_or_load( + external_descriptor, + Some(internal_descriptor), + db, + Network::Testnet, + )?; let address = wallet.get_address(bdk::wallet::AddressIndex::New); println!("Generated Address: {}", address); diff --git a/example-crates/wallet_esplora_async/src/main.rs b/example-crates/wallet_esplora_async/src/main.rs index 72a9911d9..56c13b774 100644 --- a/example-crates/wallet_esplora_async/src/main.rs +++ b/example-crates/wallet_esplora_async/src/main.rs @@ -2,7 +2,6 @@ use std::{io::Write, str::FromStr}; use bdk::{ bitcoin::{Address, Network}, - chain::PersistBackend, wallet::{AddressIndex, Update}, SignOptions, Wallet, }; @@ -17,20 +16,16 @@ const PARALLEL_REQUESTS: usize = 5; #[tokio::main] async fn main() -> Result<(), Box> { let db_path = std::env::temp_dir().join("bdk-esplora-async-example"); - let mut db = Store::::open_or_create_new(DB_MAGIC.as_bytes(), db_path)?; + let db = Store::::open_or_create_new(DB_MAGIC.as_bytes(), db_path)?; let external_descriptor = "wpkh(tprv8ZgxMBicQKsPdy6LMhUtFHAgpocR8GC6QmwMSFpZs7h6Eziw3SpThFfczTDh5rW2krkqffa11UpX3XkeTTB2FvzZKWXqPY54Y6Rq4AQ5R8L/84'/1'/0'/0/*)"; let internal_descriptor = "wpkh(tprv8ZgxMBicQKsPdy6LMhUtFHAgpocR8GC6QmwMSFpZs7h6Eziw3SpThFfczTDh5rW2krkqffa11UpX3XkeTTB2FvzZKWXqPY54Y6Rq4AQ5R8L/84'/1'/0'/1/*)"; - let mut wallet = if db.is_empty()? { - Wallet::new( - external_descriptor, - Some(internal_descriptor), - db, - Network::Testnet, - )? - } else { - Wallet::load(external_descriptor, Some(internal_descriptor), db)? - }; + let mut wallet = Wallet::new_or_load( + external_descriptor, + Some(internal_descriptor), + db, + Network::Testnet, + )?; let address = wallet.get_address(AddressIndex::New); println!("Generated Address: {}", address); diff --git a/example-crates/wallet_esplora_blocking/src/main.rs b/example-crates/wallet_esplora_blocking/src/main.rs index aa434b639..e6173baef 100644 --- a/example-crates/wallet_esplora_blocking/src/main.rs +++ b/example-crates/wallet_esplora_blocking/src/main.rs @@ -7,7 +7,6 @@ use std::{io::Write, str::FromStr}; use bdk::{ bitcoin::{Address, Network}, - chain::PersistBackend, wallet::{AddressIndex, Update}, SignOptions, Wallet, }; @@ -16,20 +15,16 @@ use bdk_file_store::Store; fn main() -> Result<(), Box> { let db_path = std::env::temp_dir().join("bdk-esplora-example"); - let mut db = Store::::open_or_create_new(DB_MAGIC.as_bytes(), db_path)?; + let db = Store::::open_or_create_new(DB_MAGIC.as_bytes(), db_path)?; let external_descriptor = "wpkh(tprv8ZgxMBicQKsPdy6LMhUtFHAgpocR8GC6QmwMSFpZs7h6Eziw3SpThFfczTDh5rW2krkqffa11UpX3XkeTTB2FvzZKWXqPY54Y6Rq4AQ5R8L/84'/1'/0'/0/*)"; let internal_descriptor = "wpkh(tprv8ZgxMBicQKsPdy6LMhUtFHAgpocR8GC6QmwMSFpZs7h6Eziw3SpThFfczTDh5rW2krkqffa11UpX3XkeTTB2FvzZKWXqPY54Y6Rq4AQ5R8L/84'/1'/0'/1/*)"; - let mut wallet = if db.is_empty()? { - Wallet::new( - external_descriptor, - Some(internal_descriptor), - db, - Network::Testnet, - )? - } else { - Wallet::load(external_descriptor, Some(internal_descriptor), db)? - }; + let mut wallet = Wallet::new_or_load( + external_descriptor, + Some(internal_descriptor), + db, + Network::Testnet, + )?; let address = wallet.get_address(AddressIndex::New); println!("Generated Address: {}", address); From 96f1d94e2c8378d820a59e5ceafe686477243bf8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Mon, 30 Oct 2023 18:02:44 +0800 Subject: [PATCH 08/13] test(file_store): add construction method tests --- crates/file_store/src/store.rs | 38 ++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/crates/file_store/src/store.rs b/crates/file_store/src/store.rs index 22e8b39d6..8af10cbd0 100644 --- a/crates/file_store/src/store.rs +++ b/crates/file_store/src/store.rs @@ -216,6 +216,44 @@ mod test { #[derive(Debug)] struct TestTracker; + /// Check behavior of [`Store::create_new`] and [`Store::open`]. + #[test] + fn construct_store() { + let temp_dir = tempfile::tempdir().unwrap(); + let file_path = temp_dir.path().join("db_file"); + let _ = Store::::open(&TEST_MAGIC_BYTES, &file_path) + .expect_err("must not open as file does not exist yet"); + let _ = Store::::create_new(&TEST_MAGIC_BYTES, &file_path) + .expect("must create file"); + // cannot create new as file already exists + let _ = Store::::create_new(&TEST_MAGIC_BYTES, &file_path) + .expect_err("must fail as file already exists now"); + let _ = Store::::open(&TEST_MAGIC_BYTES, &file_path) + .expect("must open as file exists now"); + } + + #[test] + fn open_or_create_new() { + let temp_dir = tempfile::tempdir().unwrap(); + let file_path = temp_dir.path().join("db_file"); + let changeset = vec!["hello".to_string(), "world".to_string()]; + + { + let mut db = Store::::open_or_create_new(&TEST_MAGIC_BYTES, &file_path) + .expect("must create"); + assert!(file_path.exists()); + db.append_changeset(&changeset).expect("must succeed"); + } + + { + let mut db = Store::::open_or_create_new(&TEST_MAGIC_BYTES, &file_path) + .expect("must recover"); + let (recovered_changeset, r) = db.aggregate_changesets(); + r.expect("must succeed"); + assert_eq!(recovered_changeset, changeset); + } + } + #[test] fn is_empty() { let mut file = NamedTempFile::new().unwrap(); From c3265e2514070bd4da92ca343fe884e13e831360 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Tue, 31 Oct 2023 06:46:27 +0800 Subject: [PATCH 09/13] test(bdk): add tests for wallet constructor methods --- crates/bdk/Cargo.toml | 2 + crates/bdk/tests/wallet.rs | 98 +++++++++++++++++++++++++++++++++++++- 2 files changed, 99 insertions(+), 1 deletion(-) diff --git a/crates/bdk/Cargo.toml b/crates/bdk/Cargo.toml index 354a7d1e0..8c519d891 100644 --- a/crates/bdk/Cargo.toml +++ b/crates/bdk/Cargo.toml @@ -47,6 +47,8 @@ dev-getrandom-wasm = ["getrandom/js"] lazy_static = "1.4" env_logger = "0.7" assert_matches = "1.5.0" +tempfile = "3" +bdk_file_store = { path = "../file_store" } [package.metadata.docs.rs] all-features = true diff --git a/crates/bdk/tests/wallet.rs b/crates/bdk/tests/wallet.rs index 3aab7016d..27dc957a9 100644 --- a/crates/bdk/tests/wallet.rs +++ b/crates/bdk/tests/wallet.rs @@ -1,3 +1,5 @@ +use std::str::FromStr; + use assert_matches::assert_matches; use bdk::descriptor::calc_checksum; use bdk::psbt::PsbtUtils; @@ -17,7 +19,6 @@ use bitcoin::{ }; use bitcoin::{psbt, Network}; use bitcoin::{BlockHash, Txid}; -use core::str::FromStr; mod common; use common::*; @@ -60,6 +61,101 @@ fn receive_output_in_latest_block(wallet: &mut Wallet, value: u64) -> OutPoint { // OP_PUSH. const P2WPKH_FAKE_WITNESS_SIZE: usize = 106; +const DB_MAGIC: &[u8] = &[0x21, 0x24, 0x48]; + +#[test] +fn load_recovers_wallet() { + let temp_dir = tempfile::tempdir().expect("must create tempdir"); + let file_path = temp_dir.path().join("store.db"); + + // create new wallet + let wallet_keychains = { + let db = bdk_file_store::Store::create_new(DB_MAGIC, &file_path).expect("must create db"); + let wallet = + Wallet::new(get_test_wpkh(), None, db, Network::Testnet).expect("must init wallet"); + wallet.keychains().clone() + }; + + // recover wallet + { + let db = bdk_file_store::Store::open(DB_MAGIC, &file_path).expect("must recover db"); + let wallet = Wallet::load(get_test_wpkh(), None, db).expect("must recover wallet"); + assert_eq!(wallet.network(), Network::Testnet); + assert_eq!(wallet.spk_index().keychains(), &wallet_keychains); + } +} + +#[test] +fn new_or_load() { + let temp_dir = tempfile::tempdir().expect("must create tempdir"); + let file_path = temp_dir.path().join("store.db"); + + // init wallet when non-existant + let wallet_keychains = { + let db = bdk_file_store::Store::open_or_create_new(DB_MAGIC, &file_path) + .expect("must create db"); + let wallet = Wallet::new_or_load(get_test_wpkh(), None, db, Network::Testnet) + .expect("must init wallet"); + wallet.keychains().clone() + }; + + // wrong network + { + let db = + bdk_file_store::Store::open_or_create_new(DB_MAGIC, &file_path).expect("must open db"); + let err = Wallet::new_or_load(get_test_wpkh(), None, db, Network::Bitcoin) + .expect_err("wrong network"); + assert!( + matches!( + err, + bdk::wallet::NewOrLoadError::LoadedNetworkDoesNotMatch { + got: Some(Network::Testnet), + expected: Network::Bitcoin + } + ), + "err: {}", + err, + ); + } + + // wrong genesis hash + { + let exp_blockhash = BlockHash::all_zeros(); + let got_blockhash = + bitcoin::blockdata::constants::genesis_block(Network::Testnet).block_hash(); + + let db = + bdk_file_store::Store::open_or_create_new(DB_MAGIC, &file_path).expect("must open db"); + let err = Wallet::new_or_load_with_genesis_hash( + get_test_wpkh(), + None, + db, + Network::Testnet, + exp_blockhash, + ) + .expect_err("wrong genesis hash"); + assert!( + matches!( + err, + bdk::wallet::NewOrLoadError::LoadedGenesisDoesNotMatch { got, expected } + if got == Some(got_blockhash) && expected == exp_blockhash + ), + "err: {}", + err, + ); + } + + // all parameters match + { + let db = + bdk_file_store::Store::open_or_create_new(DB_MAGIC, &file_path).expect("must open db"); + let wallet = Wallet::new_or_load(get_test_wpkh(), None, db, Network::Testnet) + .expect("must recover wallet"); + assert_eq!(wallet.network(), Network::Testnet); + assert_eq!(wallet.keychains(), &wallet_keychains); + } +} + #[test] fn test_descriptor_checksum() { let (wallet, _) = get_funded_wallet(get_test_wpkh()); From 06a956ad20ca5f1dcd108e45a1a1fed924128cdb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Wed, 1 Nov 2023 09:21:24 +0800 Subject: [PATCH 10/13] feat!: change `load_from_persistence` to return an option `PersistBackend::is_empty` is removed. Instead, `load_from_persistence` returns an option of the changeset. `None` means persistence is empty. This is a better API than a separate method. We can now differentiate between a persisted single changeset and nothing persisted. `Store::aggregate_changeset` is refactored to return a `Result` instead of a tuple. A new error type (`AggregateChangesetsError`) is introduced to include the partially-aggregated changeset in the error. This is a more idiomatic API. --- crates/bdk/src/wallet/mod.rs | 94 ++++++++++++++++++--------- crates/chain/src/persist.rs | 21 ++---- crates/file_store/src/store.rs | 79 +++++++++++----------- example-crates/example_cli/src/lib.rs | 2 +- 4 files changed, 110 insertions(+), 86 deletions(-) diff --git a/crates/bdk/src/wallet/mod.rs b/crates/bdk/src/wallet/mod.rs index ea76ad656..c9a1c28cb 100644 --- a/crates/bdk/src/wallet/mod.rs +++ b/crates/bdk/src/wallet/mod.rs @@ -293,6 +293,8 @@ pub enum LoadError { Descriptor(crate::descriptor::DescriptorError), /// Loading data from the persistence backend failed. Load(L), + /// Wallet not initialized, persistence backend is empty. + NotInitialized, /// Data loaded from persistence is missing network type. MissingNetwork, /// Data loaded from persistence is missing genesis hash. @@ -307,6 +309,9 @@ where match self { LoadError::Descriptor(e) => e.fmt(f), LoadError::Load(e) => e.fmt(f), + LoadError::NotInitialized => { + write!(f, "wallet is not initialized, persistence backend is empty") + } LoadError::MissingNetwork => write!(f, "loaded data is missing network type"), LoadError::MissingGenesis => write!(f, "loaded data is missing genesis hash"), } @@ -330,6 +335,8 @@ pub enum NewOrLoadError { Write(W), /// Loading from the persistence backend failed. Load(L), + /// Wallet is not initialized, persistence backend is empty. + NotInitialized, /// The loaded genesis hash does not match what was provided. LoadedGenesisDoesNotMatch { /// The expected genesis block hash. @@ -356,6 +363,9 @@ where NewOrLoadError::Descriptor(e) => e.fmt(f), NewOrLoadError::Write(e) => write!(f, "failed to write to persistence: {}", e), NewOrLoadError::Load(e) => write!(f, "failed to load from persistence: {}", e), + NewOrLoadError::NotInitialized => { + write!(f, "wallet is not initialized, persistence backend is empty") + } NewOrLoadError::LoadedGenesisDoesNotMatch { expected, got } => { write!(f, "loaded genesis hash is not {}, got {:?}", expected, got) } @@ -451,11 +461,26 @@ impl Wallet { change_descriptor: Option, mut db: D, ) -> Result> + where + D: PersistBackend, + { + let changeset = db + .load_from_persistence() + .map_err(LoadError::Load)? + .ok_or(LoadError::NotInitialized)?; + Self::load_from_changeset(descriptor, change_descriptor, db, changeset) + } + + fn load_from_changeset( + descriptor: E, + change_descriptor: Option, + db: D, + changeset: ChangeSet, + ) -> Result> where D: PersistBackend, { let secp = Secp256k1::new(); - let changeset = db.load_from_persistence().map_err(LoadError::Load)?; let network = changeset.network.ok_or(LoadError::MissingNetwork)?; let chain = LocalChain::from_changeset(changeset.chain).map_err(|_| LoadError::MissingGenesis)?; @@ -517,8 +542,43 @@ impl Wallet { where D: PersistBackend, { - if db.is_empty().map_err(NewOrLoadError::Load)? { - return Self::new_with_genesis_hash( + let changeset = db.load_from_persistence().map_err(NewOrLoadError::Load)?; + match changeset { + Some(changeset) => { + let wallet = + Self::load_from_changeset(descriptor, change_descriptor, db, changeset) + .map_err(|e| match e { + LoadError::Descriptor(e) => NewOrLoadError::Descriptor(e), + LoadError::Load(e) => NewOrLoadError::Load(e), + LoadError::NotInitialized => NewOrLoadError::NotInitialized, + LoadError::MissingNetwork => { + NewOrLoadError::LoadedNetworkDoesNotMatch { + expected: network, + got: None, + } + } + LoadError::MissingGenesis => { + NewOrLoadError::LoadedGenesisDoesNotMatch { + expected: genesis_hash, + got: None, + } + } + })?; + if wallet.network != network { + return Err(NewOrLoadError::LoadedNetworkDoesNotMatch { + expected: network, + got: Some(wallet.network), + }); + } + if wallet.chain.genesis_hash() != genesis_hash { + return Err(NewOrLoadError::LoadedGenesisDoesNotMatch { + expected: genesis_hash, + got: Some(wallet.chain.genesis_hash()), + }); + } + Ok(wallet) + } + None => Self::new_with_genesis_hash( descriptor, change_descriptor, db, @@ -528,34 +588,8 @@ impl Wallet { .map_err(|e| match e { NewError::Descriptor(e) => NewOrLoadError::Descriptor(e), NewError::Write(e) => NewOrLoadError::Write(e), - }); - } - - let wallet = Self::load(descriptor, change_descriptor, db).map_err(|e| match e { - LoadError::Descriptor(e) => NewOrLoadError::Descriptor(e), - LoadError::Load(e) => NewOrLoadError::Load(e), - LoadError::MissingNetwork => NewOrLoadError::LoadedNetworkDoesNotMatch { - expected: network, - got: None, - }, - LoadError::MissingGenesis => NewOrLoadError::LoadedGenesisDoesNotMatch { - expected: genesis_hash, - got: None, - }, - })?; - if wallet.network != network { - return Err(NewOrLoadError::LoadedNetworkDoesNotMatch { - expected: network, - got: Some(wallet.network), - }); - } - if wallet.chain.genesis_hash() != genesis_hash { - return Err(NewOrLoadError::LoadedGenesisDoesNotMatch { - expected: genesis_hash, - got: Some(wallet.chain.genesis_hash()), - }); + }), } - Ok(wallet) } /// Get the Bitcoin network the wallet is using. diff --git a/crates/chain/src/persist.rs b/crates/chain/src/persist.rs index 634e369e9..3c8c8b9e1 100644 --- a/crates/chain/src/persist.rs +++ b/crates/chain/src/persist.rs @@ -79,19 +79,10 @@ pub trait PersistBackend { fn write_changes(&mut self, changeset: &C) -> Result<(), Self::WriteError>; /// Return the aggregate changeset `C` from persistence. - fn load_from_persistence(&mut self) -> Result; - - /// Returns whether the persistence backend contains no data. - fn is_empty(&mut self) -> Result - where - C: Append, - { - self.load_from_persistence() - .map(|changeset| changeset.is_empty()) - } + fn load_from_persistence(&mut self) -> Result, Self::LoadError>; } -impl PersistBackend for () { +impl PersistBackend for () { type WriteError = Infallible; type LoadError = Infallible; @@ -100,11 +91,7 @@ impl PersistBackend for () { Ok(()) } - fn load_from_persistence(&mut self) -> Result { - Ok(C::default()) - } - - fn is_empty(&mut self) -> Result { - Ok(true) + fn load_from_persistence(&mut self) -> Result, Self::LoadError> { + Ok(None) } } diff --git a/crates/file_store/src/store.rs b/crates/file_store/src/store.rs index 8af10cbd0..bf88b8d34 100644 --- a/crates/file_store/src/store.rs +++ b/crates/file_store/src/store.rs @@ -23,7 +23,7 @@ pub struct Store<'a, C> { impl<'a, C> PersistBackend for Store<'a, C> where - C: Default + Append + serde::Serialize + serde::de::DeserializeOwned, + C: Append + serde::Serialize + serde::de::DeserializeOwned, { type WriteError = std::io::Error; @@ -33,23 +33,14 @@ where self.append_changeset(changeset) } - fn load_from_persistence(&mut self) -> Result { - let (changeset, result) = self.aggregate_changesets(); - result.map(|_| changeset) - } - - fn is_empty(&mut self) -> Result { - let init_pos = self.db_file.stream_position()?; - let stream_len = self.db_file.seek(io::SeekFrom::End(0))?; - let magic_len = self.magic.len() as u64; - self.db_file.seek(io::SeekFrom::Start(init_pos))?; - Ok(stream_len == magic_len) + fn load_from_persistence(&mut self) -> Result, Self::LoadError> { + self.aggregate_changesets().map_err(|e| e.iter_error) } } impl<'a, C> Store<'a, C> where - C: Default + Append + serde::Serialize + serde::de::DeserializeOwned, + C: Append + serde::Serialize + serde::de::DeserializeOwned, { /// Create a new [`Store`] file in write-only mode; error if the file exists. /// @@ -156,16 +147,24 @@ where /// /// **WARNING**: This method changes the write position of the underlying file. The next /// changeset will be written over the erroring entry (or the end of the file if none existed). - pub fn aggregate_changesets(&mut self) -> (C, Result<(), IterError>) { - let mut changeset = C::default(); - let result = (|| { - for next_changeset in self.iter_changesets() { - changeset.append(next_changeset?); + pub fn aggregate_changesets(&mut self) -> Result, AggregateChangesetsError> { + let mut changeset = Option::::None; + for next_changeset in self.iter_changesets() { + let next_changeset = match next_changeset { + Ok(next_changeset) => next_changeset, + Err(iter_error) => { + return Err(AggregateChangesetsError { + changeset, + iter_error, + }) + } + }; + match &mut changeset { + Some(changeset) => changeset.append(next_changeset), + changeset => *changeset = Some(next_changeset), } - Ok(()) - })(); - - (changeset, result) + } + Ok(changeset) } /// Append a new changeset to the file and truncate the file to the end of the appended @@ -196,6 +195,24 @@ where } } +/// Error type for [`Store::aggregate_changesets`]. +#[derive(Debug)] +pub struct AggregateChangesetsError { + /// The partially-aggregated changeset. + pub changeset: Option, + + /// The error returned by [`EntryIter`]. + pub iter_error: IterError, +} + +impl std::fmt::Display for AggregateChangesetsError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + std::fmt::Display::fmt(&self.iter_error, f) + } +} + +impl std::error::Error for AggregateChangesetsError {} + #[cfg(test)] mod test { use super::*; @@ -248,25 +265,11 @@ mod test { { let mut db = Store::::open_or_create_new(&TEST_MAGIC_BYTES, &file_path) .expect("must recover"); - let (recovered_changeset, r) = db.aggregate_changesets(); - r.expect("must succeed"); - assert_eq!(recovered_changeset, changeset); + let recovered_changeset = db.aggregate_changesets().expect("must succeed"); + assert_eq!(recovered_changeset, Some(changeset)); } } - #[test] - fn is_empty() { - let mut file = NamedTempFile::new().unwrap(); - file.write_all(&TEST_MAGIC_BYTES).expect("should write"); - - let mut db = - Store::::open(&TEST_MAGIC_BYTES, file.path()).expect("must open"); - assert!(db.is_empty().expect("must read")); - db.write_changes(&vec!["hello".to_string(), "world".to_string()]) - .expect("must write"); - assert!(!db.is_empty().expect("must read")); - } - #[test] fn new_fails_if_file_is_too_short() { let mut file = NamedTempFile::new().unwrap(); diff --git a/example-crates/example_cli/src/lib.rs b/example-crates/example_cli/src/lib.rs index 0b5d9cd37..f9574c0e0 100644 --- a/example-crates/example_cli/src/lib.rs +++ b/example-crates/example_cli/src/lib.rs @@ -687,7 +687,7 @@ where Err(err) => return Err(anyhow::anyhow!("failed to init db backend: {:?}", err)), }; - let init_changeset = db_backend.load_from_persistence()?; + let init_changeset = db_backend.load_from_persistence()?.unwrap_or_default(); Ok(( args, From 79b84bed0ec399a375490c2c0f16c6ea3f5969b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Mon, 6 Nov 2023 11:52:03 +0800 Subject: [PATCH 11/13] feat(bdk): changeset's `Append` impl checks that network is consistent --- crates/bdk/src/wallet/mod.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/crates/bdk/src/wallet/mod.rs b/crates/bdk/src/wallet/mod.rs index c9a1c28cb..e396b8d4f 100644 --- a/crates/bdk/src/wallet/mod.rs +++ b/crates/bdk/src/wallet/mod.rs @@ -138,6 +138,10 @@ impl Append for ChangeSet { Append::append(&mut self.chain, other.chain); Append::append(&mut self.indexed_tx_graph, other.indexed_tx_graph); if other.network.is_some() { + debug_assert!( + self.network.is_none() || self.network == other.network, + "network type must be consistent" + ); self.network = other.network; } } From 9a250baf6203a077fcfc03c3f6b386d5fba03d60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Thu, 16 Nov 2023 07:17:16 +0800 Subject: [PATCH 12/13] chore: make clippy happy --- crates/bdk/src/wallet/coin_selection.rs | 20 +++++++++--------- crates/bdk/tests/wallet.rs | 2 +- .../chain/tests/test_keychain_txout_index.rs | 2 +- crates/chain/tests/test_tx_graph.rs | 21 ++++++++----------- 4 files changed, 21 insertions(+), 24 deletions(-) diff --git a/crates/bdk/src/wallet/coin_selection.rs b/crates/bdk/src/wallet/coin_selection.rs index c3e84af2b..a0179d31b 100644 --- a/crates/bdk/src/wallet/coin_selection.rs +++ b/crates/bdk/src/wallet/coin_selection.rs @@ -836,7 +836,7 @@ mod test { let drain_script = ScriptBuf::default(); let target_amount = 250_000 + FEE_AMOUNT; - let result = LargestFirstCoinSelection::default() + let result = LargestFirstCoinSelection .coin_select( utxos, vec![], @@ -857,7 +857,7 @@ mod test { let drain_script = ScriptBuf::default(); let target_amount = 20_000 + FEE_AMOUNT; - let result = LargestFirstCoinSelection::default() + let result = LargestFirstCoinSelection .coin_select( utxos, vec![], @@ -878,7 +878,7 @@ mod test { let drain_script = ScriptBuf::default(); let target_amount = 20_000 + FEE_AMOUNT; - let result = LargestFirstCoinSelection::default() + let result = LargestFirstCoinSelection .coin_select( vec![], utxos, @@ -900,7 +900,7 @@ mod test { let drain_script = ScriptBuf::default(); let target_amount = 500_000 + FEE_AMOUNT; - LargestFirstCoinSelection::default() + LargestFirstCoinSelection .coin_select( vec![], utxos, @@ -918,7 +918,7 @@ mod test { let drain_script = ScriptBuf::default(); let target_amount = 250_000 + FEE_AMOUNT; - LargestFirstCoinSelection::default() + LargestFirstCoinSelection .coin_select( vec![], utxos, @@ -935,7 +935,7 @@ mod test { let drain_script = ScriptBuf::default(); let target_amount = 180_000 + FEE_AMOUNT; - let result = OldestFirstCoinSelection::default() + let result = OldestFirstCoinSelection .coin_select( vec![], utxos, @@ -956,7 +956,7 @@ mod test { let drain_script = ScriptBuf::default(); let target_amount = 20_000 + FEE_AMOUNT; - let result = OldestFirstCoinSelection::default() + let result = OldestFirstCoinSelection .coin_select( utxos, vec![], @@ -977,7 +977,7 @@ mod test { let drain_script = ScriptBuf::default(); let target_amount = 20_000 + FEE_AMOUNT; - let result = OldestFirstCoinSelection::default() + let result = OldestFirstCoinSelection .coin_select( vec![], utxos, @@ -999,7 +999,7 @@ mod test { let drain_script = ScriptBuf::default(); let target_amount = 600_000 + FEE_AMOUNT; - OldestFirstCoinSelection::default() + OldestFirstCoinSelection .coin_select( vec![], utxos, @@ -1018,7 +1018,7 @@ mod test { let target_amount: u64 = utxos.iter().map(|wu| wu.utxo.txout().value).sum::() - 50; let drain_script = ScriptBuf::default(); - OldestFirstCoinSelection::default() + OldestFirstCoinSelection .coin_select( vec![], utxos, diff --git a/crates/bdk/tests/wallet.rs b/crates/bdk/tests/wallet.rs index 27dc957a9..7161e58d1 100644 --- a/crates/bdk/tests/wallet.rs +++ b/crates/bdk/tests/wallet.rs @@ -3181,7 +3181,7 @@ fn test_taproot_script_spend_sign_exclude_some_leaves() { .values() .map(|(script, version)| TapLeafHash::from_script(script, *version)) .collect(); - let included_script_leaves = vec![script_leaves.pop().unwrap()]; + let included_script_leaves = [script_leaves.pop().unwrap()]; let excluded_script_leaves = script_leaves; assert!( diff --git a/crates/chain/tests/test_keychain_txout_index.rs b/crates/chain/tests/test_keychain_txout_index.rs index c4f434715..161aad06d 100644 --- a/crates/chain/tests/test_keychain_txout_index.rs +++ b/crates/chain/tests/test_keychain_txout_index.rs @@ -312,7 +312,7 @@ fn test_wildcard_derivations() { let _ = txout_index.reveal_to_target(&TestKeychain::External, 25); (0..=15) - .chain(vec![17, 20, 23].into_iter()) + .chain([17, 20, 23]) .for_each(|index| assert!(txout_index.mark_used(&TestKeychain::External, index))); assert_eq!(txout_index.next_index(&TestKeychain::External), (26, true)); diff --git a/crates/chain/tests/test_tx_graph.rs b/crates/chain/tests/test_tx_graph.rs index b5311ea0b..224e263b7 100644 --- a/crates/chain/tests/test_tx_graph.rs +++ b/crates/chain/tests/test_tx_graph.rs @@ -910,18 +910,15 @@ fn test_chain_spends() { let _ = graph.insert_tx(tx_1.clone()); let _ = graph.insert_tx(tx_2.clone()); - [95, 98] - .iter() - .zip([&tx_0, &tx_1].into_iter()) - .for_each(|(ht, tx)| { - let _ = graph.insert_anchor( - tx.txid(), - ConfirmationHeightAnchor { - anchor_block: tip.block_id(), - confirmation_height: *ht, - }, - ); - }); + for (ht, tx) in [(95, &tx_0), (98, &tx_1)] { + let _ = graph.insert_anchor( + tx.txid(), + ConfirmationHeightAnchor { + anchor_block: tip.block_id(), + confirmation_height: ht, + }, + ); + } // Assert that confirmed spends are returned correctly. assert_eq!( From f1b112e8f9563c2afb9097911ee5e1afbbc55d22 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Thu, 16 Nov 2023 07:23:56 +0800 Subject: [PATCH 13/13] docs(bitcoind_rpc): update docs for `Emitter::new` --- crates/bitcoind_rpc/src/lib.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/bitcoind_rpc/src/lib.rs b/crates/bitcoind_rpc/src/lib.rs index 07f6ea3d4..0fafbc767 100644 --- a/crates/bitcoind_rpc/src/lib.rs +++ b/crates/bitcoind_rpc/src/lib.rs @@ -43,7 +43,11 @@ pub struct Emitter<'c, C> { } impl<'c, C: bitcoincore_rpc::RpcApi> Emitter<'c, C> { - /// TODO + /// Construct a new [`Emitter`] with the given RPC `client`, `last_cp` and `start_height`. + /// + /// * `last_cp` is the check point used to find the latest block which is still part of the best + /// chain. + /// * `start_height` is the block height to start emitting blocks from. pub fn new(client: &'c C, last_cp: CheckPoint, start_height: u32) -> Self { Self { client,