From 8e1101a9e186889f9e85f3a3ab56025c2e3d3add Mon Sep 17 00:00:00 2001 From: gui Date: Thu, 19 Sep 2024 22:06:39 +0900 Subject: [PATCH 1/2] upgrade --- bridges/bin/runtime-common/src/extensions.rs | 43 ++++++------- bridges/modules/relayers/src/extension/mod.rs | 60 +++++++++++-------- bridges/primitives/relayers/src/extension.rs | 2 +- 3 files changed, 54 insertions(+), 51 deletions(-) diff --git a/bridges/bin/runtime-common/src/extensions.rs b/bridges/bin/runtime-common/src/extensions.rs index 516b873a3f4e..a27cb1ce6cb5 100644 --- a/bridges/bin/runtime-common/src/extensions.rs +++ b/bridges/bin/runtime-common/src/extensions.rs @@ -47,8 +47,7 @@ pub trait BridgeRuntimeFilterCall { /// Data that may be passed from the validate to `post_dispatch`. type ToPostDispatch; /// Called during validation. Needs to checks whether a runtime call, submitted - /// by the `who` is valid. `who` may be `None` if transaction is not signed - /// by a regular account. + /// by the `who` is valid. fn validate(who: &AccountId, call: &Call) -> (Self::ToPostDispatch, TransactionValidity); /// Called after transaction is dispatched. fn post_dispatch(_who: &AccountId, _has_failed: bool, _to_post_dispatch: Self::ToPostDispatch) { @@ -277,7 +276,7 @@ macro_rules! generate_bridge_reject_obsolete_headers_and_messages { impl sp_runtime::traits::TransactionExtension<$call> for BridgeRejectObsoleteHeadersAndMessages { const IDENTIFIER: &'static str = "BridgeRejectObsoleteHeadersAndMessages"; type Implicit = (); - type Pre = Option<( + type Val = Option<( $account_id, ( $( <$filter_call as $crate::extensions::BridgeRuntimeFilterCall< @@ -286,7 +285,7 @@ macro_rules! generate_bridge_reject_obsolete_headers_and_messages { >>::ToPostDispatch, )* ), )>; - type Val = (); + type Pre = Self::Val; fn validate( &self, @@ -303,44 +302,37 @@ macro_rules! generate_bridge_reject_obsolete_headers_and_messages { <$call as sp_runtime::traits::Dispatchable>::RuntimeOrigin, ), sp_runtime::transaction_validity::TransactionValidityError > { + use $crate::extensions::__private::tuplex::PushBack; use sp_runtime::traits::AsSystemOriginSigner; + + let who = origin.as_system_origin_signer() else { + return Ok((Default::default(), None, origin)); + } + + let to_post_dispatch = (); let tx_validity = sp_runtime::transaction_validity::ValidTransaction::default(); - let who = origin.as_system_origin_signer().ok_or(sp_runtime::transaction_validity::InvalidTransaction::BadSigner)?; $( - let (_from_validate, call_filter_validity) = < + let (from_validate, call_filter_validity) = < $filter_call as $crate::extensions::BridgeRuntimeFilterCall< $account_id, $call, >>::validate(&who, call); + let to_post_dispatch = to_post_dispatch.push_back(from_validate); let tx_validity = tx_validity.combine_with(call_filter_validity?); )* - Ok((tx_validity, (), origin)) + Ok((tx_validity, Some((who, reto_post_dispatch)), origin)) } fn prepare( self, - _val: Self::Val, + val: Self::Val, origin: &<$call as sp_runtime::traits::Dispatchable>::RuntimeOrigin, call: &$call, _info: &sp_runtime::traits::DispatchInfoOf<$call>, _len: usize, ) -> Result { - use $crate::extensions::__private::tuplex::PushBack; - use sp_runtime::traits::AsSystemOriginSigner; - - let to_post_dispatch = (); - let relayer = origin.as_system_origin_signer().ok_or(sp_runtime::transaction_validity::InvalidTransaction::BadSigner)?; - $( - let (from_validate, _call_filter_validity) = < - $filter_call as - $crate::extensions::BridgeRuntimeFilterCall< - $account_id, - $call, - >>::validate(&relayer, call); - let to_post_dispatch = to_post_dispatch.push_back(from_validate); - )* - Ok(Some((relayer.clone(), to_post_dispatch))) + Ok(val) } #[allow(unused_variables)] @@ -353,7 +345,10 @@ macro_rules! generate_bridge_reject_obsolete_headers_and_messages { ) -> Result { use $crate::extensions::__private::tuplex::PopFront; - let Some((relayer, to_post_dispatch)) = to_post_dispatch else { return Ok(frame_support::pallet_prelude::Weight::zero()) }; + let Some((relayer, to_post_dispatch)) = to_post_dispatch else { + return Ok(frame_support::pallet_prelude::Weight::zero()) + }; + let has_failed = result.is_err(); $( let (item, to_post_dispatch) = to_post_dispatch.pop_front(); diff --git a/bridges/modules/relayers/src/extension/mod.rs b/bridges/modules/relayers/src/extension/mod.rs index 1a5149047330..32672f86686c 100644 --- a/bridges/modules/relayers/src/extension/mod.rs +++ b/bridges/modules/relayers/src/extension/mod.rs @@ -139,13 +139,12 @@ where /// virtually boosted. The relayer registration (we only boost priority for registered /// relayer transactions) must be checked outside. fn bundled_messages_for_priority_boost( - call_info: Option<&ExtensionCallInfo>, + parsed_call: &ExtensionCallInfo, ) -> Option { // we only boost priority of message delivery transactions - let parsed_call = match call_info { - Some(parsed_call) if parsed_call.is_receive_messages_proof_call() => parsed_call, - _ => return None, - }; + if !parsed_call.is_receive_messages_proof_call() { + return None; + } // compute total number of messages in transaction let bundled_messages = parsed_call.messages_call_info().bundled_messages().saturating_len(); @@ -163,7 +162,7 @@ where /// Given post-dispatch information, analyze the outcome of relayer call and return /// actions that need to be performed on relayer account. fn analyze_call_result( - pre: Option>>, + pre: Option>, info: &DispatchInfo, post_info: &PostDispatchInfo, len: usize, @@ -171,7 +170,7 @@ where ) -> RelayerAccountAction { // We don't refund anything for transactions that we don't support. let (relayer, call_info) = match pre { - Some(Some(pre)) => (pre.relayer, pre.call_info), + Some(pre) => (pre.relayer, pre.call_info), _ => return RelayerAccountAction::None, }; @@ -201,7 +200,7 @@ where // - when relayer is registered after `validate` is called and priority is not boosted: // relayer should be ready for slashing after registration. let may_slash_relayer = - Self::bundled_messages_for_priority_boost(Some(&call_info)).is_some(); + Self::bundled_messages_for_priority_boost(&call_info).is_some(); let slash_relayer_if_delivery_result = may_slash_relayer .then(|| RelayerAccountAction::Slash(relayer.clone(), reward_account_params)) .unwrap_or(RelayerAccountAction::None); @@ -281,7 +280,7 @@ where const IDENTIFIER: &'static str = C::IdProvider::STR; type Implicit = (); type Pre = Option>; - type Val = Option>; + type Val = Self::Pre; fn validate( &self, @@ -292,26 +291,36 @@ where _self_implicit: Self::Implicit, _inherited_implication: &impl Encode, ) -> ValidateResult { - let who = origin.as_system_origin_signer().ok_or(InvalidTransaction::BadSigner)?; // this is the only relevant line of code for the `pre_dispatch` // // we're not calling `validate` from `pre_dispatch` directly because of performance // reasons, so if you're adding some code that may fail here, please check if it needs // to be added to the `pre_dispatch` as well - let parsed_call = C::parse_and_check_for_obsolete_call(call)?; + let parsed_call = match C::parse_and_check_for_obsolete_call(call)? { + Some(parsed_call) => parsed_call, + None => return Ok((Default::default(), None, origin)), + }; + + // Those calls are only for signed transactions. + let relayer = origin.as_system_origin_signer().ok_or(InvalidTransaction::BadSigner)?; + + let data = PreDispatchData { + relayer: relayer.clone(), + call_info: parsed_call, + }; // the following code just plays with transaction priority and never returns an error // we only boost priority of presumably correct message delivery transactions - let bundled_messages = match Self::bundled_messages_for_priority_boost(parsed_call.as_ref()) + let bundled_messages = match Self::bundled_messages_for_priority_boost(&data.call_info) { Some(bundled_messages) => bundled_messages, - None => return Ok((Default::default(), parsed_call, origin)), + None => return Ok((Default::default(), Some(data), origin)), }; // we only boost priority if relayer has staked required balance - if !RelayersPallet::::is_registration_active(who) { - return Ok((Default::default(), parsed_call, origin)) + if !RelayersPallet::::is_registration_active(&data.relayer) { + return Ok((Default::default(), Some(data), origin)) } // compute priority boost @@ -324,34 +333,33 @@ where "{}.{:?}: has boosted priority of message delivery transaction \ of relayer {:?}: {} messages -> {} priority", Self::IDENTIFIER, - parsed_call.as_ref().map(|p| p.messages_call_info().lane_id()), - who, + data.call_info.messages_call_info().lane_id(), + data.relayer, bundled_messages, priority_boost, ); let validity = valid_transaction.build()?; - Ok((validity, parsed_call, origin)) + Ok((validity, Some(data), origin)) } fn prepare( self, val: Self::Val, - origin: &::RuntimeOrigin, + _origin: &::RuntimeOrigin, _call: &R::RuntimeCall, _info: &DispatchInfoOf, _len: usize, ) -> Result { - let who = origin.as_system_origin_signer().ok_or(InvalidTransaction::BadSigner)?; - Ok(val.map(|call_info| { + Ok(val.map(|data| { log::trace!( target: LOG_TARGET, - "{}.{:?}: parsed bridge transaction in pre-dispatch: {:?}", + "{}.{:?}: parsed bridge transaction in prepare: {:?}", Self::IDENTIFIER, - call_info.messages_call_info().lane_id(), - call_info, + data.call_info.messages_call_info().lane_id(), + data.call_info, ); - PreDispatchData { relayer: who.clone(), call_info } + data })) } @@ -363,7 +371,7 @@ where result: &DispatchResult, ) -> Result { let lane_id = pre.as_ref().map(|p| p.call_info.messages_call_info().lane_id()); - let call_result = Self::analyze_call_result(Some(pre), info, post_info, len, result); + let call_result = Self::analyze_call_result(pre, info, post_info, len, result); match call_result { RelayerAccountAction::None => (), diff --git a/bridges/primitives/relayers/src/extension.rs b/bridges/primitives/relayers/src/extension.rs index 5ab8e6cde96b..8c473c894093 100644 --- a/bridges/primitives/relayers/src/extension.rs +++ b/bridges/primitives/relayers/src/extension.rs @@ -132,7 +132,7 @@ pub trait ExtensionConfig { /// GRANDPA chain, it must be it. type RemoteGrandpaChainBlockNumber: Clone + Copy + Debug; - /// Given runtime call, check if it is supported by the signed extension. Additionally, + /// Given runtime call, check if it is supported by the transaction extension. Additionally, /// check if call (or any of batched calls) are obsolete. fn parse_and_check_for_obsolete_call( call: &::RuntimeCall, From 370157cd72301cc4aad2481daa5c296398e85b5b Mon Sep 17 00:00:00 2001 From: gui Date: Fri, 20 Sep 2024 12:26:51 +0900 Subject: [PATCH 2/2] fix --- bridges/bin/runtime-common/src/extensions.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/bridges/bin/runtime-common/src/extensions.rs b/bridges/bin/runtime-common/src/extensions.rs index a27cb1ce6cb5..cfeab51e65d1 100644 --- a/bridges/bin/runtime-common/src/extensions.rs +++ b/bridges/bin/runtime-common/src/extensions.rs @@ -305,9 +305,9 @@ macro_rules! generate_bridge_reject_obsolete_headers_and_messages { use $crate::extensions::__private::tuplex::PushBack; use sp_runtime::traits::AsSystemOriginSigner; - let who = origin.as_system_origin_signer() else { + let Some(who) = origin.as_system_origin_signer() else { return Ok((Default::default(), None, origin)); - } + }; let to_post_dispatch = (); let tx_validity = sp_runtime::transaction_validity::ValidTransaction::default(); @@ -317,11 +317,11 @@ macro_rules! generate_bridge_reject_obsolete_headers_and_messages { $crate::extensions::BridgeRuntimeFilterCall< $account_id, $call, - >>::validate(&who, call); + >>::validate(who, call); let to_post_dispatch = to_post_dispatch.push_back(from_validate); let tx_validity = tx_validity.combine_with(call_filter_validity?); )* - Ok((tx_validity, Some((who, reto_post_dispatch)), origin)) + Ok((tx_validity, Some((who.clone(), to_post_dispatch)), origin)) } fn prepare(