bip32: return error when trying to derive a path too deep

This restores the Result return from derive_xpriv which we had removed
in a previous PR, but such is life.

Fixes #4308

bitcoin/examples/bip32.rs

@@ -38,7 +38,7 @@ fn main() {
 
     // derive child xpub
     let path = "84h/0h/0h".parse::<DerivationPath>().unwrap();
-    let child = root.derive_xpriv(&secp, &path);
+    let child = root.derive_xpriv(&secp, &path).expect("only deriving three steps");
     println!("Child at {}: {}", path, child);
     let xpub = Xpub::from_xpriv(&secp, &child);
     println!("Public key at {}: {}", path, xpub);

bitcoin/examples/ecdsa-psbt-simple.rs

@@ -62,11 +62,12 @@ fn get_external_address_xpriv<C: Signing>(
 ) -> Xpriv {
     let derivation_path =
         BIP84_DERIVATION_PATH.into_derivation_path().expect("valid derivation path");
-    let child_xpriv = master_xpriv.derive_xpriv(secp, &derivation_path);
+    let child_xpriv =
+        master_xpriv.derive_xpriv(secp, &derivation_path).expect("only deriving three steps");
     let external_index = ChildNumber::ZERO_NORMAL;
     let idx = ChildNumber::from_normal_idx(index).expect("valid index number");
 
-    child_xpriv.derive_xpriv(secp, &[external_index, idx])
+    child_xpriv.derive_xpriv(secp, &[external_index, idx]).expect("only deriving two more steps")
 }
 
 // Derive the internal address xpriv.
@@ -77,11 +78,12 @@ fn get_internal_address_xpriv<C: Signing>(
 ) -> Xpriv {
     let derivation_path =
         BIP84_DERIVATION_PATH.into_derivation_path().expect("valid derivation path");
-    let child_xpriv = master_xpriv.derive_xpriv(secp, &derivation_path);
+    let child_xpriv =
+        master_xpriv.derive_xpriv(secp, &derivation_path).expect("only deriving three steps");
     let internal_index = ChildNumber::ONE_NORMAL;
     let idx = ChildNumber::from_normal_idx(index).expect("valid index number");
 
-    child_xpriv.derive_xpriv(secp, &[internal_index, idx])
+    child_xpriv.derive_xpriv(secp, &[internal_index, idx]).expect("only deriving two more steps")
 }
 
 // The address to send to.

bitcoin/examples/ecdsa-psbt.rs

@@ -118,11 +118,12 @@ impl ColdStorage {
         // Hardened children require secret data to derive.
 
         let path = "84h/0h/0h".into_derivation_path()?;
-        let account_0_xpriv = master_xpriv.derive_xpriv(secp, &path);
+        let account_0_xpriv =
+            master_xpriv.derive_xpriv(secp, &path).expect("derivation path is short");
         let account_0_xpub = Xpub::from_xpriv(secp, &account_0_xpriv);
 
         let path = INPUT_UTXO_DERIVATION_PATH.into_derivation_path()?;
-        let input_xpriv = master_xpriv.derive_xpriv(secp, &path);
+        let input_xpriv = master_xpriv.derive_xpriv(secp, &path).expect("derivation path is short");
         let input_xpub = Xpub::from_xpriv(secp, &input_xpriv);
 
         let wallet = ColdStorage { master_xpriv, master_xpub };

bitcoin/examples/taproot-psbt-simple.rs

@@ -60,11 +60,12 @@ fn get_external_address_xpriv<C: Signing>(
 ) -> Xpriv {
     let derivation_path =
         BIP86_DERIVATION_PATH.into_derivation_path().expect("valid derivation path");
-    let child_xpriv = master_xpriv.derive_xpriv(secp, &derivation_path);
+    let child_xpriv =
+        master_xpriv.derive_xpriv(secp, &derivation_path).expect("only deriving three steps");
     let external_index = ChildNumber::ZERO_NORMAL;
     let idx = ChildNumber::from_normal_idx(index).expect("valid index number");
 
-    child_xpriv.derive_xpriv(secp, &[external_index, idx])
+    child_xpriv.derive_xpriv(secp, &[external_index, idx]).expect("only deriving two more steps")
 }
 
 // Derive the internal address xpriv.
@@ -75,11 +76,12 @@ fn get_internal_address_xpriv<C: Signing>(
 ) -> Xpriv {
     let derivation_path =
         BIP86_DERIVATION_PATH.into_derivation_path().expect("valid derivation path");
-    let child_xpriv = master_xpriv.derive_xpriv(secp, &derivation_path);
+    let child_xpriv =
+        master_xpriv.derive_xpriv(secp, &derivation_path).expect("only deriving three steps");
     let internal_index = ChildNumber::ONE_NORMAL;
     let idx = ChildNumber::from_normal_idx(index).expect("valid index number");
 
-    child_xpriv.derive_xpriv(secp, &[internal_index, idx])
+    child_xpriv.derive_xpriv(secp, &[internal_index, idx]).expect("only deriving two more steps")
 }
 
 // Get the Taproot Key Origin.

bitcoin/examples/taproot-psbt.rs

@@ -299,7 +299,7 @@ fn generate_bip86_key_spend_tx(
                 .ok_or("missing Taproot key origin")?;
 
             let secret_key =
-                master_xpriv.derive_xpriv(secp, &derivation_path).to_private_key().inner;
+                master_xpriv.derive_xpriv(secp, &derivation_path)?.to_private_key().inner;
             sign_psbt_taproot(
                 secret_key,
                 input.tap_internal_key.unwrap(),
@@ -393,8 +393,11 @@ impl BenefactorWallet {
         // We use some other derivation path in this example for our inheritance protocol. The important thing is to ensure
         // that we use an unhardened path so we can make use of xpubs.
         let derivation_path = format!("101/1/0/0/{}", self.next).parse::<DerivationPath>()?;
-        let internal_keypair =
+        let internal_keypair = self
-            self.master_xpriv.derive_xpriv(&self.secp, &derivation_path).to_keypair(&self.secp);
+            .master_xpriv
+            .derive_xpriv(&self.secp, &derivation_path)
+            .expect("derivation path is short")
+            .to_keypair(&self.secp);
         let beneficiary_key =
             self.beneficiary_xpub.derive_xpub(&self.secp, &derivation_path)?.to_x_only_public_key();
 
@@ -486,6 +489,7 @@ impl BenefactorWallet {
             let new_internal_keypair = self
                 .master_xpriv
                 .derive_xpriv(&self.secp, &new_derivation_path)
+                .expect("derivation path is short")
                 .to_keypair(&self.secp);
             let beneficiary_key = self
                 .beneficiary_xpub
@@ -538,6 +542,7 @@ impl BenefactorWallet {
                 let secret_key = self
                     .master_xpriv
                     .derive_xpriv(&self.secp, &derivation_path)
+                    .expect("derivation path is short")
                     .to_private_key()
                     .inner;
                 sign_psbt_taproot(
@@ -660,8 +665,11 @@ impl BeneficiaryWallet {
         for (x_only_pubkey, (leaf_hashes, (_, derivation_path))) in
             &psbt.inputs[0].tap_key_origins.clone()
         {
-            let secret_key =
+            let secret_key = self
-                self.master_xpriv.derive_xpriv(&self.secp, &derivation_path).to_private_key().inner;
+                .master_xpriv
+                .derive_xpriv(&self.secp, &derivation_path)?
+                .to_private_key()
+                .inner;
             for lh in leaf_hashes {
                 let sighash_type = TapSighashType::All;
                 let hash = SighashCache::new(&unsigned_tx).taproot_script_spend_signature_hash(

bitcoin/src/bip32.rs

@@ -185,6 +185,9 @@ impl ChildNumber {
 
     /// Returns the child number that is a single increment from this one.
     pub fn increment(self) -> Result<ChildNumber, IndexOutOfRangeError> {
+        // Bare addition in this function is okay, because we have an invariant that
+        // `index` is always within [0, 2^31 - 1]. FIXME this is not actually an
+        // invariant because the fields are public.
         match self {
             ChildNumber::Normal { index: idx } => ChildNumber::from_normal_idx(idx + 1),
             ChildNumber::Hardened { index: idx } => ChildNumber::from_hardened_idx(idx + 1),
@@ -706,7 +709,7 @@ impl Xpriv {
         &self,
         secp: &Secp256k1<C>,
         path: &P,
-    ) -> Xpriv {
+    ) -> Result<Xpriv, DerivationError> {
         self.derive_xpriv(secp, path)
     }
 
@@ -717,16 +720,20 @@ impl Xpriv {
         &self,
         secp: &Secp256k1<C>,
         path: &P,
-    ) -> Xpriv {
+    ) -> Result<Xpriv, DerivationError> {
         let mut sk: Xpriv = *self;
         for cnum in path.as_ref() {
-            sk = sk.ckd_priv(secp, *cnum)
+            sk = sk.ckd_priv(secp, *cnum)?;
         }
-        sk
+        Ok(sk)
     }
 
     /// Private->Private child key derivation
-    fn ckd_priv<C: secp256k1::Signing>(&self, secp: &Secp256k1<C>, i: ChildNumber) -> Xpriv {
+    fn ckd_priv<C: secp256k1::Signing>(
+        &self,
+        secp: &Secp256k1<C>,
+        i: ChildNumber,
+    ) -> Result<Xpriv, DerivationError> {
         let mut engine = HmacEngine::<sha512::HashEngine>::new(&self.chain_code[..]);
         match i {
             ChildNumber::Normal { .. } => {
@@ -750,14 +757,14 @@ impl Xpriv {
         let tweaked =
             sk.add_tweak(&self.private_key.into()).expect("statistically impossible to hit");
 
-        Xpriv {
+        Ok(Xpriv {
             network: self.network,
-            depth: self.depth + 1,
+            depth: self.depth.checked_add(1).ok_or(DerivationError::MaximumDepthExceeded)?,
             parent_fingerprint: self.fingerprint(secp),
             child_number: i,
             private_key: tweaked,
             chain_code: ChainCode::from_hmac(hmac),
-        }
+        })
     }
 
     /// Decoding extended private key from binary data according to BIP 32
@@ -910,7 +917,7 @@ impl Xpub {
 
         Ok(Xpub {
             network: self.network,
-            depth: self.depth + 1,
+            depth: self.depth.checked_add(1).ok_or(DerivationError::MaximumDepthExceeded)?,
             parent_fingerprint: self.fingerprint(),
             child_number: i,
             public_key: tweaked,
@@ -1188,7 +1195,7 @@ mod tests {
         let mut pk = Xpub::from_xpriv(secp, &sk);
 
         // Check derivation convenience method for Xpriv
-        assert_eq!(&sk.derive_xpriv(secp, &path).to_string()[..], expected_sk);
+        assert_eq!(&sk.derive_xpriv(secp, &path).unwrap().to_string()[..], expected_sk);
 
         // Check derivation convenience method for Xpub, should error
         // appropriately if any ChildNumber is hardened
@@ -1203,7 +1210,7 @@ mod tests {
 
         // Derive keys, checking hardened and non-hardened derivation one-by-one
         for &num in path.0.iter() {
-            sk = sk.ckd_priv(secp, num);
+            sk = sk.ckd_priv(secp, num).unwrap();
             match num {
                 Normal { .. } => {
                     let pk2 = pk.ckd_pub(secp, num).unwrap();

bitcoin/src/psbt/mod.rs

@@ -775,14 +775,14 @@ impl GetKey for Xpriv {
             KeyRequest::XOnlyPubkey(_) => Err(GetKeyError::NotSupported),
             KeyRequest::Bip32((fingerprint, path)) => {
                 let key = if self.fingerprint(secp) == *fingerprint {
-                    let k = self.derive_xpriv(secp, &path);
+                    let k = self.derive_xpriv(secp, &path).map_err(GetKeyError::Bip32)?;
                     Some(k.to_private_key())
                 } else if self.parent_fingerprint == *fingerprint
                     && !path.is_empty()
                     && path[0] == self.child_number
                 {
                     let path = DerivationPath::from_iter(path.into_iter().skip(1).copied());
-                    let k = self.derive_xpriv(secp, &path);
+                    let k = self.derive_xpriv(secp, &path).map_err(GetKeyError::Bip32)?;
                     Some(k.to_private_key())
                 } else {
                     None
@@ -915,8 +915,8 @@ impl_get_key_for_xonly_map!(HashMap);
 #[derive(Debug, Clone, PartialEq, Eq)]
 #[non_exhaustive]
 pub enum GetKeyError {
-    /// A bip32 error.
+    /// A bip32 derivation error.
-    Bip32(bip32::ParseError),
+    Bip32(bip32::DerivationError),
     /// The GetKey operation is not supported for this key request.
     NotSupported,
 }
@@ -930,7 +930,7 @@ impl fmt::Display for GetKeyError {
         use GetKeyError::*;
 
         match *self {
-            Bip32(ref e) => write_err!(f, "a bip23 error"; e),
+            Bip32(ref e) => write_err!(f, "bip32 derivation"; e),
             NotSupported =>
                 f.write_str("the GetKey operation is not supported for this key request"),
         }
@@ -949,10 +949,6 @@ impl std::error::Error for GetKeyError {
     }
 }
 
-impl From<bip32::ParseError> for GetKeyError {
-    fn from(e: bip32::ParseError) -> Self { GetKeyError::Bip32(e) }
-}
-
 /// The various output types supported by the Bitcoin network.
 #[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
 #[non_exhaustive]
@@ -1454,7 +1450,7 @@ mod tests {
             ChildNumber::from_normal_idx(31337).unwrap(),
         ];
 
-        sk = sk.derive_xpriv(secp, &dpath);
+        sk = sk.derive_xpriv(secp, &dpath).unwrap();
 
         let pk = Xpub::from_xpriv(secp, &sk);
 

bitcoin/tests/bip_174.rs

@@ -315,7 +315,8 @@ fn parse_and_verify_keys(
 
         let path =
             derivation_path.into_derivation_path().expect("failed to convert derivation path");
-        let derived_priv = ext_priv.derive_xpriv(secp, &path).to_private_key();
+        let derived_priv =
+            ext_priv.derive_xpriv(secp, &path).expect("derivation path too long").to_private_key();
         assert_eq!(wif_priv, derived_priv);
         let derived_pub = derived_priv.public_key(secp);
         key_map.insert(derived_pub, derived_priv);