Merge #802: [Taptree API project] a bunch of test improvements cleanups

bb9a76c descriptor: add lifting unit test (Andrew Poelstra)
d1dadc6 fuzz: add regression test for taptree behavior (Andrew Poelstra)
e0e6338 fuzz: in descriptor parsing regression test, try lifting the result (Andrew Poelstra)
0b29186 fuzz: update README for current Nix instructions (Andrew Poelstra)
1af2b19 fuzz: use FuzzPk in regression_descriptor_parse.rs (Andrew Poelstra)
e072944 fuzz: pull FuzzPk out of miniscript_satisfy target (Andrew Poelstra)
7e1c1ba ci: add `taptree_of_horror` to list of examples to run (Andrew Poelstra)

Pull request description:

  I have a new branch prepared which overhauls the `TapTree` API, allowing users with a `tr` descriptor to iterate over all the branches, computing control blocks and leaf hashes as they go, which simplifies non-PSBT implementations of satisfiers (and also simplifies our own PSBT implementation).

  This is the first collection of patches, which simply adds some fuzz and unit tests covering things that I had trouble with during development.

ACKs for top commit:
  sanket1729:
    ACK bb9a76c

Tree-SHA512: 693962714f2a8227dcbc72d1cd7fec592e3c9e10b5c1ecb830a22aca57fd79c5c70a1d640cfd82d9339b668dfbdebc65157bf55744ab14b76f2ccb1dbb341587

Author: apoelstra

.github/workflows/cron-daily-fuzz.yml

@@ -25,6 +25,7 @@ parse_descriptor,
 parse_descriptor_priv,
 parse_descriptor_secret,
 regression_descriptor_parse,
+regression_taptree,
 roundtrip_concrete,
 roundtrip_descriptor,
 roundtrip_miniscript_script,

contrib/test_vars.sh

@@ -12,4 +12,4 @@ FEATURES_WITHOUT_STD="compiler trace serde rand base64"
 
 # Run these examples.
 # Note `examples/big` should not be run.
-EXAMPLES="htlc:std,compiler parse:std sign_multisig:std verify_tx:std xpub_descriptors:std taproot:std,compiler psbt_sign_finalize:std,base64"
+EXAMPLES="htlc:std,compiler parse:std sign_multisig:std verify_tx:std xpub_descriptors:std taproot:std,compiler psbt_sign_finalize:std,base64 taptree_of_horror:std,compiler"

fuzz/Cargo.toml

@@ -46,6 +46,10 @@ path = "fuzz_targets/parse_descriptor_secret.rs"
 name = "regression_descriptor_parse"
 path = "fuzz_targets/regression_descriptor_parse.rs"
 
+[[bin]]
+name = "regression_taptree"
+path = "fuzz_targets/regression_taptree.rs"
+
 [[bin]]
 name = "roundtrip_concrete"
 path = "fuzz_targets/roundtrip_concrete.rs"

fuzz/README.md

@@ -13,9 +13,13 @@ To build honggfuzz, you must have libunwind on your system, as well as
 libopcodes and libbfd from binutils **2.38** on your system. The most
 recently-released binutils 2.39 has changed their API in a breaking way.
 
-On Nix, you can obtain these libraries by running
+On Nix, you can obtain these libraries, and disable some hardening flags
+which conflict with the way honggfuzz builds its targets, by running
 
     nix-shell -p libopcodes_2_38 -p libunwind
+    # In the nix-shell run these
+    NIX_HARDENING_ENABLE=''${NIX_HARDENING_ENABLE/fortify/}
+    NIX_HARDENING_ENABLE=''${NIX_HARDENING_ENABLE/fortify3/}
 
 and then run fuzz.sh as above.
 

fuzz/fuzz_targets/miniscript_satisfy.rs

@@ -1,72 +1,14 @@
 #![allow(unexpected_cfgs)]
 
-use std::fmt;
-use std::str::FromStr;
 use std::sync::atomic::{AtomicUsize, Ordering};
 
+use descriptor_fuzz::FuzzPk;
 use honggfuzz::fuzz;
-use miniscript::bitcoin::hashes::{hash160, ripemd160, sha256, Hash};
+use miniscript::bitcoin::hashes::hash160;
 use miniscript::bitcoin::locktime::{absolute, relative};
 use miniscript::bitcoin::taproot::Signature;
 use miniscript::bitcoin::{secp256k1, PublicKey, TapLeafHash, TapSighashType, XOnlyPublicKey};
