From 092b6ee50cf6dab9c4c910e75c88184fdcddd18f Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Thu, 14 Jul 2022 15:18:15 +0300 Subject: [PATCH] Add another condition to the reject-obsolete-parachain-heads extension (#1505) * add another condition to the reject-obsolete-parachain-heads extension * add tracing to obsolete-tx-extensions * fix tests * extension_rejects_header_from_new_relay_block_with_same_hash * fmt * fix benchmarks --- bridges/bin/millau/runtime/src/lib.rs | 1 + .../runtime-common/src/messages_extension.rs | 16 +++ .../src/parachains_benchmarking.rs | 8 +- bridges/modules/grandpa/src/extension.rs | 7 ++ bridges/modules/grandpa/src/lib.rs | 2 +- bridges/modules/messages/src/lib.rs | 2 +- .../modules/parachains/src/benchmarking.rs | 16 +-- bridges/modules/parachains/src/extension.rs | 59 +++++++-- bridges/modules/parachains/src/lib.rs | 112 +++++++++++------- .../lib-substrate-relay/src/parachains/mod.rs | 6 +- .../src/parachains/source.rs | 47 ++++++-- .../src/parachains/target.rs | 4 +- .../relays/parachains/src/parachains_loop.rs | 37 ++++-- 13 files changed, 225 insertions(+), 92 deletions(-) diff --git a/bridges/bin/millau/runtime/src/lib.rs b/bridges/bin/millau/runtime/src/lib.rs index 4041cfe171b30..4ee80e9e124d3 100644 --- a/bridges/bin/millau/runtime/src/lib.rs +++ b/bridges/bin/millau/runtime/src/lib.rs @@ -1035,6 +1035,7 @@ impl_runtime_apis! { pallet_bridge_parachains::RelayBlockNumber, pallet_bridge_parachains::RelayBlockHash, bp_polkadot_core::parachains::ParaHeadsProof, + Vec<(bp_polkadot_core::parachains::ParaId, bp_polkadot_core::parachains::ParaHash)>, ) { bridge_runtime_common::parachains_benchmarking::prepare_parachain_heads_proof::( parachains, diff --git a/bridges/bin/runtime-common/src/messages_extension.rs b/bridges/bin/runtime-common/src/messages_extension.rs index fd1d9d7a934b5..48b30fa646e2a 100644 --- a/bridges/bin/runtime-common/src/messages_extension.rs +++ b/bridges/bin/runtime-common/src/messages_extension.rs @@ -70,6 +70,14 @@ macro_rules! declare_bridge_reject_obsolete_messages { 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(); } @@ -84,6 +92,14 @@ macro_rules! declare_bridge_reject_obsolete_messages { 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(); } diff --git a/bridges/bin/runtime-common/src/parachains_benchmarking.rs b/bridges/bin/runtime-common/src/parachains_benchmarking.rs index f707f652d8c38..97a1cd3ee7d9f 100644 --- a/bridges/bin/runtime-common/src/parachains_benchmarking.rs +++ b/bridges/bin/runtime-common/src/parachains_benchmarking.rs @@ -21,7 +21,7 @@ use crate::messages_benchmarking::{grow_trie, insert_header_to_grandpa_pallet}; use bp_parachains::parachain_head_storage_key_at_source; -use bp_polkadot_core::parachains::{ParaHead, ParaHeadsProof, ParaId}; +use bp_polkadot_core::parachains::{ParaHash, ParaHead, ParaHeadsProof, ParaId}; use bp_runtime::StorageProofSize; use codec::Encode; use frame_support::traits::Get; @@ -37,7 +37,7 @@ pub fn prepare_parachain_heads_proof( parachains: &[ParaId], parachain_head_size: u32, size: StorageProofSize, -) -> (RelayBlockNumber, RelayBlockHash, ParaHeadsProof) +) -> (RelayBlockNumber, RelayBlockHash, ParaHeadsProof, Vec<(ParaId, ParaHash)>) where R: pallet_bridge_parachains::Config + pallet_bridge_grandpa::Config, @@ -48,6 +48,7 @@ where let parachain_head = ParaHead(vec![0u8; parachain_head_size as usize]); // insert all heads to the trie + let mut parachain_heads = Vec::with_capacity(parachains.len()); let mut storage_keys = Vec::with_capacity(parachains.len()); let mut state_root = Default::default(); let mut mdb = MemoryDB::default(); @@ -62,6 +63,7 @@ where .map_err(|_| "TrieMut::insert has failed") .expect("TrieMut::insert should not fail in benchmarks"); storage_keys.push(storage_key); + parachain_heads.push((*parachain, parachain_head.hash())) } } state_root = grow_trie(state_root, &mut mdb, size); @@ -76,5 +78,5 @@ where let (relay_block_number, relay_block_hash) = insert_header_to_grandpa_pallet::(state_root); - (relay_block_number, relay_block_hash, ParaHeadsProof(proof)) + (relay_block_number, relay_block_hash, ParaHeadsProof(proof), parachain_heads) } diff --git a/bridges/modules/grandpa/src/extension.rs b/bridges/modules/grandpa/src/extension.rs index bda7d49d7c436..007c5f3df7475 100644 --- a/bridges/modules/grandpa/src/extension.rs +++ b/bridges/modules/grandpa/src/extension.rs @@ -75,6 +75,13 @@ macro_rules! declare_bridge_reject_obsolete_grandpa_header { 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() } }, diff --git a/bridges/modules/grandpa/src/lib.rs b/bridges/modules/grandpa/src/lib.rs index c75ccc2e80a81..4074421fc52d5 100644 --- a/bridges/modules/grandpa/src/lib.rs +++ b/bridges/modules/grandpa/src/lib.rs @@ -60,7 +60,7 @@ pub use pallet::*; pub use weights::WeightInfo; /// The target that will be used when publishing logs related to this pallet. -const LOG_TARGET: &str = "runtime::bridge-grandpa"; +pub const LOG_TARGET: &str = "runtime::bridge-grandpa"; /// Block number of the bridged chain. pub type BridgedBlockNumber = BlockNumberOf<>::BridgedChain>; diff --git a/bridges/modules/messages/src/lib.rs b/bridges/modules/messages/src/lib.rs index ca4b7044b2cfb..48ca546def8c1 100644 --- a/bridges/modules/messages/src/lib.rs +++ b/bridges/modules/messages/src/lib.rs @@ -91,7 +91,7 @@ mod mock; pub use pallet::*; /// The target that will be used when publishing logs related to this pallet. -const LOG_TARGET: &str = "runtime::bridge-messages"; +pub const LOG_TARGET: &str = "runtime::bridge-messages"; #[frame_support::pallet] pub mod pallet { diff --git a/bridges/modules/parachains/src/benchmarking.rs b/bridges/modules/parachains/src/benchmarking.rs index 749182a82ea69..aba296dfc1d5b 100644 --- a/bridges/modules/parachains/src/benchmarking.rs +++ b/bridges/modules/parachains/src/benchmarking.rs @@ -21,7 +21,7 @@ use crate::{ RelayBlockNumber, }; -use bp_polkadot_core::parachains::{ParaHeadsProof, ParaId}; +use bp_polkadot_core::parachains::{ParaHash, ParaHeadsProof, ParaId}; use bp_runtime::StorageProofSize; use frame_benchmarking::{account, benchmarks_instance_pallet}; use frame_system::RawOrigin; @@ -37,7 +37,7 @@ pub trait Config: crate::Config { parachains: &[ParaId], parachain_head_size: u32, proof_size: StorageProofSize, - ) -> (RelayBlockNumber, RelayBlockHash, ParaHeadsProof); + ) -> (RelayBlockNumber, RelayBlockHash, ParaHeadsProof, Vec<(ParaId, ParaHash)>); } benchmarks_instance_pallet! { @@ -57,13 +57,13 @@ benchmarks_instance_pallet! { let sender = account("sender", 0, 0); let parachains = (1..=p).map(ParaId).collect::>(); - let (relay_block_number, relay_block_hash, parachain_heads_proof) = T::prepare_parachain_heads_proof( + let (relay_block_number, relay_block_hash, parachain_heads_proof, parachains_heads) = T::prepare_parachain_heads_proof( ¶chains, DEFAULT_PARACHAIN_HEAD_SIZE, StorageProofSize::Minimal(0), ); let at_relay_block = (relay_block_number, relay_block_hash); - }: submit_parachain_heads(RawOrigin::Signed(sender), at_relay_block, parachains.clone(), parachain_heads_proof) + }: submit_parachain_heads(RawOrigin::Signed(sender), at_relay_block, parachains_heads, parachain_heads_proof) verify { for parachain in parachains { assert!(crate::Pallet::::best_parachain_head(parachain).is_some()); @@ -74,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_number, relay_block_hash, parachain_heads_proof) = T::prepare_parachain_heads_proof( + let (relay_block_number, relay_block_hash, parachain_heads_proof, parachains_heads) = T::prepare_parachain_heads_proof( ¶chains, DEFAULT_PARACHAIN_HEAD_SIZE, StorageProofSize::HasExtraNodes(1024), ); let at_relay_block = (relay_block_number, relay_block_hash); - }: submit_parachain_heads(RawOrigin::Signed(sender), at_relay_block, parachains.clone(), parachain_heads_proof) + }: submit_parachain_heads(RawOrigin::Signed(sender), at_relay_block, parachains_heads, parachain_heads_proof) verify { for parachain in parachains { assert!(crate::Pallet::::best_parachain_head(parachain).is_some()); @@ -91,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_number, relay_block_hash, parachain_heads_proof) = T::prepare_parachain_heads_proof( + let (relay_block_number, relay_block_hash, parachain_heads_proof, parachains_heads) = T::prepare_parachain_heads_proof( ¶chains, DEFAULT_PARACHAIN_HEAD_SIZE, StorageProofSize::HasExtraNodes(16 * 1024), ); let at_relay_block = (relay_block_number, relay_block_hash); - }: submit_parachain_heads(RawOrigin::Signed(sender), at_relay_block, parachains.clone(), parachain_heads_proof) + }: submit_parachain_heads(RawOrigin::Signed(sender), at_relay_block, parachains_heads, 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 index 1d0eb7a7ff908..efcd9ec567ff8 100644 --- a/bridges/modules/parachains/src/extension.rs +++ b/bridges/modules/parachains/src/extension.rs @@ -72,15 +72,34 @@ macro_rules! declare_bridge_reject_obsolete_parachain_header { ref parachains, .. }) if parachains.len() == 1 => { - let parachain = parachains.get(0).expect("verified by match condition; qed"); + 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 => - sp_runtime::transaction_validity::InvalidTransaction::Stale.into(), + >= 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()), } }, @@ -118,7 +137,7 @@ mod tests { mock::{run_test, Call, TestRuntime}, BestParaHead, BestParaHeads, RelayBlockNumber, }; - use bp_polkadot_core::parachains::{ParaHeadsProof, ParaId}; + use bp_polkadot_core::parachains::{ParaHash, ParaHeadsProof, ParaId}; use frame_support::weights::{DispatchClass, DispatchInfo, Pays}; use sp_runtime::traits::SignedExtension; @@ -127,7 +146,10 @@ mod tests { Call::Parachains => () } - fn validate_submit_parachain_heads(num: RelayBlockNumber, parachains: Vec) -> bool { + fn validate_submit_parachain_heads( + num: RelayBlockNumber, + parachains: Vec<(ParaId, ParaHash)>, + ) -> bool { BridgeRejectObsoleteParachainHeader .validate( &42, @@ -147,29 +169,39 @@ mod tests { ParaId(1), BestParaHead { at_relay_block_number: 10, - head_hash: Default::default(), + head_hash: [1u8; 32].into(), next_imported_hash_position: 0, }, ); } #[test] - fn extension_rejects_obsolete_header() { + fn extension_rejects_header_from_the_obsolete_relay_block() { 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)])); + assert!(!validate_submit_parachain_heads(5, vec![(ParaId(1), [1u8; 32].into())])); + }); + } + + #[test] + fn extension_rejects_header_from_the_same_relay_block() { + 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), [1u8; 32].into())])); }); } #[test] - fn extension_rejects_same_header() { + fn extension_rejects_header_from_new_relay_block_with_same_hash() { 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)])); + assert!(!validate_submit_parachain_heads(20, vec![(ParaId(1), [1u8; 32].into())])); }); } @@ -179,7 +211,7 @@ mod tests { // 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)])); + assert!(validate_submit_parachain_heads(15, vec![(ParaId(1), [2u8; 32].into())])); }); } @@ -189,7 +221,10 @@ mod tests { // 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)])); + assert!(validate_submit_parachain_heads( + 5, + vec![(ParaId(1), [1u8; 32].into()), (ParaId(2), [1u8; 32].into())] + )); }); } } diff --git a/bridges/modules/parachains/src/lib.rs b/bridges/modules/parachains/src/lib.rs index ec2d16427e71a..05e14575eacf3 100644 --- a/bridges/modules/parachains/src/lib.rs +++ b/bridges/modules/parachains/src/lib.rs @@ -49,7 +49,7 @@ mod extension; mod mock; /// The target that will be used when publishing logs related to this pallet. -const LOG_TARGET: &str = "runtime::bridge-parachains"; +pub const LOG_TARGET: &str = "runtime::bridge-parachains"; /// Block hash of the bridged relay chain. pub type RelayBlockHash = bp_polkadot_core::Hash; @@ -209,7 +209,7 @@ pub mod pallet { pub fn submit_parachain_heads( _origin: OriginFor, at_relay_block: (RelayBlockNumber, RelayBlockHash), - parachains: Vec, + parachains: Vec<(ParaId, ParaHash)>, parachain_heads_proof: ParaHeadsProof, ) -> DispatchResultWithPostInfo { Self::ensure_not_halted().map_err(Error::::BridgeModule)?; @@ -235,7 +235,7 @@ pub mod pallet { relay_block_hash, sp_trie::StorageProof::new(parachain_heads_proof.0), move |storage| { - for parachain in parachains { + for (parachain, parachain_head_hash) in parachains { // if we're not tracking this parachain, we'll just ignore its head proof here if !T::TrackedParachains::contains(¶chain) { log::trace!( @@ -272,12 +272,27 @@ pub mod pallet { }, }; + // if relayer has specified invalid parachain head hash, ignore the head + // (this isn't strictly necessary, but better safe than sorry) + let actual_parachain_head_hash = parachain_head.hash(); + if parachain_head_hash != actual_parachain_head_hash { + log::trace!( + target: LOG_TARGET, + "The submitter has specified invalid parachain {:?} head hash: {:?} vs {:?}", + parachain, + parachain_head_hash, + actual_parachain_head_hash, + ); + continue; + } + let prune_happened: Result<_, ()> = BestParaHeads::::try_mutate(parachain, |stored_best_head| { let artifacts = Pallet::::update_parachain_head( parachain, stored_best_head.take(), relay_block_number, parachain_head, + parachain_head_hash, )?; *stored_best_head = Some(artifacts.best_head); Ok(artifacts.prune_happened) @@ -364,10 +379,10 @@ pub mod pallet { stored_best_head: Option, updated_at_relay_block_number: RelayBlockNumber, updated_head: ParaHead, + updated_head_hash: ParaHash, ) -> Result { // check if head has been already updated at better relay chain block. Without this // check, we may import heads in random order - let updated_head_hash = updated_head.hash(); let next_imported_hash_position = match stored_best_head { Some(stored_best_head) if stored_best_head.at_relay_block_number <= updated_at_relay_block_number => @@ -376,7 +391,7 @@ pub mod pallet { if updated_head_hash == stored_best_head.head_hash { log::trace!( target: LOG_TARGET, - "The head of parachain {:?} can't be updated to {}, because it has been already updated\ + "The head of parachain {:?} can't be updated to {}, because it has been already updated \ to the same value at previous relay chain block: {} < {}", parachain, updated_head_hash, @@ -392,7 +407,7 @@ pub mod pallet { Some(stored_best_head) => { log::trace!( target: LOG_TARGET, - "The head of parachain {:?} can't be updated to {}, because it has been already updated\ + "The head of parachain {:?} can't be updated to {}, because it has been already updated \ to {} at better relay chain block: {} > {}", parachain, updated_head_hash, @@ -530,7 +545,8 @@ mod tests { fn prepare_parachain_heads_proof( heads: Vec<(u32, ParaHead)>, - ) -> (RelayBlockHash, ParaHeadsProof) { + ) -> (RelayBlockHash, ParaHeadsProof, Vec<(ParaId, ParaHash)>) { + let mut parachains = Vec::with_capacity(heads.len()); let mut root = Default::default(); let mut mdb = MemoryDB::default(); { @@ -541,6 +557,7 @@ mod tests { trie.insert(&storage_key.0, &head.encode()) .map_err(|_| "TrieMut::insert has failed") .expect("TrieMut::insert should not fail in tests"); + parachains.push((ParaId(parachain), head.hash())); } } @@ -551,7 +568,7 @@ mod tests { .expect("record_all_keys should not fail in benchmarks"); let storage_proof = proof_recorder.drain().into_iter().map(|n| n.data.to_vec()).collect(); - (root, ParaHeadsProof(storage_proof)) + (root, ParaHeadsProof(storage_proof), parachains) } fn initial_best_head(parachain: u32) -> BestParaHead { @@ -573,12 +590,13 @@ mod tests { fn import_parachain_1_head( relay_chain_block: RelayBlockNumber, relay_state_root: RelayBlockHash, + parachains: Vec<(ParaId, ParaHash)>, proof: ParaHeadsProof, ) -> DispatchResultWithPostInfo { Pallet::::submit_parachain_heads( Origin::signed(1), (relay_chain_block, test_relay_header(relay_chain_block, relay_state_root).hash()), - vec![ParaId(1)], + parachains, proof, ) } @@ -595,7 +613,8 @@ mod tests { #[test] fn submit_parachain_heads_checks_operating_mode() { - let (state_root, proof) = prepare_parachain_heads_proof(vec![(1, head_data(1, 0))]); + let (state_root, proof, parachains) = + prepare_parachain_heads_proof(vec![(1, head_data(1, 0))]); run_test(|| { initialize(state_root); @@ -606,7 +625,7 @@ mod tests { Pallet::::submit_parachain_heads( Origin::signed(1), (0, test_relay_header(0, state_root).hash()), - vec![ParaId(1), ParaId(2), ParaId(3)], + parachains.clone(), proof.clone(), ), Error::::BridgeModule(OwnedBridgeModuleError::Halted) @@ -617,7 +636,7 @@ mod tests { assert_ok!(Pallet::::submit_parachain_heads( Origin::signed(1), (0, test_relay_header(0, state_root).hash()), - vec![ParaId(1)], + parachains, proof, ),); }); @@ -625,7 +644,7 @@ mod tests { #[test] fn imports_initial_parachain_heads() { - let (state_root, proof) = + let (state_root, proof, parachains) = prepare_parachain_heads_proof(vec![(1, head_data(1, 0)), (3, head_data(3, 10))]); run_test(|| { initialize(state_root); @@ -634,7 +653,7 @@ mod tests { assert_ok!(Pallet::::submit_parachain_heads( Origin::signed(1), (0, test_relay_header(0, state_root).hash()), - vec![ParaId(1), ParaId(2), ParaId(3)], + parachains, proof, ),); @@ -667,12 +686,14 @@ mod tests { #[test] fn imports_parachain_heads_is_able_to_progress() { - let (state_root_5, proof_5) = prepare_parachain_heads_proof(vec![(1, head_data(1, 5))]); - let (state_root_10, proof_10) = prepare_parachain_heads_proof(vec![(1, head_data(1, 10))]); + let (state_root_5, proof_5, parachains_5) = + prepare_parachain_heads_proof(vec![(1, head_data(1, 5))]); + let (state_root_10, proof_10, parachains_10) = + prepare_parachain_heads_proof(vec![(1, head_data(1, 10))]); run_test(|| { // start with relay block #0 and import head#5 of parachain#1 initialize(state_root_5); - assert_ok!(import_parachain_1_head(0, state_root_5, proof_5)); + assert_ok!(import_parachain_1_head(0, state_root_5, parachains_5, proof_5)); assert_eq!( BestParaHeads::::get(ParaId(1)), Some(BestParaHead { @@ -692,7 +713,7 @@ mod tests { // import head#10 of parachain#1 at relay block #1 proceed(1, state_root_10); - assert_ok!(import_parachain_1_head(1, state_root_10, proof_10)); + assert_ok!(import_parachain_1_head(1, state_root_10, parachains_10, proof_10)); assert_eq!( BestParaHeads::::get(ParaId(1)), Some(BestParaHead { @@ -714,7 +735,7 @@ mod tests { #[test] fn ignores_untracked_parachain() { - let (state_root, proof) = prepare_parachain_heads_proof(vec![ + let (state_root, proof, parachains) = prepare_parachain_heads_proof(vec![ (1, head_data(1, 5)), (UNTRACKED_PARACHAIN_ID, head_data(1, 5)), (2, head_data(1, 5)), @@ -726,7 +747,7 @@ mod tests { assert_ok!(Pallet::::submit_parachain_heads( Origin::signed(1), (0, test_relay_header(0, state_root).hash()), - vec![ParaId(1), ParaId(UNTRACKED_PARACHAIN_ID), ParaId(2)], + parachains, proof, )); assert_eq!( @@ -751,32 +772,35 @@ mod tests { #[test] fn does_nothing_when_already_imported_this_head_at_previous_relay_header() { - let (state_root, proof) = prepare_parachain_heads_proof(vec![(1, head_data(1, 0))]); + let (state_root, proof, parachains) = + prepare_parachain_heads_proof(vec![(1, head_data(1, 0))]); run_test(|| { // import head#0 of parachain#1 at relay block#0 initialize(state_root); - assert_ok!(import_parachain_1_head(0, state_root, proof.clone())); + assert_ok!(import_parachain_1_head(0, state_root, parachains.clone(), proof.clone())); assert_eq!(BestParaHeads::::get(ParaId(1)), Some(initial_best_head(1))); // try to import head#0 of parachain#1 at relay block#1 // => call succeeds, but nothing is changed proceed(1, state_root); - assert_ok!(import_parachain_1_head(1, state_root, proof)); + assert_ok!(import_parachain_1_head(1, state_root, parachains, proof)); assert_eq!(BestParaHeads::::get(ParaId(1)), Some(initial_best_head(1))); }); } #[test] fn does_nothing_when_already_imported_head_at_better_relay_header() { - let (state_root_5, proof_5) = prepare_parachain_heads_proof(vec![(1, head_data(1, 5))]); - let (state_root_10, proof_10) = prepare_parachain_heads_proof(vec![(1, head_data(1, 10))]); + let (state_root_5, proof_5, parachains_5) = + prepare_parachain_heads_proof(vec![(1, head_data(1, 5))]); + let (state_root_10, proof_10, parachains_10) = + prepare_parachain_heads_proof(vec![(1, head_data(1, 10))]); run_test(|| { // start with relay block #0 initialize(state_root_5); // head#10 of parachain#1 at relay block#1 proceed(1, state_root_10); - assert_ok!(import_parachain_1_head(1, state_root_10, proof_10)); + assert_ok!(import_parachain_1_head(1, state_root_10, parachains_10, proof_10)); assert_eq!( BestParaHeads::::get(ParaId(1)), Some(BestParaHead { @@ -786,9 +810,9 @@ mod tests { }) ); - // now try to import head#1 at relay block#0 + // now try to import head#5 at relay block#0 // => nothing is changed, because better head has already been imported - assert_ok!(import_parachain_1_head(0, state_root_5, proof_5)); + assert_ok!(import_parachain_1_head(0, state_root_5, parachains_5, proof_5)); assert_eq!( BestParaHeads::::get(ParaId(1)), Some(BestParaHead { @@ -807,7 +831,8 @@ mod tests { // import exactly `HeadsToKeep` headers for i in 0..heads_to_keep { - let (state_root, proof) = prepare_parachain_heads_proof(vec![(1, head_data(1, i))]); + let (state_root, proof, parachains) = + prepare_parachain_heads_proof(vec![(1, head_data(1, i))]); if i == 0 { initialize(state_root); } else { @@ -815,7 +840,7 @@ mod tests { } let expected_weight = weight_of_import_parachain_1_head(&proof, false); - let result = import_parachain_1_head(i, state_root, proof); + let result = import_parachain_1_head(i, state_root, parachains, proof); assert_ok!(result); assert_eq!(result.expect("checked above").actual_weight, Some(expected_weight)); } @@ -827,11 +852,11 @@ mod tests { } // import next relay chain header and next parachain head - let (state_root, proof) = + let (state_root, proof, parachains) = prepare_parachain_heads_proof(vec![(1, head_data(1, heads_to_keep))]); proceed(heads_to_keep, state_root); let expected_weight = weight_of_import_parachain_1_head(&proof, true); - let result = import_parachain_1_head(heads_to_keep, state_root, proof); + let result = import_parachain_1_head(heads_to_keep, state_root, parachains, proof); assert_ok!(result); assert_eq!(result.expect("checked above").actual_weight, Some(expected_weight)); @@ -848,14 +873,15 @@ mod tests { #[test] fn fails_on_unknown_relay_chain_block() { - let (state_root, proof) = prepare_parachain_heads_proof(vec![(1, head_data(1, 5))]); + let (state_root, proof, parachains) = + prepare_parachain_heads_proof(vec![(1, head_data(1, 5))]); run_test(|| { // start with relay block #0 initialize(state_root); // try to import head#5 of parachain#1 at unknown relay chain block #1 assert_noop!( - import_parachain_1_head(1, state_root, proof), + import_parachain_1_head(1, state_root, parachains, proof), Error::::UnknownRelayChainBlock ); }); @@ -863,14 +889,15 @@ mod tests { #[test] fn fails_on_invalid_storage_proof() { - let (_state_root, proof) = prepare_parachain_heads_proof(vec![(1, head_data(1, 5))]); + let (_state_root, proof, parachains) = + prepare_parachain_heads_proof(vec![(1, head_data(1, 5))]); run_test(|| { // start with relay block #0 initialize(Default::default()); // try to import head#5 of parachain#1 at relay chain block #0 assert_noop!( - import_parachain_1_head(0, Default::default(), proof), + import_parachain_1_head(0, Default::default(), parachains, proof), Error::::InvalidStorageProof ); }); @@ -878,15 +905,16 @@ mod tests { #[test] fn is_not_rewriting_existing_head_if_failed_to_read_updated_head() { - let (state_root_5, proof_5) = prepare_parachain_heads_proof(vec![(1, head_data(1, 5))]); - let (state_root_10_at_20, proof_10_at_20) = + let (state_root_5, proof_5, parachains_5) = + prepare_parachain_heads_proof(vec![(1, head_data(1, 5))]); + let (state_root_10_at_20, proof_10_at_20, parachains_10_at_20) = prepare_parachain_heads_proof(vec![(2, head_data(2, 10))]); - let (state_root_10_at_30, proof_10_at_30) = + let (state_root_10_at_30, proof_10_at_30, parachains_10_at_30) = prepare_parachain_heads_proof(vec![(1, head_data(1, 10))]); run_test(|| { // we've already imported head#5 of parachain#1 at relay block#10 initialize(state_root_5); - import_parachain_1_head(0, state_root_5, proof_5).expect("ok"); + import_parachain_1_head(0, state_root_5, parachains_5, proof_5).expect("ok"); assert_eq!( Pallet::::best_parachain_head(ParaId(1)), Some(head_data(1, 5)) @@ -900,7 +928,7 @@ mod tests { assert_ok!(Pallet::::submit_parachain_heads( Origin::signed(1), (20, test_relay_header(20, state_root_10_at_20).hash()), - vec![ParaId(1)], + parachains_10_at_20, proof_10_at_20, ),); assert_eq!( @@ -916,7 +944,7 @@ mod tests { assert_ok!(Pallet::::submit_parachain_heads( Origin::signed(1), (30, test_relay_header(30, state_root_10_at_30).hash()), - vec![ParaId(1)], + parachains_10_at_30, proof_10_at_30, ),); assert_eq!( diff --git a/bridges/relays/lib-substrate-relay/src/parachains/mod.rs b/bridges/relays/lib-substrate-relay/src/parachains/mod.rs index 51549ef02bd52..f201476aa1635 100644 --- a/bridges/relays/lib-substrate-relay/src/parachains/mod.rs +++ b/bridges/relays/lib-substrate-relay/src/parachains/mod.rs @@ -18,7 +18,7 @@ //! parachain finality proofs synchronization pipelines. use async_trait::async_trait; -use bp_polkadot_core::parachains::{ParaHeadsProof, ParaId}; +use bp_polkadot_core::parachains::{ParaHash, ParaHeadsProof, ParaId}; use pallet_bridge_parachains::{ Call as BridgeParachainsCall, Config as BridgeParachainsConfig, RelayBlockHash, RelayBlockHasher, RelayBlockNumber, @@ -71,7 +71,7 @@ pub trait SubmitParachainHeadsCallBuilder: /// function of bridge parachains module at the target chain. fn build_submit_parachain_heads_call( at_relay_block: HeaderIdOf, - parachains: Vec, + parachains: Vec<(ParaId, ParaHash)>, parachain_heads_proof: ParaHeadsProof, ) -> CallOf; } @@ -97,7 +97,7 @@ where { fn build_submit_parachain_heads_call( at_relay_block: HeaderIdOf, - parachains: Vec, + parachains: Vec<(ParaId, ParaHash)>, parachain_heads_proof: ParaHeadsProof, ) -> CallOf { BridgeParachainsCall::::submit_parachain_heads { diff --git a/bridges/relays/lib-substrate-relay/src/parachains/source.rs b/bridges/relays/lib-substrate-relay/src/parachains/source.rs index c613387e7a258..ecc940467035e 100644 --- a/bridges/relays/lib-substrate-relay/src/parachains/source.rs +++ b/bridges/relays/lib-substrate-relay/src/parachains/source.rs @@ -160,23 +160,46 @@ where &self, at_block: HeaderIdOf, parachains: &[ParaId], - ) -> Result { - let storage_keys = parachains - .iter() - .map(|para_id| { - parachain_head_storage_key_at_source( - P::SourceRelayChain::PARAS_PALLET_NAME, - *para_id, - ) - }) - .collect(); + ) -> Result<(ParaHeadsProof, Vec), Self::Error> { + if parachains.len() != 1 || parachains[0].0 != P::SOURCE_PARACHAIN_PARA_ID { + return Err(SubstrateError::Custom(format!( + "Trying to prove unexpected parachains {:?}. Expected {:?}", + parachains, + P::SOURCE_PARACHAIN_PARA_ID, + ))) + } + + let parachain = parachains[0]; + let storage_key = + parachain_head_storage_key_at_source(P::SourceRelayChain::PARAS_PALLET_NAME, parachain); let parachain_heads_proof = self .client - .prove_storage(storage_keys, at_block.1) + .prove_storage(vec![storage_key.clone()], at_block.1) .await? .iter_nodes() .collect(); - Ok(ParaHeadsProof(parachain_heads_proof)) + // why we're reading parachain head here once again (it has already been read at the + // `parachain_head`)? that's because `parachain_head` sometimes returns obsolete parachain + // head and loop sometimes asks to prove this obsolete head and gets other (actual) head + // instead + // + // => since we want to provide proper hashes in our `submit_parachain_heads` call, we're + // rereading actual value here + let parachain_head = self + .client + .raw_storage_value(storage_key, Some(at_block.1)) + .await? + .map(|h| ParaHead::decode(&mut &h.0[..])) + .transpose()? + .ok_or_else(|| { + SubstrateError::Custom(format!( + "Failed to read expected parachain {:?} head at {:?}", + parachain, at_block + )) + })?; + let parachain_head_hash = parachain_head.hash(); + + Ok((ParaHeadsProof(parachain_heads_proof), vec![parachain_head_hash])) } } diff --git a/bridges/relays/lib-substrate-relay/src/parachains/target.rs b/bridges/relays/lib-substrate-relay/src/parachains/target.rs index ca30629198b5b..d26894b9bd302 100644 --- a/bridges/relays/lib-substrate-relay/src/parachains/target.rs +++ b/bridges/relays/lib-substrate-relay/src/parachains/target.rs @@ -28,7 +28,7 @@ use bp_parachains::{ best_parachain_head_hash_storage_key_at_target, imported_parachain_head_storage_key_at_target, BestParaHeadHash, }; -use bp_polkadot_core::parachains::{ParaHead, ParaHeadsProof, ParaId}; +use bp_polkadot_core::parachains::{ParaHash, ParaHead, ParaHeadsProof, ParaId}; use codec::{Decode, Encode}; use parachains_relay::{ parachains_loop::TargetClient, parachains_loop_metrics::ParachainsLoopMetrics, @@ -166,7 +166,7 @@ where async fn submit_parachain_heads_proof( &self, at_relay_block: HeaderIdOf, - updated_parachains: Vec, + updated_parachains: Vec<(ParaId, ParaHash)>, proof: ParaHeadsProof, ) -> Result<(), Self::Error> { let genesis_hash = *self.client.genesis_hash(); diff --git a/bridges/relays/parachains/src/parachains_loop.rs b/bridges/relays/parachains/src/parachains_loop.rs index 60627d23e0b15..795d07b8440ec 100644 --- a/bridges/relays/parachains/src/parachains_loop.rs +++ b/bridges/relays/parachains/src/parachains_loop.rs @@ -69,6 +69,16 @@ pub enum ParaHashAtSource { Unavailable, } +impl ParaHashAtSource { + /// Return parachain head hash, if available. + pub fn hash(&self) -> Option<&ParaHash> { + match *self { + ParaHashAtSource::Some(ref para_hash) => Some(para_hash), + _ => None, + } + } +} + /// Source client used in parachain heads synchronization loop. #[async_trait] pub trait SourceClient: RelayClient { @@ -87,11 +97,15 @@ pub trait SourceClient: RelayClient { ) -> Result; /// Get parachain heads proof. + /// + /// The number and order of entries in the resulting parachain head hashes vector must match the + /// number and order of parachains in the `parachains` vector. The incorrect implementation will + /// result in panic. async fn prove_parachain_heads( &self, at_block: HeaderIdOf, parachains: &[ParaId], - ) -> Result; + ) -> Result<(ParaHeadsProof, Vec), Self::Error>; } /// Target client used in parachain heads synchronization loop. @@ -121,7 +135,7 @@ pub trait TargetClient: RelayClient { async fn submit_parachain_heads_proof( &self, at_source_block: HeaderIdOf, - updated_parachains: Vec, + updated_parachains: Vec<(ParaId, ParaHash)>, proof: ParaHeadsProof, ) -> Result<(), Self::Error>; } @@ -274,7 +288,7 @@ where ); if is_update_required { - let heads_proofs = source_client + let (heads_proofs, head_hashes) = source_client .prove_parachain_heads(best_finalized_relay_block, &updated_ids) .await .map_err(|e| { @@ -292,10 +306,17 @@ where P::SourceChain::NAME, P::TargetChain::NAME, ); + + assert_eq!( + head_hashes.len(), + updated_ids.len(), + "Incorrect parachains SourceClient implementation" + ); + target_client .submit_parachain_heads_proof( best_finalized_relay_block, - updated_ids.clone(), + updated_ids.iter().cloned().zip(head_hashes).collect(), heads_proofs, ) .await @@ -394,7 +415,7 @@ where needs_update }) - .map(|((para_id, _), _)| para_id) + .map(|((para, _), _)| para) .collect() } @@ -666,7 +687,7 @@ mod tests { &self, _at_block: HeaderIdOf, parachains: &[ParaId], - ) -> Result { + ) -> Result<(ParaHeadsProof, Vec), TestError> { let mut proofs = Vec::new(); for para_id in parachains { proofs.push( @@ -680,7 +701,7 @@ mod tests { .ok_or(TestError::MissingParachainHeadProof)?, ); } - Ok(ParaHeadsProof(proofs)) + Ok((ParaHeadsProof(proofs), vec![Default::default(); parachains.len()])) } } @@ -709,7 +730,7 @@ mod tests { async fn submit_parachain_heads_proof( &self, _at_source_block: HeaderIdOf, - _updated_parachains: Vec, + _updated_parachains: Vec<(ParaId, ParaHash)>, _proof: ParaHeadsProof, ) -> Result<(), Self::Error> { self.data.lock().await.target_submit_result.clone()?;