Merge #814: Backport #809 to 0.10.x

06400b1 bump patch version of 10.2 (Andrew Poelstra)
1fed0ec add regression test for #806 (Andrew Poelstra)
2f21e23 descriptor: fix key parsing error handling in parse_desc (Andrew Poelstra)
0e40319 ci: update CI job to run all the fuzz tests (Andrew Poelstra)
6ff58af lib: remove some deny lints (Andrew Poelstra)

Pull request description:

  Backports #809 to 10.x. This is a direct rebase of #810 except that I added a commit with some lint fixes. (I had been avoiding this, but we've been having a lot of backports lately so I think I ought to just address it.)

  After this I will do 11.x.

ACKs for top commit:
  sanket1729:
    utACK 06400b1

Tree-SHA512: 6a30480becf20aa64e1f4528cf0d1df75bd2ddbaad95ae6bd4a3f5885624a791c7d7a77020b27675c348b2ff95467e0cb8bb42ca2c58112d7322f75480608dca

Author: apoelstra

.github/workflows/fuzz.yml

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

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "miniscript"
-version = "10.2.0"
+version = "10.2.1"
 authors = ["Andrew Poelstra <[email protected]>, Sanket Kanjalkar <[email protected]>"]
 license = "CC0-1.0"
 homepage = "https://github.com/rust-bitcoin/rust-miniscript/"

fuzz/Cargo.toml

@@ -15,33 +15,33 @@ miniscript = { path = "..", features = [ "compiler" ] }
 regex = "1.4"
 
 [[bin]]
-name = "roundtrip_miniscript_str"
-path = "fuzz_targets/roundtrip_miniscript_str.rs"
-
-[[bin]]
-name = "roundtrip_miniscript_script"
-path = "fuzz_targets/roundtrip_miniscript_script.rs"
+name = "compile_descriptor"
+path = "fuzz_targets/compile_descriptor.rs"
 
 [[bin]]
 name = "parse_descriptor"
 path = "fuzz_targets/parse_descriptor.rs"
 
-[[bin]]
-name = "roundtrip_semantic"
-path = "fuzz_targets/roundtrip_semantic.rs"
-
 [[bin]]
 name = "parse_descriptor_secret"
 path = "fuzz_targets/parse_descriptor_secret.rs"
 
+[[bin]]
+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

@@ -695,12 +695,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))
     }
@@ -2037,4 +2034,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

@@ -81,9 +81,6 @@
 #![deny(non_camel_case_types)]
 #![deny(non_snake_case)]
 #![deny(unused_mut)]
-#![deny(dead_code)]
-#![deny(unused_imports)]
-#![deny(missing_docs)]
 
 #[cfg(target_pointer_width = "16")]
 compile_error!(
@@ -394,10 +391,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,
         }
     }
 }