From 79c1ab8fc94e68b8c1f06252364be4f10372ecb6 Mon Sep 17 00:00:00 2001 From: HackFisher Date: Fri, 17 Jun 2022 23:30:43 +0800 Subject: [PATCH] cherry pick #137 (#140) * cherry pick #137 * pick #141 Co-authored-by: bear --- bin/runtime-common/src/messages.rs | 12 ++++++++++- modules/dispatch/src/lib.rs | 27 ++++++++++++++++++++----- modules/fee-market/src/tests.rs | 7 +++++++ modules/messages/src/inbound_lane.rs | 19 ++++++++++------- modules/messages/src/lib.rs | 7 ++++--- modules/messages/src/mock.rs | 7 +++++++ primitives/message-dispatch/src/lib.rs | 20 ++++++++++++++++-- primitives/messages/src/target_chain.rs | 16 +++++++++++++++ 8 files changed, 97 insertions(+), 18 deletions(-) diff --git a/bin/runtime-common/src/messages.rs b/bin/runtime-common/src/messages.rs index 7f6a0330a..51704c6f5 100644 --- a/bin/runtime-common/src/messages.rs +++ b/bin/runtime-common/src/messages.rs @@ -523,7 +523,7 @@ pub mod target { /// /// Our Call is opaque (`Vec`) for Bridged chain. So it is encoded, prefixed with /// vector length. Custom decode implementation here is exactly to deal with this. - #[derive(Decode, Encode, RuntimeDebug, PartialEq)] + #[derive(Decode, Encode, Clone, RuntimeDebug, PartialEq)] pub struct FromBridgedChainEncodedMessageCall { encoded_call: Vec, _marker: PhantomData, @@ -573,6 +573,16 @@ pub mod target { message.data.payload.as_ref().map(|payload| payload.weight).unwrap_or(0) } + fn pre_dispatch( + relayer_account: &AccountIdOf>, + message: &DispatchMessage>>, + ) -> Result<(), &'static str> { + pallet_bridge_dispatch::Pallet::::pre_dispatch( + relayer_account, + message.data.payload.as_ref().map_err(drop), + ) + } + fn dispatch( relayer_account: &AccountIdOf>, message: DispatchMessage>>, diff --git a/modules/dispatch/src/lib.rs b/modules/dispatch/src/lib.rs index 0bb9b566e..e1e969d87 100644 --- a/modules/dispatch/src/lib.rs +++ b/modules/dispatch/src/lib.rs @@ -92,7 +92,7 @@ pub mod pallet { /// that all other stuff (like `spec_version`) is ok. If we would try to decode /// `Call` which has been encoded using previous `spec_version`, then we might end /// up with decoding error, instead of `MessageVersionSpecMismatch`. - type EncodedCall: Decode + Encode + Into>::Call, ()>>; + type EncodedCall: Decode + Encode + Into>::Call, ()>> + Clone; /// A type which can be turned into an AccountId from a 256-bit hash. /// /// Used when deriving target chain AccountIds from source chain AccountIds. @@ -160,6 +160,16 @@ impl, I: 'static> MessageDispatch message.weight } + fn pre_dispatch( + relayer_account: &T::AccountId, + message: Result<&Self::Message, ()>, + ) -> Result<(), &'static str> { + let raw_message = message.map_err(|_| "Invalid Message")?; + let call = raw_message.clone().call.into().map_err(|_| "Invalid Call")?; + + T::CallValidator::check_receiving_before_dispatch(relayer_account, &call) + } + fn dispatch Result<(), ()>>( source_chain: ChainId, target_chain: ChainId, @@ -275,8 +285,8 @@ impl, I: 'static> MessageDispatch let dispatch_origin = T::IntoDispatchOrigin::into_dispatch_origin(&origin_derived_account, &call); - // filter the call - if let Err(_) = T::CallValidator::pre_dispatch(relayer_account, &dispatch_origin, &call) { + // validate the call + if let Err(_e) = T::CallValidator::call_validate(relayer_account, &dispatch_origin, &call) { log::trace!( target: "runtime::bridge-dispatch", "Message {:?}/{:?}: the call ({:?}) is rejected by filter", @@ -560,7 +570,7 @@ mod tests { type TargetChainSignature = TestSignature; } - #[derive(Decode, Encode)] + #[derive(Decode, Encode, Clone)] pub struct EncodedCall(Vec); impl From for Result { @@ -571,7 +581,14 @@ mod tests { pub struct CallValidator; impl CallValidate for CallValidator { - fn pre_dispatch( + fn check_receiving_before_dispatch( + _relayer_account: &AccountId, + _call: &Call, + ) -> Result<(), &'static str> { + Ok(()) + } + + fn call_validate( _relayer_account: &AccountId, _origin: &Origin, call: &Call, diff --git a/modules/fee-market/src/tests.rs b/modules/fee-market/src/tests.rs index 400346a0f..a9f55b666 100644 --- a/modules/fee-market/src/tests.rs +++ b/modules/fee-market/src/tests.rs @@ -344,6 +344,13 @@ impl MessageDispatch for TestMessageDispatch { } } + fn pre_dispatch( + _relayer_account: &AccountId, + _message: &DispatchMessage, + ) -> Result<(), &'static str> { + Ok(()) + } + fn dispatch( _relayer_account: &AccountId, message: DispatchMessage, diff --git a/modules/messages/src/inbound_lane.rs b/modules/messages/src/inbound_lane.rs index 2481b1f15..78df0877a 100644 --- a/modules/messages/src/inbound_lane.rs +++ b/modules/messages/src/inbound_lane.rs @@ -58,6 +58,8 @@ pub enum ReceivalResult { TooManyUnrewardedRelayers, /// There are too many unconfirmed messages at the lane. TooManyUnconfirmedMessages, + /// Pre-dispatch validation failed before message dispatch. + PreDispatchValidateFailed, } /// Inbound messages lane. @@ -141,14 +143,17 @@ impl InboundLane { return ReceivalResult::TooManyUnconfirmedMessages; } + let dispatch_message = DispatchMessage { + key: MessageKey { lane_id: self.storage.id(), nonce }, + data: message_data, + }; + // if there are some extra pre-dispatch validation errors, reject this message. + if P::pre_dispatch(relayer_at_this_chain, &dispatch_message).is_err() { + return ReceivalResult::PreDispatchValidateFailed + } + // then, dispatch message - let dispatch_result = P::dispatch( - relayer_at_this_chain, - DispatchMessage { - key: MessageKey { lane_id: self.storage.id(), nonce }, - data: message_data, - }, - ); + let dispatch_result = P::dispatch(relayer_at_this_chain, dispatch_message); // now let's update inbound lane storage let push_new = match data.relayers.back_mut() { diff --git a/modules/messages/src/lib.rs b/modules/messages/src/lib.rs index b092416be..6c2659800 100644 --- a/modules/messages/src/lib.rs +++ b/modules/messages/src/lib.rs @@ -466,9 +466,10 @@ pub mod pallet { !dispatch_result.dispatch_fee_paid_during_dispatch, ) }, - ReceivalResult::InvalidNonce - | ReceivalResult::TooManyUnrewardedRelayers - | ReceivalResult::TooManyUnconfirmedMessages => (dispatch_weight, true), + ReceivalResult::InvalidNonce | + ReceivalResult::TooManyUnrewardedRelayers | + ReceivalResult::PreDispatchValidateFailed | + ReceivalResult::TooManyUnconfirmedMessages => (dispatch_weight, true), }; let unspent_weight = sp_std::cmp::min(unspent_weight, dispatch_weight); diff --git a/modules/messages/src/mock.rs b/modules/messages/src/mock.rs index 5d7a21afe..8ab089c7c 100644 --- a/modules/messages/src/mock.rs +++ b/modules/messages/src/mock.rs @@ -489,6 +489,13 @@ impl MessageDispatch for TestMessageDispatch { } } + fn pre_dispatch( + _relayer_account: &AccountId, + _message: &DispatchMessage, + ) -> Result<(), &'static str> { + Ok(()) + } + fn dispatch( _relayer_account: &AccountId, message: DispatchMessage, diff --git a/primitives/message-dispatch/src/lib.rs b/primitives/message-dispatch/src/lib.rs index e6f68cc35..2cde19ffb 100644 --- a/primitives/message-dispatch/src/lib.rs +++ b/primitives/message-dispatch/src/lib.rs @@ -46,6 +46,15 @@ pub trait MessageDispatch { /// of dispatch weight. fn dispatch_weight(message: &Self::Message) -> Weight; + /// Checking in message receiving step before dispatch + /// + /// This will be called before the call enter dispatch phase. If failed, the message(call) will + /// be not be processed by this relayer, latter relayers can still continue process it. + fn pre_dispatch( + relayer_account: &AccountId, + message: Result<&Self::Message, ()>, + ) -> Result<(), &'static str>; + /// Dispatches the message internally. /// /// `source_chain` indicates the chain where the message came from. @@ -154,8 +163,15 @@ pub trait IntoDispatchOrigin { /// A generic trait to validate message before dispatch. pub trait CallValidate { - /// call validation - fn pre_dispatch( + /// Checking in message receiving step before dispatch + /// + /// This will be called before the call enter dispatch phase. If failed, the message(call) will + /// be not be processed by this relayer, latter relayers can still continue process it. + fn check_receiving_before_dispatch(relayer_account: &AccountId, call: &Call) -> Result<(), &'static str>; + /// In-dispatch call validation + /// + /// This will be called in the dispatch process, If failed, return message dispatch errors. + fn call_validate( relayer_account: &AccountId, origin: &Origin, call: &Call, diff --git a/primitives/messages/src/target_chain.rs b/primitives/messages/src/target_chain.rs index a84ea7af9..685a4e9a8 100644 --- a/primitives/messages/src/target_chain.rs +++ b/primitives/messages/src/target_chain.rs @@ -97,6 +97,15 @@ pub trait MessageDispatch { /// of dispatch weight. fn dispatch_weight(message: &DispatchMessage) -> Weight; + /// Checking in message receiving step before dispatch + /// + /// This will be called before the call enter dispatch phase. If failed, the message(call) will + /// be not be processed by this relayer, latter relayers can still continue process it. + fn pre_dispatch( + relayer_account: &AccountId, + message: &DispatchMessage, + ) -> Result<(), &'static str>; + /// Called when inbound message is received. /// /// It is up to the implementers of this trait to determine whether the message @@ -160,6 +169,13 @@ impl MessageDispatch for ForbidInboundMessages { Weight::MAX } + fn pre_dispatch( + _: &AccountId, + _message: &DispatchMessage, + ) -> Result<(), &'static str> { + Ok(()) + } + fn dispatch( _: &AccountId, _: DispatchMessage,