Merge branch 'master' into oty-fix-bencher

Author: ggwpez

bridges/modules/relayers/src/benchmarking.rs

@@ -20,9 +20,7 @@
 
 use crate::*;
 
-use frame_benchmarking::{
-	benchmarks_instance_pallet, whitelisted_caller, BenchmarkError, BenchmarkResult,
-};
+use frame_benchmarking::v2::*;
 use frame_support::{assert_ok, weights::Weight};
 use frame_system::RawOrigin;
 use sp_runtime::traits::One;
@@ -52,121 +50,149 @@ fn assert_last_event<T: Config<I>, I: 'static>(
 	frame_system::Pallet::<T>::assert_last_event(generic_event.into());
 }
 
-benchmarks_instance_pallet! {
-	where_clause { where
+#[instance_benchmarks(
+	where
 		BeneficiaryOf<T, I>: From<<T as frame_system::Config>::AccountId>,
-	}
+)]
+mod benchmarks {
+	use super::*;
 
-	// Benchmark `claim_rewards` call.
-	claim_rewards {
-		let reward_kind = T::bench_reward();
+	#[benchmark]
+	fn claim_rewards() {
 		let relayer: T::AccountId = whitelisted_caller();
+		let reward_kind = T::bench_reward();
 		let reward_balance = T::RewardBalance::from(REWARD_AMOUNT);
-
 		let _ = T::prepare_rewards_account(reward_kind, reward_balance);
 		RelayerRewards::<T, I>::insert(&relayer, reward_kind, reward_balance);
-	}: _(RawOrigin::Signed(relayer.clone()), reward_kind)
-	verify {
+
+		#[extrinsic_call]
+		_(RawOrigin::Signed(relayer.clone()), reward_kind);
+
 		// we can't check anything here, because `PaymentProcedure` is responsible for
 		// payment logic, so we assume that if call has succeeded, the procedure has
 		// also completed successfully
-		assert_last_event::<T, I>(Event::RewardPaid {
-			relayer: relayer.clone(),
-			reward_kind,
-			reward_balance,
-			beneficiary: relayer.into(),
-		}.into());
+		assert_last_event::<T, I>(
+			Event::RewardPaid {
+				relayer: relayer.clone(),
+				reward_kind,
+				reward_balance,
+				beneficiary: relayer.into(),
+			}
+			.into(),
+		);
 	}
 
-	// Benchmark `claim_rewards_to` call.
-	claim_rewards_to {
-		let reward_kind = T::bench_reward();
+	#[benchmark]
+	fn claim_rewards_to() -> Result<(), BenchmarkError> {
 		let relayer: T::AccountId = whitelisted_caller();
+		let reward_kind = T::bench_reward();
 		let reward_balance = T::RewardBalance::from(REWARD_AMOUNT);
 
-		let Some(alternative_beneficiary) = T::prepare_rewards_account(reward_kind, reward_balance) else {
+		let Some(alternative_beneficiary) = T::prepare_rewards_account(reward_kind, reward_balance)
+		else {
 			return Err(BenchmarkError::Override(BenchmarkResult::from_weight(Weight::MAX)));
 		};
 		RelayerRewards::<T, I>::insert(&relayer, reward_kind, reward_balance);
-	}: _(RawOrigin::Signed(relayer.clone()), reward_kind, alternative_beneficiary.clone())
-	verify {
+
+		#[extrinsic_call]
+		_(RawOrigin::Signed(relayer.clone()), reward_kind, alternative_beneficiary.clone());
+
 		// we can't check anything here, because `PaymentProcedure` is responsible for
 		// payment logic, so we assume that if call has succeeded, the procedure has
 		// also completed successfully
-		assert_last_event::<T, I>(Event::RewardPaid {
-			relayer,
-			reward_kind,
-			reward_balance,
-			beneficiary: alternative_beneficiary,
-		}.into());
+		assert_last_event::<T, I>(
+			Event::RewardPaid {
+				relayer: relayer.clone(),
+				reward_kind,
+				reward_balance,
+				beneficiary: alternative_beneficiary,
+			}
+			.into(),
+		);
+
+		Ok(())
 	}
 
-	// Benchmark `register` call.
-	register {
+	#[benchmark]
+	fn register() {
 		let relayer: T::AccountId = whitelisted_caller();
 		let valid_till = frame_system::Pallet::<T>::block_number()
 			.saturating_add(crate::Pallet::<T, I>::required_registration_lease())
 			.saturating_add(One::one())
 			.saturating_add(One::one());
-
 		T::deposit_account(relayer.clone(), crate::Pallet::<T, I>::required_stake());
-	}: _(RawOrigin::Signed(relayer.clone()), valid_till)
-	verify {
+
+		#[extrinsic_call]
+		_(RawOrigin::Signed(relayer.clone()), valid_till);
+
 		assert!(crate::Pallet::<T, I>::is_registration_active(&relayer));
 	}
 
-	// Benchmark `deregister` call.
-	deregister {
+	#[benchmark]
+	fn deregister() {
 		let relayer: T::AccountId = whitelisted_caller();
 		let valid_till = frame_system::Pallet::<T>::block_number()
 			.saturating_add(crate::Pallet::<T, I>::required_registration_lease())
 			.saturating_add(One::one())
 			.saturating_add(One::one());
 		T::deposit_account(relayer.clone(), crate::Pallet::<T, I>::required_stake());
-		crate::Pallet::<T, I>::register(RawOrigin::Signed(relayer.clone()).into(), valid_till).unwrap();
-
+		crate::Pallet::<T, I>::register(RawOrigin::Signed(relayer.clone()).into(), valid_till)
+			.unwrap();
 		frame_system::Pallet::<T>::set_block_number(valid_till.saturating_add(One::one()));
-	}: _(RawOrigin::Signed(relayer.clone()))
-	verify {
+
+		#[extrinsic_call]
+		_(RawOrigin::Signed(relayer.clone()));
+
 		assert!(!crate::Pallet::<T, I>::is_registration_active(&relayer));
 	}
 
 	// Benchmark `slash_and_deregister` method of the pallet. We are adding this weight to
 	// the weight of message delivery call if `BridgeRelayersTransactionExtension` signed extension
 	// is deployed at runtime level.
-	slash_and_deregister {
+	#[benchmark]
+	fn slash_and_deregister() {
 		// prepare and register relayer account
 		let relayer: T::AccountId = whitelisted_caller();
 		let valid_till = frame_system::Pallet::<T>::block_number()
 			.saturating_add(crate::Pallet::<T, I>::required_registration_lease())
 			.saturating_add(One::one())
 			.saturating_add(One::one());
 		T::deposit_account(relayer.clone(), crate::Pallet::<T, I>::required_stake());
-		assert_ok!(crate::Pallet::<T, I>::register(RawOrigin::Signed(relayer.clone()).into(), valid_till));
+		assert_ok!(crate::Pallet::<T, I>::register(
+			RawOrigin::Signed(relayer.clone()).into(),
+			valid_till
+		));
 
 		// create slash destination account
 		let slash_destination: T::AccountId = whitelisted_caller();
 		T::deposit_account(slash_destination.clone(), Zero::zero());
-	}: {
-		crate::Pallet::<T, I>::slash_and_deregister(&relayer, bp_relayers::ExplicitOrAccountParams::Explicit::<_, ()>(slash_destination))
-	}
-	verify {
+
+		#[block]
+		{
+			crate::Pallet::<T, I>::slash_and_deregister(
+				&relayer,
+				bp_relayers::ExplicitOrAccountParams::Explicit::<_, ()>(slash_destination),
+			);
+		}
+
 		assert!(!crate::Pallet::<T, I>::is_registration_active(&relayer));
 	}
 
 	// Benchmark `register_relayer_reward` method of the pallet. We are adding this weight to
 	// the weight of message delivery call if `BridgeRelayersTransactionExtension` signed extension
 	// is deployed at runtime level.
-	register_relayer_reward {
+	#[benchmark]
+	fn register_relayer_reward() {
 		let reward_kind = T::bench_reward();
 		let relayer: T::AccountId = whitelisted_caller();
 
-	}: {
-		crate::Pallet::<T, I>::register_relayer_reward(reward_kind, &relayer, One::one());
-	}
-	verify {
+		#[block]
+		{
+			crate::Pallet::<T, I>::register_relayer_reward(reward_kind, &relayer, One::one());
+		}
+
 		assert_eq!(RelayerRewards::<T, I>::get(relayer, &reward_kind), Some(One::one()));
 	}
 
-	impl_benchmark_test_suite!(Pallet, crate::mock::new_test_ext(), crate::mock::TestRuntime)
+	impl_benchmark_test_suite!(Pallet, crate::mock::new_test_ext(), crate::mock::TestRuntime);
 }

bridges/modules/relayers/src/lib.rs

@@ -18,7 +18,6 @@
 //! coordinate relations between relayers.
 
 #![cfg_attr(not(feature = "std"), no_std)]
-#![warn(missing_docs)]
 
 extern crate alloc;
 
@@ -367,6 +366,11 @@ pub mod pallet {
 					);
 				},
 			}
+
+			Self::deposit_event(Event::<T, I>::SlashedAndDeregistered {
+				relayer: relayer.clone(),
+				registration,
+			});
 		}
 
 		/// Register reward for given relayer.
@@ -553,6 +557,8 @@ mod tests {
 	use super::*;
 	use mock::{RuntimeEvent as TestEvent, *};
 
+	use bp_messages::{HashedLaneId, LaneIdType};
+	use bp_relayers::{RewardsAccountOwner, RewardsAccountParams};
 	use frame_support::{assert_noop, assert_ok, traits::fungible::Mutate};
 	use frame_system::{EventRecord, Pallet as System, Phase};
 	use sp_runtime::DispatchError;
@@ -589,6 +595,43 @@ mod tests {
 		});
 	}
 
+	#[test]
+	fn slash_and_deregister_works() {
+		run_test(|| {
+			get_ready_for_events();
+
+			// register
+			assert_ok!(Pallet::<TestRuntime>::register(
+				RuntimeOrigin::signed(REGISTER_RELAYER),
+				150,
+			));
+			// check if registered
+			let registration = Pallet::<TestRuntime>::registered_relayer(REGISTER_RELAYER).unwrap();
+			assert_eq!(registration, Registration { valid_till: 150, stake: Stake::get() });
+
+			// slash and deregister
+			let slash_destination = RewardsAccountParams::new(
+				HashedLaneId::try_new(1, 2).unwrap(),
+				*b"test",
+				RewardsAccountOwner::ThisChain,
+			);
+			let slash_destination = bp_relayers::ExplicitOrAccountParams::Params(slash_destination);
+			Pallet::<TestRuntime>::slash_and_deregister(&REGISTER_RELAYER, slash_destination);
+			// check if event emitted
+			assert_eq!(
+				System::<TestRuntime>::events().last(),
+				Some(&EventRecord {
+					phase: Phase::Initialization,
+					event: TestEvent::BridgeRelayers(Event::SlashedAndDeregistered {
+						relayer: REGISTER_RELAYER,
+						registration,
+					}),
+					topics: vec![],
+				})
+			)
+		});
+	}
+
 	#[test]
 	fn root_cant_claim_anything() {
 		run_test(|| {

docs/sdk/src/guides/your_first_node.rs

@@ -256,9 +256,14 @@ mod tests {
 
 		let expected_blocks = (10_000 / block_time).saturating_div(2);
 		assert!(expected_blocks > 0, "test configuration is bad, should give it more time");
-		assert!(String::from_utf8(output.stderr)
-			.unwrap()
-			.contains(format!("Imported #{}", expected_blocks).to_string().as_str()));
+		let output = String::from_utf8(output.stderr).unwrap();
+		let want = format!("Imported #{}", expected_blocks);
+		if !output.contains(&want) {
+			panic!(
+				"Output did not contain the pattern:\n\npattern: {}\n\noutput: {}\n",
+				want, output
+			);
+		}
 	}
 
 	#[test]
@@ -282,6 +287,7 @@ mod tests {
 	}
 
 	#[test]
+	#[ignore]
 	fn parachain_runtime_works() {
 		// TODO: None doesn't work. But maybe it should? it would be misleading as many users might
 		// use it.