Skip to content

Fix a bunch of clippy lints #627

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Nov 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions clippy.toml
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
msrv = "1.48.0"
# plan API returns Self as an error type for an large-ish enum
large-error-threshold = 256
4 changes: 2 additions & 2 deletions src/descriptor/checksum.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,11 +114,11 @@ impl Engine {
let mut chars = [0 as char; CHECKSUM_LENGTH];
let mut checksum_remaining = CHECKSUM_LENGTH;

for j in 0..CHECKSUM_LENGTH {
for checksum_ch in &mut chars {
checksum_remaining -= 1;
let unpacked = self.inner.residue().unpack(checksum_remaining);
let fe = Fe32::try_from(unpacked).expect("5 bits fits in an fe32");
chars[j] = fe.to_char();
*checksum_ch = fe.to_char();
}
chars
}
Expand Down
14 changes: 5 additions & 9 deletions src/descriptor/key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -589,7 +589,7 @@ impl DescriptorPublicKey {
};
xpub.derivation_paths
.paths()
.into_iter()
.iter()
.map(|p| origin_path.extend(p))
.collect()
}
Expand Down Expand Up @@ -852,8 +852,7 @@ fn parse_xkey_deriv<K: InnerXKey>(
// 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()
Expand Down Expand Up @@ -940,12 +939,9 @@ impl<K: InnerXKey> DescriptorXKey<K> {
let (fingerprint, path) = keysource;

let (compare_fingerprint, compare_path) = match self.origin {
Some((fingerprint, ref path)) => (
fingerprint,
path.into_iter()
.chain(self.derivation_path.into_iter())
.collect(),
),
Some((fingerprint, ref path)) => {
(fingerprint, path.into_iter().chain(&self.derivation_path).collect())
}
None => (
self.xkey.xkey_fingerprint(secp),
self.derivation_path.into_iter().collect::<Vec<_>>(),
Expand Down
10 changes: 2 additions & 8 deletions src/descriptor/tr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,13 +90,7 @@ impl<Pk: MiniscriptKey> PartialEq for Tr<Pk> {
impl<Pk: MiniscriptKey> Eq for Tr<Pk> {}

impl<Pk: MiniscriptKey> PartialOrd for Tr<Pk> {
fn partial_cmp(&self, other: &Self) -> Option<cmp::Ordering> {
match self.internal_key.partial_cmp(&other.internal_key) {
Some(cmp::Ordering::Equal) => {}
ord => return ord,
}
self.tree.partial_cmp(&other.tree)
}
fn partial_cmp(&self, other: &Self) -> Option<cmp::Ordering> { Some(self.cmp(other)) }
}

impl<Pk: MiniscriptKey> Ord for Tr<Pk> {
Expand Down Expand Up @@ -737,7 +731,7 @@ where
wit.push(Placeholder::TapScript(leaf_script.0));
wit.push(Placeholder::TapControlBlock(control_block));

let wit_size = witness_size(&wit);
let wit_size = witness_size(wit);
if min_wit_len.is_some() && Some(wit_size) > min_wit_len {
continue;
} else {
Expand Down
2 changes: 1 addition & 1 deletion src/iter/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ impl<'a, Pk: MiniscriptKey> TreeLike for &'a policy::Concrete<Pk> {
}
}

impl<'a, Pk: MiniscriptKey> TreeLike for Arc<policy::Concrete<Pk>> {
impl<Pk: MiniscriptKey> TreeLike for Arc<policy::Concrete<Pk>> {
fn as_node(&self) -> Tree<Self> {
use policy::Concrete::*;
match self.as_ref() {
Expand Down
4 changes: 3 additions & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,8 @@
#![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

#[cfg(target_pointer_width = "16")]
compile_error!(
Expand Down Expand Up @@ -116,7 +118,6 @@ mod macros;
mod pub_macros;

use internals::hex::exts::DisplayHex;
pub use pub_macros::*;

pub mod descriptor;
pub mod expression;
Expand Down Expand Up @@ -812,6 +813,7 @@ mod tests {
}
}

#[allow(unused_imports)] // this is an internal prelude module; not all imports are used with every feature combination
mod prelude {
// Mutex implementation from LDK
// https://github.com/lightningdevkit/rust-lightning/blob/9bdce47f0e0516e37c89c09f1975dfc06b5870b1/lightning-invoice/src/sync.rs
Expand Down
44 changes: 19 additions & 25 deletions src/miniscript/types/extra_props.rs
Original file line number Diff line number Diff line change
Expand Up @@ -792,11 +792,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| acc + x)
} else {
opt_add(acc, y)
y.map(|y| acc + y)
}
});

Expand All @@ -805,11 +805,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| acc + x)
} else {
opt_max(acc, y)
y.map(|y| acc + y)
}
});

Expand All @@ -819,26 +819,25 @@ 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(|(x0, x1)| (acc.0 + x0, acc.1 + x1))
} else {
opt_tuple_add(acc, y)
y.map(|(y0, y1)| (acc.0 + y0, acc.1 + y1))
}
});

