diff --git a/bridges/bin/millau/runtime/src/lib.rs b/bridges/bin/millau/runtime/src/lib.rs index 36ce83049d1c5ba52d1cafa791d7ad5736181d49..fc2f19b945889d82f1e73724bc4e3bf6e25d6117 100644 --- a/bridges/bin/millau/runtime/src/lib.rs +++ b/bridges/bin/millau/runtime/src/lib.rs @@ -78,7 +78,7 @@ pub use frame_support::{ constants::WEIGHT_PER_SECOND, ConstantMultiplier, DispatchClass, IdentityFee, RuntimeDbWeight, Weight, }, - StorageValue, + RuntimeDebug, StorageValue, }; pub use frame_system::Call as SystemCall; @@ -89,6 +89,7 @@ pub use pallet_bridge_parachains::Call as BridgeParachainsCall; pub use pallet_sudo::Call as SudoCall; pub use pallet_timestamp::Call as TimestampCall; +use bridge_runtime_common::generate_bridge_reject_obsolete_headers_and_messages; #[cfg(any(feature = "std", test))] pub use sp_runtime::BuildStorage; pub use sp_runtime::{Perbill, Permill}; @@ -592,21 +593,14 @@ construct_runtime!( } ); -pallet_bridge_grandpa::declare_bridge_reject_obsolete_grandpa_header! { - Runtime, - Call::BridgeRialtoGrandpa => RialtoGrandpaInstance, - Call::BridgeWestendGrandpa => WestendGrandpaInstance -} - -pallet_bridge_parachains::declare_bridge_reject_obsolete_parachain_header! { - Runtime, - Call::BridgeRialtoParachains => WithRialtoParachainsInstance -} - -bridge_runtime_common::declare_bridge_reject_obsolete_messages! { - Runtime, - Call::BridgeRialtoMessages => WithRialtoMessagesInstance, - Call::BridgeRialtoParachainMessages => WithRialtoParachainMessagesInstance +generate_bridge_reject_obsolete_headers_and_messages! { + Call, AccountId, + // Grandpa + BridgeRialtoGrandpa, BridgeWestendGrandpa, + // Parachains + BridgeRialtoParachains, + //Messages + BridgeRialtoMessages, BridgeRialtoParachainMessages } /// The address format for describing accounts. @@ -629,9 +623,7 @@ pub type SignedExtra = ( frame_system::CheckNonce<Runtime>, frame_system::CheckWeight<Runtime>, pallet_transaction_payment::ChargeTransactionPayment<Runtime>, - BridgeRejectObsoleteGrandpaHeader, - BridgeRejectObsoleteParachainHeader, - BridgeRejectObsoleteMessages, + BridgeRejectObsoleteHeadersAndMessages, ); /// The payload being signed in transactions. pub type SignedPayload = generic::SignedPayload<Call, SignedExtra>; diff --git a/bridges/bin/runtime-common/src/lib.rs b/bridges/bin/runtime-common/src/lib.rs index 9776fd5827d820258b2cf5c2faee6244a4cce8b4..5b2cea8ddd7ff3739e746d454b5dba0b4b20f836 100644 --- a/bridges/bin/runtime-common/src/lib.rs +++ b/bridges/bin/runtime-common/src/lib.rs @@ -18,6 +18,9 @@ #![cfg_attr(not(feature = "std"), no_std)] +use bp_runtime::FilterCall; +use sp_runtime::transaction_validity::TransactionValidity; + pub mod messages; pub mod messages_api; pub mod messages_benchmarking; @@ -26,3 +29,167 @@ pub mod parachains_benchmarking; #[cfg(feature = "integrity-test")] pub mod integrity; + +/// A duplication of the `FilterCall` trait. +/// +/// We need this trait in order to be able to implement it for the messages pallet, +/// since the implementation is done outside of the pallet crate. +pub trait BridgeRuntimeFilterCall<Call> { + /// Checks if a runtime call is valid. + fn validate(call: &Call) -> TransactionValidity; +} + +impl<Call, T, I> BridgeRuntimeFilterCall<Call> for pallet_bridge_grandpa::Pallet<T, I> +where + pallet_bridge_grandpa::Pallet<T, I>: FilterCall<Call>, +{ + fn validate(call: &Call) -> TransactionValidity { + <pallet_bridge_grandpa::Pallet<T, I> as FilterCall<Call>>::validate(call) + } +} + +impl<Call, T, I> BridgeRuntimeFilterCall<Call> for pallet_bridge_parachains::Pallet<T, I> +where + pallet_bridge_parachains::Pallet<T, I>: FilterCall<Call>, +{ + fn validate(call: &Call) -> TransactionValidity { + <pallet_bridge_parachains::Pallet<T, I> as FilterCall<Call>>::validate(call) + } +} + +/// Declares a runtime-specific `BridgeRejectObsoleteHeadersAndMessages` signed extension. +/// +/// ## Example +/// +/// ```nocompile +/// generate_bridge_reject_obsolete_headers_and_messages!{ +/// Call, AccountId +/// BridgeRialtoGrandpa, BridgeWestendGrandpa, +/// BridgeRialtoParachains +/// } +/// ``` +/// +/// The goal of this extension is to avoid "mining" transactions that provide outdated bridged +/// headers and messages. Without that extension, even honest relayers may lose their funds if +/// there are multiple relays running and submitting the same information. +#[macro_export] +macro_rules! generate_bridge_reject_obsolete_headers_and_messages { + ($call:ty, $account_id:ty, $($filter_call:ty),*) => { + #[derive(Clone, codec::Decode, codec::Encode, Eq, PartialEq, frame_support::RuntimeDebug, scale_info::TypeInfo)] + pub struct BridgeRejectObsoleteHeadersAndMessages; + impl sp_runtime::traits::SignedExtension for BridgeRejectObsoleteHeadersAndMessages { + const IDENTIFIER: &'static str = "BridgeRejectObsoleteHeadersAndMessages"; + type AccountId = $account_id; + type Call = $call; + type AdditionalSigned = (); + type Pre = (); + + fn additional_signed(&self) -> sp_std::result::Result< + (), + sp_runtime::transaction_validity::TransactionValidityError, + > { + Ok(()) + } + + fn validate( + &self, + _who: &Self::AccountId, + call: &Self::Call, + _info: &sp_runtime::traits::DispatchInfoOf<Self::Call>, + _len: usize, + ) -> sp_runtime::transaction_validity::TransactionValidity { + let valid = sp_runtime::transaction_validity::ValidTransaction::default(); + $( + let valid = valid + .combine_with(<$filter_call as $crate::BridgeRuntimeFilterCall<$call>>::validate(call)?); + )* + Ok(valid) + } + + fn pre_dispatch( + self, + who: &Self::AccountId, + call: &Self::Call, + info: &sp_runtime::traits::DispatchInfoOf<Self::Call>, + len: usize, + ) -> Result<Self::Pre, sp_runtime::transaction_validity::TransactionValidityError> { + self.validate(who, call, info, len).map(drop) + } + } + }; +} + +#[cfg(test)] +mod tests { + use crate::BridgeRuntimeFilterCall; + use frame_support::{assert_err, assert_ok}; + use sp_runtime::{ + traits::SignedExtension, + transaction_validity::{InvalidTransaction, TransactionValidity, ValidTransaction}, + }; + + pub struct MockCall { + data: u32, + } + + impl sp_runtime::traits::Dispatchable for MockCall { + type Origin = (); + type Config = (); + type Info = (); + type PostInfo = (); + + fn dispatch( + self, + _origin: Self::Origin, + ) -> sp_runtime::DispatchResultWithInfo<Self::PostInfo> { + unimplemented!() + } + } + + struct FirstFilterCall; + impl BridgeRuntimeFilterCall<MockCall> for FirstFilterCall { + fn validate(call: &MockCall) -> TransactionValidity { + if call.data <= 1 { + return InvalidTransaction::Custom(1).into() + } + + Ok(ValidTransaction { priority: 1, ..Default::default() }) + } + } + + struct SecondFilterCall; + impl BridgeRuntimeFilterCall<MockCall> for SecondFilterCall { + fn validate(call: &MockCall) -> TransactionValidity { + if call.data <= 2 { + return InvalidTransaction::Custom(2).into() + } + + Ok(ValidTransaction { priority: 2, ..Default::default() }) + } + } + + #[test] + fn test() { + generate_bridge_reject_obsolete_headers_and_messages!( + MockCall, + (), + FirstFilterCall, + SecondFilterCall + ); + + assert_err!( + BridgeRejectObsoleteHeadersAndMessages.validate(&(), &MockCall { data: 1 }, &(), 0), + InvalidTransaction::Custom(1) + ); + + assert_err!( + BridgeRejectObsoleteHeadersAndMessages.validate(&(), &MockCall { data: 2 }, &(), 0), + InvalidTransaction::Custom(2) + ); + + assert_ok!( + BridgeRejectObsoleteHeadersAndMessages.validate(&(), &MockCall { data: 3 }, &(), 0), + ValidTransaction { priority: 3, ..Default::default() } + ) + } +} diff --git a/bridges/bin/runtime-common/src/messages_extension.rs b/bridges/bin/runtime-common/src/messages_extension.rs index 48b30fa646e2a92653e09a9e02dda5bf6fcf4615..95a8b947e74bedfd3377a3d1063d7952856eabdd 100644 --- a/bridges/bin/runtime-common/src/messages_extension.rs +++ b/bridges/bin/runtime-common/src/messages_extension.rs @@ -14,136 +14,100 @@ // You should have received a copy of the GNU General Public License // along with Parity Bridges Common. If not, see <http://www.gnu.org/licenses/>. -/// Declares a runtime-specific `BridgeRejectObsoleteMessages` and -/// `BridgeRejectObsoleteMessageConfirmations` signed extensions. -/// -/// ## Example -/// -/// ```nocompile -/// bridge_runtime_common::declare_bridge_reject_obsolete_messages!{ -/// Runtime, -/// Call::BridgeRialtoMessages => WithRialtoMessagesInstance, -/// Call::BridgeRialtoParachainMessages => WithRialtoParachainMessagesInstance, -/// } -/// ``` -/// -/// The goal of this extension is to avoid "mining" messages delivery and delivery confirmation -/// transactions, that are delivering outdated messages/confirmations. Without that extension, +use crate::{ + messages::{ + source::FromBridgedChainMessagesDeliveryProof, target::FromBridgedChainMessagesProof, + }, + BridgeRuntimeFilterCall, +}; +use frame_support::{dispatch::CallableCallFor, traits::IsSubType}; +use pallet_bridge_messages::{Config, Pallet}; +use sp_runtime::transaction_validity::TransactionValidity; + +/// Validate messages in order to avoid "mining" messages delivery and delivery confirmation +/// transactions, that are delivering outdated messages/confirmations. Without this validation, /// even honest relayers may lose their funds if there are multiple relays running and submitting /// the same messages/confirmations. -#[macro_export] -macro_rules! declare_bridge_reject_obsolete_messages { - ($runtime:ident, $($call:path => $instance:ty),*) => { - /// Transaction-with-obsolete-messages check that will reject transaction if - /// it submits obsolete messages/confirmations. - #[derive(Clone, codec::Decode, codec::Encode, Eq, PartialEq, frame_support::RuntimeDebug, scale_info::TypeInfo)] - pub struct BridgeRejectObsoleteMessages; - - impl sp_runtime::traits::SignedExtension for BridgeRejectObsoleteMessages { - const IDENTIFIER: &'static str = "BridgeRejectObsoleteMessages"; - type AccountId = <$runtime as frame_system::Config>::AccountId; - type Call = <$runtime as frame_system::Config>::Call; - type AdditionalSigned = (); - type Pre = (); - - fn additional_signed(&self) -> sp_std::result::Result< - (), - sp_runtime::transaction_validity::TransactionValidityError, - > { - Ok(()) - } - - fn validate( - &self, - _who: &Self::AccountId, - call: &Self::Call, - _info: &sp_runtime::traits::DispatchInfoOf<Self::Call>, - _len: usize, - ) -> sp_runtime::transaction_validity::TransactionValidity { - match *call { - $( - $call(pallet_bridge_messages::Call::<$runtime, $instance>::receive_messages_proof { - ref proof, - .. - }) => { - let nonces_end = proof.nonces_end; - - let inbound_lane_data = pallet_bridge_messages::InboundLanes::<$runtime, $instance>::get(&proof.lane); - if proof.nonces_end <= inbound_lane_data.last_delivered_nonce() { - log::trace!( - target: pallet_bridge_messages::LOG_TARGET, - "Rejecting obsolete messages delivery transaction: lane {:?}, bundled {:?}, best {:?}", - proof.lane, - proof.nonces_end, - inbound_lane_data.last_delivered_nonce(), - ); - - return sp_runtime::transaction_validity::InvalidTransaction::Stale.into(); - } - - Ok(sp_runtime::transaction_validity::ValidTransaction::default()) - }, - $call(pallet_bridge_messages::Call::<$runtime, $instance>::receive_messages_delivery_proof { - ref proof, - ref relayers_state, - .. - }) => { - let latest_delivered_nonce = relayers_state.last_delivered_nonce; - - let outbound_lane_data = pallet_bridge_messages::OutboundLanes::<$runtime, $instance>::get(&proof.lane); - if latest_delivered_nonce <= outbound_lane_data.latest_received_nonce { - log::trace!( - target: pallet_bridge_messages::LOG_TARGET, - "Rejecting obsolete messages confirmation transaction: lane {:?}, bundled {:?}, best {:?}", - proof.lane, - latest_delivered_nonce, - outbound_lane_data.latest_received_nonce, - ); - - return sp_runtime::transaction_validity::InvalidTransaction::Stale.into(); - } - - Ok(sp_runtime::transaction_validity::ValidTransaction::default()) - } - )* - _ => Ok(sp_runtime::transaction_validity::ValidTransaction::default()), +impl< + BridgedHeaderHash, + SourceHeaderChain: bp_messages::target_chain::SourceHeaderChain< + <T as Config<I>>::InboundMessageFee, + MessagesProof = FromBridgedChainMessagesProof<BridgedHeaderHash>, + >, + TargetHeaderChain: bp_messages::source_chain::TargetHeaderChain< + <T as Config<I>>::OutboundPayload, + <T as frame_system::Config>::AccountId, + MessagesDeliveryProof = FromBridgedChainMessagesDeliveryProof<BridgedHeaderHash>, + >, + Call: IsSubType<CallableCallFor<Pallet<T, I>, T>>, + T: frame_system::Config<Call = Call> + + Config<I, SourceHeaderChain = SourceHeaderChain, TargetHeaderChain = TargetHeaderChain>, + I: 'static, + > BridgeRuntimeFilterCall<Call> for Pallet<T, I> +{ + fn validate(call: &Call) -> TransactionValidity { + match call.is_sub_type() { + Some(pallet_bridge_messages::Call::<T, I>::receive_messages_proof { + ref proof, + .. + }) => { + let inbound_lane_data = + pallet_bridge_messages::InboundLanes::<T, I>::get(&proof.lane); + if proof.nonces_end <= inbound_lane_data.last_delivered_nonce() { + log::trace!( + target: pallet_bridge_messages::LOG_TARGET, + "Rejecting obsolete messages delivery transaction: \ + lane {:?}, bundled {:?}, best {:?}", + proof.lane, + proof.nonces_end, + inbound_lane_data.last_delivered_nonce(), + ); + + return sp_runtime::transaction_validity::InvalidTransaction::Stale.into() } - } - - fn pre_dispatch( - self, - who: &Self::AccountId, - call: &Self::Call, - info: &sp_runtime::traits::DispatchInfoOf<Self::Call>, - len: usize, - ) -> Result<Self::Pre, sp_runtime::transaction_validity::TransactionValidityError> { - self.validate(who, call, info, len).map(drop) - } - - fn post_dispatch( - _maybe_pre: Option<Self::Pre>, - _info: &sp_runtime::traits::DispatchInfoOf<Self::Call>, - _post_info: &sp_runtime::traits::PostDispatchInfoOf<Self::Call>, - _len: usize, - _result: &sp_runtime::DispatchResult, - ) -> Result<(), sp_runtime::transaction_validity::TransactionValidityError> { - Ok(()) - } + }, + Some(pallet_bridge_messages::Call::<T, I>::receive_messages_delivery_proof { + ref proof, + ref relayers_state, + .. + }) => { + let latest_delivered_nonce = relayers_state.last_delivered_nonce; + + let outbound_lane_data = + pallet_bridge_messages::OutboundLanes::<T, I>::get(&proof.lane); + if latest_delivered_nonce <= outbound_lane_data.latest_received_nonce { + log::trace!( + target: pallet_bridge_messages::LOG_TARGET, + "Rejecting obsolete messages confirmation transaction: \ + lane {:?}, bundled {:?}, best {:?}", + proof.lane, + latest_delivered_nonce, + outbound_lane_data.latest_received_nonce, + ); + + return sp_runtime::transaction_validity::InvalidTransaction::Stale.into() + } + }, + _ => {}, } - }; + + Ok(sp_runtime::transaction_validity::ValidTransaction::default()) + } } #[cfg(test)] mod tests { use bp_messages::UnrewardedRelayersState; - use frame_support::weights::{DispatchClass, DispatchInfo, Pays}; use millau_runtime::{ - bridge_runtime_common::messages::{ - source::FromBridgedChainMessagesDeliveryProof, target::FromBridgedChainMessagesProof, + bridge_runtime_common::{ + messages::{ + source::FromBridgedChainMessagesDeliveryProof, + target::FromBridgedChainMessagesProof, + }, + BridgeRuntimeFilterCall, }, - BridgeRejectObsoleteMessages, Call, Runtime, WithRialtoMessagesInstance, + Call, Runtime, WithRialtoMessagesInstance, }; - use sp_runtime::traits::SignedExtension; fn deliver_message_10() { pallet_bridge_messages::InboundLanes::<Runtime, WithRialtoMessagesInstance>::insert( @@ -156,13 +120,9 @@ mod tests { nonces_start: bp_messages::MessageNonce, nonces_end: bp_messages::MessageNonce, ) -> bool { - BridgeRejectObsoleteMessages - .validate( - &[0u8; 32].into(), - &Call::BridgeRialtoMessages(pallet_bridge_messages::Call::< - Runtime, - WithRialtoMessagesInstance, - >::receive_messages_proof { + pallet_bridge_messages::Pallet::<Runtime, WithRialtoMessagesInstance>::validate( + &Call::BridgeRialtoMessages( + pallet_bridge_messages::Call::<Runtime, ()>::receive_messages_proof { relayer_id_at_bridged_chain: [0u8; 32].into(), messages_count: (nonces_end - nonces_start + 1) as u32, dispatch_weight: 0, @@ -173,11 +133,10 @@ mod tests { nonces_start, nonces_end, }, - }), - &DispatchInfo { weight: 0, class: DispatchClass::Operational, pays_fee: Pays::Yes }, - 0, - ) - .is_ok() + }, + ), + ) + .is_ok() } #[test] @@ -222,27 +181,23 @@ mod tests { } fn validate_message_confirmation(last_delivered_nonce: bp_messages::MessageNonce) -> bool { - BridgeRejectObsoleteMessages - .validate( - &[0u8; 32].into(), - &Call::BridgeRialtoMessages(pallet_bridge_messages::Call::< - Runtime, - WithRialtoMessagesInstance, - >::receive_messages_delivery_proof { - proof: FromBridgedChainMessagesDeliveryProof { - bridged_header_hash: Default::default(), - storage_proof: Vec::new(), - lane: [0, 0, 0, 0], - }, - relayers_state: UnrewardedRelayersState { - last_delivered_nonce, - ..Default::default() - }, - }), - &DispatchInfo { weight: 0, class: DispatchClass::Operational, pays_fee: Pays::Yes }, - 0, - ) - .is_ok() + pallet_bridge_messages::Pallet::<Runtime, WithRialtoMessagesInstance>::validate( + &Call::BridgeRialtoMessages(pallet_bridge_messages::Call::< + Runtime, + WithRialtoMessagesInstance, + >::receive_messages_delivery_proof { + proof: FromBridgedChainMessagesDeliveryProof { + bridged_header_hash: Default::default(), + storage_proof: Vec::new(), + lane: [0, 0, 0, 0], + }, + relayers_state: UnrewardedRelayersState { + last_delivered_nonce, + ..Default::default() + }, + }), + ) + .is_ok() } #[test] diff --git a/bridges/modules/grandpa/src/extension.rs b/bridges/modules/grandpa/src/extension.rs index 007c5f3df7475caf38280d1483034753674fe6c7..a724b6518d84bdc2cc19997baca753ff86ad653f 100644 --- a/bridges/modules/grandpa/src/extension.rs +++ b/bridges/modules/grandpa/src/extension.rs @@ -14,132 +14,68 @@ // You should have received a copy of the GNU General Public License // along with Parity Bridges Common. If not, see <http://www.gnu.org/licenses/>. -/// Declares a runtime-specific `BridgeRejectObsoleteGrandpaHeader` signed extension. -/// -/// ## Example -/// -/// ```nocompile -/// pallet_bridge_grandpa::declare_bridge_reject_obsolete_grandpa_header!{ -/// Runtime, -/// Call::BridgeRialtoGrandpa => RialtoGrandpaInstance, -/// Call::BridgeWestendGrandpa => WestendGrandpaInstance, -/// } -/// ``` -/// -/// The goal of this extension is to avoid "mining" transactions that provide -/// outdated bridged chain headers. Without that extension, even honest relayers -/// may lose their funds if there are multiple relays running and submitting the -/// same information. -#[macro_export] -macro_rules! declare_bridge_reject_obsolete_grandpa_header { - ($runtime:ident, $($call:path => $instance:ty),*) => { - /// Transaction-with-obsolete-bridged-header check that will reject transaction if - /// it submits obsolete bridged header. - #[derive(Clone, codec::Decode, codec::Encode, Eq, PartialEq, frame_support::RuntimeDebug, scale_info::TypeInfo)] - pub struct BridgeRejectObsoleteGrandpaHeader; - - impl sp_runtime::traits::SignedExtension for BridgeRejectObsoleteGrandpaHeader { - const IDENTIFIER: &'static str = "BridgeRejectObsoleteGrandpaHeader"; - type AccountId = <$runtime as frame_system::Config>::AccountId; - type Call = <$runtime as frame_system::Config>::Call; - type AdditionalSigned = (); - type Pre = (); - - fn additional_signed(&self) -> sp_std::result::Result< - (), - sp_runtime::transaction_validity::TransactionValidityError, - > { - Ok(()) - } - - fn validate( - &self, - _who: &Self::AccountId, - call: &Self::Call, - _info: &sp_runtime::traits::DispatchInfoOf<Self::Call>, - _len: usize, - ) -> sp_runtime::transaction_validity::TransactionValidity { - match *call { - $( - $call($crate::Call::<$runtime, $instance>::submit_finality_proof { ref finality_target, ..}) => { - use sp_runtime::traits::Header as HeaderT; - - let bundled_block_number = *finality_target.number(); - - let best_finalized = $crate::BestFinalized::<$runtime, $instance>::get(); - let best_finalized_number = match best_finalized { - Some((best_finalized_number, _)) => best_finalized_number, - None => return sp_runtime::transaction_validity::InvalidTransaction::Call.into(), - }; - - if best_finalized_number < bundled_block_number { - Ok(sp_runtime::transaction_validity::ValidTransaction::default()) - } else { - log::trace!( - target: $crate::LOG_TARGET, - "Rejecting obsolete bridged header: bundled {:?}, best {:?}", - bundled_block_number, - best_finalized_number, - ); - - sp_runtime::transaction_validity::InvalidTransaction::Stale.into() - } - }, - )* - _ => Ok(sp_runtime::transaction_validity::ValidTransaction::default()), - } - } - - fn pre_dispatch( - self, - who: &Self::AccountId, - call: &Self::Call, - info: &sp_runtime::traits::DispatchInfoOf<Self::Call>, - len: usize, - ) -> Result<Self::Pre, sp_runtime::transaction_validity::TransactionValidityError> { - self.validate(who, call, info, len).map(drop) - } - - fn post_dispatch( - _maybe_pre: Option<Self::Pre>, - _info: &sp_runtime::traits::DispatchInfoOf<Self::Call>, - _post_info: &sp_runtime::traits::PostDispatchInfoOf<Self::Call>, - _len: usize, - _result: &sp_runtime::DispatchResult, - ) -> Result<(), sp_runtime::transaction_validity::TransactionValidityError> { - Ok(()) - } +use crate::{Config, Pallet}; +use bp_runtime::FilterCall; +use frame_support::{dispatch::CallableCallFor, traits::IsSubType}; +use sp_runtime::{ + traits::Header, + transaction_validity::{InvalidTransaction, TransactionValidity, ValidTransaction}, +}; + +/// Validate Grandpa headers in order to avoid "mining" transactions that provide outdated +/// bridged chain headers. Without this validation, even honest relayers may lose their funds +/// if there are multiple relays running and submitting the same information. +impl< + Call: IsSubType<CallableCallFor<Pallet<T, I>, T>>, + T: frame_system::Config<Call = Call> + Config<I>, + I: 'static, + > FilterCall<Call> for Pallet<T, I> +{ + fn validate(call: &<T as frame_system::Config>::Call) -> TransactionValidity { + let bundled_block_number = match call.is_sub_type() { + Some(crate::Call::<T, I>::submit_finality_proof { ref finality_target, .. }) => + *finality_target.number(), + _ => return Ok(ValidTransaction::default()), + }; + + let best_finalized = crate::BestFinalized::<T, I>::get(); + let best_finalized_number = match best_finalized { + Some((best_finalized_number, _)) => best_finalized_number, + None => return InvalidTransaction::Call.into(), + }; + + if best_finalized_number >= bundled_block_number { + log::trace!( + target: crate::LOG_TARGET, + "Rejecting obsolete bridged header: bundled {:?}, best {:?}", + bundled_block_number, + best_finalized_number, + ); + + return InvalidTransaction::Stale.into() } - }; + + Ok(ValidTransaction::default()) + } } #[cfg(test)] mod tests { + use super::FilterCall; use crate::{ mock::{run_test, test_header, Call, TestNumber, TestRuntime}, BestFinalized, }; use bp_test_utils::make_default_justification; - use frame_support::weights::{DispatchClass, DispatchInfo, Pays}; - use sp_runtime::traits::SignedExtension; - - declare_bridge_reject_obsolete_grandpa_header! { - TestRuntime, - Call::Grandpa => () - } fn validate_block_submit(num: TestNumber) -> bool { - BridgeRejectObsoleteGrandpaHeader - .validate( - &42, - &Call::Grandpa(crate::Call::<TestRuntime, ()>::submit_finality_proof { - finality_target: Box::new(test_header(num)), - justification: make_default_justification(&test_header(num)), - }), - &DispatchInfo { weight: 0, class: DispatchClass::Operational, pays_fee: Pays::Yes }, - 0, - ) - .is_ok() + crate::Pallet::<TestRuntime>::validate(&Call::Grandpa( + crate::Call::<TestRuntime, ()>::submit_finality_proof { + finality_target: Box::new(test_header(num)), + justification: make_default_justification(&test_header(num)), + }, + )) + .is_ok() } fn sync_to_header_10() { diff --git a/bridges/modules/parachains/src/extension.rs b/bridges/modules/parachains/src/extension.rs index efcd9ec567ff81ab5fa0d4820b2544178df4955c..3ed6dd9cbeda54cc1b8fd9a0684a1800fc740d93 100644 --- a/bridges/modules/parachains/src/extension.rs +++ b/bridges/modules/parachains/src/extension.rs @@ -14,154 +14,98 @@ // You should have received a copy of the GNU General Public License // along with Parity Bridges Common. If not, see <http://www.gnu.org/licenses/>. -/// Declares a runtime-specific `BridgeRejectObsoleteParachainHeader` signed extension. -/// -/// ## Example -/// -/// ```nocompile -/// pallet_bridge_grandpa::declare_bridge_reject_obsolete_parachain_header!{ -/// Runtime, -/// Call::BridgeRialtoParachains => RialtoGrandpaInstance, -/// Call::BridgeWestendParachains => WestendGrandpaInstance, -/// } -/// ``` -/// -/// The goal of this extension is to avoid "mining" transactions that provide -/// outdated bridged parachain heads. Without that extension, even honest relayers +use crate::{Config, Pallet, RelayBlockHash, RelayBlockHasher, RelayBlockNumber}; +use bp_runtime::FilterCall; +use frame_support::{dispatch::CallableCallFor, traits::IsSubType}; +use sp_runtime::transaction_validity::{InvalidTransaction, TransactionValidity, ValidTransaction}; + +/// Validate parachain heads in order to avoid "mining" transactions that provide +/// outdated bridged parachain heads. Without this validation, even honest relayers /// may lose their funds if there are multiple relays running and submitting the /// same information. /// -/// This extension only works with transactions that are updating single parachain +/// This validation only works with transactions that are updating single parachain /// head. We can't use unbounded validation - it may take too long and either break /// block production, or "eat" significant portion of block production time literally /// for nothing. In addition, the single-parachain-head-per-transaction is how the /// pallet will be used in our environment. -#[macro_export] -macro_rules! declare_bridge_reject_obsolete_parachain_header { - ($runtime:ident, $($call:path => $instance:ty),*) => { - /// Transaction-with-obsolete-bridged-parachain-header check that will reject transaction if - /// it submits obsolete bridged parachain header. - #[derive(Clone, codec::Decode, codec::Encode, Eq, PartialEq, frame_support::RuntimeDebug, scale_info::TypeInfo)] - pub struct BridgeRejectObsoleteParachainHeader; - - impl sp_runtime::traits::SignedExtension for BridgeRejectObsoleteParachainHeader { - const IDENTIFIER: &'static str = "BridgeRejectObsoleteParachainHeader"; - type AccountId = <$runtime as frame_system::Config>::AccountId; - type Call = <$runtime as frame_system::Config>::Call; - type AdditionalSigned = (); - type Pre = (); - - fn additional_signed(&self) -> sp_std::result::Result< - (), - sp_runtime::transaction_validity::TransactionValidityError, - > { - Ok(()) - } - - fn validate( - &self, - _who: &Self::AccountId, - call: &Self::Call, - _info: &sp_runtime::traits::DispatchInfoOf<Self::Call>, - _len: usize, - ) -> sp_runtime::transaction_validity::TransactionValidity { - match *call { - $( - $call($crate::Call::<$runtime, $instance>::submit_parachain_heads { - ref at_relay_block, - ref parachains, - .. - }) if parachains.len() == 1 => { - let (parachain, parachain_head_hash) = parachains.get(0).expect("verified by match condition; qed"); - - let bundled_relay_block_number = at_relay_block.0; - - let best_parachain_head = $crate::BestParaHeads::<$runtime, $instance>::get(parachain); - - match best_parachain_head { - Some(best_parachain_head) if best_parachain_head.at_relay_block_number - >= bundled_relay_block_number => { - log::trace!( - target: $crate::LOG_TARGET, - "Rejecting obsolete parachain-head {:?} transaction: bundled relay block number: \ - {:?} best relay block number: {:?}", - parachain, - bundled_relay_block_number, - best_parachain_head.at_relay_block_number, - ); - sp_runtime::transaction_validity::InvalidTransaction::Stale.into() - } - Some(best_parachain_head) if best_parachain_head.head_hash == *parachain_head_hash => { - log::trace!( - target: $crate::LOG_TARGET, - "Rejecting obsolete parachain-head {:?} transaction: head hash {:?}", - parachain, - best_parachain_head.head_hash, - ); - sp_runtime::transaction_validity::InvalidTransaction::Stale.into() - } - _ => Ok(sp_runtime::transaction_validity::ValidTransaction::default()), - } - }, - )* - _ => Ok(sp_runtime::transaction_validity::ValidTransaction::default()), - } - } - - fn pre_dispatch( - self, - who: &Self::AccountId, - call: &Self::Call, - info: &sp_runtime::traits::DispatchInfoOf<Self::Call>, - len: usize, - ) -> Result<Self::Pre, sp_runtime::transaction_validity::TransactionValidityError> { - self.validate(who, call, info, len).map(drop) - } - - fn post_dispatch( - _maybe_pre: Option<Self::Pre>, - _info: &sp_runtime::traits::DispatchInfoOf<Self::Call>, - _post_info: &sp_runtime::traits::PostDispatchInfoOf<Self::Call>, - _len: usize, - _result: &sp_runtime::DispatchResult, - ) -> Result<(), sp_runtime::transaction_validity::TransactionValidityError> { - Ok(()) - } +impl< + Call: IsSubType<CallableCallFor<Pallet<T, I>, T>>, + T: frame_system::Config<Call = Call> + Config<I>, + I: 'static, + > FilterCall<Call> for Pallet<T, I> +where + <T as pallet_bridge_grandpa::Config<T::BridgesGrandpaPalletInstance>>::BridgedChain: + bp_runtime::Chain< + BlockNumber = RelayBlockNumber, + Hash = RelayBlockHash, + Hasher = RelayBlockHasher, + >, +{ + fn validate(call: &Call) -> TransactionValidity { + let (bundled_relay_block_number, parachains) = match call.is_sub_type() { + Some(crate::Call::<T, I>::submit_parachain_heads { + ref at_relay_block, + ref parachains, + .. + }) if parachains.len() == 1 => (at_relay_block.0, parachains), + _ => return Ok(ValidTransaction::default()), + }; + + let (parachain, parachain_head_hash) = + parachains.get(0).expect("verified by match condition; qed"); + let best_parachain_head = crate::BestParaHeads::<T, I>::get(parachain); + + match best_parachain_head { + Some(best_parachain_head) + if best_parachain_head.at_relay_block_number >= bundled_relay_block_number => + { + log::trace!( + target: crate::LOG_TARGET, + "Rejecting obsolete parachain-head {:?} transaction: \ + bundled relay block number: {:?} \ + best relay block number: {:?}", + parachain, + bundled_relay_block_number, + best_parachain_head.at_relay_block_number, + ); + InvalidTransaction::Stale.into() + }, + Some(best_parachain_head) if best_parachain_head.head_hash == *parachain_head_hash => { + log::trace!( + target: crate::LOG_TARGET, + "Rejecting obsolete parachain-head {:?} transaction: head hash {:?}", + parachain, + best_parachain_head.head_hash, + ); + InvalidTransaction::Stale.into() + }, + _ => Ok(ValidTransaction::default()), } - }; + } } #[cfg(test)] mod tests { use crate::{ + extension::FilterCall, mock::{run_test, Call, TestRuntime}, BestParaHead, BestParaHeads, RelayBlockNumber, }; use bp_polkadot_core::parachains::{ParaHash, ParaHeadsProof, ParaId}; - use frame_support::weights::{DispatchClass, DispatchInfo, Pays}; - use sp_runtime::traits::SignedExtension; - - declare_bridge_reject_obsolete_parachain_header! { - TestRuntime, - Call::Parachains => () - } fn validate_submit_parachain_heads( num: RelayBlockNumber, parachains: Vec<(ParaId, ParaHash)>, ) -> bool { - BridgeRejectObsoleteParachainHeader - .validate( - &42, - &Call::Parachains(crate::Call::<TestRuntime, ()>::submit_parachain_heads { - at_relay_block: (num, Default::default()), - parachains, - parachain_heads_proof: ParaHeadsProof(Vec::new()), - }), - &DispatchInfo { weight: 0, class: DispatchClass::Operational, pays_fee: Pays::Yes }, - 0, - ) - .is_ok() + crate::Pallet::<TestRuntime>::validate(&Call::Parachains( + crate::Call::<TestRuntime, ()>::submit_parachain_heads { + at_relay_block: (num, Default::default()), + parachains, + parachain_heads_proof: ParaHeadsProof(Vec::new()), + }, + )) + .is_ok() } fn sync_to_relay_header_10() { diff --git a/bridges/primitives/runtime/src/lib.rs b/bridges/primitives/runtime/src/lib.rs index 982b8f9a0eb84d89d57f94c0e862656bad79255d..9807fa50864dbf22fb2925cfb5c98025e5a19d95 100644 --- a/bridges/primitives/runtime/src/lib.rs +++ b/bridges/primitives/runtime/src/lib.rs @@ -35,6 +35,7 @@ pub use chain::{ }; pub use frame_support::storage::storage_prefix as storage_value_final_key; use num_traits::{CheckedSub, One}; +use sp_runtime::transaction_validity::TransactionValidity; pub use storage_proof::{ Error as StorageProofError, ProofSize as StorageProofSize, StorageProofChecker, }; @@ -400,6 +401,12 @@ pub trait OwnedBridgeModule<T: frame_system::Config> { } } +/// A trait for querying whether a runtime call is valid. +pub trait FilterCall<Call> { + /// Checks if a runtime call is valid. + fn validate(call: &Call) -> TransactionValidity; +} + #[cfg(test)] mod tests { use super::*; diff --git a/bridges/relays/client-millau/src/lib.rs b/bridges/relays/client-millau/src/lib.rs index 4250bd34de75f6f7830679bea9c5bdba317f65b0..08bd1d76324d85b1bedd68bee62403ab74821852 100644 --- a/bridges/relays/client-millau/src/lib.rs +++ b/bridges/relays/client-millau/src/lib.rs @@ -114,9 +114,7 @@ impl TransactionSignScheme for Millau { frame_system::CheckNonce::<millau_runtime::Runtime>::from(param.unsigned.nonce), frame_system::CheckWeight::<millau_runtime::Runtime>::new(), pallet_transaction_payment::ChargeTransactionPayment::<millau_runtime::Runtime>::from(param.unsigned.tip), - millau_runtime::BridgeRejectObsoleteGrandpaHeader, - millau_runtime::BridgeRejectObsoleteParachainHeader, - millau_runtime::BridgeRejectObsoleteMessages, + millau_runtime::BridgeRejectObsoleteHeadersAndMessages, ), ( (), @@ -128,8 +126,6 @@ impl TransactionSignScheme for Millau { (), (), (), - (), - (), ), ); let signature = raw_payload.using_encoded(|payload| param.signer.sign(payload));