Merge #302: refactor: have a structured IndexOutOfBoundsError type

42d9b90 refactor!: have a structured IndexOutOfBoundsError type (chaitika)

Pull request description:

  ### Description
  Fixes #267

  ### Notes to the reviewers
  I can rebase to master after #298 is merged to fix fmt errors.

  ### Changelog notice

  ```md
  Added
  - newtype `IndexOutOfBoundsError`

  Changed
  - `PolicyError::IndexOutOfRange` now wraps a `IndexOutOfBoundsError`
  - `SignerError::InputIndexOutOfRange` now wraps a `IndexOutOfBoundsError`
  ```

  ### Checklists

  #### All Submissions:

  * [X] I've signed all my commits
  * [X] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
  * [X] I ran `just p` before pushing

  #### New Features:

  * [ ] I've added tests for the new feature
  * [ ] I've added docs for the new feature

  #### Bugfixes:

  * [x] This pull request breaks the existing API
  * [ ] I've added tests to reproduce the issue which are now passing
  * [X] I'm linking the issue being fixed by this PR

ACKs for top commit:
  ValuedMammal:
    ACK 42d9b90

Tree-SHA512: 1ea31ff611f4ab03aba4b01ee898a3bbb2ef9393554408ccc5f70a2f04ec73e26b7654455e935537ef2c63d0cc5acf4330cfd964608462568d84f0b43147013a

Author: ValuedMammal

wallet/src/descriptor/policy.rs

