Skip to content

Commit

Permalink
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
  • Loading branch information
f3r10 committed Jan 16, 2025
1 parent 91d7d3c commit 2582e02
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 2 deletions.
42 changes: 40 additions & 2 deletions crates/chain/src/indexer/keychain_txout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,17 @@ use crate::{
DescriptorExt, DescriptorId, Indexed, Indexer, KeychainIndexed, SpkIterator,
};
use alloc::{borrow::ToOwned, vec::Vec};
use bitcoin::{Amount, OutPoint, ScriptBuf, SignedAmount, Transaction, TxOut, Txid};
use bitcoin::{
bip32::ChildNumber, Amount, OutPoint, ScriptBuf, SignedAmount, Transaction, TxOut, Txid,
};
use core::{
fmt::Debug,
ops::{Bound, RangeBounds},
};
use miniscript::{
descriptor::{DescriptorXKey, Wildcard},
ForEachKey,
};

use crate::Merge;

Expand Down Expand Up @@ -355,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)
Expand Down Expand Up @@ -776,7 +799,7 @@ impl<K: Clone + Ord + Debug> KeychainTxOutIndex<K> {
}
}

#[derive(Clone, Debug, PartialEq)]
#[derive(Debug, PartialEq)]
/// Error returned from [`KeychainTxOutIndex::insert_descriptor`]
pub enum InsertDescriptorError<K> {
/// The descriptor has already been assigned to a keychain so you can't assign it to another
Expand All @@ -793,6 +816,16 @@ pub enum InsertDescriptorError<K> {
/// The descriptor that the keychain is already assigned to
existing_assignment: Descriptor<DescriptorPublicKey>,
},
/// Miniscript error
Miniscript(miniscript::Error),
/// The descriptor contains hardened derivation steps on public extended keys
HardenedDerivationXpub,
}

impl<K> From<miniscript::Error> for InsertDescriptorError<K> {
fn from(err: miniscript::Error) -> Self {
InsertDescriptorError::Miniscript(err)
}
}

impl<K: core::fmt::Debug> core::fmt::Display for InsertDescriptorError<K> {
Expand All @@ -816,6 +849,11 @@ impl<K: core::fmt::Debug> core::fmt::Display for InsertDescriptorError<K> {
"attempt to re-assign keychain {keychain:?} already assigned to {existing:?}"
)
}
InsertDescriptorError::Miniscript(err) => write!(f, "Miniscript error: {}", err),
InsertDescriptorError::HardenedDerivationXpub => write!(
f,
"The descriptor contains hardened derivation steps on public extended keys"
),
}
}
}
Expand Down
13 changes: 13 additions & 0 deletions crates/chain/tests/test_keychain_txout_index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
Expand Down
6 changes: 6 additions & 0 deletions crates/wallet/src/wallet/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2530,6 +2530,12 @@ fn create_indexer(
InsertDescriptorError::KeychainAlreadyAssigned { .. } => {
unreachable!("this is the first time we're assigning internal")
}
InsertDescriptorError::Miniscript(err) => {
crate::descriptor::error::Error::Miniscript(err)
}
InsertDescriptorError::HardenedDerivationXpub => {
crate::descriptor::error::Error::HardenedDerivationXpub
}
}
})?);
}
Expand Down

0 comments on commit 2582e02

Please sign in to comment.