Skip to content

Commit ba4d2fd

Browse files
committed
[wallet] Verify unconfirmed transactions after syncing
Verify the unconfirmed transactions we download against the consensus rules. This is currently exposed as an extra `verify` feature, since it depends on a pre-release version of `bitcoinconsensus`. Closes #352
1 parent 5633475 commit ba4d2fd

File tree

11 files changed

+223
-2
lines changed

11 files changed

+223
-2
lines changed

.github/workflows/cont_integration.yml

+1
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ jobs:
2222
- compact_filters
2323
- esplora,key-value-db,electrum
2424
- compiler
25+
- verify
2526
steps:
2627
- name: checkout
2728
uses: actions/checkout@v2

CHANGELOG.md

+3
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
66

77
## [Unreleased]
88

9+
### Wallet
10+
- Add a `verify` feature that can be enable to verify the unconfirmed txs we download against the consensus rules
11+
912
## [v0.7.0] - [v0.6.0]
1013

1114
### Policy

Cargo.toml

+2
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ cc = { version = ">=1.0.64", optional = true }
3131
socks = { version = "0.3", optional = true }
3232
lazy_static = { version = "1.4", optional = true }
3333
tiny-bip39 = { version = "^0.8", optional = true }
34+
bitcoinconsensus = { version = "0.19.0-3", optional = true }
3435

3536
# Needed by bdk_blockchain_tests macro
3637
bitcoincore-rpc = { version = "0.13", optional = true }
@@ -48,6 +49,7 @@ rand = { version = "^0.7", features = ["wasm-bindgen"] }
4849
[features]
4950
minimal = []
5051
compiler = ["miniscript/compiler"]
52+
verify = ["bitcoinconsensus"]
5153
default = ["key-value-db", "electrum"]
5254
electrum = ["electrum-client"]
5355
esplora = ["reqwest", "futures"]

src/blockchain/utils.rs

+1
Original file line numberDiff line numberDiff line change
@@ -357,6 +357,7 @@ fn save_transaction_details_and_utxos<D: BatchDatabase>(
357357
sent: outgoing,
358358
height,
359359
timestamp,
360+
verified: height.is_some(),
360361
fees: inputs_sum.saturating_sub(outputs_sum), /* if the tx is a coinbase, fees would be negative */
361362
};
362363
updates.set_tx(&tx_details)?;

src/database/memory.rs

+1
Original file line numberDiff line numberDiff line change
@@ -485,6 +485,7 @@ macro_rules! populate_test_db {
485485
received: 0,
486486
sent: 0,
487487
fees: 0,
488+
verified: height.is_some(),
488489
};
489490

490491
db.set_tx(&tx_details).unwrap();

src/database/mod.rs

+1
Original file line numberDiff line numberDiff line change
@@ -319,6 +319,7 @@ pub mod test {
319319
sent: 420420,
320320
fees: 140,
321321
height: Some(1000),
322+
verified: true,
322323
};
323324

324325
tree.set_tx(&tx_details).unwrap();

src/error.rs

+13
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,9 @@ pub enum Error {
8080
InvalidPolicyPathError(crate::descriptor::policy::PolicyError),
8181
/// Signing error
8282
Signer(crate::wallet::signer::SignerError),
83+
#[cfg(feature = "verify")]
84+
/// Transaction verification error
85+
Verification(crate::wallet::verify::VerifyError),
8386

8487
/// Progress value must be between `0.0` (included) and `100.0` (included)
8588
InvalidProgressValue(f32),
@@ -189,3 +192,13 @@ impl From<crate::blockchain::compact_filters::CompactFiltersError> for Error {
189192
}
190193
}
191194
}
195+
196+
#[cfg(feature = "verify")]
197+
impl From<crate::wallet::verify::VerifyError> for Error {
198+
fn from(other: crate::wallet::verify::VerifyError) -> Self {
199+
match other {
200+
crate::wallet::verify::VerifyError::Global(inner) => *inner,
201+
err => Error::Verification(err),
202+
}
203+
}
204+
}

src/types.rs

+4
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,10 @@ pub struct TransactionDetails {
165165
pub fees: u64,
166166
/// Confirmed in block height, `None` means unconfirmed
167167
pub height: Option<u32>,
168+
/// Whether the signatures/script execution of the tx has been verified. Mostly relevant for
169+
/// uncunconfirmed txs
170+
#[serde(default = "bool::default")] // default to `false` if not specified
171+
pub verified: bool,
168172
}
169173

170174
#[cfg(test)]

src/wallet/export.rs

+1
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,7 @@ mod test {
226226
sent: 0,
227227
fees: 500,
228228
height: Some(5000),
229+
verified: true,
229230
})
230231
.unwrap();
231232

src/wallet/mod.rs