-use miniscript::{hash256, Miniscript, MiniscriptKey, Satisfier, Segwitv0, Tap, ToPublicKey};
-
-// FIXME pull this out into a library used by all the fuzztests
-#[derive(Clone, PartialOrd, Ord, PartialEq, Eq, Debug, Hash)]
-struct FuzzPk {
-    compressed: bool,
-}
-
-impl FuzzPk {
-    pub fn new_from_control_byte(control: u8) -> Self { Self { compressed: control & 1 == 1 } }
-}
-
-impl FromStr for FuzzPk {
-    type Err = std::num::ParseIntError;
-    fn from_str(s: &str) -> Result<Self, Self::Err> {
-        let byte = u8::from_str_radix(s, 16)?;
-        Ok(Self::new_from_control_byte(byte))
-    }
-}
-
-impl fmt::Display for FuzzPk {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { "[fuzz pubkey]".fmt(f) }
-}
-
-impl MiniscriptKey for FuzzPk {
-    type Sha256 = u8;
-    type Ripemd160 = u8;
-    type Hash160 = u8;
-    type Hash256 = u8;
-}
-
-impl ToPublicKey for FuzzPk {
-    fn to_public_key(&self) -> PublicKey {
-        let secp_pk = secp256k1::PublicKey::from_slice(&[
-            0x04, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x3b, 0x78,
-            0xce, 0x56, 0x3f, 0x89, 0xa0, 0xed, 0x94, 0x14, 0xf5, 0xaa, 0x28, 0xad, 0x0d, 0x96,
-            0xd6, 0x79, 0x5f, 0x9c, 0x63, 0x3f, 0x39, 0x79, 0xbf, 0x72, 0xae, 0x82, 0x02, 0x98,
-            0x3d, 0xc9, 0x89, 0xae, 0xc7, 0xf2, 0xff, 0x2e, 0xd9, 0x1b, 0xdd, 0x69, 0xce, 0x02,
-            0xfc, 0x07, 0x00, 0xca, 0x10, 0x0e, 0x59, 0xdd, 0xf3,
-        ])
-        .unwrap();
-        PublicKey { inner: secp_pk, compressed: self.compressed }
-    }
-
-    fn to_sha256(hash: &Self::Sha256) -> sha256::Hash { sha256::Hash::from_byte_array([*hash; 32]) }
-
-    fn to_hash256(hash: &Self::Hash256) -> hash256::Hash {
-        hash256::Hash::from_byte_array([*hash; 32])
-    }
-
-    fn to_ripemd160(hash: &Self::Ripemd160) -> ripemd160::Hash {
-        ripemd160::Hash::from_byte_array([*hash; 20])
-    }
-
-    fn to_hash160(hash: &Self::Ripemd160) -> hash160::Hash {
-        hash160::Hash::from_byte_array([*hash; 20])
-    }
-}
+use miniscript::{Miniscript, Satisfier, Segwitv0, Tap};
 
 struct FuzzSatisfier<'b> {
     idx: AtomicUsize,
