Make pallet-bridge-rewards generic over RewardKind (#7492)

Closes: #7272 Relates to: #6578 Relates to: #7274 ## Description The PR enhances the `pallet-bridge-rewards` by making it generic over the `RewardKind` type (previously hardcoded as `RewardsAccountParams`). This modification allows the pallet to support multiple reward types (e.g., P/K bridge, Snowbridge), increasing its flexibility and applicability across various bridge scenarios. Other pallets can register rewards using `bp_relayers::RewardLedger`, which is implemented by the rewards pallet. The runtime can then be configured with different mechanisms for paying/claiming rewards via `bp_relayers::PaymentProcedure` (e.g., see the `pub struct BridgeRewardPayer;` implementation for BridgeHubWestend). ## Follow-up The removed balances/rewards statistics from the complex relay (no longer used) will eventually be reintroduced or fixed in the standalone relayers via paritytech/parity-bridges-common#3004 (comment). --------- Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Francisco Aguirre <[email protected]> Co-authored-by: Serban Iorga <[email protected]>
paritytech · Feb 19, 2025 · ba7cb48 · ba7cb48
1 parent 0c258c6
commit ba7cb48
Show file tree

Hide file tree

Showing 44 changed files with 1,395 additions and 560 deletions.
diff --git a/.github/workflows/runtimes-matrix.json b/.github/workflows/runtimes-matrix.json
@@ -67,7 +67,7 @@
   },
   {
     "name": "bridge-hub-westend",
-    "package": "bridge-hub-rococo-runtime",
+    "package": "bridge-hub-westend-runtime",
     "path": "cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend",
     "header": "cumulus/file_header.txt",
     "template": "cumulus/templates/xcm-bench-template.hbs",

diff --git a/Cargo.lock b/Cargo.lock
diff --git a/bridges/bin/runtime-common/src/extensions.rs b/bridges/bin/runtime-common/src/extensions.rs
@@ -111,7 +111,7 @@ where
 		// let's slash registered relayer
 		RelayersPallet::<T>::slash_and_deregister(
 			relayer,
-			ExplicitOrAccountParams::Explicit(SlashAccount::get()),
+			ExplicitOrAccountParams::Explicit::<_, ()>(SlashAccount::get()),
 		);
 	}
 }
@@ -182,7 +182,7 @@ where
 		// let's slash registered relayer
 		RelayersPallet::<T>::slash_and_deregister(
 			relayer,
-			ExplicitOrAccountParams::Explicit(SlashAccount::get()),
+			ExplicitOrAccountParams::Explicit::<_, ()>(SlashAccount::get()),
 		);
 	}
 }

diff --git a/bridges/bin/runtime-common/src/mock.rs b/bridges/bin/runtime-common/src/mock.rs
@@ -24,7 +24,7 @@ use bp_messages::{
 	ChainWithMessages, HashedLaneId, LaneIdType, MessageNonce,
 };
 use bp_parachains::SingleParaStoredHeaderDataBuilder;
