From 1819d8978839b56ecdcc6bccecf776303087c850 Mon Sep 17 00:00:00 2001 From: Xavier Lau Date: Tue, 29 Oct 2024 21:21:48 +0800 Subject: [PATCH] Migrate pallet-vesting benchmark to v2 (#6254) Part of: - #6202. --- substrate/frame/vesting/src/benchmarking.rs | 348 ++++++++++-------- substrate/primitives/panic-handler/src/lib.rs | 4 +- 2 files changed, 192 insertions(+), 160 deletions(-) diff --git a/substrate/frame/vesting/src/benchmarking.rs b/substrate/frame/vesting/src/benchmarking.rs index 736dd6eac1a8c..503655243fb44 100644 --- a/substrate/frame/vesting/src/benchmarking.rs +++ b/substrate/frame/vesting/src/benchmarking.rs @@ -19,13 +19,12 @@ #![cfg(feature = "runtime-benchmarks")] -use frame_benchmarking::v1::{account, benchmarks, whitelisted_caller}; +use frame_benchmarking::{v2::*, BenchmarkError}; use frame_support::assert_ok; -use frame_system::{pallet_prelude::BlockNumberFor, Pallet as System, RawOrigin}; +use frame_system::{pallet_prelude::BlockNumberFor, RawOrigin}; use sp_runtime::traits::{Bounded, CheckedDiv, CheckedMul}; -use super::{Vesting as VestingStorage, *}; -use crate::Pallet as Vesting; +use crate::*; const SEED: u32 = 0; @@ -35,7 +34,7 @@ type BalanceOf = fn add_locks(who: &T::AccountId, n: u8) { for id in 0..n { let lock_id = [id; 8]; - let locked = 256u32; + let locked = 256_u32; let reasons = WithdrawReasons::TRANSFER | WithdrawReasons::RESERVE; T::Currency::set_lock(lock_id, who, locked.into(), reasons); } @@ -46,12 +45,12 @@ fn add_vesting_schedules( n: u32, ) -> Result, &'static str> { let min_transfer = T::MinVestedTransfer::get(); - let locked = min_transfer.checked_mul(&20u32.into()).unwrap(); + let locked = min_transfer.checked_mul(&20_u32.into()).unwrap(); // Schedule has a duration of 20. let per_block = min_transfer; - let starting_block = 1u32; + let starting_block = 1_u32; - let source: T::AccountId = account("source", 0, SEED); + let source = account("source", 0, SEED); T::Currency::make_free_balance_be(&source, BalanceOf::::max_value()); T::BlockNumberProvider::set_block_number(BlockNumberFor::::zero()); @@ -61,7 +60,7 @@ fn add_vesting_schedules( total_locked += locked; let schedule = VestingInfo::new(locked, per_block, starting_block.into()); - assert_ok!(Vesting::::do_vested_transfer(&source, target, schedule)); + assert_ok!(Pallet::::do_vested_transfer(&source, target, schedule)); // Top up to guarantee we can always transfer another schedule. T::Currency::make_free_balance_be(&source, BalanceOf::::max_value()); @@ -70,66 +69,76 @@ fn add_vesting_schedules( Ok(total_locked) } -benchmarks! { - vest_locked { - let l in 0 .. MaxLocksOf::::get() - 1; - let s in 1 .. T::MAX_VESTING_SCHEDULES; +#[benchmarks] +mod benchmarks { + use super::*; - let caller: T::AccountId = whitelisted_caller(); + #[benchmark] + fn vest_locked( + l: Linear<0, { MaxLocksOf::::get() - 1 }>, + s: Linear<1, T::MAX_VESTING_SCHEDULES>, + ) -> Result<(), BenchmarkError> { + let caller = whitelisted_caller(); T::Currency::make_free_balance_be(&caller, T::Currency::minimum_balance()); add_locks::(&caller, l as u8); let expected_balance = add_vesting_schedules::(&caller, s)?; // At block zero, everything is vested. - assert_eq!(System::::block_number(), BlockNumberFor::::zero()); + assert_eq!(frame_system::Pallet::::block_number(), BlockNumberFor::::zero()); assert_eq!( - Vesting::::vesting_balance(&caller), + Pallet::::vesting_balance(&caller), Some(expected_balance), "Vesting schedule not added", ); - }: vest(RawOrigin::Signed(caller.clone())) - verify { + + #[extrinsic_call] + vest(RawOrigin::Signed(caller.clone())); + // Nothing happened since everything is still vested. assert_eq!( - Vesting::::vesting_balance(&caller), + Pallet::::vesting_balance(&caller), Some(expected_balance), "Vesting schedule was removed", ); - } - vest_unlocked { - let l in 0 .. MaxLocksOf::::get() - 1; - let s in 1 .. T::MAX_VESTING_SCHEDULES; + Ok(()) + } - let caller: T::AccountId = whitelisted_caller(); + #[benchmark] + fn vest_unlocked( + l: Linear<0, { MaxLocksOf::::get() - 1 }>, + s: Linear<1, T::MAX_VESTING_SCHEDULES>, + ) -> Result<(), BenchmarkError> { + let caller = whitelisted_caller(); T::Currency::make_free_balance_be(&caller, T::Currency::minimum_balance()); add_locks::(&caller, l as u8); add_vesting_schedules::(&caller, s)?; // At block 21, everything is unlocked. - T::BlockNumberProvider::set_block_number(21u32.into()); + T::BlockNumberProvider::set_block_number(21_u32.into()); assert_eq!( - Vesting::::vesting_balance(&caller), + Pallet::::vesting_balance(&caller), Some(BalanceOf::::zero()), "Vesting schedule still active", ); - }: vest(RawOrigin::Signed(caller.clone())) - verify { + + #[extrinsic_call] + vest(RawOrigin::Signed(caller.clone())); + // Vesting schedule is removed! - assert_eq!( - Vesting::::vesting_balance(&caller), - None, - "Vesting schedule was not removed", - ); - } + assert_eq!(Pallet::::vesting_balance(&caller), None, "Vesting schedule was not removed",); - vest_other_locked { - let l in 0 .. MaxLocksOf::::get() - 1; - let s in 1 .. T::MAX_VESTING_SCHEDULES; + Ok(()) + } - let other: T::AccountId = account("other", 0, SEED); + #[benchmark] + fn vest_other_locked( + l: Linear<0, { MaxLocksOf::::get() - 1 }>, + s: Linear<1, T::MAX_VESTING_SCHEDULES>, + ) -> Result<(), BenchmarkError> { + let other = account::("other", 0, SEED); let other_lookup = T::Lookup::unlookup(other.clone()); T::Currency::make_free_balance_be(&other, T::Currency::minimum_balance()); @@ -137,64 +146,70 @@ benchmarks! { let expected_balance = add_vesting_schedules::(&other, s)?; // At block zero, everything is vested. - assert_eq!(System::::block_number(), BlockNumberFor::::zero()); + assert_eq!(frame_system::Pallet::::block_number(), BlockNumberFor::::zero()); assert_eq!( - Vesting::::vesting_balance(&other), + Pallet::::vesting_balance(&other), Some(expected_balance), "Vesting schedule not added", ); - let caller: T::AccountId = whitelisted_caller(); - }: vest_other(RawOrigin::Signed(caller.clone()), other_lookup) - verify { + let caller = whitelisted_caller::(); + + #[extrinsic_call] + vest_other(RawOrigin::Signed(caller.clone()), other_lookup); + // Nothing happened since everything is still vested. assert_eq!( - Vesting::::vesting_balance(&other), + Pallet::::vesting_balance(&other), Some(expected_balance), "Vesting schedule was removed", ); - } - vest_other_unlocked { - let l in 0 .. MaxLocksOf::::get() - 1; - let s in 1 .. T::MAX_VESTING_SCHEDULES; + Ok(()) + } - let other: T::AccountId = account("other", 0, SEED); + #[benchmark] + fn vest_other_unlocked( + l: Linear<0, { MaxLocksOf::::get() - 1 }>, + s: Linear<1, { T::MAX_VESTING_SCHEDULES }>, + ) -> Result<(), BenchmarkError> { + let other = account::("other", 0, SEED); let other_lookup = T::Lookup::unlookup(other.clone()); T::Currency::make_free_balance_be(&other, T::Currency::minimum_balance()); add_locks::(&other, l as u8); add_vesting_schedules::(&other, s)?; // At block 21 everything is unlocked. - T::BlockNumberProvider::set_block_number(21u32.into()); + T::BlockNumberProvider::set_block_number(21_u32.into()); assert_eq!( - Vesting::::vesting_balance(&other), + Pallet::::vesting_balance(&other), Some(BalanceOf::::zero()), "Vesting schedule still active", ); - let caller: T::AccountId = whitelisted_caller(); - }: vest_other(RawOrigin::Signed(caller.clone()), other_lookup) - verify { + let caller = whitelisted_caller::(); + + #[extrinsic_call] + vest_other(RawOrigin::Signed(caller.clone()), other_lookup); + // Vesting schedule is removed. - assert_eq!( - Vesting::::vesting_balance(&other), - None, - "Vesting schedule was not removed", - ); - } + assert_eq!(Pallet::::vesting_balance(&other), None, "Vesting schedule was not removed",); - vested_transfer { - let l in 0 .. MaxLocksOf::::get() - 1; - let s in 0 .. T::MAX_VESTING_SCHEDULES - 1; + Ok(()) + } - let caller: T::AccountId = whitelisted_caller(); + #[benchmark] + fn vested_transfer( + l: Linear<0, { MaxLocksOf::::get() - 1 }>, + s: Linear<0, { T::MAX_VESTING_SCHEDULES - 1 }>, + ) -> Result<(), BenchmarkError> { + let caller = whitelisted_caller(); T::Currency::make_free_balance_be(&caller, BalanceOf::::max_value()); - let target: T::AccountId = account("target", 0, SEED); + let target = account::("target", 0, SEED); let target_lookup = T::Lookup::unlookup(target.clone()); - // Give target existing locks + // Give target existing locks. T::Currency::make_free_balance_be(&target, T::Currency::minimum_balance()); add_locks::(&target, l as u8); // Add one vesting schedules. @@ -202,74 +217,75 @@ benchmarks! { let mut expected_balance = add_vesting_schedules::(&target, s)?; let transfer_amount = T::MinVestedTransfer::get(); - let per_block = transfer_amount.checked_div(&20u32.into()).unwrap(); + let per_block = transfer_amount.checked_div(&20_u32.into()).unwrap(); expected_balance += transfer_amount; - let vesting_schedule = VestingInfo::new( - transfer_amount, - per_block, - 1u32.into(), - ); - }: _(RawOrigin::Signed(caller), target_lookup, vesting_schedule) - verify { + let vesting_schedule = VestingInfo::new(transfer_amount, per_block, 1_u32.into()); + + #[extrinsic_call] + _(RawOrigin::Signed(caller.clone()), target_lookup, vesting_schedule); + assert_eq!( orig_balance + expected_balance, T::Currency::free_balance(&target), "Transfer didn't happen", ); assert_eq!( - Vesting::::vesting_balance(&target), + Pallet::::vesting_balance(&target), Some(expected_balance), "Lock not correctly updated", ); - } - force_vested_transfer { - let l in 0 .. MaxLocksOf::::get() - 1; - let s in 0 .. T::MAX_VESTING_SCHEDULES - 1; + Ok(()) + } - let source: T::AccountId = account("source", 0, SEED); + #[benchmark] + fn force_vested_transfer( + l: Linear<0, { MaxLocksOf::::get() - 1 }>, + s: Linear<0, { T::MAX_VESTING_SCHEDULES - 1 }>, + ) -> Result<(), BenchmarkError> { + let source = account::("source", 0, SEED); let source_lookup = T::Lookup::unlookup(source.clone()); T::Currency::make_free_balance_be(&source, BalanceOf::::max_value()); - let target: T::AccountId = account("target", 0, SEED); + let target = account::("target", 0, SEED); let target_lookup = T::Lookup::unlookup(target.clone()); - // Give target existing locks + // Give target existing locks. T::Currency::make_free_balance_be(&target, T::Currency::minimum_balance()); add_locks::(&target, l as u8); - // Add one less than max vesting schedules + // Add one less than max vesting schedules. let orig_balance = T::Currency::free_balance(&target); let mut expected_balance = add_vesting_schedules::(&target, s)?; let transfer_amount = T::MinVestedTransfer::get(); - let per_block = transfer_amount.checked_div(&20u32.into()).unwrap(); + let per_block = transfer_amount.checked_div(&20_u32.into()).unwrap(); expected_balance += transfer_amount; - let vesting_schedule = VestingInfo::new( - transfer_amount, - per_block, - 1u32.into(), - ); - }: _(RawOrigin::Root, source_lookup, target_lookup, vesting_schedule) - verify { + let vesting_schedule = VestingInfo::new(transfer_amount, per_block, 1_u32.into()); + + #[extrinsic_call] + _(RawOrigin::Root, source_lookup, target_lookup, vesting_schedule); + assert_eq!( orig_balance + expected_balance, T::Currency::free_balance(&target), "Transfer didn't happen", ); assert_eq!( - Vesting::::vesting_balance(&target), + Pallet::::vesting_balance(&target), Some(expected_balance), - "Lock not correctly updated", - ); - } + "Lock not correctly updated", + ); - not_unlocking_merge_schedules { - let l in 0 .. MaxLocksOf::::get() - 1; - let s in 2 .. T::MAX_VESTING_SCHEDULES; + Ok(()) + } - let caller: T::AccountId = account("caller", 0, SEED); - let caller_lookup = T::Lookup::unlookup(caller.clone()); + #[benchmark] + fn not_unlocking_merge_schedules( + l: Linear<0, { MaxLocksOf::::get() - 1 }>, + s: Linear<2, { T::MAX_VESTING_SCHEDULES }>, + ) -> Result<(), BenchmarkError> { + let caller = whitelisted_caller::(); // Give target existing locks. T::Currency::make_free_balance_be(&caller, T::Currency::minimum_balance()); add_locks::(&caller, l as u8); @@ -277,115 +293,127 @@ benchmarks! { let expected_balance = add_vesting_schedules::(&caller, s)?; // Schedules are not vesting at block 0. - assert_eq!(System::::block_number(), BlockNumberFor::::zero()); + assert_eq!(frame_system::Pallet::::block_number(), BlockNumberFor::::zero()); assert_eq!( - Vesting::::vesting_balance(&caller), + Pallet::::vesting_balance(&caller), Some(expected_balance), "Vesting balance should equal sum locked of all schedules", ); assert_eq!( - VestingStorage::::get(&caller).unwrap().len(), + Vesting::::get(&caller).unwrap().len(), s as usize, "There should be exactly max vesting schedules" ); - }: merge_schedules(RawOrigin::Signed(caller.clone()), 0, s - 1) - verify { + + #[extrinsic_call] + merge_schedules(RawOrigin::Signed(caller.clone()), 0, s - 1); + let expected_schedule = VestingInfo::new( - T::MinVestedTransfer::get() * 20u32.into() * 2u32.into(), - T::MinVestedTransfer::get() * 2u32.into(), - 1u32.into(), + T::MinVestedTransfer::get() * 20_u32.into() * 2_u32.into(), + T::MinVestedTransfer::get() * 2_u32.into(), + 1_u32.into(), ); let expected_index = (s - 2) as usize; + assert_eq!(Vesting::::get(&caller).unwrap()[expected_index], expected_schedule); assert_eq!( - VestingStorage::::get(&caller).unwrap()[expected_index], - expected_schedule - ); - assert_eq!( - Vesting::::vesting_balance(&caller), + Pallet::::vesting_balance(&caller), Some(expected_balance), "Vesting balance should equal total locked of all schedules", ); assert_eq!( - VestingStorage::::get(&caller).unwrap().len(), + Vesting::::get(&caller).unwrap().len(), (s - 1) as usize, "Schedule count should reduce by 1" ); - } - unlocking_merge_schedules { - let l in 0 .. MaxLocksOf::::get() - 1; - let s in 2 .. T::MAX_VESTING_SCHEDULES; + Ok(()) + } + #[benchmark] + fn unlocking_merge_schedules( + l: Linear<0, { MaxLocksOf::::get() - 1 }>, + s: Linear<2, { T::MAX_VESTING_SCHEDULES }>, + ) -> Result<(), BenchmarkError> { // Destination used just for currency transfers in asserts. let test_dest: T::AccountId = account("test_dest", 0, SEED); - let caller: T::AccountId = account("caller", 0, SEED); - let caller_lookup = T::Lookup::unlookup(caller.clone()); - // Give target other locks. + let caller = whitelisted_caller::(); + // Give target existing locks. T::Currency::make_free_balance_be(&caller, T::Currency::minimum_balance()); add_locks::(&caller, l as u8); // Add max vesting schedules. let total_transferred = add_vesting_schedules::(&caller, s)?; - // Go to about half way through all the schedules duration. (They all start at 1, and have a duration of 20 or 21). - T::BlockNumberProvider::set_block_number(11u32.into()); - // We expect half the original locked balance (+ any remainder that vests on the last block). - let expected_balance = total_transferred / 2u32.into(); + // Go to about half way through all the schedules duration. (They all start at 1, and have a + // duration of 20 or 21). + T::BlockNumberProvider::set_block_number(11_u32.into()); + // We expect half the original locked balance (+ any remainder that vests on the last + // block). + let expected_balance = total_transferred / 2_u32.into(); assert_eq!( - Vesting::::vesting_balance(&caller), + Pallet::::vesting_balance(&caller), Some(expected_balance), "Vesting balance should reflect that we are half way through all schedules duration", ); assert_eq!( - VestingStorage::::get(&caller).unwrap().len(), + Vesting::::get(&caller).unwrap().len(), s as usize, "There should be exactly max vesting schedules" ); // The balance is not actually transferable because it has not been unlocked. - assert!(T::Currency::transfer(&caller, &test_dest, expected_balance, ExistenceRequirement::AllowDeath).is_err()); - }: merge_schedules(RawOrigin::Signed(caller.clone()), 0, s - 1) - verify { + assert!(T::Currency::transfer( + &caller, + &test_dest, + expected_balance, + ExistenceRequirement::AllowDeath + ) + .is_err()); + + #[extrinsic_call] + merge_schedules(RawOrigin::Signed(caller.clone()), 0, s - 1); + let expected_schedule = VestingInfo::new( - T::MinVestedTransfer::get() * 2u32.into() * 10u32.into(), - T::MinVestedTransfer::get() * 2u32.into(), - 11u32.into(), + T::MinVestedTransfer::get() * 2_u32.into() * 10_u32.into(), + T::MinVestedTransfer::get() * 2_u32.into(), + 11_u32.into(), ); let expected_index = (s - 2) as usize; assert_eq!( - VestingStorage::::get(&caller).unwrap()[expected_index], + Vesting::::get(&caller).unwrap()[expected_index], expected_schedule, "New schedule is properly created and placed" ); assert_eq!( - VestingStorage::::get(&caller).unwrap()[expected_index], - expected_schedule - ); - assert_eq!( - Vesting::::vesting_balance(&caller), + Pallet::::vesting_balance(&caller), Some(expected_balance), "Vesting balance should equal half total locked of all schedules", ); assert_eq!( - VestingStorage::::get(&caller).unwrap().len(), + Vesting::::get(&caller).unwrap().len(), (s - 1) as usize, "Schedule count should reduce by 1" ); // Since merge unlocks all schedules we can now transfer the balance. - assert_ok!( - T::Currency::transfer(&caller, &test_dest, expected_balance, ExistenceRequirement::AllowDeath) - ); + assert_ok!(T::Currency::transfer( + &caller, + &test_dest, + expected_balance, + ExistenceRequirement::AllowDeath + )); + + Ok(()) } -force_remove_vesting_schedule { - let l in 0 .. MaxLocksOf::::get() - 1; - let s in 2 .. T::MAX_VESTING_SCHEDULES; - - let source: T::AccountId = account("source", 0, SEED); - let source_lookup: ::Source = T::Lookup::unlookup(source.clone()); + #[benchmark] + fn force_remove_vesting_schedule( + l: Linear<0, { MaxLocksOf::::get() - 1 }>, + s: Linear<2, { T::MAX_VESTING_SCHEDULES }>, + ) -> Result<(), BenchmarkError> { + let source = account::("source", 0, SEED); T::Currency::make_free_balance_be(&source, BalanceOf::::max_value()); - let target: T::AccountId = account("target", 0, SEED); - let target_lookup: ::Source = T::Lookup::unlookup(target.clone()); + let target = account::("target", 0, SEED); + let target_lookup = T::Lookup::unlookup(target.clone()); T::Currency::make_free_balance_be(&target, T::Currency::minimum_balance()); // Give target existing locks. @@ -394,18 +422,22 @@ force_remove_vesting_schedule { // The last vesting schedule. let schedule_index = s - 1; - }: _(RawOrigin::Root, target_lookup, schedule_index) - verify { + + #[extrinsic_call] + _(RawOrigin::Root, target_lookup, schedule_index); + assert_eq!( - VestingStorage::::get(&target).unwrap().len(), + Vesting::::get(&target).unwrap().len(), schedule_index as usize, "Schedule count should reduce by 1" ); + + Ok(()) } - impl_benchmark_test_suite!( - Vesting, + impl_benchmark_test_suite! { + Pallet, crate::mock::ExtBuilder::default().existential_deposit(256).build(), - crate::mock::Test, - ); + crate::mock::Test + } } diff --git a/substrate/primitives/panic-handler/src/lib.rs b/substrate/primitives/panic-handler/src/lib.rs index c4a7eb8dc67c7..81ccaaee828e4 100644 --- a/substrate/primitives/panic-handler/src/lib.rs +++ b/substrate/primitives/panic-handler/src/lib.rs @@ -30,7 +30,7 @@ use std::{ cell::Cell, io::{self, Write}, marker::PhantomData, - panic::{self, PanicInfo}, + panic::{self, PanicHookInfo}, sync::LazyLock, thread, }; @@ -149,7 +149,7 @@ fn strip_control_codes(input: &str) -> std::borrow::Cow { } /// Function being called when a panic happens. -fn panic_hook(info: &PanicInfo, report_url: &str, version: &str) { +fn panic_hook(info: &PanicHookInfo, report_url: &str, version: &str) { let location = info.location(); let file = location.as_ref().map(|l| l.file()).unwrap_or(""); let line = location.as_ref().map(|l| l.line()).unwrap_or(0);