diff --git a/bridges/modules/relayers/src/benchmarking.rs b/bridges/modules/relayers/src/benchmarking.rs index 706454e0497dd..b79c0fb3ff0b5 100644 --- 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::::insert(&relayer, T::Reward::from(REWARD_AMOUNT)); - }: _(RawOrigin::Signed(relayer)) + RelayerRewards::::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 index f3195f1288ebe..921ec0e4d9820 100644 --- 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 Pallet { /// Claim accumulated rewards. #[pallet::weight(T::WeightInfo::claim_rewards())] - pub fn claim_rewards(origin: OriginFor) -> DispatchResult { + pub fn claim_rewards(origin: OriginFor, lane_id: LaneId) -> DispatchResult { let relayer = ensure_signed(origin)?; - RelayerRewards::::try_mutate_exists(&relayer, |maybe_reward| -> DispatchResult { - let reward = maybe_reward.take().ok_or(Error::::NoRewardForRelayer)?; - T::PaymentProcedure::pay_reward(&relayer, reward).map_err(|e| { - log::trace!( - target: LOG_TARGET, - "Failed to pay rewards to {:?}: {:?}", - relayer, - e, - ); - Error::::FailedToPayReward - })?; - - Self::deposit_event(Event::::RewardPaid { relayer: relayer.clone(), reward }); - Ok(()) - }) + RelayerRewards::::try_mutate_exists( + &relayer, + lane_id, + |maybe_reward| -> DispatchResult { + let reward = maybe_reward.take().ok_or(Error::::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::::FailedToPayReward + })?; + + Self::deposit_event(Event::::RewardPaid { + relayer: relayer.clone(), + lane_id, + reward, + }); + Ok(()) + }, + ) + } + } + + impl Pallet { + /// 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::::mutate(relayer, lane_id, |old_reward: &mut Option| { + 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 = - StorageMap<_, Blake2_128Concat, T::AccountId, T::Reward, OptionQuery>; + pub type RelayerRewards = 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::::claim_rewards(RuntimeOrigin::root()), + Pallet::::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::::claim_rewards(RuntimeOrigin::signed(REGULAR_RELAYER)), + Pallet::::claim_rewards( + RuntimeOrigin::signed(REGULAR_RELAYER), + TEST_LANE_ID + ), Error::::NoRewardForRelayer, ); }); @@ -148,9 +192,12 @@ mod tests { #[test] fn relayer_cant_claim_if_payment_procedure_fails() { run_test(|| { - RelayerRewards::::insert(FAILING_RELAYER, 100); + RelayerRewards::::insert(FAILING_RELAYER, TEST_LANE_ID, 100); assert_noop!( - Pallet::::claim_rewards(RuntimeOrigin::signed(FAILING_RELAYER)), + Pallet::::claim_rewards( + RuntimeOrigin::signed(FAILING_RELAYER), + TEST_LANE_ID + ), Error::::FailedToPayReward, ); }); @@ -161,11 +208,12 @@ mod tests { run_test(|| { get_ready_for_events(); - RelayerRewards::::insert(REGULAR_RELAYER, 100); - assert_ok!(Pallet::::claim_rewards(RuntimeOrigin::signed( - REGULAR_RELAYER - ))); - assert_eq!(RelayerRewards::::get(REGULAR_RELAYER), None); + RelayerRewards::::insert(REGULAR_RELAYER, TEST_LANE_ID, 100); + assert_ok!(Pallet::::claim_rewards( + RuntimeOrigin::signed(REGULAR_RELAYER), + TEST_LANE_ID + )); + assert_eq!(RelayerRewards::::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::::pay_reward(&1, 100).unwrap(); + bp_relayers::MintReward::::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 index 7cf5575f5f384..55db2d575e7ef 100644 --- 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 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 index 772d07d2ad971..4939d274c6095 100644 --- 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>, confirmation_relayer: &T::AccountId, received_range: &RangeInclusive, @@ -52,8 +52,9 @@ where register_relayers_rewards::( 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( confirmation_relayer: &T::AccountId, relayers_rewards: RelayersRewards, + lane_id: bp_messages::LaneId, delivery_fee: T::Reward, confirmation_fee: T::Reward, ) { @@ -87,7 +89,7 @@ fn register_relayers_rewards( relayer_reward = relayer_reward.saturating_sub(confirmation_reward); confirmation_relayer_reward = confirmation_relayer_reward.saturating_add(confirmation_reward); - register_relayer_reward::(&relayer, relayer_reward); + Pallet::::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( } // finally - pay reward to confirmation relayer - register_relayer_reward::(confirmation_relayer, confirmation_relayer_reward); -} - -/// Remember that the reward shall be paid to the relayer. -fn register_relayer_reward(relayer: &T::AccountId, reward: T::Reward) { - if reward.is_zero() { - return - } - - RelayerRewards::::mutate(relayer, |old_reward: &mut Option| { - 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::::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::(&RELAYER_2, relayers_rewards(), 50, 10); - - assert_eq!(RelayerRewards::::get(RELAYER_1), Some(80)); - assert_eq!(RelayerRewards::::get(RELAYER_2), Some(170)); + register_relayers_rewards::( + &RELAYER_2, + relayers_rewards(), + TEST_LANE_ID, + 50, + 10, + ); + + assert_eq!(RelayerRewards::::get(RELAYER_1, TEST_LANE_ID), Some(80)); + assert_eq!(RelayerRewards::::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::(&RELAYER_3, relayers_rewards(), 50, 10); - - assert_eq!(RelayerRewards::::get(RELAYER_1), Some(80)); - assert_eq!(RelayerRewards::::get(RELAYER_2), Some(120)); - assert_eq!(RelayerRewards::::get(RELAYER_3), Some(50)); + register_relayers_rewards::( + &RELAYER_3, + relayers_rewards(), + TEST_LANE_ID, + 50, + 10, + ); + + assert_eq!(RelayerRewards::::get(RELAYER_1, TEST_LANE_ID), Some(80)); + assert_eq!(RelayerRewards::::get(RELAYER_2, TEST_LANE_ID), Some(120)); + assert_eq!(RelayerRewards::::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::(&RELAYER_3, relayers_rewards(), 50, 1000); - - assert_eq!(RelayerRewards::::get(RELAYER_1), None); - assert_eq!(RelayerRewards::::get(RELAYER_2), None); - assert_eq!(RelayerRewards::::get(RELAYER_3), Some(250)); + register_relayers_rewards::( + &RELAYER_3, + relayers_rewards(), + TEST_LANE_ID, + 50, + 1000, + ); + + assert_eq!(RelayerRewards::::get(RELAYER_1, TEST_LANE_ID), None); + assert_eq!(RelayerRewards::::get(RELAYER_2, TEST_LANE_ID), None); + assert_eq!(RelayerRewards::::get(RELAYER_3, TEST_LANE_ID), Some(250)); }); } } diff --git a/bridges/primitives/relayers/Cargo.toml b/bridges/primitives/relayers/Cargo.toml index 908412477c221..4dc09f8d12658 100644 --- 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", ] diff --git a/bridges/primitives/relayers/src/lib.rs b/bridges/primitives/relayers/src/lib.rs index d1c82d612e2e9..bd456d36310ca 100644 --- a/bridges/primitives/relayers/src/lib.rs +++ b/bridges/primitives/relayers/src/lib.rs @@ -19,6 +19,7 @@ #![warn(missing_docs)] #![cfg_attr(not(feature = "std"), no_std)] +use bp_messages::LaneId; use sp_std::{fmt::Debug, marker::PhantomData}; /// Reward payment procedure. @@ -26,8 +27,8 @@ pub trait PaymentProcedure { /// Error that may be returned by the procedure. type Error: Debug; - /// Pay reward to the relayer. - fn pay_reward(relayer: &Relayer, reward: Reward) -> Result<(), Self::Error>; + /// Pay reward to the relayer for serving given message lane. + fn pay_reward(relayer: &Relayer, lane_id: LaneId, reward: Reward) -> Result<(), Self::Error>; } /// Reward payment procedure that is simply minting given amount of tokens. @@ -39,7 +40,11 @@ where { type Error = sp_runtime::DispatchError; - fn pay_reward(relayer: &Relayer, reward: T::Balance) -> Result<(), Self::Error> { + fn pay_reward( + relayer: &Relayer, + _lane_id: LaneId, + reward: T::Balance, + ) -> Result<(), Self::Error> { T::mint_into(relayer, reward) } }