Per-lane rewards in pallet-bridge-relayers (paritytech#1665)

* per-lane rewards in pallet-bridge-relayers * add lane id to RewardPaid event * clippy * fix benchmarks
serban300 · Apr 9, 2024 · 6d9f1eb · 6d9f1eb
1 parent bc23df6
commit 6d9f1eb
Show file tree

Hide file tree

Showing 6 changed files with 151 additions and 76 deletions.
diff --git a/bridges/modules/relayers/src/benchmarking.rs b/bridges/modules/relayers/src/benchmarking.rs
@@ -29,9 +29,10 @@ const REWARD_AMOUNT: u32 = u32::MAX;
 benchmarks! {
 	// Benchmark `claim_rewards` call.
 	claim_rewards {
+		let lane = [0, 0, 0, 0];
 		let relayer: T::AccountId = whitelisted_caller();
-		RelayerRewards::<T>::insert(&relayer, T::Reward::from(REWARD_AMOUNT));
-	}: _(RawOrigin::Signed(relayer))
+		RelayerRewards::<T>::insert(&relayer, lane, T::Reward::from(REWARD_AMOUNT));
+	}: _(RawOrigin::Signed(relayer), lane)
 	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

diff --git a/bridges/modules/relayers/src/lib.rs b/bridges/modules/relayers/src/lib.rs
@@ -20,8 +20,10 @@
 #![cfg_attr(not(feature = "std"), no_std)]
 #![warn(missing_docs)]
 
+use bp_messages::LaneId;
 use bp_relayers::PaymentProcedure;
-use sp_arithmetic::traits::AtLeast32BitUnsigned;
+use frame_support::sp_runtime::Saturating;
+use sp_arithmetic::traits::{AtLeast32BitUnsigned, Zero};
 use sp_std::marker::PhantomData;
 use weights::WeightInfo;
 
@@ -63,24 +65,54 @@ pub mod pallet {
 	impl<T: Config> Pallet<T> {
 		/// Claim accumulated rewards.
 		#[pallet::weight(T::WeightInfo::claim_rewards())]
-		pub fn claim_rewards(origin: OriginFor<T>) -> DispatchResult {
+		pub fn claim_rewards(origin: OriginFor<T>, lane_id: LaneId) -> DispatchResult {
 			let relayer = ensure_signed(origin)?;
 
-			RelayerRewards::<T>::try_mutate_exists(&relayer, |maybe_reward| -> DispatchResult {
-				let reward = maybe_reward.take().ok_or(Error::<T>::NoRewardForRelayer)?;
-				T::PaymentProcedure::pay_reward(&relayer, reward).map_err(|e| {
-					log::trace!(
-						target: LOG_TARGET,
-						"Failed to pay rewards to {:?}: {:?}",
-						relayer,
-						e,
-					);
-					Error::<T>::FailedToPayReward
-				})?;
-
-				Self::deposit_event(Event::<T>::RewardPaid { relayer: relayer.clone(), reward });
-				Ok(())
-			})
+			RelayerRewards::<T>::try_mutate_exists(
+				&relayer,
+				lane_id,
+				|maybe_reward| -> DispatchResult {
+					let reward = maybe_reward.take().ok_or(Error::<T>::NoRewardForRelayer)?;
+					T::PaymentProcedure::pay_reward(&relayer, lane_id, reward).map_err(|e| {
+						log::trace!(
+							target: LOG_TARGET,
+							"Failed to pay {:?} rewards to {:?}: {:?}",
+							lane_id,
+							relayer,
+							e,
+						);
+						Error::<T>::FailedToPayReward
+					})?;
+
+					Self::deposit_event(Event::<T>::RewardPaid {
+						relayer: relayer.clone(),
+						lane_id,
+						reward,
+					});
+					Ok(())
+				},
+			)
+		}
+	}
+
+	impl<T: Config> Pallet<T> {
+		/// Register reward for given relayer.
+		pub fn register_relayer_reward(lane_id: LaneId, relayer: &T::AccountId, reward: T::Reward) {
+			if reward.is_zero() {
+				return
+			}
+
+			RelayerRewards::<T>::mutate(relayer, lane_id, |old_reward: &mut Option<T::Reward>| {
+				let new_reward = old_reward.unwrap_or_else(Zero::zero).saturating_add(reward);
+				*old_reward = Some(new_reward);
+
+				log::trace!(
+					target: crate::LOG_TARGET,
+					"Relayer {:?} can now claim reward: {:?}",
+					relayer,
+					new_reward,
+				);
+			});
 		}
 	}
 
@@ -91,6 +123,8 @@ pub mod pallet {
 		RewardPaid {
 			/// Relayer account that has been rewarded.
 			relayer: T::AccountId,
+			/// Relayer has received reward for serving this lane.
+			lane_id: LaneId,
 			/// Reward amount.
 			reward: T::Reward,
 		},
@@ -106,8 +140,15 @@ pub mod pallet {
 
 	/// Map of the relayer => accumulated reward.
 	#[pallet::storage]
-	pub type RelayerRewards<T: Config> =
-		StorageMap<_, Blake2_128Concat, T::AccountId, T::Reward, OptionQuery>;
+	pub type RelayerRewards<T: Config> = StorageDoubleMap<
+		_,
+		Blake2_128Concat,
+		T::AccountId,
+		Identity,
+		LaneId,
+		T::Reward,
+		OptionQuery,
+	>;
 }
 
 #[cfg(test)]
@@ -129,7 +170,7 @@ mod tests {
 	fn root_cant_claim_anything() {
 		run_test(|| {
 			assert_noop!(
-				Pallet::<TestRuntime>::claim_rewards(RuntimeOrigin::root()),
+				Pallet::<TestRuntime>::claim_rewards(RuntimeOrigin::root(), TEST_LANE_ID),
 				DispatchError::BadOrigin,
 			);
 		});
@@ -139,7 +180,10 @@ mod tests {
 	fn relayer_cant_claim_if_no_reward_exists() {
 		run_test(|| {
 			assert_noop!(
-				Pallet::<TestRuntime>::claim_rewards(RuntimeOrigin::signed(REGULAR_RELAYER)),
+				Pallet::<TestRuntime>::claim_rewards(
+					RuntimeOrigin::signed(REGULAR_RELAYER),
+					TEST_LANE_ID
+				),
 				Error::<TestRuntime>::NoRewardForRelayer,
 			);
 		});
@@ -148,9 +192,12 @@ mod tests {
 	#[test]
 	fn relayer_cant_claim_if_payment_procedure_fails() {
 		run_test(|| {
-			RelayerRewards::<TestRuntime>::insert(FAILING_RELAYER, 100);
+			RelayerRewards::<TestRuntime>::insert(FAILING_RELAYER, TEST_LANE_ID, 100);
 			assert_noop!(
-				Pallet::<TestRuntime>::claim_rewards(RuntimeOrigin::signed(FAILING_RELAYER)),
+				Pallet::<TestRuntime>::claim_rewards(
+					RuntimeOrigin::signed(FAILING_RELAYER),
+					TEST_LANE_ID
+				),
 				Error::<TestRuntime>::FailedToPayReward,
 			);
 		});
@@ -161,11 +208,12 @@ mod tests {
 		run_test(|| {
 			get_ready_for_events();
 
-			RelayerRewards::<TestRuntime>::insert(REGULAR_RELAYER, 100);
-			assert_ok!(Pallet::<TestRuntime>::claim_rewards(RuntimeOrigin::signed(
-				REGULAR_RELAYER
-			)));
-			assert_eq!(RelayerRewards::<TestRuntime>::get(REGULAR_RELAYER), None);
+			RelayerRewards::<TestRuntime>::insert(REGULAR_RELAYER, TEST_LANE_ID, 100);
+			assert_ok!(Pallet::<TestRuntime>::claim_rewards(
+				RuntimeOrigin::signed(REGULAR_RELAYER),
+				TEST_LANE_ID
+			));
+			assert_eq!(RelayerRewards::<TestRuntime>::get(REGULAR_RELAYER, TEST_LANE_ID), None);
 
 			//Check if the `RewardPaid` event was emitted.
 			assert_eq!(
@@ -174,6 +222,7 @@ mod tests {
 					phase: Phase::Initialization,
 					event: TestEvent::Relayers(RewardPaid {
 						relayer: REGULAR_RELAYER,
+						lane_id: TEST_LANE_ID,
 						reward: 100
 					}),
 					topics: vec![],
@@ -189,7 +238,8 @@ mod tests {
 		run_test(|| {
 			assert_eq!(Balances::balance(&1), 0);
 			assert_eq!(Balances::total_issuance(), 0);
-			bp_relayers::MintReward::<Balances, AccountId>::pay_reward(&1, 100).unwrap();
+			bp_relayers::MintReward::<Balances, AccountId>::pay_reward(&1, TEST_LANE_ID, 100)
+				.unwrap();
 			assert_eq!(Balances::balance(&1), 100);
 			assert_eq!(Balances::total_issuance(), 100);
 		});

diff --git a/bridges/modules/relayers/src/mock.rs b/bridges/modules/relayers/src/mock.rs
@@ -18,7 +18,9 @@
 
 use crate as pallet_bridge_relayers;
 
-use bp_messages::{source_chain::ForbidOutboundMessages, target_chain::ForbidInboundMessages};
+use bp_messages::{
+	source_chain::ForbidOutboundMessages, target_chain::ForbidInboundMessages, LaneId,
+};
 use bp_relayers::PaymentProcedure;
 use frame_support::{parameter_types, weights::RuntimeDbWeight};
 use sp_core::H256;
@@ -124,6 +126,9 @@ impl pallet_bridge_relayers::Config for TestRuntime {
 	type WeightInfo = ();
 }
 
+/// Message lane that we're using in tests.
+pub const TEST_LANE_ID: LaneId = [0, 0, 0, 0];
+
 /// Regular relayer that may receive rewards.
 pub const REGULAR_RELAYER: AccountId = 1;
 
@@ -136,7 +141,11 @@ pub struct TestPaymentProcedure;
 impl PaymentProcedure<AccountId, Balance> for TestPaymentProcedure {
 	type Error = ();
 
-	fn pay_reward(relayer: &AccountId, _reward: Balance) -> Result<(), Self::Error> {
+	fn pay_reward(
+		relayer: &AccountId,
+		_lane_id: LaneId,
+		_reward: Balance,
+	) -> Result<(), Self::Error> {
 		match *relayer {
 			FAILING_RELAYER => Err(()),
 			_ => Ok(()),

diff --git a/bridges/modules/relayers/src/payment_adapter.rs b/bridges/modules/relayers/src/payment_adapter.rs
@@ -17,7 +17,7 @@
 //! Code that allows relayers pallet to be used as a delivery+dispatch payment mechanism
 //! for the messages pallet.
 
-use crate::{Config, RelayerRewards};
+use crate::{Config, Pallet};
 
 use bp_messages::source_chain::{MessageDeliveryAndDispatchPayment, RelayersRewards};
 use frame_support::sp_runtime::SaturatedConversion;
@@ -39,7 +39,7 @@ where
 	type Error = &'static str;
 
 	fn pay_relayers_rewards(
-		_lane_id: bp_messages::LaneId,
+		lane_id: bp_messages::LaneId,
 		messages_relayers: VecDeque<bp_messages::UnrewardedRelayer<T::AccountId>>,
 		confirmation_relayer: &T::AccountId,
 		received_range: &RangeInclusive<bp_messages::MessageNonce>,
@@ -52,8 +52,9 @@ where
 		register_relayers_rewards::<T>(
 			confirmation_relayer,
 			relayers_rewards,
+			lane_id,
 			// TODO (https://github.com/paritytech/parity-bridges-common/issues/1318): this shall be fixed
-			// in some way. ATM the future of the `register_relayers_rewards` is not yet known
+			// in some way. ATM the future of the `register_relayer_reward` is not yet known
 			100_000_u32.into(),
 			10_000_u32.into(),
 		);
@@ -64,6 +65,7 @@ where
 fn register_relayers_rewards<T: Config>(
 	confirmation_relayer: &T::AccountId,
 	relayers_rewards: RelayersRewards<T::AccountId>,
+	lane_id: bp_messages::LaneId,
 	delivery_fee: T::Reward,
 	confirmation_fee: T::Reward,
 ) {
@@ -87,7 +89,7 @@ fn register_relayers_rewards<T: Config>(
 			relayer_reward = relayer_reward.saturating_sub(confirmation_reward);
 			confirmation_relayer_reward =
 				confirmation_relayer_reward.saturating_add(confirmation_reward);
-			register_relayer_reward::<T>(&relayer, relayer_reward);
+			Pallet::<T>::register_relayer_reward(lane_id, &relayer, relayer_reward);
 		} else {
 			// If delivery confirmation is submitted by this relayer, let's add confirmation fee
 			// from other relayers to this relayer reward.
@@ -97,32 +99,17 @@ fn register_relayers_rewards<T: Config>(
 	}
 
 	// finally - pay reward to confirmation relayer
-	register_relayer_reward::<T>(confirmation_relayer, confirmation_relayer_reward);
-}
-
-/// Remember that the reward shall be paid to the relayer.
-fn register_relayer_reward<T: Config>(relayer: &T::AccountId, reward: T::Reward) {
-	if reward.is_zero() {
-		return
-	}
-
-	RelayerRewards::<T>::mutate(relayer, |old_reward: &mut Option<T::Reward>| {
-		let new_reward = old_reward.unwrap_or_else(Zero::zero).saturating_add(reward);
-		*old_reward = Some(new_reward);
-
-		log::trace!(
-			target: crate::LOG_TARGET,
-			"Relayer {:?} can now claim reward: {:?}",
-			relayer,
-			new_reward,
-		);
-	});
+	Pallet::<T>::register_relayer_reward(
+		lane_id,
+		confirmation_relayer,
+		confirmation_relayer_reward,
+	);
 }
 
 #[cfg(test)]
 mod tests {
 	use super::*;
-	use crate::mock::*;
+	use crate::{mock::*, RelayerRewards};
 
 	const RELAYER_1: AccountId = 1;
 	const RELAYER_2: AccountId = 2;
@@ -135,32 +122,50 @@ mod tests {
 	#[test]
 	fn confirmation_relayer_is_rewarded_if_it_has_also_delivered_messages() {
 		run_test(|| {
-			register_relayers_rewards::<TestRuntime>(&RELAYER_2, relayers_rewards(), 50, 10);
-
-			assert_eq!(RelayerRewards::<TestRuntime>::get(RELAYER_1), Some(80));
-			assert_eq!(RelayerRewards::<TestRuntime>::get(RELAYER_2), Some(170));
+			register_relayers_rewards::<TestRuntime>(
+				&RELAYER_2,
+				relayers_rewards(),
+				TEST_LANE_ID,
+				50,
+				10,
+			);
+
+			assert_eq!(RelayerRewards::<TestRuntime>::get(RELAYER_1, TEST_LANE_ID), Some(80));
+			assert_eq!(RelayerRewards::<TestRuntime>::get(RELAYER_2, TEST_LANE_ID), Some(170));
 		});
 	}
 
 	#[test]
 	fn confirmation_relayer_is_rewarded_if_it_has_not_delivered_any_delivered_messages() {
 		run_test(|| {
-			register_relayers_rewards::<TestRuntime>(&RELAYER_3, relayers_rewards(), 50, 10);
-
-			assert_eq!(RelayerRewards::<TestRuntime>::get(RELAYER_1), Some(80));
-			assert_eq!(RelayerRewards::<TestRuntime>::get(RELAYER_2), Some(120));
-			assert_eq!(RelayerRewards::<TestRuntime>::get(RELAYER_3), Some(50));
+			register_relayers_rewards::<TestRuntime>(
+				&RELAYER_3,
+				relayers_rewards(),
+				TEST_LANE_ID,
+				50,
+				10,
+			);
+
+			assert_eq!(RelayerRewards::<TestRuntime>::get(RELAYER_1, TEST_LANE_ID), Some(80));
+			assert_eq!(RelayerRewards::<TestRuntime>::get(RELAYER_2, TEST_LANE_ID), Some(120));
+			assert_eq!(RelayerRewards::<TestRuntime>::get(RELAYER_3, TEST_LANE_ID), Some(50));
 		});
 	}
 
 	#[test]
 	fn only_confirmation_relayer_is_rewarded_if_confirmation_fee_has_significantly_increased() {
 		run_test(|| {
-			register_relayers_rewards::<TestRuntime>(&RELAYER_3, relayers_rewards(), 50, 1000);
-
-			assert_eq!(RelayerRewards::<TestRuntime>::get(RELAYER_1), None);
-			assert_eq!(RelayerRewards::<TestRuntime>::get(RELAYER_2), None);
-			assert_eq!(RelayerRewards::<TestRuntime>::get(RELAYER_3), Some(250));
+			register_relayers_rewards::<TestRuntime>(
+				&RELAYER_3,
+				relayers_rewards(),
+				TEST_LANE_ID,
+				50,
+				1000,
+			);
+
+			assert_eq!(RelayerRewards::<TestRuntime>::get(RELAYER_1, TEST_LANE_ID), None);
+			assert_eq!(RelayerRewards::<TestRuntime>::get(RELAYER_2, TEST_LANE_ID), None);
+			assert_eq!(RelayerRewards::<TestRuntime>::get(RELAYER_3, TEST_LANE_ID), Some(250));
 		});
 	}
 }
diff --git a/bridges/primitives/relayers/Cargo.toml b/bridges/primitives/relayers/Cargo.toml
@@ -8,6 +8,10 @@ license = "GPL-3.0-or-later WITH Classpath-exception-2.0"
 
 [dependencies]
 
+# Bridge Dependencies
+
+bp-messages = { path = "../messages", default-features = false }
+
 # Substrate Dependencies
 
 frame-support = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false }
@@ -21,7 +25,8 @@ hex-literal = "0.3"
 [features]
 default = ["std"]
 std = [
+	"bp-messages/std",
 	"frame-support/std",
 	"sp-runtime/std",
-    "sp-std/std",
+	"sp-std/std",
 ]