ops_count_sat_vec.sort_by(sat_minus_dissat);
let op_count_sat =
ops_count_sat_vec
.iter()
.enumerate()
.fold(Some(0), |acc, (i, &(x, y))| {
if i <= k {
opt_add(acc, x)
} else {
opt_add(acc, Some(y))
}
});
let op_count_sat = ops_count_sat_vec
.iter()
.enumerate()
.try_fold(0, |acc, (i, &(x, y))| {
if i <= k {
x.map(|x| acc + x)
} else {
Some(acc + y)
}
});

Ok(ExtData {
pk_cost: pk_cost + n - 1, //all pk cost + (n-1)*ADD
Expand Down Expand Up @@ -1049,11 +1048,6 @@ fn opt_max<T: Ord>(a: Option<T>, b: Option<T>) -> Option<T> {
/// Returns Some(x+y) is both x and y are Some. Otherwise, returns `None`.
fn opt_add(a: Option<usize>, b: Option<usize>) -> Option<usize> { 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::*;
Expand Down
39 changes: 17 additions & 22 deletions src/plan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -255,13 +255,13 @@ impl Plan {
// scriptSig len (1) + OP_0 (1) + OP_PUSHBYTES_32 (1) + <script hash> (32)
(_, DescriptorType::ShWsh) | (_, DescriptorType::ShWshSortedMulti) => 1 + 1 + 1 + 32,
// Native Segwit v0 (scriptSig len (1))
__ => 1,
_ => 1,
}
}

/// The size in bytes of the witness that satisfies this plan
pub fn witness_size(&self) -> usize {
if let Some(_) = self.descriptor.desc_type().segwit_version() {
if self.descriptor.desc_type().segwit_version().is_some() {
witness_size(self.template.as_ref())
} else {
0 // should be 1 if there's at least one segwit input in the tx, but that's out of
Expand Down Expand Up @@ -383,7 +383,7 @@ impl Plan {
.entry(pk)
.and_modify(|(leaf_hashes, _)| {
if let Some(lh) = leaf_hash {
if leaf_hashes.iter().find(|&&i| i == lh).is_none() {
if leaf_hashes.iter().all(|&i| i != lh) {
leaf_hashes.push(lh);
}
}
Expand All @@ -397,17 +397,14 @@ impl Plan {
}
} else {
for item in &self.template {
match item {
Placeholder::EcdsaSigPk(pk) => {
let public_key = pk.to_public_key().inner;
let master_fingerprint = pk.master_fingerprint();
for derivation_path in pk.full_derivation_paths() {
input
.bip32_derivation
.insert(public_key, (master_fingerprint, derivation_path));
}
if let Placeholder::EcdsaSigPk(pk) = item {
let public_key = pk.to_public_key().inner;
let master_fingerprint = pk.master_fingerprint();
for derivation_path in pk.full_derivation_paths() {
input
.bip32_derivation
.insert(public_key, (master_fingerprint, derivation_path));
}
_ => {}
}
}

Expand Down Expand Up @@ -669,7 +666,7 @@ impl IntoAssets for DescriptorPublicKey {
}

impl IntoAssets for Vec<DescriptorPublicKey> {
fn into_assets(self) -> Assets { Assets::from_iter(self.into_iter()) }
fn into_assets(self) -> Assets { Assets::from_iter(self) }
}

impl IntoAssets for sha256::Hash {
Expand Down Expand Up @@ -705,6 +702,7 @@ impl Assets {
pub fn new() -> Self { Self::default() }

/// Add some assets
#[allow(clippy::should_implement_trait)] // looks like the `ops::Add` trait
pub fn add<A: IntoAssets>(mut self, asset: A) -> Self {
self.append(asset.into_assets());
self
Expand All @@ -723,14 +721,11 @@ impl Assets {
}

fn append(&mut self, b: Self) {
self.keys.extend(b.keys.into_iter());
self.sha256_preimages.extend(b.sha256_preimages.into_iter());
self.hash256_preimages
.extend(b.hash256_preimages.into_iter());
self.ripemd160_preimages
.extend(b.ripemd160_preimages.into_iter());
self.hash160_preimages
.extend(b.hash160_preimages.into_iter());
self.keys.extend(b.keys);
self.sha256_preimages.extend(b.sha256_preimages);
self.hash256_preimages.extend(b.hash256_preimages);
self.ripemd160_preimages.extend(b.ripemd160_preimages);
self.hash160_preimages.extend(b.hash160_preimages);

self.relative_timelock = b.relative_timelock.or(self.relative_timelock);
self.absolute_timelock = b.absolute_timelock.or(self.absolute_timelock);
Expand Down
2 changes: 1 addition & 1 deletion src/psbt/finalizer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -428,7 +428,7 @@ pub(super) fn finalize_input<C: secp256k1::Verification>(
// Now mutate the psbt input. Note that we cannot error after this point.
// If the input is mutated, it means that the finalization succeeded.
{
let original = mem::replace(&mut psbt.inputs[index], Default::default());
let original = mem::take(&mut psbt.inputs[index]);
let input = &mut psbt.inputs[index];
input.non_witness_utxo = original.non_witness_utxo;
input.witness_utxo = original.witness_utxo;
Expand Down