Reward delivery confirmation transaction submitter (aka confirmation relayer) (#599)

* reward confirmation relayer

* Update modules/message-lane/src/instant_payments.rs

Co-authored-by: Hernando Castano <HCastano@users.noreply.github.com>

* added some constants

* pallet_balances

* removed comment

* fix typo

Co-authored-by: Hernando Castano <HCastano@users.noreply.github.com>

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

@@ -315,6 +315,8 @@ parameter_types! {
 		bp_millau::MAX_UNREWARDED_RELAYER_ENTRIES_AT_INBOUND_LANE;
 	pub const MaxUnconfirmedMessagesAtInboundLane: bp_message_lane::MessageNonce =
 		bp_millau::MAX_UNCONFIRMED_MESSAGES_AT_INBOUND_LANE;
+	// TODO: https://github.com/paritytech/parity-bridges-common/pull/598
+	pub GetDeliveryConfirmationTransactionFee: Balance = 0;
 	pub const RootAccountForPayments: Option<AccountId> = None;
 }
 
@@ -340,6 +342,7 @@ impl pallet_message_lane::Config for Runtime {
 	type MessageDeliveryAndDispatchPayment = pallet_message_lane::instant_payments::InstantCurrencyPayments<
 		Runtime,
 		pallet_balances::Module<Runtime>,
+		GetDeliveryConfirmationTransactionFee,
 		RootAccountForPayments,
 	>;
 

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

@@ -422,6 +422,8 @@ parameter_types! {
 		bp_millau::MAX_UNREWARDED_RELAYER_ENTRIES_AT_INBOUND_LANE;
 	pub const MaxUnconfirmedMessagesAtInboundLane: bp_message_lane::MessageNonce =
 		bp_rialto::MAX_UNCONFIRMED_MESSAGES_AT_INBOUND_LANE;
+	// TODO: https://github.com/paritytech/parity-bridges-common/pull/598
+	pub GetDeliveryConfirmationTransactionFee: Balance = 0;
 	pub const RootAccountForPayments: Option<AccountId> = None;
 }
 
@@ -447,6 +449,7 @@ impl pallet_message_lane::Config for Runtime {
 	type MessageDeliveryAndDispatchPayment = pallet_message_lane::instant_payments::InstantCurrencyPayments<
 		Runtime,
 		pallet_balances::Module<Runtime>,
+		GetDeliveryConfirmationTransactionFee,
 		RootAccountForPayments,
 	>;
 

bridges/modules/message-lane/Cargo.toml

@@ -28,6 +28,7 @@ sp-std = { git = "https://github.com/paritytech/substrate.git", branch = "master
 [dev-dependencies]
 hex-literal = "0.3"
 sp-io = { git = "https://github.com/paritytech/substrate.git", branch = "master" }
+pallet-balances = { git = "https://github.com/paritytech/substrate.git", branch = "master" }
 
 [features]
 default = ["std"]

bridges/modules/message-lane/src/instant_payments.rs

@@ -19,28 +19,40 @@
 //! The payment is first transferred to a special `relayers-fund` account and only transferred
 //! to the actual relayer in case confirmation is received.
 
-use bp_message_lane::source_chain::{MessageDeliveryAndDispatchPayment, Sender};
+use bp_message_lane::{
+	source_chain::{MessageDeliveryAndDispatchPayment, RelayersRewards, Sender},
+	MessageNonce,
+};
+use codec::Encode;
 use frame_support::traits::{Currency as CurrencyT, ExistenceRequirement, Get};
+use num_traits::Zero;
+use sp_runtime::traits::Saturating;
+use sp_std::fmt::Debug;
 
 /// Instant message payments made in given currency.
 ///
 /// The balance is initally reserved in a special `relayers-fund` account, and transferred
 /// to the relayer when message delivery is confirmed.
 ///
+/// Additionaly, confirmation transaction submitter (`confirmation_relayer`) is reimbursed
+/// with the confirmation rewards (part of message fee, reserved to pay for delivery confirmation).
+///
 /// NOTE The `relayers-fund` account must always exist i.e. be over Existential Deposit (ED; the
 /// pallet enforces that) to make sure that even if the message cost is below ED it is still payed
 /// 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: frame_system::Config, Currency, RootAccount> {
-	_phantom: sp_std::marker::PhantomData<(T, Currency, RootAccount)>,
+pub struct InstantCurrencyPayments<T, Currency, GetConfirmationFee, RootAccount> {
+	_phantom: sp_std::marker::PhantomData<(T, Currency, GetConfirmationFee, RootAccount)>,
 }
 
-impl<T, Currency, RootAccount> MessageDeliveryAndDispatchPayment<T::AccountId, Currency::Balance>
-	for InstantCurrencyPayments<T, Currency, RootAccount>
+impl<T, Currency, GetConfirmationFee, RootAccount> MessageDeliveryAndDispatchPayment<T::AccountId, Currency::Balance>
+	for InstantCurrencyPayments<T, Currency, GetConfirmationFee, RootAccount>
 where
 	T: frame_system::Config,
 	Currency: CurrencyT<T::AccountId>,
+	Currency::Balance: From<MessageNonce>,
+	GetConfirmationFee: Get<Currency::Balance>,
 	RootAccount: Get<Option<T::AccountId>>,
 {
 	type Error = &'static str;
@@ -77,35 +89,163 @@ where
 		.map_err(Into::into)
 	}
 
-	fn pay_relayer_reward(
-		_confirmation_relayer: &T::AccountId,
-		relayer: &T::AccountId,
-		reward: &Currency::Balance,
+	fn pay_relayers_rewards(
+		confirmation_relayer: &T::AccountId,
+		relayers_rewards: RelayersRewards<T::AccountId, Currency::Balance>,
 		relayer_fund_account: &T::AccountId,
 	) {
-		let pay_result = Currency::transfer(
-			&relayer_fund_account,
-			relayer,
-			*reward,
-			// the relayer fund account must stay above ED (needs to be pre-funded)
-			ExistenceRequirement::KeepAlive,
+		pay_relayers_rewards::<Currency, _>(
+			confirmation_relayer,
+			relayers_rewards,
+			relayer_fund_account,
+			GetConfirmationFee::get(),
 		);
-
-		// we can't actually do anything here, because rewards are paid as a part of unrelated transaction
-		match pay_result {
-			Ok(_) => frame_support::debug::trace!(
-				target: "runtime",
-				"Rewarded relayer {:?} with {:?}",
-				relayer,
-				reward,
-			),
-			Err(error) => frame_support::debug::trace!(
-				target: "runtime",
-				"Failed to pay relayer {:?} reward {:?}: {:?}",
-				relayer,
-				reward,
-				error,
-			),
-		}
+	}
+}
+
+/// Pay rewards to given relayers, optionally rewarding confirmation relayer.
+fn pay_relayers_rewards<Currency, AccountId>(
+	confirmation_relayer: &AccountId,
+	relayers_rewards: RelayersRewards<AccountId, Currency::Balance>,
+	relayer_fund_account: &AccountId,
+	confirmation_fee: Currency::Balance,
+) where
+	AccountId: Debug + Default + Encode + PartialEq,
+	Currency: CurrencyT<AccountId>,
+	Currency::Balance: From<u64>,
+{
+	// reward every relayer except `confirmation_relayer`
+	let mut confirmation_relayer_reward = Currency::Balance::zero();
+	for (relayer, reward) in relayers_rewards {
+		let mut relayer_reward = reward.reward;
+
+		if relayer != *confirmation_relayer {
+			// If delivery confirmation is submitted by other relayer, let's deduct confirmation fee
+			// from relayer reward.
+			//
+			// 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 = confirmation_fee.saturating_mul(reward.messages.into());
+			if confirmation_reward > relayer_reward {
+				confirmation_reward = relayer_reward;
+			}
+			relayer_reward = relayer_reward.saturating_sub(confirmation_reward);
+			confirmation_relayer_reward = confirmation_relayer_reward.saturating_add(confirmation_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;
+		}
+
+		pay_relayer_reward::<Currency, _>(relayer_fund_account, &relayer, relayer_reward);
+	}
+
+	// finally - pay reward to confirmation relayer
+	pay_relayer_reward::<Currency, _>(relayer_fund_account, confirmation_relayer, confirmation_relayer_reward);
+}
+
+/// Transfer funds from relayers fund account to given relayer.
+fn pay_relayer_reward<Currency, AccountId>(
+	relayer_fund_account: &AccountId,
+	relayer_account: &AccountId,
+	reward: Currency::Balance,
+) where
+	AccountId: Debug,
+	Currency: CurrencyT<AccountId>,
+{
+	if reward.is_zero() {
+		return;
+	}
+
+	let pay_result = Currency::transfer(
+		relayer_fund_account,
+		relayer_account,
+		reward,
+		// the relayer fund account must stay above ED (needs to be pre-funded)
+		ExistenceRequirement::KeepAlive,
+	);
+
+	match pay_result {
+		Ok(_) => frame_support::debug::trace!(
+			target: "runtime",
+			"Rewarded relayer {:?} with {:?}",
+			relayer_account,
+			reward,
+		),
+		Err(error) => frame_support::debug::trace!(
+			target: "runtime",
+			"Failed to pay relayer {:?} reward {:?}: {:?}",
+			relayer_account,
+			reward,
+			error,
+		),
+	}
+}
+
+#[cfg(test)]
+mod tests {
+	use super::*;
+	use crate::mock::{run_test, AccountId as TestAccountId, Balance as TestBalance, TestRuntime};
+	use bp_message_lane::source_chain::RelayerRewards;
+
+	type Balances = pallet_balances::Module<TestRuntime>;
+
+	const RELAYER_1: TestAccountId = 1;
+	const RELAYER_2: TestAccountId = 2;
+	const RELAYER_3: TestAccountId = 3;
+	const RELAYERS_FUND_ACCOUNT: TestAccountId = crate::mock::ENDOWED_ACCOUNT;
+
+	fn relayers_rewards() -> RelayersRewards<TestAccountId, TestBalance> {
+		vec![
+			(
+				RELAYER_1,
+				RelayerRewards {
+					reward: 100,
+					messages: 2,
+				},
+			),
+			(
+				RELAYER_2,
+				RelayerRewards {
+					reward: 100,
+					messages: 3,
+				},
+			),
+		]
+		.into_iter()
+		.collect()
+	}
+
+	#[test]
+	fn confirmation_relayer_is_rewarded_if_it_has_also_delivered_messages() {
+		run_test(|| {
+			pay_relayers_rewards::<Balances, _>(&RELAYER_2, relayers_rewards(), &RELAYERS_FUND_ACCOUNT, 10);
+
+			assert_eq!(Balances::free_balance(&RELAYER_1), 80);
+			assert_eq!(Balances::free_balance(&RELAYER_2), 120);
+		});
+	}
+
+	#[test]
+	fn confirmation_relayer_is_rewarded_if_it_has_not_delivered_any_delivered_messages() {
+		run_test(|| {
+			pay_relayers_rewards::<Balances, _>(&RELAYER_3, relayers_rewards(), &RELAYERS_FUND_ACCOUNT, 10);
+
+			assert_eq!(Balances::free_balance(&RELAYER_1), 80);
+			assert_eq!(Balances::free_balance(&RELAYER_2), 70);
+			assert_eq!(Balances::free_balance(&RELAYER_3), 50);
+		});
+	}
+
+	#[test]
+	fn only_confirmation_relayer_is_rewarded_if_confirmation_fee_has_significantly_increased() {
+		run_test(|| {
+			pay_relayers_rewards::<Balances, _>(&RELAYER_3, relayers_rewards(), &RELAYERS_FUND_ACCOUNT, 1000);
+
+			assert_eq!(Balances::free_balance(&RELAYER_1), 0);
+			assert_eq!(Balances::free_balance(&RELAYER_2), 0);
+			assert_eq!(Balances::free_balance(&RELAYER_3), 200);
+		});
 	}
 }

bridges/modules/message-lane/src/lib.rs

@@ -41,7 +41,7 @@ use crate::inbound_lane::{InboundLane, InboundLaneStorage};
 use crate::outbound_lane::{OutboundLane, OutboundLaneStorage};
 
 use bp_message_lane::{
-	source_chain::{LaneMessageVerifier, MessageDeliveryAndDispatchPayment, TargetHeaderChain},
+	source_chain::{LaneMessageVerifier, MessageDeliveryAndDispatchPayment, RelayersRewards, TargetHeaderChain},
 	target_chain::{DispatchMessage, MessageDispatch, ProvedLaneMessages, ProvedMessages, SourceHeaderChain},
 	total_unrewarded_messages, InboundLaneData, LaneId, MessageData, MessageKey, MessageNonce, MessagePayload,
 	OutboundLaneData, UnrewardedRelayersState,
@@ -106,7 +106,7 @@ pub trait Config<I = DefaultInstance>: frame_system::Config {
 	/// 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: From<u32> + Parameter + SaturatingAdd + Zero;
+	type OutboundMessageFee: Default + From<u32> + Parameter + SaturatingAdd + Zero;
 
 	/// Payload type of inbound messages. This payload is dispatched on this chain.
 	type InboundPayload: Decode;
@@ -464,40 +464,42 @@ decl_module! {
 
 			// 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 received_range = lane.confirm_delivery(last_delivered_nonce);
 			if let Some(received_range) = received_range {
 				Self::deposit_event(RawEvent::MessagesDelivered(lane_id, received_range.0, received_range.1));
 
-				// reward relayers that have delivered messages
+				// remember to reward relayers that have delivered messages
 				// this loop is bounded by `T::MaxUnrewardedRelayerEntriesAtInboundLane` on the bridged chain
-				let relayer_fund_account = Self::relayer_fund_account_id();
 				for (nonce_low, nonce_high, relayer) in lane_data.relayers {
 					let nonce_begin = sp_std::cmp::max(nonce_low, received_range.0);
 					let nonce_end = sp_std::cmp::min(nonce_high, received_range.1);
 
 					// 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_fee: T::OutboundMessageFee = Zero::zero();
+					let mut relayer_reward = relayers_rewards.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_fee = relayer_fee.saturating_add(&message_data.fee);
-					}
-
-					if !relayer_fee.is_zero() {
-						<T as Config<I>>::MessageDeliveryAndDispatchPayment::pay_relayer_reward(
-							&confirmation_relayer,
-							&relayer,
-							&relayer_fee,
-							&relayer_fund_account,
-						);
+						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() {
+				let relayer_fund_account = Self::relayer_fund_account_id();
+				<T as Config<I>>::MessageDeliveryAndDispatchPayment::pay_relayers_rewards(
+					&confirmation_relayer,
+					relayers_rewards,
+					&relayer_fund_account,
+				);
+			}
+
 			frame_support::debug::trace!(
 				"Received messages delivery proof up to (and including) {} at lane {:?}",
 				last_delivered_nonce,

bridges/modules/message-lane/src/mock.rs

@@ -17,7 +17,9 @@
 use crate::Config;
 
 use bp_message_lane::{
-	source_chain::{LaneMessageVerifier, MessageDeliveryAndDispatchPayment, Sender, TargetHeaderChain},
+	source_chain::{
+		LaneMessageVerifier, MessageDeliveryAndDispatchPayment, RelayersRewards, Sender, TargetHeaderChain,
+	},
 	target_chain::{DispatchMessage, MessageDispatch, ProvedLaneMessages, ProvedMessages, SourceHeaderChain},
 	InboundLaneData, LaneId, Message, MessageData, MessageKey, MessageNonce,
 };
@@ -33,6 +35,7 @@ use sp_runtime::{
 use std::collections::BTreeMap;
 
 pub type AccountId = u64;
+pub type Balance = u64;
 #[derive(Decode, Encode, Clone, Debug, PartialEq, Eq)]
 pub struct TestPayload(pub u64, pub Weight);
 pub type TestMessageFee = u64;
@@ -56,6 +59,7 @@ mod message_lane {
 impl_outer_event! {
 	pub enum TestEvent for TestRuntime {
 		frame_system<T>,
+		pallet_balances<T>,
 		message_lane<T>,
 	}
 }
@@ -85,7 +89,7 @@ impl frame_system::Config for TestRuntime {
 	type BlockHashCount = BlockHashCount;
 	type Version = ();
 	type PalletInfo = ();
-	type AccountData = ();
+	type AccountData = pallet_balances::AccountData<Balance>;
 	type OnNewAccount = ();
 	type OnKilledAccount = ();
 	type BaseCallFilter = ();
@@ -96,6 +100,20 @@ impl frame_system::Config for TestRuntime {
 	type SS58Prefix = ();
 }
 
+parameter_types! {
+	pub const ExistentialDeposit: u64 = 1;
+}
+
+impl pallet_balances::Config for TestRuntime {
+	type MaxLocks = ();
+	type Balance = Balance;
+	type DustRemoval = ();
+	type Event = TestEvent;
+	type ExistentialDeposit = ExistentialDeposit;
+	type AccountStore = frame_system::Module<TestRuntime>;
+	type WeightInfo = ();
+}
+
 parameter_types! {
 	pub const MaxMessagesToPruneAtOnce: u64 = 10;
 	pub const MaxUnrewardedRelayerEntriesAtInboundLane: u64 = 16;
@@ -132,6 +150,9 @@ impl Size for TestPayload {
 	}
 }
 
+/// Account that has balance to use in tests.
+pub const ENDOWED_ACCOUNT: AccountId = 0xDEAD;
+
 /// Account id of test relayer.
 pub const TEST_RELAYER_A: AccountId = 100;
 
@@ -265,14 +286,15 @@ impl MessageDeliveryAndDispatchPayment<AccountId, TestMessageFee> for TestMessag
 		Ok(())
 	}
 
-	fn pay_relayer_reward(
+	fn pay_relayers_rewards(
 		_confirmation_relayer: &AccountId,
-		relayer: &AccountId,
-		fee: &TestMessageFee,
+		relayers_rewards: RelayersRewards<AccountId, TestMessageFee>,
 		_relayer_fund_account: &AccountId,
 	) {
-		let key = (b":relayer-reward:", relayer, fee).encode();
-		frame_support::storage::unhashed::put(&key, &true);
+		for (relayer, reward) in relayers_rewards {
+			let key = (b":relayer-reward:", relayer, reward.reward).encode();
+			frame_support::storage::unhashed::put(&key, &true);
+		}
 	}
 }
 
@@ -334,9 +356,14 @@ pub fn message_data(payload: TestPayload) -> MessageData<TestMessageFee> {
 
 /// Run message lane test.
 pub fn run_test<T>(test: impl FnOnce() -> T) -> T {
-	let t = frame_system::GenesisConfig::default()
+	let mut t = frame_system::GenesisConfig::default()
 		.build_storage::<TestRuntime>()
 		.unwrap();
+	pallet_balances::GenesisConfig::<TestRuntime> {
+		balances: vec![(ENDOWED_ACCOUNT, 1_000_000)],
+	}
+	.assimilate_storage(&mut t)
+	.unwrap();
 	let mut ext = sp_io::TestExternalities::new(t);
 	ext.execute_with(test)
 }

bridges/primitives/message-lane/src/source_chain.rs

@@ -16,14 +16,26 @@
 
 //! Primitives of message lane module, that are used on the source chain.
 
-use crate::{InboundLaneData, LaneId};
+use crate::{InboundLaneData, LaneId, MessageNonce};
 
-use frame_support::Parameter;
-use sp_std::fmt::Debug;
+use frame_support::{Parameter, RuntimeDebug};
+use sp_std::{collections::btree_map::BTreeMap, fmt::Debug};
 
 /// The sender of the message on the source chain.
 pub type Sender<AccountId> = frame_system::RawOrigin<AccountId>;
 
+/// Relayers rewards, grouped by relayer account id.
+pub type RelayersRewards<AccountId, Balance> = BTreeMap<AccountId, RelayerRewards<Balance>>;
+
+/// Single relayer rewards.
+#[derive(RuntimeDebug, Default)]
+pub struct RelayerRewards<Balance> {
+	/// Total rewards that are to be paid to the relayer.
+	pub reward: Balance,
+	/// Total number of messages relayed by this relayer.
+	pub messages: MessageNonce,
+}
+
 /// Target chain API. Used by source chain to verify target chain proofs.
 ///
 /// All implementations of this trait should only work with finalized data that
@@ -102,11 +114,13 @@ pub trait MessageDeliveryAndDispatchPayment<AccountId, Balance> {
 		relayer_fund_account: &AccountId,
 	) -> Result<(), Self::Error>;
 
-	/// Pay reward for delivering message to the given relayer account.
-	fn pay_relayer_reward(
+	/// Pay rewards for delivering messages to the given relayers.
+	///
+	/// 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(
 		confirmation_relayer: &AccountId,
-		relayer: &AccountId,
-		reward: &Balance,
+		relayers_rewards: RelayersRewards<AccountId, Balance>,
 		relayer_fund_account: &AccountId,
 	);
 

bridges/relays/utils/Cargo.toml

@@ -9,7 +9,7 @@ license = "GPL-3.0-or-later WITH Classpath-exception-2.0"
 ansi_term = "0.12"
 async-std = "1.6.5"
 backoff = "0.2"
-env_logger = "0.7.0"
+env_logger = "0.8.2"
 futures = "0.3.5"
 log = "0.4.11"
 num-traits = "0.2"