Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Include the descriptor in keychain::Changeset #1203

Merged
merged 13 commits into from
May 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
282 changes: 214 additions & 68 deletions crates/bdk/src/wallet/mod.rs

Large diffs are not rendered by default.

73 changes: 65 additions & 8 deletions crates/bdk/tests/wallet.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use std::str::FromStr;

use assert_matches::assert_matches;
use bdk::descriptor::calc_checksum;
use bdk::descriptor::{calc_checksum, IntoWalletDescriptor};
use bdk::psbt::PsbtUtils;
use bdk::signer::{SignOptions, SignerError};
use bdk::wallet::coin_selection::{self, LargestFirstCoinSelection};
Expand All @@ -10,9 +10,11 @@ use bdk::wallet::tx_builder::AddForeignUtxoError;
use bdk::wallet::NewError;
use bdk::wallet::{AddressInfo, Balance, Wallet};
use bdk::KeychainKind;
use bdk_chain::collections::BTreeMap;
use bdk_chain::COINBASE_MATURITY;
use bdk_chain::{BlockId, ConfirmationTime};
use bitcoin::hashes::Hash;
use bitcoin::key::Secp256k1;
use bitcoin::psbt;
use bitcoin::script::PushBytesBuf;
use bitcoin::sighash::{EcdsaSighashType, TapSighashType};
Expand Down Expand Up @@ -84,14 +86,24 @@ fn load_recovers_wallet() {
// recover wallet
{
let db = bdk_file_store::Store::open(DB_MAGIC, &file_path).expect("must recover db");
let wallet =
Wallet::load(get_test_tr_single_sig_xprv(), None, db).expect("must recover wallet");
let wallet = Wallet::load(db).expect("must recover wallet");
assert_eq!(wallet.network(), Network::Testnet);
assert_eq!(wallet.spk_index().keychains(), wallet_spk_index.keychains());
assert_eq!(
wallet.spk_index().keychains().collect::<Vec<_>>(),
wallet_spk_index.keychains().collect::<Vec<_>>()
);
assert_eq!(
wallet.spk_index().last_revealed_indices(),
wallet_spk_index.last_revealed_indices()
);
let secp = Secp256k1::new();
assert_eq!(
*wallet.get_descriptor_for_keychain(KeychainKind::External),
get_test_tr_single_sig_xprv()
.into_wallet_descriptor(&secp, wallet.network())
.unwrap()
.0
);
}

// `new` can only be called on empty db
Expand All @@ -108,12 +120,12 @@ fn new_or_load() {
let file_path = temp_dir.path().join("store.db");

// init wallet when non-existent
let wallet_keychains = {
let wallet_keychains: BTreeMap<_, _> = {
let db = bdk_file_store::Store::open_or_create_new(DB_MAGIC, &file_path)
.expect("must create db");
let wallet = Wallet::new_or_load(get_test_wpkh(), None, db, Network::Testnet)
.expect("must init wallet");
wallet.keychains().clone()
wallet.keychains().map(|(k, v)| (*k, v.clone())).collect()
};

// wrong network
Expand Down Expand Up @@ -162,14 +174,60 @@ fn new_or_load() {
);
}

// wrong external descriptor
{
let exp_descriptor = get_test_tr_single_sig();
let got_descriptor = get_test_wpkh()
.into_wallet_descriptor(&Secp256k1::new(), Network::Testnet)
.unwrap()
.0;

let db =
bdk_file_store::Store::open_or_create_new(DB_MAGIC, &file_path).expect("must open db");
let err = Wallet::new_or_load(exp_descriptor, None, db, Network::Testnet)
.expect_err("wrong external descriptor");
assert!(
matches!(
err,
bdk::wallet::NewOrLoadError::LoadedDescriptorDoesNotMatch { ref got, keychain }
if got == &Some(got_descriptor) && keychain == KeychainKind::External
),
"err: {}",
err,
);
}

// wrong internal descriptor
{
let exp_descriptor = Some(get_test_tr_single_sig());
let got_descriptor = None;

let db =
bdk_file_store::Store::open_or_create_new(DB_MAGIC, &file_path).expect("must open db");
let err = Wallet::new_or_load(get_test_wpkh(), exp_descriptor, db, Network::Testnet)
.expect_err("wrong internal descriptor");
assert!(
matches!(
err,
bdk::wallet::NewOrLoadError::LoadedDescriptorDoesNotMatch { ref got, keychain }
if got == &got_descriptor && keychain == KeychainKind::Internal
),
"err: {}",
err,
);
}

// all parameters match
{
let db =
bdk_file_store::Store::open_or_create_new(DB_MAGIC, &file_path).expect("must open db");
let wallet = Wallet::new_or_load(get_test_wpkh(), None, db, Network::Testnet)
.expect("must recover wallet");
assert_eq!(wallet.network(), Network::Testnet);
assert_eq!(wallet.keychains(), &wallet_keychains);
assert!(wallet
.keychains()
.map(|(k, v)| (*k, v.clone()))
.eq(wallet_keychains));
}
}

Expand All @@ -181,7 +239,6 @@ fn test_descriptor_checksum() {

let raw_descriptor = wallet
.keychains()
.iter()
.next()
.unwrap()
.1
Expand Down
6 changes: 3 additions & 3 deletions crates/chain/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,6 @@ rand = "0.8"
proptest = "1.2.0"

[features]
default = ["std"]
std = ["bitcoin/std", "miniscript/std"]
serde = ["serde_crate", "bitcoin/serde"]
default = ["std", "miniscript"]
std = ["bitcoin/std", "miniscript?/std"]
serde = ["serde_crate", "bitcoin/serde", "miniscript?/serde"]
28 changes: 27 additions & 1 deletion crates/chain/src/descriptor_ext.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,29 @@
use crate::miniscript::{Descriptor, DescriptorPublicKey};
use crate::{
alloc::{string::ToString, vec::Vec},
miniscript::{Descriptor, DescriptorPublicKey},
};
use bitcoin::hashes::{hash_newtype, sha256, Hash};

hash_newtype! {
/// Represents the ID of a descriptor, defined as the sha256 hash of
/// the descriptor string, checksum excluded.
///
/// This is useful for having a fixed-length unique representation of a descriptor,
/// in particular, we use it to persist application state changes related to the
/// descriptor without having to re-write the whole descriptor each time.
///
pub struct DescriptorId(pub sha256::Hash);
}
danielabrozzoni marked this conversation as resolved.
Show resolved Hide resolved

/// A trait to extend the functionality of a miniscript descriptor.
pub trait DescriptorExt {
/// Returns the minimum value (in satoshis) at which an output is broadcastable.
/// Panics if the descriptor wildcard is hardened.
fn dust_value(&self) -> u64;

/// Returns the descriptor id, calculated as the sha256 of the descriptor, checksum not
/// included.
fn descriptor_id(&self) -> DescriptorId;
}

impl DescriptorExt for Descriptor<DescriptorPublicKey> {
Expand All @@ -15,4 +34,11 @@ impl DescriptorExt for Descriptor<DescriptorPublicKey> {
.dust_value()
.to_sat()
}

fn descriptor_id(&self) -> DescriptorId {
let desc = self.to_string();
let desc_without_checksum = desc.split('#').next().expect("Must be here");
let descriptor_bytes = <Vec<u8>>::from(desc_without_checksum.as_bytes());
DescriptorId(sha256::Hash::hash(&descriptor_bytes))
}
}
6 changes: 3 additions & 3 deletions crates/chain/src/indexed_tx_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ use alloc::vec::Vec;
use bitcoin::{Block, OutPoint, Transaction, TxOut, Txid};

use crate::{
keychain,
tx_graph::{self, TxGraph},
Anchor, AnchorFromBlockPosition, Append, BlockId,
};
Expand Down Expand Up @@ -320,8 +319,9 @@ impl<A, IA: Default> From<tx_graph::ChangeSet<A>> for ChangeSet<A, IA> {
}
}

impl<A, K> From<keychain::ChangeSet<K>> for ChangeSet<A, keychain::ChangeSet<K>> {
fn from(indexer: keychain::ChangeSet<K>) -> Self {
#[cfg(feature = "miniscript")]
impl<A, K> From<crate::keychain::ChangeSet<K>> for ChangeSet<A, crate::keychain::ChangeSet<K>> {
fn from(indexer: crate::keychain::ChangeSet<K>) -> Self {
Self {
graph: Default::default(),
indexer,
Expand Down
103 changes: 0 additions & 103 deletions crates/chain/src/keychain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,78 +10,12 @@
//!
//! [`SpkTxOutIndex`]: crate::SpkTxOutIndex

use crate::{collections::BTreeMap, Append};

#[cfg(feature = "miniscript")]
mod txout_index;
use bitcoin::Amount;
#[cfg(feature = "miniscript")]
pub use txout_index::*;

/// Represents updates to the derivation index of a [`KeychainTxOutIndex`].
/// It maps each keychain `K` to its last revealed index.
///
/// It can be applied to [`KeychainTxOutIndex`] with [`apply_changeset`]. [`ChangeSet`]s are
/// monotone in that they will never decrease the revealed derivation index.
///
/// [`KeychainTxOutIndex`]: crate::keychain::KeychainTxOutIndex
/// [`apply_changeset`]: crate::keychain::KeychainTxOutIndex::apply_changeset
#[derive(Clone, Debug, PartialEq)]
#[cfg_attr(
feature = "serde",
derive(serde::Deserialize, serde::Serialize),
serde(
crate = "serde_crate",
bound(
deserialize = "K: Ord + serde::Deserialize<'de>",
serialize = "K: Ord + serde::Serialize"
)
)
)]
#[must_use]
pub struct ChangeSet<K>(pub BTreeMap<K, u32>);

impl<K> ChangeSet<K> {
/// Get the inner map of the keychain to its new derivation index.
pub fn as_inner(&self) -> &BTreeMap<K, u32> {
&self.0
}
}

impl<K: Ord> Append for ChangeSet<K> {
/// Append another [`ChangeSet`] into self.
///
/// If the keychain already exists, increase the index when the other's index > self's index.
/// If the keychain did not exist, append the new keychain.
fn append(&mut self, mut other: Self) {
self.0.iter_mut().for_each(|(key, index)| {
if let Some(other_index) = other.0.remove(key) {
*index = other_index.max(*index);
}
});
// We use `extend` instead of `BTreeMap::append` due to performance issues with `append`.
// Refer to https://github.com/rust-lang/rust/issues/34666#issuecomment-675658420
self.0.extend(other.0);
}

/// Returns whether the changeset are empty.
fn is_empty(&self) -> bool {
self.0.is_empty()
}
}

impl<K> Default for ChangeSet<K> {
fn default() -> Self {
Self(Default::default())
}
}

impl<K> AsRef<BTreeMap<K, u32>> for ChangeSet<K> {
fn as_ref(&self) -> &BTreeMap<K, u32> {
&self.0
}
}

/// Balance, differentiated into various categories.
#[derive(Debug, PartialEq, Eq, Clone, Default)]
#[cfg_attr(
Expand Down Expand Up @@ -137,40 +71,3 @@ impl core::ops::Add for Balance {
}
}
}

#[cfg(test)]
mod test {
use super::*;

#[test]
fn append_keychain_derivation_indices() {
#[derive(Ord, PartialOrd, Eq, PartialEq, Clone, Debug)]
enum Keychain {
One,
Two,
Three,
Four,
}
let mut lhs_di = BTreeMap::<Keychain, u32>::default();
let mut rhs_di = BTreeMap::<Keychain, u32>::default();
lhs_di.insert(Keychain::One, 7);
lhs_di.insert(Keychain::Two, 0);
rhs_di.insert(Keychain::One, 3);
rhs_di.insert(Keychain::Two, 5);
lhs_di.insert(Keychain::Three, 3);
rhs_di.insert(Keychain::Four, 4);

let mut lhs = ChangeSet(lhs_di);
let rhs = ChangeSet(rhs_di);
lhs.append(rhs);

// Exiting index doesn't update if the new index in `other` is lower than `self`.
assert_eq!(lhs.0.get(&Keychain::One), Some(&7));
// Existing index updates if the new index in `other` is higher than `self`.
assert_eq!(lhs.0.get(&Keychain::Two), Some(&5));
// Existing index is unchanged if keychain doesn't exist in `other`.
assert_eq!(lhs.0.get(&Keychain::Three), Some(&3));
// New keychain gets added if the keychain is in `other` but not in `self`.
assert_eq!(lhs.0.get(&Keychain::Four), Some(&4));
}
}
Loading
Loading