diff --git a/bin/millau/runtime/src/lib.rs b/bin/millau/runtime/src/lib.rs index 4913043373..5b0308fb0c 100644 --- a/bin/millau/runtime/src/lib.rs +++ b/bin/millau/runtime/src/lib.rs @@ -376,13 +376,14 @@ impl pallet_bridge_messages::Config for Runtime { type AccountIdConverter = bp_millau::AccountIdConverter; + type SenderOrigin = Origin; type TargetHeaderChain = crate::rialto_messages::Rialto; type LaneMessageVerifier = crate::rialto_messages::ToRialtoMessageVerifier; type MessageDeliveryAndDispatchPayment = pallet_bridge_messages::instant_payments::InstantCurrencyPayments< Runtime, + WithRialtoMessagesInstance, pallet_balances::Pallet, GetDeliveryConfirmationTransactionFee, - RootAccountForPayments, >; type OnDeliveryConfirmed = (); diff --git a/bin/millau/runtime/src/rialto_messages.rs b/bin/millau/runtime/src/rialto_messages.rs index 98fe27ff03..7d8774f4c8 100644 --- a/bin/millau/runtime/src/rialto_messages.rs +++ b/bin/millau/runtime/src/rialto_messages.rs @@ -19,7 +19,7 @@ use crate::Runtime; use bp_messages::{ - source_chain::TargetHeaderChain, + source_chain::{SenderOrigin, TargetHeaderChain}, target_chain::{ProvedMessages, SourceHeaderChain}, InboundLaneData, LaneId, Message, MessageNonce, Parameter as MessagesParameter, }; @@ -104,8 +104,8 @@ impl messages::ThisChainWithMessages for Millau { type Origin = crate::Origin; type Call = crate::Call; - fn is_message_accepted(_send_origin: &Self::Origin, lane: &LaneId) -> bool { - *lane == [0, 0, 0, 0] || *lane == [0, 0, 0, 1] + fn is_message_accepted(send_origin: &Self::Origin, lane: &LaneId) -> bool { + send_origin.linked_account().is_some() && (*lane == [0, 0, 0, 0] || *lane == [0, 0, 0, 1]) } fn maximal_pending_messages_at_outbound_lane() -> MessageNonce { @@ -248,6 +248,17 @@ impl SourceHeaderChain for Rialto { } } +impl SenderOrigin for crate::Origin { + fn linked_account(&self) -> Option { + match self.caller { + crate::OriginCaller::system(frame_system::RawOrigin::Signed(ref submitter)) => Some(submitter.clone()), + crate::OriginCaller::system(frame_system::RawOrigin::Root) + | crate::OriginCaller::system(frame_system::RawOrigin::None) => crate::RootAccountForPayments::get(), + _ => None, + } + } +} + /// Millau -> Rialto message lane pallet parameters. #[derive(RuntimeDebug, Clone, Encode, Decode, PartialEq, Eq)] pub enum MillauToRialtoMessagesParameter { diff --git a/bin/rialto/runtime/src/lib.rs b/bin/rialto/runtime/src/lib.rs index e53beed389..eb2f14fedb 100644 --- a/bin/rialto/runtime/src/lib.rs +++ b/bin/rialto/runtime/src/lib.rs @@ -483,13 +483,14 @@ impl pallet_bridge_messages::Config for Runtime { type AccountIdConverter = bp_rialto::AccountIdConverter; + type SenderOrigin = Origin; type TargetHeaderChain = crate::millau_messages::Millau; type LaneMessageVerifier = crate::millau_messages::ToMillauMessageVerifier; type MessageDeliveryAndDispatchPayment = pallet_bridge_messages::instant_payments::InstantCurrencyPayments< Runtime, + WithMillauMessagesInstance, pallet_balances::Pallet, GetDeliveryConfirmationTransactionFee, - RootAccountForPayments, >; type OnDeliveryConfirmed = (); diff --git a/bin/rialto/runtime/src/millau_messages.rs b/bin/rialto/runtime/src/millau_messages.rs index 47107d9aa6..6fd72a7a48 100644 --- a/bin/rialto/runtime/src/millau_messages.rs +++ b/bin/rialto/runtime/src/millau_messages.rs @@ -19,7 +19,7 @@ use crate::Runtime; use bp_messages::{ - source_chain::TargetHeaderChain, + source_chain::{SenderOrigin, TargetHeaderChain}, target_chain::{ProvedMessages, SourceHeaderChain}, InboundLaneData, LaneId, Message, MessageNonce, Parameter as MessagesParameter, }; @@ -104,8 +104,8 @@ impl messages::ThisChainWithMessages for Rialto { type Origin = crate::Origin; type Call = crate::Call; - fn is_message_accepted(_send_origin: &Self::Origin, lane: &LaneId) -> bool { - *lane == [0, 0, 0, 0] || *lane == [0, 0, 0, 1] + fn is_message_accepted(send_origin: &Self::Origin, lane: &LaneId) -> bool { + send_origin.linked_account().is_some() && (*lane == [0, 0, 0, 0] || *lane == [0, 0, 0, 1]) } fn maximal_pending_messages_at_outbound_lane() -> MessageNonce { @@ -248,6 +248,17 @@ impl SourceHeaderChain for Millau { } } +impl SenderOrigin for crate::Origin { + fn linked_account(&self) -> Option { + match self.caller { + crate::OriginCaller::system(frame_system::RawOrigin::Signed(ref submitter)) => Some(submitter.clone()), + crate::OriginCaller::system(frame_system::RawOrigin::Root) + | crate::OriginCaller::system(frame_system::RawOrigin::None) => crate::RootAccountForPayments::get(), + _ => None, + } + } +} + /// Rialto -> Millau message lane pallet parameters. #[derive(RuntimeDebug, Clone, Encode, Decode, PartialEq, Eq)] pub enum RialtoToMillauMessagesParameter { diff --git a/bin/runtime-common/README.md b/bin/runtime-common/README.md index dcf748218f..5f2298cd78 100644 --- a/bin/runtime-common/README.md +++ b/bin/runtime-common/README.md @@ -64,7 +64,9 @@ This trait represents this chain from bridge point of view. Let's review every m - `ThisChainWithMessages::is_message_accepted`: is used to check whether given lane accepts messages. The send-message origin is passed to the function, so you may e.g. verify that only - given pallet is able to send messages over selected lane. + given pallet is able to send messages over selected lane. **IMPORTANT**: if you assume that the + message must be paid by the sender, you must ensure that the sender origin has linked the account + for paying message delivery and dispatch fee. - `ThisChainWithMessages::maximal_pending_messages_at_outbound_lane`: you should return maximal number of pending (undelivered) messages from this function. Returning small values would require diff --git a/modules/messages/src/instant_payments.rs b/modules/messages/src/instant_payments.rs index 057bcfbde6..7bf40f68ee 100644 --- a/modules/messages/src/instant_payments.rs +++ b/modules/messages/src/instant_payments.rs @@ -20,7 +20,7 @@ //! to the actual relayer in case confirmation is received. use bp_messages::{ - source_chain::{MessageDeliveryAndDispatchPayment, RelayersRewards}, + source_chain::{MessageDeliveryAndDispatchPayment, RelayersRewards, SenderOrigin}, MessageNonce, }; use codec::Encode; @@ -42,19 +42,18 @@ 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 { - _phantom: sp_std::marker::PhantomData<(T, Currency, GetConfirmationFee, RootAccount)>, +pub struct InstantCurrencyPayments { + _phantom: sp_std::marker::PhantomData<(T, I, Currency, GetConfirmationFee)>, } -impl - MessageDeliveryAndDispatchPayment - for InstantCurrencyPayments +impl + MessageDeliveryAndDispatchPayment + for InstantCurrencyPayments where - T: frame_system::Config, + T: crate::Config, Currency: CurrencyT, Currency::Balance: From, GetConfirmationFee: Get, - RootAccount: Get>, { type Error = &'static str; @@ -68,17 +67,18 @@ where } fn pay_delivery_and_dispatch_fee( - submitter: &T::Origin, + submitter: &T::SenderOrigin, fee: &Currency::Balance, relayer_fund_account: &T::AccountId, ) -> Result<(), Self::Error> { - let root_account = RootAccount::get(); - let submitter_account = match submitter.clone().into() { - Ok(frame_system::RawOrigin::Signed(submitter)) => submitter, - Ok(frame_system::RawOrigin::Root) | Ok(frame_system::RawOrigin::None) => { - root_account.ok_or("Sending messages using Root or None origin is disallowed.")? + let submitter_account = match submitter.linked_account() { + Some(submitter_account) => submitter_account, + None => { + // message lane verifier has accepted the message before, so this message + // is unpaid **by design** + // => let's just do nothing + return Ok(()); } - Err(_) => return Err("Invalid message sender origin"), }; Currency::transfer( diff --git a/modules/messages/src/lib.rs b/modules/messages/src/lib.rs index 28d1aefcbe..ce6aa9cfb5 100644 --- a/modules/messages/src/lib.rs +++ b/modules/messages/src/lib.rs @@ -48,7 +48,8 @@ use crate::weights::WeightInfo; use bp_messages::{ source_chain::{ - LaneMessageVerifier, MessageDeliveryAndDispatchPayment, OnDeliveryConfirmed, RelayersRewards, TargetHeaderChain, + LaneMessageVerifier, MessageDeliveryAndDispatchPayment, OnDeliveryConfirmed, RelayersRewards, SenderOrigin, + TargetHeaderChain, }, target_chain::{DispatchMessage, MessageDispatch, ProvedLaneMessages, ProvedMessages, SourceHeaderChain}, total_unrewarded_messages, DeliveredMessages, InboundLaneData, LaneId, MessageData, MessageKey, MessageNonce, @@ -142,18 +143,20 @@ pub trait Config: frame_system::Config { // Types that are used by outbound_lane (on source chain). + /// Message sender origin. + type SenderOrigin: SenderOrigin + From; /// Target header chain. type TargetHeaderChain: TargetHeaderChain; /// Message payload verifier. type LaneMessageVerifier: LaneMessageVerifier< - Self::Origin, + Self::SenderOrigin, Self::AccountId, Self::OutboundPayload, Self::OutboundMessageFee, >; /// Message delivery payment. type MessageDeliveryAndDispatchPayment: MessageDeliveryAndDispatchPayment< - Self::Origin, + Self::SenderOrigin, Self::AccountId, Self::OutboundMessageFee, >; @@ -342,6 +345,7 @@ decl_module! { })?; // now let's enforce any additional lane rules + let origin = T::SenderOrigin::from(origin); let mut lane = outbound_lane::(lane_id); T::LaneMessageVerifier::verify_message( &origin, @@ -419,7 +423,7 @@ decl_module! { // withdraw additional fee from submitter T::MessageDeliveryAndDispatchPayment::pay_delivery_and_dispatch_fee( - &origin, + &T::SenderOrigin::from(origin), &additional_fee, &Self::relayer_fund_account_id(), ).map_err(|err| { diff --git a/modules/messages/src/mock.rs b/modules/messages/src/mock.rs index deccf32d6a..94242416a6 100644 --- a/modules/messages/src/mock.rs +++ b/modules/messages/src/mock.rs @@ -22,7 +22,7 @@ use crate::Config; use bitvec::prelude::*; use bp_messages::{ source_chain::{ - LaneMessageVerifier, MessageDeliveryAndDispatchPayment, OnDeliveryConfirmed, RelayersRewards, + LaneMessageVerifier, MessageDeliveryAndDispatchPayment, OnDeliveryConfirmed, RelayersRewards, SenderOrigin, TargetHeaderChain, }, target_chain::{DispatchMessage, MessageDispatch, ProvedLaneMessages, ProvedMessages, SourceHeaderChain}, @@ -172,6 +172,7 @@ impl Config for TestRuntime { type AccountIdConverter = AccountIdConverter; + type SenderOrigin = Origin; type TargetHeaderChain = TestTargetHeaderChain; type LaneMessageVerifier = TestLaneMessageVerifier; type MessageDeliveryAndDispatchPayment = TestMessageDeliveryAndDispatchPayment; @@ -181,6 +182,15 @@ impl Config for TestRuntime { type MessageDispatch = TestMessageDispatch; } +impl SenderOrigin for Origin { + fn linked_account(&self) -> Option { + 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 diff --git a/primitives/messages/src/source_chain.rs b/primitives/messages/src/source_chain.rs index b2002bbc8b..73ae6bf11a 100644 --- a/primitives/messages/src/source_chain.rs +++ b/primitives/messages/src/source_chain.rs @@ -22,6 +22,22 @@ use bp_runtime::Size; use frame_support::{weights::Weight, Parameter, RuntimeDebug}; use sp_std::{collections::btree_map::BTreeMap, fmt::Debug}; +/// The sender of the message on the source chain. +pub trait SenderOrigin { + /// Return id of the account that is sending this message. + /// + /// In regular messages configuration, when regular message is sent you'll always get `Some(_)` from + /// this call. This is the account that is paying send costs. However, there are some examples when + /// `None` may be returned from the call: + /// + /// - if the send-message call origin is either `frame_system::RawOrigin::Root` or `frame_system::RawOrigin::None` + /// and your configuration forbids such messages; + /// - if your configuration allows 'unpaid' messages sent by pallets. Then the pallet may just use its + /// own defined origin (not linked to any account) and the message will be accepted. This may be useful for + /// pallets that are sending important system-wide information (like update of runtime version). + fn linked_account(&self) -> Option; +} + /// Relayers rewards, grouped by relayer account id. pub type RelayersRewards = BTreeMap>; @@ -74,13 +90,13 @@ pub trait TargetHeaderChain { /// Lane3 until some block, ...), then it may be built using this verifier. /// /// Any fee requirements should also be enforced here. -pub trait LaneMessageVerifier { +pub trait LaneMessageVerifier { /// Error type. type Error: Debug + Into<&'static str>; /// Verify message payload and return Ok(()) if message is valid and allowed to be sent over the lane. fn verify_message( - submitter: &Origin, + submitter: &SenderOrigin, delivery_and_dispatch_fee: &Fee, lane: &LaneId, outbound_data: &OutboundLaneData, @@ -101,14 +117,14 @@ pub trait LaneMessageVerifier { /// So to be sure that any non-altruist relayer would agree to deliver message, submitter /// should set `delivery_and_dispatch_fee` to at least (equialent of): sum of fees from (2) /// to (4) above, plus some interest for the relayer. -pub trait MessageDeliveryAndDispatchPayment { +pub trait MessageDeliveryAndDispatchPayment { /// Error type. type Error: Debug + Into<&'static str>; /// Withhold/write-off delivery_and_dispatch_fee from submitter account to /// some relayers-fund account. fn pay_delivery_and_dispatch_fee( - submitter: &Origin, + submitter: &SenderOrigin, fee: &Balance, relayer_fund_account: &AccountId, ) -> Result<(), Self::Error>; @@ -185,11 +201,13 @@ impl TargetHeaderChain for ForbidOutboun } } -impl LaneMessageVerifier for ForbidOutboundMessages { +impl LaneMessageVerifier + for ForbidOutboundMessages +{ type Error = &'static str; fn verify_message( - _submitter: &Origin, + _submitter: &SenderOrigin, _delivery_and_dispatch_fee: &Fee, _lane: &LaneId, _outbound_data: &OutboundLaneData, @@ -199,13 +217,13 @@ impl LaneMessageVerifier MessageDeliveryAndDispatchPayment +impl MessageDeliveryAndDispatchPayment for ForbidOutboundMessages { type Error = &'static str; fn pay_delivery_and_dispatch_fee( - _submitter: &Origin, + _submitter: &SenderOrigin, _fee: &Balance, _relayer_fund_account: &AccountId, ) -> Result<(), Self::Error> {