Relayer rewards improvements (#624)

* Document relayers fund existence and add root account.

* Introduce initialize method instead of assuming that relayer_fund_account is always required.

* cargo fmt --all

* Fix benchmarks.

* cargo fmt --all

* Fix docs for the relayer fund account.

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

@@ -312,6 +312,7 @@ 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;
+	pub const RootAccountForPayments: Option<AccountId> = None;
 }
 
 impl pallet_message_lane::Config for Runtime {
@@ -333,8 +334,11 @@ impl pallet_message_lane::Config for Runtime {
 
 	type TargetHeaderChain = crate::rialto_messages::Rialto;
 	type LaneMessageVerifier = crate::rialto_messages::ToRialtoMessageVerifier;
-	type MessageDeliveryAndDispatchPayment =
-		pallet_message_lane::instant_payments::InstantCurrencyPayments<AccountId, pallet_balances::Module<Runtime>>;
+	type MessageDeliveryAndDispatchPayment = pallet_message_lane::instant_payments::InstantCurrencyPayments<
+		Runtime,
+		pallet_balances::Module<Runtime>,
+		RootAccountForPayments,
+	>;
 
 	type SourceHeaderChain = crate::rialto_messages::Rialto;
 	type MessageDispatch = crate::rialto_messages::FromRialtoMessageDispatch;

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

@@ -419,6 +419,7 @@ 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;
+	pub const RootAccountForPayments: Option<AccountId> = None;
 }
 
 pub(crate) type WithMillauMessageLaneInstance = pallet_message_lane::DefaultInstance;
@@ -440,8 +441,11 @@ impl pallet_message_lane::Config for Runtime {
 
 	type TargetHeaderChain = crate::millau_messages::Millau;
 	type LaneMessageVerifier = crate::millau_messages::ToMillauMessageVerifier;
-	type MessageDeliveryAndDispatchPayment =
-		pallet_message_lane::instant_payments::InstantCurrencyPayments<AccountId, pallet_balances::Module<Runtime>>;
+	type MessageDeliveryAndDispatchPayment = pallet_message_lane::instant_payments::InstantCurrencyPayments<
+		Runtime,
+		pallet_balances::Module<Runtime>,
+		RootAccountForPayments,
+	>;
 
 	type SourceHeaderChain = crate::millau_messages::Millau;
 	type MessageDispatch = crate::millau_messages::FromMillauMessageDispatch;

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

@@ -16,9 +16,7 @@
 
 //! Message lane pallet benchmarking.
 
-use crate::{
-	inbound_lane::InboundLaneStorage, inbound_lane_storage, outbound_lane, relayer_fund_account_id, Call, Instance,
-};
+use crate::{inbound_lane::InboundLaneStorage, inbound_lane_storage, outbound_lane, Call, Instance};
 
 use bp_message_lane::{
 	source_chain::TargetHeaderChain, target_chain::SourceHeaderChain, InboundLaneData, LaneId, MessageData,
@@ -224,7 +222,7 @@ benchmarks_instance! {
 	//
 	// This is base benchmark for all other confirmations delivery benchmarks.
 	receive_delivery_proof_for_single_message {
-		let relayers_fund_id = relayer_fund_account_id::<T, I>();
+		let relayers_fund_id = crate::Module::<T, I>::relayer_fund_account_id();
 		let relayer_id: T::AccountId = account("relayer", 0, SEED);
 		let relayer_balance = T::account_balance(&relayer_id);
 		T::endow_account(&relayers_fund_id);
@@ -261,7 +259,7 @@ benchmarks_instance! {
 	// as `weight(receive_delivery_proof_for_two_messages_by_single_relayer)
 	//   - weight(receive_delivery_proof_for_single_message)`.
 	receive_delivery_proof_for_two_messages_by_single_relayer {
-		let relayers_fund_id = relayer_fund_account_id::<T, I>();
+		let relayers_fund_id = crate::Module::<T, I>::relayer_fund_account_id();
 		let relayer_id: T::AccountId = account("relayer", 0, SEED);
 		let relayer_balance = T::account_balance(&relayer_id);
 		T::endow_account(&relayers_fund_id);
@@ -299,7 +297,7 @@ benchmarks_instance! {
 	// as `weight(receive_delivery_proof_for_two_messages_by_two_relayers)
 	//   - weight(receive_delivery_proof_for_two_messages_by_single_relayer)`.
 	receive_delivery_proof_for_two_messages_by_two_relayers {
-		let relayers_fund_id = relayer_fund_account_id::<T, I>();
+		let relayers_fund_id = crate::Module::<T, I>::relayer_fund_account_id();
 		let relayer1_id: T::AccountId = account("relayer1", 1, SEED);
 		let relayer1_balance = T::account_balance(&relayer1_id);
 		let relayer2_id: T::AccountId = account("relayer2", 2, SEED);
@@ -431,7 +429,7 @@ benchmarks_instance! {
 			.try_into()
 			.expect("Value of MaxUnrewardedRelayerEntriesAtInboundLane is too large");
 
-		let relayers_fund_id = relayer_fund_account_id::<T, I>();
+		let relayers_fund_id = crate::Module::<T, I>::relayer_fund_account_id();
 		let relayer_id: T::AccountId = account("relayer", 0, SEED);
 		let relayer_balance = T::account_balance(&relayer_id);
 		T::endow_account(&relayers_fund_id);
@@ -470,7 +468,7 @@ benchmarks_instance! {
 			.try_into()
 			.expect("Value of MaxUnconfirmedMessagesAtInboundLane is too large ");
 
-		let relayers_fund_id = relayer_fund_account_id::<T, I>();
+		let relayers_fund_id = crate::Module::<T, I>::relayer_fund_account_id();
 		let confirmation_relayer_id = account("relayer", 0, SEED);
 		let relayers: BTreeMap<T::AccountId, T::OutboundMessageFee> = (1..=i)
 			.map(|j| {

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

@@ -15,57 +15,80 @@
 // along with Parity Bridges Common.  If not, see <http://www.gnu.org/licenses/>.
 
 //! Implementation of `MessageDeliveryAndDispatchPayment` trait on top of `Currency` trait.
-//! All payments are instant.
+//!
+//! 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 codec::Encode;
-use frame_support::traits::{Currency as CurrencyT, ExistenceRequirement};
-use sp_std::fmt::Debug;
+use frame_support::traits::{Currency as CurrencyT, ExistenceRequirement, Get};
 
-/// Instant message payments made in given currency. Until claimed, fee is stored in special
-/// 'relayers-fund' account.
-pub struct InstantCurrencyPayments<AccountId, Currency> {
-	_phantom: sp_std::marker::PhantomData<(AccountId, Currency)>,
+/// 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.
+///
+/// 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)>,
 }
 
-impl<AccountId, Currency> MessageDeliveryAndDispatchPayment<AccountId, Currency::Balance>
-	for InstantCurrencyPayments<AccountId, Currency>
+impl<T, Currency, RootAccount> MessageDeliveryAndDispatchPayment<T::AccountId, Currency::Balance>
+	for InstantCurrencyPayments<T, Currency, RootAccount>
 where
-	Currency: CurrencyT<AccountId>,
-	AccountId: Debug + Default + Encode,
+	T: frame_system::Config,
+	Currency: CurrencyT<T::AccountId>,
+	RootAccount: Get<Option<T::AccountId>>,
 {
 	type Error = &'static str;
 
+	fn initialize(relayer_fund_account: &T::AccountId) -> usize {
+		assert!(
+			frame_system::Module::<T>::account_exists(relayer_fund_account),
+			"The relayer fund account ({:?}) must exist for the message lanes pallet to work correctly.",
+			relayer_fund_account,
+		);
+		1
+	}
+
 	fn pay_delivery_and_dispatch_fee(
-		submitter: &Sender<AccountId>,
+		submitter: &Sender<T::AccountId>,
 		fee: &Currency::Balance,
-		relayer_fund_account: &AccountId,
+		relayer_fund_account: &T::AccountId,
 	) -> Result<(), Self::Error> {
-		match submitter {
-			Sender::Signed(submitter) => {
-				Currency::transfer(submitter, relayer_fund_account, *fee, ExistenceRequirement::AllowDeath)
-					.map_err(Into::into)
-			}
-			Sender::Root => {
-				Err("Sending messages from Root account is not supported yet. See GitHub issue #559 for more.")
-			}
-			Sender::None => {
-				Err("Sending messages from None account is not supported yet. See GitHub issue #559 for more.")
-			}
-		}
+		let root_account = RootAccount::get();
+		let account = match submitter {
+			Sender::Signed(submitter) => submitter,
+			Sender::Root | Sender::None => root_account
+				.as_ref()
+				.ok_or("Sending messages using Root or None origin is disallowed.")?,
+		};
+
+		Currency::transfer(
+			account,
+			relayer_fund_account,
+			*fee,
+			// it's fine for the submitter to go below Existential Deposit and die.
+			ExistenceRequirement::AllowDeath,
+		)
+		.map_err(Into::into)
 	}
 
 	fn pay_relayer_reward(
-		_confirmation_relayer: &AccountId,
-		relayer: &AccountId,
+		_confirmation_relayer: &T::AccountId,
+		relayer: &T::AccountId,
 		reward: &Currency::Balance,
-		relayer_fund_account: &AccountId,
+		relayer_fund_account: &T::AccountId,
 	) {
 		let pay_result = Currency::transfer(
 			&relayer_fund_account,
 			relayer,
 			*reward,
-			ExistenceRequirement::AllowDeath,
+			// the relayer fund account must stay above ED (needs to be pre-funded)
+			ExistenceRequirement::KeepAlive,
 		);
 
 		// we can't actually do anything here, because rewards are paid as a part of unrelated transaction

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

@@ -214,6 +214,14 @@ decl_module! {
 		/// Deposit one of this module's events by using the default implementation.
 		fn deposit_event() = default;
 
+		/// Ensure runtime invariants.
+		fn on_runtime_upgrade() -> Weight {
+			let reads = T::MessageDeliveryAndDispatchPayment::initialize(
+				&Self::relayer_fund_account_id()
+			);
+			T::DbWeight::get().reads(reads as u64)
+		}
+
 		/// Change `ModuleOwner`.
 		///
 		/// May only be called either by root, or by `ModuleOwner`.
@@ -298,7 +306,7 @@ decl_module! {
 			T::MessageDeliveryAndDispatchPayment::pay_delivery_and_dispatch_fee(
 				&submitter,
 				&delivery_and_dispatch_fee,
-				&relayer_fund_account_id::<T, I>(),
+				&Self::relayer_fund_account_id(),
 			).map_err(|err| {
 				frame_support::debug::trace!(
 					"Message to lane {:?} is rejected because submitter {:?} is unable to pay fee {:?}: {:?}",
@@ -463,7 +471,7 @@ decl_module! {
 
 				// reward relayers that have delivered messages
 				// this loop is bounded by `T::MaxUnrewardedRelayerEntriesAtInboundLane` on the bridged chain
-				let relayer_fund_account = relayer_fund_account_id::<T, I>();
+				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);
@@ -538,6 +546,17 @@ impl<T: Config<I>, I: Instance> Module<T, I> {
 			total_messages: total_unrewarded_messages(&relayers),
 		}
 	}
+
+	/// AccountId of the shared relayer fund account.
+	///
+	/// This account is passed to `MessageDeliveryAndDispatchPayment` trait, and depending
+	/// on the implementation it can be used to store relayers rewards.
+	/// See [InstantCurrencyPayments] for a concrete implementation.
+	pub fn relayer_fund_account_id() -> T::AccountId {
+		use sp_runtime::traits::Convert;
+		let encoded_id = bp_runtime::derive_relayer_fund_account_id(bp_runtime::NO_INSTANCE_ID);
+		T::AccountIdConverter::convert(encoded_id)
+	}
 }
 
 /// Getting storage keys for messages and lanes states. These keys are normally used when building
@@ -730,16 +749,6 @@ fn verify_and_decode_messages_proof<Chain: SourceHeaderChain<Fee>, Fee, Dispatch
 	})
 }
 
-/// AccountId of the shared relayer fund account.
-///
-/// This account stores all the fees paid by submitters. Relayers are able to claim these
-/// funds as at their convenience.
-fn relayer_fund_account_id<T: Config<I>, I: Instance>() -> T::AccountId {
-	use sp_runtime::traits::Convert;
-	let encoded_id = bp_runtime::derive_relayer_fund_account_id(bp_runtime::NO_INSTANCE_ID);
-	T::AccountIdConverter::convert(encoded_id)
-}
-
 #[cfg(test)]
 mod tests {
 	use super::*;

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

@@ -109,4 +109,12 @@ pub trait MessageDeliveryAndDispatchPayment<AccountId, Balance> {
 		reward: &Balance,
 		relayer_fund_account: &AccountId,
 	);
+
+	/// Perform some initialization in externalities-provided environment.
+	///
+	/// For instance you may ensure that particular required accounts or storage items are present.
+	/// Returns the number of storage reads performed.
+	fn initialize(_relayer_fund_account: &AccountId) -> usize {
+		0
+	}
 }