@@ -198,14 +140,14 @@ fn do_test(data: &[u8]) {
 
     let s = String::from_utf8_lossy(s);
     if control & 1 == 1 {
-        let ms = match Miniscript::<FuzzPk, Segwitv0>::from_str(&s) {
+        let ms = match s.parse::<Miniscript<FuzzPk, Segwitv0>>() {
             Ok(d) => d,
             Err(_) => return,
         };
 
         let _ = ms.build_template(&fuzz_sat);
     } else {
-        let ms = match Miniscript::<FuzzPk, Tap>::from_str(&s) {
+        let ms = match s.parse::<Miniscript<FuzzPk, Tap>>() {
             Ok(d) => d,
             Err(_) => return,
         };

fuzz/fuzz_targets/regression_descriptor_parse.rs

@@ -1,11 +1,11 @@
 use core::str::FromStr;
 
 use honggfuzz::fuzz;
-use miniscript::{Descriptor, DescriptorPublicKey};
-use old_miniscript::{Descriptor as OldDescriptor, DescriptorPublicKey as OldDescriptorPublicKey};
+use miniscript::Descriptor;
+use old_miniscript::Descriptor as OldDescriptor;
 
-type Desc = Descriptor<DescriptorPublicKey>;
-type OldDesc = OldDescriptor<OldDescriptorPublicKey>;
+type Desc = Descriptor<descriptor_fuzz::FuzzPk>;
+type OldDesc = OldDescriptor<descriptor_fuzz::FuzzPk>;
 
 fn do_test(data: &[u8]) {
     let data_str = String::from_utf8_lossy(data);
@@ -14,12 +14,33 @@ fn do_test(data: &[u8]) {
         (Ok(x), Err(e)) => panic!("new logic parses {} as {:?}, old fails with {}", data_str, x, e),
         (Err(e), Ok(x)) => panic!("old logic parses {} as {:?}, new fails with {}", data_str, x, e),
         (Ok(new), Ok(old)) => {
+            use miniscript::policy::Liftable as _;
+            use old_miniscript::policy::Liftable as _;
+
             assert_eq!(
                 old.to_string(),
                 new.to_string(),
                 "input {} (left is old, right is new)",
                 data_str
-            )
+            );
+
+            match (new.lift(), old.lift()) {
+                (Err(_), Err(_)) => {}
+                (Ok(x), Err(e)) => {
+                    panic!("new logic lifts {} as {:?}, old fails with {}", data_str, x, e)
+                }
+                (Err(e), Ok(x)) => {
+                    panic!("old logic lifts {} as {:?}, new fails with {}", data_str, x, e)
+                }
+                (Ok(new), Ok(old)) => {
+                    assert_eq!(
+                        old.to_string(),
+                        new.to_string(),
+                        "lifted input {} (left is old, right is new)",
+                        data_str
+                    )
+                }
+            }
         }
     }
 }
@@ -35,9 +56,5 @@ fn main() {
 #[cfg(test)]
 mod tests {
     #[test]
-    fn duplicate_crash() {
-        crate::do_test(
-            b"tr(02dddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddd,{1,unun:0})",
-        )
-    }
+    fn duplicate_crash() { crate::do_test(b"tr(d,{0,{0,0}})") }
 }

fuzz/fuzz_targets/regression_taptree.rs

@@ -0,0 +1,46 @@
+use descriptor_fuzz::FuzzPk;
+use honggfuzz::fuzz;
+use miniscript::descriptor::Tr;
+use old_miniscript::descriptor::Tr as OldTr;
+
+fn do_test(data: &[u8]) {
+    let data_str = String::from_utf8_lossy(data);
+    match (data_str.parse::<Tr<FuzzPk>>(), data_str.parse::<OldTr<FuzzPk>>()) {
+        (Err(_), Err(_)) => {}
+        (Ok(_), Err(_)) => {} // 12.x logic rejects some parses for sanity reasons
+        (Err(e), Ok(x)) => panic!("old logic parses {} as {:?}, new fails with {}", data_str, x, e),
+        (Ok(new), Ok(old)) => {
+            let new_si = new.spend_info();
+            let old_si = old.spend_info();
+            assert_eq!(
+                old_si.internal_key(),
+                new_si.internal_key(),
+                "merkle root mismatch (left is old, new is right)",
+            );
+            assert_eq!(
+                old_si.merkle_root(),
+                new_si.merkle_root(),
+                "merkle root mismatch (left is old, new is right)",
+            );
+            assert_eq!(
+                old_si.output_key(),
+                new_si.output_key(),
+                "merkle root mismatch (left is old, new is right)",
+            );
+        }
+    }
+}
+
+fn main() {
+    loop {
+        fuzz!(|data| {
+            do_test(data);
+        });
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    #[test]
+    fn duplicate_crash() { crate::do_test(b"tr(0,{0,0})"); }
+}

fuzz/src/lib.rs

@@ -0,0 +1,106 @@
+// Written in 2019 by Andrew Poelstra <[email protected]>
+// SPDX-License-Identifier: CC0-1.0
+
+//! Miniscript Fuzzing Library
+//!
+//! This contains data structures and utilities used by the fuzz tests.
+
+use core::{fmt, str};
+
+use miniscript::bitcoin::hashes::{hash160, ripemd160, sha256, Hash};
+use miniscript::bitcoin::{secp256k1, PublicKey};
+use miniscript::{hash256, MiniscriptKey, ToPublicKey};
+
+/// A public key which is encoded as a single hex byte (two hex characters).
+///
+/// Implements `ToPublicKey` but (for now) always maps to the same bitcoin public key.
+#[derive(Clone, PartialOrd, Ord, PartialEq, Eq, Debug, Hash)]
+pub struct FuzzPk {
+    compressed: bool,
+}
+
+impl FuzzPk {
+    pub fn new_from_control_byte(control: u8) -> Self { Self { compressed: control & 1 == 1 } }
+}
+
+impl str::FromStr for FuzzPk {
+    type Err = std::num::ParseIntError;
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        let byte = u8::from_str_radix(s, 16)?;
+        Ok(Self::new_from_control_byte(byte))
+    }
+}
+
+impl fmt::Display for FuzzPk {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { "[fuzz pubkey]".fmt(f) }
+}
+
+impl MiniscriptKey for FuzzPk {
+    type Sha256 = u8;
+    type Ripemd160 = u8;
+    type Hash160 = u8;
+    type Hash256 = u8;
+}
+
+impl ToPublicKey for FuzzPk {
+    fn to_public_key(&self) -> PublicKey {
+        let secp_pk = secp256k1::PublicKey::from_slice(&[
+            0x04, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x3b, 0x78,
+            0xce, 0x56, 0x3f, 0x89, 0xa0, 0xed, 0x94, 0x14, 0xf5, 0xaa, 0x28, 0xad, 0x0d, 0x96,
+            0xd6, 0x79, 0x5f, 0x9c, 0x63, 0x3f, 0x39, 0x79, 0xbf, 0x72, 0xae, 0x82, 0x02, 0x98,
+            0x3d, 0xc9, 0x89, 0xae, 0xc7, 0xf2, 0xff, 0x2e, 0xd9, 0x1b, 0xdd, 0x69, 0xce, 0x02,
+            0xfc, 0x07, 0x00, 0xca, 0x10, 0x0e, 0x59, 0xdd, 0xf3,
+        ])
+        .unwrap();
+        PublicKey { inner: secp_pk, compressed: self.compressed }
+    }
+
+    fn to_sha256(hash: &Self::Sha256) -> sha256::Hash { sha256::Hash::from_byte_array([*hash; 32]) }
+
+    fn to_hash256(hash: &Self::Hash256) -> hash256::Hash {
+        hash256::Hash::from_byte_array([*hash; 32])
+    }
+
+    fn to_ripemd160(hash: &Self::Ripemd160) -> ripemd160::Hash {
+        ripemd160::Hash::from_byte_array([*hash; 20])
+    }
+
+    fn to_hash160(hash: &Self::Ripemd160) -> hash160::Hash {
+        hash160::Hash::from_byte_array([*hash; 20])
+    }
+}
+
+impl old_miniscript::MiniscriptKey for FuzzPk {
+    type Sha256 = u8;
+    type Ripemd160 = u8;
+    type Hash160 = u8;
+    type Hash256 = u8;
+}
+
+impl old_miniscript::ToPublicKey for FuzzPk {
+    fn to_public_key(&self) -> PublicKey {
+        let secp_pk = secp256k1::PublicKey::from_slice(&[
+            0x04, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x3b, 0x78,
+            0xce, 0x56, 0x3f, 0x89, 0xa0, 0xed, 0x94, 0x14, 0xf5, 0xaa, 0x28, 0xad, 0x0d, 0x96,
+            0xd6, 0x79, 0x5f, 0x9c, 0x63, 0x3f, 0x39, 0x79, 0xbf, 0x72, 0xae, 0x82, 0x02, 0x98,
+            0x3d, 0xc9, 0x89, 0xae, 0xc7, 0xf2, 0xff, 0x2e, 0xd9, 0x1b, 0xdd, 0x69, 0xce, 0x02,
+            0xfc, 0x07, 0x00, 0xca, 0x10, 0x0e, 0x59, 0xdd, 0xf3,
+        ])
+        .unwrap();
+        PublicKey { inner: secp_pk, compressed: self.compressed }
+    }
+
+    fn to_sha256(hash: &Self::Sha256) -> sha256::Hash { sha256::Hash::from_byte_array([*hash; 32]) }
+
+    fn to_hash256(hash: &Self::Hash256) -> old_miniscript::hash256::Hash {
+        old_miniscript::hash256::Hash::from_byte_array([*hash; 32])
+    }
+
+    fn to_ripemd160(hash: &Self::Ripemd160) -> ripemd160::Hash {
+        ripemd160::Hash::from_byte_array([*hash; 20])
+    }
+
+    fn to_hash160(hash: &Self::Ripemd160) -> hash160::Hash {
+        hash160::Hash::from_byte_array([*hash; 20])
+    }
+}

src/descriptor/mod.rs

@@ -2100,6 +2100,24 @@ pk(03f28773c2d975288bc7d1d205c3748651b075fbc6610e58cddeeddf8f19405aa8))";
         .unwrap_err();
     }
 
+    #[test]
+    fn tr_lift() {
+        use crate::policy::Liftable as _;
+
+        // Taproot structure is erased but key order preserved..
+        let desc = Descriptor::<String>::from_str("tr(ROOT,{pk(A1),{pk(B1),pk(B2)}})").unwrap();
+        let lift = desc.lift().unwrap();
+        assert_eq!(lift.to_string(), "or(pk(ROOT),or(pk(A1),pk(B1),pk(B2)))",);
+        let desc = Descriptor::<String>::from_str("tr(ROOT,{{pk(A1),pk(B1)},pk(B2)})").unwrap();
+        let lift = desc.lift().unwrap();
+        assert_eq!(lift.to_string(), "or(pk(ROOT),or(pk(A1),pk(B1),pk(B2)))",);
+
+        // And normalization happens
+        let desc = Descriptor::<String>::from_str("tr(ROOT,{0,{0,0}})").unwrap();
+        let lift = desc.lift().unwrap();
+        assert_eq!(lift.to_string(), "or(pk(ROOT),UNSATISFIABLE)",);
+    }
+
     #[test]
     fn test_context_pks() {
         let comp_key = bitcoin::PublicKey::from_str(