From 7d39d44e55b06d0e351d3f77ea76577942f9504d Mon Sep 17 00:00:00 2001 From: bear Date: Fri, 17 Jun 2022 16:05:07 +0800 Subject: [PATCH 01/11] Add todo --- modules/messages/src/inbound_lane.rs | 5 ++++- primitives/message-dispatch/src/lib.rs | 1 + 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/modules/messages/src/inbound_lane.rs b/modules/messages/src/inbound_lane.rs index 00875bb87..2299ba062 100644 --- a/modules/messages/src/inbound_lane.rs +++ b/modules/messages/src/inbound_lane.rs @@ -58,6 +58,7 @@ pub enum ReceivalResult { TooManyUnrewardedRelayers, /// There are too many unconfirmed messages at the lane. TooManyUnconfirmedMessages, + RelayerInsufficientBalance, } /// Inbound messages lane. @@ -117,7 +118,7 @@ impl InboundLane { } /// Receive new message. - pub fn receive_message, AccountId>( + pub fn receive_message, V: CallValidate, AccountId>( &mut self, relayer_at_bridged_chain: &S::Relayer, relayer_at_this_chain: &AccountId, @@ -141,6 +142,8 @@ impl InboundLane { return ReceivalResult::TooManyUnconfirmedMessages } + // Ensure relayer has enough balance to pay for the derived account. + // then, dispatch message let dispatch_result = P::dispatch( relayer_at_this_chain, diff --git a/primitives/message-dispatch/src/lib.rs b/primitives/message-dispatch/src/lib.rs index e6f68cc35..24671564a 100644 --- a/primitives/message-dispatch/src/lib.rs +++ b/primitives/message-dispatch/src/lib.rs @@ -154,6 +154,7 @@ pub trait IntoDispatchOrigin { /// A generic trait to validate message before dispatch. pub trait CallValidate { + fn check_relayer_balance(relayer_account: &AccountId) -> bool; /// call validation fn pre_dispatch( relayer_account: &AccountId, From d90fd0809cb47982dee625055fe7ebb8bf54bb35 Mon Sep 17 00:00:00 2001 From: bear Date: Fri, 17 Jun 2022 17:00:08 +0800 Subject: [PATCH 02/11] A first review --- bin/runtime-common/src/messages.rs | 9 +++++++++ modules/dispatch/src/lib.rs | 5 +++++ modules/messages/src/inbound_lane.rs | 17 +++++++++-------- modules/messages/src/lib.rs | 1 + primitives/message-dispatch/src/lib.rs | 2 ++ primitives/messages/src/target_chain.rs | 5 +++++ 6 files changed, 31 insertions(+), 8 deletions(-) diff --git a/bin/runtime-common/src/messages.rs b/bin/runtime-common/src/messages.rs index 78598120c..a4b3c6378 100644 --- a/bin/runtime-common/src/messages.rs +++ b/bin/runtime-common/src/messages.rs @@ -601,6 +601,15 @@ pub mod target { message.data.payload.as_ref().map(|payload| payload.weight).unwrap_or(0) } + fn pre_dispatch( + message: &DispatchMessage>>, + ) -> bool { + pallet_bridge_dispatch::Pallet::::pre_dispatch( + // TODO: update this type + &message.data.payload.as_ref().unwrap(), + ) + } + fn dispatch( relayer_account: &AccountIdOf>, message: DispatchMessage>>, diff --git a/modules/dispatch/src/lib.rs b/modules/dispatch/src/lib.rs index 7c434716b..13c3fb7dc 100644 --- a/modules/dispatch/src/lib.rs +++ b/modules/dispatch/src/lib.rs @@ -159,6 +159,11 @@ impl, I: 'static> MessageDispatch message.weight } + fn pre_dispatch(message: &Self::Message) -> bool { + // T::CallValidator::check_relayer_balance(relayer_account, &dispatch_origin, &call) + true + } + fn dispatch Result<(), ()>>( source_chain: ChainId, target_chain: ChainId, diff --git a/modules/messages/src/inbound_lane.rs b/modules/messages/src/inbound_lane.rs index 2299ba062..26ff4544d 100644 --- a/modules/messages/src/inbound_lane.rs +++ b/modules/messages/src/inbound_lane.rs @@ -118,7 +118,7 @@ impl InboundLane { } /// Receive new message. - pub fn receive_message, V: CallValidate, AccountId>( + pub fn receive_message, AccountId>( &mut self, relayer_at_bridged_chain: &S::Relayer, relayer_at_this_chain: &AccountId, @@ -142,16 +142,17 @@ impl InboundLane { return ReceivalResult::TooManyUnconfirmedMessages } + let dispatch_message = DispatchMessage { + key: MessageKey { lane_id: self.storage.id(), nonce }, + data: message_data, + }; // Ensure relayer has enough balance to pay for the derived account. + if P::pre_dispatch(&dispatch_message) { + return ReceivalResult::RelayerInsufficientBalance + } // 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 11cf81bef..cd6cb87fc 100644 --- a/modules/messages/src/lib.rs +++ b/modules/messages/src/lib.rs @@ -471,6 +471,7 @@ pub mod pallet { }, ReceivalResult::InvalidNonce | ReceivalResult::TooManyUnrewardedRelayers | + ReceivalResult::RelayerInsufficientBalance | ReceivalResult::TooManyUnconfirmedMessages => (dispatch_weight, true), }; diff --git a/primitives/message-dispatch/src/lib.rs b/primitives/message-dispatch/src/lib.rs index 24671564a..0a9b1dc69 100644 --- a/primitives/message-dispatch/src/lib.rs +++ b/primitives/message-dispatch/src/lib.rs @@ -46,6 +46,8 @@ pub trait MessageDispatch { /// of dispatch weight. fn dispatch_weight(message: &Self::Message) -> Weight; + fn pre_dispatch(message: &Self::Message) -> bool; + /// Dispatches the message internally. /// /// `source_chain` indicates the chain where the message came from. diff --git a/primitives/messages/src/target_chain.rs b/primitives/messages/src/target_chain.rs index a84ea7af9..ebd4307a5 100644 --- a/primitives/messages/src/target_chain.rs +++ b/primitives/messages/src/target_chain.rs @@ -97,6 +97,7 @@ pub trait MessageDispatch { /// of dispatch weight. fn dispatch_weight(message: &DispatchMessage) -> Weight; + fn pre_dispatch(message: &DispatchMessage) -> bool; /// Called when inbound message is received. /// /// It is up to the implementers of this trait to determine whether the message @@ -160,6 +161,10 @@ impl MessageDispatch for ForbidInboundMessages { Weight::MAX } + fn pre_dispatch(_message: &DispatchMessage) -> bool { + false + } + fn dispatch( _: &AccountId, _: DispatchMessage, From a5199f4594772353cdda41523a77f064be5a298d Mon Sep 17 00:00:00 2001 From: bear Date: Fri, 17 Jun 2022 17:18:52 +0800 Subject: [PATCH 03/11] Add `relayer_account_id` param --- bin/runtime-common/src/messages.rs | 2 ++ modules/dispatch/src/lib.rs | 12 ++++++++++-- modules/messages/src/inbound_lane.rs | 3 ++- primitives/message-dispatch/src/lib.rs | 5 +++-- primitives/messages/src/target_chain.rs | 7 +++++-- 5 files changed, 22 insertions(+), 7 deletions(-) diff --git a/bin/runtime-common/src/messages.rs b/bin/runtime-common/src/messages.rs index a4b3c6378..3bb6d8cdb 100644 --- a/bin/runtime-common/src/messages.rs +++ b/bin/runtime-common/src/messages.rs @@ -602,9 +602,11 @@ pub mod target { } fn pre_dispatch( + relayer_account: &AccountIdOf>, message: &DispatchMessage>>, ) -> bool { pallet_bridge_dispatch::Pallet::::pre_dispatch( + relayer_account, // TODO: update this type &message.data.payload.as_ref().unwrap(), ) diff --git a/modules/dispatch/src/lib.rs b/modules/dispatch/src/lib.rs index 13c3fb7dc..4da081629 100644 --- a/modules/dispatch/src/lib.rs +++ b/modules/dispatch/src/lib.rs @@ -159,8 +159,16 @@ impl, I: 'static> MessageDispatch message.weight } - fn pre_dispatch(message: &Self::Message) -> bool { - // T::CallValidator::check_relayer_balance(relayer_account, &dispatch_origin, &call) + fn pre_dispatch(relayer_account: &T::AccountId, message: &Self::Message) -> bool { + // TODO: Might be written better + // let call = match message.call.clone().into() { + // Ok(call) => call, + // Err(_) => { + // return false + // }, + // }; + + // T::CallValidator::check_relayer_balance(relayer_account, &call) true } diff --git a/modules/messages/src/inbound_lane.rs b/modules/messages/src/inbound_lane.rs index 26ff4544d..8b7431ec2 100644 --- a/modules/messages/src/inbound_lane.rs +++ b/modules/messages/src/inbound_lane.rs @@ -146,8 +146,9 @@ impl InboundLane { key: MessageKey { lane_id: self.storage.id(), nonce }, data: message_data, }; + // TODO: update the commment // Ensure relayer has enough balance to pay for the derived account. - if P::pre_dispatch(&dispatch_message) { + if P::pre_dispatch(relayer_at_this_chain, &dispatch_message) { return ReceivalResult::RelayerInsufficientBalance } diff --git a/primitives/message-dispatch/src/lib.rs b/primitives/message-dispatch/src/lib.rs index 0a9b1dc69..8a480859a 100644 --- a/primitives/message-dispatch/src/lib.rs +++ b/primitives/message-dispatch/src/lib.rs @@ -46,7 +46,7 @@ pub trait MessageDispatch { /// of dispatch weight. fn dispatch_weight(message: &Self::Message) -> Weight; - fn pre_dispatch(message: &Self::Message) -> bool; + fn pre_dispatch(relayer_account: &AccountId, message: &Self::Message) -> bool; /// Dispatches the message internally. /// @@ -156,7 +156,8 @@ pub trait IntoDispatchOrigin { /// A generic trait to validate message before dispatch. pub trait CallValidate { - fn check_relayer_balance(relayer_account: &AccountId) -> bool; + // TODO: A better name + fn check_relayer_balance(relayer_account: &AccountId, call: &Call) -> bool; /// call validation fn pre_dispatch( relayer_account: &AccountId, diff --git a/primitives/messages/src/target_chain.rs b/primitives/messages/src/target_chain.rs index ebd4307a5..4cde517df 100644 --- a/primitives/messages/src/target_chain.rs +++ b/primitives/messages/src/target_chain.rs @@ -97,7 +97,10 @@ pub trait MessageDispatch { /// of dispatch weight. fn dispatch_weight(message: &DispatchMessage) -> Weight; - fn pre_dispatch(message: &DispatchMessage) -> bool; + fn pre_dispatch( + relayer_account: &AccountId, + message: &DispatchMessage, + ) -> bool; /// Called when inbound message is received. /// /// It is up to the implementers of this trait to determine whether the message @@ -161,7 +164,7 @@ impl MessageDispatch for ForbidInboundMessages { Weight::MAX } - fn pre_dispatch(_message: &DispatchMessage) -> bool { + fn pre_dispatch(_: &AccountId, _message: &DispatchMessage) -> bool { false } From 0cd0760d6a0ce804016a4f85e5b692220c31f41e Mon Sep 17 00:00:00 2001 From: bear Date: Fri, 17 Jun 2022 17:31:54 +0800 Subject: [PATCH 04/11] Add clone trait bound --- modules/dispatch/src/lib.rs | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/modules/dispatch/src/lib.rs b/modules/dispatch/src/lib.rs index 4da081629..d46eb348b 100644 --- a/modules/dispatch/src/lib.rs +++ b/modules/dispatch/src/lib.rs @@ -91,7 +91,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. @@ -161,15 +161,13 @@ impl, I: 'static> MessageDispatch fn pre_dispatch(relayer_account: &T::AccountId, message: &Self::Message) -> bool { // TODO: Might be written better - // let call = match message.call.clone().into() { - // Ok(call) => call, - // Err(_) => { - // return false - // }, - // }; - - // T::CallValidator::check_relayer_balance(relayer_account, &call) - true + // let message_clone = message.clone(); + let call = match message.clone().call.into() { + Ok(call) => call, + Err(_) => return false, + }; + + T::CallValidator::check_relayer_balance(relayer_account, &call) } fn dispatch Result<(), ()>>( From e9f6c07b4c689e229cd6006fcce6341de0ea3219 Mon Sep 17 00:00:00 2001 From: bear Date: Fri, 17 Jun 2022 17:47:00 +0800 Subject: [PATCH 05/11] Update returned type, add some comment --- bin/runtime-common/src/messages.rs | 2 +- modules/dispatch/src/lib.rs | 7 +++++-- modules/messages/src/inbound_lane.rs | 6 +++--- primitives/message-dispatch/src/lib.rs | 11 +++++++++-- primitives/messages/src/target_chain.rs | 14 +++++++++++--- 5 files changed, 29 insertions(+), 11 deletions(-) diff --git a/bin/runtime-common/src/messages.rs b/bin/runtime-common/src/messages.rs index 3bb6d8cdb..3baf193e6 100644 --- a/bin/runtime-common/src/messages.rs +++ b/bin/runtime-common/src/messages.rs @@ -604,7 +604,7 @@ pub mod target { fn pre_dispatch( relayer_account: &AccountIdOf>, message: &DispatchMessage>>, - ) -> bool { + ) -> Result<(), &'static str> { pallet_bridge_dispatch::Pallet::::pre_dispatch( relayer_account, // TODO: update this type diff --git a/modules/dispatch/src/lib.rs b/modules/dispatch/src/lib.rs index d46eb348b..25d459460 100644 --- a/modules/dispatch/src/lib.rs +++ b/modules/dispatch/src/lib.rs @@ -159,12 +159,15 @@ impl, I: 'static> MessageDispatch message.weight } - fn pre_dispatch(relayer_account: &T::AccountId, message: &Self::Message) -> bool { + fn pre_dispatch( + relayer_account: &T::AccountId, + message: &Self::Message, + ) -> Result<(), &'static str> { // TODO: Might be written better // let message_clone = message.clone(); let call = match message.clone().call.into() { Ok(call) => call, - Err(_) => return false, + Err(_) => return Err("Invalid Call"), }; T::CallValidator::check_relayer_balance(relayer_account, &call) diff --git a/modules/messages/src/inbound_lane.rs b/modules/messages/src/inbound_lane.rs index 8b7431ec2..249a269ca 100644 --- a/modules/messages/src/inbound_lane.rs +++ b/modules/messages/src/inbound_lane.rs @@ -146,9 +146,9 @@ impl InboundLane { key: MessageKey { lane_id: self.storage.id(), nonce }, data: message_data, }; - // TODO: update the commment - // Ensure relayer has enough balance to pay for the derived account. - if P::pre_dispatch(relayer_at_this_chain, &dispatch_message) { + // if there are some extra message validation errors, reject this message + if P::pre_dispatch(relayer_at_this_chain, &dispatch_message).is_err() { + // TODO: Update the type return ReceivalResult::RelayerInsufficientBalance } diff --git a/primitives/message-dispatch/src/lib.rs b/primitives/message-dispatch/src/lib.rs index 8a480859a..e064b5cb1 100644 --- a/primitives/message-dispatch/src/lib.rs +++ b/primitives/message-dispatch/src/lib.rs @@ -46,7 +46,14 @@ pub trait MessageDispatch { /// of dispatch weight. fn dispatch_weight(message: &Self::Message) -> Weight; - fn pre_dispatch(relayer_account: &AccountId, message: &Self::Message) -> bool; + /// Pre-dispatch validation. + /// + /// This function is called before message is dispatched for necessary validation. if failed, + /// the message will not be dispatch. + fn pre_dispatch( + relayer_account: &AccountId, + message: &Self::Message, + ) -> Result<(), &'static str>; /// Dispatches the message internally. /// @@ -157,7 +164,7 @@ pub trait IntoDispatchOrigin { /// A generic trait to validate message before dispatch. pub trait CallValidate { // TODO: A better name - fn check_relayer_balance(relayer_account: &AccountId, call: &Call) -> bool; + fn check_relayer_balance(relayer_account: &AccountId, call: &Call) -> Result<(), &'static str>; /// call validation fn pre_dispatch( relayer_account: &AccountId, diff --git a/primitives/messages/src/target_chain.rs b/primitives/messages/src/target_chain.rs index 4cde517df..b39f3a1bc 100644 --- a/primitives/messages/src/target_chain.rs +++ b/primitives/messages/src/target_chain.rs @@ -97,10 +97,15 @@ pub trait MessageDispatch { /// of dispatch weight. fn dispatch_weight(message: &DispatchMessage) -> Weight; + /// pre-validate message. + /// + /// This function accepts the same params with the dispatch function, and called before + /// message is dispatched for necessary validation. fn pre_dispatch( relayer_account: &AccountId, message: &DispatchMessage, - ) -> bool; + ) -> Result<(), &'static str>; + /// Called when inbound message is received. /// /// It is up to the implementers of this trait to determine whether the message @@ -164,8 +169,11 @@ impl MessageDispatch for ForbidInboundMessages { Weight::MAX } - fn pre_dispatch(_: &AccountId, _message: &DispatchMessage) -> bool { - false + fn pre_dispatch( + _: &AccountId, + _message: &DispatchMessage, + ) -> Result<(), &'static str> { + Ok(()) } fn dispatch( From 5b68a23bce0e261936002519d056cbe43bdbb83e Mon Sep 17 00:00:00 2001 From: bear Date: Fri, 17 Jun 2022 18:14:02 +0800 Subject: [PATCH 06/11] Remove unwrap --- bin/runtime-common/src/messages.rs | 3 +-- modules/dispatch/src/lib.rs | 12 ++++-------- primitives/message-dispatch/src/lib.rs | 2 +- 3 files changed, 6 insertions(+), 11 deletions(-) diff --git a/bin/runtime-common/src/messages.rs b/bin/runtime-common/src/messages.rs index 3baf193e6..d9d1657f0 100644 --- a/bin/runtime-common/src/messages.rs +++ b/bin/runtime-common/src/messages.rs @@ -607,8 +607,7 @@ pub mod target { ) -> Result<(), &'static str> { pallet_bridge_dispatch::Pallet::::pre_dispatch( relayer_account, - // TODO: update this type - &message.data.payload.as_ref().unwrap(), + message.data.payload.map_err(drop), ) } diff --git a/modules/dispatch/src/lib.rs b/modules/dispatch/src/lib.rs index 25d459460..06a207de8 100644 --- a/modules/dispatch/src/lib.rs +++ b/modules/dispatch/src/lib.rs @@ -91,7 +91,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, ()>> + Clone; + type EncodedCall: Decode + Encode + Into>::Call, ()>>; /// A type which can be turned into an AccountId from a 256-bit hash. /// /// Used when deriving target chain AccountIds from source chain AccountIds. @@ -161,14 +161,10 @@ impl, I: 'static> MessageDispatch fn pre_dispatch( relayer_account: &T::AccountId, - message: &Self::Message, + message: Result, ) -> Result<(), &'static str> { - // TODO: Might be written better - // let message_clone = message.clone(); - let call = match message.clone().call.into() { - Ok(call) => call, - Err(_) => return Err("Invalid Call"), - }; + let message = message.map_err(|_| "Invalid Message")?; + let call = message.call.into().map_err(|_| "Invalid Call")?; T::CallValidator::check_relayer_balance(relayer_account, &call) } diff --git a/primitives/message-dispatch/src/lib.rs b/primitives/message-dispatch/src/lib.rs index e064b5cb1..bb24163ae 100644 --- a/primitives/message-dispatch/src/lib.rs +++ b/primitives/message-dispatch/src/lib.rs @@ -52,7 +52,7 @@ pub trait MessageDispatch { /// the message will not be dispatch. fn pre_dispatch( relayer_account: &AccountId, - message: &Self::Message, + message: Result, ) -> Result<(), &'static str>; /// Dispatches the message internally. From 63173045500addece663a0adf81d7207acd89438 Mon Sep 17 00:00:00 2001 From: bear Date: Fri, 17 Jun 2022 18:32:02 +0800 Subject: [PATCH 07/11] Rename things --- bin/runtime-common/src/messages.rs | 2 +- modules/dispatch/src/lib.rs | 12 ++++++------ primitives/message-dispatch/src/lib.rs | 15 ++++++++++----- 3 files changed, 17 insertions(+), 12 deletions(-) diff --git a/bin/runtime-common/src/messages.rs b/bin/runtime-common/src/messages.rs index d9d1657f0..be9330351 100644 --- a/bin/runtime-common/src/messages.rs +++ b/bin/runtime-common/src/messages.rs @@ -607,7 +607,7 @@ pub mod target { ) -> Result<(), &'static str> { pallet_bridge_dispatch::Pallet::::pre_dispatch( relayer_account, - message.data.payload.map_err(drop), + message.data.payload.as_ref().map_err(drop), ) } diff --git a/modules/dispatch/src/lib.rs b/modules/dispatch/src/lib.rs index 06a207de8..063748a3f 100644 --- a/modules/dispatch/src/lib.rs +++ b/modules/dispatch/src/lib.rs @@ -91,7 +91,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. @@ -161,12 +161,12 @@ impl, I: 'static> MessageDispatch fn pre_dispatch( relayer_account: &T::AccountId, - message: Result, + message: Result<&Self::Message, ()>, ) -> Result<(), &'static str> { - let message = message.map_err(|_| "Invalid Message")?; - let call = message.call.into().map_err(|_| "Invalid Call")?; + let raw_message = message.map_err(|_| "Invalid Message")?; + let call = raw_message.clone().call.into().map_err(|_| "Invalid Call")?; - T::CallValidator::check_relayer_balance(relayer_account, &call) + T::CallValidator::pre_dispatch(relayer_account, &call) } fn dispatch Result<(), ()>>( @@ -285,7 +285,7 @@ impl, I: 'static> MessageDispatch T::IntoDispatchOrigin::into_dispatch_origin(&origin_derived_account, &call); // validate the call - if let Err(e) = T::CallValidator::pre_dispatch(relayer_account, &dispatch_origin, &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 the validator", diff --git a/primitives/message-dispatch/src/lib.rs b/primitives/message-dispatch/src/lib.rs index bb24163ae..cff062705 100644 --- a/primitives/message-dispatch/src/lib.rs +++ b/primitives/message-dispatch/src/lib.rs @@ -52,7 +52,7 @@ pub trait MessageDispatch { /// the message will not be dispatch. fn pre_dispatch( relayer_account: &AccountId, - message: Result, + message: Result<&Self::Message, ()>, ) -> Result<(), &'static str>; /// Dispatches the message internally. @@ -163,10 +163,15 @@ pub trait IntoDispatchOrigin { /// A generic trait to validate message before dispatch. pub trait CallValidate { - // TODO: A better name - fn check_relayer_balance(relayer_account: &AccountId, call: &Call) -> Result<(), &'static str>; - /// call validation - fn pre_dispatch( + /// Pre-dispatch call validation. + /// + /// This will be called before the call enter dispatch phase. If failed, the message(call) will + /// be rejected. + fn pre_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, From 475acf233b92ed2e1900e8b276c8840a77053f0a Mon Sep 17 00:00:00 2001 From: bear Date: Fri, 17 Jun 2022 18:33:32 +0800 Subject: [PATCH 08/11] Rename error type --- modules/messages/src/inbound_lane.rs | 4 ++-- modules/messages/src/lib.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/modules/messages/src/inbound_lane.rs b/modules/messages/src/inbound_lane.rs index 249a269ca..98edeecdb 100644 --- a/modules/messages/src/inbound_lane.rs +++ b/modules/messages/src/inbound_lane.rs @@ -58,7 +58,7 @@ pub enum ReceivalResult { TooManyUnrewardedRelayers, /// There are too many unconfirmed messages at the lane. TooManyUnconfirmedMessages, - RelayerInsufficientBalance, + PreDispatchValidateFailed, } /// Inbound messages lane. @@ -149,7 +149,7 @@ impl InboundLane { // if there are some extra message validation errors, reject this message if P::pre_dispatch(relayer_at_this_chain, &dispatch_message).is_err() { // TODO: Update the type - return ReceivalResult::RelayerInsufficientBalance + return ReceivalResult::PreDispatchValidateFailed } // then, dispatch message diff --git a/modules/messages/src/lib.rs b/modules/messages/src/lib.rs index cd6cb87fc..3aa09dbb8 100644 --- a/modules/messages/src/lib.rs +++ b/modules/messages/src/lib.rs @@ -471,7 +471,7 @@ pub mod pallet { }, ReceivalResult::InvalidNonce | ReceivalResult::TooManyUnrewardedRelayers | - ReceivalResult::RelayerInsufficientBalance | + ReceivalResult::PreDispatchValidateFailed | ReceivalResult::TooManyUnconfirmedMessages => (dispatch_weight, true), }; From 1393e25971be8dbc9f4321773c028c29fc8923d2 Mon Sep 17 00:00:00 2001 From: bear Date: Fri, 17 Jun 2022 19:40:55 +0800 Subject: [PATCH 09/11] Remove todo, it's time to test --- modules/messages/src/inbound_lane.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/messages/src/inbound_lane.rs b/modules/messages/src/inbound_lane.rs index 98edeecdb..6def535fd 100644 --- a/modules/messages/src/inbound_lane.rs +++ b/modules/messages/src/inbound_lane.rs @@ -58,6 +58,7 @@ pub enum ReceivalResult { TooManyUnrewardedRelayers, /// There are too many unconfirmed messages at the lane. TooManyUnconfirmedMessages, + /// Pre-dispatch validation failed before message dispatch. PreDispatchValidateFailed, } @@ -146,9 +147,8 @@ impl InboundLane { key: MessageKey { lane_id: self.storage.id(), nonce }, data: message_data, }; - // if there are some extra message validation errors, reject this message + // if there are some extra pre-dispatch validation errors, reject this message. if P::pre_dispatch(relayer_at_this_chain, &dispatch_message).is_err() { - // TODO: Update the type return ReceivalResult::PreDispatchValidateFailed } From 35f347022d727650ccbe26d7b07480f0f39fe91a Mon Sep 17 00:00:00 2001 From: HackFisher Date: Fri, 17 Jun 2022 19:50:43 +0800 Subject: [PATCH 10/11] fix test (#139) --- modules/dispatch/src/lib.rs | 9 ++++++++- modules/fee-market/src/tests.rs | 7 +++++++ modules/messages/src/mock.rs | 7 +++++++ 3 files changed, 22 insertions(+), 1 deletion(-) diff --git a/modules/dispatch/src/lib.rs b/modules/dispatch/src/lib.rs index 063748a3f..b111e0aa0 100644 --- a/modules/dispatch/src/lib.rs +++ b/modules/dispatch/src/lib.rs @@ -556,7 +556,7 @@ mod tests { type AccountIdConverter = AccountIdConverter; } - #[derive(Decode, Encode)] + #[derive(Decode, Encode, Clone)] pub struct EncodedCall(Vec); impl From for Result { @@ -568,6 +568,13 @@ mod tests { pub struct CallValidator; impl CallValidate for CallValidator { fn pre_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 8497b512c..1b3cb46d8 100644 --- a/modules/fee-market/src/tests.rs +++ b/modules/fee-market/src/tests.rs @@ -341,6 +341,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/mock.rs b/modules/messages/src/mock.rs index a333c95bb..a7391e90d 100644 --- a/modules/messages/src/mock.rs +++ b/modules/messages/src/mock.rs @@ -482,6 +482,13 @@ impl MessageDispatch for TestMessageDispatch { } } + fn pre_dispatch( + _relayer_account: &AccountId, + _message: &DispatchMessage, + ) -> Result<(), &'static str> { + Ok(()) + } + fn dispatch( _relayer_account: &AccountId, message: DispatchMessage, From 30ca1223f0a5f04d42ae685e6ad60a01134b3b13 Mon Sep 17 00:00:00 2001 From: HackFisher Date: Fri, 17 Jun 2022 20:04:37 +0800 Subject: [PATCH 11/11] update docs and partially distinguish naming between CallValidator and MessageDispatch --- modules/dispatch/src/lib.rs | 4 ++-- primitives/message-dispatch/src/lib.rs | 12 ++++++------ primitives/messages/src/target_chain.rs | 6 +++--- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/modules/dispatch/src/lib.rs b/modules/dispatch/src/lib.rs index b111e0aa0..ac404c5b3 100644 --- a/modules/dispatch/src/lib.rs +++ b/modules/dispatch/src/lib.rs @@ -166,7 +166,7 @@ impl, I: 'static> MessageDispatch let raw_message = message.map_err(|_| "Invalid Message")?; let call = raw_message.clone().call.into().map_err(|_| "Invalid Call")?; - T::CallValidator::pre_dispatch(relayer_account, &call) + T::CallValidator::check_receiving_before_dispatch(relayer_account, &call) } fn dispatch Result<(), ()>>( @@ -567,7 +567,7 @@ 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> { diff --git a/primitives/message-dispatch/src/lib.rs b/primitives/message-dispatch/src/lib.rs index cff062705..2cde19ffb 100644 --- a/primitives/message-dispatch/src/lib.rs +++ b/primitives/message-dispatch/src/lib.rs @@ -46,10 +46,10 @@ pub trait MessageDispatch { /// of dispatch weight. fn dispatch_weight(message: &Self::Message) -> Weight; - /// Pre-dispatch validation. + /// Checking in message receiving step before dispatch /// - /// This function is called before message is dispatched for necessary validation. if failed, - /// the message will not be 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, ()>, @@ -163,11 +163,11 @@ pub trait IntoDispatchOrigin { /// A generic trait to validate message before dispatch. pub trait CallValidate { - /// Pre-dispatch call validation. + /// Checking in message receiving step before dispatch /// /// This will be called before the call enter dispatch phase. If failed, the message(call) will - /// be rejected. - fn pre_dispatch(relayer_account: &AccountId, call: &Call) -> Result<(), &'static str>; + /// 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. diff --git a/primitives/messages/src/target_chain.rs b/primitives/messages/src/target_chain.rs index b39f3a1bc..685a4e9a8 100644 --- a/primitives/messages/src/target_chain.rs +++ b/primitives/messages/src/target_chain.rs @@ -97,10 +97,10 @@ pub trait MessageDispatch { /// of dispatch weight. fn dispatch_weight(message: &DispatchMessage) -> Weight; - /// pre-validate message. + /// Checking in message receiving step before dispatch /// - /// This function accepts the same params with the dispatch function, and called before - /// message is dispatched for necessary validation. + /// 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,