Fix vesting benchmark (#5269)

* Fix vesting bench starting block.

* Set subs s+1 to hit worst path.

* Remove unused origin.

* Apply review suggestions.

substrate/frame/identity/src/benchmarking.rs

@@ -53,7 +53,7 @@ fn add_registrars<T: Trait>(r: u32) -> Result<(), &'static str> {
 
 // Adds `s` sub-accounts to the identity of `who`. Each wil have 32 bytes of raw data added to it.
 // This additionally returns the vector of sub-accounts to it can be modified if needed.
-fn add_sub_accounts<T: Trait>(who: T::AccountId, s: u32) -> Result<Vec<(T::AccountId, Data)>, &'static str> {
+fn add_sub_accounts<T: Trait>(who: &T::AccountId, s: u32) -> Result<Vec<(T::AccountId, Data)>, &'static str> {
 	let mut subs = Vec::new();
 	let who_origin = RawOrigin::Signed(who.clone());
 	let data = Data::Raw(vec![0; 32]);
@@ -100,7 +100,7 @@ benchmarks! {
 		let s in 1 .. T::MaxSubAccounts::get() => {
 			// Give them s many sub accounts
 			let caller = account::<T>("caller", 0);
-			let _ = add_sub_accounts::<T>(caller, s)?;
+			let _ = add_sub_accounts::<T>(&caller, s)?;
 		};
 		let x in 1 .. T::MaxAdditionalFields::get() => {
 			// Create their main identity with x additional fields
@@ -149,11 +149,19 @@ benchmarks! {
 	)
 
 	set_subs {
-		let s in ...;
-
 		let caller = account::<T>("caller", 0);
-		let caller_origin: <T as frame_system::Trait>::Origin = RawOrigin::Signed(caller.clone()).into();
+
-		let subs = Module::<T>::subs(&caller);
+		// Give them s many sub accounts.
+		let s in 1 .. T::MaxSubAccounts::get() - 1 => {
+			let _ = add_sub_accounts::<T>(&caller, s)?;
+		};
+
+		let mut subs = Module::<T>::subs(&caller);
+
+		// Create an s + 1 sub account.
+		let data = Data::Raw(vec![0; 32]);
+		subs.push((account::<T>("sub", s + 1), data));
+
 	}: _(RawOrigin::Signed(caller), subs)
 
 	clear_identity {

substrate/frame/vesting/src/benchmarking.rs

@@ -44,6 +44,7 @@ fn setup<T: Trait>(b: u32) -> T::AccountId {
 		let starting_block = 0;
 
 		let caller = account("caller", 0, SEED);
+		System::<T>::set_block_number(0.into());
 
 		// Add schedule to avoid `NotVesting` error.
 		let _ = Vesting::<T>::add_vesting_schedule(
@@ -63,32 +64,44 @@ fn setup<T: Trait>(b: u32) -> T::AccountId {
 
 benchmarks! {
 	_ {
-		// Current block. It allows to hit different paths of `update_lock`.
-		// It doesn't seems to influence the timings which branch is taken.
-		let b in 0 .. 1 => ();
 		// Number of previous locks.
 		// It doesn't seems to influence the timings for lower values.
 		let l in 0 .. MAX_LOCKS => add_locks::<T>(l);
 	}
 
-	vest {
+	vest_locked {
-		let b in ...;
 		let l in ...;
 
-		let caller = setup::<T>(b);
+		let caller = setup::<T>(0u32);
 
-	}: _(RawOrigin::Signed(caller))
+	}: vest(RawOrigin::Signed(caller))
 
-	vest_other {
+	vest_not_locked {
-		let b in ...;
 		let l in ...;
 
-		let other: T::AccountId = setup::<T>(b);
+		let caller = setup::<T>(1u32);
+
+	}: vest(RawOrigin::Signed(caller))
+
+	vest_other_locked {
+		let l in ...;
+
+		let other: T::AccountId = setup::<T>(0u32);
 		let other_lookup: <T::Lookup as StaticLookup>::Source = T::Lookup::unlookup(other.clone());
 
 		let caller = account("caller", 0, SEED);
 
-	}: _(RawOrigin::Signed(caller), other_lookup)
+	}: vest_other(RawOrigin::Signed(caller), other_lookup)
+
+	vest_other_not_locked {
+		let l in ...;
+
+		let other: T::AccountId = setup::<T>(1u32);
+		let other_lookup: <T::Lookup as StaticLookup>::Source = T::Lookup::unlookup(other.clone());
+
+		let caller = account("caller", 0, SEED);
+
+	}: vest_other(RawOrigin::Signed(caller), other_lookup)
 
 	vested_transfer {
 		let u in 0 .. 1000;