From 0b348d6d9ef7a7ece0b2d39571513ef028f1fd9a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=B3nal=20Murray?= Date: Thu, 30 Nov 2023 11:42:10 +0000 Subject: [PATCH 1/9] Fix clippy lints behind feature gates --- bridges/modules/relayers/src/benchmarking.rs | 4 +- .../collective-content/src/benchmarking.rs | 2 +- .../src/generic/benchmarking.rs | 26 ++++----- substrate/frame/alliance/src/benchmarking.rs | 57 ++++++------------- substrate/frame/bounties/src/benchmarking.rs | 2 +- .../frame/contracts/src/benchmarking/mod.rs | 5 +- .../frame/contracts/src/migration/v12.rs | 2 +- substrate/frame/democracy/src/benchmarking.rs | 2 +- .../frame/fast-unstake/src/benchmarking.rs | 2 +- substrate/frame/recovery/src/benchmarking.rs | 8 +-- substrate/frame/scheduler/src/migration.rs | 4 +- substrate/frame/staking/src/pallet/impls.rs | 2 +- substrate/frame/uniques/src/benchmarking.rs | 4 +- 13 files changed, 49 insertions(+), 71 deletions(-) diff --git a/bridges/modules/relayers/src/benchmarking.rs b/bridges/modules/relayers/src/benchmarking.rs index 2d74ab38f9db..00c3814a4c38 100644 --- a/bridges/modules/relayers/src/benchmarking.rs +++ b/bridges/modules/relayers/src/benchmarking.rs @@ -104,7 +104,7 @@ benchmarks! { // create slash destination account let lane = LaneId([0, 0, 0, 0]); let slash_destination = RewardsAccountParams::new(lane, *b"test", RewardsAccountOwner::ThisChain); - T::prepare_rewards_account(slash_destination.clone(), Zero::zero()); + T::prepare_rewards_account(slash_destination, Zero::zero()); }: { crate::Pallet::::slash_and_deregister(&relayer, slash_destination) } @@ -121,7 +121,7 @@ benchmarks! { let account_params = RewardsAccountParams::new(lane, *b"test", RewardsAccountOwner::ThisChain); }: { - crate::Pallet::::register_relayer_reward(account_params.clone(), &relayer, One::one()); + crate::Pallet::::register_relayer_reward(account_params, &relayer, One::one()); } verify { assert_eq!(RelayerRewards::::get(relayer, &account_params), Some(One::one())); diff --git a/cumulus/parachains/pallets/collective-content/src/benchmarking.rs b/cumulus/parachains/pallets/collective-content/src/benchmarking.rs index 1f145f725b13..943386a84276 100644 --- a/cumulus/parachains/pallets/collective-content/src/benchmarking.rs +++ b/cumulus/parachains/pallets/collective-content/src/benchmarking.rs @@ -56,7 +56,7 @@ mod benchmarks { .map_err(|_| BenchmarkError::Weightless)?; #[extrinsic_call] - _(origin as T::RuntimeOrigin, cid.clone(), Some(expire_at.clone())); + _(origin as T::RuntimeOrigin, cid.clone(), Some(expire_at)); assert_eq!(>::count(), 1); assert_last_event::( diff --git a/polkadot/xcm/pallet-xcm-benchmarks/src/generic/benchmarking.rs b/polkadot/xcm/pallet-xcm-benchmarks/src/generic/benchmarking.rs index f1c48ba9b830..50a7fe45e231 100644 --- a/polkadot/xcm/pallet-xcm-benchmarks/src/generic/benchmarking.rs +++ b/polkadot/xcm/pallet-xcm-benchmarks/src/generic/benchmarking.rs @@ -175,7 +175,7 @@ benchmarks! { descend_origin { let mut executor = new_executor::(Default::default()); let who = X2(OnlyChild, OnlyChild); - let instruction = Instruction::DescendOrigin(who.clone()); + let instruction = Instruction::DescendOrigin(who); let xcm = Xcm(vec![instruction]); } : { executor.bench_process(xcm)?; @@ -242,7 +242,7 @@ benchmarks! { &origin, assets.clone().into(), &XcmContext { - origin: Some(origin.clone()), + origin: Some(origin), message_id: [0; 32], topic: None, }, @@ -279,7 +279,7 @@ benchmarks! { let origin = T::subscribe_origin()?; let query_id = Default::default(); let max_response_weight = Default::default(); - let mut executor = new_executor::(origin.clone()); + let mut executor = new_executor::(origin); let instruction = Instruction::SubscribeVersion { query_id, max_response_weight }; let xcm = Xcm(vec![instruction]); } : { @@ -299,14 +299,14 @@ benchmarks! { query_id, max_response_weight, &XcmContext { - origin: Some(origin.clone()), + origin: Some(origin), message_id: [0; 32], topic: None, }, ).map_err(|_| "Could not start subscription")?; assert!(::SubscriptionService::is_subscribed(&origin)); - let mut executor = new_executor::(origin.clone()); + let mut executor = new_executor::(origin); let instruction = Instruction::UnsubscribeVersion; let xcm = Xcm(vec![instruction]); } : { @@ -538,7 +538,7 @@ benchmarks! { let mut executor = new_executor::(origin); - let instruction = Instruction::UniversalOrigin(alias.clone()); + let instruction = Instruction::UniversalOrigin(alias); let xcm = Xcm(vec![instruction]); }: { executor.bench_process(xcm)?; @@ -632,13 +632,13 @@ benchmarks! { let (unlocker, owner, asset) = T::unlockable_asset()?; - let mut executor = new_executor::(unlocker.clone()); + let mut executor = new_executor::(unlocker); // We first place the asset in lock first... ::AssetLocker::prepare_lock( unlocker, asset.clone(), - owner.clone(), + owner, ) .map_err(|_| BenchmarkError::Skip)? .enact() @@ -658,13 +658,13 @@ benchmarks! { let (unlocker, owner, asset) = T::unlockable_asset()?; - let mut executor = new_executor::(unlocker.clone()); + let mut executor = new_executor::(unlocker); // We first place the asset in lock first... ::AssetLocker::prepare_lock( unlocker, asset.clone(), - owner.clone(), + owner, ) .map_err(|_| BenchmarkError::Skip)? .enact() @@ -686,9 +686,9 @@ benchmarks! { // We first place the asset in lock first... ::AssetLocker::prepare_lock( - locker.clone(), + locker, asset.clone(), - owner.clone(), + owner, ) .map_err(|_| BenchmarkError::Skip)? .enact() @@ -739,7 +739,7 @@ benchmarks! { let mut executor = new_executor::(origin); - let instruction = Instruction::AliasOrigin(target.clone()); + let instruction = Instruction::AliasOrigin(target); let xcm = Xcm(vec![instruction]); }: { executor.bench_process(xcm)?; diff --git a/substrate/frame/alliance/src/benchmarking.rs b/substrate/frame/alliance/src/benchmarking.rs index 37cc3314037e..cb2a04f17c57 100644 --- a/substrate/frame/alliance/src/benchmarking.rs +++ b/substrate/frame/alliance/src/benchmarking.rs @@ -183,7 +183,7 @@ mod benchmarks { let voter = &members[j as usize]; Alliance::::vote( SystemOrigin::Signed(voter.clone()).into(), - last_hash.clone(), + last_hash, index, true, )?; @@ -191,12 +191,7 @@ mod benchmarks { let voter = members[m as usize - 3].clone(); // Voter votes aye without resolving the vote. - Alliance::::vote( - SystemOrigin::Signed(voter.clone()).into(), - last_hash.clone(), - index, - true, - )?; + Alliance::::vote(SystemOrigin::Signed(voter.clone()).into(), last_hash, index, true)?; // Voter switches vote to nay, but does not kill the vote, just updates + inserts let approve = false; @@ -206,7 +201,7 @@ mod benchmarks { frame_benchmarking::benchmarking::add_to_whitelist(voter_key.into()); #[extrinsic_call] - _(SystemOrigin::Signed(voter), last_hash.clone(), index, approve); + _(SystemOrigin::Signed(voter), last_hash, index, approve); //nothing to verify Ok(()) @@ -255,24 +250,19 @@ mod benchmarks { let voter = &members[j as usize]; Alliance::::vote( SystemOrigin::Signed(voter.clone()).into(), - last_hash.clone(), + last_hash, index, true, )?; } // Voter votes aye without resolving the vote. - Alliance::::vote( - SystemOrigin::Signed(voter.clone()).into(), - last_hash.clone(), - index, - true, - )?; + Alliance::::vote(SystemOrigin::Signed(voter.clone()).into(), last_hash, index, true)?; // Voter switches vote to nay, which kills the vote Alliance::::vote( SystemOrigin::Signed(voter.clone()).into(), - last_hash.clone(), + last_hash, index, false, )?; @@ -282,7 +272,7 @@ mod benchmarks { frame_benchmarking::benchmarking::add_to_whitelist(voter_key.into()); #[extrinsic_call] - close(SystemOrigin::Signed(voter), last_hash.clone(), index, Weight::MAX, bytes_in_storage); + close(SystemOrigin::Signed(voter), last_hash, index, Weight::MAX, bytes_in_storage); assert_eq!(T::ProposalProvider::proposal_of(last_hash), None); Ok(()) @@ -330,7 +320,7 @@ mod benchmarks { // approval vote Alliance::::vote( SystemOrigin::Signed(proposer.clone()).into(), - last_hash.clone(), + last_hash, index, false, )?; @@ -340,7 +330,7 @@ mod benchmarks { let voter = &members[j as usize]; Alliance::::vote( SystemOrigin::Signed(voter.clone()).into(), - last_hash.clone(), + last_hash, index, false, )?; @@ -349,22 +339,17 @@ mod benchmarks { // Member zero is the first aye Alliance::::vote( SystemOrigin::Signed(members[0].clone()).into(), - last_hash.clone(), + last_hash, index, true, )?; let voter = members[1].clone(); // Caller switches vote to aye, which passes the vote - Alliance::::vote( - SystemOrigin::Signed(voter.clone()).into(), - last_hash.clone(), - index, - true, - )?; + Alliance::::vote(SystemOrigin::Signed(voter.clone()).into(), last_hash, index, true)?; #[extrinsic_call] - close(SystemOrigin::Signed(voter), last_hash.clone(), index, Weight::MAX, bytes_in_storage); + close(SystemOrigin::Signed(voter), last_hash, index, Weight::MAX, bytes_in_storage); assert_eq!(T::ProposalProvider::proposal_of(last_hash), None); Ok(()) @@ -414,7 +399,7 @@ mod benchmarks { let voter = &members[j as usize]; Alliance::::vote( SystemOrigin::Signed(voter.clone()).into(), - last_hash.clone(), + last_hash, index, true, )?; @@ -422,7 +407,7 @@ mod benchmarks { Alliance::::vote( SystemOrigin::Signed(voter.clone()).into(), - last_hash.clone(), + last_hash, index, false, )?; @@ -430,7 +415,7 @@ mod benchmarks { System::::set_block_number(BlockNumberFor::::max_value()); #[extrinsic_call] - close(SystemOrigin::Signed(voter), last_hash.clone(), index, Weight::MAX, bytes_in_storage); + close(SystemOrigin::Signed(voter), last_hash, index, Weight::MAX, bytes_in_storage); // The last proposal is removed. assert_eq!(T::ProposalProvider::proposal_of(last_hash), None); @@ -477,7 +462,7 @@ mod benchmarks { // The prime member votes aye, so abstentions default to aye. Alliance::::vote( SystemOrigin::Signed(proposer.clone()).into(), - last_hash.clone(), + last_hash, p - 1, true, // Vote aye. )?; @@ -489,7 +474,7 @@ mod benchmarks { let voter = &members[j as usize]; Alliance::::vote( SystemOrigin::Signed(voter.clone()).into(), - last_hash.clone(), + last_hash, index, false, )?; @@ -499,13 +484,7 @@ mod benchmarks { System::::set_block_number(BlockNumberFor::::max_value()); #[extrinsic_call] - close( - SystemOrigin::Signed(proposer), - last_hash.clone(), - index, - Weight::MAX, - bytes_in_storage, - ); + close(SystemOrigin::Signed(proposer), last_hash, index, Weight::MAX, bytes_in_storage); assert_eq!(T::ProposalProvider::proposal_of(last_hash), None); Ok(()) diff --git a/substrate/frame/bounties/src/benchmarking.rs b/substrate/frame/bounties/src/benchmarking.rs index 6fff337cba45..3558847c8fed 100644 --- a/substrate/frame/bounties/src/benchmarking.rs +++ b/substrate/frame/bounties/src/benchmarking.rs @@ -222,7 +222,7 @@ benchmarks_instance_pallet! { ); } verify { - ensure!(missed_any == false, "Missed some"); + ensure!(!missed_any, "Missed some"); if b > 0 { ensure!(budget_remaining < BalanceOf::::max_value(), "Budget not used"); assert_last_event::(Event::BountyBecameActive { index: b - 1 }.into()) diff --git a/substrate/frame/contracts/src/benchmarking/mod.rs b/substrate/frame/contracts/src/benchmarking/mod.rs index c532b354ab64..3ab76d6112a2 100644 --- a/substrate/frame/contracts/src/benchmarking/mod.rs +++ b/substrate/frame/contracts/src/benchmarking/mod.rs @@ -1749,7 +1749,7 @@ benchmarks! { .collect::>>(); let deposits_bytes: Vec = deposits.iter().flat_map(|i| i.encode()).collect(); let deposits_len = deposits_bytes.len() as u32; - let deposit_len = value_len.clone(); + let deposit_len = value_len; let callee_offset = value_len + deposits_len; let code = WasmModule::::from(ModuleDefinition { memory: Some(ImportedMemory::max::()), @@ -2246,13 +2246,12 @@ benchmarks! { let message_len = message.len() as i32; let key_type = sp_core::crypto::KeyTypeId(*b"code"); let sig_params = (0..r) - .map(|i| { + .flat_map(|i| { let pub_key = sp_io::crypto::sr25519_generate(key_type, None); let sig = sp_io::crypto::sr25519_sign(key_type, &pub_key, &message).expect("Generates signature"); let data: [u8; 96] = [AsRef::<[u8]>::as_ref(&sig), AsRef::<[u8]>::as_ref(&pub_key)].concat().try_into().unwrap(); data }) - .flatten() .collect::>(); let sig_params_len = sig_params.len() as i32; diff --git a/substrate/frame/contracts/src/migration/v12.rs b/substrate/frame/contracts/src/migration/v12.rs index 4ddc57584b30..7dee31503101 100644 --- a/substrate/frame/contracts/src/migration/v12.rs +++ b/substrate/frame/contracts/src/migration/v12.rs @@ -317,7 +317,7 @@ where let (_, old_deposit, storage_module) = state; // CodeInfoOf::max_encoded_len == OwnerInfoOf::max_encoded_len + 1 // I.e. code info adds up 1 byte per record. - let info_bytes_added = items.clone(); + let info_bytes_added = items; // We removed 1 PrefabWasmModule, and added 1 byte of determinism flag, per contract code. let storage_removed = storage_module.saturating_sub(info_bytes_added); // module+code+info - bytes diff --git a/substrate/frame/democracy/src/benchmarking.rs b/substrate/frame/democracy/src/benchmarking.rs index b4aa17726b8d..aa66137ad880 100644 --- a/substrate/frame/democracy/src/benchmarking.rs +++ b/substrate/frame/democracy/src/benchmarking.rs @@ -65,7 +65,7 @@ fn add_referendum(n: u32) -> (ReferendumIndex, T::Hash, T::Hash) { 0u32.into(), ); let preimage_hash = note_preimage::(); - MetadataOf::::insert(crate::MetadataOwner::Referendum(index), preimage_hash.clone()); + MetadataOf::::insert(crate::MetadataOwner::Referendum(index), preimage_hash); (index, hash, preimage_hash) } diff --git a/substrate/frame/fast-unstake/src/benchmarking.rs b/substrate/frame/fast-unstake/src/benchmarking.rs index 851483e3697b..4828dcb9b42c 100644 --- a/substrate/frame/fast-unstake/src/benchmarking.rs +++ b/substrate/frame/fast-unstake/src/benchmarking.rs @@ -162,7 +162,7 @@ benchmarks! { fast_unstake_events::().last(), Some(Event::BatchChecked { .. }) )); - assert!(stashes.iter().all(|(s, _)| request.stashes.iter().find(|(ss, _)| ss == s).is_some())); + assert!(stashes.iter().all(|(s, _)| request.stashes.iter().any(|(ss, _)| ss == s))); } register_fast_unstake { diff --git a/substrate/frame/recovery/src/benchmarking.rs b/substrate/frame/recovery/src/benchmarking.rs index 2deb55bb69f2..72f77336212d 100644 --- a/substrate/frame/recovery/src/benchmarking.rs +++ b/substrate/frame/recovery/src/benchmarking.rs @@ -190,7 +190,7 @@ benchmarks! { let recovery_config = RecoveryConfig { delay_period: DEFAULT_DELAY.into(), - deposit: total_deposit.clone(), + deposit: total_deposit, friends: bounded_friends.clone(), threshold: n as u16, }; @@ -243,7 +243,7 @@ benchmarks! { let recovery_config = RecoveryConfig { delay_period: 0u32.into(), - deposit: total_deposit.clone(), + deposit: total_deposit, friends: bounded_friends.clone(), threshold: n as u16, }; @@ -294,7 +294,7 @@ benchmarks! { let recovery_config = RecoveryConfig { delay_period: DEFAULT_DELAY.into(), - deposit: total_deposit.clone(), + deposit: total_deposit, friends: bounded_friends.clone(), threshold: n as u16, }; @@ -342,7 +342,7 @@ benchmarks! { let recovery_config = RecoveryConfig { delay_period: DEFAULT_DELAY.into(), - deposit: total_deposit.clone(), + deposit: total_deposit, friends: bounded_friends.clone(), threshold: n as u16, }; diff --git a/substrate/frame/scheduler/src/migration.rs b/substrate/frame/scheduler/src/migration.rs index 9c8b0da03fc0..850a245817b9 100644 --- a/substrate/frame/scheduler/src/migration.rs +++ b/substrate/frame/scheduler/src/migration.rs @@ -105,7 +105,7 @@ pub mod v3 { // Check that no agenda overflows `MaxScheduledPerBlock`. let max_scheduled_per_block = T::MaxScheduledPerBlock::get() as usize; for (block_number, agenda) in Agenda::::iter() { - if agenda.iter().cloned().filter_map(|s| s).count() > max_scheduled_per_block { + if agenda.iter().cloned().flatten().count() > max_scheduled_per_block { log::error!( target: TARGET, "Would truncate agenda of block {:?} from {} items to {} items.", @@ -119,7 +119,7 @@ pub mod v3 { // Check that bounding the calls will not overflow `MAX_LENGTH`. let max_length = T::Preimages::MAX_LENGTH as usize; for (block_number, agenda) in Agenda::::iter() { - for schedule in agenda.iter().cloned().filter_map(|s| s) { + for schedule in agenda.iter().cloned().flatten() { match schedule.call { frame_support::traits::schedule::MaybeHashed::Value(call) => { let l = call.using_encoded(|c| c.len()); diff --git a/substrate/frame/staking/src/pallet/impls.rs b/substrate/frame/staking/src/pallet/impls.rs index d294686751cd..e312de409f8c 100644 --- a/substrate/frame/staking/src/pallet/impls.rs +++ b/substrate/frame/staking/src/pallet/impls.rs @@ -1801,7 +1801,7 @@ impl StakingInterface for Pallet { ) { let others = exposures .iter() - .map(|(who, value)| IndividualExposure { who: who.clone(), value: value.clone() }) + .map(|(who, value)| IndividualExposure { who: who.clone(), value: value }) .collect::>(); let exposure = Exposure { total: Default::default(), own: Default::default(), others }; EraInfo::::set_exposure(*current_era, stash, exposure); diff --git a/substrate/frame/uniques/src/benchmarking.rs b/substrate/frame/uniques/src/benchmarking.rs index 821ca1794b86..80d02f136218 100644 --- a/substrate/frame/uniques/src/benchmarking.rs +++ b/substrate/frame/uniques/src/benchmarking.rs @@ -431,9 +431,9 @@ benchmarks_instance_pallet! { let buyer_lookup = T::Lookup::unlookup(buyer.clone()); let price = ItemPrice::::from(0u32); let origin = SystemOrigin::Signed(seller.clone()).into(); - Uniques::::set_price(origin, collection.clone(), item, Some(price.clone()), Some(buyer_lookup))?; + Uniques::::set_price(origin, collection.clone(), item, Some(price), Some(buyer_lookup))?; T::Currency::make_free_balance_be(&buyer, DepositBalanceOf::::max_value()); - }: _(SystemOrigin::Signed(buyer.clone()), collection.clone(), item, price.clone()) + }: _(SystemOrigin::Signed(buyer.clone()), collection.clone(), item, price) verify { assert_last_event::(Event::ItemBought { collection: collection.clone(), From d91fec7fa2ba5ea771b1e2b0a8fbe94923f771bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=B3nal=20Murray?= Date: Thu, 30 Nov 2023 15:58:06 +0000 Subject: [PATCH 2/9] Revert "Fix clippy lints behind feature gates" This reverts commit 0b348d6d9ef7a7ece0b2d39571513ef028f1fd9a. --- bridges/modules/relayers/src/benchmarking.rs | 4 +- .../collective-content/src/benchmarking.rs | 2 +- .../src/generic/benchmarking.rs | 26 ++++----- substrate/frame/alliance/src/benchmarking.rs | 57 +++++++++++++------ substrate/frame/bounties/src/benchmarking.rs | 2 +- .../frame/contracts/src/benchmarking/mod.rs | 5 +- .../frame/contracts/src/migration/v12.rs | 2 +- substrate/frame/democracy/src/benchmarking.rs | 2 +- .../frame/fast-unstake/src/benchmarking.rs | 2 +- substrate/frame/recovery/src/benchmarking.rs | 8 +-- substrate/frame/scheduler/src/migration.rs | 4 +- substrate/frame/staking/src/pallet/impls.rs | 2 +- substrate/frame/uniques/src/benchmarking.rs | 4 +- 13 files changed, 71 insertions(+), 49 deletions(-) diff --git a/bridges/modules/relayers/src/benchmarking.rs b/bridges/modules/relayers/src/benchmarking.rs index 00c3814a4c38..2d74ab38f9db 100644 --- a/bridges/modules/relayers/src/benchmarking.rs +++ b/bridges/modules/relayers/src/benchmarking.rs @@ -104,7 +104,7 @@ benchmarks! { // create slash destination account let lane = LaneId([0, 0, 0, 0]); let slash_destination = RewardsAccountParams::new(lane, *b"test", RewardsAccountOwner::ThisChain); - T::prepare_rewards_account(slash_destination, Zero::zero()); + T::prepare_rewards_account(slash_destination.clone(), Zero::zero()); }: { crate::Pallet::::slash_and_deregister(&relayer, slash_destination) } @@ -121,7 +121,7 @@ benchmarks! { let account_params = RewardsAccountParams::new(lane, *b"test", RewardsAccountOwner::ThisChain); }: { - crate::Pallet::::register_relayer_reward(account_params, &relayer, One::one()); + crate::Pallet::::register_relayer_reward(account_params.clone(), &relayer, One::one()); } verify { assert_eq!(RelayerRewards::::get(relayer, &account_params), Some(One::one())); diff --git a/cumulus/parachains/pallets/collective-content/src/benchmarking.rs b/cumulus/parachains/pallets/collective-content/src/benchmarking.rs index 943386a84276..1f145f725b13 100644 --- a/cumulus/parachains/pallets/collective-content/src/benchmarking.rs +++ b/cumulus/parachains/pallets/collective-content/src/benchmarking.rs @@ -56,7 +56,7 @@ mod benchmarks { .map_err(|_| BenchmarkError::Weightless)?; #[extrinsic_call] - _(origin as T::RuntimeOrigin, cid.clone(), Some(expire_at)); + _(origin as T::RuntimeOrigin, cid.clone(), Some(expire_at.clone())); assert_eq!(>::count(), 1); assert_last_event::( diff --git a/polkadot/xcm/pallet-xcm-benchmarks/src/generic/benchmarking.rs b/polkadot/xcm/pallet-xcm-benchmarks/src/generic/benchmarking.rs index 50a7fe45e231..f1c48ba9b830 100644 --- a/polkadot/xcm/pallet-xcm-benchmarks/src/generic/benchmarking.rs +++ b/polkadot/xcm/pallet-xcm-benchmarks/src/generic/benchmarking.rs @@ -175,7 +175,7 @@ benchmarks! { descend_origin { let mut executor = new_executor::(Default::default()); let who = X2(OnlyChild, OnlyChild); - let instruction = Instruction::DescendOrigin(who); + let instruction = Instruction::DescendOrigin(who.clone()); let xcm = Xcm(vec![instruction]); } : { executor.bench_process(xcm)?; @@ -242,7 +242,7 @@ benchmarks! { &origin, assets.clone().into(), &XcmContext { - origin: Some(origin), + origin: Some(origin.clone()), message_id: [0; 32], topic: None, }, @@ -279,7 +279,7 @@ benchmarks! { let origin = T::subscribe_origin()?; let query_id = Default::default(); let max_response_weight = Default::default(); - let mut executor = new_executor::(origin); + let mut executor = new_executor::(origin.clone()); let instruction = Instruction::SubscribeVersion { query_id, max_response_weight }; let xcm = Xcm(vec![instruction]); } : { @@ -299,14 +299,14 @@ benchmarks! { query_id, max_response_weight, &XcmContext { - origin: Some(origin), + origin: Some(origin.clone()), message_id: [0; 32], topic: None, }, ).map_err(|_| "Could not start subscription")?; assert!(::SubscriptionService::is_subscribed(&origin)); - let mut executor = new_executor::(origin); + let mut executor = new_executor::(origin.clone()); let instruction = Instruction::UnsubscribeVersion; let xcm = Xcm(vec![instruction]); } : { @@ -538,7 +538,7 @@ benchmarks! { let mut executor = new_executor::(origin); - let instruction = Instruction::UniversalOrigin(alias); + let instruction = Instruction::UniversalOrigin(alias.clone()); let xcm = Xcm(vec![instruction]); }: { executor.bench_process(xcm)?; @@ -632,13 +632,13 @@ benchmarks! { let (unlocker, owner, asset) = T::unlockable_asset()?; - let mut executor = new_executor::(unlocker); + let mut executor = new_executor::(unlocker.clone()); // We first place the asset in lock first... ::AssetLocker::prepare_lock( unlocker, asset.clone(), - owner, + owner.clone(), ) .map_err(|_| BenchmarkError::Skip)? .enact() @@ -658,13 +658,13 @@ benchmarks! { let (unlocker, owner, asset) = T::unlockable_asset()?; - let mut executor = new_executor::(unlocker); + let mut executor = new_executor::(unlocker.clone()); // We first place the asset in lock first... ::AssetLocker::prepare_lock( unlocker, asset.clone(), - owner, + owner.clone(), ) .map_err(|_| BenchmarkError::Skip)? .enact() @@ -686,9 +686,9 @@ benchmarks! { // We first place the asset in lock first... ::AssetLocker::prepare_lock( - locker, + locker.clone(), asset.clone(), - owner, + owner.clone(), ) .map_err(|_| BenchmarkError::Skip)? .enact() @@ -739,7 +739,7 @@ benchmarks! { let mut executor = new_executor::(origin); - let instruction = Instruction::AliasOrigin(target); + let instruction = Instruction::AliasOrigin(target.clone()); let xcm = Xcm(vec![instruction]); }: { executor.bench_process(xcm)?; diff --git a/substrate/frame/alliance/src/benchmarking.rs b/substrate/frame/alliance/src/benchmarking.rs index cb2a04f17c57..37cc3314037e 100644 --- a/substrate/frame/alliance/src/benchmarking.rs +++ b/substrate/frame/alliance/src/benchmarking.rs @@ -183,7 +183,7 @@ mod benchmarks { let voter = &members[j as usize]; Alliance::::vote( SystemOrigin::Signed(voter.clone()).into(), - last_hash, + last_hash.clone(), index, true, )?; @@ -191,7 +191,12 @@ mod benchmarks { let voter = members[m as usize - 3].clone(); // Voter votes aye without resolving the vote. - Alliance::::vote(SystemOrigin::Signed(voter.clone()).into(), last_hash, index, true)?; + Alliance::::vote( + SystemOrigin::Signed(voter.clone()).into(), + last_hash.clone(), + index, + true, + )?; // Voter switches vote to nay, but does not kill the vote, just updates + inserts let approve = false; @@ -201,7 +206,7 @@ mod benchmarks { frame_benchmarking::benchmarking::add_to_whitelist(voter_key.into()); #[extrinsic_call] - _(SystemOrigin::Signed(voter), last_hash, index, approve); + _(SystemOrigin::Signed(voter), last_hash.clone(), index, approve); //nothing to verify Ok(()) @@ -250,19 +255,24 @@ mod benchmarks { let voter = &members[j as usize]; Alliance::::vote( SystemOrigin::Signed(voter.clone()).into(), - last_hash, + last_hash.clone(), index, true, )?; } // Voter votes aye without resolving the vote. - Alliance::::vote(SystemOrigin::Signed(voter.clone()).into(), last_hash, index, true)?; + Alliance::::vote( + SystemOrigin::Signed(voter.clone()).into(), + last_hash.clone(), + index, + true, + )?; // Voter switches vote to nay, which kills the vote Alliance::::vote( SystemOrigin::Signed(voter.clone()).into(), - last_hash, + last_hash.clone(), index, false, )?; @@ -272,7 +282,7 @@ mod benchmarks { frame_benchmarking::benchmarking::add_to_whitelist(voter_key.into()); #[extrinsic_call] - close(SystemOrigin::Signed(voter), last_hash, index, Weight::MAX, bytes_in_storage); + close(SystemOrigin::Signed(voter), last_hash.clone(), index, Weight::MAX, bytes_in_storage); assert_eq!(T::ProposalProvider::proposal_of(last_hash), None); Ok(()) @@ -320,7 +330,7 @@ mod benchmarks { // approval vote Alliance::::vote( SystemOrigin::Signed(proposer.clone()).into(), - last_hash, + last_hash.clone(), index, false, )?; @@ -330,7 +340,7 @@ mod benchmarks { let voter = &members[j as usize]; Alliance::::vote( SystemOrigin::Signed(voter.clone()).into(), - last_hash, + last_hash.clone(), index, false, )?; @@ -339,17 +349,22 @@ mod benchmarks { // Member zero is the first aye Alliance::::vote( SystemOrigin::Signed(members[0].clone()).into(), - last_hash, + last_hash.clone(), index, true, )?; let voter = members[1].clone(); // Caller switches vote to aye, which passes the vote - Alliance::::vote(SystemOrigin::Signed(voter.clone()).into(), last_hash, index, true)?; + Alliance::::vote( + SystemOrigin::Signed(voter.clone()).into(), + last_hash.clone(), + index, + true, + )?; #[extrinsic_call] - close(SystemOrigin::Signed(voter), last_hash, index, Weight::MAX, bytes_in_storage); + close(SystemOrigin::Signed(voter), last_hash.clone(), index, Weight::MAX, bytes_in_storage); assert_eq!(T::ProposalProvider::proposal_of(last_hash), None); Ok(()) @@ -399,7 +414,7 @@ mod benchmarks { let voter = &members[j as usize]; Alliance::::vote( SystemOrigin::Signed(voter.clone()).into(), - last_hash, + last_hash.clone(), index, true, )?; @@ -407,7 +422,7 @@ mod benchmarks { Alliance::::vote( SystemOrigin::Signed(voter.clone()).into(), - last_hash, + last_hash.clone(), index, false, )?; @@ -415,7 +430,7 @@ mod benchmarks { System::::set_block_number(BlockNumberFor::::max_value()); #[extrinsic_call] - close(SystemOrigin::Signed(voter), last_hash, index, Weight::MAX, bytes_in_storage); + close(SystemOrigin::Signed(voter), last_hash.clone(), index, Weight::MAX, bytes_in_storage); // The last proposal is removed. assert_eq!(T::ProposalProvider::proposal_of(last_hash), None); @@ -462,7 +477,7 @@ mod benchmarks { // The prime member votes aye, so abstentions default to aye. Alliance::::vote( SystemOrigin::Signed(proposer.clone()).into(), - last_hash, + last_hash.clone(), p - 1, true, // Vote aye. )?; @@ -474,7 +489,7 @@ mod benchmarks { let voter = &members[j as usize]; Alliance::::vote( SystemOrigin::Signed(voter.clone()).into(), - last_hash, + last_hash.clone(), index, false, )?; @@ -484,7 +499,13 @@ mod benchmarks { System::::set_block_number(BlockNumberFor::::max_value()); #[extrinsic_call] - close(SystemOrigin::Signed(proposer), last_hash, index, Weight::MAX, bytes_in_storage); + close( + SystemOrigin::Signed(proposer), + last_hash.clone(), + index, + Weight::MAX, + bytes_in_storage, + ); assert_eq!(T::ProposalProvider::proposal_of(last_hash), None); Ok(()) diff --git a/substrate/frame/bounties/src/benchmarking.rs b/substrate/frame/bounties/src/benchmarking.rs index 3558847c8fed..6fff337cba45 100644 --- a/substrate/frame/bounties/src/benchmarking.rs +++ b/substrate/frame/bounties/src/benchmarking.rs @@ -222,7 +222,7 @@ benchmarks_instance_pallet! { ); } verify { - ensure!(!missed_any, "Missed some"); + ensure!(missed_any == false, "Missed some"); if b > 0 { ensure!(budget_remaining < BalanceOf::::max_value(), "Budget not used"); assert_last_event::(Event::BountyBecameActive { index: b - 1 }.into()) diff --git a/substrate/frame/contracts/src/benchmarking/mod.rs b/substrate/frame/contracts/src/benchmarking/mod.rs index 3ab76d6112a2..c532b354ab64 100644 --- a/substrate/frame/contracts/src/benchmarking/mod.rs +++ b/substrate/frame/contracts/src/benchmarking/mod.rs @@ -1749,7 +1749,7 @@ benchmarks! { .collect::>>(); let deposits_bytes: Vec = deposits.iter().flat_map(|i| i.encode()).collect(); let deposits_len = deposits_bytes.len() as u32; - let deposit_len = value_len; + let deposit_len = value_len.clone(); let callee_offset = value_len + deposits_len; let code = WasmModule::::from(ModuleDefinition { memory: Some(ImportedMemory::max::()), @@ -2246,12 +2246,13 @@ benchmarks! { let message_len = message.len() as i32; let key_type = sp_core::crypto::KeyTypeId(*b"code"); let sig_params = (0..r) - .flat_map(|i| { + .map(|i| { let pub_key = sp_io::crypto::sr25519_generate(key_type, None); let sig = sp_io::crypto::sr25519_sign(key_type, &pub_key, &message).expect("Generates signature"); let data: [u8; 96] = [AsRef::<[u8]>::as_ref(&sig), AsRef::<[u8]>::as_ref(&pub_key)].concat().try_into().unwrap(); data }) + .flatten() .collect::>(); let sig_params_len = sig_params.len() as i32; diff --git a/substrate/frame/contracts/src/migration/v12.rs b/substrate/frame/contracts/src/migration/v12.rs index 7dee31503101..4ddc57584b30 100644 --- a/substrate/frame/contracts/src/migration/v12.rs +++ b/substrate/frame/contracts/src/migration/v12.rs @@ -317,7 +317,7 @@ where let (_, old_deposit, storage_module) = state; // CodeInfoOf::max_encoded_len == OwnerInfoOf::max_encoded_len + 1 // I.e. code info adds up 1 byte per record. - let info_bytes_added = items; + let info_bytes_added = items.clone(); // We removed 1 PrefabWasmModule, and added 1 byte of determinism flag, per contract code. let storage_removed = storage_module.saturating_sub(info_bytes_added); // module+code+info - bytes diff --git a/substrate/frame/democracy/src/benchmarking.rs b/substrate/frame/democracy/src/benchmarking.rs index aa66137ad880..b4aa17726b8d 100644 --- a/substrate/frame/democracy/src/benchmarking.rs +++ b/substrate/frame/democracy/src/benchmarking.rs @@ -65,7 +65,7 @@ fn add_referendum(n: u32) -> (ReferendumIndex, T::Hash, T::Hash) { 0u32.into(), ); let preimage_hash = note_preimage::(); - MetadataOf::::insert(crate::MetadataOwner::Referendum(index), preimage_hash); + MetadataOf::::insert(crate::MetadataOwner::Referendum(index), preimage_hash.clone()); (index, hash, preimage_hash) } diff --git a/substrate/frame/fast-unstake/src/benchmarking.rs b/substrate/frame/fast-unstake/src/benchmarking.rs index 4828dcb9b42c..851483e3697b 100644 --- a/substrate/frame/fast-unstake/src/benchmarking.rs +++ b/substrate/frame/fast-unstake/src/benchmarking.rs @@ -162,7 +162,7 @@ benchmarks! { fast_unstake_events::().last(), Some(Event::BatchChecked { .. }) )); - assert!(stashes.iter().all(|(s, _)| request.stashes.iter().any(|(ss, _)| ss == s))); + assert!(stashes.iter().all(|(s, _)| request.stashes.iter().find(|(ss, _)| ss == s).is_some())); } register_fast_unstake { diff --git a/substrate/frame/recovery/src/benchmarking.rs b/substrate/frame/recovery/src/benchmarking.rs index 72f77336212d..2deb55bb69f2 100644 --- a/substrate/frame/recovery/src/benchmarking.rs +++ b/substrate/frame/recovery/src/benchmarking.rs @@ -190,7 +190,7 @@ benchmarks! { let recovery_config = RecoveryConfig { delay_period: DEFAULT_DELAY.into(), - deposit: total_deposit, + deposit: total_deposit.clone(), friends: bounded_friends.clone(), threshold: n as u16, }; @@ -243,7 +243,7 @@ benchmarks! { let recovery_config = RecoveryConfig { delay_period: 0u32.into(), - deposit: total_deposit, + deposit: total_deposit.clone(), friends: bounded_friends.clone(), threshold: n as u16, }; @@ -294,7 +294,7 @@ benchmarks! { let recovery_config = RecoveryConfig { delay_period: DEFAULT_DELAY.into(), - deposit: total_deposit, + deposit: total_deposit.clone(), friends: bounded_friends.clone(), threshold: n as u16, }; @@ -342,7 +342,7 @@ benchmarks! { let recovery_config = RecoveryConfig { delay_period: DEFAULT_DELAY.into(), - deposit: total_deposit, + deposit: total_deposit.clone(), friends: bounded_friends.clone(), threshold: n as u16, }; diff --git a/substrate/frame/scheduler/src/migration.rs b/substrate/frame/scheduler/src/migration.rs index 850a245817b9..9c8b0da03fc0 100644 --- a/substrate/frame/scheduler/src/migration.rs +++ b/substrate/frame/scheduler/src/migration.rs @@ -105,7 +105,7 @@ pub mod v3 { // Check that no agenda overflows `MaxScheduledPerBlock`. let max_scheduled_per_block = T::MaxScheduledPerBlock::get() as usize; for (block_number, agenda) in Agenda::::iter() { - if agenda.iter().cloned().flatten().count() > max_scheduled_per_block { + if agenda.iter().cloned().filter_map(|s| s).count() > max_scheduled_per_block { log::error!( target: TARGET, "Would truncate agenda of block {:?} from {} items to {} items.", @@ -119,7 +119,7 @@ pub mod v3 { // Check that bounding the calls will not overflow `MAX_LENGTH`. let max_length = T::Preimages::MAX_LENGTH as usize; for (block_number, agenda) in Agenda::::iter() { - for schedule in agenda.iter().cloned().flatten() { + for schedule in agenda.iter().cloned().filter_map(|s| s) { match schedule.call { frame_support::traits::schedule::MaybeHashed::Value(call) => { let l = call.using_encoded(|c| c.len()); diff --git a/substrate/frame/staking/src/pallet/impls.rs b/substrate/frame/staking/src/pallet/impls.rs index e312de409f8c..d294686751cd 100644 --- a/substrate/frame/staking/src/pallet/impls.rs +++ b/substrate/frame/staking/src/pallet/impls.rs @@ -1801,7 +1801,7 @@ impl StakingInterface for Pallet { ) { let others = exposures .iter() - .map(|(who, value)| IndividualExposure { who: who.clone(), value: value }) + .map(|(who, value)| IndividualExposure { who: who.clone(), value: value.clone() }) .collect::>(); let exposure = Exposure { total: Default::default(), own: Default::default(), others }; EraInfo::::set_exposure(*current_era, stash, exposure); diff --git a/substrate/frame/uniques/src/benchmarking.rs b/substrate/frame/uniques/src/benchmarking.rs index 80d02f136218..821ca1794b86 100644 --- a/substrate/frame/uniques/src/benchmarking.rs +++ b/substrate/frame/uniques/src/benchmarking.rs @@ -431,9 +431,9 @@ benchmarks_instance_pallet! { let buyer_lookup = T::Lookup::unlookup(buyer.clone()); let price = ItemPrice::::from(0u32); let origin = SystemOrigin::Signed(seller.clone()).into(); - Uniques::::set_price(origin, collection.clone(), item, Some(price), Some(buyer_lookup))?; + Uniques::::set_price(origin, collection.clone(), item, Some(price.clone()), Some(buyer_lookup))?; T::Currency::make_free_balance_be(&buyer, DepositBalanceOf::::max_value()); - }: _(SystemOrigin::Signed(buyer.clone()), collection.clone(), item, price) + }: _(SystemOrigin::Signed(buyer.clone()), collection.clone(), item, price.clone()) verify { assert_last_event::(Event::ItemBought { collection: collection.clone(), From 9b8105e2c4beae463cc3304ba4651ee77a0cf5fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=B3nal=20Murray?= Date: Thu, 30 Nov 2023 15:57:29 +0000 Subject: [PATCH 3/9] Fix clippy lints except for clone on copy --- cumulus/pallets/xcmp-queue/src/benchmarking.rs | 2 +- substrate/frame/bounties/src/benchmarking.rs | 2 +- substrate/frame/contracts/src/benchmarking/mod.rs | 3 +-- substrate/frame/fast-unstake/src/benchmarking.rs | 2 +- substrate/frame/scheduler/src/migration.rs | 4 ++-- substrate/primitives/core/src/paired_crypto.rs | 2 +- 6 files changed, 7 insertions(+), 8 deletions(-) diff --git a/cumulus/pallets/xcmp-queue/src/benchmarking.rs b/cumulus/pallets/xcmp-queue/src/benchmarking.rs index 81dfbc2bb71c..c363cc2454ad 100644 --- a/cumulus/pallets/xcmp-queue/src/benchmarking.rs +++ b/cumulus/pallets/xcmp-queue/src/benchmarking.rs @@ -85,7 +85,7 @@ mod benchmarks { } assert!( - OutboundXcmpStatus::::get().iter().find(|p| p.recipient == para).is_none(), + !OutboundXcmpStatus::::get().iter().any(|p| p.recipient == para), "No messages in the channel; therefore removed." ); } diff --git a/substrate/frame/bounties/src/benchmarking.rs b/substrate/frame/bounties/src/benchmarking.rs index 6fff337cba45..3558847c8fed 100644 --- a/substrate/frame/bounties/src/benchmarking.rs +++ b/substrate/frame/bounties/src/benchmarking.rs @@ -222,7 +222,7 @@ benchmarks_instance_pallet! { ); } verify { - ensure!(missed_any == false, "Missed some"); + ensure!(!missed_any, "Missed some"); if b > 0 { ensure!(budget_remaining < BalanceOf::::max_value(), "Budget not used"); assert_last_event::(Event::BountyBecameActive { index: b - 1 }.into()) diff --git a/substrate/frame/contracts/src/benchmarking/mod.rs b/substrate/frame/contracts/src/benchmarking/mod.rs index c532b354ab64..208d93185995 100644 --- a/substrate/frame/contracts/src/benchmarking/mod.rs +++ b/substrate/frame/contracts/src/benchmarking/mod.rs @@ -2246,13 +2246,12 @@ benchmarks! { let message_len = message.len() as i32; let key_type = sp_core::crypto::KeyTypeId(*b"code"); let sig_params = (0..r) - .map(|i| { + .flat_map(|i| { let pub_key = sp_io::crypto::sr25519_generate(key_type, None); let sig = sp_io::crypto::sr25519_sign(key_type, &pub_key, &message).expect("Generates signature"); let data: [u8; 96] = [AsRef::<[u8]>::as_ref(&sig), AsRef::<[u8]>::as_ref(&pub_key)].concat().try_into().unwrap(); data }) - .flatten() .collect::>(); let sig_params_len = sig_params.len() as i32; diff --git a/substrate/frame/fast-unstake/src/benchmarking.rs b/substrate/frame/fast-unstake/src/benchmarking.rs index 851483e3697b..4828dcb9b42c 100644 --- a/substrate/frame/fast-unstake/src/benchmarking.rs +++ b/substrate/frame/fast-unstake/src/benchmarking.rs @@ -162,7 +162,7 @@ benchmarks! { fast_unstake_events::().last(), Some(Event::BatchChecked { .. }) )); - assert!(stashes.iter().all(|(s, _)| request.stashes.iter().find(|(ss, _)| ss == s).is_some())); + assert!(stashes.iter().all(|(s, _)| request.stashes.iter().any(|(ss, _)| ss == s))); } register_fast_unstake { diff --git a/substrate/frame/scheduler/src/migration.rs b/substrate/frame/scheduler/src/migration.rs index 9c8b0da03fc0..850a245817b9 100644 --- a/substrate/frame/scheduler/src/migration.rs +++ b/substrate/frame/scheduler/src/migration.rs @@ -105,7 +105,7 @@ pub mod v3 { // Check that no agenda overflows `MaxScheduledPerBlock`. let max_scheduled_per_block = T::MaxScheduledPerBlock::get() as usize; for (block_number, agenda) in Agenda::::iter() { - if agenda.iter().cloned().filter_map(|s| s).count() > max_scheduled_per_block { + if agenda.iter().cloned().flatten().count() > max_scheduled_per_block { log::error!( target: TARGET, "Would truncate agenda of block {:?} from {} items to {} items.", @@ -119,7 +119,7 @@ pub mod v3 { // Check that bounding the calls will not overflow `MAX_LENGTH`. let max_length = T::Preimages::MAX_LENGTH as usize; for (block_number, agenda) in Agenda::::iter() { - for schedule in agenda.iter().cloned().filter_map(|s| s) { + for schedule in agenda.iter().cloned().flatten() { match schedule.call { frame_support::traits::schedule::MaybeHashed::Value(call) => { let l = call.using_encoded(|c| c.len()); diff --git a/substrate/primitives/core/src/paired_crypto.rs b/substrate/primitives/core/src/paired_crypto.rs index edf6156f268e..960b8469249e 100644 --- a/substrate/primitives/core/src/paired_crypto.rs +++ b/substrate/primitives/core/src/paired_crypto.rs @@ -128,7 +128,7 @@ pub mod ecdsa_bls377 { let Ok(right_sig) = sig.0[ecdsa::SIGNATURE_SERIALIZED_SIZE..].try_into() else { return false }; - bls377::Pair::verify(&right_sig, message.as_ref(), &right_pub) + bls377::Pair::verify(&right_sig, message, &right_pub) } } } From 922c67ccdb8791f369880f32705d0ad155db770c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=B3nal=20Murray?= Date: Wed, 13 Dec 2023 15:22:50 +0000 Subject: [PATCH 4/9] Downgrade clippy lints to warn locally, error in CI --- Cargo.toml | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 0091c2d7c8be..c88d36171784 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -481,9 +481,9 @@ suspicious_double_ref_op = { level = "allow", priority = 2 } [workspace.lints.clippy] all = { level = "allow", priority = 0 } -correctness = { level = "deny", priority = 1 } +correctness = { level = "warn", priority = 1 } +complexity = { level = "warn", priority = 1 } if-same-then-else = { level = "allow", priority = 2 } -complexity = { level = "deny", priority = 1 } zero-prefixed-literal = { level = "allow", priority = 2 } # 00_1000_000 type_complexity = { level = "allow", priority = 2 } # raison d'etre nonminimal-bool = { level = "allow", priority = 2 } # maybe @@ -492,14 +492,15 @@ too-many-arguments = { level = "allow", priority = 2 } # (Turning unnecessary_cast = { level = "allow", priority = 2 } # Types may change identity-op = { level = "allow", priority = 2 } # One case where we do 0 + useless_conversion = { level = "allow", priority = 2 } # Types may change -unit_arg = { level = "allow", priority = 2 } # styalistic. -option-map-unit-fn = { level = "allow", priority = 2 } # styalistic -bind_instead_of_map = { level = "allow", priority = 2 } # styalistic +unit_arg = { level = "allow", priority = 2 } # stylistic +option-map-unit-fn = { level = "allow", priority = 2 } # stylistic +bind_instead_of_map = { level = "allow", priority = 2 } # stylistic erasing_op = { level = "allow", priority = 2 } # E.g. 0 * DOLLARS eq_op = { level = "allow", priority = 2 } # In tests we test equality. while_immutable_condition = { level = "allow", priority = 2 } # false positives needless_option_as_deref = { level = "allow", priority = 2 } # false positives derivable_impls = { level = "allow", priority = 2 } # false positives +clone_on_copy = { level = "allow", priority = 2 } # false positives stable_sort_primitive = { level = "allow", priority = 2 } # prefer stable sort extra-unused-type-parameters = { level = "allow", priority = 2 } # stylistic default_constructed_unit_structs = { level = "allow", priority = 2 } # stylistic From 44439c600eecba02eb3168e174d3ac8d9721ed7e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=B3nal=20Murray?= Date: Wed, 13 Dec 2023 15:23:09 +0000 Subject: [PATCH 5/9] Add CI job to run clippy on all features --- .gitlab/pipeline/check.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitlab/pipeline/check.yml b/.gitlab/pipeline/check.yml index 3b14034a110f..d53b4fbe3d98 100644 --- a/.gitlab/pipeline/check.yml +++ b/.gitlab/pipeline/check.yml @@ -8,6 +8,7 @@ cargo-clippy: RUSTFLAGS: "-D warnings" script: - SKIP_WASM_BUILD=1 cargo clippy --all-targets --locked --workspace + - SKIP_WASM_BUILD=1 cargo clippy --all-targets --all-features --locked --workspace check-try-runtime: stage: check From 4604af9a96b0a3f095fa87c7da5b225312e26fdc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=B3nal=20Murray?= Date: Wed, 13 Dec 2023 15:33:05 +0000 Subject: [PATCH 6/9] Add allow for overseer generated code --- Cargo.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/Cargo.toml b/Cargo.toml index c88d36171784..d9d1d9da2140 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -489,6 +489,7 @@ type_complexity = { level = "allow", priority = 2 } # raison d' nonminimal-bool = { level = "allow", priority = 2 } # maybe borrowed-box = { level = "allow", priority = 2 } # Reasonable to fix this one too-many-arguments = { level = "allow", priority = 2 } # (Turning this on would lead to) +needless-lifetimes = { level = "allow", priority = 2 } # generated code unnecessary_cast = { level = "allow", priority = 2 } # Types may change identity-op = { level = "allow", priority = 2 } # One case where we do 0 + useless_conversion = { level = "allow", priority = 2 } # Types may change From f55c97218ff502288974535713951549e1fef9f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=B3nal=20Murray?= Date: Tue, 19 Dec 2023 15:05:23 +0000 Subject: [PATCH 7/9] Fix clone_on_copy instead of allowing it --- Cargo.toml | 1 - bridges/modules/relayers/src/benchmarking.rs | 4 +-- .../collective-content/src/benchmarking.rs | 2 +- .../src/paras_inherent/benchmarking.rs | 4 +-- .../src/generic/benchmarking.rs | 26 +++++++------- .../xcm-simulator/example/src/parachain.rs | 2 +- substrate/frame/alliance/src/benchmarking.rs | 36 +++++++++---------- .../frame/contracts/src/benchmarking/mod.rs | 2 +- .../frame/contracts/src/migration/v12.rs | 2 +- substrate/frame/democracy/src/benchmarking.rs | 2 +- .../frame/democracy/src/migrations/v1.rs | 8 ++--- substrate/frame/recovery/src/benchmarking.rs | 8 ++--- substrate/frame/sassafras/src/benchmarking.rs | 2 +- substrate/frame/scheduler/src/migration.rs | 2 +- substrate/frame/staking/src/pallet/impls.rs | 2 +- .../frame/state-trie-migration/src/lib.rs | 4 +-- substrate/frame/uniques/src/benchmarking.rs | 4 +-- 17 files changed, 55 insertions(+), 56 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 0050aaad4138..622b9d1891b4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -502,7 +502,6 @@ eq_op = { level = "allow", priority = 2 } # In tests while_immutable_condition = { level = "allow", priority = 2 } # false positives needless_option_as_deref = { level = "allow", priority = 2 } # false positives derivable_impls = { level = "allow", priority = 2 } # false positives -clone_on_copy = { level = "allow", priority = 2 } # false positives stable_sort_primitive = { level = "allow", priority = 2 } # prefer stable sort extra-unused-type-parameters = { level = "allow", priority = 2 } # stylistic default_constructed_unit_structs = { level = "allow", priority = 2 } # stylistic diff --git a/bridges/modules/relayers/src/benchmarking.rs b/bridges/modules/relayers/src/benchmarking.rs index 2d74ab38f9db..00c3814a4c38 100644 --- a/bridges/modules/relayers/src/benchmarking.rs +++ b/bridges/modules/relayers/src/benchmarking.rs @@ -104,7 +104,7 @@ benchmarks! { // create slash destination account let lane = LaneId([0, 0, 0, 0]); let slash_destination = RewardsAccountParams::new(lane, *b"test", RewardsAccountOwner::ThisChain); - T::prepare_rewards_account(slash_destination.clone(), Zero::zero()); + T::prepare_rewards_account(slash_destination, Zero::zero()); }: { crate::Pallet::::slash_and_deregister(&relayer, slash_destination) } @@ -121,7 +121,7 @@ benchmarks! { let account_params = RewardsAccountParams::new(lane, *b"test", RewardsAccountOwner::ThisChain); }: { - crate::Pallet::::register_relayer_reward(account_params.clone(), &relayer, One::one()); + crate::Pallet::::register_relayer_reward(account_params, &relayer, One::one()); } verify { assert_eq!(RelayerRewards::::get(relayer, &account_params), Some(One::one())); diff --git a/cumulus/parachains/pallets/collective-content/src/benchmarking.rs b/cumulus/parachains/pallets/collective-content/src/benchmarking.rs index 1f145f725b13..943386a84276 100644 --- a/cumulus/parachains/pallets/collective-content/src/benchmarking.rs +++ b/cumulus/parachains/pallets/collective-content/src/benchmarking.rs @@ -56,7 +56,7 @@ mod benchmarks { .map_err(|_| BenchmarkError::Weightless)?; #[extrinsic_call] - _(origin as T::RuntimeOrigin, cid.clone(), Some(expire_at.clone())); + _(origin as T::RuntimeOrigin, cid.clone(), Some(expire_at)); assert_eq!(>::count(), 1); assert_last_event::( diff --git a/polkadot/runtime/parachains/src/paras_inherent/benchmarking.rs b/polkadot/runtime/parachains/src/paras_inherent/benchmarking.rs index 3043127c3174..4509c5c3cc72 100644 --- a/polkadot/runtime/parachains/src/paras_inherent/benchmarking.rs +++ b/polkadot/runtime/parachains/src/paras_inherent/benchmarking.rs @@ -132,7 +132,7 @@ benchmarks! { // Ensure that the votes are for the correct session assert_eq!(vote.session, scenario._session); // Ensure that there are an expected number of candidates - let header = BenchBuilder::::header(scenario._block_number.clone()); + let header = BenchBuilder::::header(scenario._block_number); // Traverse candidates and assert descriptors are as expected for (para_id, backing_validators) in vote.backing_validators_per_candidate.iter().enumerate() { let descriptor = backing_validators.0.descriptor(); @@ -189,7 +189,7 @@ benchmarks! { // Ensure that the votes are for the correct session assert_eq!(vote.session, scenario._session); // Ensure that there are an expected number of candidates - let header = BenchBuilder::::header(scenario._block_number.clone()); + let header = BenchBuilder::::header(scenario._block_number); // Traverse candidates and assert descriptors are as expected for (para_id, backing_validators) in vote.backing_validators_per_candidate.iter().enumerate() { diff --git a/polkadot/xcm/pallet-xcm-benchmarks/src/generic/benchmarking.rs b/polkadot/xcm/pallet-xcm-benchmarks/src/generic/benchmarking.rs index f1c48ba9b830..50a7fe45e231 100644 --- a/polkadot/xcm/pallet-xcm-benchmarks/src/generic/benchmarking.rs +++ b/polkadot/xcm/pallet-xcm-benchmarks/src/generic/benchmarking.rs @@ -175,7 +175,7 @@ benchmarks! { descend_origin { let mut executor = new_executor::(Default::default()); let who = X2(OnlyChild, OnlyChild); - let instruction = Instruction::DescendOrigin(who.clone()); + let instruction = Instruction::DescendOrigin(who); let xcm = Xcm(vec![instruction]); } : { executor.bench_process(xcm)?; @@ -242,7 +242,7 @@ benchmarks! { &origin, assets.clone().into(), &XcmContext { - origin: Some(origin.clone()), + origin: Some(origin), message_id: [0; 32], topic: None, }, @@ -279,7 +279,7 @@ benchmarks! { let origin = T::subscribe_origin()?; let query_id = Default::default(); let max_response_weight = Default::default(); - let mut executor = new_executor::(origin.clone()); + let mut executor = new_executor::(origin); let instruction = Instruction::SubscribeVersion { query_id, max_response_weight }; let xcm = Xcm(vec![instruction]); } : { @@ -299,14 +299,14 @@ benchmarks! { query_id, max_response_weight, &XcmContext { - origin: Some(origin.clone()), + origin: Some(origin), message_id: [0; 32], topic: None, }, ).map_err(|_| "Could not start subscription")?; assert!(::SubscriptionService::is_subscribed(&origin)); - let mut executor = new_executor::(origin.clone()); + let mut executor = new_executor::(origin); let instruction = Instruction::UnsubscribeVersion; let xcm = Xcm(vec![instruction]); } : { @@ -538,7 +538,7 @@ benchmarks! { let mut executor = new_executor::(origin); - let instruction = Instruction::UniversalOrigin(alias.clone()); + let instruction = Instruction::UniversalOrigin(alias); let xcm = Xcm(vec![instruction]); }: { executor.bench_process(xcm)?; @@ -632,13 +632,13 @@ benchmarks! { let (unlocker, owner, asset) = T::unlockable_asset()?; - let mut executor = new_executor::(unlocker.clone()); + let mut executor = new_executor::(unlocker); // We first place the asset in lock first... ::AssetLocker::prepare_lock( unlocker, asset.clone(), - owner.clone(), + owner, ) .map_err(|_| BenchmarkError::Skip)? .enact() @@ -658,13 +658,13 @@ benchmarks! { let (unlocker, owner, asset) = T::unlockable_asset()?; - let mut executor = new_executor::(unlocker.clone()); + let mut executor = new_executor::(unlocker); // We first place the asset in lock first... ::AssetLocker::prepare_lock( unlocker, asset.clone(), - owner.clone(), + owner, ) .map_err(|_| BenchmarkError::Skip)? .enact() @@ -686,9 +686,9 @@ benchmarks! { // We first place the asset in lock first... ::AssetLocker::prepare_lock( - locker.clone(), + locker, asset.clone(), - owner.clone(), + owner, ) .map_err(|_| BenchmarkError::Skip)? .enact() @@ -739,7 +739,7 @@ benchmarks! { let mut executor = new_executor::(origin); - let instruction = Instruction::AliasOrigin(target.clone()); + let instruction = Instruction::AliasOrigin(target); let xcm = Xcm(vec![instruction]); }: { executor.bench_process(xcm)?; diff --git a/polkadot/xcm/xcm-simulator/example/src/parachain.rs b/polkadot/xcm/xcm-simulator/example/src/parachain.rs index 951e946372da..9908ee9703b4 100644 --- a/polkadot/xcm/xcm-simulator/example/src/parachain.rs +++ b/polkadot/xcm/xcm-simulator/example/src/parachain.rs @@ -165,7 +165,7 @@ impl EnsureOriginWithArg for ForeignCreators { #[cfg(feature = "runtime-benchmarks")] fn try_successful_origin(a: &MultiLocation) -> Result { - Ok(pallet_xcm::Origin::Xcm(a.clone()).into()) + Ok(pallet_xcm::Origin::Xcm(*a).into()) } } diff --git a/substrate/frame/alliance/src/benchmarking.rs b/substrate/frame/alliance/src/benchmarking.rs index 37cc3314037e..e3d0bb2ba41d 100644 --- a/substrate/frame/alliance/src/benchmarking.rs +++ b/substrate/frame/alliance/src/benchmarking.rs @@ -183,7 +183,7 @@ mod benchmarks { let voter = &members[j as usize]; Alliance::::vote( SystemOrigin::Signed(voter.clone()).into(), - last_hash.clone(), + last_hash, index, true, )?; @@ -193,7 +193,7 @@ mod benchmarks { // Voter votes aye without resolving the vote. Alliance::::vote( SystemOrigin::Signed(voter.clone()).into(), - last_hash.clone(), + last_hash, index, true, )?; @@ -206,7 +206,7 @@ mod benchmarks { frame_benchmarking::benchmarking::add_to_whitelist(voter_key.into()); #[extrinsic_call] - _(SystemOrigin::Signed(voter), last_hash.clone(), index, approve); + _(SystemOrigin::Signed(voter), last_hash, index, approve); //nothing to verify Ok(()) @@ -255,7 +255,7 @@ mod benchmarks { let voter = &members[j as usize]; Alliance::::vote( SystemOrigin::Signed(voter.clone()).into(), - last_hash.clone(), + last_hash, index, true, )?; @@ -264,7 +264,7 @@ mod benchmarks { // Voter votes aye without resolving the vote. Alliance::::vote( SystemOrigin::Signed(voter.clone()).into(), - last_hash.clone(), + last_hash, index, true, )?; @@ -272,7 +272,7 @@ mod benchmarks { // Voter switches vote to nay, which kills the vote Alliance::::vote( SystemOrigin::Signed(voter.clone()).into(), - last_hash.clone(), + last_hash, index, false, )?; @@ -282,7 +282,7 @@ mod benchmarks { frame_benchmarking::benchmarking::add_to_whitelist(voter_key.into()); #[extrinsic_call] - close(SystemOrigin::Signed(voter), last_hash.clone(), index, Weight::MAX, bytes_in_storage); + close(SystemOrigin::Signed(voter), last_hash, index, Weight::MAX, bytes_in_storage); assert_eq!(T::ProposalProvider::proposal_of(last_hash), None); Ok(()) @@ -330,7 +330,7 @@ mod benchmarks { // approval vote Alliance::::vote( SystemOrigin::Signed(proposer.clone()).into(), - last_hash.clone(), + last_hash, index, false, )?; @@ -340,7 +340,7 @@ mod benchmarks { let voter = &members[j as usize]; Alliance::::vote( SystemOrigin::Signed(voter.clone()).into(), - last_hash.clone(), + last_hash, index, false, )?; @@ -349,7 +349,7 @@ mod benchmarks { // Member zero is the first aye Alliance::::vote( SystemOrigin::Signed(members[0].clone()).into(), - last_hash.clone(), + last_hash, index, true, )?; @@ -358,13 +358,13 @@ mod benchmarks { // Caller switches vote to aye, which passes the vote Alliance::::vote( SystemOrigin::Signed(voter.clone()).into(), - last_hash.clone(), + last_hash, index, true, )?; #[extrinsic_call] - close(SystemOrigin::Signed(voter), last_hash.clone(), index, Weight::MAX, bytes_in_storage); + close(SystemOrigin::Signed(voter), last_hash, index, Weight::MAX, bytes_in_storage); assert_eq!(T::ProposalProvider::proposal_of(last_hash), None); Ok(()) @@ -414,7 +414,7 @@ mod benchmarks { let voter = &members[j as usize]; Alliance::::vote( SystemOrigin::Signed(voter.clone()).into(), - last_hash.clone(), + last_hash, index, true, )?; @@ -422,7 +422,7 @@ mod benchmarks { Alliance::::vote( SystemOrigin::Signed(voter.clone()).into(), - last_hash.clone(), + last_hash, index, false, )?; @@ -430,7 +430,7 @@ mod benchmarks { System::::set_block_number(BlockNumberFor::::max_value()); #[extrinsic_call] - close(SystemOrigin::Signed(voter), last_hash.clone(), index, Weight::MAX, bytes_in_storage); + close(SystemOrigin::Signed(voter), last_hash, index, Weight::MAX, bytes_in_storage); // The last proposal is removed. assert_eq!(T::ProposalProvider::proposal_of(last_hash), None); @@ -477,7 +477,7 @@ mod benchmarks { // The prime member votes aye, so abstentions default to aye. Alliance::::vote( SystemOrigin::Signed(proposer.clone()).into(), - last_hash.clone(), + last_hash, p - 1, true, // Vote aye. )?; @@ -489,7 +489,7 @@ mod benchmarks { let voter = &members[j as usize]; Alliance::::vote( SystemOrigin::Signed(voter.clone()).into(), - last_hash.clone(), + last_hash, index, false, )?; @@ -501,7 +501,7 @@ mod benchmarks { #[extrinsic_call] close( SystemOrigin::Signed(proposer), - last_hash.clone(), + last_hash, index, Weight::MAX, bytes_in_storage, diff --git a/substrate/frame/contracts/src/benchmarking/mod.rs b/substrate/frame/contracts/src/benchmarking/mod.rs index 208d93185995..3ab76d6112a2 100644 --- a/substrate/frame/contracts/src/benchmarking/mod.rs +++ b/substrate/frame/contracts/src/benchmarking/mod.rs @@ -1749,7 +1749,7 @@ benchmarks! { .collect::>>(); let deposits_bytes: Vec = deposits.iter().flat_map(|i| i.encode()).collect(); let deposits_len = deposits_bytes.len() as u32; - let deposit_len = value_len.clone(); + let deposit_len = value_len; let callee_offset = value_len + deposits_len; let code = WasmModule::::from(ModuleDefinition { memory: Some(ImportedMemory::max::()), diff --git a/substrate/frame/contracts/src/migration/v12.rs b/substrate/frame/contracts/src/migration/v12.rs index 4ddc57584b30..7dee31503101 100644 --- a/substrate/frame/contracts/src/migration/v12.rs +++ b/substrate/frame/contracts/src/migration/v12.rs @@ -317,7 +317,7 @@ where let (_, old_deposit, storage_module) = state; // CodeInfoOf::max_encoded_len == OwnerInfoOf::max_encoded_len + 1 // I.e. code info adds up 1 byte per record. - let info_bytes_added = items.clone(); + let info_bytes_added = items; // We removed 1 PrefabWasmModule, and added 1 byte of determinism flag, per contract code. let storage_removed = storage_module.saturating_sub(info_bytes_added); // module+code+info - bytes diff --git a/substrate/frame/democracy/src/benchmarking.rs b/substrate/frame/democracy/src/benchmarking.rs index b4aa17726b8d..aa66137ad880 100644 --- a/substrate/frame/democracy/src/benchmarking.rs +++ b/substrate/frame/democracy/src/benchmarking.rs @@ -65,7 +65,7 @@ fn add_referendum(n: u32) -> (ReferendumIndex, T::Hash, T::Hash) { 0u32.into(), ); let preimage_hash = note_preimage::(); - MetadataOf::::insert(crate::MetadataOwner::Referendum(index), preimage_hash.clone()); + MetadataOf::::insert(crate::MetadataOwner::Referendum(index), preimage_hash); (index, hash, preimage_hash) } diff --git a/substrate/frame/democracy/src/migrations/v1.rs b/substrate/frame/democracy/src/migrations/v1.rs index c27f437901b7..2e6c3a2d184d 100644 --- a/substrate/frame/democracy/src/migrations/v1.rs +++ b/substrate/frame/democracy/src/migrations/v1.rs @@ -172,7 +172,7 @@ mod test { let hash = H256::repeat_byte(1); let status = ReferendumStatus { end: 1u32.into(), - proposal: hash.clone(), + proposal: hash, threshold: VoteThreshold::SuperMajorityApprove, delay: 1u32.into(), tally: Tally { ayes: 1u32.into(), nays: 1u32.into(), turnout: 1u32.into() }, @@ -188,12 +188,12 @@ mod test { // Case 3: Public proposals let hash2 = H256::repeat_byte(2); v0::PublicProps::::put(vec![ - (3u32, hash.clone(), 123u64), - (4u32, hash2.clone(), 123u64), + (3u32, hash, 123u64), + (4u32, hash2, 123u64), ]); // Case 4: Next external - v0::NextExternal::::put((hash.clone(), VoteThreshold::SuperMajorityApprove)); + v0::NextExternal::::put((hash, VoteThreshold::SuperMajorityApprove)); // Migrate. let state = v1::Migration::::pre_upgrade().unwrap(); diff --git a/substrate/frame/recovery/src/benchmarking.rs b/substrate/frame/recovery/src/benchmarking.rs index 2deb55bb69f2..72f77336212d 100644 --- a/substrate/frame/recovery/src/benchmarking.rs +++ b/substrate/frame/recovery/src/benchmarking.rs @@ -190,7 +190,7 @@ benchmarks! { let recovery_config = RecoveryConfig { delay_period: DEFAULT_DELAY.into(), - deposit: total_deposit.clone(), + deposit: total_deposit, friends: bounded_friends.clone(), threshold: n as u16, }; @@ -243,7 +243,7 @@ benchmarks! { let recovery_config = RecoveryConfig { delay_period: 0u32.into(), - deposit: total_deposit.clone(), + deposit: total_deposit, friends: bounded_friends.clone(), threshold: n as u16, }; @@ -294,7 +294,7 @@ benchmarks! { let recovery_config = RecoveryConfig { delay_period: DEFAULT_DELAY.into(), - deposit: total_deposit.clone(), + deposit: total_deposit, friends: bounded_friends.clone(), threshold: n as u16, }; @@ -342,7 +342,7 @@ benchmarks! { let recovery_config = RecoveryConfig { delay_period: DEFAULT_DELAY.into(), - deposit: total_deposit.clone(), + deposit: total_deposit, friends: bounded_friends.clone(), threshold: n as u16, }; diff --git a/substrate/frame/sassafras/src/benchmarking.rs b/substrate/frame/sassafras/src/benchmarking.rs index 95a2b4bbce4e..921f2f0793d3 100644 --- a/substrate/frame/sassafras/src/benchmarking.rs +++ b/substrate/frame/sassafras/src/benchmarking.rs @@ -260,7 +260,7 @@ mod benchmarks { // Update metadata let mut meta = TicketsMeta::::get(); meta.unsorted_tickets_count = tickets_count; - TicketsMeta::::set(meta.clone()); + TicketsMeta::::set(meta); log::debug!(target: LOG_TARGET, "Before sort: {:?}", meta); #[block] diff --git a/substrate/frame/scheduler/src/migration.rs b/substrate/frame/scheduler/src/migration.rs index 850a245817b9..76e2e04b49cc 100644 --- a/substrate/frame/scheduler/src/migration.rs +++ b/substrate/frame/scheduler/src/migration.rs @@ -362,7 +362,7 @@ mod test { Some(ScheduledV3Of:: { maybe_id: Some(vec![i as u8; 320]), priority: 123, - call: MaybeHashed::Hash(undecodable_hash.clone()), + call: MaybeHashed::Hash(undecodable_hash), maybe_periodic: Some((4u64, 20)), origin: root(), _phantom: PhantomData::::default(), diff --git a/substrate/frame/staking/src/pallet/impls.rs b/substrate/frame/staking/src/pallet/impls.rs index d294686751cd..093cdfdb9cb9 100644 --- a/substrate/frame/staking/src/pallet/impls.rs +++ b/substrate/frame/staking/src/pallet/impls.rs @@ -1801,7 +1801,7 @@ impl StakingInterface for Pallet { ) { let others = exposures .iter() - .map(|(who, value)| IndividualExposure { who: who.clone(), value: value.clone() }) + .map(|(who, value)| IndividualExposure { who: who.clone(), value: *value }) .collect::>(); let exposure = Exposure { total: Default::default(), own: Default::default(), others }; EraInfo::::set_exposure(*current_era, stash, exposure); diff --git a/substrate/frame/state-trie-migration/src/lib.rs b/substrate/frame/state-trie-migration/src/lib.rs index 5330634ca076..8652e8e9561c 100644 --- a/substrate/frame/state-trie-migration/src/lib.rs +++ b/substrate/frame/state-trie-migration/src/lib.rs @@ -1637,7 +1637,7 @@ pub(crate) mod remote_tests { weight_sum += StateTrieMigration::::on_initialize(System::::block_number()); - root = System::::finalize().state_root().clone(); + root = *System::::finalize().state_root(); System::::on_finalize(System::::block_number()); } (root, weight_sum) @@ -1687,7 +1687,7 @@ pub(crate) mod remote_tests { ); loop { - let last_state_root = ext.backend.root().clone(); + let last_state_root = *ext.backend.root(); let ((finished, weight), proof) = ext.execute_and_prove(|| { let weight = run_to_block::(now + One::one()).1; if StateTrieMigration::::migration_process().finished() { diff --git a/substrate/frame/uniques/src/benchmarking.rs b/substrate/frame/uniques/src/benchmarking.rs index 821ca1794b86..80d02f136218 100644 --- a/substrate/frame/uniques/src/benchmarking.rs +++ b/substrate/frame/uniques/src/benchmarking.rs @@ -431,9 +431,9 @@ benchmarks_instance_pallet! { let buyer_lookup = T::Lookup::unlookup(buyer.clone()); let price = ItemPrice::::from(0u32); let origin = SystemOrigin::Signed(seller.clone()).into(); - Uniques::::set_price(origin, collection.clone(), item, Some(price.clone()), Some(buyer_lookup))?; + Uniques::::set_price(origin, collection.clone(), item, Some(price), Some(buyer_lookup))?; T::Currency::make_free_balance_be(&buyer, DepositBalanceOf::::max_value()); - }: _(SystemOrigin::Signed(buyer.clone()), collection.clone(), item, price.clone()) + }: _(SystemOrigin::Signed(buyer.clone()), collection.clone(), item, price) verify { assert_last_event::(Event::ItemBought { collection: collection.clone(), From 2bf4a96bf2def97113039287bed72f1f82488ce2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=B3nal=20Murray?= Date: Tue, 19 Dec 2023 17:06:24 +0000 Subject: [PATCH 8/9] fmt --- substrate/frame/alliance/src/benchmarking.rs | 29 +++---------------- .../frame/democracy/src/migrations/v1.rs | 5 +--- 2 files changed, 5 insertions(+), 29 deletions(-) diff --git a/substrate/frame/alliance/src/benchmarking.rs b/substrate/frame/alliance/src/benchmarking.rs index e3d0bb2ba41d..cb2a04f17c57 100644 --- a/substrate/frame/alliance/src/benchmarking.rs +++ b/substrate/frame/alliance/src/benchmarking.rs @@ -191,12 +191,7 @@ mod benchmarks { let voter = members[m as usize - 3].clone(); // Voter votes aye without resolving the vote. - Alliance::::vote( - SystemOrigin::Signed(voter.clone()).into(), - last_hash, - index, - true, - )?; + Alliance::::vote(SystemOrigin::Signed(voter.clone()).into(), last_hash, index, true)?; // Voter switches vote to nay, but does not kill the vote, just updates + inserts let approve = false; @@ -262,12 +257,7 @@ mod benchmarks { } // Voter votes aye without resolving the vote. - Alliance::::vote( - SystemOrigin::Signed(voter.clone()).into(), - last_hash, - index, - true, - )?; + Alliance::::vote(SystemOrigin::Signed(voter.clone()).into(), last_hash, index, true)?; // Voter switches vote to nay, which kills the vote Alliance::::vote( @@ -356,12 +346,7 @@ mod benchmarks { let voter = members[1].clone(); // Caller switches vote to aye, which passes the vote - Alliance::::vote( - SystemOrigin::Signed(voter.clone()).into(), - last_hash, - index, - true, - )?; + Alliance::::vote(SystemOrigin::Signed(voter.clone()).into(), last_hash, index, true)?; #[extrinsic_call] close(SystemOrigin::Signed(voter), last_hash, index, Weight::MAX, bytes_in_storage); @@ -499,13 +484,7 @@ mod benchmarks { System::::set_block_number(BlockNumberFor::::max_value()); #[extrinsic_call] - close( - SystemOrigin::Signed(proposer), - last_hash, - index, - Weight::MAX, - bytes_in_storage, - ); + close(SystemOrigin::Signed(proposer), last_hash, index, Weight::MAX, bytes_in_storage); assert_eq!(T::ProposalProvider::proposal_of(last_hash), None); Ok(()) diff --git a/substrate/frame/democracy/src/migrations/v1.rs b/substrate/frame/democracy/src/migrations/v1.rs index 2e6c3a2d184d..64baea8f3af7 100644 --- a/substrate/frame/democracy/src/migrations/v1.rs +++ b/substrate/frame/democracy/src/migrations/v1.rs @@ -187,10 +187,7 @@ mod test { // Case 3: Public proposals let hash2 = H256::repeat_byte(2); - v0::PublicProps::::put(vec![ - (3u32, hash, 123u64), - (4u32, hash2, 123u64), - ]); + v0::PublicProps::::put(vec![(3u32, hash, 123u64), (4u32, hash2, 123u64)]); // Case 4: Next external v0::NextExternal::::put((hash, VoteThreshold::SuperMajorityApprove)); From 8567b54292ec3c78042a1da207257b55371fc628 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=B3nal=20Murray?= Date: Wed, 20 Dec 2023 08:28:28 +0000 Subject: [PATCH 9/9] Update benchmarking.rs Co-authored-by: Liam Aharon --- cumulus/pallets/xcmp-queue/src/benchmarking.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cumulus/pallets/xcmp-queue/src/benchmarking.rs b/cumulus/pallets/xcmp-queue/src/benchmarking.rs index c363cc2454ad..49e2cc836734 100644 --- a/cumulus/pallets/xcmp-queue/src/benchmarking.rs +++ b/cumulus/pallets/xcmp-queue/src/benchmarking.rs @@ -85,7 +85,7 @@ mod benchmarks { } assert!( - !OutboundXcmpStatus::::get().iter().any(|p| p.recipient == para), + OutboundXcmpStatus::::get().iter().all(|p| p.recipient != para), "No messages in the channel; therefore removed." ); }