diff --git a/crates/chain/src/indexed_tx_graph.rs b/crates/chain/src/indexed_tx_graph.rs index 673cb203e..fc563e4cf 100644 --- a/crates/chain/src/indexed_tx_graph.rs +++ b/crates/chain/src/indexed_tx_graph.rs @@ -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, }; @@ -46,8 +47,11 @@ impl IndexedTxGraph { impl IndexedTxGraph { /// Applies the [`ChangeSet`] to the [`IndexedTxGraph`]. - pub fn apply_changeset(&mut self, changeset: ChangeSet) { - self.index.apply_changeset(changeset.indexer); + pub fn apply_changeset( + &mut self, + changeset: ChangeSet, + ) -> Result<(), InsertDescriptorError> { + self.index.apply_changeset(changeset.indexer)?; for tx in &changeset.tx_graph.txs { self.index.index_tx(tx); @@ -57,6 +61,7 @@ impl IndexedTxGraph { } self.graph.apply_changeset(changeset.tx_graph); + Ok(()) } /// Determines the [`ChangeSet`] between `self` and an empty [`IndexedTxGraph`]. diff --git a/crates/chain/src/indexer.rs b/crates/chain/src/indexer.rs index 22e839815..a58adc105 100644 --- a/crates/chain/src/indexer.rs +++ b/crates/chain/src/indexer.rs @@ -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; @@ -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; diff --git a/crates/chain/src/indexer/keychain_txout.rs b/crates/chain/src/indexer/keychain_txout.rs index a3e33415e..30a162852 100644 --- a/crates/chain/src/indexer/keychain_txout.rs +++ b/crates/chain/src/indexer/keychain_txout.rs @@ -176,7 +176,7 @@ impl Indexer for KeychainTxOutIndex { } } - fn apply_changeset(&mut self, changeset: Self::ChangeSet) { + fn apply_changeset(&mut self, changeset: Self::ChangeSet) -> Result<(), InsertDescriptorError> { self.apply_changeset(changeset) } @@ -790,18 +790,22 @@ impl KeychainTxOutIndex { } /// Applies the `ChangeSet` to the [`KeychainTxOutIndex`] - 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 { +pub enum InsertDescriptorError { /// 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 @@ -820,6 +824,8 @@ pub enum InsertDescriptorError { 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 From for InsertDescriptorError { @@ -854,6 +860,9 @@ impl core::fmt::Display for InsertDescriptorError { 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") + } } } } diff --git a/crates/chain/src/indexer/spk_txout.rs b/crates/chain/src/indexer/spk_txout.rs index 286e5d2dc..349d096d9 100644 --- a/crates/chain/src/indexer/spk_txout.rs +++ b/crates/chain/src/indexer/spk_txout.rs @@ -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 @@ -69,8 +71,12 @@ impl Indexer for SpkTxOutIndex { 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 { diff --git a/crates/chain/tests/test_keychain_txout_index.rs b/crates/chain/tests/test_keychain_txout_index.rs index 8bd3c0a10..ed60d332b 100644 --- a/crates/chain/tests/test_keychain_txout_index.rs +++ b/crates/chain/tests/test_keychain_txout_index.rs @@ -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::::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]); @@ -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::::new(0); @@ -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::>(), diff --git a/crates/wallet/src/descriptor/error.rs b/crates/wallet/src/descriptor/error.rs index e018b5352..b8b9f6638 100644 --- a/crates/wallet/src/descriptor/error.rs +++ b/crates/wallet/src/descriptor/error.rs @@ -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 for Error { @@ -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") + } } } } diff --git a/crates/wallet/src/wallet/mod.rs b/crates/wallet/src/wallet/mod.rs index 537c5fe8a..98c853c5a 100644 --- a/crates/wallet/src/wallet/mod.rs +++ b/crates/wallet/src/wallet/mod.rs @@ -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(); @@ -2536,6 +2548,9 @@ fn create_indexer( InsertDescriptorError::HardenedDerivationXpub => { crate::descriptor::error::Error::HardenedDerivationXpub } + InsertDescriptorError::OutOfBounds => { + crate::descriptor::error::Error::IndexDescriptorOutOfBounds + } } })?); } diff --git a/example-crates/example_cli/src/lib.rs b/example-crates/example_cli/src/lib.rs index 6a97252fc..86a1f26da 100644 --- a/example-crates/example_cli/src/lib.rs +++ b/example-crates/example_cli/src/lib.rs @@ -827,7 +827,7 @@ pub fn init_or_load( graph.apply_changeset(indexed_tx_graph::ChangeSet { tx_graph: changeset.tx_graph, indexer: changeset.indexer, - }); + })?; graph });