Merge #255: Add TapCtx

96cb2ec Add comment to decoding x-only keys (sanket1729)
498bad2 Address review feedback (sanket1729)
954df35 Check stack execution height in tapscript execution (sanket1729)
95ab99d Add height check to miniscript types (sanket1729)
3eb63d5 Add parse/parse_insane for schnorr keys (sanket1729)
888cd62 Add support for parsing Tapscript Miniscripts (sanket1729)
9a5384d Add TapCtx (sanket1729)
4da439f Fix warnings (sanket1729)

Pull request description:

  This is still a draft WIP for taproot PR. There are several API changes and multiple design decisions, and I am open to other possible design opinions. In particular, feedback about changes to `ToPublicKey` and the `ScriptContext` trait is appreciated.

  This does **not** include support for P2TR descriptors, multi_a fragment in miniscript.

  Still todo:

  - [ ] Track height during execution to make sure we don't exceed 1000 elements. This was not a concern previously because it was impossible to hit the limit without exceeding 201 opcodes. But with taproot, this is now possible. See #198 .

  - [ ] Implement support for multi_a fragment in Taproot. Depends on support for new opcodes from rust-bitcoin. We can use NOPs meanwhile as this is not a hard blocker.

  - [ ] Implement taproot descriptor. This would require some design considerations about the tree representation.

  - [ ] Integrate taproot with interpreter and compiler. The interpreter is probably straightforward but carrying around `bitcoin:::PublicKey` and `schnorr::Publickey` would be tricky.

  - [ ] Integrate with descriptorPublicKey so that descriptor wallets like bdk can actually use it.

ACKs for top commit:
  apoelstra:
    ACK 96cb2ec

Tree-SHA512: 261173c3a6f8c980d204fce1f868dcd17c4219fd12a8e6e197b503efb47ae9b9371d7a9208c43008f1c21db55ffc3f68e6d7fb8badbf2bf397b2970292f8ffe4

Author: apoelstra

CHANGELOG.md

@@ -1,3 +1,5 @@
+# Taproot updates
+- Changed the ToPublicKey trait to support x-only keys.
 # 6.0.1 - Aug 5, 2021
 
 - The `lift` method on a Miniscript node was fixed. It would previously mix up

fuzz/fuzz_targets/roundtrip_miniscript_script.rs

