From aaf276cffbeba9d5903d6f53417e6d8841378dd3 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Sat, 3 May 2025 12:46:38 +0000 Subject: [PATCH 1/5] lib: remove some deny lints None of these belong in lib.rs, but these in particular have been changed in rustc so that they now trigger on code that used to be okay. This prevents me from running tests with recent rustcs on backports. --- src/lib.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index f62406902..6f1605a98 100644 --- a/src/lib.rs +++ b/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 From 7c651f47152481711d97fce38610983942f14e66 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Sat, 3 May 2025 12:47:03 +0000 Subject: [PATCH 2/5] ci: update fuzz CI job to reorder tests Just runs `./generate-files.sh` after tweaking it to sort the list. Otherwise my local CI chokes on the inconsistency between different 'find' orderings. --- .github/workflows/fuzz.yml | 8 ++++---- fuzz/Cargo.toml | 24 ++++++++++++------------ fuzz/fuzz-util.sh | 6 +++++- 3 files changed, 21 insertions(+), 17 deletions(-) diff --git a/.github/workflows/fuzz.yml b/.github/workflows/fuzz.yml index b00e258da..7ea77f17d 100644 --- a/.github/workflows/fuzz.yml +++ b/.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 diff --git a/fuzz/Cargo.toml b/fuzz/Cargo.toml index c4b76a35d..8fb077cf1 100644 --- a/fuzz/Cargo.toml +++ b/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" diff --git a/fuzz/fuzz-util.sh b/fuzz/fuzz-util.sh index 804e0da92..dcce45256 100755 --- a/fuzz/fuzz-util.sh +++ b/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 } From 4f8b065613b4829404db2175fd21c531ec3b306e Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Mon, 21 Apr 2025 20:53:38 +0000 Subject: [PATCH 3/5] descriptor: fix key parsing error handling in parse_desc --- src/descriptor/mod.rs | 9 +++------ src/lib.rs | 11 +++++++++++ 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/src/descriptor/mod.rs b/src/descriptor/mod.rs index a81a2786e..c6ad70adc 100644 --- a/src/descriptor/mod.rs +++ b/src/descriptor/mod.rs @@ -725,12 +725,9 @@ impl Descriptor { } let descriptor = Descriptor::::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)) } diff --git a/src/lib.rs b/src/lib.rs index 6f1605a98..cbd32aa9e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -348,6 +348,17 @@ impl TranslateErr { } } +impl TranslateErr { + /// 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, + } + } +} + impl From for TranslateErr { fn from(v: E) -> Self { Self::TranslatorErr(v) } } From 3bf002c66b46eb8098632d3b7ae03093fb729c78 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Mon, 21 Apr 2025 20:46:11 +0000 Subject: [PATCH 4/5] add regression test for #806 When parsing a descriptor with `Descriptor::parse_descriptor`, we first parse as a string and then parse the keys. We fail to consider parsing errors in the keys, resulting in a panic. Also, clean up the panic message so it's clearer what's going on. --- src/descriptor/mod.rs | 15 +++++++++++++++ src/lib.rs | 9 +++++---- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/src/descriptor/mod.rs b/src/descriptor/mod.rs index c6ad70adc..feea99161 100644 --- a/src/descriptor/mod.rs +++ b/src/descriptor/mod.rs @@ -2050,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; + // OK + Desc::from_str("pkh(111111111111111111111111111111110000008375319363688624584A111111)") + .unwrap_err(); + // ERR: crashes in translate_pk + Desc::parse_descriptor( + &secp, + "pkh(111111111111111111111111111111110000008375319363688624584A111111)", + ) + .unwrap_err(); + } } diff --git a/src/lib.rs b/src/lib.rs index cbd32aa9e..0f2fec080 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -340,10 +340,11 @@ impl TranslateErr { /// /// 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) + } } } } From 953e679f4da7a3645e0756c5a18de5bf38ce017d Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Sat, 3 May 2025 12:47:21 +0000 Subject: [PATCH 5/5] bump patch version of 11.2 --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index f57dc899d..d8eadcdee 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "miniscript" -version = "11.2.0" +version = "11.2.1" authors = ["Andrew Poelstra , Sanket Kanjalkar "] license = "CC0-1.0" homepage = "https://github.com/rust-bitcoin/rust-miniscript/"