-use bp_relayers::PayRewardFromAccount;
+use bp_relayers::{PayRewardFromAccount, RewardsAccountParams};
 use bp_runtime::{messages::MessageDispatchResult, Chain, ChainId, Parachain};
 use codec::Encode;
 use frame_support::{
@@ -70,7 +70,8 @@ pub type BridgedChainHeader =
 	sp_runtime::generic::Header<BridgedChainBlockNumber, BridgedChainHasher>;
 
 /// Rewards payment procedure.
-pub type TestPaymentProcedure = PayRewardFromAccount<Balances, ThisChainAccountId, TestLaneIdType>;
+pub type TestPaymentProcedure =
+	PayRewardFromAccount<Balances, ThisChainAccountId, TestLaneIdType, RewardBalance>;
 /// Stake that we are using in tests.
 pub type TestStake = ConstU64<5_000>;
 /// Stake and slash mechanism to use in tests.
@@ -89,6 +90,8 @@ pub type TestLaneIdType = HashedLaneId;
 pub fn test_lane_id() -> TestLaneIdType {
 	TestLaneIdType::try_new(1, 2).unwrap()
 }
+/// Reward measurement type.
+pub type RewardBalance = u32;
 
 /// Bridged chain id used in tests.
 pub const TEST_BRIDGED_CHAIN_ID: ChainId = *b"brdg";
@@ -197,7 +200,7 @@ impl pallet_bridge_messages::Config for TestRuntime {
 		TestRuntime,
 		(),
 		(),
-		ConstU64<100_000>,
+		ConstU32<100_000>,
 	>;
 	type OnMessagesDelivered = ();
 
@@ -210,11 +213,12 @@ impl pallet_bridge_messages::Config for TestRuntime {
 
 impl pallet_bridge_relayers::Config for TestRuntime {
 	type RuntimeEvent = RuntimeEvent;
-	type Reward = ThisChainBalance;
+	type RewardBalance = RewardBalance;
+	type Reward = RewardsAccountParams<pallet_bridge_messages::LaneIdOf<TestRuntime, ()>>;
 	type PaymentProcedure = TestPaymentProcedure;
 	type StakeAndSlash = TestStakeAndSlash;
+	type Balance = ThisChainBalance;
 	type WeightInfo = ();
-	type LaneId = TestLaneIdType;
 }
 
 /// Dummy message dispatcher.

diff --git a/bridges/modules/relayers/Cargo.toml b/bridges/modules/relayers/Cargo.toml
@@ -31,7 +31,6 @@ frame-system = { workspace = true }
 pallet-transaction-payment = { workspace = true }
 sp-arithmetic = { workspace = true }
 sp-runtime = { workspace = true }
-sp-std = { workspace = true }
 
 [dev-dependencies]
 bp-parachains = { workspace = true }
@@ -69,7 +68,6 @@ std = [
 	"sp-core/std",
 	"sp-io/std",
 	"sp-runtime/std",
-	"sp-std/std",
 ]
 runtime-benchmarks = [
 	"frame-benchmarking/runtime-benchmarks",

diff --git a/bridges/modules/relayers/src/benchmarking.rs b/bridges/modules/relayers/src/benchmarking.rs
@@ -20,8 +20,10 @@
 
 use crate::*;
 
-use bp_relayers::RewardsAccountOwner;
-use frame_benchmarking::{benchmarks_instance_pallet, whitelisted_caller};
+use frame_benchmarking::{
+	benchmarks_instance_pallet, whitelisted_caller, BenchmarkError, BenchmarkResult,
+};
+use frame_support::{assert_ok, weights::Weight};
 use frame_system::RawOrigin;
 use sp_runtime::traits::One;
 
@@ -33,35 +35,70 @@ pub struct Pallet<T: Config<I>, I: 'static = ()>(crate::Pallet<T, I>);
 
 /// Trait that must be implemented by runtime.
 pub trait Config<I: 'static = ()>: crate::Config<I> {
-	/// Lane id to use in benchmarks.
-	fn bench_lane_id() -> Self::LaneId {
-		Self::LaneId::default()
-	}
+	/// `T::Reward` to use in benchmarks.
+	fn bench_reward() -> Self::Reward;
 	/// Prepare environment for paying given reward for serving given lane.
 	fn prepare_rewards_account(
-		account_params: RewardsAccountParams<Self::LaneId>,
-		reward: Self::Reward,
-	);
+		reward_kind: Self::Reward,
+		reward: Self::RewardBalance,
+	) -> Option<BeneficiaryOf<Self, I>>;
 	/// Give enough balance to given account.
-	fn deposit_account(account: Self::AccountId, balance: Self::Reward);
+	fn deposit_account(account: Self::AccountId, balance: Self::Balance);
+}
+
+fn assert_last_event<T: Config<I>, I: 'static>(
+	generic_event: <T as pallet::Config<I>>::RuntimeEvent,
+) {
+	frame_system::Pallet::<T>::assert_last_event(generic_event.into());
 }
 
 benchmarks_instance_pallet! {
+	where_clause { where
+		BeneficiaryOf<T, I>: From<<T as frame_system::Config>::AccountId>,
+	}
+
 	// Benchmark `claim_rewards` call.
 	claim_rewards {
-		let lane = T::bench_lane_id();
-		let account_params =
-			RewardsAccountParams::new(lane, *b"test", RewardsAccountOwner::ThisChain);
+		let reward_kind = T::bench_reward();
 		let relayer: T::AccountId = whitelisted_caller();
-		let reward = T::Reward::from(REWARD_AMOUNT);
+		let reward_balance = T::RewardBalance::from(REWARD_AMOUNT);
 
-		T::prepare_rewards_account(account_params, reward);
-		RelayerRewards::<T, I>::insert(&relayer, account_params, reward);
-	}: _(RawOrigin::Signed(relayer), account_params)
+		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 {
 		// 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());
+	}
+
+	// Benchmark `claim_rewards_to` call.
+	claim_rewards_to {
+		let reward_kind = T::bench_reward();
+		let relayer: T::AccountId = whitelisted_caller();
+		let reward_balance = T::RewardBalance::from(REWARD_AMOUNT);
+
+		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 {
+		// 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());
 	}
 
 	// Benchmark `register` call.
@@ -95,7 +132,7 @@ benchmarks_instance_pallet! {
 	}
 
 	// Benchmark `slash_and_deregister` method of the pallet. We are adding this weight to
-	// the weight of message delivery call if `RefundBridgedParachainMessages` signed extension
+	// the weight of message delivery call if `BridgeRelayersTransactionExtension` signed extension
 	// is deployed at runtime level.
 	slash_and_deregister {
 		// prepare and register relayer account
@@ -105,32 +142,30 @@ benchmarks_instance_pallet! {
 			.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();
+		assert_ok!(crate::Pallet::<T, I>::register(RawOrigin::Signed(relayer.clone()).into(), valid_till));
 
 		// create slash destination account
-		let lane = T::bench_lane_id();
-		let slash_destination = RewardsAccountParams::new(lane, *b"test", RewardsAccountOwner::ThisChain);
-		T::prepare_rewards_account(slash_destination, Zero::zero());
+		let slash_destination: T::AccountId = whitelisted_caller();
+		T::deposit_account(slash_destination.clone(), Zero::zero());
 	}: {
-		crate::Pallet::<T, I>::slash_and_deregister(&relayer, slash_destination.into())
+		crate::Pallet::<T, I>::slash_and_deregister(&relayer, bp_relayers::ExplicitOrAccountParams::Explicit::<_, ()>(slash_destination))
 	}
 	verify {
 		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 `RefundBridgedParachainMessages` signed extension
+	// the weight of message delivery call if `BridgeRelayersTransactionExtension` signed extension
 	// is deployed at runtime level.
 	register_relayer_reward {
-		let lane = T::bench_lane_id();
+		let reward_kind = T::bench_reward();
 		let relayer: T::AccountId = whitelisted_caller();
-		let account_params =
-			RewardsAccountParams::new(lane, *b"test", RewardsAccountOwner::ThisChain);
+
 	}: {
-		crate::Pallet::<T, I>::register_relayer_reward(account_params, &relayer, One::one());
+		crate::Pallet::<T, I>::register_relayer_reward(reward_kind, &relayer, One::one());
 	}
 	verify {
-		assert_eq!(RelayerRewards::<T, I>::get(relayer, &account_params), Some(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)

diff --git a/bridges/modules/relayers/src/extension/grandpa_adapter.rs b/bridges/modules/relayers/src/extension/grandpa_adapter.rs
@@ -23,6 +23,7 @@ use crate::{
 
 use bp_relayers::{BatchCallUnpacker, ExtensionCallData, ExtensionCallInfo, ExtensionConfig};
 use bp_runtime::{Chain, StaticStrProvider};
+use core::marker::PhantomData;
 use frame_support::dispatch::{DispatchInfo, PostDispatchInfo};
 use frame_system::Config as SystemConfig;
 use pallet_bridge_grandpa::{
@@ -37,7 +38,6 @@ use sp_runtime::{
 	transaction_validity::{TransactionPriority, TransactionValidityError},
 	Saturating,
 };
-use sp_std::marker::PhantomData;
 
 /// Adapter to be used in signed extension configuration, when bridging with remote
 /// chains that are using GRANDPA finality.

diff --git a/bridges/modules/relayers/src/extension/messages_adapter.rs b/bridges/modules/relayers/src/extension/messages_adapter.rs
@@ -21,6 +21,7 @@ use crate::{extension::verify_messages_call_succeeded, Config as BridgeRelayersC
 
 use bp_relayers::{ExtensionCallData, ExtensionCallInfo, ExtensionConfig};
 use bp_runtime::StaticStrProvider;
+use core::marker::PhantomData;
 use frame_support::dispatch::{DispatchInfo, PostDispatchInfo};
 use pallet_bridge_messages::{
 	CallSubType as BridgeMessagesCallSubType, Config as BridgeMessagesConfig, LaneIdOf,
@@ -29,7 +30,6 @@ use sp_runtime::{
 	traits::{Dispatchable, Get},
 	transaction_validity::{TransactionPriority, TransactionValidityError},
 };
-use sp_std::marker::PhantomData;
 
 /// Transaction extension that refunds a relayer for standalone messages delivery and confirmation
 /// transactions. Finality transactions are not refunded.