From a46069ff4c9eec69c818fafda46abb57fc116943 Mon Sep 17 00:00:00 2001 From: Xavier Lau Date: Thu, 15 Sep 2022 17:01:23 +0800 Subject: [PATCH] Companion of paritytech/parity-bridges-common#962 --- frame/bridge/ethereum/src/lib.rs | 2 +- frame/wormhole/backing/parachain/src/lib.rs | 20 ++++++++------ node/runtime/common/src/impls.rs | 29 ++++++++++++++++----- 3 files changed, 36 insertions(+), 15 deletions(-) diff --git a/frame/bridge/ethereum/src/lib.rs b/frame/bridge/ethereum/src/lib.rs index 7bf8c2abef..50950a9c58 100644 --- a/frame/bridge/ethereum/src/lib.rs +++ b/frame/bridge/ethereum/src/lib.rs @@ -982,7 +982,7 @@ impl SignedExtension for CheckEthereumRelayH info: &DispatchInfoOf, len: usize, ) -> Result { - Ok(self.validate(who, call, info, len).map(|_| ())) + self.validate(who, call, info, len).map(|_| ()) } fn additional_signed(&self) -> Result { diff --git a/frame/wormhole/backing/parachain/src/lib.rs b/frame/wormhole/backing/parachain/src/lib.rs index 617e7a01c5..220b5d7ca4 100644 --- a/frame/wormhole/backing/parachain/src/lib.rs +++ b/frame/wormhole/backing/parachain/src/lib.rs @@ -123,6 +123,7 @@ pub mod pallet { /// The message bridge instance to send message type MessagesBridge: MessagesBridge< + Self::Origin, Self::AccountId, RingBalance, <::OutboundPayloadCreator as IssueFromRemotePayload< @@ -158,6 +159,8 @@ pub mod pallet { RingDailyLimited, /// Message nonce duplicated. NonceDuplicated, + // Remote mapping token factory account not set. + RemoteMappingTokenFactoryAccountNotSet, } /// Period between security limitation. Zero means there is no period limitation. @@ -186,13 +189,13 @@ pub mod pallet { #[pallet::storage] #[pallet::getter(fn remote_mapping_token_factory_account)] pub type RemoteMappingTokenFactoryAccount = - StorageValue<_, AccountId, ValueQuery>; + StorageValue<_, AccountId, OptionQuery>; #[pallet::genesis_config] pub struct GenesisConfig { pub secure_limited_period: BlockNumberFor, pub secure_limited_ring_amount: RingBalance, - pub remote_mapping_token_factory_account: AccountId, + pub remote_mapping_token_factory_account: Option>, } #[cfg(feature = "std")] impl Default for GenesisConfig { @@ -200,7 +203,7 @@ pub mod pallet { Self { secure_limited_period: Zero::zero(), secure_limited_ring_amount: Zero::zero(), - remote_mapping_token_factory_account: Default::default(), + remote_mapping_token_factory_account: None, } } } @@ -212,9 +215,9 @@ pub mod pallet { >::zero(), self.secure_limited_ring_amount, )); - >::put( - self.remote_mapping_token_factory_account.clone(), - ); + if let Some(a) = self.remote_mapping_token_factory_account.clone() { + >::put(a); + } } } @@ -277,7 +280,7 @@ pub mod pallet { DispatchFeePayment::AtSourceChain, )?; T::MessagesBridge::send_message( - RawOrigin::Signed(Self::pallet_account_id()), + RawOrigin::Signed(Self::pallet_account_id()).into(), T::MessageLaneId::get(), payload, fee, @@ -309,7 +312,8 @@ pub mod pallet { // Check call origin ensure_source_account::( T::BridgedChainId::get(), - >::get(), + >::get() + .ok_or(>::RemoteMappingTokenFactoryAccountNotSet)?, &user, )?; diff --git a/node/runtime/common/src/impls.rs b/node/runtime/common/src/impls.rs index 193668c933..192f737f2e 100644 --- a/node/runtime/common/src/impls.rs +++ b/node/runtime/common/src/impls.rs @@ -124,6 +124,7 @@ impl Get> for OffchainRandomBalancing { pub struct FromThisChainMessageVerifier(PhantomData<(B, R, I)>); impl LaneMessageVerifier< + OriginOf>, AccountIdOf>, FromThisChainMessagePayload, BalanceOf>, @@ -132,6 +133,9 @@ where B: MessageBridge, R: pallet_fee_market::Config, I: 'static, + // matches requirements from the `frame_system::Config::Origin` + OriginOf>: Clone + + Into>>, OriginOf>>>, AccountIdOf>: Clone + PartialEq, pallet_fee_market::BalanceOf: From>>, { @@ -139,15 +143,15 @@ where #[cfg(not(feature = "runtime-benchmarks"))] fn verify_message( - submitter: &Sender>>, + submitter: &OriginOf>, delivery_and_dispatch_fee: &BalanceOf>, lane: &LaneId, lane_outbound_data: &OutboundLaneData, payload: &FromThisChainMessagePayload, ) -> Result<(), Self::Error> { // reject message if lane is blocked - if !ThisChain::::is_outbound_lane_enabled(lane) { - return Err(OUTBOUND_LANE_DISABLED); + if !ThisChain::::is_message_accepted(submitter, lane) { + return Err(MESSAGE_REJECTED_BY_OUTBOUND_LANE); } // reject message if there are too many pending messages at this lane @@ -161,8 +165,21 @@ where // Do the dispatch-specific check. We assume that the target chain uses // `Dispatch`, so we verify the message accordingly. - pallet_bridge_dispatch::verify_message_origin(submitter, payload) - .map_err(|_| BAD_ORIGIN)?; + let raw_origin_or_err: Result< + frame_system::RawOrigin>>, + OriginOf>, + > = submitter.clone().into(); + if let Ok(raw_origin) = raw_origin_or_err { + pallet_bridge_dispatch::verify_message_origin(&raw_origin, payload) + .map(drop) + .map_err(|_| BAD_ORIGIN)? + } else { + // so what it means that we've failed to convert origin to the + // `frame_system::RawOrigin`? now it means that the custom pallet origin has + // been used to send the message. Do we need to verify it? The answer is no, + // because pallet may craft any origin (e.g. root) && we can't verify whether it + // is valid, or not. + } // Do the delivery_and_dispatch_fee. We assume that the delivery and dispatch fee always // greater than the fee market provided fee. @@ -185,7 +202,7 @@ where #[cfg(feature = "runtime-benchmarks")] fn verify_message( - _submitter: &Sender>>, + _submitter: &OriginOf>, _delivery_and_dispatch_fee: &BalanceOf>, _lane: &LaneId, _lane_outbound_data: &OutboundLaneData,