diff --git a/.github/workflows/cont_integration.yml b/.github/workflows/cont_integration.yml index 71a49cab..5291abdf 100644 --- a/.github/workflows/cont_integration.yml +++ b/.github/workflows/cont_integration.yml @@ -23,6 +23,7 @@ jobs: - esplora,key-value-db,electrum - compiler - rpc + - verify steps: - name: checkout uses: actions/checkout@v2 diff --git a/CHANGELOG.md b/CHANGELOG.md index b9e04a11..58c404b2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Wallet #### Added - Bitcoin core RPC added as blockchain backend +- Add a `verify` feature that can be enable to verify the unconfirmed txs we download against the consensus rules ## [v0.8.0] - [v0.7.0] diff --git a/Cargo.toml b/Cargo.toml index 67cb6bad..8827b09d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -31,6 +31,7 @@ cc = { version = ">=1.0.64", optional = true } socks = { version = "0.3", optional = true } lazy_static = { version = "1.4", optional = true } tiny-bip39 = { version = "^0.8", optional = true } +bitcoinconsensus = { version = "0.19.0-3", optional = true } # Needed by bdk_blockchain_tests macro bitcoincore-rpc = { version = "0.13", optional = true } @@ -48,6 +49,7 @@ rand = { version = "^0.7", features = ["wasm-bindgen"] } [features] minimal = [] compiler = ["miniscript/compiler"] +verify = ["bitcoinconsensus"] default = ["key-value-db", "electrum"] electrum = ["electrum-client"] esplora = ["reqwest", "futures"] diff --git a/src/blockchain/rpc.rs b/src/blockchain/rpc.rs index ab55cc49..c457af59 100644 --- a/src/blockchain/rpc.rs +++ b/src/blockchain/rpc.rs @@ -233,6 +233,7 @@ impl Blockchain for RpcBlockchain { received, sent, fee: tx_result.fee.map(|f| f.as_sat().abs() as u64), + verified: true, }; debug!( "saving tx: {} tx_result.fee:{:?} td.fees:{:?}", diff --git a/src/blockchain/utils.rs b/src/blockchain/utils.rs index 0ce96460..6258a9d3 100644 --- a/src/blockchain/utils.rs +++ b/src/blockchain/utils.rs @@ -362,6 +362,7 @@ fn save_transaction_details_and_utxos( sent: outgoing, confirmation_time: ConfirmationTime::new(height, timestamp), fee: Some(inputs_sum.saturating_sub(outputs_sum)), /* if the tx is a coinbase, fees would be negative */ + verified: height.is_some(), }; updates.set_tx(&tx_details)?; diff --git a/src/database/memory.rs b/src/database/memory.rs index 7ec10bf3..df56ef63 100644 --- a/src/database/memory.rs +++ b/src/database/memory.rs @@ -485,6 +485,7 @@ macro_rules! populate_test_db { received: 0, sent: 0, confirmation_time, + verified: current_height.is_some(), }; db.set_tx(&tx_details).unwrap(); diff --git a/src/database/mod.rs b/src/database/mod.rs index b18ba1c0..217153fd 100644 --- a/src/database/mod.rs +++ b/src/database/mod.rs @@ -321,6 +321,7 @@ pub mod test { timestamp: 123456, height: 1000, }), + verified: true, }; tree.set_tx(&tx_details).unwrap(); diff --git a/src/error.rs b/src/error.rs index 0b38c05e..8d622276 100644 --- a/src/error.rs +++ b/src/error.rs @@ -90,6 +90,10 @@ pub enum Error { /// found network, for example the network of the bitcoin node found: Network, }, + #[cfg(feature = "verify")] + /// Transaction verification error + Verification(crate::wallet::verify::VerifyError), + /// Progress value must be between `0.0` (included) and `100.0` (included) InvalidProgressValue(f32), /// Progress update error (maybe the channel has been closed) @@ -206,3 +210,13 @@ impl From for Error { } } } + +#[cfg(feature = "verify")] +impl From for Error { + fn from(other: crate::wallet::verify::VerifyError) -> Self { + match other { + crate::wallet::verify::VerifyError::Global(inner) => *inner, + err => Error::Verification(err), + } + } +} diff --git a/src/types.rs b/src/types.rs index c71b87f4..ab635ee1 100644 --- a/src/types.rs +++ b/src/types.rs @@ -165,6 +165,15 @@ pub struct TransactionDetails { /// If the transaction is confirmed, contains height and timestamp of the block containing the /// transaction, unconfirmed transaction contains `None`. pub confirmation_time: Option, + /// Whether the tx has been verified against the consensus rules + /// + /// Confirmed txs are considered "verified" by default, while unconfirmed txs are checked to + /// ensure an unstrusted [`Blockchain`](crate::blockchain::Blockchain) backend can't trick the + /// wallet into using an invalid tx as an RBF template. + /// + /// The check is only perfomed when the `verify` feature is enabled. + #[serde(default = "bool::default")] // default to `false` if not specified + pub verified: bool, } /// Block height and timestamp of the block containing the confirmed transaction diff --git a/src/wallet/export.rs b/src/wallet/export.rs index 58ff77d2..047e93a1 100644 --- a/src/wallet/export.rs +++ b/src/wallet/export.rs @@ -230,6 +230,7 @@ mod test { timestamp: 12345678, height: 5000, }), + verified: true, }) .unwrap(); diff --git a/src/wallet/mod.rs b/src/wallet/mod.rs index 499c9a1b..e29b8a62 100644 --- a/src/wallet/mod.rs +++ b/src/wallet/mod.rs @@ -42,6 +42,9 @@ pub mod signer; pub mod time; pub mod tx_builder; pub(crate) mod utils; +#[cfg(feature = "verify")] +#[cfg_attr(docsrs, doc(cfg(feature = "verify")))] +pub mod verify; pub use utils::IsDust; @@ -710,6 +713,7 @@ where received, sent, fee: Some(fee_amount), + verified: true, }; Ok((psbt, transaction_details)) @@ -1526,14 +1530,33 @@ where None, self.database.borrow_mut().deref_mut(), progress_update, - )) + ))?; } else { maybe_await!(self.client.sync( None, self.database.borrow_mut().deref_mut(), progress_update, - )) + ))?; } + + #[cfg(feature = "verify")] + { + debug!("Verifying transactions..."); + for mut tx in self.database.borrow().iter_txs(true)? { + if !tx.verified { + verify::verify_tx( + tx.transaction.as_ref().ok_or(Error::TransactionNotFound)?, + self.database.borrow().deref(), + &self.client, + )?; + + tx.verified = true; + self.database.borrow_mut().set_tx(&tx)?; + } + } + } + + Ok(()) } /// Return a reference to the internal blockchain client diff --git a/src/wallet/verify.rs b/src/wallet/verify.rs new file mode 100644 index 00000000..c2ba9325 --- /dev/null +++ b/src/wallet/verify.rs @@ -0,0 +1,171 @@ +// Bitcoin Dev Kit +// Written in 2021 by Alekos Filini +// +// Copyright (c) 2020-2021 Bitcoin Dev Kit Developers +// +// This file is licensed under the Apache License, Version 2.0 or the MIT license +// , at your option. +// You may not use this file except in accordance with one or both of these +// licenses. + +use std::fmt; + +use bitcoin::consensus::serialize; +use bitcoin::{OutPoint, Transaction, Txid}; + +use crate::blockchain::Blockchain; +use crate::database::Database; +use crate::error::Error; + +/// Verify a transaction against the consensus rules +/// +/// This function uses [`bitcoinconsensus`] to verify transactions by fetching the required data +/// either from the [`Database`] or using the [`Blockchain`]. +/// +/// 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( + tx: &Transaction, + database: &D, + blockchain: &B, +) -> Result<(), VerifyError> { + log::debug!("Verifying {}", tx.txid()); + + let serialized_tx = serialize(tx); + + for (index, input) in tx.input.iter().enumerate() { + let prev_tx = if let Some(prev_tx) = database.get_raw_tx(&input.previous_output.txid)? { + prev_tx + } else if let Some(prev_tx) = blockchain.get_tx(&input.previous_output.txid)? { + prev_tx + } else { + return Err(VerifyError::MissingInputTx(input.previous_output.txid)); + }; + + let spent_output = prev_tx + .output + .get(input.previous_output.vout as usize) + .ok_or(VerifyError::InvalidInput(input.previous_output))?; + + bitcoinconsensus::verify( + &spent_output.script_pubkey.to_bytes(), + spent_output.value, + &serialized_tx, + index, + )?; + } + + Ok(()) +} + +#[derive(Debug)] +pub enum VerifyError { + MissingInputTx(Txid), + InvalidInput(OutPoint), + + Consensus(bitcoinconsensus::Error), + + Global(Box), +} + +impl fmt::Display for VerifyError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{:?}", self) + } +} + +impl std::error::Error for VerifyError {} + +impl From for VerifyError { + fn from(other: Error) -> Self { + VerifyError::Global(Box::new(other)) + } +} +impl From for VerifyError { + fn from(other: bitcoinconsensus::Error) -> Self { + VerifyError::Consensus(other) + } +} + +#[cfg(test)] +mod test { + use std::collections::HashSet; + + 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, + _stop_gap: Option, + _database: &mut D, + _progress_update: P, + ) -> Result<(), Error> { + Ok(()) + } + 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] + fn test_verify_fail_unsigned_tx() { + let prev_tx: Transaction = deserialize(&Vec::::from_hex("020000000101192dea5e66d444380e106f8e53acb171703f00d43fb6b3ae88ca5644bdb7e1000000006b48304502210098328d026ce138411f957966c1cf7f7597ccbb170f5d5655ee3e9f47b18f6999022017c3526fc9147830e1340e04934476a3d1521af5b4de4e98baf49ec4c072079e01210276f847f77ec8dd66d78affd3c318a0ed26d89dab33fa143333c207402fcec352feffffff023d0ac203000000001976a9144bfbaf6afb76cc5771bc6404810d1cc041a6933988aca4b956050000000017a91494d5543c74a3ee98e0cf8e8caef5dc813a0f34b48768cb0700").unwrap()).unwrap(); + let signed_tx: Transaction = deserialize(&Vec::::from_hex("02000000013f7cebd65c27431a90bba7f796914fe8cc2ddfc3f2cbd6f7e5f2fc854534da95000000006b483045022100de1ac3bcdfb0332207c4a91f3832bd2c2915840165f876ab47c5f8996b971c3602201c6c053d750fadde599e6f5c4e1963df0f01fc0d97815e8157e3d59fe09ca30d012103699b464d1d8bc9e47d4fb1cdaa89a1c5783d68363c4dbc4b524ed3d857148617feffffff02836d3c01000000001976a914fc25d6d5c94003bf5b0c7b640a248e2c637fcfb088ac7ada8202000000001976a914fbed3d9b11183209a57999d54d59f67c019e756c88ac6acb0700").unwrap()).unwrap(); + + let mut database = MemoryDatabase::new(); + let blockchain = DummyBlockchain; + + let mut unsigned_tx = signed_tx.clone(); + for input in &mut unsigned_tx.input { + input.script_sig = Default::default(); + input.witness = Default::default(); + } + + let result = verify_tx(&signed_tx, &database, &blockchain); + assert!(result.is_err(), "Should fail with missing input tx"); + assert!( + matches!(result, Err(VerifyError::MissingInputTx(txid)) if txid == prev_tx.txid()), + "Error should be a `MissingInputTx` error" + ); + + // insert the prev_tx + database.set_raw_tx(&prev_tx).unwrap(); + + let result = verify_tx(&unsigned_tx, &database, &blockchain); + assert!(result.is_err(), "Should fail since the TX is unsigned"); + assert!( + matches!(result, Err(VerifyError::Consensus(_))), + "Error should be a `Consensus` error" + ); + + let result = verify_tx(&signed_tx, &database, &blockchain); + assert!( + result.is_ok(), + "Should work since the TX is correctly signed" + ); + } +}