Merge #450: Upgrade bitcoin dependency to v0.29.0

24a2498 Depend on bitcoin 0.29.0 (Tobin C. Harding)

Pull request description:

  This is going to need a pretty careful review by @sanket1729 and/or @apoelstra to double check all the sequence/lock_time stuff. Even with the new APIs this stuff is non-trivial. This patch changes the current behaviour around logic for these two types. I am unable to work out if current behaviour has bugs in it or if I just don't fully understand it.

  ### Note

  Please ignore the branch name, it stale now since this PR upgrades to the _newly released_ bitcoin v0.29.1

ACKs for top commit:
  sanket1729:
    ACK 24a2498. Reviewed by range-diff from c51595b .
  apoelstra:
    ACK 24a2498

Tree-SHA512: 54d85d7e8efbeaf82f0b05fa2db8bf004ec1b4b07bfcca95e4a57f611bff44c043b76597c3951ab62ca71ba60f72c3cd9bdb20f78a373874ef974e7ce3970eaa

Author: sanket1729

Cargo.toml

@@ -16,22 +16,22 @@ std = ["bitcoin/std", "bitcoin/secp-recovery"]
 no-std = ["hashbrown", "bitcoin/no-std"]
 compiler = []
 trace = []
+
 unstable = []
-serde = ["actual-serde", "bitcoin/use-serde"]
+serde = ["actual-serde", "bitcoin/serde"]
 rand = ["bitcoin/rand"]
 
 [dependencies]
-bitcoin = { version = "0.28.1", default-features = false }
+bitcoin = { version = "0.29.1", default-features = false }
 hashbrown = { version = "0.11", optional = true }
 
 # Do NOT use this as a feature! Use the `serde` feature instead.
 actual-serde = { package = "serde", version = "1.0", optional = true }
 
-
 [dev-dependencies]
-bitcoind = {version = "0.26.1", features=["22_0"]}
+bitcoind = { version = "0.27.0", features=["23_0"] }
 actual-rand = { package = "rand", version = "0.8.4"}
-secp256k1 = {version = "0.22.1", features = ["rand-std"]}
+secp256k1 = {version = "0.24.0", features = ["rand-std"]}
 
 [[example]]
 name = "htlc"

examples/sign_multisig.rs

@@ -18,7 +18,7 @@ use std::collections::HashMap;
 use std::str::FromStr;
 
 use bitcoin::blockdata::witness::Witness;
