From de4e2ec8b20ce371f8a407eb004b369c2822b2a0 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Thu, 8 Aug 2024 14:49:02 +0000 Subject: [PATCH 01/11] cargo: lint unused cfg parameters --- Cargo.toml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Cargo.toml b/Cargo.toml index 6aa99e8a..ceb46a2f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -35,6 +35,8 @@ bitcoin = { version = "0.31.0", features = ["base64"] } secp256k1 = {version = "0.28.0", features = ["rand-std"]} actual-base64 = { package = "base64", version = "0.13.0" } +[lints.rust] +unexpected_cfgs = { level = "deny", check-cfg = ['cfg(miniscript_bench)'] } [[example]] name = "htlc" From 4bab769d508d36139aa03875b9af3bbb09060152 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Thu, 8 Aug 2024 14:52:12 +0000 Subject: [PATCH 02/11] rustfmt: change to new notation --- examples/sign_multisig.rs | 2 +- src/descriptor/csfs_cov/script_internals.rs | 38 ++++++++++----------- src/miniscript/ms_tests.rs | 10 +++--- 3 files changed, 25 insertions(+), 25 deletions(-) diff --git a/examples/sign_multisig.rs b/examples/sign_multisig.rs index 2f046906..d4a118d1 100644 --- a/examples/sign_multisig.rs +++ b/examples/sign_multisig.rs @@ -46,7 +46,7 @@ fn main() { }], }; - #[cfg_attr(feature="cargo-fmt", rustfmt_skip)] + #[rustfmt::skip] let public_keys = vec![ bitcoin::PublicKey::from_slice(&[2; 33]).expect("key 1"), bitcoin::PublicKey::from_slice(&[ diff --git a/src/descriptor/csfs_cov/script_internals.rs b/src/descriptor/csfs_cov/script_internals.rs index 35dc3dc8..4968cf0b 100644 --- a/src/descriptor/csfs_cov/script_internals.rs +++ b/src/descriptor/csfs_cov/script_internals.rs @@ -67,7 +67,10 @@ impl CovOperations for script::Builder { .push_opcode(all::OP_ENDIF) } + #[rustfmt::skip] fn verify_cov(self, key: &bitcoin::PublicKey) -> Self { + use elements::opcodes::all::{OP_CAT, OP_SWAP}; + let mut builder = self; // The miniscript is of type B, which should have pushed 1 // onto the stack if it satisfied correctly.(which it should) @@ -100,25 +103,22 @@ impl CovOperations for script::Builder { builder = builder.push_opcode(all::OP_TOALTSTACK); // alt_stk = [bitcoinsig] // stk = [ecsig i10 i9 i8 i7 i6 i5 i4 i3b i3 i2 i1] - // Ignore fmt skip because it butchers these lines - #[cfg_attr(feature="cargo-fmt", rustfmt_skip)] - { - // Do the size checks on all respective items in sighash calculation - use elements::opcodes::all::{OP_CAT, OP_SWAP}; - builder = builder.chk_size(4).push_opcode(OP_SWAP); // item 1: ver - builder = builder.chk_size(32).push_opcode(OP_CAT).push_opcode(OP_SWAP);//item 2: hashprevouts - builder = builder.chk_size(32).push_opcode(OP_CAT).push_opcode(OP_SWAP);//item 3: hashsequence - builder = builder.chk_size(32).push_opcode(OP_CAT).push_opcode(OP_SWAP);//item 3b: hashissuances - builder = builder.chk_size(36).push_opcode(OP_CAT).push_opcode(OP_SWAP);//item 4: outpoint - // Item 5: Script code is of constant size because we only consider everything after - // codeseparator. This will be replaced with a push slice in a later commit - builder = builder.chk_size(3).push_opcode(OP_CAT).push_opcode(OP_SWAP);//item 5: script code - builder = builder.chk_amt().push_opcode(OP_CAT).push_opcode(OP_SWAP); //item 6: check confAmt - builder = builder.chk_size(4).push_opcode(OP_CAT).push_opcode(OP_SWAP); //item 7: sequence - builder = builder.chk_size(32).push_opcode(OP_CAT).push_opcode(OP_SWAP);//item 8: hashoutputs - builder = builder.chk_size(4).push_opcode(OP_CAT).push_opcode(OP_SWAP); //item 9: nlocktime - builder = builder.chk_size(4).push_opcode(OP_CAT); //item 10: sighash type - } + + // Do the size checks on all respective items in sighash calculation + builder = builder.chk_size(4).push_opcode(OP_SWAP); // item 1: ver + builder = builder.chk_size(32).push_opcode(OP_CAT).push_opcode(OP_SWAP);//item 2: hashprevouts + builder = builder.chk_size(32).push_opcode(OP_CAT).push_opcode(OP_SWAP);//item 3: hashsequence + builder = builder.chk_size(32).push_opcode(OP_CAT).push_opcode(OP_SWAP);//item 3b: hashissuances + builder = builder.chk_size(36).push_opcode(OP_CAT).push_opcode(OP_SWAP);//item 4: outpoint + // Item 5: Script code is of constant size because we only consider everything after + // codeseparator. This will be replaced with a push slice in a later commit + builder = builder.chk_size(3).push_opcode(OP_CAT).push_opcode(OP_SWAP); //item 5: script code + builder = builder.chk_amt().push_opcode(OP_CAT).push_opcode(OP_SWAP); //item 6: check confAmt + builder = builder.chk_size(4).push_opcode(OP_CAT).push_opcode(OP_SWAP); //item 7: sequence + builder = builder.chk_size(32).push_opcode(OP_CAT).push_opcode(OP_SWAP);//item 8: hashoutputs + builder = builder.chk_size(4).push_opcode(OP_CAT).push_opcode(OP_SWAP); //item 9: nlocktime + builder = builder.chk_size(4).push_opcode(OP_CAT); //item 10: sighash type + // Now sighash is on the top of the stack // alt_stk = [bitcoinsig] // stk = [ecsig (i1||i2||i3||i3b||i4||i5||i6||i7||i8||i9||i10)] diff --git a/src/miniscript/ms_tests.rs b/src/miniscript/ms_tests.rs index c105708c..e5f453fb 100644 --- a/src/miniscript/ms_tests.rs +++ b/src/miniscript/ms_tests.rs @@ -68,7 +68,7 @@ mod tests { } #[test] - #[cfg_attr(feature="cargo-fmt", rustfmt_skip)] + #[rustfmt::skip] fn invalid_tests_from_alloy() { invalid_ms("or_b(or_i(0,sha256(926a54995ca48600920a19bf7bc502ca5f2f7d07e6f804c4f00ebf0325084dbc)),after(1))"); invalid_ms("or_b(s:pk_h(A),after(500000001))"); @@ -5646,7 +5646,7 @@ mod tests { invalid_ms("c:or_b(sha256(926a54995ca48600920a19bf7bc502ca5f2f7d07e6f804c4f00ebf0325084dbc),pk_k(A))"); } #[test] - #[cfg_attr(feature="cargo-fmt", rustfmt_skip)] + #[rustfmt::skip] fn mall_8f1e8_tests_from_alloy() { ms_test("or_d(or_d(sha256(926a54995ca48600920a19bf7bc502ca5f2f7d07e6f804c4f00ebf0325084dbc),sha256(926a54995ca48600920a19bf7bc502ca5f2f7d07e6f804c4f00ebf0325084dbc)),after(500000001))", "Bf"); ms_test("andor(sha256(926a54995ca48600920a19bf7bc502ca5f2f7d07e6f804c4f00ebf0325084dbc),or_d(multi(2,A,B,C),sha256(926a54995ca48600920a19bf7bc502ca5f2f7d07e6f804c4f00ebf0325084dbc)),after(1))", "B"); @@ -9143,7 +9143,7 @@ mod tests { } #[test] - #[cfg_attr(feature="cargo-fmt", rustfmt_skip)] + #[rustfmt::skip] fn main_tests_from_alloy() { ms_test("or_d(or_d(multi(2,A,B,C),or_d(multi(2,D,E,F),multi(2,G,I,J))),multi(2,K,L,M))", "Bdusem"); ms_test("andor(multi(2,A,B,C),or_d(multi(2,D,E,F),sha256(926a54995ca48600920a19bf7bc502ca5f2f7d07e6f804c4f00ebf0325084dbc)),c:pk_h(G))", "Bdusem"); @@ -15044,7 +15044,7 @@ mod tests { } #[test] - #[cfg_attr(feature="cargo-fmt", rustfmt_skip)] + #[rustfmt::skip] fn malleable_tests_from_alloy() { ms_test("and_v(v:after(500000001),or_d(j:multi(2,A,B,C),multi(2,D,E,F)))", "usB"); ms_test("or_b(j:multi(2,A,B,C),a:andor(multi(2,D,E,F),multi(2,G,I,J),multi(2,K,L,M)))", "dBesu"); @@ -22076,8 +22076,8 @@ mod tests { // This does not actually test timelock mixing. See: https://github.com/rust-bitcoin/rust-miniscript/issues/514 // for details #[test] + #[rustfmt::skip] fn conflict_tests_from_alloy() { - #[cfg_attr(feature="cargo-fmt", rustfmt_skip)] { ms_test("andor(multi(2,A,B,C),andor(multi(2,D,E,F),after(500000001),n:after(1)),0)","Bedsm"); ms_test("and_v(v:after(500000001),or_d(multi(2,A,B,C),and_b(multi(2,D,E,F),a:after(1))))","Busm"); From 9815ca7384a792434fca9e4d2f9f2dbf1afcde3a Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Thu, 8 Aug 2024 14:53:11 +0000 Subject: [PATCH 03/11] rustc: silence some "dead code" warnings --- src/lib.rs | 2 -- src/psbt/mod.rs | 2 ++ 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index a1119898..3e1a49e6 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -125,8 +125,6 @@ mod macros; #[macro_use] mod pub_macros; -pub use pub_macros::*; - pub mod confidential; pub mod descriptor; pub mod expression; diff --git a/src/psbt/mod.rs b/src/psbt/mod.rs index b5528ffc..8694c192 100644 --- a/src/psbt/mod.rs +++ b/src/psbt/mod.rs @@ -1088,7 +1088,9 @@ trait PsbtFields { fn tap_key_origins( &mut self, ) -> &mut BTreeMap, bip32::KeySource)>; + #[allow(dead_code)] fn proprietary(&mut self) -> &mut BTreeMap>; + #[allow(dead_code)] fn unknown(&mut self) -> &mut BTreeMap>; // `tap_tree` only appears in psbt::Output, so it's returned as an option of a mutable ref From bb88c18d1bc9b2de086e249d70fc7bf21c2684ce Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Thu, 8 Aug 2024 15:01:36 +0000 Subject: [PATCH 04/11] clippy: replace fold calls with try_fold --- src/descriptor/key.rs | 3 +-- src/miniscript/types/extra_props.rs | 31 ++++++++++++----------------- 2 files changed, 14 insertions(+), 20 deletions(-) diff --git a/src/descriptor/key.rs b/src/descriptor/key.rs index 0aa445b7..fd3fc525 100644 --- a/src/descriptor/key.rs +++ b/src/descriptor/key.rs @@ -874,8 +874,7 @@ fn parse_xkey_deriv( // step all the vectors of indexes contain a single element. If it did though, one of the // vectors contains more than one element. // Now transform this list of vectors of steps into distinct derivation paths. - .fold(Ok(Vec::new()), |paths, index_list| { - let mut paths = paths?; + .try_fold(Vec::new(), |mut paths, index_list| { let mut index_list = index_list?.into_iter(); let first_index = index_list .next() diff --git a/src/miniscript/types/extra_props.rs b/src/miniscript/types/extra_props.rs index bd5f4a69..9806b7f7 100644 --- a/src/miniscript/types/extra_props.rs +++ b/src/miniscript/types/extra_props.rs @@ -51,7 +51,7 @@ impl OpLimits { /// Worst case opcode count when this element is satisfied pub fn op_count(&self) -> Option { - opt_add(Some(self.count), self.sat) + self.sat.map(|sat| self.count + sat) } } @@ -821,11 +821,11 @@ impl Property for ExtData { .iter() .rev() .enumerate() - .fold(Some(0), |acc, (i, &(x, y))| { + .try_fold(0, |acc, (i, &(x, y))| { if i <= k { - opt_add(acc, x) + x.map(|x| x + acc) } else { - opt_add(acc, y) + y.map(|y| y + acc) } }); @@ -834,11 +834,11 @@ impl Property for ExtData { .iter() .rev() .enumerate() - .fold(Some(0), |acc, (i, &(x, y))| { + .try_fold(0, |acc, (i, &(x, y))| { if i <= k { - opt_max(acc, x) + x.map(|x| cmp::max(x, acc)) } else { - opt_max(acc, y) + y.map(|y| cmp::max(y, acc)) } }); @@ -848,11 +848,11 @@ impl Property for ExtData { max_sat_size_vec .iter() .enumerate() - .fold(Some((0, 0)), |acc, (i, &(x, y))| { + .try_fold((0, 0), |acc, (i, &(x, y))| { if i <= k { - opt_tuple_add(acc, x) + x.map(|x| (acc.0 + x.0, acc.1 + x.1)) } else { - opt_tuple_add(acc, y) + y.map(|y| (acc.0 + y.0, acc.1 + y.1)) } }); @@ -861,11 +861,11 @@ impl Property for ExtData { ops_count_sat_vec .iter() .enumerate() - .fold(Some(0), |acc, (i, &(x, y))| { + .try_fold(0, |acc, (i, &(x, y))| { if i <= k { - opt_add(acc, x) + x.map(|x| x + acc) } else { - opt_add(acc, Some(y)) + Some(y + acc) } }); @@ -1083,11 +1083,6 @@ fn opt_add(a: Option, b: Option) -> Option { a.and_then(|x| b.map(|y| x + y)) } -/// Returns Some((x0+y0, x1+y1)) is both x and y are Some. Otherwise, returns `None`. -fn opt_tuple_add(a: Option<(usize, usize)>, b: Option<(usize, usize)>) -> Option<(usize, usize)> { - a.and_then(|x| b.map(|(w, s)| (w + x.0, s + x.1))) -} - #[cfg(test)] mod tests { use super::*; From fcb2c3d0e7b0c1e41569f14b6d76dc255977dcb0 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Thu, 8 Aug 2024 15:17:56 +0000 Subject: [PATCH 05/11] clippy: canonical partial_cmp implementations --- src/descriptor/tr.rs | 6 +----- src/extensions/param.rs | 4 +--- src/policy/compiler.rs | 2 +- 3 files changed, 3 insertions(+), 9 deletions(-) diff --git a/src/descriptor/tr.rs b/src/descriptor/tr.rs index 2096e789..1139b95a 100644 --- a/src/descriptor/tr.rs +++ b/src/descriptor/tr.rs @@ -87,11 +87,7 @@ impl Eq for Tr {} impl PartialOrd for Tr { fn partial_cmp(&self, other: &Self) -> Option { - match self.internal_key.partial_cmp(&other.internal_key) { - Some(cmp::Ordering::Equal) => {} - ord => return ord, - } - self.tree.partial_cmp(&other.tree) + Some(self.cmp(other)) } } diff --git a/src/extensions/param.rs b/src/extensions/param.rs index 76f0a8c9..b2ebe512 100644 --- a/src/extensions/param.rs +++ b/src/extensions/param.rs @@ -132,9 +132,7 @@ impl CovExtArgs { impl PartialOrd for CovExtArgs { fn partial_cmp(&self, other: &Self) -> Option { - // HACKY implementation, need Ord/PartialOrd to make it work with other components - // in the library - self.to_string().partial_cmp(&other.to_string()) + Some(self.cmp(other)) } } diff --git a/src/policy/compiler.rs b/src/policy/compiler.rs index 7e1e0fc4..ceff7a49 100644 --- a/src/policy/compiler.rs +++ b/src/policy/compiler.rs @@ -34,7 +34,7 @@ impl Eq for OrdF64 {} // to derive both or neither. Better to be explicit. impl PartialOrd for OrdF64 { fn partial_cmp(&self, other: &OrdF64) -> Option { - self.0.partial_cmp(&other.0) + Some(self.cmp(other)) } } impl Ord for OrdF64 { From 8894215cd959a13e16c230f5fa1b9bd77e4b6bd5 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Thu, 8 Aug 2024 15:04:08 +0000 Subject: [PATCH 06/11] clippy: miscellaneous simple fixes --- src/confidential/mod.rs | 2 +- src/descriptor/csfs_cov/mod.rs | 7 ------- src/descriptor/key.rs | 2 +- src/descriptor/mod.rs | 1 - src/descriptor/pegin/dynafed_pegin.rs | 2 +- src/descriptor/tr.rs | 2 +- src/miniscript/context.rs | 2 +- src/miniscript/satisfy.rs | 2 +- src/simplicity.rs | 4 +--- 9 files changed, 7 insertions(+), 17 deletions(-) diff --git a/src/confidential/mod.rs b/src/confidential/mod.rs index 2935c46f..992b480a 100644 --- a/src/confidential/mod.rs +++ b/src/confidential/mod.rs @@ -408,7 +408,7 @@ mod tests { ConfidentialTest { key: Key::Bare(single_ct_key.clone()), descriptor: crate::Descriptor::new_wpkh(single_spk_key).unwrap(), - descriptor_str: format!("ct(02dce16018bbbb8e36de7b394df5b5166e9adb7498be7d881a85a09aeecf76b623,elwpkh(03774eec7a3d550d18e9f89414152025b3b0ad6a342b19481f702d843cff06dfc4))#h5e0p6m9"), + descriptor_str: "ct(02dce16018bbbb8e36de7b394df5b5166e9adb7498be7d881a85a09aeecf76b623,elwpkh(03774eec7a3d550d18e9f89414152025b3b0ad6a342b19481f702d843cff06dfc4))#h5e0p6m9".to_string(), conf_addr: "lq1qq0r6pegudzm0tzpszelc34qjln4fdxawgwmgnza63wwpzdy6jrm0grmqvvk2ce5ksnxcs9ecgtnryt7xg3406y5ccl0k2glns", unconf_addr: "ex1qpasxxt9vv6tgfnvgzuuy9e3j9lryg6hawrval4", }, diff --git a/src/descriptor/csfs_cov/mod.rs b/src/descriptor/csfs_cov/mod.rs index bb96e4a9..c609f592 100644 --- a/src/descriptor/csfs_cov/mod.rs +++ b/src/descriptor/csfs_cov/mod.rs @@ -373,13 +373,6 @@ mod tests { .to_string(), "ert1qamjdykcfzkcsvc9z32a6qcz3mwr85a3k7z7qf2uaufem2q3lsjxqj4y4fy" ); - - println!( - "{}", - desc.address(&elements::AddressParams::ELEMENTS) - .unwrap() - .to_string() - ); } #[test] fn spend_tx() { diff --git a/src/descriptor/key.rs b/src/descriptor/key.rs index fd3fc525..3d325943 100644 --- a/src/descriptor/key.rs +++ b/src/descriptor/key.rs @@ -947,7 +947,7 @@ impl DescriptorXKey { Some((fingerprint, ref path)) => ( fingerprint, path.into_iter() - .chain(self.derivation_path.into_iter()) + .chain(&self.derivation_path) .collect(), ), None => ( diff --git a/src/descriptor/mod.rs b/src/descriptor/mod.rs index 2d37aef9..86afc3b2 100644 --- a/src/descriptor/mod.rs +++ b/src/descriptor/mod.rs @@ -1016,7 +1016,6 @@ impl Descriptor { /// /// For multipath descriptors it will return as many descriptors as there is /// "parallel" paths. For regular descriptors it will just return itself. - #[allow(clippy::blocks_in_if_conditions)] pub fn into_single_descriptors(self) -> Result, Error> { // All single-path descriptors contained in this descriptor. let mut descriptors = Vec::new(); diff --git a/src/descriptor/pegin/dynafed_pegin.rs b/src/descriptor/pegin/dynafed_pegin.rs index 002cfd21..964db112 100644 --- a/src/descriptor/pegin/dynafed_pegin.rs +++ b/src/descriptor/pegin/dynafed_pegin.rs @@ -297,7 +297,7 @@ where C: secp256k1_zkp::Verification, { fn pk(&mut self, pk: &Pk) -> Result { - Ok(tweak_key(&pk.to_public_key(), self.1, &self.0[..])) + Ok(tweak_key(&pk.to_public_key(), self.1, self.0)) } // We don't need to implement these methods as we are not using them in the policy. diff --git a/src/descriptor/tr.rs b/src/descriptor/tr.rs index 1139b95a..2b7b5c2f 100644 --- a/src/descriptor/tr.rs +++ b/src/descriptor/tr.rs @@ -973,7 +973,7 @@ mod tests { assert!(!tr.for_each_key(|k| k.starts_with("acc"))); } - fn verify_from_str<'a>(desc_str: &str, internal_key: &str, scripts: &[TapLeafScript<'a, String, NoExt>]) { + fn verify_from_str(desc_str: &str, internal_key: &str, scripts: &[TapLeafScript]) { let desc = Tr::::from_str(desc_str).unwrap(); assert_eq!(desc_str, &desc.to_string()); assert_eq!(internal_key, &desc.internal_key); diff --git a/src/miniscript/context.rs b/src/miniscript/context.rs index 11b431aa..dafc8247 100644 --- a/src/miniscript/context.rs +++ b/src/miniscript/context.rs @@ -757,7 +757,7 @@ impl ScriptContext for BareCtx { } match ms.node { Terminal::PkK(ref key) if key.is_x_only_key() => { - return Err(ScriptContextError::XOnlyKeysNotAllowed( + Err(ScriptContextError::XOnlyKeysNotAllowed( key.to_string(), Self::name_str(), )) diff --git a/src/miniscript/satisfy.rs b/src/miniscript/satisfy.rs index 7261e12d..60217b2a 100644 --- a/src/miniscript/satisfy.rs +++ b/src/miniscript/satisfy.rs @@ -9,7 +9,7 @@ use std::collections::{BTreeMap, HashMap}; use std::sync::Arc; -use std::{cmp, i64, mem}; +use std::{cmp, mem}; use bitcoin::hashes::hash160; use bitcoin::secp256k1::XOnlyPublicKey; diff --git a/src/simplicity.rs b/src/simplicity.rs index d42504e6..8068f3e8 100644 --- a/src/simplicity.rs +++ b/src/simplicity.rs @@ -98,9 +98,7 @@ impl_from_str!( // We cannot use our wrapper because we don't own the Policy (we have a reference) // Implementing a wrapper of Cow<'a, Policy> leads to lifetime issues // when implementing ForEachKey, because for_each_key() has its own lifetime 'a -pub fn for_each_key<'a, Pk: MiniscriptKey, F: FnMut(&'a Pk) -> bool>(policy: &'a Policy, mut pred: F) -> bool -where - Pk: 'a, +pub fn for_each_key<'a, Pk: MiniscriptKey + 'a, F: FnMut(&'a Pk) -> bool>(policy: &'a Policy, mut pred: F) -> bool { let mut stack = vec![policy]; From 97b22294f1f0de793c9321e8fcf0ca2dffb8ddbc Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Thu, 8 Aug 2024 15:21:14 +0000 Subject: [PATCH 07/11] clippy: unnecessary vecs --- examples/sign_multisig.rs | 2 +- src/policy/concrete.rs | 2 +- src/psbt/mod.rs | 1 + 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/examples/sign_multisig.rs b/examples/sign_multisig.rs index d4a118d1..9a683325 100644 --- a/examples/sign_multisig.rs +++ b/examples/sign_multisig.rs @@ -47,7 +47,7 @@ fn main() { }; #[rustfmt::skip] - let public_keys = vec![ + let public_keys = [ bitcoin::PublicKey::from_slice(&[2; 33]).expect("key 1"), bitcoin::PublicKey::from_slice(&[ 0x02, diff --git a/src/policy/concrete.rs b/src/policy/concrete.rs index 97ff05f6..8bf1b859 100644 --- a/src/policy/concrete.rs +++ b/src/policy/concrete.rs @@ -1264,7 +1264,7 @@ fn with_huffman_tree( /// any one of the conditions exclusively. #[cfg(feature = "compiler")] fn generate_combination( - policy_vec: &Vec>>, + policy_vec: &[Arc>], prob: f64, k: usize, ) -> Vec<(f64, Arc>)> { diff --git a/src/psbt/mod.rs b/src/psbt/mod.rs index 8694c192..06630816 100644 --- a/src/psbt/mod.rs +++ b/src/psbt/mod.rs @@ -443,6 +443,7 @@ impl<'psbt, Pk: MiniscriptKey + ToPublicKey> Satisfier for PsbtInputSatisfie } } +#[allow(clippy::ptr_arg)] // complains about &Vec but this is used in a closure context fn try_vec_as_preimage32(vec: &Vec) -> Option { if vec.len() == 32 { let mut arr = [0u8; 32]; From 8bc9311b737dc94ebeb19286e95d2f309db8fa7d Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Thu, 8 Aug 2024 15:22:10 +0000 Subject: [PATCH 08/11] doc: improve list of fields covered by sighash This will result in the rendered numbering being different from the numbering in the source code, but that's probably better than the existing situation where the "3b" item does not show up as an independent list item. There's no good way I can see to fix this because of limitations of rustfmt's list facilities. And anyway this is legacy code. So just do something that makes clippy happy. --- src/descriptor/csfs_cov/cov.rs | 2 +- src/descriptor/csfs_cov/mod.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/descriptor/csfs_cov/cov.rs b/src/descriptor/csfs_cov/cov.rs index c2274301..2c6a5a08 100644 --- a/src/descriptor/csfs_cov/cov.rs +++ b/src/descriptor/csfs_cov/cov.rs @@ -25,7 +25,7 @@ //! 1. nVersion of the transaction (4-byte little endian) //! 2. hashPrevouts (32-byte hash) //! 3. hashSequence (32-byte hash) -//! 3b. ELEMENTS EXTRA hashIssuances (32-byte hash) +//! 3. ELEMENTS EXTRA hashIssuances (32-byte hash) //! 4. outpoint (32-byte hash + 4-byte little endian) //! 5. scriptCode of the input (serialized as scripts inside CTxOuts) //! 6. value of the output spent by this input (8-byte little endian) diff --git a/src/descriptor/csfs_cov/mod.rs b/src/descriptor/csfs_cov/mod.rs index c609f592..b1295dd3 100644 --- a/src/descriptor/csfs_cov/mod.rs +++ b/src/descriptor/csfs_cov/mod.rs @@ -26,7 +26,7 @@ //! 1. nVersion of the transaction (4-byte little endian) //! 2. hashPrevouts (32-byte hash) //! 3. hashSequence (32-byte hash) -//! 3b. ELEMENTS EXTRA hashIssuances (32-byte hash) +//! 3. ELEMENTS EXTRA hashIssuances (32-byte hash) //! 4. outpoint (32-byte hash + 4-byte little endian) //! 5. scriptCode of the input (serialized as scripts inside CTxOuts) //! 6. value of the output spent by this input (8-byte little endian) From 7c977b5367a6c025aec39d4bb7ab9c1daa9c7802 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Thu, 8 Aug 2024 15:35:37 +0000 Subject: [PATCH 09/11] ForEachKey: clean up Concrete impl, remove Semantic impl The Concrete impl used a weird complier gitch `let pred = |x| pred(x)` to avoid an infinite recursion bug (which in turn triggered a compiler bug that they didn't fix even though I filed it a DAY AFTER IT WAS INTRODUCED ON NIGHTLY and instead they waited 3 months and released it on stable forcing me to backport a workaround to a gazillion versions of rust-miniscript). The bug was https://github.com/rust-lang/rust/issues/110475 BTW, just to make sure this commit triggers another notification on that issue. Clippy doesn't like this. Clippy is obviously wrong but rather than having this stupid fight again just switch to a different idiom. Meanwhile, the Semantic impl of ForEachKey panics iff any keys are present, regardless of the closure passed to it. This is funny but completely useless and we should just remove it. --- src/policy/concrete.rs | 13 ++++++++----- src/policy/semantic.rs | 22 +--------------------- 2 files changed, 9 insertions(+), 26 deletions(-) diff --git a/src/policy/concrete.rs b/src/policy/concrete.rs index 8bf1b859..ab89af78 100644 --- a/src/policy/concrete.rs +++ b/src/policy/concrete.rs @@ -662,7 +662,12 @@ impl ForEachKey for Policy { where Pk: 'a, { - let mut pred = |x| pred(x); + self.for_each_key_internal(&mut pred) + } +} + +impl Policy { + fn for_each_key_internal<'a, F: FnMut(&'a Pk) -> bool>(&'a self, pred: &mut F) -> bool { match *self { Policy::Unsatisfiable | Policy::Trivial => true, Policy::Key(ref pk) => pred(pk), @@ -673,14 +678,12 @@ impl ForEachKey for Policy { | Policy::After(..) | Policy::Older(..) => true, Policy::Threshold(_, ref subs) | Policy::And(ref subs) => { - subs.iter().all(|sub| sub.for_each_key(&mut pred)) + subs.iter().all(|sub| sub.for_each_key_internal(pred)) } - Policy::Or(ref subs) => subs.iter().all(|(_, sub)| sub.for_each_key(&mut pred)), + Policy::Or(ref subs) => subs.iter().all(|(_, sub)| sub.for_each_key_internal(pred)), } } -} -impl Policy { /// Convert a policy using one kind of public key to another /// type of public key /// diff --git a/src/policy/semantic.rs b/src/policy/semantic.rs index 1dbb30a9..3e7e5325 100644 --- a/src/policy/semantic.rs +++ b/src/policy/semantic.rs @@ -10,7 +10,7 @@ use elements::{LockTime, Sequence}; use super::concrete::PolicyError; use super::ENTAILMENT_MAX_TERMINALS; -use crate::{errstr, expression, AbsLockTime, Error, ForEachKey, MiniscriptKey, Translator}; +use crate::{errstr, expression, AbsLockTime, Error, MiniscriptKey, Translator}; /// Abstract policy which corresponds to the semantics of a Miniscript /// and which allows complex forms of analysis, e.g. filtering and @@ -59,26 +59,6 @@ where } } -impl ForEachKey for Policy { - fn for_each_key<'a, F: FnMut(&'a Pk) -> bool>(&'a self, mut pred: F) -> bool - where - Pk: 'a, - { - let mut pred = |x| pred(x); - match *self { - Policy::Unsatisfiable | Policy::Trivial => true, - Policy::Key(ref _pkh) => todo!("Semantic Policy KeyHash must store Pk"), - Policy::Sha256(..) - | Policy::Hash256(..) - | Policy::Ripemd160(..) - | Policy::Hash160(..) - | Policy::After(..) - | Policy::Older(..) => true, - Policy::Threshold(_, ref subs) => subs.iter().all(|sub| sub.for_each_key(&mut pred)), - } - } -} - impl Policy { /// Convert a policy using one kind of public key to another /// type of public key From b10c7a274e8fd79a0693c6efed8fb6eafbe5ed3f Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Thu, 8 Aug 2024 14:43:41 +0000 Subject: [PATCH 10/11] ci: add one more pin We should really move to a system of lockfiles (and more generally move to the rust-bitcoin-maintainer-tools set of scripts since the existing scripts are badly broken -- a "fuzz" job that sets an unused DO_LINT variable and never fuzzes; a DO_FMT flag in contrib/test.sh which is never set, etc) but this is an easy fix for now. --- contrib/test.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/contrib/test.sh b/contrib/test.sh index 71236ce4..e860f36b 100755 --- a/contrib/test.sh +++ b/contrib/test.sh @@ -11,6 +11,7 @@ rustc --version if cargo --version | grep "1\.58"; then cargo update -p byteorder --precise 1.4.3 cargo update -p cc --precise 1.0.94 + cargo update -p ppv-lite86 --precise 0.2.17 fi # Format if told to From e151a85e0227246eb3c3552c9037ab41e272dfaa Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Thu, 8 Aug 2024 16:18:56 +0000 Subject: [PATCH 11/11] ci: bump fuzz MSRV to 1.65 This is just copied from rust-bitcoin --- .github/workflows/fuzz.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/fuzz.yml b/.github/workflows/fuzz.yml index 93cf2a56..470bf1ac 100644 --- a/.github/workflows/fuzz.yml +++ b/.github/workflows/fuzz.yml @@ -40,7 +40,7 @@ roundtrip_confidential, key: cache-${{ matrix.target }}-${{ hashFiles('**/Cargo.toml','**/Cargo.lock') }} - uses: actions-rs/toolchain@v1 with: - toolchain: 1.58 + toolchain: 1.65 override: true profile: minimal - name: fuzz