Extend the lower bounds of some of the benchmarks to also include 0 (#12386)

* Extend the lower bounds of some of the benchmarks to also include `0`

* Fix verify snippet for `pallet_bounties/spend_funds`

substrate/frame/alliance/src/benchmarking.rs

@@ -832,8 +832,8 @@ benchmarks_instance_pallet! {
 	}
 
 	add_unscrupulous_items {
-		let n in 1 .. T::MaxUnscrupulousItems::get();
+		let n in 0 .. T::MaxUnscrupulousItems::get();
-		let l in 1 .. T::MaxWebsiteUrlLength::get();
+		let l in 0 .. T::MaxWebsiteUrlLength::get();
 
 		set_members::<T, I>();
 
@@ -856,8 +856,8 @@ benchmarks_instance_pallet! {
 	}
 
 	remove_unscrupulous_items {
-		let n in 1 .. T::MaxUnscrupulousItems::get();
+		let n in 0 .. T::MaxUnscrupulousItems::get();
-		let l in 1 .. T::MaxWebsiteUrlLength::get();
+		let l in 0 .. T::MaxWebsiteUrlLength::get();
 
 		set_members::<T, I>();
 

substrate/frame/benchmarking/src/baseline.rs

@@ -90,7 +90,7 @@ benchmarks! {
 	}
 
 	sr25519_verification {
-		let i in 1 .. 100;
+		let i in 0 .. 100;
 
 		let public = SignerId::generate_pair(None);
 

substrate/frame/bounties/src/benchmarking.rs

@@ -197,7 +197,7 @@ benchmarks_instance_pallet! {
 	}
 
 	spend_funds {
-		let b in 1 .. 100;
+		let b in 0 .. 100;
 		setup_pot_account::<T, I>();
 		create_approved_bounties::<T, I>(b)?;
 
@@ -214,9 +214,13 @@ benchmarks_instance_pallet! {
 		);
 	}
 	verify {
-		ensure!(budget_remaining < BalanceOf::<T, I>::max_value(), "Budget not used");
 		ensure!(missed_any == false, "Missed some");
-		assert_last_event::<T, I>(Event::BountyBecameActive { index: b - 1 }.into())
+		if b > 0 {
+			ensure!(budget_remaining < BalanceOf::<T, I>::max_value(), "Budget not used");
+			assert_last_event::<T, I>(Event::BountyBecameActive { index: b - 1 }.into())
+		} else {
+			ensure!(budget_remaining == BalanceOf::<T, I>::max_value(), "Budget used");
+		}
 	}
 
 	impl_benchmark_test_suite!(Bounties, crate::tests::new_test_ext(), crate::tests::Test)

substrate/frame/collective/src/benchmarking.rs

@@ -36,68 +36,69 @@ fn assert_last_event<T: Config<I>, I: 'static>(generic_event: <T as Config<I>>::
 
 benchmarks_instance_pallet! {
 	set_members {
-		let m in 1 .. T::MaxMembers::get();
+		let m in 0 .. T::MaxMembers::get();
-		let n in 1 .. T::MaxMembers::get();
+		let n in 0 .. T::MaxMembers::get();
-		let p in 1 .. T::MaxProposals::get();
+		let p in 0 .. T::MaxProposals::get();
 
 		// Set old members.
 		// We compute the difference of old and new members, so it should influence timing.
 		let mut old_members = vec![];
-		let mut last_old_member = account::<T::AccountId>("old member", 0, SEED);
 		for i in 0 .. m {
-			last_old_member = account::<T::AccountId>("old member", i, SEED);
+			let old_member = account::<T::AccountId>("old member", i, SEED);
-			old_members.push(last_old_member.clone());
+			old_members.push(old_member);
 		}
 		let old_members_count = old_members.len() as u32;
 
 		Collective::<T, I>::set_members(
 			SystemOrigin::Root.into(),
 			old_members.clone(),
-			Some(last_old_member.clone()),
+			old_members.last().cloned(),
 			T::MaxMembers::get(),
 		)?;
 
-		// Set a high threshold for proposals passing so that they stay around.
+		// If there were any old members generate a bunch of proposals.
-		let threshold = m.max(2);
+		if m > 0 {
-		// Length of the proposals should be irrelevant to `set_members`.
+			// Set a high threshold for proposals passing so that they stay around.
-		let length = 100;
+			let threshold = m.max(2);
-		for i in 0 .. p {
+			// Length of the proposals should be irrelevant to `set_members`.
-			// Proposals should be different so that different proposal hashes are generated
+			let length = 100;
-			let proposal: T::Proposal = SystemCall::<T>::remark {
+			for i in 0 .. p {
-				remark: vec![i as u8; length]
+				// Proposals should be different so that different proposal hashes are generated
-			}.into();
+				let proposal: T::Proposal = SystemCall::<T>::remark {
-			Collective::<T, I>::propose(
+					remark: vec![i as u8; length]
-				SystemOrigin::Signed(last_old_member.clone()).into(),
+				}.into();
-				threshold,
+				Collective::<T, I>::propose(
-				Box::new(proposal.clone()),
+					SystemOrigin::Signed(old_members.last().unwrap().clone()).into(),
-				MAX_BYTES,
+					threshold,
-			)?;
+					Box::new(proposal.clone()),
-			let hash = T::Hashing::hash_of(&proposal);
+					MAX_BYTES,
-			// Vote on the proposal to increase state relevant for `set_members`.
-			// Not voting for `last_old_member` because they proposed and not voting for the first member
-			// to keep the proposal from passing.
-			for j in 2 .. m - 1 {
-				let voter = &old_members[j as usize];
-				let approve = true;
-				Collective::<T, I>::vote(
-					SystemOrigin::Signed(voter.clone()).into(),
-					hash,
-					i,
-					approve,
 				)?;
+				let hash = T::Hashing::hash_of(&proposal);
+				// Vote on the proposal to increase state relevant for `set_members`.
+				// Not voting for last old member because they proposed and not voting for the first member
+				// to keep the proposal from passing.
+				for j in 2 .. m - 1 {
+					let voter = &old_members[j as usize];
+					let approve = true;
+					Collective::<T, I>::vote(
+						SystemOrigin::Signed(voter.clone()).into(),
+						hash,
+						i,
+						approve,
+					)?;
+				}
 			}
 		}
 
 		// Construct `new_members`.
 		// It should influence timing since it will sort this vector.
 		let mut new_members = vec![];
-		let mut last_member = account::<T::AccountId>("member", 0, SEED);
 		for i in 0 .. n {
-			last_member = account::<T::AccountId>("member", i, SEED);
+			let member = account::<T::AccountId>("member", i, SEED);
-			new_members.push(last_member.clone());
+			new_members.push(member);
 		}
 
-	}: _(SystemOrigin::Root, new_members.clone(), Some(last_member), T::MaxMembers::get())
+	}: _(SystemOrigin::Root, new_members.clone(), new_members.last().cloned(), T::MaxMembers::get())
 	verify {
 		new_members.sort();
 		assert_eq!(Collective::<T, I>::members(), new_members);

substrate/frame/elections-phragmen/src/benchmarking.rs

@@ -362,7 +362,7 @@ benchmarks! {
 		// total number of voters.
 		let v in (T::MaxVoters::get() / 2) .. T::MaxVoters::get();
 		// those that are defunct and need removal.
-		let d in 1 .. (T::MaxVoters::get() / 2);
+		let d in 0 .. (T::MaxVoters::get() / 2);
 
 		// remove any previous stuff.
 		clean::<T>();

substrate/frame/examples/basic/src/benchmarking.rs

@@ -34,22 +34,22 @@ use frame_system::RawOrigin;
 // Details on using the benchmarks macro can be seen at:
 //   https://paritytech.github.io/substrate/master/frame_benchmarking/trait.Benchmarking.html#tymethod.benchmarks
 benchmarks! {
-	// This will measure the execution time of `set_dummy` for b in [1..1000] range.
+	// This will measure the execution time of `set_dummy` for b in [0..1000] range.
 	set_dummy_benchmark {
 		// This is the benchmark setup phase
-		let b in 1 .. 1000;
+		let b in 0 .. 1000;
 	}: set_dummy(RawOrigin::Root, b.into()) // The execution phase is just running `set_dummy` extrinsic call
 	verify {
 		// This is the optional benchmark verification phase, asserting certain states.
 		assert_eq!(Pallet::<T>::dummy(), Some(b.into()))
 	}
 
-	// This will measure the execution time of `accumulate_dummy` for b in [1..1000] range.
+	// This will measure the execution time of `accumulate_dummy` for b in [0..1000] range.
 	// The benchmark execution phase is shorthanded. When the name of the benchmark case is the same
 	// as the extrinsic call. `_(...)` is used to represent the extrinsic name.
 	// The benchmark verification phase is omitted.
 	accumulate_dummy {
-		let b in 1 .. 1000;
+		let b in 0 .. 1000;
 		// The caller account is whitelisted for DB reads/write by the benchmarking macro.
 		let caller: T::AccountId = whitelisted_caller();
 	}: _(RawOrigin::Signed(caller), b.into())

substrate/frame/gilt/src/benchmarking.rs

@@ -97,7 +97,7 @@ benchmarks! {
 
 	pursue_target_per_item {
 		// bids taken
-		let b in 1..T::MaxQueueLen::get();
+		let b in 0..T::MaxQueueLen::get();
 
 		let caller: T::AccountId = whitelisted_caller();
 		T::Currency::make_free_balance_be(&caller, T::MinFreeze::get() * BalanceOf::<T>::from(b + 1));
@@ -113,7 +113,7 @@ benchmarks! {
 
 	pursue_target_per_queue {
 		// total queues hit
-		let q in 1..T::QueueCount::get();
+		let q in 0..T::QueueCount::get();
 
 		let caller: T::AccountId = whitelisted_caller();
 		T::Currency::make_free_balance_be(&caller, T::MinFreeze::get() * BalanceOf::<T>::from(q + 1));

substrate/frame/identity/src/benchmarking.rs

@@ -130,7 +130,7 @@ benchmarks! {
 
 	set_identity {
 		let r in 1 .. T::MaxRegistrars::get() => add_registrars::<T>(r)?;
-		let x in 1 .. T::MaxAdditionalFields::get();
+		let x in 0 .. T::MaxAdditionalFields::get();
 		let caller = {
 			// The target user
 			let caller: T::AccountId = whitelisted_caller();
@@ -166,7 +166,7 @@ benchmarks! {
 	set_subs_new {
 		let caller: T::AccountId = whitelisted_caller();
 		// Create a new subs vec with s sub accounts
-		let s in 1 .. T::MaxSubAccounts::get() => ();
+		let s in 0 .. T::MaxSubAccounts::get() => ();
 		let subs = create_sub_accounts::<T>(&caller, s)?;
 		ensure!(SubsOf::<T>::get(&caller).1.len() == 0, "Caller already has subs");
 	}: set_subs(RawOrigin::Signed(caller.clone()), subs)
@@ -177,7 +177,7 @@ benchmarks! {
 	set_subs_old {
 		let caller: T::AccountId = whitelisted_caller();
 		// Give them p many previous sub accounts.
-		let p in 1 .. T::MaxSubAccounts::get() => {
+		let p in 0 .. T::MaxSubAccounts::get() => {
 			let _ = add_sub_accounts::<T>(&caller, p)?;
 		};
 		// Remove all subs.
@@ -198,12 +198,12 @@ benchmarks! {
 		let _ = T::Currency::make_free_balance_be(&caller, BalanceOf::<T>::max_value());
 
 		let r in 1 .. T::MaxRegistrars::get() => add_registrars::<T>(r)?;
-		let s in 1 .. T::MaxSubAccounts::get() => {
+		let s in 0 .. T::MaxSubAccounts::get() => {
 			// Give them s many sub accounts
 			let caller: T::AccountId = whitelisted_caller();
 			let _ = add_sub_accounts::<T>(&caller, s)?;
 		};
-		let x in 1 .. T::MaxAdditionalFields::get();
+		let x in 0 .. T::MaxAdditionalFields::get();
 
 		// Create their main identity with x additional fields
 		let info = create_identity_info::<T>(x);
@@ -233,7 +233,7 @@ benchmarks! {
 		let _ = T::Currency::make_free_balance_be(&caller, BalanceOf::<T>::max_value());
 
 		let r in 1 .. T::MaxRegistrars::get() => add_registrars::<T>(r)?;
-		let x in 1 .. T::MaxAdditionalFields::get() => {
+		let x in 0 .. T::MaxAdditionalFields::get() => {
 			// Create their main identity with x additional fields
 			let info = create_identity_info::<T>(x);
 			let caller: T::AccountId = whitelisted_caller();
@@ -251,7 +251,7 @@ benchmarks! {
 		let _ = T::Currency::make_free_balance_be(&caller, BalanceOf::<T>::max_value());
 
 		let r in 1 .. T::MaxRegistrars::get() => add_registrars::<T>(r)?;
-		let x in 1 .. T::MaxAdditionalFields::get() => {
+		let x in 0 .. T::MaxAdditionalFields::get() => {
 			// Create their main identity with x additional fields
 			let info = create_identity_info::<T>(x);
 			let caller: T::AccountId = whitelisted_caller();
@@ -332,7 +332,7 @@ benchmarks! {
 		let _ = T::Currency::make_free_balance_be(&caller, BalanceOf::<T>::max_value());
 
 		let r in 1 .. T::MaxRegistrars::get() - 1 => add_registrars::<T>(r)?;
-		let x in 1 .. T::MaxAdditionalFields::get();
+		let x in 0 .. T::MaxAdditionalFields::get();
 
 		let info = create_identity_info::<T>(x);
 		let info_hash = T::Hashing::hash_of(&info);
@@ -348,8 +348,8 @@ benchmarks! {
 
 	kill_identity {
 		let r in 1 .. T::MaxRegistrars::get() => add_registrars::<T>(r)?;
-		let s in 1 .. T::MaxSubAccounts::get();
+		let s in 0 .. T::MaxSubAccounts::get();
-		let x in 1 .. T::MaxAdditionalFields::get();
+		let x in 0 .. T::MaxAdditionalFields::get();
 
 		let target: T::AccountId = account("target", 0, SEED);
 		let target_origin: <T as frame_system::Config>::RuntimeOrigin = RawOrigin::Signed(target.clone()).into();
@@ -379,7 +379,7 @@ benchmarks! {
 	}
 
 	add_sub {
-		let s in 1 .. T::MaxSubAccounts::get() - 1;
+		let s in 0 .. T::MaxSubAccounts::get() - 1;
 
 		let caller: T::AccountId = whitelisted_caller();
 		let _ = add_sub_accounts::<T>(&caller, s)?;
@@ -415,7 +415,7 @@ benchmarks! {
 	}
 
 	quit_sub {
-		let s in 1 .. T::MaxSubAccounts::get() - 1;
+		let s in 0 .. T::MaxSubAccounts::get() - 1;
 
 		let caller: T::AccountId = whitelisted_caller();
 		let sup = account("super", 0, SEED);

substrate/frame/ranked-collective/src/benchmarking.rs

@@ -138,7 +138,7 @@ benchmarks_instance_pallet! {
 	}
 
 	cleanup_poll {
-		let n in 1 .. 100;
+		let n in 0 .. 100;
 
 		// Create a poll
 		let class = T::Polls::classes().into_iter().next().unwrap();

substrate/frame/staking/src/benchmarking.rs

@@ -88,6 +88,7 @@ pub fn create_validator_with_nominators<T: Config>(
 	points_total += 10;
 	points_individual.push((v_stash.clone(), 10));
 
+	let original_nominator_count = Nominators::<T>::count();
 	let mut nominators = Vec::new();
 
 	// Give the validator n nominators, but keep total users in the system the same.
@@ -114,7 +115,7 @@ pub fn create_validator_with_nominators<T: Config>(
 	assert_eq!(new_validators.len(), 1);
 	assert_eq!(new_validators[0], v_stash, "Our validator was not selected!");
 	assert_ne!(Validators::<T>::count(), 0);
-	assert_ne!(Nominators::<T>::count(), 0);
+	assert_eq!(Nominators::<T>::count(), original_nominator_count + nominators.len() as u32);
 
 	// Give Era Points
 	let reward = EraRewardPoints::<T::AccountId> {
@@ -544,7 +545,7 @@ benchmarks! {
 	}
 
 	payout_stakers_dead_controller {
-		let n in 1 .. T::MaxNominatorRewardedPerValidator::get() as u32;
+		let n in 0 .. T::MaxNominatorRewardedPerValidator::get() as u32;
 		let (validator, nominators) = create_validator_with_nominators::<T>(
 			n,
 			T::MaxNominatorRewardedPerValidator::get() as u32,
@@ -577,7 +578,7 @@ benchmarks! {
 	}
 
 	payout_stakers_alive_staked {
-		let n in 1 .. T::MaxNominatorRewardedPerValidator::get() as u32;
+		let n in 0 .. T::MaxNominatorRewardedPerValidator::get() as u32;
 		let (validator, nominators) = create_validator_with_nominators::<T>(
 			n,
 			T::MaxNominatorRewardedPerValidator::get() as u32,
@@ -695,7 +696,7 @@ benchmarks! {
 
 	new_era {
 		let v in 1 .. 10;
-		let n in 1 .. 100;
+		let n in 0 .. 100;
 
 		create_validators_with_nominators_for_era::<T>(
 			v,
@@ -714,7 +715,7 @@ benchmarks! {
 	#[extra]
 	payout_all {
 		let v in 1 .. 10;
-		let n in 1 .. 100;
+		let n in 0 .. 100;
 		create_validators_with_nominators_for_era::<T>(
 			v,
 			n,

substrate/frame/system/benchmarking/src/lib.rs

@@ -64,7 +64,7 @@ benchmarks! {
 
 	#[skip_meta]
 	set_storage {
-		let i in 1 .. 1000;
+		let i in 0 .. 1000;
 
 		// Set up i items to add
 		let mut items = Vec::new();
@@ -72,56 +72,69 @@ benchmarks! {
 			let hash = (i, j).using_encoded(T::Hashing::hash).as_ref().to_vec();
 			items.push((hash.clone(), hash.clone()));
 		}
+
+		let items_to_verify = items.clone();
 	}: _(RawOrigin::Root, items)
 	verify {
-		let last_hash = (i, i - 1).using_encoded(T::Hashing::hash);
+		// Verify that they're actually in the storage.
-		let value = storage::unhashed::get_raw(last_hash.as_ref()).ok_or("No value stored")?;
+		for (item, _) in items_to_verify {
-		assert_eq!(value, last_hash.as_ref().to_vec());
+			let value = storage::unhashed::get_raw(&item).ok_or("No value stored")?;
+			assert_eq!(value, *item);
+		}
 	}
 
 	#[skip_meta]
 	kill_storage {
-		let i in 1 .. 1000;
+		let i in 0 .. 1000;
 
 		// Add i items to storage
-		let mut items = Vec::new();
+		let mut items = Vec::with_capacity(i as usize);
 		for j in 0 .. i {
 			let hash = (i, j).using_encoded(T::Hashing::hash).as_ref().to_vec();
 			storage::unhashed::put_raw(&hash, &hash);
 			items.push(hash);
 		}
 
-		// We will verify this value is removed
+		// Verify that they're actually in the storage.
-		let last_hash = (i, i - 1).using_encoded(T::Hashing::hash);
+		for item in &items {
-		let value = storage::unhashed::get_raw(last_hash.as_ref()).ok_or("No value stored")?;
+			let value = storage::unhashed::get_raw(item).ok_or("No value stored")?;
-		assert_eq!(value, last_hash.as_ref().to_vec());
+			assert_eq!(value, *item);
+		}
 
+		let items_to_verify = items.clone();
 	}: _(RawOrigin::Root, items)
 	verify {
-		assert_eq!(storage::unhashed::get_raw(last_hash.as_ref()), None);
+		// Verify that they're not in the storage anymore.
+		for item in items_to_verify {
+			assert!(storage::unhashed::get_raw(&item).is_none());
+		}
 	}
 
 	#[skip_meta]
 	kill_prefix {
-		let p in 1 .. 1000;
+		let p in 0 .. 1000;
 
 		let prefix = p.using_encoded(T::Hashing::hash).as_ref().to_vec();
+		let mut items = Vec::with_capacity(p as usize);
 		// add p items that share a prefix
 		for i in 0 .. p {
 			let hash = (p, i).using_encoded(T::Hashing::hash).as_ref().to_vec();
 			let key = [&prefix[..], &hash[..]].concat();
 			storage::unhashed::put_raw(&key, &key);
+			items.push(key);
 		}
 
-		// We will verify this value is removed
+		// Verify that they're actually in the storage.
-		let last_hash = (p, p - 1).using_encoded(T::Hashing::hash).as_ref().to_vec();
+		for item in &items {
-		let last_key = [&prefix[..], &last_hash[..]].concat();
+			let value = storage::unhashed::get_raw(item).ok_or("No value stored")?;
-		let value = storage::unhashed::get_raw(&last_key).ok_or("No value stored")?;
+			assert_eq!(value, *item);
-		assert_eq!(value, last_key);
+		}
-
 	}: _(RawOrigin::Root, prefix, p)
 	verify {
-		assert_eq!(storage::unhashed::get_raw(&last_key), None);
+		// Verify that they're not in the storage anymore.
+		for item in items {
+			assert!(storage::unhashed::get_raw(&item).is_none());
+		}
 	}
 
 	impl_benchmark_test_suite!(Pallet, crate::mock::new_test_ext(), crate::mock::Test);