fix(chain): handle changeset where the index is greater that BIP32_MAX_INDEX

Author: f3r10

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<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);
@@ -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`].

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;

crates/chain/src/indexer/keychain_txout.rs

@@ -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)
     }
 
@@ -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
@@ -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> {
@@ -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")
+            }
         }
     }
 }

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<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 {

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::<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]);
@@ -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);
@@ -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<_>>(),

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<crate::keys::KeyError> 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")
+            }
         }
     }
 }

crates/wallet/src/wallet/mod.rs

@@ -602,8 +602,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();
 
@@ -2553,6 +2565,9 @@ fn create_indexer(
                     InsertDescriptorError::HardenedDerivationXpub => {
                         crate::descriptor::error::Error::HardenedDerivationXpub
                     }
+                    InsertDescriptorError::OutOfBounds => {
+                        crate::descriptor::error::Error::IndexDescriptorOutOfBounds
+                    }
                 }
             })?);
     }

example-crates/example_cli/src/lib.rs

@@ -815,7 +815,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
             });