@@ -8,7 +8,7 @@ fn do_test(data: &[u8]) {
     // Try round-tripping as a script
     let script = script::Script::from(data.to_owned());
 
-    if let Ok(pt) = Miniscript::<_, Segwitv0>::parse(&script) {
+    if let Ok(pt) = Miniscript::<miniscript::bitcoin::PublicKey, Segwitv0>::parse(&script) {
         let output = pt.encode();
         assert_eq!(pt.script_size(), output.len());
         assert_eq!(output, script);

src/descriptor/bare.rs

@@ -339,7 +339,7 @@ impl<Pk: MiniscriptKey> DescriptorTrait<Pk> for Pkh<Pk> {
     }
 
     fn max_satisfaction_weight(&self) -> Result<usize, Error> {
-        Ok(4 * (1 + 73 + self.pk.serialized_len()))
+        Ok(4 * (1 + 73 + BareCtx::pk_len(&self.pk)))
     }
 
     fn script_code(&self) -> Script

src/descriptor/mod.rs

@@ -921,7 +921,7 @@ mod tests {
         struct SimpleSat {
             sig: secp256k1::Signature,
             pk: bitcoin::PublicKey,
-        };
+        }
 
         impl Satisfier<bitcoin::PublicKey> for SimpleSat {
             fn lookup_sig(&self, pk: &bitcoin::PublicKey) -> Option<BitcoinSig> {

src/descriptor/segwitv0.rs

@@ -293,7 +293,9 @@ impl<Pk: MiniscriptKey> Wpkh<Pk> {
     pub fn new(pk: Pk) -> Result<Self, Error> {
         // do the top-level checks
         if pk.is_uncompressed() {
-            Err(Error::ContextError(ScriptContextError::CompressedOnly))
+            Err(Error::ContextError(ScriptContextError::CompressedOnly(
+                pk.to_string(),
+            )))
         } else {
             Ok(Self { pk: pk })
         }
@@ -376,7 +378,9 @@ where
 impl<Pk: MiniscriptKey> DescriptorTrait<Pk> for Wpkh<Pk> {
     fn sanity_check(&self) -> Result<(), Error> {
         if self.pk.is_uncompressed() {
-            Err(Error::ContextError(ScriptContextError::CompressedOnly))
+            Err(Error::ContextError(ScriptContextError::CompressedOnly(
+                self.pk.to_string(),
+            )))
         } else {
             Ok(())
         }
@@ -430,7 +434,7 @@ impl<Pk: MiniscriptKey> DescriptorTrait<Pk> for Wpkh<Pk> {
     }
 
     fn max_satisfaction_weight(&self) -> Result<usize, Error> {
-        Ok(4 + 1 + 73 + self.pk.serialized_len())
+        Ok(4 + 1 + 73 + Segwitv0::pk_len(&self.pk))
     }
 
     fn script_code(&self) -> Script

src/descriptor/sortedmulti.rs

@@ -178,7 +178,7 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> SortedMultiVec<Pk, Ctx> {
         script_num_size(self.k)
             + 1
             + script_num_size(self.pks.len())
-            + self.pks.iter().map(|pk| pk.serialized_len()).sum::<usize>()
+            + self.pks.iter().map(|pk| Ctx::pk_len(pk)).sum::<usize>()
     }
 
     /// Maximum number of witness elements used to satisfy the Miniscript

src/interpreter/error.rs

@@ -103,7 +103,7 @@ impl From<::Error> for Error {
 }
 
 impl error::Error for Error {
-    fn cause(&self) -> Option<&error::Error> {
+    fn cause(&self) -> Option<&dyn error::Error> {
         match *self {
             Error::Secp(ref err) => Some(err),
             ref x => Some(x),

src/interpreter/inner.rs

@@ -50,7 +50,8 @@ fn script_from_stackelem<'a>(
 ) -> Result<Miniscript<bitcoin::PublicKey, NoChecks>, Error> {
     match *elem {
         stack::Element::Push(sl) => {
-            Miniscript::parse_insane(&bitcoin::Script::from(sl.to_owned())).map_err(Error::from)
+            Miniscript::<bitcoin::PublicKey, _>::parse_insane(&bitcoin::Script::from(sl.to_owned()))
+                .map_err(Error::from)
         }
         stack::Element::Satisfied => Miniscript::from_ast(::Terminal::True).map_err(Error::from),
         stack::Element::Dissatisfied => {
@@ -270,7 +271,7 @@ pub fn from_txdata<'txin>(
     // ** bare script **
     } else {
         if wit_stack.is_empty() {
-            let miniscript = Miniscript::parse_insane(spk)?;
+            let miniscript = Miniscript::<bitcoin::PublicKey, _>::parse_insane(spk)?;
             Ok((
                 Inner::Script(miniscript, ScriptType::Bare),
                 ssig_stack,

src/interpreter/mod.rs

@@ -847,7 +847,7 @@ mod tests {
                 height: 1002,
                 has_errored: false,
             }
-        };
+        }
 
         let pk = ms_str!("c:pk_k({})", pks[0]);
         let pkh = ms_str!("c:pk_h({})", pks[1].to_pubkeyhash());

src/lib.rs

@@ -87,7 +87,6 @@
 //! ```
 //!
 //!
-#![allow(bare_trait_objects)]
 #![cfg_attr(all(test, feature = "unstable"), feature(test))]
 // Coding conventions
 #![deny(unsafe_code)]
@@ -125,7 +124,7 @@ use bitcoin::hashes::{hash160, sha256, Hash};
 
 pub use descriptor::{Descriptor, DescriptorPublicKey, DescriptorTrait};
 pub use interpreter::Interpreter;
-pub use miniscript::context::{BareCtx, Legacy, ScriptContext, Segwitv0};
+pub use miniscript::context::{BareCtx, Legacy, ScriptContext, Segwitv0, Tap};
 pub use miniscript::decode::Terminal;
 pub use miniscript::satisfy::{BitcoinSig, Preimage32, Satisfier};
 pub use miniscript::Miniscript;
@@ -142,16 +141,6 @@ pub trait MiniscriptKey: Clone + Eq + Ord + fmt::Debug + fmt::Display + hash::Ha
 
     /// Converts an object to PublicHash
     fn to_pubkeyhash(&self) -> Self::Hash;
-
-    /// Computes the size of a public key when serialized in a script,
-    /// including the length bytes
-    fn serialized_len(&self) -> usize {
-        if self.is_uncompressed() {
-            66
-        } else {
-            34
-        }
-    }
 }
 
 impl MiniscriptKey for bitcoin::PublicKey {
@@ -170,6 +159,14 @@ impl MiniscriptKey for bitcoin::PublicKey {
     }
 }
 
+impl MiniscriptKey for bitcoin::schnorr::PublicKey {
+    type Hash = hash160::Hash;
+
+    fn to_pubkeyhash(&self) -> Self::Hash {
+        hash160::Hash::hash(&self.serialize())
+    }
+}
+
 impl MiniscriptKey for String {
     type Hash = String;
 
@@ -183,6 +180,12 @@ pub trait ToPublicKey: MiniscriptKey {
     /// Converts an object to a public key
     fn to_public_key(&self) -> bitcoin::PublicKey;
 
+    /// Convert an object to x-only pubkey
+    fn to_x_only_pubkey(&self) -> bitcoin::schnorr::PublicKey {
+        let pk = self.to_public_key();
+        bitcoin::schnorr::PublicKey::from(pk.key)
+    }
+
     /// Converts a hashed version of the public key to a `hash160` hash.
     ///
     /// This method must be consistent with `to_public_key`, in the sense
@@ -202,6 +205,25 @@ impl ToPublicKey for bitcoin::PublicKey {
     }
 }
 
+impl ToPublicKey for bitcoin::schnorr::PublicKey {
+    fn to_public_key(&self) -> bitcoin::PublicKey {
+        // This code should never be used.
+        // But is implemented for completeness
+        let mut data: Vec<u8> = vec![0x02];
+        data.extend(self.serialize().iter());
+        bitcoin::PublicKey::from_slice(&data)
+            .expect("Failed to construct 33 Publickey from 0x02 appended x-only key")
+    }
+
+    fn to_x_only_pubkey(&self) -> bitcoin::schnorr::PublicKey {
+        *self
+    }
+
+    fn hash_to_hash160(hash: &hash160::Hash) -> hash160::Hash {
+        *hash
+    }
+}
+
 /// Dummy key which de/serializes to the empty string; useful sometimes for testing
 #[derive(Copy, Clone, PartialOrd, Ord, PartialEq, Eq, Debug)]
 pub struct DummyKey;
@@ -448,7 +470,7 @@ pub enum Error {
     InvalidOpcode(opcodes::All),
     /// Some opcode occurred followed by `OP_VERIFY` when it had
     /// a `VERIFY` version that should have been used instead
-    NonMinimalVerify(miniscript::lex::Token),
+    NonMinimalVerify(String),
     /// Push was illegal in some context
     InvalidPush(Vec<u8>),
     /// rust-bitcoin script error
@@ -517,6 +539,8 @@ pub enum Error {
     ImpossibleSatisfaction,
     /// Bare descriptors don't have any addresses
     BareDescriptorAddr,
+    /// PubKey invalid under current context
+    PubKeyCtxError(miniscript::decode::KeyParseError, &'static str),
 }
 
 #[doc(hidden)]
@@ -563,7 +587,7 @@ fn errstr(s: &str) -> Error {
 }
 
 impl error::Error for Error {
-    fn cause(&self) -> Option<&error::Error> {
+    fn cause(&self) -> Option<&dyn error::Error> {
         match *self {
             Error::BadPubkey(ref e) => Some(e),
             _ => None,
@@ -580,7 +604,7 @@ impl fmt::Display for Error {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         match *self {
             Error::InvalidOpcode(op) => write!(f, "invalid opcode {}", op),
-            Error::NonMinimalVerify(tok) => write!(f, "{} VERIFY", tok),
+            Error::NonMinimalVerify(ref tok) => write!(f, "{} VERIFY", tok),
             Error::InvalidPush(ref push) => write!(f, "invalid push {:?}", push), // TODO hexify this
             Error::Script(ref e) => fmt::Display::fmt(e, f),
             Error::CmsTooManyKeys(n) => write!(f, "checkmultisig with {} keys", n),
@@ -634,6 +658,9 @@ impl fmt::Display for Error {
             Error::AnalysisError(ref e) => e.fmt(f),
             Error::ImpossibleSatisfaction => write!(f, "Impossible to satisfy Miniscript"),
             Error::BareDescriptorAddr => write!(f, "Bare descriptors don't have address"),
+            Error::PubKeyCtxError(ref pk, ref ctx) => {
+                write!(f, "Pubkey error: {} under {} scriptcontext", pk, ctx)
+            }
         }
     }
 }

src/miniscript/astelem.rs

@@ -32,6 +32,8 @@ use expression;
 use miniscript::types::{self, Property};
 use miniscript::ScriptContext;
 use script_num_size;
+
+use util::MsKeyBuilder;
 use {Error, ForEach, ForEachKey, Miniscript, MiniscriptKey, Terminal, ToPublicKey, TranslatePk};
 
 impl<Pk: MiniscriptKey, Ctx: ScriptContext> Terminal<Pk, Ctx> {
@@ -628,7 +630,7 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> Terminal<Pk, Ctx> {
         Pk: ToPublicKey,
     {
         match *self {
-            Terminal::PkK(ref pk) => builder.push_key(&pk.to_public_key()),
+            Terminal::PkK(ref pk) => builder.push_ms_key::<_, Ctx>(pk),
             Terminal::PkH(ref hash) => builder
                 .push_opcode(opcodes::all::OP_DUP)
                 .push_opcode(opcodes::all::OP_HASH160)
@@ -734,6 +736,7 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> Terminal<Pk, Ctx> {
                     .push_opcode(opcodes::all::OP_EQUAL)
             }
             Terminal::Multi(k, ref keys) => {
+                debug_assert!(!Ctx::is_tap());
                 builder = builder.push_int(k as i64);
                 for pk in keys {
                     builder = builder.push_key(&pk.to_public_key());
@@ -754,7 +757,7 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> Terminal<Pk, Ctx> {
     /// will handle the segwit/non-segwit technicalities for you.
     pub fn script_size(&self) -> usize {
         match *self {
-            Terminal::PkK(ref pk) => pk.serialized_len(),
+            Terminal::PkK(ref pk) => Ctx::pk_len(pk),
             Terminal::PkH(..) => 24,
             Terminal::After(n) => script_num_size(n as usize) + 1,
             Terminal::Older(n) => script_num_size(n as usize) + 1,
@@ -794,7 +797,7 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> Terminal<Pk, Ctx> {
                 script_num_size(k)
                     + 1
                     + script_num_size(pks.len())
-                    + pks.iter().map(|pk| pk.serialized_len()).sum::<usize>()
+                    + pks.iter().map(|pk| Ctx::pk_len(pk)).sum::<usize>()
             }
         }
     }