Fix benchmarks and adds CI to test them (#12068)

* Fix benchmarks and adds CI to test them

Instead of waiting for benchmarks failing in Polkadot CI, we also can just test them in Substrate :P

* Do not overflow

client/allocator/src/freeing_bump.rs

@@ -331,7 +331,7 @@ pub struct AllocationStats {
 	/// The sum of every allocation ever made.
 	///
 	/// This increases every time a new allocation is made.
-	pub bytes_allocated_sum: u32,
+	pub bytes_allocated_sum: u128,
 
 	/// The amount of address space (in bytes) used by the allocator.
 	///
@@ -435,7 +435,7 @@ impl FreeingBumpHeapAllocator {
 		Header::Occupied(order).write_into(mem, header_ptr)?;
 
 		self.stats.bytes_allocated += order.size() + HEADER_SIZE;
-		self.stats.bytes_allocated_sum += order.size() + HEADER_SIZE;
+		self.stats.bytes_allocated_sum += u128::from(order.size() + HEADER_SIZE);
 		self.stats.bytes_allocated_peak =
 			std::cmp::max(self.stats.bytes_allocated_peak, self.stats.bytes_allocated);
 		self.stats.address_space_used = self.bumper - self.original_heap_base;

frame/democracy/src/benchmarking.rs

@@ -43,7 +43,9 @@ fn assert_last_event<T: Config>(generic_event: <T as Config>::Event) {
 
 fn funded_account<T: Config>(name: &'static str, index: u32) -> T::AccountId {
 	let caller: T::AccountId = account(name, index, SEED);
-	T::Currency::make_free_balance_be(&caller, BalanceOf::<T>::max_value());
+	// Give the account half of the maximum value of the `Balance` type.
+	// Otherwise some transfers will fail with an overflow error.
+	T::Currency::make_free_balance_be(&caller, BalanceOf::<T>::max_value() / 2u32.into());
 	caller
 }
 

frame/identity/src/benchmarking.rs

@@ -120,7 +120,8 @@ benchmarks! {
 		let r in 1 .. T::MaxRegistrars::get() - 1 => add_registrars::<T>(r)?;
 		ensure!(Registrars::<T>::get().len() as u32 == r, "Registrars not set up correctly.");
 		let origin = T::RegistrarOrigin::successful_origin();
-	}: _<T::Origin>(origin, account("registrar", r + 1, SEED))
+		let account = T::Lookup::unlookup(account("registrar", r + 1, SEED));
+	}: _<T::Origin>(origin, account)
 	verify {
 		ensure!(Registrars::<T>::get().len() as u32 == r + 1, "Registrars not added.");
 	}
@@ -287,7 +288,8 @@ benchmarks! {
 		Identity::<T>::add_registrar(registrar_origin, caller_lookup)?;
 		let registrars = Registrars::<T>::get();
 		ensure!(registrars[r as usize].as_ref().unwrap().account == caller, "id not set.");
-	}: _(RawOrigin::Signed(caller), r, account("new", 0, SEED))
+		let new_account = T::Lookup::unlookup(account("new", 0, SEED));
+	}: _(RawOrigin::Signed(caller), r, new_account)
 	verify {
 		let registrars = Registrars::<T>::get();
 		ensure!(registrars[r as usize].as_ref().unwrap().account == account("new", 0, SEED), "id not changed.");

frame/offences/benchmarking/src/lib.rs

@@ -305,7 +305,7 @@ benchmarks! {
 	verify {
 		let bond_amount: u32 = UniqueSaturatedInto::<u32>::unique_saturated_into(bond_amount::<T>());
 		let slash_amount = slash_fraction * bond_amount;
-		let reward_amount = slash_amount * (1 + n) / 2;
+		let reward_amount = slash_amount.saturating_mul(1 + n) / 2;
 		let reward = reward_amount / r;
 		let slash = |id| core::iter::once(
 			<T as StakingConfig>::Event::from(StakingEvent::<T>::Slashed(id, BalanceOf::<T>::from(slash_amount)))

frame/proxy/src/benchmarking.rs

@@ -35,9 +35,11 @@ fn add_proxies<T: Config>(n: u32, maybe_who: Option<T::AccountId>) -> Result<(),
 	let caller = maybe_who.unwrap_or_else(whitelisted_caller);
 	T::Currency::make_free_balance_be(&caller, BalanceOf::<T>::max_value() / 2u32.into());
 	for i in 0..n {
+		let real = T::Lookup::unlookup(account("target", i, SEED));
+
 		Proxy::<T>::add_proxy(
 			RawOrigin::Signed(caller.clone()).into(),
-			account("target", i, SEED),
+			real,
 			T::ProxyType::default(),
 			T::BlockNumber::zero(),
 		)?;
@@ -180,9 +182,10 @@ benchmarks! {
 	add_proxy {
 		let p in 1 .. (T::MaxProxies::get() - 1) => add_proxies::<T>(p, None)?;
 		let caller: T::AccountId = whitelisted_caller();
+		let real = T::Lookup::unlookup(account("target", T::MaxProxies::get(), SEED));
 	}: _(
 		RawOrigin::Signed(caller.clone()),
-		account("target", T::MaxProxies::get(), SEED),
+		real,
 		T::ProxyType::default(),
 		T::BlockNumber::zero()
 	)
@@ -194,9 +197,10 @@ benchmarks! {
 	remove_proxy {
 		let p in 1 .. (T::MaxProxies::get() - 1) => add_proxies::<T>(p, None)?;
 		let caller: T::AccountId = whitelisted_caller();
+		let delegate = T::Lookup::unlookup(account("target", 0, SEED));
 	}: _(
 		RawOrigin::Signed(caller.clone()),
-		account("target", 0, SEED),
+		delegate,
 		T::ProxyType::default(),
 		T::BlockNumber::zero()
 	)

scripts/ci/gitlab/pipeline/test.yml

@@ -277,6 +277,24 @@ test-linux-stable-extra:
     - time cargo test --doc --workspace --locked --release --verbose --features runtime-benchmarks --manifest-path ./bin/node/cli/Cargo.toml
     - rusty-cachier cache upload
 
+# This job runs all benchmarks defined in the `/bin/node/runtime` once to check that there are no errors.
+quick-benchmarks:
+  stage:                           test
+  extends:
+    - .docker-env
+    - .test-refs
+  variables:
+    # Enable debug assertions since we are running optimized builds for testing
+    # but still want to have debug assertions.
+    RUSTFLAGS:                     "-Cdebug-assertions=y -Dwarnings"
+    RUST_BACKTRACE:                "full"
+    WASM_BUILD_NO_COLOR:           1
+    WASM_BUILD_RUSTFLAGS:          "-Cdebug-assertions=y -Dwarnings"
+  script:
+    - rusty-cachier snapshot create
+    - time cargo run --release --features runtime-benchmarks -- benchmark pallet --execution wasm --wasm-execution compiled --chain dev --pallet "*" --extrinsic "*" --steps 2 --repeat 1
+    - rusty-cachier cache upload
+
 test-frame-examples-compile-to-wasm:
   # into one job
   stage:                           test

utils/frame/benchmarking-cli/src/pallet/command.rs

@@ -228,6 +228,11 @@ impl PalletCmd {
 		let mut component_ranges = HashMap::<(Vec<u8>, Vec<u8>), Vec<ComponentRange>>::new();
 
 		for (pallet, extrinsic, components) in benchmarks_to_run {
+			log::info!(
+				"Starting benchmark: {}::{}",
+				String::from_utf8(pallet.clone()).expect("Encoded from String; qed"),
+				String::from_utf8(extrinsic.clone()).expect("Encoded from String; qed"),
+			);
 			let all_components = if components.is_empty() {
 				vec![Default::default()]
 			} else {