diff --git a/bridges/bin/millau/runtime/src/lib.rs b/bridges/bin/millau/runtime/src/lib.rs index 58830a476a84e..f9d8f891fd44a 100644 --- a/bridges/bin/millau/runtime/src/lib.rs +++ b/bridges/bin/millau/runtime/src/lib.rs @@ -565,6 +565,11 @@ pallet_bridge_grandpa::declare_bridge_reject_obsolete_grandpa_header! { Call::BridgeWestendGrandpa => WestendGrandpaInstance } +pallet_bridge_parachains::declare_bridge_reject_obsolete_parachain_header! { + Runtime, + Call::BridgeRialtoParachains => WithRialtoParachainsInstance +} + /// The address format for describing accounts. pub type Address = AccountId; /// Block header type as expected by this runtime. @@ -586,6 +591,7 @@ pub type SignedExtra = ( frame_system::CheckWeight, pallet_transaction_payment::ChargeTransactionPayment, BridgeRejectObsoleteGrandpaHeader, + BridgeRejectObsoleteParachainHeader, ); /// The payload being signed in transactions. pub type SignedPayload = generic::SignedPayload; @@ -994,7 +1000,11 @@ impl_runtime_apis! { parachains: &[bp_polkadot_core::parachains::ParaId], parachain_head_size: u32, proof_size: bp_runtime::StorageProofSize, - ) -> (pallet_bridge_parachains::RelayBlockHash, bp_polkadot_core::parachains::ParaHeadsProof) { + ) -> ( + pallet_bridge_parachains::RelayBlockNumber, + pallet_bridge_parachains::RelayBlockHash, + bp_polkadot_core::parachains::ParaHeadsProof, + ) { bridge_runtime_common::parachains_benchmarking::prepare_parachain_heads_proof::( parachains, parachain_head_size, diff --git a/bridges/bin/runtime-common/src/messages_benchmarking.rs b/bridges/bin/runtime-common/src/messages_benchmarking.rs index 880cb8fd55c18..865b55f57ccdf 100644 --- a/bridges/bin/runtime-common/src/messages_benchmarking.rs +++ b/bridges/bin/runtime-common/src/messages_benchmarking.rs @@ -84,7 +84,7 @@ where // finally - prepare storage proof and update environment let (state_root, storage_proof) = prepare_messages_storage_proof::(¶ms, message_payload); - let bridged_header_hash = insert_header_to_grandpa_pallet::(state_root); + let (_, bridged_header_hash) = insert_header_to_grandpa_pallet::(state_root); ( FromBridgedChainMessagesProof { @@ -132,7 +132,7 @@ where let storage_proof = proof_recorder.drain().into_iter().map(|n| n.data.to_vec()).collect(); // finally insert header with given state root to our storage - let bridged_header_hash = insert_header_to_grandpa_pallet::(root); + let (_, bridged_header_hash) = insert_header_to_grandpa_pallet::(root); FromBridgedChainMessagesDeliveryProof { bridged_header_hash: bridged_header_hash.into(), @@ -207,14 +207,15 @@ where /// Insert header to the bridge GRANDPA pallet. pub(crate) fn insert_header_to_grandpa_pallet( state_root: bp_runtime::HashOf, -) -> bp_runtime::HashOf +) -> (bp_runtime::BlockNumberOf, bp_runtime::HashOf) where R: pallet_bridge_grandpa::Config, GI: 'static, R::BridgedChain: bp_runtime::Chain, { + let bridged_block_number = Zero::zero(); let bridged_header = bp_runtime::HeaderOf::::new( - Zero::zero(), + bridged_block_number, Default::default(), state_root, Default::default(), @@ -222,7 +223,7 @@ where ); let bridged_header_hash = bridged_header.hash(); pallet_bridge_grandpa::initialize_for_benchmarks::(bridged_header); - bridged_header_hash + (bridged_block_number, bridged_header_hash) } /// Populate trie with dummy keys+values until trie has at least given size. diff --git a/bridges/bin/runtime-common/src/parachains_benchmarking.rs b/bridges/bin/runtime-common/src/parachains_benchmarking.rs index e2635eb2543c1..f707f652d8c38 100644 --- a/bridges/bin/runtime-common/src/parachains_benchmarking.rs +++ b/bridges/bin/runtime-common/src/parachains_benchmarking.rs @@ -25,7 +25,7 @@ use bp_polkadot_core::parachains::{ParaHead, ParaHeadsProof, ParaId}; use bp_runtime::StorageProofSize; use codec::Encode; use frame_support::traits::Get; -use pallet_bridge_parachains::{RelayBlockHash, RelayBlockHasher}; +use pallet_bridge_parachains::{RelayBlockHash, RelayBlockHasher, RelayBlockNumber}; use sp_std::prelude::*; use sp_trie::{record_all_keys, trie_types::TrieDBMutV1, LayoutV1, MemoryDB, Recorder, TrieMut}; @@ -37,13 +37,13 @@ pub fn prepare_parachain_heads_proof( parachains: &[ParaId], parachain_head_size: u32, size: StorageProofSize, -) -> (RelayBlockHash, ParaHeadsProof) +) -> (RelayBlockNumber, RelayBlockHash, ParaHeadsProof) where R: pallet_bridge_parachains::Config + pallet_bridge_grandpa::Config, PI: 'static, >::BridgedChain: - bp_runtime::Chain, + bp_runtime::Chain, { let parachain_head = ParaHead(vec![0u8; parachain_head_size as usize]); @@ -73,8 +73,8 @@ where .expect("record_all_keys should not fail in benchmarks"); let proof = proof_recorder.drain().into_iter().map(|n| n.data.to_vec()).collect(); - let relay_block_hash = + let (relay_block_number, relay_block_hash) = insert_header_to_grandpa_pallet::(state_root); - (relay_block_hash, ParaHeadsProof(proof)) + (relay_block_number, relay_block_hash, ParaHeadsProof(proof)) } diff --git a/bridges/modules/grandpa/src/extension.rs b/bridges/modules/grandpa/src/extension.rs index 43ad622105225..654b74bf446ea 100644 --- a/bridges/modules/grandpa/src/extension.rs +++ b/bridges/modules/grandpa/src/extension.rs @@ -25,6 +25,11 @@ /// 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),*) => { diff --git a/bridges/modules/parachains/src/benchmarking.rs b/bridges/modules/parachains/src/benchmarking.rs index 2734dfafd6fa2..749182a82ea69 100644 --- a/bridges/modules/parachains/src/benchmarking.rs +++ b/bridges/modules/parachains/src/benchmarking.rs @@ -37,7 +37,7 @@ pub trait Config: crate::Config { parachains: &[ParaId], parachain_head_size: u32, proof_size: StorageProofSize, - ) -> (RelayBlockHash, ParaHeadsProof); + ) -> (RelayBlockNumber, RelayBlockHash, ParaHeadsProof); } benchmarks_instance_pallet! { @@ -57,12 +57,13 @@ benchmarks_instance_pallet! { let sender = account("sender", 0, 0); let parachains = (1..=p).map(ParaId).collect::>(); - let (relay_block_hash, parachain_heads_proof) = T::prepare_parachain_heads_proof( + let (relay_block_number, relay_block_hash, parachain_heads_proof) = T::prepare_parachain_heads_proof( ¶chains, DEFAULT_PARACHAIN_HEAD_SIZE, StorageProofSize::Minimal(0), ); - }: submit_parachain_heads(RawOrigin::Signed(sender), relay_block_hash, parachains.clone(), parachain_heads_proof) + let at_relay_block = (relay_block_number, relay_block_hash); + }: submit_parachain_heads(RawOrigin::Signed(sender), at_relay_block, parachains.clone(), parachain_heads_proof) verify { for parachain in parachains { assert!(crate::Pallet::::best_parachain_head(parachain).is_some()); @@ -73,13 +74,13 @@ benchmarks_instance_pallet! { submit_parachain_heads_with_1kb_proof { let sender = account("sender", 0, 0); let parachains = vec![ParaId(1)]; - let (relay_block_hash, parachain_heads_proof) = T::prepare_parachain_heads_proof( + let (relay_block_number, relay_block_hash, parachain_heads_proof) = T::prepare_parachain_heads_proof( ¶chains, DEFAULT_PARACHAIN_HEAD_SIZE, StorageProofSize::HasExtraNodes(1024), ); - - }: submit_parachain_heads(RawOrigin::Signed(sender), relay_block_hash, parachains.clone(), parachain_heads_proof) + let at_relay_block = (relay_block_number, relay_block_hash); + }: submit_parachain_heads(RawOrigin::Signed(sender), at_relay_block, parachains.clone(), parachain_heads_proof) verify { for parachain in parachains { assert!(crate::Pallet::::best_parachain_head(parachain).is_some()); @@ -90,13 +91,13 @@ benchmarks_instance_pallet! { submit_parachain_heads_with_16kb_proof { let sender = account("sender", 0, 0); let parachains = vec![ParaId(1)]; - let (relay_block_hash, parachain_heads_proof) = T::prepare_parachain_heads_proof( + let (relay_block_number, relay_block_hash, parachain_heads_proof) = T::prepare_parachain_heads_proof( ¶chains, DEFAULT_PARACHAIN_HEAD_SIZE, StorageProofSize::HasExtraNodes(16 * 1024), ); - - }: submit_parachain_heads(RawOrigin::Signed(sender), relay_block_hash, parachains.clone(), parachain_heads_proof) + let at_relay_block = (relay_block_number, relay_block_hash); + }: submit_parachain_heads(RawOrigin::Signed(sender), at_relay_block, parachains.clone(), parachain_heads_proof) verify { for parachain in parachains { assert!(crate::Pallet::::best_parachain_head(parachain).is_some()); diff --git a/bridges/modules/parachains/src/extension.rs b/bridges/modules/parachains/src/extension.rs new file mode 100644 index 0000000000000..1d0eb7a7ff908 --- /dev/null +++ b/bridges/modules/parachains/src/extension.rs @@ -0,0 +1,195 @@ +// Copyright 2021 Parity Technologies (UK) Ltd. +// This file is part of Parity Bridges Common. + +// Parity Bridges Common is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// Parity Bridges Common is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with Parity Bridges Common. If not, see . + +/// 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 +/// 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 +/// 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, + _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 = 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 => + 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, + len: usize, + ) -> Result { + self.validate(who, call, info, len).map(drop) + } + + fn post_dispatch( + _maybe_pre: Option, + _info: &sp_runtime::traits::DispatchInfoOf, + _post_info: &sp_runtime::traits::PostDispatchInfoOf, + _len: usize, + _result: &sp_runtime::DispatchResult, + ) -> Result<(), sp_runtime::transaction_validity::TransactionValidityError> { + Ok(()) + } + } + }; +} + +#[cfg(test)] +mod tests { + use crate::{ + mock::{run_test, Call, TestRuntime}, + BestParaHead, BestParaHeads, RelayBlockNumber, + }; + use bp_polkadot_core::parachains::{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) -> bool { + BridgeRejectObsoleteParachainHeader + .validate( + &42, + &Call::Parachains(crate::Call::::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() + } + + fn sync_to_relay_header_10() { + BestParaHeads::::insert( + ParaId(1), + BestParaHead { + at_relay_block_number: 10, + head_hash: Default::default(), + next_imported_hash_position: 0, + }, + ); + } + + #[test] + fn extension_rejects_obsolete_header() { + run_test(|| { + // when current best finalized is #10 and we're trying to import header#5 => tx is + // rejected + sync_to_relay_header_10(); + assert!(!validate_submit_parachain_heads(5, vec![ParaId(1)])); + }); + } + + #[test] + fn extension_rejects_same_header() { + run_test(|| { + // when current best finalized is #10 and we're trying to import header#10 => tx is + // rejected + sync_to_relay_header_10(); + assert!(!validate_submit_parachain_heads(10, vec![ParaId(1)])); + }); + } + + #[test] + fn extension_accepts_new_header() { + run_test(|| { + // when current best finalized is #10 and we're trying to import header#15 => tx is + // accepted + sync_to_relay_header_10(); + assert!(validate_submit_parachain_heads(15, vec![ParaId(1)])); + }); + } + + #[test] + fn extension_accepts_if_more_than_one_parachain_is_submitted() { + run_test(|| { + // when current best finalized is #10 and we're trying to import header#5, but another + // parachain head is also supplied => tx is accepted + sync_to_relay_header_10(); + assert!(validate_submit_parachain_heads(5, vec![ParaId(1), ParaId(2)])); + }); + } +} diff --git a/bridges/modules/parachains/src/lib.rs b/bridges/modules/parachains/src/lib.rs index 4cedffc300b82..cf634e5b5ca5c 100644 --- a/bridges/modules/parachains/src/lib.rs +++ b/bridges/modules/parachains/src/lib.rs @@ -44,6 +44,7 @@ pub mod weights_ext; #[cfg(feature = "runtime-benchmarks")] pub mod benchmarking; +mod extension; #[cfg(test)] mod mock; @@ -84,8 +85,10 @@ pub mod pallet { #[pallet::error] pub enum Error { - /// Relay chain block is unknown to us. + /// Relay chain block hash is unknown to us. UnknownRelayChainBlock, + /// The number of stored relay block is different from what the relayer has provided. + InvalidRelayChainBlockNumber, /// Invalid storage proof has been passed. InvalidStorageProof, /// Given parachain head is unknown. @@ -175,17 +178,21 @@ pub mod pallet { ))] pub fn submit_parachain_heads( _origin: OriginFor, - relay_block_hash: RelayBlockHash, + at_relay_block: (RelayBlockNumber, RelayBlockHash), parachains: Vec, parachain_heads_proof: ParaHeadsProof, ) -> DispatchResultWithPostInfo { // we'll need relay chain header to verify that parachains heads are always increasing. + let (relay_block_number, relay_block_hash) = at_relay_block; let relay_block = pallet_bridge_grandpa::ImportedHeaders::< T, T::BridgesGrandpaPalletInstance, >::get(relay_block_hash) .ok_or(Error::::UnknownRelayChainBlock)?; - let relay_block_number = *relay_block.number(); + ensure!( + *relay_block.number() == relay_block_number, + Error::::InvalidRelayChainBlockNumber, + ); // now parse storage proof and read parachain heads let mut actual_weight = WeightInfoOf::::submit_parachain_heads_weight( @@ -484,7 +491,7 @@ mod tests { ) -> DispatchResultWithPostInfo { Pallet::::submit_parachain_heads( Origin::signed(1), - test_relay_header(relay_chain_block, relay_state_root).hash(), + (relay_chain_block, test_relay_header(relay_chain_block, relay_state_root).hash()), vec![ParaId(1)], proof, ) @@ -510,7 +517,7 @@ mod tests { // we're trying to update heads of parachains 1, 2 and 3 assert_ok!(Pallet::::submit_parachain_heads( Origin::signed(1), - test_relay_header(0, state_root).hash(), + (0, test_relay_header(0, state_root).hash()), vec![ParaId(1), ParaId(2), ParaId(3)], proof, ),); @@ -602,7 +609,7 @@ mod tests { initialize(state_root); assert_ok!(Pallet::::submit_parachain_heads( Origin::signed(1), - test_relay_header(0, state_root).hash(), + (0, test_relay_header(0, state_root).hash()), vec![ParaId(1), ParaId(UNTRACKED_PARACHAIN_ID), ParaId(2)], proof, )); @@ -776,7 +783,7 @@ mod tests { proceed(20, state_root_10_at_20); assert_ok!(Pallet::::submit_parachain_heads( Origin::signed(1), - test_relay_header(20, state_root_10_at_20).hash(), + (20, test_relay_header(20, state_root_10_at_20).hash()), vec![ParaId(1)], proof_10_at_20, ),); @@ -792,7 +799,7 @@ mod tests { proceed(30, state_root_10_at_30); assert_ok!(Pallet::::submit_parachain_heads( Origin::signed(1), - test_relay_header(30, state_root_10_at_30).hash(), + (30, test_relay_header(30, state_root_10_at_30).hash()), vec![ParaId(1)], proof_10_at_30, ),); diff --git a/bridges/modules/parachains/src/mock.rs b/bridges/modules/parachains/src/mock.rs index e07b0a91010d1..eaf5d3b4fdd91 100644 --- a/bridges/modules/parachains/src/mock.rs +++ b/bridges/modules/parachains/src/mock.rs @@ -46,7 +46,7 @@ construct_runtime! { System: frame_system::{Pallet, Call, Config, Storage, Event}, Grandpa1: pallet_bridge_grandpa::::{Pallet}, Grandpa2: pallet_bridge_grandpa::::{Pallet}, - Parachains: pallet_bridge_parachains::{Pallet}, + Parachains: pallet_bridge_parachains::{Call, Pallet}, } } diff --git a/bridges/relays/client-millau/src/lib.rs b/bridges/relays/client-millau/src/lib.rs index 2779fb0fd3b35..e0837e50bb72a 100644 --- a/bridges/relays/client-millau/src/lib.rs +++ b/bridges/relays/client-millau/src/lib.rs @@ -116,6 +116,7 @@ impl TransactionSignScheme for Millau { frame_system::CheckWeight::::new(), pallet_transaction_payment::ChargeTransactionPayment::::from(param.unsigned.tip), millau_runtime::BridgeRejectObsoleteGrandpaHeader, + millau_runtime::BridgeRejectObsoleteParachainHeader, ), ( (), @@ -127,6 +128,7 @@ impl TransactionSignScheme for Millau { (), (), (), + (), ), ); let signature = raw_payload.using_encoded(|payload| param.signer.sign(payload)); diff --git a/bridges/relays/lib-substrate-relay/src/parachains/mod.rs b/bridges/relays/lib-substrate-relay/src/parachains/mod.rs index ef14b7bb10322..51549ef02bd52 100644 --- a/bridges/relays/lib-substrate-relay/src/parachains/mod.rs +++ b/bridges/relays/lib-substrate-relay/src/parachains/mod.rs @@ -24,7 +24,7 @@ use pallet_bridge_parachains::{ RelayBlockHasher, RelayBlockNumber, }; use parachains_relay::ParachainsPipeline; -use relay_substrate_client::{CallOf, Chain, HashOf, RelayChain, TransactionSignScheme}; +use relay_substrate_client::{CallOf, Chain, HeaderIdOf, RelayChain, TransactionSignScheme}; use std::{fmt::Debug, marker::PhantomData}; pub mod source; @@ -70,7 +70,7 @@ pub trait SubmitParachainHeadsCallBuilder: /// Given parachains and their heads proof, build call of `submit_parachain_heads` /// function of bridge parachains module at the target chain. fn build_submit_parachain_heads_call( - relay_block_hash: HashOf, + at_relay_block: HeaderIdOf, parachains: Vec, parachain_heads_proof: ParaHeadsProof, ) -> CallOf; @@ -85,7 +85,7 @@ pub struct DirectSubmitParachainHeadsCallBuilder { impl SubmitParachainHeadsCallBuilder

