Merge bitcoindevkit/bdk#664: Deprecate AddressValidator

45db468c9be0c8bb2dc5fac52fbd52b6b41b4d53 Deprecate `AddressValidator` (志宇)

Pull request description:

  ### Description

  `AddressValidator` should be deprecated as noted by @afilini [on Discord](https://discord.com/channels/753336465005608961/753367451319926827/994899488957272064):

  > address validators are supposed to be used for a slightly different thing, which is when you ask the hardware wallet to independently generate the address for a derivation index and then you compare what you see on your computer/phone with what the hardware wallet is displaying
  > in the case of change addresses i agree that it's not as important (because as you said the device can just refuse to sign) but for consistency we implemented it for both external and internal addresses
  > more broadly, they can be thought of as a way to get a callback every time an address is generated, which may also be useful for other things (for example when i was working on a green-compatible client written in bdk i used that feature to ping the server every time a new address was generated, because that's required in their protocol)
  > that said, i think currently pretty much nobody uses them and i am myself moving away from the concept that "everything needs to happen inside bdk": currently my mindset is targeted more towards reducing complexity by breaking down individual parts and wrapping them or making them "extensible" in some way
  > that is to say: if you want to verify addresses in your hardware wallet you don't necessarily need bdk to do it for you (actually, you would still have to implement the callback manually), you can just call bdk to get a new addr and then ping the device yourself. and this would allow us to reduce complexity and delete some code
  > actually, here's an idea: unless somebody here is opposed to this, i can make a pr to deprecate address validators in the next (0.20) release. if after that again nobody complains we can completely remove them and point users towards different strategies to achieve the same goal

  ### Checklists

  * [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
  * [x] I've updated `CHANGELOG.md`

ACKs for top commit:
  afilini:
    ACK 45db468c9be0c8bb2dc5fac52fbd52b6b41b4d53

Tree-SHA512: 71071f4494537ece9153f5308cb4f576189016afa8ac87bc57bfdcda03ee94d5f7a3477d04f6dd37eeeea2fada6aaad42ad29c964df0971beeda7418ada65f6d
This commit is contained in:
Alekos Filini 2022-07-11 11:48:54 +02:00
commit 4bd1fd2441
No known key found for this signature in database
GPG Key ID: 431401E4A4530061
4 changed files with 17 additions and 0 deletions

View File

@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- 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`
## [v0.19.0] - [v0.18.0]

View File

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

View File

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

View File

@ -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<D> {
signers: Arc<SignersContainer>,
change_signers: Arc<SignersContainer>,
#[allow(deprecated)]
address_validators: Vec<Arc<dyn AddressValidator>>,
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<dyn AddressValidator>) {
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<dyn AddressValidator>] {
&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)?;
}