Support dedicated lanes for pallets (#962)

* pass call origin to the message verifier

* is_outbound_lane_enabled -> is_message_accepted

* trait SenderOrigin

* only accept messages from token swap pallet to token swap lane

* tests for edge cases of pay_delivery_and_dispatch_fee

* fixed origin verification

* fmt

* fix benchmarks compilation

* fix TODO with None account and non-zero message fee (already covered by tests)

* revert cargo fmt changes temporarily

bridges/modules/messages/src/instant_payments.rs

@@ -22,7 +22,7 @@
 use crate::OutboundMessages;
 
 use bp_messages::{
-	source_chain::{MessageDeliveryAndDispatchPayment, RelayersRewards, Sender},
+	source_chain::{MessageDeliveryAndDispatchPayment, RelayersRewards, SenderOrigin},
 	LaneId, MessageKey, MessageNonce, UnrewardedRelayer,
 };
 use codec::Encode;
@@ -31,6 +31,10 @@ use num_traits::{SaturatingAdd, Zero};
 use sp_runtime::traits::Saturating;
 use sp_std::{collections::vec_deque::VecDeque, fmt::Debug, ops::RangeInclusive};
 
+/// Error that occurs when message fee is non-zero, but payer is not defined.
+const NON_ZERO_MESSAGE_FEE_CANT_BE_PAID_BY_NONE: &str =
+	"Non-zero message fee can't be paid by <None>";
+
 /// Instant message payments made in given currency.
 ///
 /// The balance is initially reserved in a special `relayers-fund` account, and transferred
@@ -44,42 +48,50 @@ use sp_std::{collections::vec_deque::VecDeque, fmt::Debug, ops::RangeInclusive};
 /// 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, I, Currency, GetConfirmationFee, RootAccount> {
-	_phantom: sp_std::marker::PhantomData<(T, I, Currency, GetConfirmationFee, RootAccount)>,
+pub struct InstantCurrencyPayments<T, I, Currency, GetConfirmationFee> {
+	_phantom: sp_std::marker::PhantomData<(T, I, Currency, GetConfirmationFee)>,
 }
 
-impl<T, I, Currency, GetConfirmationFee, RootAccount>
-	MessageDeliveryAndDispatchPayment<T::AccountId, Currency::Balance>
-	for InstantCurrencyPayments<T, I, Currency, GetConfirmationFee, RootAccount>
+impl<T, I, Currency, GetConfirmationFee>
+	MessageDeliveryAndDispatchPayment<T::Origin, T::AccountId, Currency::Balance>
+	for InstantCurrencyPayments<T, I, Currency, GetConfirmationFee>
 where
 	T: frame_system::Config + crate::Config<I>,
 	I: 'static,
+	T::Origin: SenderOrigin<T::AccountId>,
 	Currency: CurrencyT<T::AccountId, Balance = T::OutboundMessageFee>,
 	Currency::Balance: From<MessageNonce>,
 	GetConfirmationFee: Get<Currency::Balance>,
-	RootAccount: Get<Option<T::AccountId>>,
 {
 	type Error = &'static str;
 
 	fn pay_delivery_and_dispatch_fee(
-		submitter: &Sender<T::AccountId>,
+		submitter: &T::Origin,
 		fee: &Currency::Balance,
 		relayer_fund_account: &T::AccountId,
 	) -> Result<(), Self::Error> {
+		let submitter_account = match submitter.linked_account() {
+			Some(submitter_account) => submitter_account,
+			None if !fee.is_zero() => {
+				// if we'll accept some message that has declared that the `fee` has been paid but
+				// it isn't actually paid, then it'll lead to problems with delivery confirmation
+				// payments (see `pay_relayer_rewards` && `confirmation_relayer` in particular)
+				return Err(NON_ZERO_MESSAGE_FEE_CANT_BE_PAID_BY_NONE)
+			},
+			None => {
+				// message lane verifier has accepted the message before, so this message
+				// is unpaid **by design**
+				// => let's just do nothing
+				return Ok(())
+			},
+		};
+
 		if !frame_system::Pallet::<T>::account_exists(relayer_fund_account) {
 			return Err("The relayer fund account must exist for the message lanes pallet to work correctly.");
 		}
 
-		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,
+			&submitter_account,
 			relayer_fund_account,
 			*fee,
 			// it's fine for the submitter to go below Existential Deposit and die.
@@ -226,7 +238,9 @@ fn pay_relayer_reward<Currency, AccountId>(
 #[cfg(test)]
 mod tests {
 	use super::*;
-	use crate::mock::{run_test, AccountId as TestAccountId, Balance as TestBalance, TestRuntime};
+	use crate::mock::{
+		run_test, AccountId as TestAccountId, Balance as TestBalance, Origin, TestRuntime,
+	};
 	use bp_messages::source_chain::RelayerRewards;
 
 	type Balances = pallet_balances::Pallet<TestRuntime>;
@@ -245,6 +259,48 @@ mod tests {
 		.collect()
 	}
 
+	#[test]
+	fn pay_delivery_and_dispatch_fee_fails_on_non_zero_fee_and_unknown_payer() {
+		frame_support::parameter_types! {
+			const GetConfirmationFee: TestBalance = 0;
+		};
+
+		run_test(|| {
+			let result = InstantCurrencyPayments::<
+				TestRuntime,
+				(),
+				Balances,
+				GetConfirmationFee,
+			>::pay_delivery_and_dispatch_fee(
+				&Origin::root(),
+				&100,
+				&RELAYERS_FUND_ACCOUNT,
+			);
+			assert_eq!(result, Err(NON_ZERO_MESSAGE_FEE_CANT_BE_PAID_BY_NONE));
+		});
+	}
+
+	#[test]
+	fn pay_delivery_and_dispatch_succeeds_on_zero_fee_and_unknown_payer() {
+		frame_support::parameter_types! {
+			const GetConfirmationFee: TestBalance = 0;
+		};
+
+		run_test(|| {
+			let result = InstantCurrencyPayments::<
+				TestRuntime,
+				(),
+				Balances,
+				GetConfirmationFee,
+			>::pay_delivery_and_dispatch_fee(
+				&Origin::root(),
+				&0,
+				&RELAYERS_FUND_ACCOUNT,
+			);
+			assert!(result.is_ok());
+		});
+	}
+
 	#[test]
 	fn confirmation_relayer_is_rewarded_if_it_has_also_delivered_messages() {
 		run_test(|| {

bridges/modules/messages/src/lib.rs

@@ -170,12 +170,14 @@ pub mod pallet {
 		type TargetHeaderChain: TargetHeaderChain<Self::OutboundPayload, Self::AccountId>;
 		/// Message payload verifier.
 		type LaneMessageVerifier: LaneMessageVerifier<
+			Self::Origin,
 			Self::AccountId,
 			Self::OutboundPayload,
 			Self::OutboundMessageFee,
 		>;
 		/// Message delivery payment.
 		type MessageDeliveryAndDispatchPayment: MessageDeliveryAndDispatchPayment<
+			Self::Origin,
 			Self::AccountId,
 			Self::OutboundMessageFee,
 		>;
@@ -276,16 +278,12 @@ pub mod pallet {
 			payload: T::OutboundPayload,
 			delivery_and_dispatch_fee: T::OutboundMessageFee,
 		) -> DispatchResultWithPostInfo {
-			crate::send_message::<T, I>(
-				origin.into().map_err(|_| BadOrigin)?,
-				lane_id,
-				payload,
-				delivery_and_dispatch_fee,
+			crate::send_message::<T, I>(origin, lane_id, payload, delivery_and_dispatch_fee).map(
+				|sent_message| PostDispatchInfo {
+					actual_weight: Some(sent_message.weight),
+					pays_fee: Pays::Yes,
+				},
 			)
-			.map(|sent_message| PostDispatchInfo {
-				actual_weight: Some(sent_message.weight),
-				pays_fee: Pays::Yes,
-			})
 		}
 
 		/// Pay additional fee for the message.
@@ -313,17 +311,15 @@ pub mod pallet {
 			);
 
 			// withdraw additional fee from submitter
-			let submitter = origin.into().map_err(|_| BadOrigin)?;
 			T::MessageDeliveryAndDispatchPayment::pay_delivery_and_dispatch_fee(
-				&submitter,
+				&origin,
 				&additional_fee,
 				&relayer_fund_account_id::<T::AccountId, T::AccountIdConverter>(),
 			)
 			.map_err(|err| {
 				log::trace!(
 					target: "runtime::bridge-messages",
-					"Submitter {:?} can't pay additional fee {:?} for the message {:?}/{:?} to {:?}: {:?}",
-					submitter,
+					"Submitter can't pay additional fee {:?} for the message {:?}/{:?} to {:?}: {:?}",
 					additional_fee,
 					lane_id,
 					nonce,
@@ -780,6 +776,7 @@ pub fn relayer_fund_account_id<AccountId, AccountIdConverter: Convert<H256, Acco
 
 impl<T, I>
 	bp_messages::source_chain::MessagesBridge<
+		T::Origin,
 		T::AccountId,
 		T::OutboundMessageFee,
 		T::OutboundPayload,
@@ -791,7 +788,7 @@ where
 	type Error = sp_runtime::DispatchErrorWithPostInfo<PostDispatchInfo>;
 
 	fn send_message(
-		sender: bp_messages::source_chain::Sender<T::AccountId>,
+		sender: T::Origin,
 		lane: LaneId,
 		message: T::OutboundPayload,
 		delivery_and_dispatch_fee: T::OutboundMessageFee,
@@ -802,7 +799,7 @@ where
 
 /// Function that actually sends message.
 fn send_message<T: Config<I>, I: 'static>(
-	submitter: bp_messages::source_chain::Sender<T::AccountId>,
+	submitter: T::Origin,
 	lane_id: LaneId,
 	payload: T::OutboundPayload,
 	delivery_and_dispatch_fee: T::OutboundMessageFee,
@@ -856,9 +853,8 @@ fn send_message<T: Config<I>, I: 'static>(
 	.map_err(|err| {
 		log::trace!(
 			target: "runtime::bridge-messages",
-			"Message to lane {:?} is rejected because submitter {:?} is unable to pay fee {:?}: {:?}",
+			"Message to lane {:?} is rejected because submitter is unable to pay fee {:?}: {:?}",
 			lane_id,
-			submitter,
 			delivery_and_dispatch_fee,
 			err,
 		);

bridges/modules/messages/src/mock.rs

@@ -23,7 +23,7 @@ use bitvec::prelude::*;
 use bp_messages::{
 	source_chain::{
 		LaneMessageVerifier, MessageDeliveryAndDispatchPayment, OnDeliveryConfirmed,
-		OnMessageAccepted, Sender, TargetHeaderChain,
+		OnMessageAccepted, SenderOrigin, TargetHeaderChain,
 	},
 	target_chain::{
 		DispatchMessage, MessageDispatch, ProvedLaneMessages, ProvedMessages, SourceHeaderChain,
@@ -194,6 +194,16 @@ impl Config for TestRuntime {
 	type BridgedChainId = TestBridgedChainId;
 }
 
+impl SenderOrigin<AccountId> for Origin {
+	fn linked_account(&self) -> Option<AccountId> {
+		match self.caller {
+			OriginCaller::system(frame_system::RawOrigin::Signed(ref submitter)) =>
+				Some(submitter.clone()),
+			_ => None,
+		}
+	}
+}
+
 impl Size for TestPayload {
 	fn size_hint(&self) -> u32 {
 		16 + self.extra.len() as u32
@@ -294,11 +304,13 @@ impl TargetHeaderChain<TestPayload, TestRelayer> for TestTargetHeaderChain {
 #[derive(Debug, Default)]
 pub struct TestLaneMessageVerifier;
 
-impl LaneMessageVerifier<AccountId, TestPayload, TestMessageFee> for TestLaneMessageVerifier {
+impl LaneMessageVerifier<Origin, AccountId, TestPayload, TestMessageFee>
+	for TestLaneMessageVerifier
+{
 	type Error = &'static str;
 
 	fn verify_message(
-		_submitter: &Sender<AccountId>,
+		_submitter: &Origin,
 		delivery_and_dispatch_fee: &TestMessageFee,
 		_lane: &LaneId,
 		_lane_outbound_data: &OutboundLaneData,
@@ -324,8 +336,8 @@ impl TestMessageDeliveryAndDispatchPayment {
 
 	/// Returns true if given fee has been paid by given submitter.
 	pub fn is_fee_paid(submitter: AccountId, fee: TestMessageFee) -> bool {
-		frame_support::storage::unhashed::get(b":message-fee:") ==
-			Some((Sender::Signed(submitter), fee))
+		let raw_origin: Result<frame_system::RawOrigin<_>, _> = Origin::signed(submitter).into();
+		frame_support::storage::unhashed::get(b":message-fee:") == Some((raw_origin.unwrap(), fee))
 	}
 
 	/// Returns true if given relayer has been rewarded with given balance. The reward-paid flag is
@@ -336,13 +348,13 @@ impl TestMessageDeliveryAndDispatchPayment {
 	}
 }
 
-impl MessageDeliveryAndDispatchPayment<AccountId, TestMessageFee>
+impl MessageDeliveryAndDispatchPayment<Origin, AccountId, TestMessageFee>
 	for TestMessageDeliveryAndDispatchPayment
 {
 	type Error = &'static str;
 
 	fn pay_delivery_and_dispatch_fee(
-		submitter: &Sender<AccountId>,
+		submitter: &Origin,
 		fee: &TestMessageFee,
 		_relayer_fund_account: &AccountId,
 	) -> Result<(), Self::Error> {
@@ -350,7 +362,8 @@ impl MessageDeliveryAndDispatchPayment<AccountId, TestMessageFee>
 			return Err(TEST_ERROR)
 		}
 
-		frame_support::storage::unhashed::put(b":message-fee:", &(submitter, fee));
+		let raw_origin: Result<frame_system::RawOrigin<_>, _> = submitter.clone().into();
+		frame_support::storage::unhashed::put(b":message-fee:", &(raw_origin.unwrap(), fee));
 		Ok(())
 	}