Move calculation relayer reward into MessageDeliveryAndDispatchPayment (#1153)

* Refactor logic

* Thanks svyatonik help, it compile

* Fix failed unit test

* Remove compile warning

* Rename

* Return result in pay_relayers_rewards

* Fix runtime compile issue

* Use MessageNonce

* Fix review issue

* Missing u64 replacement

* Revert return type changes

* Fix merge issue

* Remove useless clone

bridges/bin/millau/runtime/src/lib.rs

@@ -402,6 +402,7 @@ impl pallet_bridge_messages::Config<WithRialtoMessagesInstance> for Runtime {
 	type MessageDeliveryAndDispatchPayment =
 		pallet_bridge_messages::instant_payments::InstantCurrencyPayments<
 			Runtime,
+			(),
 			pallet_balances::Pallet<Runtime>,
 			GetDeliveryConfirmationTransactionFee,
 			RootAccountForPayments,

bridges/bin/rialto/runtime/src/lib.rs

@@ -545,6 +545,7 @@ impl pallet_bridge_messages::Config<WithMillauMessagesInstance> for Runtime {
 	type MessageDeliveryAndDispatchPayment =
 		pallet_bridge_messages::instant_payments::InstantCurrencyPayments<
 			Runtime,
+			(),
 			pallet_balances::Pallet<Runtime>,
 			GetDeliveryConfirmationTransactionFee,
 			RootAccountForPayments,

bridges/modules/messages/src/instant_payments.rs

@@ -19,15 +19,17 @@
 //! The payment is first transferred to a special `relayers-fund` account and only transferred
 //! to the actual relayer in case confirmation is received.
 
+use crate::OutboundMessages;
+
 use bp_messages::{
 	source_chain::{MessageDeliveryAndDispatchPayment, RelayersRewards, Sender},
-	MessageNonce,
+	LaneId, MessageKey, MessageNonce, UnrewardedRelayer,
 };
 use codec::Encode;
 use frame_support::traits::{Currency as CurrencyT, ExistenceRequirement, Get};
-use num_traits::Zero;
+use num_traits::{SaturatingAdd, Zero};
 use sp_runtime::traits::Saturating;
-use sp_std::fmt::Debug;
+use sp_std::{collections::vec_deque::VecDeque, fmt::Debug, ops::RangeInclusive};
 
 /// Instant message payments made in given currency.
 ///
@@ -42,16 +44,17 @@ use sp_std::fmt::Debug;
 /// to the relayer account.
 /// NOTE It's within relayer's interest to keep their balance above ED as well, to make sure they
 /// can receive the payment.
-pub struct InstantCurrencyPayments<T, Currency, GetConfirmationFee, RootAccount> {
-	_phantom: sp_std::marker::PhantomData<(T, Currency, GetConfirmationFee, RootAccount)>,
+pub struct InstantCurrencyPayments<T, I, Currency, GetConfirmationFee, RootAccount> {
+	_phantom: sp_std::marker::PhantomData<(T, I, Currency, GetConfirmationFee, RootAccount)>,
 }
 
-impl<T, Currency, GetConfirmationFee, RootAccount>
+impl<T, I, Currency, GetConfirmationFee, RootAccount>
 	MessageDeliveryAndDispatchPayment<T::AccountId, Currency::Balance>
-	for InstantCurrencyPayments<T, Currency, GetConfirmationFee, RootAccount>
+	for InstantCurrencyPayments<T, I, Currency, GetConfirmationFee, RootAccount>
 where
-	T: frame_system::Config,
-	Currency: CurrencyT<T::AccountId>,
+	T: frame_system::Config + crate::Config<I>,
+	I: 'static,
+	Currency: CurrencyT<T::AccountId, Balance = T::OutboundMessageFee>,
 	Currency::Balance: From<MessageNonce>,
 	GetConfirmationFee: Get<Currency::Balance>,
 	RootAccount: Get<Option<T::AccountId>>,
@@ -86,19 +89,55 @@ where
 	}
 
 	fn pay_relayers_rewards(
+		lane_id: LaneId,
+		messages_relayers: VecDeque<UnrewardedRelayer<T::AccountId>>,
 		confirmation_relayer: &T::AccountId,
-		relayers_rewards: RelayersRewards<T::AccountId, Currency::Balance>,
+		received_range: &RangeInclusive<MessageNonce>,
 		relayer_fund_account: &T::AccountId,
 	) {
-		pay_relayers_rewards::<Currency, _>(
-			confirmation_relayer,
-			relayers_rewards,
-			relayer_fund_account,
-			GetConfirmationFee::get(),
-		);
+		let relayers_rewards =
+			cal_relayers_rewards::<T, I>(lane_id, messages_relayers, received_range);
+		if !relayers_rewards.is_empty() {
+			pay_relayers_rewards::<Currency, _>(
+				confirmation_relayer,
+				relayers_rewards,
+				relayer_fund_account,
+				GetConfirmationFee::get(),
+			);
+		}
 	}
 }
 
+/// Calculate the relayers rewards
+pub(crate) fn cal_relayers_rewards<T, I>(
+	lane_id: LaneId,
+	messages_relayers: VecDeque<UnrewardedRelayer<T::AccountId>>,
+	received_range: &RangeInclusive<MessageNonce>,
+) -> RelayersRewards<T::AccountId, T::OutboundMessageFee>
+where
+	T: frame_system::Config + crate::Config<I>,
+	I: 'static,
+{
+	// remember to reward relayers that have delivered messages
+	// this loop is bounded by `T::MaxUnrewardedRelayerEntriesAtInboundLane` on the bridged chain
+	let mut relayers_rewards: RelayersRewards<_, T::OutboundMessageFee> = RelayersRewards::new();
+	for entry in messages_relayers {
+		let nonce_begin = sp_std::cmp::max(entry.messages.begin, *received_range.start());
+		let nonce_end = sp_std::cmp::min(entry.messages.end, *received_range.end());
+
+		// 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 {
+			let message_data = OutboundMessages::<T, I>::get(MessageKey { lane_id, nonce })
+				.expect("message was just confirmed; we never prune unconfirmed messages; qed");
+			relayer_reward.reward = relayer_reward.reward.saturating_add(&message_data.fee);
+			relayer_reward.messages += 1;
+		}
+	}
+	relayers_rewards
+}
+
 /// Pay rewards to given relayers, optionally rewarding confirmation relayer.
 fn pay_relayers_rewards<Currency, AccountId>(
 	confirmation_relayer: &AccountId,

bridges/modules/messages/src/lib.rs

@@ -51,7 +51,7 @@ use crate::{
 use bp_messages::{
 	source_chain::{
 		LaneMessageVerifier, MessageDeliveryAndDispatchPayment, OnDeliveryConfirmed,
-		OnMessageAccepted, RelayersRewards, TargetHeaderChain,
+		OnMessageAccepted, TargetHeaderChain,
 	},
 	target_chain::{
 		DispatchMessage, MessageDispatch, ProvedLaneMessages, ProvedMessages, SourceHeaderChain,
@@ -143,7 +143,13 @@ pub mod pallet {
 		/// Payload type of outbound messages. This payload is dispatched on the bridged chain.
 		type OutboundPayload: Parameter + Size;
 		/// Message fee type of outbound messages. This fee is paid on this chain.
-		type OutboundMessageFee: Default + From<u64> + PartialOrd + Parameter + SaturatingAdd + Zero;
+		type OutboundMessageFee: Default
+			+ From<u64>
+			+ PartialOrd
+			+ Parameter
+			+ SaturatingAdd
+			+ Zero
+			+ Copy;
 
 		/// Payload type of inbound messages. This payload is dispatched on this chain.
 		type InboundPayload: Decode;
@@ -548,8 +554,6 @@ pub mod pallet {
 
 			// mark messages as delivered
 			let mut lane = outbound_lane::<T, I>(lane_id);
-			let mut relayers_rewards: RelayersRewards<_, T::OutboundMessageFee> =
-				RelayersRewards::new();
 			let last_delivered_nonce = lane_data.last_delivered_nonce();
 			let confirmed_messages = match lane.confirm_delivery(
 				relayers_state.total_messages,
@@ -620,40 +624,14 @@ pub mod pallet {
 				let received_range = confirmed_messages.begin..=confirmed_messages.end;
 				Self::deposit_event(Event::MessagesDelivered(lane_id, confirmed_messages));
 
-				// remember to reward relayers that have delivered messages
-				// this loop is bounded by `T::MaxUnrewardedRelayerEntriesAtInboundLane` on the
-				// bridged chain
-				for entry in lane_data.relayers {
-					let nonce_begin =
-						sp_std::cmp::max(entry.messages.begin, *received_range.start());
-					let nonce_end = sp_std::cmp::min(entry.messages.end, *received_range.end());
-
-					// 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 {
-						let message_data = OutboundMessages::<T, I>::get(MessageKey {
-							lane_id,
-							nonce,
-						})
-						.expect(
-							"message was just confirmed; we never prune unconfirmed messages; qed",
-						);
-						relayer_reward.reward =
-							relayer_reward.reward.saturating_add(&message_data.fee);
-						relayer_reward.messages += 1;
-					}
-				}
-			}
-
-			// if some new messages have been confirmed, reward relayers
-			if !relayers_rewards.is_empty() {
+				// if some new messages have been confirmed, reward relayers
 				let relayer_fund_account =
 					relayer_fund_account_id::<T::AccountId, T::AccountIdConverter>();
 				<T as Config<I>>::MessageDeliveryAndDispatchPayment::pay_relayers_rewards(
+					lane_id,
+					lane_data.relayers,
 					&confirmation_relayer,
-					relayers_rewards,
+					&received_range,
 					&relayer_fund_account,
 				);
 			}

bridges/modules/messages/src/mock.rs

@@ -17,13 +17,13 @@
 // From construct_runtime macro
 #![allow(clippy::from_over_into)]
 
-use crate::Config;
+use crate::{instant_payments::cal_relayers_rewards, Config};
 
 use bitvec::prelude::*;
 use bp_messages::{
 	source_chain::{
 		LaneMessageVerifier, MessageDeliveryAndDispatchPayment, OnDeliveryConfirmed,
-		OnMessageAccepted, RelayersRewards, Sender, TargetHeaderChain,
+		OnMessageAccepted, Sender, TargetHeaderChain,
 	},
 	target_chain::{
 		DispatchMessage, MessageDispatch, ProvedLaneMessages, ProvedMessages, SourceHeaderChain,
@@ -43,7 +43,10 @@ use sp_runtime::{
 	traits::{BlakeTwo256, IdentityLookup},
 	FixedU128, Perbill,
 };
-use std::collections::BTreeMap;
+use std::{
+	collections::{BTreeMap, VecDeque},
+	ops::RangeInclusive,
+};
 
 pub type AccountId = u64;
 pub type Balance = u64;
@@ -350,11 +353,15 @@ impl MessageDeliveryAndDispatchPayment<AccountId, TestMessageFee>
 	}
 
 	fn pay_relayers_rewards(
+		lane_id: LaneId,
+		message_relayers: VecDeque<UnrewardedRelayer<AccountId>>,
 		_confirmation_relayer: &AccountId,
-		relayers_rewards: RelayersRewards<AccountId, TestMessageFee>,
+		received_range: &RangeInclusive<MessageNonce>,
 		_relayer_fund_account: &AccountId,
 	) {
-		for (relayer, reward) in relayers_rewards {
+		let relayers_rewards =
+			cal_relayers_rewards::<TestRuntime, ()>(lane_id, message_relayers, received_range);
+		for (relayer, reward) in &relayers_rewards {
 			let key = (b":relayer-reward:", relayer, reward.reward).encode();
 			frame_support::storage::unhashed::put(&key, &true);
 		}

bridges/primitives/messages/src/source_chain.rs

@@ -18,9 +18,14 @@
 
 use crate::{DeliveredMessages, InboundLaneData, LaneId, MessageNonce, OutboundLaneData};
 
+use crate::UnrewardedRelayer;
 use bp_runtime::Size;
 use frame_support::{weights::Weight, Parameter, RuntimeDebug};
-use sp_std::{collections::btree_map::BTreeMap, fmt::Debug};
+use sp_std::{
+	collections::{btree_map::BTreeMap, vec_deque::VecDeque},
+	fmt::Debug,
+	ops::RangeInclusive,
+};
 
 /// The sender of the message on the source chain.
 pub type Sender<AccountId> = frame_system::RawOrigin<AccountId>;
@@ -122,8 +127,10 @@ pub trait MessageDeliveryAndDispatchPayment<AccountId, Balance> {
 	/// The implementation may also choose to pay reward to the `confirmation_relayer`, which is
 	/// a relayer that has submitted delivery confirmation transaction.
 	fn pay_relayers_rewards(
+		lane_id: LaneId,
+		messages_relayers: VecDeque<UnrewardedRelayer<AccountId>>,
 		confirmation_relayer: &AccountId,
-		relayers_rewards: RelayersRewards<AccountId, Balance>,
+		received_range: &RangeInclusive<MessageNonce>,
 		relayer_fund_account: &AccountId,
 	);
 }
@@ -240,8 +247,10 @@ impl<AccountId, Balance> MessageDeliveryAndDispatchPayment<AccountId, Balance>
 	}
 
 	fn pay_relayers_rewards(
+		_lane_id: LaneId,
+		_messages_relayers: VecDeque<UnrewardedRelayer<AccountId>>,
 		_confirmation_relayer: &AccountId,
-		_relayers_rewards: RelayersRewards<AccountId, Balance>,
+		_received_range: &RangeInclusive<MessageNonce>,
 		_relayer_fund_account: &AccountId,
 	) {
 	}