diff --git a/CHANGELOG.md b/CHANGELOG.md index f4982ae2..c28ca2be 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,11 +5,18 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] + + +## [v0.20.0] - [v0.19.0] + - New MSRV set to `1.56.1` - Fee sniping discouraging through nLockTime - if the user specifies a `current_height`, we use that as a nlocktime, otherwise we use the last sync height (or 0 if we never synced) - Fix hang when `ElectrumBlockchainConfig::stop_gap` is zero. - Set coin type in BIP44, BIP49, and BIP84 templates - Get block hash given a block height - A `get_block_hash` method is now defined on the `GetBlockHash` trait and implemented on every blockchain backend. This method expects a block height and returns the corresponding block hash. +- Add `remove_partial_sigs` and `try_finalize` to `SignOptions` +- Deprecate `AddressValidator` +- Fix Electrum wallet sync potentially causing address index decrement - compare proposed index and current index before applying batch operations during sync. ## [v0.19.0] - [v0.18.0] @@ -26,7 +33,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Signing Taproot PSBTs (key spend and script spend) - Support for `tr()` descriptors in the `descriptor!()` macro - Add support for Bitcoin Core 23.0 when using the `rpc` blockchain -- Add `remove_partial_sigs` and `try_finalize` to `SignOptions` ## [v0.18.0] - [v0.17.0] @@ -472,4 +478,5 @@ final transaction is created by calling `finish` on the builder. [v0.17.0]: https://github.com/bitcoindevkit/bdk/compare/v0.16.1...v0.17.0 [v0.18.0]: https://github.com/bitcoindevkit/bdk/compare/v0.17.0...v0.18.0 [v0.19.0]: https://github.com/bitcoindevkit/bdk/compare/v0.18.0...v0.19.0 -[unreleased]: https://github.com/bitcoindevkit/bdk/compare/v0.19.0...HEAD +[v0.20.0]: https://github.com/bitcoindevkit/bdk/compare/v0.19.0...v0.20.0 +[unreleased]: https://github.com/bitcoindevkit/bdk/compare/v0.20.0...HEAD diff --git a/Cargo.toml b/Cargo.toml index 49d5359e..84171ab5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "bdk" -version = "0.19.1-dev" +version = "0.20.1-dev" edition = "2018" authors = ["Alekos Filini ", "Riccardo Casatta "] homepage = "https://bitcoindevkit.org" diff --git a/examples/address_validator.rs b/examples/address_validator.rs index 85a23560..26c36dfe 100644 --- a/examples/address_validator.rs +++ b/examples/address_validator.rs @@ -14,6 +14,7 @@ use std::sync::Arc; use bdk::bitcoin; use bdk::database::MemoryDatabase; use bdk::descriptor::HdKeyPaths; +#[allow(deprecated)] use bdk::wallet::address_validator::{AddressValidator, AddressValidatorError}; use bdk::KeychainKind; use bdk::Wallet; @@ -25,6 +26,7 @@ use bitcoin::{Network, Script}; #[derive(Debug)] struct DummyValidator; +#[allow(deprecated)] impl AddressValidator for DummyValidator { fn validate( &self, @@ -50,6 +52,7 @@ fn main() -> Result<(), bdk::Error> { let descriptor = "sh(and_v(v:pk(tpubDDpWvmUrPZrhSPmUzCMBHffvC3HyMAPnWDSAQNBTnj1iZeJa7BZQEttFiP4DS4GCcXQHezdXhn86Hj6LHX5EDstXPWrMaSneRWM8yUf6NFd/*),after(630000)))"; let mut wallet = Wallet::new(descriptor, None, Network::Regtest, MemoryDatabase::new())?; + #[allow(deprecated)] wallet.add_address_validator(Arc::new(DummyValidator)); wallet.get_address(New)?; diff --git a/src/blockchain/rpc.rs b/src/blockchain/rpc.rs index 9ba6f0f3..1d0d884c 100644 --- a/src/blockchain/rpc.rs +++ b/src/blockchain/rpc.rs @@ -340,7 +340,7 @@ impl WalletSync for RpcBlockchain { ), received, sent, - fee: tx_result.fee.map(|f| f.as_sat().abs() as u64), + fee: tx_result.fee.map(|f| f.as_sat().unsigned_abs()), }; debug!( "saving tx: {} tx_result.fee:{:?} td.fees:{:?}", diff --git a/src/blockchain/script_sync.rs b/src/blockchain/script_sync.rs index ac78188e..05752736 100644 --- a/src/blockchain/script_sync.rs +++ b/src/blockchain/script_sync.rs @@ -314,6 +314,22 @@ impl<'a, D: BatchDatabase> State<'a, D> { let finished_txs = make_txs_consistent(&self.finished_txs); let observed_txids: HashSet = finished_txs.iter().map(|tx| tx.txid).collect(); let txids_to_delete = existing_txids.difference(&observed_txids); + + // Ensure `last_active_index` does not decrement database's current state. + let index_updates = self + .last_active_index + .iter() + .map(|(keychain, sync_index)| { + let sync_index = *sync_index as u32; + let index_res = match self.db.get_last_index(*keychain) { + Ok(Some(db_index)) => Ok(std::cmp::max(db_index, sync_index)), + Ok(None) => Ok(sync_index), + Err(err) => Err(err), + }; + index_res.map(|index| (*keychain, index)) + }) + .collect::, _>>()?; + let mut batch = self.db.begin_batch(); // Delete old txs that no longer exist @@ -377,8 +393,10 @@ impl<'a, D: BatchDatabase> State<'a, D> { batch.set_tx(finished_tx)?; } - for (keychain, last_active_index) in self.last_active_index { - batch.set_last_index(keychain, last_active_index as u32)?; + // apply index updates + for (keychain, new_index) in index_updates { + debug!("updating index ({}, {})", keychain.as_byte(), new_index); + batch.set_last_index(keychain, new_index)?; } info!( diff --git a/src/lib.rs b/src/lib.rs index 6db7103e..4c4bb3c2 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -44,7 +44,7 @@ //! interact with the bitcoin P2P network. //! //! ```toml -//! bdk = "0.18.0" +//! bdk = "0.20.0" //! ``` //! //! # Examples diff --git a/src/testutils/blockchain_tests.rs b/src/testutils/blockchain_tests.rs index 2ae54632..7c08699c 100644 --- a/src/testutils/blockchain_tests.rs +++ b/src/testutils/blockchain_tests.rs @@ -372,6 +372,7 @@ macro_rules! bdk_blockchain_tests { use $crate::blockchain::Blockchain; use $crate::database::MemoryDatabase; use $crate::types::KeychainKind; + use $crate::wallet::AddressIndex; use $crate::{Wallet, FeeRate, SyncOptions}; use $crate::testutils; @@ -651,6 +652,60 @@ macro_rules! bdk_blockchain_tests { assert_eq!(wallet.list_unspent().unwrap().len(), 1, "incorrect number of unspents"); } + // Syncing wallet should not result in wallet address index to decrement. + // This is critical as we should always ensure to not reuse addresses. + #[test] + fn test_sync_address_index_should_not_decrement() { + let (wallet, blockchain, _descriptors, mut test_client) = init_single_sig(); + + const ADDRS_TO_FUND: u32 = 7; + const ADDRS_TO_IGNORE: u32 = 11; + + let mut first_addr_index: u32 = 0; + + (0..ADDRS_TO_FUND + ADDRS_TO_IGNORE).for_each(|i| { + let new_addr = wallet.get_address(AddressIndex::New).unwrap(); + + if i == 0 { + first_addr_index = new_addr.index; + } + assert_eq!(new_addr.index, i+first_addr_index, "unexpected new address index (before sync)"); + + if i < ADDRS_TO_FUND { + test_client.receive(testutils! { + @tx ((@addr new_addr.address) => 50_000) + }); + } + }); + + wallet.sync(&blockchain, SyncOptions::default()).unwrap(); + + let new_addr = wallet.get_address(AddressIndex::New).unwrap(); + assert_eq!(new_addr.index, ADDRS_TO_FUND+ADDRS_TO_IGNORE+first_addr_index, "unexpected new address index (after sync)"); + } + + // Even if user does not explicitly grab new addresses, the address index should + // increment after sync (if wallet has a balance). + #[test] + fn test_sync_address_index_should_increment() { + let (wallet, blockchain, descriptors, mut test_client) = init_single_sig(); + + const START_FUND: u32 = 4; + const END_FUND: u32 = 20; + + // "secretly" fund wallet via given range + (START_FUND..END_FUND).for_each(|addr_index| { + test_client.receive(testutils! { + @tx ((@external descriptors, addr_index) => 50_000) + }); + }); + + wallet.sync(&blockchain, SyncOptions::default()).unwrap(); + + let address = wallet.get_address(AddressIndex::New).unwrap(); + assert_eq!(address.index, END_FUND, "unexpected new address index (after sync)"); + } + /// Send two conflicting transactions to the same address twice in a row. /// The coins should only be received once! #[test] diff --git a/src/wallet/address_validator.rs b/src/wallet/address_validator.rs index a0e418ab..eaac582c 100644 --- a/src/wallet/address_validator.rs +++ b/src/wallet/address_validator.rs @@ -100,6 +100,7 @@ impl std::error::Error for AddressValidatorError {} /// validator will be propagated up to the original caller that triggered the address generation. /// /// For a usage example see [this module](crate::address_validator)'s documentation. +#[deprecated = "AddressValidator was rarely used. Address validation can occur outside of BDK"] pub trait AddressValidator: Send + Sync + fmt::Debug { /// Validate or inspect an address fn validate( @@ -120,6 +121,7 @@ mod test { #[derive(Debug)] struct TestValidator; + #[allow(deprecated)] impl AddressValidator for TestValidator { fn validate( &self, @@ -135,6 +137,7 @@ mod test { #[should_panic(expected = "InvalidScript")] fn test_address_validator_external() { let (mut wallet, _, _) = get_funded_wallet(get_test_wpkh()); + #[allow(deprecated)] wallet.add_address_validator(Arc::new(TestValidator)); wallet.get_address(New).unwrap(); @@ -144,6 +147,7 @@ mod test { #[should_panic(expected = "InvalidScript")] fn test_address_validator_internal() { let (mut wallet, descriptors, _) = get_funded_wallet(get_test_wpkh()); + #[allow(deprecated)] wallet.add_address_validator(Arc::new(TestValidator)); let addr = crate::testutils!(@external descriptors, 10); diff --git a/src/wallet/mod.rs b/src/wallet/mod.rs index cab3b020..9231c3b7 100644 --- a/src/wallet/mod.rs +++ b/src/wallet/mod.rs @@ -50,6 +50,7 @@ pub mod verify; pub use utils::IsDust; +#[allow(deprecated)] use address_validator::AddressValidator; use coin_selection::DefaultCoinSelectionAlgorithm; use signer::{SignOptions, SignerOrdering, SignersContainer, TransactionSigner}; @@ -94,6 +95,7 @@ pub struct Wallet { signers: Arc, change_signers: Arc, + #[allow(deprecated)] address_validators: Vec>, network: Network, @@ -500,11 +502,17 @@ where /// Add an address validator /// /// See [the `address_validator` module](address_validator) for an example. + #[deprecated] + #[allow(deprecated)] pub fn add_address_validator(&mut self, validator: Arc) { self.address_validators.push(validator); } /// Get the address validators + /// + /// See [the `address_validator` module](address_validator). + #[deprecated] + #[allow(deprecated)] pub fn get_address_validators(&self) -> &[Arc] { &self.address_validators } @@ -1267,6 +1275,7 @@ where let script = derived_descriptor.script_pubkey(); for validator in &self.address_validators { + #[allow(deprecated)] validator.validate(keychain, &hd_keypaths, &script)?; } @@ -2058,7 +2067,7 @@ pub(crate) mod test { .set_sync_time(sync_time) .unwrap(); let current_height = 25; - builder.set_current_height(current_height); + builder.current_height(current_height); let (psbt, _) = builder.finish().unwrap(); // current_height will override the last sync height @@ -2106,7 +2115,7 @@ pub(crate) mod test { let mut builder = wallet.build_tx(); builder .add_recipient(addr.script_pubkey(), 25_000) - .set_current_height(630_001) + .current_height(630_001) .nlocktime(630_000); let (psbt, _) = builder.finish().unwrap(); @@ -4907,7 +4916,7 @@ pub(crate) mod test { let mut builder = wallet.build_tx(); builder .add_recipient(addr.script_pubkey(), balance / 2) - .set_current_height(confirmation_time); + .current_height(confirmation_time); assert!(matches!( builder.finish().unwrap_err(), Error::InsufficientFunds { @@ -4920,7 +4929,7 @@ pub(crate) mod test { let mut builder = wallet.build_tx(); builder .add_recipient(addr.script_pubkey(), balance / 2) - .set_current_height(not_yet_mature_time); + .current_height(not_yet_mature_time); assert!(matches!( builder.finish().unwrap_err(), Error::InsufficientFunds { @@ -4933,7 +4942,7 @@ pub(crate) mod test { let mut builder = wallet.build_tx(); builder .add_recipient(addr.script_pubkey(), balance / 2) - .set_current_height(maturity_time); + .current_height(maturity_time); builder.finish().unwrap(); } } diff --git a/src/wallet/signer.rs b/src/wallet/signer.rs index e93618da..cb190bf9 100644 --- a/src/wallet/signer.rs +++ b/src/wallet/signer.rs @@ -667,10 +667,12 @@ pub struct SignOptions { /// /// Defaults to `false` which will only allow signing using `SIGHASH_ALL`. pub allow_all_sighashes: bool, + /// Whether to remove partial_sigs from psbt inputs while finalizing psbt. /// /// Defaults to `true` which will remove partial_sigs after finalizing. pub remove_partial_sigs: bool, + /// Whether to try finalizing psbt input after the inputs are signed. /// /// Defaults to `true` which will try fianlizing psbt after inputs are signed. diff --git a/src/wallet/tx_builder.rs b/src/wallet/tx_builder.rs index 315a299a..0fee8aa9 100644 --- a/src/wallet/tx_builder.rs +++ b/src/wallet/tx_builder.rs @@ -556,7 +556,7 @@ impl<'a, D: BatchDatabase, Cs: CoinSelectionAlgorithm, Ctx: TxBuilderContext> /// add them using [`TxBuilder::add_utxos`]. /// /// In both cases, if you don't provide a current height, we use the last sync height. - pub fn set_current_height(&mut self, height: u32) -> &mut Self { + pub fn current_height(&mut self, height: u32) -> &mut Self { self.params.current_height = Some(height); self }