Skip to content

Commit

Permalink
fix(chain): handle changeset where the index is greater that BIP32_MA…
Browse files Browse the repository at this point in the history
…X_INDEX
  • Loading branch information
f3r10 committed Jan 19, 2025
1 parent 2582e02 commit a3f087c
Show file tree
Hide file tree
Showing 8 changed files with 68 additions and 12 deletions.
9 changes: 7 additions & 2 deletions crates/chain/src/indexed_tx_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ 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,
};
Expand Down Expand Up @@ -46,8 +47,11 @@ 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>) {
self.index.apply_changeset(changeset.indexer);
pub fn apply_changeset(
&mut self,
changeset: ChangeSet<A, I::ChangeSet>,
) -> Result<(), InsertDescriptorError> {
self.index.apply_changeset(changeset.indexer)?;

for tx in &changeset.tx_graph.txs {
self.index.index_tx(tx);
Expand All @@ -57,6 +61,7 @@ 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`].
Expand Down
3 changes: 2 additions & 1 deletion crates/chain/src/indexer.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
//! [`Indexer`] provides utilities for indexing transaction data.
use bitcoin::{OutPoint, Transaction, TxOut};
use keychain_txout::InsertDescriptorError;

#[cfg(feature = "miniscript")]
pub mod keychain_txout;
Expand All @@ -23,7 +24,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);
fn apply_changeset(&mut self, changeset: Self::ChangeSet) -> Result<(), InsertDescriptorError>;

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

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

Expand Down Expand Up @@ -790,18 +790,22 @@ impl<K: Clone + Ord + Debug> KeychainTxOutIndex<K> {
}

/// Applies the `ChangeSet<K>` to the [`KeychainTxOutIndex<K>`]
pub fn apply_changeset(&mut self, changeset: ChangeSet) {
pub fn apply_changeset(&mut self, changeset: ChangeSet) -> Result<(), InsertDescriptorError> {
for (&desc_id, &index) in &changeset.last_revealed {
let v = self.last_revealed.entry(desc_id).or_default();
if index > BIP32_MAX_INDEX {
return Err(InsertDescriptorError::OutOfBounds);
}
*v = index.max(*v);
self.replenish_inner_index_did(desc_id, self.lookahead);
}
Ok(())
}
}

#[derive(Debug, PartialEq)]
/// Error returned from [`KeychainTxOutIndex::insert_descriptor`]
pub enum InsertDescriptorError<K> {
pub enum InsertDescriptorError<K = ()> {
/// The descriptor has already been assigned to a keychain so you can't assign it to another
DescriptorAlreadyAssigned {
/// The descriptor you have attempted to reassign
Expand All @@ -820,6 +824,8 @@ pub enum InsertDescriptorError<K> {
Miniscript(miniscript::Error),
/// The descriptor contains hardened derivation steps on public extended keys
HardenedDerivationXpub,
/// The last revealed index of derivation is out of bounds
OutOfBounds,
}

impl<K> From<miniscript::Error> for InsertDescriptorError<K> {
Expand Down Expand Up @@ -854,6 +860,9 @@ impl<K: core::fmt::Debug> core::fmt::Display for InsertDescriptorError<K> {
f,
"The descriptor contains hardened derivation steps on public extended keys"
),
InsertDescriptorError::OutOfBounds => {
write!(f, "The last revealed index of derivation is out of bounds")
}
}
}
}
Expand Down
8 changes: 7 additions & 1 deletion crates/chain/src/indexer/spk_txout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ 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
Expand Down Expand Up @@ -69,8 +71,12 @@ 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) {
fn apply_changeset(
&mut self,
_changeset: Self::ChangeSet,
) -> Result<(), InsertDescriptorError> {
// This applies nothing.
Ok(())
}

fn is_tx_relevant(&self, tx: &Transaction) -> bool {
Expand Down
19 changes: 17 additions & 2 deletions crates/chain/tests/test_keychain_txout_index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -613,6 +613,17 @@ fn insert_descriptor_should_reject_hardened_steps() {
assert!(indexer.insert_descriptor("keychain", desc).is_err())
}

#[test]
fn apply_changesets_should_reject_invalid_index() {
let desc = parse_descriptor(DESCRIPTORS[3]);
let changeset: ChangeSet = ChangeSet {
last_revealed: [(desc.descriptor_id(), bdk_chain::BIP32_MAX_INDEX + 1)].into(),
};

let mut indexer_a = KeychainTxOutIndex::<TestKeychain>::new(0);
assert!(indexer_a.apply_changeset(changeset.clone()).is_err())
}

#[test]
fn applying_changesets_one_by_one_vs_aggregate_must_have_same_result() {
let desc = parse_descriptor(DESCRIPTORS[0]);
Expand All @@ -630,7 +641,9 @@ 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());
indexer_a
.apply_changeset(changeset.clone())
.expect("must apply_changeset");
}

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

assert_eq!(
indexer_a.keychains().collect::<Vec<_>>(),
Expand Down
5 changes: 5 additions & 0 deletions crates/wallet/src/descriptor/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ pub enum Error {
Hex(bitcoin::hex::HexToBytesError),
/// The provided wallet descriptors are identical
ExternalAndInternalAreTheSame,
/// The last revealed index of derivation is out of bounds
IndexDescriptorOutOfBounds,
}

impl From<crate::keys::KeyError> for Error {
Expand Down Expand Up @@ -83,6 +85,9 @@ impl fmt::Display for Error {
Self::ExternalAndInternalAreTheSame => {
write!(f, "External and internal descriptors are the same")
}
Self::IndexDescriptorOutOfBounds => {
write!(f, "The last revealed index of derivation is out of bounds")
}
}
}
}
Expand Down
19 changes: 17 additions & 2 deletions crates/wallet/src/wallet/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -608,8 +608,20 @@ impl Wallet {
.map_err(LoadError::Descriptor)?;

let mut indexed_graph = IndexedTxGraph::new(index);
indexed_graph.apply_changeset(changeset.indexer.into());
indexed_graph.apply_changeset(changeset.tx_graph.into());
indexed_graph
.apply_changeset(changeset.indexer.into())
.map_err(|_| {
LoadError::Descriptor(
crate::descriptor::DescriptorError::IndexDescriptorOutOfBounds,
)
})?;
indexed_graph
.apply_changeset(changeset.tx_graph.into())
.map_err(|_| {
LoadError::Descriptor(
crate::descriptor::DescriptorError::IndexDescriptorOutOfBounds,
)
})?;

let stage = ChangeSet::default();

Expand Down Expand Up @@ -2536,6 +2548,9 @@ fn create_indexer(
InsertDescriptorError::HardenedDerivationXpub => {
crate::descriptor::error::Error::HardenedDerivationXpub
}
InsertDescriptorError::OutOfBounds => {
crate::descriptor::error::Error::IndexDescriptorOutOfBounds
}
}
})?);
}
Expand Down
2 changes: 1 addition & 1 deletion example-crates/example_cli/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
});

Expand Down

0 comments on commit a3f087c

Please sign in to comment.