From c0e75fc1a8560cdacf705ff60ef8833befd8280c Mon Sep 17 00:00:00 2001 From: LLFourn Date: Wed, 23 Feb 2022 10:38:35 +1100 Subject: [PATCH] Split get_tx into its own trait to make supporting verify_tx easier --- src/blockchain/any.rs | 10 +++++--- src/blockchain/compact_filters/mod.rs | 16 ++++++------ src/blockchain/electrum.rs | 10 +++++--- src/blockchain/esplora/reqwest.rs | 11 ++++++--- src/blockchain/esplora/ureq.rs | 10 +++++--- src/blockchain/mod.rs | 21 +++++++++++----- src/blockchain/rpc.rs | 14 +++++------ src/testutils/blockchain_tests.rs | 6 ++--- src/wallet/mod.rs | 1 - src/wallet/verify.rs | 35 ++++----------------------- 10 files changed, 65 insertions(+), 69 deletions(-) diff --git a/src/blockchain/any.rs b/src/blockchain/any.rs index ef4abffa..079524ae 100644 --- a/src/blockchain/any.rs +++ b/src/blockchain/any.rs @@ -89,9 +89,6 @@ impl Blockchain for AnyBlockchain { maybe_await!(impl_inner_method!(self, get_capabilities)) } - fn get_tx(&self, txid: &Txid) -> Result, Error> { - maybe_await!(impl_inner_method!(self, get_tx, txid)) - } fn broadcast(&self, tx: &Transaction) -> Result<(), Error> { maybe_await!(impl_inner_method!(self, broadcast, tx)) } @@ -108,6 +105,13 @@ impl GetHeight for AnyBlockchain { } } +#[maybe_async] +impl GetTx for AnyBlockchain { + fn get_tx(&self, txid: &Txid) -> Result, Error> { + maybe_await!(impl_inner_method!(self, get_tx, txid)) + } +} + #[maybe_async] impl WalletSync for AnyBlockchain { fn wallet_sync( diff --git a/src/blockchain/compact_filters/mod.rs b/src/blockchain/compact_filters/mod.rs index d0b96421..e7e235b5 100644 --- a/src/blockchain/compact_filters/mod.rs +++ b/src/blockchain/compact_filters/mod.rs @@ -67,7 +67,7 @@ mod peer; mod store; mod sync; -use super::{Blockchain, Capability, ConfigurableBlockchain, GetHeight, Progress, WalletSync}; +use crate::blockchain::*; use crate::database::{BatchDatabase, BatchOperations, DatabaseUtils}; use crate::error::Error; use crate::types::{KeychainKind, LocalUtxo, TransactionDetails}; @@ -225,12 +225,6 @@ impl Blockchain for CompactFiltersBlockchain { vec![Capability::FullHistory].into_iter().collect() } - fn get_tx(&self, txid: &Txid) -> Result, Error> { - Ok(self.peers[0] - .get_mempool() - .get_tx(&Inventory::Transaction(*txid))) - } - fn broadcast(&self, tx: &Transaction) -> Result<(), Error> { self.peers[0].broadcast_tx(tx.clone())?; @@ -249,6 +243,14 @@ impl GetHeight for CompactFiltersBlockchain { } } +impl GetTx for CompactFiltersBlockchain { + fn get_tx(&self, txid: &Txid) -> Result, Error> { + Ok(self.peers[0] + .get_mempool() + .get_tx(&Inventory::Transaction(*txid))) + } +} + impl WalletSync for CompactFiltersBlockchain { #[allow(clippy::mutex_atomic)] // Mutex is easier to understand than a CAS loop. fn wallet_setup( diff --git a/src/blockchain/electrum.rs b/src/blockchain/electrum.rs index b10b90d5..0b8691bc 100644 --- a/src/blockchain/electrum.rs +++ b/src/blockchain/electrum.rs @@ -68,10 +68,6 @@ impl Blockchain for ElectrumBlockchain { .collect() } - fn get_tx(&self, txid: &Txid) -> Result, Error> { - Ok(self.client.transaction_get(txid).map(Option::Some)?) - } - fn broadcast(&self, tx: &Transaction) -> Result<(), Error> { Ok(self.client.transaction_broadcast(tx).map(|_| ())?) } @@ -94,6 +90,12 @@ impl GetHeight for ElectrumBlockchain { } } +impl GetTx for ElectrumBlockchain { + fn get_tx(&self, txid: &Txid) -> Result, Error> { + Ok(self.client.transaction_get(txid).map(Option::Some)?) + } +} + impl WalletSync for ElectrumBlockchain { fn wallet_setup( &self, diff --git a/src/blockchain/esplora/reqwest.rs b/src/blockchain/esplora/reqwest.rs index 0a9ed4dd..2141b8e6 100644 --- a/src/blockchain/esplora/reqwest.rs +++ b/src/blockchain/esplora/reqwest.rs @@ -91,10 +91,6 @@ impl Blockchain for EsploraBlockchain { .collect() } - fn get_tx(&self, txid: &Txid) -> Result, Error> { - Ok(await_or_block!(self.url_client._get_tx(txid))?) - } - fn broadcast(&self, tx: &Transaction) -> Result<(), Error> { Ok(await_or_block!(self.url_client._broadcast(tx))?) } @@ -112,6 +108,13 @@ impl GetHeight for EsploraBlockchain { } } +#[maybe_async] +impl GetTx for EsploraBlockchain { + fn get_tx(&self, txid: &Txid) -> Result, Error> { + Ok(await_or_block!(self.url_client._get_tx(txid))?) + } +} + #[maybe_async] impl WalletSync for EsploraBlockchain { fn wallet_setup( diff --git a/src/blockchain/esplora/ureq.rs b/src/blockchain/esplora/ureq.rs index 3e240f43..9bfa378f 100644 --- a/src/blockchain/esplora/ureq.rs +++ b/src/blockchain/esplora/ureq.rs @@ -87,10 +87,6 @@ impl Blockchain for EsploraBlockchain { .collect() } - fn get_tx(&self, txid: &Txid) -> Result, Error> { - Ok(self.url_client._get_tx(txid)?) - } - fn broadcast(&self, tx: &Transaction) -> Result<(), Error> { let _txid = self.url_client._broadcast(tx)?; Ok(()) @@ -108,6 +104,12 @@ impl GetHeight for EsploraBlockchain { } } +impl GetTx for EsploraBlockchain { + fn get_tx(&self, txid: &Txid) -> Result, Error> { + Ok(self.url_client._get_tx(txid)?) + } +} + impl WalletSync for EsploraBlockchain { fn wallet_setup( &self, diff --git a/src/blockchain/mod.rs b/src/blockchain/mod.rs index 5962fca2..714fdf6a 100644 --- a/src/blockchain/mod.rs +++ b/src/blockchain/mod.rs @@ -86,11 +86,9 @@ pub enum Capability { /// Trait that defines the actions that must be supported by a blockchain backend #[maybe_async] -pub trait Blockchain: WalletSync + GetHeight { +pub trait Blockchain: WalletSync + GetHeight + GetTx { /// Return the set of [`Capability`] supported by this backend fn get_capabilities(&self) -> HashSet; - /// Fetch a transaction from the blockchain given its txid - fn get_tx(&self, txid: &Txid) -> Result, Error>; /// Broadcast a transaction fn broadcast(&self, tx: &Transaction) -> Result<(), Error>; /// Estimate the fee rate required to confirm a transaction in a given `target` of blocks @@ -104,6 +102,13 @@ pub trait GetHeight { fn get_height(&self) -> Result; } +#[maybe_async] +/// Trait for getting a transaction by txid +pub trait GetTx { + /// Fetch a transaction given its txid + fn get_tx(&self, txid: &Txid) -> Result, Error>; +} + /// Trait for blockchains that can sync by updating the database directly. #[maybe_async] pub trait WalletSync { @@ -230,9 +235,6 @@ impl Blockchain for Arc { maybe_await!(self.deref().get_capabilities()) } - fn get_tx(&self, txid: &Txid) -> Result, Error> { - maybe_await!(self.deref().get_tx(txid)) - } fn broadcast(&self, tx: &Transaction) -> Result<(), Error> { maybe_await!(self.deref().broadcast(tx)) } @@ -242,6 +244,13 @@ impl Blockchain for Arc { } } +#[maybe_async] +impl GetTx for Arc { + fn get_tx(&self, txid: &Txid) -> Result, Error> { + maybe_await!(self.deref().get_tx(txid)) + } +} + #[maybe_async] impl GetHeight for Arc { fn get_height(&self) -> Result { diff --git a/src/blockchain/rpc.rs b/src/blockchain/rpc.rs index b96af993..a474a0e2 100644 --- a/src/blockchain/rpc.rs +++ b/src/blockchain/rpc.rs @@ -33,9 +33,7 @@ use crate::bitcoin::consensus::deserialize; use crate::bitcoin::{Address, Network, OutPoint, Transaction, TxOut, Txid}; -use crate::blockchain::{ - Blockchain, Capability, ConfigurableBlockchain, GetHeight, Progress, WalletSync, -}; +use crate::blockchain::*; use crate::database::{BatchDatabase, DatabaseUtils}; use crate::{BlockTime, Error, FeeRate, KeychainKind, LocalUtxo, TransactionDetails}; use bitcoincore_rpc::json::{ @@ -141,10 +139,6 @@ impl Blockchain for RpcBlockchain { self.capabilities.clone() } - fn get_tx(&self, txid: &Txid) -> Result, Error> { - Ok(Some(self.client.get_raw_transaction(txid, None)?)) - } - fn broadcast(&self, tx: &Transaction) -> Result<(), Error> { Ok(self.client.send_raw_transaction(tx).map(|_| ())?) } @@ -161,6 +155,12 @@ impl Blockchain for RpcBlockchain { } } +impl GetTx for RpcBlockchain { + fn get_tx(&self, txid: &Txid) -> Result, Error> { + Ok(Some(self.client.get_raw_transaction(txid, None)?)) + } +} + impl GetHeight for RpcBlockchain { fn get_height(&self) -> Result { Ok(self.client.get_blockchain_info().map(|i| i.blocks as u32)?) diff --git a/src/testutils/blockchain_tests.rs b/src/testutils/blockchain_tests.rs index 68858a84..eefdd75b 100644 --- a/src/testutils/blockchain_tests.rs +++ b/src/testutils/blockchain_tests.rs @@ -1097,7 +1097,7 @@ macro_rules! bdk_blockchain_tests { // 2. // Core (#2) -> Us (#4) - let (wallet, _, mut test_client) = init_single_sig(); + let (wallet, blockchain, _, mut test_client) = init_single_sig(); let bdk_address = wallet.get_address(AddressIndex::New).unwrap().address; let core_address = test_client.get_new_address(None, None).unwrap(); let tx = testutils! { @@ -1108,7 +1108,7 @@ macro_rules! bdk_blockchain_tests { let txid_1 = test_client.receive(tx); let tx_1: Transaction = deserialize(&test_client.get_transaction(&txid_1, None).unwrap().hex).unwrap(); let vout_1 = tx_1.output.into_iter().position(|o| o.script_pubkey == core_address.script_pubkey()).unwrap() as u32; - wallet.sync(noop_progress(), None).unwrap(); + wallet.sync(&blockchain, SyncOptions::default()).unwrap(); let tx_1 = wallet.list_transactions(false).unwrap().into_iter().find(|tx| tx.txid == txid_1).unwrap(); assert_eq!(tx_1.received, 50_000); assert_eq!(tx_1.sent, 0); @@ -1119,7 +1119,7 @@ macro_rules! bdk_blockchain_tests { }; let txid_2 = test_client.receive(tx); - wallet.sync(noop_progress(), None).unwrap(); + wallet.sync(&blockchain, SyncOptions::default()).unwrap(); let tx_2 = wallet.list_transactions(false).unwrap().into_iter().find(|tx| tx.txid == txid_2).unwrap(); assert_eq!(tx_2.received, 10_000); assert_eq!(tx_2.sent, 0); diff --git a/src/wallet/mod.rs b/src/wallet/mod.rs index b37a108c..d0f86344 100644 --- a/src/wallet/mod.rs +++ b/src/wallet/mod.rs @@ -171,7 +171,6 @@ impl Wallet where D: BatchDatabase, { - #[deprecated = "Just use Wallet::new -- all wallets are offline now!"] /// Create a new "offline" wallet pub fn new_offline( diff --git a/src/wallet/verify.rs b/src/wallet/verify.rs index 9b563339..0f12e056 100644 --- a/src/wallet/verify.rs +++ b/src/wallet/verify.rs @@ -17,7 +17,7 @@ use std::fmt; use bitcoin::consensus::serialize; use bitcoin::{OutPoint, Transaction, Txid}; -use crate::blockchain::Blockchain; +use crate::blockchain::GetTx; use crate::database::Database; use crate::error::Error; @@ -29,7 +29,7 @@ use crate::error::Error; /// Depending on the [capabilities](crate::blockchain::Blockchain::get_capabilities) of the /// [`Blockchain`] backend, the method could fail when called with old "historical" transactions or /// with unconfirmed transactions that have been evicted from the backend's memory. -pub fn verify_tx( +pub fn verify_tx( tx: &Transaction, database: &D, blockchain: &B, @@ -104,43 +104,18 @@ impl_error!(bitcoinconsensus::Error, Consensus, VerifyError); #[cfg(test)] mod test { - use std::collections::HashSet; - + use super::*; + use crate::database::{BatchOperations, MemoryDatabase}; use bitcoin::consensus::encode::deserialize; use bitcoin::hashes::hex::FromHex; use bitcoin::{Transaction, Txid}; - use crate::blockchain::{Blockchain, Capability, Progress}; - use crate::database::{BatchDatabase, BatchOperations, MemoryDatabase}; - use crate::FeeRate; - - use super::*; - struct DummyBlockchain; - impl Blockchain for DummyBlockchain { - fn get_capabilities(&self) -> HashSet { - Default::default() - } - fn setup( - &self, - _database: &mut D, - _progress_update: P, - ) -> Result<(), Error> { - Ok(()) - } + impl GetTx for DummyBlockchain { fn get_tx(&self, _txid: &Txid) -> Result, Error> { Ok(None) } - fn broadcast(&self, _tx: &Transaction) -> Result<(), Error> { - Ok(()) - } - fn get_height(&self) -> Result { - Ok(42) - } - fn estimate_fee(&self, _target: usize) -> Result { - Ok(FeeRate::default_min_relay_fee()) - } } #[test]