frost/bdk
3
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge bitcoindevkit/bdk#502: Refactor verification logic

Browse Source 
1999d97aeb3d97ee24a9b59a2f1453a26943b595 Remove `verify` flag from `TransactionDetails` via new MIGRATIONS (Steve Myers)
0195bc0636d9a58013f0cbc3781d213b0cfc1509 Update CHANGELOG (rajarshimaitra)
1d7ea89d8a792013212b1d269895b02fe3b410bd Refactor sync time verification (rajarshimaitra)
b05ee78c7335f7e3a1593dc4ff7686e6f72ff5c0 Remove verifcation flag from compact_filters (rajarshimaitra)
53c30b0479c74dde17cd27f8eac7f540e492067d Add verification tests in CI (rajarshimaitra)
6a09075d1a87509e9117eab727132efdf8ea6e1d Remove verify flag from sqlite DB (rajarshimaitra)
61a95d0d15b9944e8b5d13db64ef6ffd9a8d6e29 Update changelog (rajarshimaitra)
08f312a82f889f6bf5dfedfaa565ac2cb59683ae Remove `verify` flag from `TransactionDetails` (rajarshimaitra)
acbf0ae08e0732579652aece9bc48169351884cf Add sync verification for `esplora` (rajarshimaitra)
4761155707a819c4db437e71a36621a027d5302a Add sync verification in `electrum` (rajarshimaitra)
98a3b3282a0ff59bbdf900adc765f6912d1975c1 Remove sync verification (rajarshimaitra)

Pull request description:

  <!-- You can erase any parts of this template not applicable to your Pull Request. -->

  ### Description

  As discussed in https://github.com/bitcoindevkit/bdk/issues/498 and also in team call,
   - default verification from wallet sync is removed
   - `verify_tx` refactored as an wallet API
   - in `sync` verification added for electrum and esplora backends, gated by `verify` flag.
   - `verify` flag is removed from `TransactionDetails`.

  ### Notes to the reviewers

  I haven't looked into `comapct_filters` to see how verification can fit there, but that will probably be required in future.

  ### Checklists

  #### All Submissions:

  * [x] I've signed all my commits
  * [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
  * [x] I ran `cargo fmt` and `cargo clippy` before committing

  #### Wallet API change:

  * [x] I've updated `CHANGELOG.md`

Top commit has no ACKs.

Tree-SHA512: 72e307008a137468d96d5c2a6ec804b18fa52363606f3c978208ae5dc22973a7f0aa37488e9bb98dde88409a12d59cc5f00c675d2d408e57e661bf6210bee67b
This commit is contained in:
Steve Myers 2022-02-21 19:21:48 -08:00
commit 19f028714b
No known key found for this signature in database
GPG Key ID: 8105A46B22C2D051
14 changed files with 38 additions and 57 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
.github/workflows/cont_integration.yml vendored
View File

@ -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

2
CHANGELOG.md
View File

@ -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]

1
src/blockchain/compact_filters/mod.rs
View File

@ -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)),
};

2
src/blockchain/electrum.rs
View File

@ -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::<Result<Vec<_>, Error>>()?;

4
src/blockchain/esplora/reqwest.rs
View File

@ -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::<Result<_, Error>>()?;
tx_req.satisfy(full_txs)?
}
Request::Finish(batch_update) => break batch_update,

4
src/blockchain/esplora/ureq.rs
View File

@ -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::<Result<_, Error>>()?;
tx_req.satisfy(full_txs)?
}
Request::Finish(batch_update) => break batch_update,

1
src/blockchain/rpc.rs
View File

@ -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:{:?}",

19
src/blockchain/script_sync.rs
View File

@ -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::<Result<Vec<_>, _>>()?;

1
src/database/memory.rs
View File

@ -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();

1
src/database/mod.rs
View File

@ -348,7 +348,6 @@ pub mod test {
timestamp: 123456,
height: 1000,
}),
verified: true,
};
tree.set_tx(&tx_details).unwrap();

26
src/database/sqlite.rs
View File

@ -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<Vec<TransactionDetails>, 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<TransactionDetails> = 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<u64> = row.get(4)?;
let height: Option<u32> = row.get(5)?;
let verified: bool = row.get(6)?;
let raw_tx: Option<Vec<u8>> = row.get(7)?;
let tx: Option<Transaction> = 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<Vec<TransactionDetails>, 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<TransactionDetails> = vec![];
let mut rows = statement.query([])?;
@ -420,7 +420,6 @@ impl SqliteDatabase {
let sent: u64 = row.get(3)?;
let fee: Option<u64> = row.get(4)?;
let height: Option<u32> = 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<Option<TransactionDetails>, 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<u64> = row.get(3)?;
let height: Option<u32> = row.get(4)?;
let verified: bool = row.get(5)?;
let raw_tx: Option<Vec<u8>> = row.get(6)?;
let raw_tx: Option<Vec<u8>> = row.get(5)?;
let tx: Option<Transaction> = 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),

9
src/types.rs
View File

@ -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<BlockTime>,
/// 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

1
src/wallet/export.rs
View File

@ -230,7 +230,6 @@ mod test {
timestamp: 12345678,
height: 5000,
}),
verified: true,
})
.unwrap();

18
src/wallet/mod.rs
View File

@ -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())?,