From 6117f15845d38cf861e24c7e6aa04690281c4186 Mon Sep 17 00:00:00 2001 From: Serban Iorga Date: Fri, 12 Aug 2022 19:09:51 +0300 Subject: [PATCH] relayers pallet - small changes (#1547) Signed-off-by: Serban Iorga Signed-off-by: Serban Iorga --- bridges/modules/messages/src/lib.rs | 2 +- bridges/modules/relayers/src/lib.rs | 29 ++++++++++- .../modules/relayers/src/payment_adapter.rs | 51 +++++++++---------- bridges/primitives/relayers/Cargo.toml | 11 ---- 4 files changed, 52 insertions(+), 41 deletions(-) diff --git a/bridges/modules/messages/src/lib.rs b/bridges/modules/messages/src/lib.rs index 9806a264c9946..6e87c51ca986f 100644 --- a/bridges/modules/messages/src/lib.rs +++ b/bridges/modules/messages/src/lib.rs @@ -947,7 +947,7 @@ where // loop won't proceed if current entry is ahead of received range (begin > end). // this loop is bound by `T::MaxUnconfirmedMessagesAtInboundLane` on the bridged chain let mut relayer_reward = relayers_rewards.entry(entry.relayer).or_default(); - for nonce in nonce_begin..nonce_end + 1 { + for nonce in nonce_begin..=nonce_end { let key = MessageKey { lane_id, nonce }; let message_data = OutboundMessages::::get(key) .expect("message was just confirmed; we never prune unconfirmed messages; qed"); diff --git a/bridges/modules/relayers/src/lib.rs b/bridges/modules/relayers/src/lib.rs index 596dd89f31339..779cc285c99fa 100644 --- a/bridges/modules/relayers/src/lib.rs +++ b/bridges/modules/relayers/src/lib.rs @@ -34,6 +34,9 @@ mod payment_adapter; pub mod weights; +/// The target that will be used when publishing logs related to this pallet. +pub const LOG_TARGET: &str = "runtime::bridge-relayers"; + #[frame_support::pallet] pub mod pallet { use super::*; @@ -67,7 +70,7 @@ pub mod pallet { let reward = maybe_reward.take().ok_or(Error::::NoRewardForRelayer)?; T::PaymentProcedure::pay_reward(&relayer, reward).map_err(|e| { log::trace!( - target: "runtime::bridge-relayers", + target: LOG_TARGET, "Failed to pay rewards to {:?}: {:?}", relayer, e, @@ -110,11 +113,18 @@ pub mod pallet { #[cfg(test)] mod tests { use super::*; - use mock::*; + use mock::{Event as TestEvent, *}; + use crate::Event::RewardPaid; use frame_support::{assert_noop, assert_ok, traits::fungible::Inspect}; + use frame_system::{EventRecord, Pallet as System, Phase}; use sp_runtime::DispatchError; + fn get_ready_for_events() { + System::::set_block_number(1); + System::::reset_events(); + } + #[test] fn root_cant_claim_anything() { run_test(|| { @@ -149,9 +159,24 @@ mod tests { #[test] fn relayer_can_claim_reward() { run_test(|| { + get_ready_for_events(); + RelayerRewards::::insert(REGULAR_RELAYER, 100); assert_ok!(Pallet::::claim_rewards(Origin::signed(REGULAR_RELAYER))); assert_eq!(RelayerRewards::::get(REGULAR_RELAYER), None); + + //Check if the `RewardPaid` event was emitted. + assert_eq!( + System::::events(), + vec![EventRecord { + phase: Phase::Initialization, + event: TestEvent::Relayers(RewardPaid { + relayer: REGULAR_RELAYER, + reward: 100 + }), + topics: vec![], + }], + ); }); } diff --git a/bridges/modules/relayers/src/payment_adapter.rs b/bridges/modules/relayers/src/payment_adapter.rs index 6aa1248959dca..46a7ad94d4af2 100644 --- a/bridges/modules/relayers/src/payment_adapter.rs +++ b/bridges/modules/relayers/src/payment_adapter.rs @@ -20,8 +20,8 @@ use crate::{Config, RelayerRewards}; use bp_messages::source_chain::{MessageDeliveryAndDispatchPayment, RelayersRewards}; -use frame_support::traits::Get; -use sp_arithmetic::traits::{Bounded, Saturating, Zero}; +use frame_support::{sp_runtime::SaturatedConversion, traits::Get}; +use sp_arithmetic::traits::{Saturating, Zero}; use sp_std::{collections::vec_deque::VecDeque, marker::PhantomData, ops::RangeInclusive}; /// Adapter that allows relayers pallet to be used as a delivery+dispatch payment mechanism @@ -60,18 +60,17 @@ where messages_relayers, received_range, ); - if !relayers_rewards.is_empty() { - schedule_relayers_rewards::( - confirmation_relayer, - relayers_rewards, - GetConfirmationFee::get(), - ); - } + + register_relayers_rewards::( + confirmation_relayer, + relayers_rewards, + GetConfirmationFee::get(), + ); } } // Update rewards to given relayers, optionally rewarding confirmation relayer. -fn schedule_relayers_rewards( +fn register_relayers_rewards( confirmation_relayer: &T::AccountId, relayers_rewards: RelayersRewards, confirmation_fee: T::Reward, @@ -87,44 +86,42 @@ fn schedule_relayers_rewards( // // If confirmation fee has been increased (or if it was the only component of message // fee), then messages relayer may receive zero reward. - let mut confirmation_reward = T::Reward::try_from(reward.messages) - .unwrap_or_else(|_| Bounded::max_value()) - .saturating_mul(confirmation_fee); - if confirmation_reward > relayer_reward { - confirmation_reward = relayer_reward; - } + let mut confirmation_reward = + T::Reward::saturated_from(reward.messages).saturating_mul(confirmation_fee); + confirmation_reward = sp_std::cmp::min(confirmation_reward, relayer_reward); relayer_reward = relayer_reward.saturating_sub(confirmation_reward); confirmation_relayer_reward = confirmation_relayer_reward.saturating_add(confirmation_reward); + + register_relayer_reward::(&relayer, relayer_reward); } else { // If delivery confirmation is submitted by this relayer, let's add confirmation fee // from other relayers to this relayer reward. - confirmation_relayer_reward = confirmation_relayer_reward.saturating_add(reward.reward); - continue + confirmation_relayer_reward = + confirmation_relayer_reward.saturating_add(relayer_reward); } - - schedule_relayer_reward::(&relayer, relayer_reward); } // finally - pay reward to confirmation relayer - schedule_relayer_reward::(confirmation_relayer, confirmation_relayer_reward); + register_relayer_reward::(confirmation_relayer, confirmation_relayer_reward); } /// Remember that the reward shall be paid to the relayer. -fn schedule_relayer_reward(relayer: &T::AccountId, reward: T::Reward) { +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: "T::bridge-relayers", + target: crate::LOG_TARGET, "Relayer {:?} can now claim reward: {:?}", relayer, new_reward, ); - *old_reward = Some(new_reward); }); } @@ -149,7 +146,7 @@ mod tests { #[test] fn confirmation_relayer_is_rewarded_if_it_has_also_delivered_messages() { run_test(|| { - schedule_relayers_rewards::(&RELAYER_2, relayers_rewards(), 10); + register_relayers_rewards::(&RELAYER_2, relayers_rewards(), 10); assert_eq!(RelayerRewards::::get(&RELAYER_1), Some(80)); assert_eq!(RelayerRewards::::get(&RELAYER_2), Some(120)); @@ -159,7 +156,7 @@ mod tests { #[test] fn confirmation_relayer_is_rewarded_if_it_has_not_delivered_any_delivered_messages() { run_test(|| { - schedule_relayers_rewards::(&RELAYER_3, relayers_rewards(), 10); + register_relayers_rewards::(&RELAYER_3, relayers_rewards(), 10); assert_eq!(RelayerRewards::::get(&RELAYER_1), Some(80)); assert_eq!(RelayerRewards::::get(&RELAYER_2), Some(70)); @@ -170,7 +167,7 @@ mod tests { #[test] fn only_confirmation_relayer_is_rewarded_if_confirmation_fee_has_significantly_increased() { run_test(|| { - schedule_relayers_rewards::(&RELAYER_3, relayers_rewards(), 1000); + register_relayers_rewards::(&RELAYER_3, relayers_rewards(), 1000); assert_eq!(RelayerRewards::::get(&RELAYER_1), None); assert_eq!(RelayerRewards::::get(&RELAYER_2), None); diff --git a/bridges/primitives/relayers/Cargo.toml b/bridges/primitives/relayers/Cargo.toml index 0d6d020a0997b..908412477c221 100644 --- a/bridges/primitives/relayers/Cargo.toml +++ b/bridges/primitives/relayers/Cargo.toml @@ -8,15 +8,9 @@ license = "GPL-3.0-or-later WITH Classpath-exception-2.0" [dependencies] -# Bridge dependencies - -#bp-runtime = { path = "../runtime", default-features = false } - # Substrate Dependencies frame-support = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false } -#frame-system = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false } -#sp-core = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false } sp-runtime = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false } sp-std = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false } @@ -27,12 +21,7 @@ hex-literal = "0.3" [features] default = ["std"] std = [ -# "bp-runtime/std", "frame-support/std", -# "frame-system/std", -# "scale-info/std", -# "serde", -# "sp-core/std", "sp-runtime/std", "sp-std/std", ]