+25-2
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,9 @@ pub mod signer;
4141
pub mod time;
4242
pub mod tx_builder;
4343
pub(crate) mod utils;
44+
#[cfg(feature = "verify")]
45+
#[cfg_attr(docsrs, doc(cfg(feature = "verify")))]
46+
pub mod verify;
4447

4548
pub use utils::IsDust;
4649

@@ -673,6 +676,7 @@ where
673676
sent,
674677
fees: fee_amount,
675678
height: None,
679+
verified: true,
676680
};
677681

678682
Ok((psbt, transaction_details))
@@ -1474,14 +1478,33 @@ where
14741478
None,
14751479
self.database.borrow_mut().deref_mut(),
14761480
progress_update,
1477-
))
1481+
))?;
14781482
} else {
14791483
maybe_await!(self.client.sync(
14801484
None,
14811485
self.database.borrow_mut().deref_mut(),
14821486
progress_update,
1483-
))
1487+
))?;
14841488
}
1489+
1490+
#[cfg(feature = "verify")]
1491+
{
1492+
debug!("Verifying transactions...");
1493+
for mut tx in self.database.borrow().iter_txs(true)? {
1494+
if !tx.verified {
1495+
verify::verify_tx(
1496+
tx.transaction.as_ref().ok_or(Error::TransactionNotFound)?,
1497+
self.database.borrow().deref(),
1498+
&self.client,
1499+
)?;
1500+
1501+
tx.verified = true;
1502+
self.database.borrow_mut().set_tx(&tx)?;
1503+
}
1504+
}
1505+
}
1506+
1507+
Ok(())
14851508
}
14861509

14871510
/// Return a reference to the internal blockchain client

src/wallet/verify.rs

