diff --git a/.github/workflows/cont_integration.yml b/.github/workflows/cont_integration.yml index bab633dd..5bf54f07 100644 --- a/.github/workflows/cont_integration.yml +++ b/.github/workflows/cont_integration.yml @@ -89,13 +89,13 @@ jobs: matrix: blockchain: - name: electrum - features: test-electrum + features: test-electrum,verify - name: rpc features: test-rpc - name: esplora - features: test-esplora,use-esplora-reqwest + features: test-esplora,use-esplora-reqwest,verify - name: esplora - features: test-esplora,use-esplora-ureq + features: test-esplora,use-esplora-ureq,verify steps: - name: Checkout uses: actions/checkout@v2 diff --git a/CHANGELOG.md b/CHANGELOG.md index 319c83cc..75801e01 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] - Pin tokio dependency version to ~1.14 to prevent errors due to their new MSRV 1.49.0 +- Removed default verification from `wallet::sync`. sync-time verification is added in `script_sync` and is activated by `verify` feature flag. +- `verify` flag removed from `TransactionDetails`. ## [v0.16.0] - [v0.15.0] diff --git a/src/blockchain/compact_filters/mod.rs b/src/blockchain/compact_filters/mod.rs index 93b5316c..56e9efc3 100644 --- a/src/blockchain/compact_filters/mod.rs +++ b/src/blockchain/compact_filters/mod.rs @@ -207,7 +207,6 @@ impl CompactFiltersBlockchain { received: incoming, sent: outgoing, confirmation_time: BlockTime::new(height, timestamp), - verified: height.is_some(), fee: Some(inputs_sum.saturating_sub(outputs_sum)), }; diff --git a/src/blockchain/electrum.rs b/src/blockchain/electrum.rs index cb079a87..1ab0db1c 100644 --- a/src/blockchain/electrum.rs +++ b/src/blockchain/electrum.rs @@ -175,6 +175,7 @@ impl Blockchain for ElectrumBlockchain { let full_details = full_transactions .into_iter() .map(|tx| { + let mut input_index = 0usize; let prev_outputs = tx .input .iter() @@ -189,6 +190,7 @@ impl Blockchain for ElectrumBlockchain { .output .get(input.previous_output.vout as usize) .ok_or_else(electrum_goof)?; + input_index += 1; Ok(Some(txout.clone())) }) .collect::, Error>>()?; diff --git a/src/blockchain/esplora/reqwest.rs b/src/blockchain/esplora/reqwest.rs index aaf487e4..494c6d30 100644 --- a/src/blockchain/esplora/reqwest.rs +++ b/src/blockchain/esplora/reqwest.rs @@ -167,9 +167,9 @@ impl Blockchain for EsploraBlockchain { .request() .map(|txid| { let tx = tx_index.get(txid).expect("must be in index"); - (tx.previous_outputs(), tx.to_tx()) + Ok((tx.previous_outputs(), tx.to_tx())) }) - .collect(); + .collect::>()?; tx_req.satisfy(full_txs)? } Request::Finish(batch_update) => break batch_update, diff --git a/src/blockchain/esplora/ureq.rs b/src/blockchain/esplora/ureq.rs index f3895a15..856f6958 100644 --- a/src/blockchain/esplora/ureq.rs +++ b/src/blockchain/esplora/ureq.rs @@ -166,9 +166,9 @@ impl Blockchain for EsploraBlockchain { .request() .map(|txid| { let tx = tx_index.get(txid).expect("must be in index"); - (tx.previous_outputs(), tx.to_tx()) + Ok((tx.previous_outputs(), tx.to_tx())) }) - .collect(); + .collect::>()?; tx_req.satisfy(full_txs)? } Request::Finish(batch_update) => break batch_update, diff --git a/src/blockchain/rpc.rs b/src/blockchain/rpc.rs index 251913ef..1b56cbaa 100644 --- a/src/blockchain/rpc.rs +++ b/src/blockchain/rpc.rs @@ -273,7 +273,6 @@ 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/script_sync.rs b/src/blockchain/script_sync.rs index 4c9b0222..6fd3b9c8 100644 --- a/src/blockchain/script_sync.rs +++ b/src/blockchain/script_sync.rs @@ -178,7 +178,9 @@ impl<'a, D: BatchDatabase> TxReq<'a, D> { let mut inputs_sum: u64 = 0; let mut outputs_sum: u64 = 0; - for (txout, input) in vout.into_iter().zip(tx.input.iter()) { + for (txout, (_input_index, input)) in + vout.into_iter().zip(tx.input.iter().enumerate()) + { let txout = match txout { Some(txout) => txout, None => { @@ -190,7 +192,19 @@ impl<'a, D: BatchDatabase> TxReq<'a, D> { continue; } }; - + // Verify this input if requested via feature flag + #[cfg(feature = "verify")] + { + use crate::wallet::verify::VerifyError; + let serialized_tx = bitcoin::consensus::serialize(&tx); + bitcoinconsensus::verify( + txout.script_pubkey.to_bytes().as_ref(), + txout.value, + &serialized_tx, + _input_index, + ) + .map_err(VerifyError::from)?; + } inputs_sum += txout.value; if self.state.db.is_mine(&txout.script_pubkey)? { sent += txout.value; @@ -214,7 +228,6 @@ impl<'a, D: BatchDatabase> TxReq<'a, D> { // we're going to fill this in later confirmation_time: None, fee: Some(fee), - verified: false, }) }) .collect::, _>>()?; diff --git a/src/database/memory.rs b/src/database/memory.rs index 916ddcba..afde6fee 100644 --- a/src/database/memory.rs +++ b/src/database/memory.rs @@ -515,7 +515,6 @@ 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 b0b9c9ea..2318dcc9 100644 --- a/src/database/mod.rs +++ b/src/database/mod.rs @@ -348,7 +348,6 @@ pub mod test { timestamp: 123456, height: 1000, }), - verified: true, }; tree.set_tx(&tx_details).unwrap(); diff --git a/src/database/sqlite.rs b/src/database/sqlite.rs index 597ef654..b9c639bd 100644 --- a/src/database/sqlite.rs +++ b/src/database/sqlite.rs @@ -35,7 +35,11 @@ static MIGRATIONS: &[&str] = &[ "CREATE UNIQUE INDEX idx_indices_keychain ON last_derivation_indices(keychain);", "CREATE TABLE checksums (keychain TEXT, checksum BLOB);", "CREATE INDEX idx_checksums_keychain ON checksums(keychain);", - "CREATE TABLE sync_time (id INTEGER PRIMARY KEY, height INTEGER, timestamp INTEGER);" + "CREATE TABLE sync_time (id INTEGER PRIMARY KEY, height INTEGER, timestamp INTEGER);", + "ALTER TABLE transaction_details RENAME TO transaction_details_old;", + "CREATE TABLE transaction_details (txid BLOB, timestamp INTEGER, received INTEGER, sent INTEGER, fee INTEGER, height INTEGER);", + "INSERT INTO transaction_details SELECT txid, timestamp, received, sent, fee, height FROM transaction_details_old;", + "DROP TABLE transaction_details_old;", ]; /// Sqlite database stored on filesystem @@ -127,7 +131,7 @@ impl SqliteDatabase { let txid: &[u8] = &transaction.txid; - let mut statement = self.connection.prepare_cached("INSERT INTO transaction_details (txid, timestamp, received, sent, fee, height, verified) VALUES (:txid, :timestamp, :received, :sent, :fee, :height, :verified)")?; + let mut statement = self.connection.prepare_cached("INSERT INTO transaction_details (txid, timestamp, received, sent, fee, height) VALUES (:txid, :timestamp, :received, :sent, :fee, :height)")?; statement.execute(named_params! { ":txid": txid, @@ -136,7 +140,6 @@ impl SqliteDatabase { ":sent": transaction.sent, ":fee": transaction.fee, ":height": height, - ":verified": transaction.verified })?; Ok(self.connection.last_insert_rowid()) @@ -153,7 +156,7 @@ impl SqliteDatabase { let txid: &[u8] = &transaction.txid; - let mut statement = self.connection.prepare_cached("UPDATE transaction_details SET timestamp=:timestamp, received=:received, sent=:sent, fee=:fee, height=:height, verified=:verified WHERE txid=:txid")?; + let mut statement = self.connection.prepare_cached("UPDATE transaction_details SET timestamp=:timestamp, received=:received, sent=:sent, fee=:fee, height=:height WHERE txid=:txid")?; statement.execute(named_params! { ":txid": txid, @@ -162,7 +165,6 @@ impl SqliteDatabase { ":sent": transaction.sent, ":fee": transaction.fee, ":height": height, - ":verified": transaction.verified, })?; Ok(()) @@ -367,7 +369,7 @@ impl SqliteDatabase { } fn select_transaction_details_with_raw(&self) -> Result, Error> { - let mut statement = self.connection.prepare_cached("SELECT transaction_details.txid, transaction_details.timestamp, transaction_details.received, transaction_details.sent, transaction_details.fee, transaction_details.height, transaction_details.verified, transactions.raw_tx FROM transaction_details, transactions WHERE transaction_details.txid = transactions.txid")?; + let mut statement = self.connection.prepare_cached("SELECT transaction_details.txid, transaction_details.timestamp, transaction_details.received, transaction_details.sent, transaction_details.fee, transaction_details.height, transactions.raw_tx FROM transaction_details, transactions WHERE transaction_details.txid = transactions.txid")?; let mut transaction_details: Vec = vec![]; let mut rows = statement.query([])?; while let Some(row) = rows.next()? { @@ -378,7 +380,6 @@ impl SqliteDatabase { let sent: u64 = row.get(3)?; let fee: Option = row.get(4)?; let height: Option = row.get(5)?; - let verified: bool = row.get(6)?; let raw_tx: Option> = row.get(7)?; let tx: Option = match raw_tx { Some(raw_tx) => { @@ -400,7 +401,6 @@ impl SqliteDatabase { sent, fee, confirmation_time, - verified, }); } Ok(transaction_details) @@ -408,7 +408,7 @@ impl SqliteDatabase { fn select_transaction_details(&self) -> Result, Error> { let mut statement = self.connection.prepare_cached( - "SELECT txid, timestamp, received, sent, fee, height, verified FROM transaction_details", + "SELECT txid, timestamp, received, sent, fee, height FROM transaction_details", )?; let mut transaction_details: Vec = vec![]; let mut rows = statement.query([])?; @@ -420,7 +420,6 @@ impl SqliteDatabase { let sent: u64 = row.get(3)?; let fee: Option = row.get(4)?; let height: Option = row.get(5)?; - let verified: bool = row.get(6)?; let confirmation_time = match (height, timestamp) { (Some(height), Some(timestamp)) => Some(BlockTime { height, timestamp }), @@ -434,7 +433,6 @@ impl SqliteDatabase { sent, fee, confirmation_time, - verified, }); } Ok(transaction_details) @@ -444,7 +442,7 @@ impl SqliteDatabase { &self, txid: &[u8], ) -> Result, Error> { - let mut statement = self.connection.prepare_cached("SELECT transaction_details.timestamp, transaction_details.received, transaction_details.sent, transaction_details.fee, transaction_details.height, transaction_details.verified, transactions.raw_tx FROM transaction_details, transactions WHERE transaction_details.txid=transactions.txid AND transaction_details.txid=:txid")?; + let mut statement = self.connection.prepare_cached("SELECT transaction_details.timestamp, transaction_details.received, transaction_details.sent, transaction_details.fee, transaction_details.height, transactions.raw_tx FROM transaction_details, transactions WHERE transaction_details.txid=transactions.txid AND transaction_details.txid=:txid")?; let mut rows = statement.query(named_params! { ":txid": txid })?; match rows.next()? { @@ -454,9 +452,8 @@ impl SqliteDatabase { let sent: u64 = row.get(2)?; let fee: Option = row.get(3)?; let height: Option = row.get(4)?; - let verified: bool = row.get(5)?; - let raw_tx: Option> = row.get(6)?; + let raw_tx: Option> = row.get(5)?; let tx: Option = match raw_tx { Some(raw_tx) => { let tx: Transaction = deserialize(&raw_tx)?; @@ -477,7 +474,6 @@ impl SqliteDatabase { sent, fee, confirmation_time, - verified, })) } None => Ok(None), diff --git a/src/types.rs b/src/types.rs index f9f34427..eadfc57b 100644 --- a/src/types.rs +++ b/src/types.rs @@ -211,15 +211,6 @@ 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 performed when the `verify` feature is enabled. - #[serde(default = "bool::default")] // default to `false` if not specified - pub verified: bool, } /// Block height and timestamp of a block diff --git a/src/wallet/export.rs b/src/wallet/export.rs index e39d178e..85898345 100644 --- a/src/wallet/export.rs +++ b/src/wallet/export.rs @@ -230,7 +230,6 @@ mod test { timestamp: 12345678, height: 5000, }), - verified: true, }) .unwrap(); diff --git a/src/wallet/mod.rs b/src/wallet/mod.rs index bf7993d4..a24f922d 100644 --- a/src/wallet/mod.rs +++ b/src/wallet/mod.rs @@ -712,7 +712,6 @@ where received, sent, fee: Some(fee_amount), - verified: true, }; Ok((psbt, transaction_details)) @@ -1537,23 +1536,6 @@ where .sync(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)?; - } - } - } - let sync_time = SyncTime { block_time: BlockTime { height: maybe_await!(self.client.get_height())?,