Merge #816: Backport #809 to 11.x

953e679 bump patch version of 11.2 (Andrew Poelstra)
3bf002c add regression test for #806 (Andrew Poelstra)
4f8b065 descriptor: fix key parsing error handling in parse_desc (Andrew Poelstra)
7c651f4 ci: update fuzz CI job to reorder tests (Andrew Poelstra)
aaf276c lib: remove some deny lints (Andrew Poelstra)

Pull request description:

  Backport of #809 to 11.x. Last backport for this PR.

ACKs for top commit:
  sanket1729:
    utACK 953e679

Tree-SHA512: 8f9ee0b49f2d2e1e4560a950bd84dad2bef285e72466b3ccf19170157958e5749bc73194581b40c7dc977c6938627ccb444d58a2161934e148281891c6b319de

Author: apoelstra

.github/workflows/fuzz.yml

@@ -16,14 +16,14 @@ jobs:
       fail-fast: false
       matrix:
         fuzz_target: [
-roundtrip_semantic,
+compile_descriptor,
 parse_descriptor,
 parse_descriptor_secret,
+roundtrip_concrete,
+roundtrip_descriptor,
 roundtrip_miniscript_script,
 roundtrip_miniscript_str,
-roundtrip_descriptor,
-roundtrip_concrete,
-compile_descriptor,
+roundtrip_semantic,
         ]
     steps:
       - name: Install test dependencies

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "miniscript"
-version = "11.2.0"
+version = "11.2.1"
 authors = ["Andrew Poelstra <apoelstra@wpsoftware.net>, Sanket Kanjalkar <sanket1729@gmail.com>"]
 license = "CC0-1.0"
 homepage = "https://github.com/rust-bitcoin/rust-miniscript/"

fuzz/Cargo.toml

@@ -15,8 +15,8 @@ miniscript = { path = "..", features = [ "compiler" ] }
 regex = "1.0"
 
 [[bin]]
-name = "roundtrip_semantic"
-path = "fuzz_targets/roundtrip_semantic.rs"
+name = "compile_descriptor"
+path = "fuzz_targets/compile_descriptor.rs"
 
 [[bin]]
 name = "parse_descriptor"
@@ -27,21 +27,21 @@ name = "parse_descriptor_secret"
 path = "fuzz_targets/parse_descriptor_secret.rs"
 
 [[bin]]
-name = "roundtrip_miniscript_script"
-path = "fuzz_targets/roundtrip_miniscript_script.rs"
-
-[[bin]]
-name = "roundtrip_miniscript_str"
-path = "fuzz_targets/roundtrip_miniscript_str.rs"
+name = "roundtrip_concrete"
+path = "fuzz_targets/roundtrip_concrete.rs"
 
 [[bin]]
 name = "roundtrip_descriptor"
 path = "fuzz_targets/roundtrip_descriptor.rs"
 
 [[bin]]
-name = "roundtrip_concrete"
-path = "fuzz_targets/roundtrip_concrete.rs"
+name = "roundtrip_miniscript_script"
+path = "fuzz_targets/roundtrip_miniscript_script.rs"
 
 [[bin]]
-name = "compile_descriptor"
-path = "fuzz_targets/compile_descriptor.rs"
+name = "roundtrip_miniscript_str"
+path = "fuzz_targets/roundtrip_miniscript_str.rs"
+
+[[bin]]
+name = "roundtrip_semantic"
+path = "fuzz_targets/roundtrip_semantic.rs"

fuzz/fuzz-util.sh

@@ -2,9 +2,13 @@
 
 REPO_DIR=$(git rev-parse --show-toplevel)
 
+# Sort order is effected by locale. See `man sort`.
+# > Set LC_ALL=C to get the traditional sort order that uses native byte values.
+export LC_ALL=C
+
 listTargetFiles() {
   pushd "$REPO_DIR/fuzz" > /dev/null || exit 1
-  find fuzz_targets/ -type f -name "*.rs"
+  find fuzz_targets/ -type f -name "*.rs" | sort
   popd > /dev/null || exit 1
 }
 

src/descriptor/mod.rs

@@ -725,12 +725,9 @@ impl Descriptor<DescriptorPublicKey> {
         }
 
         let descriptor = Descriptor::<String>::from_str(s)?;
-        let descriptor = descriptor.translate_pk(&mut keymap_pk).map_err(|e| {
-            Error::Unexpected(
-                e.expect_translator_err("No Outer context errors")
-                    .to_string(),
-            )
-        })?;
+        let descriptor = descriptor
+            .translate_pk(&mut keymap_pk)
+            .map_err(TranslateErr::flatten)?;
 
         Ok((descriptor, keymap_pk.0))
     }
@@ -2053,4 +2050,19 @@ pk(03f28773c2d975288bc7d1d205c3748651b075fbc6610e58cddeeddf8f19405aa8))";
         Desc::from_str(&format!("tr({},pk({}))", x_only_key, uncomp_key)).unwrap_err();
         Desc::from_str(&format!("tr({},pk({}))", x_only_key, x_only_key)).unwrap();
     }
+
+    #[test]
+    fn regression_806() {
+        let secp = secp256k1::Secp256k1::signing_only();
+        type Desc = Descriptor<DescriptorPublicKey>;
+        // OK
+        Desc::from_str("pkh(111111111111111111111111111111110000008375319363688624584A111111)")
+            .unwrap_err();
+        // ERR: crashes in translate_pk
+        Desc::parse_descriptor(
+            &secp,
+            "pkh(111111111111111111111111111111110000008375319363688624584A111111)",
+        )
+        .unwrap_err();
+    }
 }

src/lib.rs

@@ -82,9 +82,6 @@
 #![deny(non_camel_case_types)]
 #![deny(non_snake_case)]
 #![deny(unused_mut)]
-#![deny(dead_code)]
-#![deny(unused_imports)]
-#![deny(missing_docs)]
 // Clippy lints that we have disabled
 #![allow(clippy::iter_kv_map)] // https://github.com/rust-lang/rust-clippy/issues/11752
 
@@ -343,10 +340,22 @@ impl<E> TranslateErr<E> {
     ///
     /// This function will panic if the Error is OutError.
     pub fn expect_translator_err(self, msg: &str) -> E {
-        if let Self::TranslatorErr(v) = self {
-            v
-        } else {
-            panic!("{}", msg)
+        match self {
+            Self::TranslatorErr(v) => v,
+            Self::OuterError(ref e) => {
+                panic!("Unexpected Miniscript error when translating: {}\nMessage: {}", e, msg)
+            }
+        }
+    }
+}
+
+impl TranslateErr<Error> {
+    /// If we are doing a translation where our "outer error" is the generic
+    /// Miniscript error, eliminate the `TranslateErr` type which is just noise.
+    pub fn flatten(self) -> Error {
+        match self {
+            Self::TranslatorErr(e) => e,
+            Self::OuterError(e) => e,
         }
     }
 }