Skip to content

Commit

Permalink
Merge #537: refactor wallet address caching into its own public method
Browse files Browse the repository at this point in the history
edf2f0c refactor wallet address caching into its own public method for offline wallet use (a5an0)

Pull request description:

  ### Description

  Currently, the only way to ensure that a wallet's internal database has addresses loaded and cached is through `Wallet::sync`: that function generates and caches up to a number of addresses if they aren't already in the database, and then uses the wallet's blockchain client to sync those addresses. If you are using an offline wallet, there is no mechanism to ensure that the database has addresses loaded. This is a problem for usecases like an offline wallet being used as a multisig signer and wanting to validate change addresses as `Wallet::is_mine` will only work properly if the owned-address is loaded in the database.

  This PR takes the address caching functionality out of `Wallet::sync` and puts it in a new public method that is available to offline wallets. `Wallet::sync` uses this method internally.

  ### 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
  * [X] I've added docs for the new feature
  * [X] I've updated `CHANGELOG.md`

Top commit has no ACKs.

Tree-SHA512: 8ddc58d71457163bb20ff663ac508feb4e77000688161b63841a94db30b3f29f60f35fa2467bd99546123148873e3aed11e2f13ae6cbceda6605b83c227d9079
  • Loading branch information
notmandatory committed Mar 2, 2022
2 parents f0188f4 + 4291f84 commit adf7d0c
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 35 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.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`.
- Add `get_internal_address` to allow you to get internal addresses just as you get external addresses.
- added `ensure_addresses_cached` to `Wallet` to let offline wallets load and cache addresses in their database

## [v0.16.1] - [v0.16.0]

Expand Down
81 changes: 46 additions & 35 deletions src/wallet/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,50 @@ where
}
}

/// Ensures that there are at least `max_addresses` addresses cached in the database if the
/// descriptor is derivable, or 1 address if it is not.
/// Will return `Ok(true)` if there are new addresses generated (either external or internal),
/// and `Ok(false)` if all the required addresses are already cached. This function is useful to
/// explicitly cache addresses in a wallet to do things like check [`Wallet::is_mine`] on
/// transaction output scripts.
pub fn ensure_addresses_cached(&self, max_addresses: u32) -> Result<bool, Error> {
let mut new_addresses_cached = false;
let max_address = match self.descriptor.is_deriveable() {
false => 0,
true => max_addresses,
};
debug!("max_address {}", max_address);
if self
.database
.borrow()
.get_script_pubkey_from_path(KeychainKind::External, max_address.saturating_sub(1))?
.is_none()
{
debug!("caching external addresses");
new_addresses_cached = true;
self.cache_addresses(KeychainKind::External, 0, max_address)?;
}

if let Some(change_descriptor) = &self.change_descriptor {
let max_address = match change_descriptor.is_deriveable() {
false => 0,
true => max_addresses,
};

if self
.database
.borrow()
.get_script_pubkey_from_path(KeychainKind::Internal, max_address.saturating_sub(1))?
.is_none()
{
debug!("caching internal addresses");
new_addresses_cached = true;
self.cache_addresses(KeychainKind::Internal, 0, max_address)?;
}
}
Ok(new_addresses_cached)
}

/// Return whether or not a `script` is part of this wallet (either internal or external)
pub fn is_mine(&self, script: &Script) -> Result<bool, Error> {
self.database.borrow().is_mine(script)
Expand Down Expand Up @@ -1504,41 +1548,8 @@ where
) -> Result<(), Error> {
debug!("Begin sync...");

let mut run_setup = false;

let max_address = match self.descriptor.is_deriveable() {
false => 0,
true => max_address_param.unwrap_or(CACHE_ADDR_BATCH_SIZE),
};
debug!("max_address {}", max_address);
if self
.database
.borrow()
.get_script_pubkey_from_path(KeychainKind::External, max_address.saturating_sub(1))?
.is_none()
{
debug!("caching external addresses");
run_setup = true;
self.cache_addresses(KeychainKind::External, 0, max_address)?;
}

if let Some(change_descriptor) = &self.change_descriptor {
let max_address = match change_descriptor.is_deriveable() {
false => 0,
true => max_address_param.unwrap_or(CACHE_ADDR_BATCH_SIZE),
};

if self
.database
.borrow()
.get_script_pubkey_from_path(KeychainKind::Internal, max_address.saturating_sub(1))?
.is_none()
{
debug!("caching internal addresses");
run_setup = true;
self.cache_addresses(KeychainKind::Internal, 0, max_address)?;
}
}
let run_setup =
self.ensure_addresses_cached(max_address_param.unwrap_or(CACHE_ADDR_BATCH_SIZE))?;

debug!("run_setup: {}", run_setup);
// TODO: what if i generate an address first and cache some addresses?
Expand Down

0 comments on commit adf7d0c

Please sign in to comment.