Skip to content

Commit

Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
fix(chain): check/sanitize descriptor has not hardened child steps be…
Browse files Browse the repository at this point in the history
…fore insert it
f3r10 committed Jan 16, 2025

Verified

This commit was signed with the committer’s verified signature.
f3r10 Fernando Ledesma
1 parent d3232f5 commit 64a0976
Showing 7 changed files with 42 additions and 54 deletions.
9 changes: 2 additions & 7 deletions crates/chain/src/indexed_tx_graph.rs
Original file line number Diff line number Diff line change
@@ -6,7 +6,6 @@ use alloc::{sync::Arc, vec::Vec};
use bitcoin::{Block, OutPoint, Transaction, TxOut, Txid};

use crate::{
keychain_txout::InsertDescriptorError,
tx_graph::{self, TxGraph},
Anchor, BlockId, Indexer, Merge, TxPosInBlock,
};
@@ -47,11 +46,8 @@ impl<A, I> IndexedTxGraph<A, I> {

impl<A: Anchor, I: Indexer> IndexedTxGraph<A, I> {
/// Applies the [`ChangeSet`] to the [`IndexedTxGraph`].
pub fn apply_changeset(
&mut self,
changeset: ChangeSet<A, I::ChangeSet>,
) -> Result<(), InsertDescriptorError> {
self.index.apply_changeset(changeset.indexer)?;
pub fn apply_changeset(&mut self, changeset: ChangeSet<A, I::ChangeSet>) {
self.index.apply_changeset(changeset.indexer);

for tx in &changeset.tx_graph.txs {
self.index.index_tx(tx);
@@ -61,7 +57,6 @@ impl<A: Anchor, I: Indexer> IndexedTxGraph<A, I> {
}

self.graph.apply_changeset(changeset.tx_graph);
Ok(())
}

/// Determines the [`ChangeSet`] between `self` and an empty [`IndexedTxGraph`].
5 changes: 1 addition & 4 deletions crates/chain/src/indexer.rs
Original file line number Diff line number Diff line change
@@ -23,10 +23,7 @@ pub trait Indexer {
fn index_tx(&mut self, tx: &Transaction) -> Self::ChangeSet;

/// Apply changeset to itself.
fn apply_changeset(
&mut self,
changeset: Self::ChangeSet,
) -> Result<(), keychain_txout::InsertDescriptorError>;
fn apply_changeset(&mut self, changeset: Self::ChangeSet);

/// Determines the [`ChangeSet`](Indexer::ChangeSet) between `self` and an empty [`Indexer`].
fn initial_changeset(&self) -> Self::ChangeSet;
43 changes: 20 additions & 23 deletions crates/chain/src/indexer/keychain_txout.rs
Original file line number Diff line number Diff line change
@@ -176,7 +176,7 @@ impl<K: Clone + Ord + Debug> Indexer for KeychainTxOutIndex<K> {
}
}

fn apply_changeset(&mut self, changeset: Self::ChangeSet) -> Result<(), InsertDescriptorError> {
fn apply_changeset(&mut self, changeset: Self::ChangeSet) {
self.apply_changeset(changeset)
}

@@ -361,6 +361,23 @@ impl<K: Clone + Ord + Debug> KeychainTxOutIndex<K> {
keychain: K,
descriptor: Descriptor<DescriptorPublicKey>,
) -> Result<bool, InsertDescriptorError<K>> {
// Ensure the keys don't contain any hardened derivation steps or hardened wildcards
let descriptor_contains_hardened_steps = descriptor.for_any_key(|k| {
if let DescriptorPublicKey::XPub(DescriptorXKey {
derivation_path,
wildcard,
..
}) = k
{
return *wildcard == Wildcard::Hardened
|| derivation_path.into_iter().any(ChildNumber::is_hardened);
}
false
});
if descriptor_contains_hardened_steps {
return Err(InsertDescriptorError::HardenedDerivationXpub);
}
descriptor.sanity_check()?;
let did = descriptor.descriptor_id();
if !self.keychain_to_descriptor_id.contains_key(&keychain)
&& !self.descriptor_id_to_keychain.contains_key(&did)
@@ -773,32 +790,12 @@ impl<K: Clone + Ord + Debug> KeychainTxOutIndex<K> {
}

/// Applies the `ChangeSet<K>` to the [`KeychainTxOutIndex<K>`]
pub fn apply_changeset(&mut self, changeset: ChangeSet) -> Result<(), InsertDescriptorError> {
pub fn apply_changeset(&mut self, changeset: ChangeSet) {
for (&desc_id, &index) in &changeset.last_revealed {
let descriptor = self.descriptors.get(&desc_id).expect("invariant");
// Ensure the keys don't contain any hardened derivation steps or hardened wildcards
let descriptor_contains_hardened_steps = descriptor.for_any_key(|k| {
if let DescriptorPublicKey::XPub(DescriptorXKey {
derivation_path,
wildcard,
..
}) = k
{
return *wildcard == Wildcard::Hardened
|| derivation_path.into_iter().any(ChildNumber::is_hardened);
}

false
});
if descriptor_contains_hardened_steps {
return Err(InsertDescriptorError::HardenedDerivationXpub);
}
descriptor.sanity_check()?;
let v = self.last_revealed.entry(desc_id).or_default();
*v = index.max(*v);
self.replenish_inner_index_did(desc_id, self.lookahead);
}
Ok(())
}
}

@@ -825,7 +822,7 @@ pub enum InsertDescriptorError<K = ()> {
HardenedDerivationXpub,
}

impl From<miniscript::Error> for InsertDescriptorError {
impl<K> From<miniscript::Error> for InsertDescriptorError<K> {
fn from(err: miniscript::Error) -> Self {
InsertDescriptorError::Miniscript(err)
}
8 changes: 1 addition & 7 deletions crates/chain/src/indexer/spk_txout.rs
Original file line number Diff line number Diff line change
@@ -8,8 +8,6 @@ use crate::{
};
use bitcoin::{Amount, OutPoint, ScriptBuf, SignedAmount, Transaction, TxOut, Txid};

use super::keychain_txout::InsertDescriptorError;

/// An index storing [`TxOut`]s that have a script pubkey that matches those in a list.
///
/// The basic idea is that you insert script pubkeys you care about into the index with
@@ -71,12 +69,8 @@ impl<I: Clone + Ord + core::fmt::Debug> Indexer for SpkTxOutIndex<I> {

fn initial_changeset(&self) -> Self::ChangeSet {}

fn apply_changeset(
&mut self,
_changeset: Self::ChangeSet,
) -> Result<(), InsertDescriptorError> {
fn apply_changeset(&mut self, _changeset: Self::ChangeSet) {
// This applies nothing.
Ok(())
}

fn is_tx_relevant(&self, tx: &Transaction) -> bool {
21 changes: 15 additions & 6 deletions crates/chain/tests/test_keychain_txout_index.rs
Original file line number Diff line number Diff line change
@@ -600,6 +600,19 @@ fn lookahead_to_target() {
}
}

#[test]
fn insert_descriptor_should_reject_hardened_steps() {
use bdk_chain::keychain_txout::KeychainTxOutIndex;

// This descriptor has hardened child steps
let s = "wpkh([e273fe42/84h/1h/0h/0h]tpubDEBY7DLZ5kK6pMC2qdtVvKpe6CiAmVDx1SiLtLS3V4GAGyRvsuVbCYReJ9oQz1rfMapghJyUAYLqkqJ3Xadp3GSTCtdAFcKPgzAXC1hzz8a/*h)";
let (desc, _) =
<Descriptor<DescriptorPublicKey>>::parse_descriptor(&Secp256k1::new(), s).unwrap();

let mut indexer = KeychainTxOutIndex::<&str>::new(10);
assert!(indexer.insert_descriptor("keychain", desc).is_err())
}

#[test]
fn applying_changesets_one_by_one_vs_aggregate_must_have_same_result() {
let desc = parse_descriptor(DESCRIPTORS[0]);
@@ -617,9 +630,7 @@ fn applying_changesets_one_by_one_vs_aggregate_must_have_same_result() {
.insert_descriptor(TestKeychain::External, desc.clone())
.expect("must insert keychain");
for changeset in changesets {
indexer_a
.apply_changeset(changeset.clone())
.expect("must apply changeset");
indexer_a.apply_changeset(changeset.clone());
}

let mut indexer_b = KeychainTxOutIndex::<TestKeychain>::new(0);
@@ -634,9 +645,7 @@ fn applying_changesets_one_by_one_vs_aggregate_must_have_same_result() {
agg
})
.expect("must aggregate changesets");
indexer_b
.apply_changeset(aggregate_changesets)
.expect("must apply changeset");
indexer_b.apply_changeset(aggregate_changesets);

assert_eq!(
indexer_a.keychains().collect::<Vec<_>>(),
8 changes: 2 additions & 6 deletions crates/wallet/src/wallet/mod.rs
Original file line number Diff line number Diff line change
@@ -608,12 +608,8 @@ impl Wallet {
.map_err(LoadError::Descriptor)?;

let mut indexed_graph = IndexedTxGraph::new(index);
indexed_graph
.apply_changeset(changeset.indexer.into())
.map_err(|err| LoadError::Descriptor(err.into()))?;
indexed_graph
.apply_changeset(changeset.tx_graph.into())
.map_err(|err| LoadError::Descriptor(err.into()))?;
indexed_graph.apply_changeset(changeset.indexer.into());
indexed_graph.apply_changeset(changeset.tx_graph.into());

let stage = ChangeSet::default();

2 changes: 1 addition & 1 deletion example-crates/example_cli/src/lib.rs
Original file line number Diff line number Diff line change
@@ -827,7 +827,7 @@ pub fn init_or_load<CS: clap::Subcommand, S: clap::Args>(
graph.apply_changeset(indexed_tx_graph::ChangeSet {
tx_graph: changeset.tx_graph,
indexer: changeset.indexer,
})?;
});
graph
});

0 comments on commit 64a0976

Please sign in to comment.