From 009f68a06a067e638d073337fd86b712248ef6bd Mon Sep 17 00:00:00 2001 From: Tobin Harding Date: Thu, 6 May 2021 14:22:30 +1000 Subject: [PATCH 1/6] Use assert!(foo) instead of assert_eq!(foo, true) It is redundant to pass true/false to `assert_eq!` since `assert!` already asserts true/false. This may, however, be controversial if someone thinks that ``` assert_eq!(foo, false); ``` is more clear than ``` assert!(!foo); ``` Use `assert!` directly instead of `assert_eq!` with true/false argument. --- src/wallet/mod.rs | 12 ++++++------ src/wallet/utils.rs | 20 ++++++++++---------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/wallet/mod.rs b/src/wallet/mod.rs index 71fc5d7d..6e9646bb 100644 --- a/src/wallet/mod.rs +++ b/src/wallet/mod.rs @@ -3514,7 +3514,7 @@ pub(crate) mod test { let (mut psbt, _) = builder.finish().unwrap(); let finalized = wallet.sign(&mut psbt, Default::default()).unwrap(); - assert_eq!(finalized, true); + assert!(finalized); let extracted = psbt.extract_tx(); assert_eq!(extracted.input[0].witness.len(), 2); @@ -3531,7 +3531,7 @@ pub(crate) mod test { let (mut psbt, _) = builder.finish().unwrap(); let finalized = wallet.sign(&mut psbt, Default::default()).unwrap(); - assert_eq!(finalized, true); + assert!(finalized); let extracted = psbt.extract_tx(); assert_eq!(extracted.input[0].witness.len(), 2); @@ -3548,7 +3548,7 @@ pub(crate) mod test { let (mut psbt, _) = builder.finish().unwrap(); let finalized = wallet.sign(&mut psbt, Default::default()).unwrap(); - assert_eq!(finalized, true); + assert!(finalized); let extracted = psbt.extract_tx(); assert_eq!(extracted.input[0].witness.len(), 2); @@ -3565,7 +3565,7 @@ pub(crate) mod test { let (mut psbt, _) = builder.finish().unwrap(); let finalized = wallet.sign(&mut psbt, Default::default()).unwrap(); - assert_eq!(finalized, true); + assert!(finalized); let extracted = psbt.extract_tx(); assert_eq!(extracted.input[0].witness.len(), 2); @@ -3583,7 +3583,7 @@ pub(crate) mod test { let (mut psbt, _) = builder.finish().unwrap(); let finalized = wallet.sign(&mut psbt, Default::default()).unwrap(); - assert_eq!(finalized, true); + assert!(finalized); let extracted = psbt.extract_tx(); assert_eq!(extracted.input[0].witness.len(), 2); @@ -3603,7 +3603,7 @@ pub(crate) mod test { assert_eq!(psbt.inputs[0].bip32_derivation.len(), 0); let finalized = wallet.sign(&mut psbt, Default::default()).unwrap(); - assert_eq!(finalized, true); + assert!(finalized); let extracted = psbt.extract_tx(); assert_eq!(extracted.input[0].witness.len(), 2); diff --git a/src/wallet/utils.rs b/src/wallet/utils.rs index c19c05d1..311f691b 100644 --- a/src/wallet/utils.rs +++ b/src/wallet/utils.rs @@ -201,31 +201,31 @@ mod test { #[test] fn test_check_nsequence_rbf_msb_set() { let result = check_nsequence_rbf(0x80000000, 5000); - assert_eq!(result, false); + assert!(!result); } #[test] fn test_check_nsequence_rbf_lt_csv() { let result = check_nsequence_rbf(4000, 5000); - assert_eq!(result, false); + assert!(!result); } #[test] fn test_check_nsequence_rbf_different_unit() { let result = check_nsequence_rbf(SEQUENCE_LOCKTIME_TYPE_FLAG + 5000, 5000); - assert_eq!(result, false); + assert!(!result); } #[test] fn test_check_nsequence_rbf_mask() { let result = check_nsequence_rbf(0x3f + 10_000, 5000); - assert_eq!(result, true); + assert!(result); } #[test] fn test_check_nsequence_rbf_same_unit_blocks() { let result = check_nsequence_rbf(10_000, 5000); - assert_eq!(result, true); + assert!(result); } #[test] @@ -234,25 +234,25 @@ mod test { SEQUENCE_LOCKTIME_TYPE_FLAG + 10_000, SEQUENCE_LOCKTIME_TYPE_FLAG + 5000, ); - assert_eq!(result, true); + assert!(result); } #[test] fn test_check_nlocktime_lt_cltv() { let result = check_nlocktime(4000, 5000); - assert_eq!(result, false); + assert!(!result); } #[test] fn test_check_nlocktime_different_unit() { let result = check_nlocktime(BLOCKS_TIMELOCK_THRESHOLD + 5000, 5000); - assert_eq!(result, false); + assert!(!result); } #[test] fn test_check_nlocktime_same_unit_blocks() { let result = check_nlocktime(10_000, 5000); - assert_eq!(result, true); + assert!(result); } #[test] @@ -261,6 +261,6 @@ mod test { BLOCKS_TIMELOCK_THRESHOLD + 10_000, BLOCKS_TIMELOCK_THRESHOLD + 5000, ); - assert_eq!(result, true); + assert!(result); } } From 74cc80d127ce4af5c10efec375e318cf92668647 Mon Sep 17 00:00:00 2001 From: Tobin Harding Date: Thu, 6 May 2021 14:29:51 +1000 Subject: [PATCH 2/6] Remove unnecessary clone Clippy emits: warning: using `clone` on type `descriptor::policy::Condition` which implements the `Copy` trait Remove the clone and rely on `Copy`. --- src/wallet/mod.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/wallet/mod.rs b/src/wallet/mod.rs index 6e9646bb..b4b92899 100644 --- a/src/wallet/mod.rs +++ b/src/wallet/mod.rs @@ -424,9 +424,8 @@ where }) .transpose()?; - let requirements = external_requirements - .clone() - .merge(&internal_requirements.unwrap_or_default())?; + let requirements = + external_requirements.merge(&internal_requirements.unwrap_or_default())?; debug!("Policy requirements: {:?}", requirements); let version = match params.version { From de811bea309774b9c48257b3727634d98598cbe5 Mon Sep 17 00:00:00 2001 From: Tobin Harding Date: Thu, 6 May 2021 14:32:08 +1000 Subject: [PATCH 3/6] Use !any() instead of find()...is_none() Clippy emits: warning: called `is_none()` after searching an `Iterator` with `find` As suggested, use the construct: `!foo.iter().any(...)` --- src/wallet/mod.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/wallet/mod.rs b/src/wallet/mod.rs index b4b92899..11b58849 100644 --- a/src/wallet/mod.rs +++ b/src/wallet/mod.rs @@ -1166,11 +1166,11 @@ where // must_spend <- manually selected utxos // may_spend <- all other available utxos let mut may_spend = self.get_available_utxos()?; + may_spend.retain(|may_spend| { - manually_selected + !manually_selected .iter() - .find(|manually_selected| manually_selected.utxo.outpoint() == may_spend.0.outpoint) - .is_none() + .any(|manually_selected| manually_selected.utxo.outpoint() == may_spend.0.outpoint) }); let mut must_spend = manually_selected; From de4035171016bd625656efe7acb7db43433fe48c Mon Sep 17 00:00:00 2001 From: Tobin Harding Date: Thu, 6 May 2021 14:34:22 +1000 Subject: [PATCH 4/6] Use consistent field ordering Clippy emits: warning: struct constructor field order is inconsistent with struct definition field order As suggested, re-order the fields to be consistent with the struct definition. --- src/wallet/coin_selection.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wallet/coin_selection.rs b/src/wallet/coin_selection.rs index 5653662d..f2e2ca5f 100644 --- a/src/wallet/coin_selection.rs +++ b/src/wallet/coin_selection.rs @@ -255,8 +255,8 @@ impl OutputGroup { let effective_value = weighted_utxo.utxo.txout().value as i64 - fee.ceil() as i64; OutputGroup { weighted_utxo, - effective_value, fee, + effective_value, } } } From 7f06dc333058b40ec648b2ac6fe6e2cb5a73546d Mon Sep 17 00:00:00 2001 From: Tobin Harding Date: Tue, 11 May 2021 10:47:18 +1000 Subject: [PATCH 5/6] Clear clippy manual_map warning The lint `manual_map` is new so we cannot explicitly allow it and maintain backwards comparability. Instead, allow all lints for `get_utxo_for` with a comment explaining why. --- src/psbt/mod.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/psbt/mod.rs b/src/psbt/mod.rs index 2bda4852..1b4da364 100644 --- a/src/psbt/mod.rs +++ b/src/psbt/mod.rs @@ -17,6 +17,7 @@ pub trait PsbtUtils { } impl PsbtUtils for Psbt { + #[allow(clippy::all)] // We want to allow `manual_map` but it is too new. fn get_utxo_for(&self, input_index: usize) -> Option { let tx = &self.global.unsigned_tx; From e1066e955cf6c4fc0a20b4a895f064e70c696dae Mon Sep 17 00:00:00 2001 From: Tobin Harding Date: Tue, 11 May 2021 10:50:34 +1000 Subject: [PATCH 6/6] Remove unneeded unit expression Clippy emits: warning: unneeded unit expression As suggested, remove the unneeded unit expression. --- src/descriptor/dsl.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/descriptor/dsl.rs b/src/descriptor/dsl.rs index 16d8977d..e5bd1de6 100644 --- a/src/descriptor/dsl.rs +++ b/src/descriptor/dsl.rs @@ -535,9 +535,7 @@ macro_rules! fragment_internal { ( @t , $( $tail:tt )* ) => ({ $crate::fragment_internal!( @t $( $tail )* ) }); - ( @t ) => ({ - () - }); + ( @t ) => ({}); // Fallback to calling `fragment!()` ( $( $tokens:tt )* ) => ({