-use bitcoin::secp256k1;
+use bitcoin::{secp256k1, PackedLockTime, Sequence};
 
 fn main() {
     let mut tx = spending_transaction();
@@ -91,11 +91,11 @@ fn main() {
 fn spending_transaction() -> bitcoin::Transaction {
     bitcoin::Transaction {
         version: 2,
-        lock_time: 0,
+        lock_time: PackedLockTime::ZERO,
         input: vec![bitcoin::TxIn {
             previous_output: Default::default(),
             script_sig: bitcoin::Script::new(),
-            sequence: 0xffffffff,
+            sequence: Sequence::MAX,
             witness: Witness::default(),
         }],
         output: vec![bitcoin::TxOut {

examples/taproot.rs

@@ -102,7 +102,7 @@ fn main() {
     let secp = secp256k1::Secp256k1::new();
     let key_pair = KeyPair::new(&secp, &mut rand::thread_rng());
     // Random unspendable XOnlyPublicKey provided for compilation to Taproot Descriptor
-    let unspendable_pubkey = bitcoin::XOnlyPublicKey::from_keypair(&key_pair);
+    let (unspendable_pubkey, _parity) = bitcoin::XOnlyPublicKey::from_keypair(&key_pair);
 
     pk_map.insert("UNSPENDABLE_KEY".to_string(), unspendable_pubkey);
     let pubkeys = hardcoded_xonlypubkeys();

examples/verify_tx.rs

@@ -19,6 +19,7 @@ use std::str::FromStr;
 use bitcoin::consensus::Decodable;
 use bitcoin::secp256k1::{self, Secp256k1};
 use bitcoin::util::sighash;
+use bitcoin::{LockTime, Sequence};
 use miniscript::interpreter::KeySigPair;
 
 fn main() {
@@ -33,8 +34,8 @@ fn main() {
         &spk_input_1,
         &tx.input[0].script_sig,
         &tx.input[0].witness,
-        0,
-        0,
+        Sequence::ZERO,
+        LockTime::ZERO,
     )
     .unwrap();
 

src/descriptor/mod.rs

@@ -860,7 +860,7 @@ mod tests {
     use bitcoin::hashes::hex::{FromHex, ToHex};
     use bitcoin::hashes::{hash160, sha256};
     use bitcoin::util::bip32;
-    use bitcoin::{self, secp256k1, EcdsaSighashType, PublicKey};
+    use bitcoin::{self, secp256k1, EcdsaSighashType, PublicKey, Sequence};
 
     use super::checksum::desc_checksum;
     use super::tr::Tr;
@@ -1155,7 +1155,7 @@ mod tests {
         let mut txin = bitcoin::TxIn {
             previous_output: bitcoin::OutPoint::default(),
             script_sig: bitcoin::Script::new(),
-            sequence: 100,
+            sequence: Sequence::from_height(100),
             witness: Witness::default(),
         };
         let bare = Descriptor::new_bare(ms.clone()).unwrap();
@@ -1166,7 +1166,7 @@ mod tests {
             bitcoin::TxIn {
                 previous_output: bitcoin::OutPoint::default(),
                 script_sig: script::Builder::new().push_slice(&sigser[..]).into_script(),
-                sequence: 100,
+                sequence: Sequence::from_height(100),
                 witness: Witness::default(),
             }
         );
@@ -1182,7 +1182,7 @@ mod tests {
                     .push_slice(&sigser[..])
                     .push_key(&pk)
                     .into_script(),
-                sequence: 100,
+                sequence: Sequence::from_height(100),
                 witness: Witness::default(),
             }
         );
@@ -1195,7 +1195,7 @@ mod tests {
             bitcoin::TxIn {
                 previous_output: bitcoin::OutPoint::default(),
                 script_sig: bitcoin::Script::new(),
-                sequence: 100,
+                sequence: Sequence::from_height(100),
                 witness: Witness::from_vec(vec![sigser.clone(), pk.to_bytes(),]),
             }
         );
@@ -1216,7 +1216,7 @@ mod tests {
                 script_sig: script::Builder::new()
                     .push_slice(&redeem_script[..])
                     .into_script(),
-                sequence: 100,
+                sequence: Sequence::from_height(100),
                 witness: Witness::from_vec(vec![sigser.clone(), pk.to_bytes(),]),
             }
         );
@@ -1238,7 +1238,7 @@ mod tests {
                     .push_slice(&sigser[..])
                     .push_slice(&ms.encode()[..])
                     .into_script(),
-                sequence: 100,
+                sequence: Sequence::from_height(100),
                 witness: Witness::default(),
             }
         );
@@ -1253,7 +1253,7 @@ mod tests {
             bitcoin::TxIn {
                 previous_output: bitcoin::OutPoint::default(),
                 script_sig: bitcoin::Script::new(),
-                sequence: 100,
+                sequence: Sequence::from_height(100),
                 witness: Witness::from_vec(vec![sigser.clone(), ms.encode().into_bytes(),]),
             }
         );
@@ -1268,7 +1268,7 @@ mod tests {
                 script_sig: script::Builder::new()
                     .push_slice(&ms.encode().to_v0_p2wsh()[..])
                     .into_script(),
-                sequence: 100,
+                sequence: Sequence::from_height(100),
                 witness: Witness::from_vec(vec![sigser.clone(), ms.encode().into_bytes(),]),
             }
         );
@@ -1401,7 +1401,7 @@ mod tests {
         let mut txin = bitcoin::TxIn {
             previous_output: bitcoin::OutPoint::default(),
             script_sig: bitcoin::Script::new(),
-            sequence: 0,
+            sequence: Sequence::ZERO,
             witness: Witness::default(),
         };
         let satisfier = {

src/descriptor/tr.rs

@@ -5,7 +5,7 @@ use core::{fmt, hash};
 
 use bitcoin::blockdata::opcodes;
 use bitcoin::util::taproot::{
-    LeafVersion, TaprootBuilder, TaprootBuilderError, TaprootSpendInfo, TAPROOT_CONTROL_BASE_SIZE,
+    LeafVersion, TaprootBuilder, TaprootSpendInfo, TAPROOT_CONTROL_BASE_SIZE,
     TAPROOT_CONTROL_MAX_NODE_COUNT, TAPROOT_CONTROL_NODE_SIZE,
 };
 use bitcoin::{secp256k1, Address, Network, Script};
@@ -238,26 +238,7 @@ impl<Pk: MiniscriptKey> Tr<Pk> {
             // Assert builder cannot error here because we have a well formed descriptor
             match builder.finalize(&secp, self.internal_key.to_x_only_pubkey()) {
                 Ok(data) => data,
-                Err(e) => match e {
-                    TaprootBuilderError::InvalidMerkleTreeDepth(_) => {
-                        unreachable!("Depth checked in struct construction")
-                    }
-                    TaprootBuilderError::NodeNotInDfsOrder => {
-                        unreachable!("Insertion is called in DFS order")
-                    }
-                    TaprootBuilderError::OverCompleteTree => {
-                        unreachable!("Taptree is a well formed tree")
-                    }
-                    TaprootBuilderError::InvalidInternalKey(_) => {
-                        unreachable!("Internal key checked for validity")
-                    }
-                    TaprootBuilderError::IncompleteTree => {
-                        unreachable!("Taptree is a well formed tree")
-                    }
-                    TaprootBuilderError::EmptyTree => {
-                        unreachable!("Taptree is a well formed tree with atleast 1 element")
-                    }
-                },
+                Err(_) => unreachable!("We know the builder can be finalized"),
             }
         };
         let spend_info = Arc::new(data);

src/interpreter/error.rs

@@ -29,6 +29,8 @@ use crate::prelude::*;
 pub enum Error {
     /// Could not satisfy, absolute locktime not met
     AbsoluteLocktimeNotMet(u32),
+    /// Could not satisfy, lock time values are different units
+    AbsoluteLocktimeComparisonInvalid(u32, u32),
     /// Cannot Infer a taproot descriptor
     /// Key spends cannot infer the internal key of the descriptor
     /// Inferring script spends is possible, but is hidden nodes are currently
@@ -129,6 +131,11 @@ impl fmt::Display for Error {
                 "required absolute locktime CLTV of {} blocks, not met",
                 n
             ),
+            Error::AbsoluteLocktimeComparisonInvalid(n, lock_time) => write!(
+                f,
+                "could not satisfy, lock time values are different units n: {} lock_time: {}",
+                n, lock_time
+            ),
             Error::CannotInferTrDescriptors => write!(f, "Cannot infer taproot descriptors"),
             Error::ControlBlockParse(ref e) => write!(f, "Control block parse error {}", e),
             Error::ControlBlockVerificationError => {
@@ -198,6 +205,7 @@ impl error::Error for Error {
 
         match self {
             AbsoluteLocktimeNotMet(_)
+            | AbsoluteLocktimeComparisonInvalid(_, _)
             | CannotInferTrDescriptors
             | ControlBlockVerificationError
             | CouldNotEvaluate

src/interpreter/mod.rs

@@ -25,7 +25,7 @@ use core::str::FromStr;
 use bitcoin::blockdata::witness::Witness;
 use bitcoin::hashes::{hash160, ripemd160, sha256};
 use bitcoin::util::{sighash, taproot};
-use bitcoin::{self, secp256k1, TxOut};
+use bitcoin::{self, secp256k1, LockTime, Sequence, TxOut};
 
 use crate::miniscript::context::NoChecks;
 use crate::miniscript::ScriptContext;
@@ -48,8 +48,8 @@ pub struct Interpreter<'txin> {
     /// For non-Taproot spends, the scriptCode; for Taproot script-spends, this
     /// is the leaf script; for key-spends it is `None`.
     script_code: Option<bitcoin::Script>,
-    age: u32,
-    lock_time: u32,
+    age: Sequence,
+    lock_time: LockTime,
 }
 
 // A type representing functions for checking signatures that accept both
@@ -173,8 +173,8 @@ impl<'txin> Interpreter<'txin> {
         spk: &bitcoin::Script,
         script_sig: &'txin bitcoin::Script,
         witness: &'txin Witness,
-        age: u32,       // CSV, relative lock time.
-        lock_time: u32, // CLTV, absolute lock time.
+        age: Sequence,       // CSV, relative lock time.
+        lock_time: LockTime, // CLTV, absolute lock time.
     ) -> Result<Self, Error> {
         let (inner, stack, script_code) = inner::from_txdata(spk, script_sig, witness)?;
         Ok(Interpreter {
@@ -491,12 +491,12 @@ pub enum SatisfiedConstraint {
     ///Relative Timelock for CSV.
     RelativeTimelock {
         /// The value of RelativeTimelock
-        time: u32,
+        n: Sequence,
     },
     ///Absolute Timelock for CLTV.
     AbsoluteTimelock {
         /// The value of Absolute timelock
-        time: u32,
+        n: LockTime,
     },
 }
 
@@ -531,8 +531,8 @@ pub struct Iter<'intp, 'txin: 'intp> {
     public_key: Option<&'intp BitcoinKey>,
     state: Vec<NodeEvaluationState<'intp>>,
     stack: Stack<'txin>,
-    age: u32,
-    lock_time: u32,
+    age: Sequence,
+    lock_time: LockTime,
     has_errored: bool,
 }
 
@@ -619,7 +619,7 @@ where
                 Terminal::After(ref n) => {
                     debug_assert_eq!(node_state.n_evaluated, 0);
                     debug_assert_eq!(node_state.n_satisfied, 0);
-                    let res = self.stack.evaluate_after(n, self.lock_time);
+                    let res = self.stack.evaluate_after(&n.into(), self.lock_time);
                     if res.is_some() {
                         return res;
                     }
@@ -1094,8 +1094,9 @@ mod tests {
             pks.push(pk);
             der_sigs.push(sigser);
 
-            let keypair = bitcoin::KeyPair::from_secret_key(&secp, sk);
-            x_only_pks.push(bitcoin::XOnlyPublicKey::from_keypair(&keypair));
+            let keypair = bitcoin::KeyPair::from_secret_key(&secp, &sk);
+            let (x_only_pk, _parity) = bitcoin::XOnlyPublicKey::from_keypair(&keypair);
+            x_only_pks.push(x_only_pk);
             let schnorr_sig = secp.sign_schnorr_with_aux_rand(&msg, &keypair, &[0u8; 32]);
             let schnorr_sig = bitcoin::SchnorrSig {
                 sig: schnorr_sig,
@@ -1144,8 +1145,8 @@ mod tests {
                     n_evaluated: 0,
                     n_satisfied: 0,
                 }],
-                age: 1002,
-                lock_time: 1002,
+                age: Sequence::from_height(1002),
+                lock_time: LockTime::from_height(1002).unwrap(),
                 has_errored: false,
             }
         }
@@ -1208,7 +1209,9 @@ mod tests {
         let after_satisfied: Result<Vec<SatisfiedConstraint>, Error> = constraints.collect();
         assert_eq!(
             after_satisfied.unwrap(),
-            vec![SatisfiedConstraint::AbsoluteTimelock { time: 1000 }]
+            vec![SatisfiedConstraint::AbsoluteTimelock {
+                n: LockTime::from_height(1000).unwrap()
+            }]
         );
 
         //Check Older
@@ -1218,7 +1221,9 @@ mod tests {
         let older_satisfied: Result<Vec<SatisfiedConstraint>, Error> = constraints.collect();
         assert_eq!(
             older_satisfied.unwrap(),
-            vec![SatisfiedConstraint::RelativeTimelock { time: 1000 }]
+            vec![SatisfiedConstraint::RelativeTimelock {
+                n: Sequence::from_height(1000)
+            }]
         );
 
         //Check Sha256