@@ -61,6 +61,7 @@ use miniscript::{
 
 use crate::descriptor::ExtractPolicy;
 use crate::keys::ExtScriptContext;
+use crate::types::IndexOutOfBoundsError;
 use crate::wallet::signer::{SignerId, SignersContainer};
 use crate::wallet::utils::{After, Older, SecpCtx};
 
@@ -324,7 +325,7 @@ impl Satisfaction {
                 ..
             } => {
                 if inner_index >= *n || items.contains(&inner_index) {
-                    return Err(PolicyError::IndexOutOfRange(inner_index));
+                    return Err(IndexOutOfBoundsError::new(inner_index, *n))?;
                 }
 
                 match inner {
@@ -514,7 +515,7 @@ pub enum PolicyError {
     NotEnoughItemsSelected(String),
     /// Index out of range for an item to satisfy a [`SatisfiableItem::Thresh`] or a
     /// [`SatisfiableItem::Multisig`]
-    IndexOutOfRange(usize),
+    IndexOutOfRange(IndexOutOfBoundsError),
     /// Can not add to an item that is [`Satisfaction::None`] or [`Satisfaction::Complete`]
     AddOnLeaf,
     /// Can not add to an item that is [`Satisfaction::PartialComplete`]
@@ -530,7 +531,7 @@ impl fmt::Display for PolicyError {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match self {
             Self::NotEnoughItemsSelected(err) => write!(f, "Not enough items selected: {err}"),
-            Self::IndexOutOfRange(index) => write!(f, "Index out of range: {index}"),
+            Self::IndexOutOfRange(err) => write!(f, "{err}"),
             Self::AddOnLeaf => write!(f, "Add on leaf"),
             Self::AddOnPartialComplete => write!(f, "Add on partial complete"),
             Self::MixedTimelockUnits => write!(f, "Mixed timelock units"),
@@ -539,6 +540,12 @@ impl fmt::Display for PolicyError {
     }
 }
 
+impl From<IndexOutOfBoundsError> for PolicyError {
+    fn from(err: IndexOutOfBoundsError) -> Self {
+        Self::IndexOutOfRange(err)
+    }
+}
+
 #[cfg(feature = "std")]
 impl std::error::Error for PolicyError {}
 
@@ -695,7 +702,7 @@ impl Policy {
                 // make sure all the indexes in the `selected` list are within range
                 for index in &selected {
                     if *index >= items.len() {
-                        return Err(PolicyError::IndexOutOfRange(*index));
+                        return Err(IndexOutOfBoundsError::new(*index, items.len()))?;
                     }
                 }
 
@@ -718,7 +725,7 @@ impl Policy {
                     return Err(PolicyError::NotEnoughItemsSelected(self.id.clone()));
                 }
                 if let Some(item) = selected.into_iter().find(|&i| i >= keys.len()) {
-                    return Err(PolicyError::IndexOutOfRange(item));
+                    return Err(IndexOutOfBoundsError::new(item, keys.len()))?;
                 }
 
                 Ok(Condition::default())
@@ -1570,7 +1577,12 @@ mod test {
         // index out of range
         let out_of_range =
             policy.get_condition(&vec![(policy.id.clone(), vec![5])].into_iter().collect());
-        assert_eq!(out_of_range, Err(PolicyError::IndexOutOfRange(5)));
+        assert_eq!(
+            out_of_range,
+            Err(PolicyError::IndexOutOfRange(IndexOutOfBoundsError::new(
+                5, 2
+            )))
+        );
     }
 
     const ALICE_TPRV_STR:&str = "tprv8ZgxMBicQKsPf6T5X327efHnvJDr45Xnb8W4JifNWtEoqXu9MRYS4v1oYe6DFcMVETxy5w3bqpubYRqvcVTqovG1LifFcVUuJcbwJwrhYzP";

wallet/src/types.rs

@@ -144,3 +144,32 @@ impl Utxo {
         }
     }
 }
+
+/// Index out of bounds error.
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub struct IndexOutOfBoundsError {
+    /// The index that is out of range.
+    pub index: usize,
+    /// The length of the container.
+    pub len: usize,
+}
+
+impl IndexOutOfBoundsError {
+    /// Create a new `IndexOutOfBoundsError`.
+    pub fn new(index: usize, len: usize) -> Self {
+        Self { index, len }
+    }
+}
+
+impl fmt::Display for IndexOutOfBoundsError {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(
+            f,
+            "Index out of bounds: index {} is greater than or equal to length {}",
+            self.index, self.len
+        )
+    }
+}
+
+#[cfg(feature = "std")]
+impl std::error::Error for IndexOutOfBoundsError {}

wallet/src/wallet/mod.rs

@@ -1999,7 +1999,7 @@ impl Wallet {
             let psbt_input = &psbt
                 .inputs
                 .get(n)
-                .ok_or(SignerError::InputIndexOutOfRange)?;
+                .ok_or(IndexOutOfBoundsError::new(n, psbt.inputs.len()))?;
             if psbt_input.final_script_sig.is_some() || psbt_input.final_script_witness.is_some() {
                 continue;
             }
@@ -2037,12 +2037,13 @@ impl Wallet {
                         ),
                     ) {
                         Ok(_) => {
+                            let length = psbt.inputs.len();
                             // Set the UTXO fields, final script_sig and witness
                             // and clear everything else.
                             let psbt_input = psbt
                                 .inputs
                                 .get_mut(n)
-                                .ok_or(SignerError::InputIndexOutOfRange)?;
+                                .ok_or(IndexOutOfBoundsError::new(n, length))?;
                             let original = mem::take(psbt_input);
                             psbt_input.non_witness_utxo = original.non_witness_utxo;
                             psbt_input.witness_utxo = original.witness_utxo;

wallet/src/wallet/signer.rs

@@ -106,6 +106,7 @@ use miniscript::{SigType, ToPublicKey};
 use super::utils::SecpCtx;
 use crate::descriptor::{DescriptorMeta, XKeyUtils};
 use crate::psbt::PsbtUtils;
+use crate::types::IndexOutOfBoundsError;
 use crate::wallet::error::MiniscriptPsbtError;
 
 /// Identifier of a signer in the `SignersContainers`. Used as a key to find the right signer among
@@ -142,7 +143,7 @@ pub enum SignerError {
     /// The user canceled the operation
     UserCanceled,
     /// Input index is out of range
-    InputIndexOutOfRange,
+    InputIndexOutOfRange(IndexOutOfBoundsError),
     /// The `non_witness_utxo` field of the transaction is required to sign this input
     MissingNonWitnessUtxo,
     /// The `non_witness_utxo` specified is invalid
@@ -179,7 +180,7 @@ impl fmt::Display for SignerError {
             Self::MissingKey => write!(f, "Missing private key"),
             Self::InvalidKey => write!(f, "The private key in use has the right fingerprint but derives differently than expected"),
             Self::UserCanceled => write!(f, "The user canceled the operation"),
-            Self::InputIndexOutOfRange => write!(f, "Input index out of range"),
+            Self::InputIndexOutOfRange(err) => write!(f, "{err}"),
             Self::MissingNonWitnessUtxo => write!(f, "Missing non-witness UTXO"),
             Self::InvalidNonWitnessUtxo => write!(f, "Invalid non-witness UTXO"),
             Self::MissingWitnessUtxo => write!(f, "Missing witness UTXO"),
@@ -195,6 +196,12 @@ impl fmt::Display for SignerError {
     }
 }
 
+impl From<IndexOutOfBoundsError> for SignerError {
+    fn from(err: IndexOutOfBoundsError) -> Self {
+        Self::InputIndexOutOfRange(err)
+    }
+}
+
 #[cfg(feature = "std")]
 impl std::error::Error for SignerError {}
 
@@ -318,7 +325,7 @@ impl InputSigner for SignerWrapper<DescriptorXKey<Xpriv>> {
         secp: &SecpCtx,
     ) -> Result<(), SignerError> {
         if input_index >= psbt.inputs.len() {
-            return Err(SignerError::InputIndexOutOfRange);
+            return Err(IndexOutOfBoundsError::new(input_index, psbt.inputs.len()))?;
         }
 
         if psbt.inputs[input_index].final_script_sig.is_some()
@@ -442,8 +449,14 @@ impl InputSigner for SignerWrapper<PrivateKey> {
         sign_options: &SignOptions,
         secp: &SecpCtx,
     ) -> Result<(), SignerError> {
-        if input_index >= psbt.inputs.len() || input_index >= psbt.unsigned_tx.input.len() {
-            return Err(SignerError::InputIndexOutOfRange);
+        if input_index >= psbt.inputs.len() {
+            return Err(IndexOutOfBoundsError::new(input_index, psbt.inputs.len()))?;
+        }
+        if input_index >= psbt.unsigned_tx.input.len() {
+            return Err(IndexOutOfBoundsError::new(
+                input_index,
+                psbt.unsigned_tx.input.len(),
+            ))?;
         }
 
         if psbt.inputs[input_index].final_script_sig.is_some()
@@ -834,8 +847,14 @@ fn compute_tap_sighash(
     input_index: usize,
     extra: Option<taproot::TapLeafHash>,
 ) -> Result<(sighash::TapSighash, TapSighashType), SignerError> {
-    if input_index >= psbt.inputs.len() || input_index >= psbt.unsigned_tx.input.len() {
-        return Err(SignerError::InputIndexOutOfRange);
+    if input_index >= psbt.inputs.len() {
+        Err(IndexOutOfBoundsError::new(input_index, psbt.inputs.len()))?;
+    }
+    if input_index >= psbt.unsigned_tx.input.len() {
+        Err(IndexOutOfBoundsError::new(
+            input_index,
+            psbt.unsigned_tx.input.len(),
+        ))?;
     }
 
     let psbt_input = &psbt.inputs[input_index];