+171
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,171 @@
1+
// Bitcoin Dev Kit
2+
// Written in 2021 by Alekos Filini <[email protected]>
3+
//
4+
// Copyright (c) 2020-2021 Bitcoin Dev Kit Developers
5+
//
6+
// This file is licensed under the Apache License, Version 2.0 <LICENSE-APACHE
7+
// or http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
8+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your option.
9+
// You may not use this file except in accordance with one or both of these
10+
// licenses.
11+
12+
use std::fmt;
13+
14+
use bitcoin::consensus::serialize;
15+
use bitcoin::{OutPoint, Transaction, Txid};
16+
17+
use crate::blockchain::Blockchain;
18+
use crate::database::Database;
19+
use crate::error::Error;
20+
21+
/// Verify a transaction against the consensus rules
22+
///
23+
/// This function uses [`bitcoinconsensus`] to verify transactions by fetching the required data
24+
/// either from the [`Database`] or using the [`Blockchain`].
25+
///
26+
/// Depending on the [capabilities](crate::blockchain::Blockchain::get_capabilities) of the
27+
/// [`Blockchain`] backend, the method could fail when called with old "historical" transactions or
28+
/// with unconfirmed transactions that have been evicted from the backend's memory.
29+
pub fn verify_tx<D: Database, B: Blockchain>(
30+
tx: &Transaction,
31+
database: &D,
32+
blockchain: &B,
33+
) -> Result<(), VerifyError> {
34+
log::debug!("Verifying {}", tx.txid());
35+
36+
let serialized_tx = serialize(tx);
37+
38+
for (index, input) in tx.input.iter().enumerate() {
39+
let prev_tx = if let Some(prev_tx) = database.get_raw_tx(&input.previous_output.txid)? {
40+
prev_tx
41+
} else if let Some(prev_tx) = blockchain.get_tx(&input.previous_output.txid)? {
42+
prev_tx
43+
} else {
44+
return Err(VerifyError::MissingInputTx(input.previous_output.txid));
45+
};
46+
47+
let spent_output = prev_tx
48+
.output
49+
.get(input.previous_output.vout as usize)
50+
.ok_or_else(|| VerifyError::InvalidInput(input.previous_output))?;
51+
52+
bitcoinconsensus::verify(
53+
&spent_output.script_pubkey.to_bytes(),
54+
spent_output.value,
55+
&serialized_tx,
56+
index,
57+
)?;
58+
}
59+
60+
Ok(())
61+
}
62+
63+
#[derive(Debug)]
64+
pub enum VerifyError {
65+
MissingInputTx(Txid),
66+
InvalidInput(OutPoint),
67+
68+
Consensus(bitcoinconsensus::Error),
69+
70+
Global(Box<Error>),
71+
}
72+
73+
impl fmt::Display for VerifyError {
74+
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
75+
write!(f, "{:?}", self)
76+
}
77+
}
78+
79+
impl std::error::Error for VerifyError {}
80+
81+
impl From<Error> for VerifyError {
82+
fn from(other: Error) -> Self {
83+
VerifyError::Global(Box::new(other))
84+
}
85+
}
86+
impl From<bitcoinconsensus::Error> for VerifyError {
87+
fn from(other: bitcoinconsensus::Error) -> Self {
88+
VerifyError::Consensus(other)
89+
}
90+
}
91+
92+
#[cfg(test)]
93+
mod test {
94+
use std::collections::HashSet;
95+
96+
use bitcoin::consensus::encode::deserialize;
97+
use bitcoin::hashes::hex::FromHex;
98+
use bitcoin::{Transaction, Txid};
99+
100+
use crate::blockchain::{Blockchain, Capability, Progress};
101+
use crate::database::{BatchDatabase, BatchOperations, MemoryDatabase};
102+
use crate::FeeRate;
103+
104+
use super::*;
105+
106+
struct DummyBlockchain;
107+
108+
impl Blockchain for DummyBlockchain {
109+
fn get_capabilities(&self) -> HashSet<Capability> {
110+
Default::default()
111+
}
112+
fn setup<D: BatchDatabase, P: 'static + Progress>(
113+
&self,
114+
_stop_gap: Option<usize>,
115+
_database: &mut D,
116+
_progress_update: P,
117+
) -> Result<(), Error> {
118+
Ok(())
119+
}
120+
fn get_tx(&self, _txid: &Txid) -> Result<Option<Transaction>, Error> {
121+
Ok(None)
122+
}
123+
fn broadcast(&self, _tx: &Transaction) -> Result<(), Error> {
124+
Ok(())
125+
}
126+
fn get_height(&self) -> Result<u32, Error> {
127+
Ok(42)
128+
}
129+
fn estimate_fee(&self, _target: usize) -> Result<FeeRate, Error> {
130+
Ok(FeeRate::default_min_relay_fee())
131+
}
132+
}
133+
134+
#[test]
135+
fn test_verify_fail_unsigned_tx() {
136+
let prev_tx: Transaction = deserialize(&Vec::<u8>::from_hex("020000000101192dea5e66d444380e106f8e53acb171703f00d43fb6b3ae88ca5644bdb7e1000000006b48304502210098328d026ce138411f957966c1cf7f7597ccbb170f5d5655ee3e9f47b18f6999022017c3526fc9147830e1340e04934476a3d1521af5b4de4e98baf49ec4c072079e01210276f847f77ec8dd66d78affd3c318a0ed26d89dab33fa143333c207402fcec352feffffff023d0ac203000000001976a9144bfbaf6afb76cc5771bc6404810d1cc041a6933988aca4b956050000000017a91494d5543c74a3ee98e0cf8e8caef5dc813a0f34b48768cb0700").unwrap()).unwrap();
137+
let signed_tx: Transaction = deserialize(&Vec::<u8>::from_hex("02000000013f7cebd65c27431a90bba7f796914fe8cc2ddfc3f2cbd6f7e5f2fc854534da95000000006b483045022100de1ac3bcdfb0332207c4a91f3832bd2c2915840165f876ab47c5f8996b971c3602201c6c053d750fadde599e6f5c4e1963df0f01fc0d97815e8157e3d59fe09ca30d012103699b464d1d8bc9e47d4fb1cdaa89a1c5783d68363c4dbc4b524ed3d857148617feffffff02836d3c01000000001976a914fc25d6d5c94003bf5b0c7b640a248e2c637fcfb088ac7ada8202000000001976a914fbed3d9b11183209a57999d54d59f67c019e756c88ac6acb0700").unwrap()).unwrap();
138+
139+
let mut database = MemoryDatabase::new();
140+
let blockchain = DummyBlockchain;
141+
142+
let mut unsigned_tx = signed_tx.clone();
143+
for input in &mut unsigned_tx.input {
144+
input.script_sig = Default::default();
145+
input.witness = Default::default();
146+
}
147+
148+
let result = verify_tx(&signed_tx, &database, &blockchain);
149+
assert!(result.is_err(), "Should fail with missing input tx");
150+
assert!(
151+
matches!(result, Err(VerifyError::MissingInputTx(txid)) if txid == prev_tx.txid()),
152+
"Error should be a `MissingInputTx` error"
153+
);
154+
155+
// insert the prev_tx
156+
database.set_raw_tx(&prev_tx).unwrap();
157+
158+
let result = verify_tx(&unsigned_tx, &database, &blockchain);
159+
assert!(result.is_err(), "Should fail since the TX is unsigned");
160+
assert!(
161+
matches!(result, Err(VerifyError::Consensus(_))),
162+
"Error should be a `Consensus` error"
163+
);
164+
165+
let result = verify_tx(&signed_tx, &database, &blockchain);
166+
assert!(
167+
result.is_ok(),
168+
"Should work since the TX is correctly signed"
169+
);
170+
}
171+
}

0 commit comments

Comments
 (0)