for DirectSubmitParachainHeadsCallBuilder where P: SubstrateParachainsPipeline, - P::SourceRelayChain: Chain, + P::SourceRelayChain: Chain, R: BridgeParachainsConfig + Send + Sync, I: 'static + Send + Sync, R::BridgedChain: bp_runtime::Chain< @@ -96,12 +96,12 @@ where CallOf: From>, { fn build_submit_parachain_heads_call( - relay_block_hash: HashOf, + at_relay_block: HeaderIdOf, parachains: Vec, parachain_heads_proof: ParaHeadsProof, ) -> CallOf { BridgeParachainsCall::::submit_parachain_heads { - relay_block_hash, + at_relay_block: (at_relay_block.0, at_relay_block.1), parachains, parachain_heads_proof, } diff --git a/bridges/relays/lib-substrate-relay/src/parachains/target.rs b/bridges/relays/lib-substrate-relay/src/parachains/target.rs index 3a4e0b2c27fe8..9ff35f5a0c106 100644 --- a/bridges/relays/lib-substrate-relay/src/parachains/target.rs +++ b/bridges/relays/lib-substrate-relay/src/parachains/target.rs @@ -134,7 +134,7 @@ where let transaction_params = self.transaction_params.clone(); let (spec_version, transaction_version) = self.client.simple_runtime_version().await?; let call = P::SubmitParachainHeadsCallBuilder::build_submit_parachain_heads_call( - at_relay_block.1, + at_relay_block, updated_parachains, proof, );