From 6ff58af74a7e019a5d4655c3141d0f35963a1bb2 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Tue, 29 Apr 2025 13:43:34 +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 668cdcab8..d9b31a707 100644 --- a/src/lib.rs +++ b/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!( From 0e40319b4384f310c0a787dd450b7bd49399d952 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Tue, 29 Apr 2025 21:45:36 +0000 Subject: [PATCH 2/5] ci: update CI job to run all the fuzz tests Just runs `./generate-files.sh`. --- .github/workflows/fuzz.yml | 10 +++++----- fuzz/Cargo.toml | 28 ++++++++++++++-------------- fuzz/fuzz-util.sh | 6 +++++- 3 files changed, 24 insertions(+), 20 deletions(-) diff --git a/.github/workflows/fuzz.yml b/.github/workflows/fuzz.yml index e3225591b..314a9ba78 100644 --- a/.github/workflows/fuzz.yml +++ b/.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 diff --git a/fuzz/Cargo.toml b/fuzz/Cargo.toml index 542183526..9080da553 100644 --- a/fuzz/Cargo.toml +++ b/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" 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 2f21e2331e185fe01f686c4f36f6bcf5247e0c6a 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 99d89f381..4fc89975a 100644 --- a/src/descriptor/mod.rs +++ b/src/descriptor/mod.rs @@ -695,12 +695,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 d9b31a707..7b4ee672c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -399,6 +399,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 1fed0ecd9c5903b9e55f5e5a3bb28b911955de27 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 4fc89975a..390d86140 100644 --- a/src/descriptor/mod.rs +++ b/src/descriptor/mod.rs @@ -2034,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; + // 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 7b4ee672c..23a99c8c7 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -391,10 +391,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 06400b136ac6409d8eb8a698a7eb69612c9ed9b1 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Mon, 21 Apr 2025 20:54:59 +0000 Subject: [PATCH 5/5] bump patch version of 10.2 --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index ca8c7d6d9..5bcbd6233 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "miniscript" -version = "10.2.0" +version = "10.2.1" authors = ["Andrew Poelstra , Sanket Kanjalkar "] license = "CC0-1.0" homepage = "https://github.com/rust-bitcoin/rust-miniscript/"