Cleanup iter APIs, retintroduce examples

Author: sanket1729

Cargo.toml

@@ -33,9 +33,8 @@ name = "parse"
 [[example]]
 name = "sign_multisig"
 
-# TODO: Re-enable in future commit after API re-design
-# [[example]]
-# name = "verify_tx"
+[[example]]
+name = "verify_tx"
 
 [[example]]
 name = "psbt"

examples/verify_tx.rs

@@ -19,6 +19,8 @@ extern crate miniscript;
 
 use bitcoin::consensus::Decodable;
 use bitcoin::secp256k1; // secp256k1 re-exported from rust-bitcoin
+use bitcoin::util::sighash;
+use miniscript::interpreter::KeySigPair;
 use std::str::FromStr;
 
 fn main() {
@@ -83,7 +85,7 @@ fn main() {
         0xa9, 0x14, 0x92, 0x09, 0xa8, 0xf9, 0x0c, 0x58, 0x4b, 0xb5, 0x97, 0x4d, 0x58, 0x68, 0x72,
         0x49, 0xe5, 0x32, 0xde, 0x59, 0xf4, 0xbc, 0x87,
     ]);
-    let mut interpreter = miniscript::Interpreter::from_txdata(
+    let interpreter = miniscript::Interpreter::from_txdata(
         &spk_input_1,
         &transaction.input[0].script_sig,
         &transaction.input[0].witness,
@@ -105,10 +107,14 @@ fn main() {
     //    the blockchain, standardness would've required they be
     //    either valid or 0-length.
     println!("\nExample one");
-    for elem in interpreter.iter(|_, _| true) {
+    for elem in interpreter.iter_assume_sigs() {
         // Don't bother checking signatures
         match elem.expect("no evaluation error") {
-            miniscript::interpreter::SatisfiedConstraint::PublicKey { key, sig } => {
+            miniscript::interpreter::SatisfiedConstraint::PublicKey { key_sig } => {
+                // Check that the signature is ecdsa sig
+                let (key, sig) = key_sig
+                    .as_ecdsa()
+                    .expect("Expected Ecdsa sig, found schnorr sig");
                 println!("Signed with {}: {}", key, sig);
             }
             _ => {}
@@ -122,7 +128,7 @@ fn main() {
     // from the MiniscriptKey which can supplied by `to_pk_ctx` parameter. For example,
     // when calculating the script pubkey of a descriptor with xpubs, the secp context and
     // child information maybe required.
-    let mut interpreter = miniscript::Interpreter::from_txdata(
+    let interpreter = miniscript::Interpreter::from_txdata(
         &spk_input_1,
         &transaction.input[0].script_sig,
         &transaction.input[0].witness,
@@ -131,21 +137,15 @@ fn main() {
     )
     .unwrap();
 
-    // We can set the amount passed to `sighash_verify` to 0 because this is a legacy
-    // transaction and so the amount won't actually be checked by the signature
-    let vfyfn = interpreter
-        .sighash_verify(&secp, &transaction, 0, 0)
-        .expect("Can only fail in sighash single when corresponding output is not present");
-    // Restrict to sighash_all just to demonstrate how to add additional filters
-    // `&_` needed here because of https://github.com/rust-lang/rust/issues/79187
-    let vfyfn = move |pk: &_, bitcoinsig: miniscript::bitcoin::EcdsaSig| {
-        bitcoinsig.hash_ty == bitcoin::EcdsaSigHashType::All && vfyfn(pk, bitcoinsig)
-    };
+    // We can set prevouts to be empty list because this is a legacy transaction
+    // and this information is not required for sighash computation.
+    let prevouts = sighash::Prevouts::All(&[]);
 
     println!("\nExample two");
-    for elem in interpreter.iter(vfyfn) {
+    for elem in interpreter.iter(&secp, &transaction, 0, &prevouts) {
         match elem.expect("no evaluation error") {
-            miniscript::interpreter::SatisfiedConstraint::PublicKey { key, sig } => {
+            miniscript::interpreter::SatisfiedConstraint::PublicKey { key_sig } => {
+                let (key, sig) = key_sig.as_ecdsa().unwrap();
                 println!("Signed with {}: {}", key, sig);
             }
             _ => {}
@@ -156,7 +156,7 @@ fn main() {
     //    what happens given an apparently invalid script
     let secp = secp256k1::Secp256k1::new();
     let message = secp256k1::Message::from_slice(&[0x01; 32][..]).expect("32-byte hash");
-    let mut interpreter = miniscript::Interpreter::from_txdata(
+    let interpreter = miniscript::Interpreter::from_txdata(
         &spk_input_1,
         &transaction.input[0].script_sig,
         &transaction.input[0].witness,
@@ -165,12 +165,13 @@ fn main() {
     )
     .unwrap();
 
-    let iter = interpreter.iter(|pk, ecdsa_sig| {
+    let iter = interpreter.iter_custom(Box::new(|key_sig: &KeySigPair| {
+        let (pk, ecdsa_sig) = key_sig.as_ecdsa().expect("Ecdsa Sig");
         ecdsa_sig.hash_ty == bitcoin::EcdsaSigHashType::All
             && secp
                 .verify_ecdsa(&message, &ecdsa_sig.sig, &pk.inner)
                 .is_ok()
-    });
+    }));
     println!("\nExample three");
     for elem in iter {
         let error = elem.expect_err("evaluation error");

src/interpreter/inner.rs

@@ -376,7 +376,6 @@ pub(super) fn pre_taproot_to_no_checks<Ctx: ScriptContext>(
 }
 
 // Convert a miniscript from a specified context into an insane miniscript
-#[allow(dead_code)]
 pub(super) fn taproot_to_no_checks<Ctx: ScriptContext>(
     ms: &Miniscript<bitcoin::XOnlyPublicKey, Ctx>,
 ) -> Miniscript<BitcoinKey, NoChecks> {

src/interpreter/mod.rs

@@ -25,8 +25,6 @@ use std::fmt;
 use std::str::FromStr;
 
 use bitcoin::hashes::{hash160, ripemd160, sha256, sha256d};
-// use bitcoin::util::sighash;
-#[allow(unused_imports)]
 use bitcoin::{self, secp256k1};
 use miniscript::context::NoChecks;
 use miniscript::ScriptContext;
@@ -65,6 +63,24 @@ pub enum KeySigPair {
     Schnorr(bitcoin::XOnlyPublicKey, bitcoin::SchnorrSig),
 }
 
+impl KeySigPair {
+    /// Obtain a pair of ([`bitcoin::PublicKey`], [`bitcoin::EcdsaSig`]) from [`KeySigPair`]
+    pub fn as_ecdsa(&self) -> Option<(bitcoin::PublicKey, bitcoin::EcdsaSig)> {
+        match self {
+            KeySigPair::Ecdsa(pk, sig) => Some((*pk, *sig)),
+            KeySigPair::Schnorr(_, _) => None,
+        }
+    }
+
+    /// Obtain a pair of ([`bitcoin::XOnlyPublicKey`], [`bitcoin::SchnorrSig`]) from [`KeySigPair`]
+    pub fn as_schnorr(&self) -> Option<(bitcoin::XOnlyPublicKey, bitcoin::SchnorrSig)> {
+        match self {
+            KeySigPair::Ecdsa(_, _) => None,
+            KeySigPair::Schnorr(pk, sig) => Some((*pk, *sig)),
+        }
+    }
+}
+
 // Internally used enum for different types of bitcoin keys
 // Even though we implement MiniscriptKey for BitcoinKey, we make sure that there
 // are little mis-use
@@ -165,19 +181,10 @@ impl<'txin> Interpreter<'txin> {
         })
     }
 
-    /// Creates an iterator over the satisfied spending conditions
-    ///
-    /// Returns all satisfied constraints, even if they were redundant (i.e. did
-    /// not contribute to the script being satisfied). For example, if a signature
-    /// were provided for an `and_b(Pk,false)` fragment, that signature will be
-    /// returned, even though the entire and_b must have failed and must not have
-    /// been used.
-    ///
-    /// In case the script is actually dissatisfied, this may return several values
-    /// before ultimately returning an error.
+    /// Same as [`Interpreter::iter`], but allows for a custom verification function.
     /// See [Self::iter_assume_sigs] for a simpler API without information about Prevouts
     /// but skips the signature verification
-    pub fn iter<'iter>(
+    pub fn iter_custom<'iter>(
         &'iter self,
         verify_sig: Box<dyn FnMut(&KeySigPair) -> bool + 'iter>,
     ) -> Iter<'txin, 'iter> {
@@ -292,28 +299,38 @@ impl<'txin> Interpreter<'txin> {
         }
     }
 
-    /// Iterate over all satisfied constraints while checking signatures
+    /// Creates an iterator over the satisfied spending conditions
+    ///
+    /// Returns all satisfied constraints, even if they were redundant (i.e. did
+    /// not contribute to the script being satisfied). For example, if a signature
+    /// were provided for an `and_b(Pk,false)` fragment, that signature will be
+    /// returned, even though the entire and_b must have failed and must not have
+    /// been used.
+    ///
+    /// In case the script is actually dissatisfied, this may return several values
+    /// before ultimately returning an error.
+    ///
     /// Not all fields are used by legacy/segwitv0 descriptors; if you are sure this is a legacy
     /// spend (you can check with the `is_legacy\is_segwitv0` method) you can provide dummy data for
     /// the amount/prevouts.
     /// - For legacy outputs, no information about prevouts is required
     /// - For segwitv0 outputs, prevout at corresponding index with correct amount must be provided
     /// - For taproot outputs, information about all prevouts must be supplied
-    pub fn iter_check_sigs<'iter, C: secp256k1::Verification>(
+    pub fn iter<'iter, C: secp256k1::Verification>(
         &'iter self,
         secp: &'iter secp256k1::Secp256k1<C>,
         tx: &'txin bitcoin::Transaction,
         input_idx: usize,
         prevouts: &'iter sighash::Prevouts, // actually a 'prevouts, but 'prevouts: 'iter
     ) -> Iter<'txin, 'iter> {
-        self.iter(Box::new(move |sig| {
+        self.iter_custom(Box::new(move |sig| {
             self.verify_sig(secp, tx, input_idx, prevouts, sig)
         }))
     }
 
     /// Creates an iterator over the satisfied spending conditions without checking signatures
     pub fn iter_assume_sigs<'iter>(&'iter self) -> Iter<'txin, 'iter> {
-        self.iter(Box::new(|_| true))
+        self.iter_custom(Box::new(|_| true))
     }
 
     /// Outputs a "descriptor" string which reproduces the spent coins

src/psbt/finalizer.rs

@@ -313,7 +313,7 @@ pub fn interpreter_check<C: secp256k1::Verification>(
                 secp.ctx();
                 true
             };
-            let iter = interpreter.iter(Box::new(y));
+            let iter = interpreter.iter_custom(Box::new(y));
             if let Some(error) = iter.filter_map(Result::err).next() {
                 return Err(Error::InputError(InputError::Interpreter(error), index));
             };