From 4f3f574fb330a46d86fa319175c97fbcbd9610af Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Fri, 9 Sep 2022 13:09:05 +0300 Subject: [PATCH] Add events to the bridge parachains pallet (#1568) * add events to the bridge parachains pallet * clippy + spellcheck * fix compilation * untracked is not a word? --- bridges/bin/millau/runtime/src/lib.rs | 6 +- bridges/modules/parachains/src/extension.rs | 12 +- bridges/modules/parachains/src/lib.rs | 263 +++++++++++++++++++- bridges/modules/parachains/src/mock.rs | 11 +- 4 files changed, 275 insertions(+), 17 deletions(-) diff --git a/bridges/bin/millau/runtime/src/lib.rs b/bridges/bin/millau/runtime/src/lib.rs index 66f454a0ecc9e..2fea97cea8d49 100644 --- a/bridges/bin/millau/runtime/src/lib.rs +++ b/bridges/bin/millau/runtime/src/lib.rs @@ -540,6 +540,7 @@ parameter_types! { pub type WithRialtoParachainsInstance = (); impl pallet_bridge_parachains::Config for Runtime { + type Event = Event; type WeightInfo = pallet_bridge_parachains::weights::MillauWeight; type BridgesGrandpaPalletInstance = RialtoGrandpaInstance; type ParasPalletName = RialtoParasPalletName; @@ -551,6 +552,7 @@ impl pallet_bridge_parachains::Config for Runtime pub type WithWestendParachainsInstance = pallet_bridge_parachains::Instance1; impl pallet_bridge_parachains::Config for Runtime { + type Event = Event; type WeightInfo = pallet_bridge_parachains::weights::MillauWeight; type BridgesGrandpaPalletInstance = WestendGrandpaInstance; type ParasPalletName = WestendParasPalletName; @@ -592,10 +594,10 @@ construct_runtime!( // Westend bridge modules. BridgeWestendGrandpa: pallet_bridge_grandpa::::{Pallet, Call, Config, Storage}, - BridgeWestendParachains: pallet_bridge_parachains::::{Pallet, Call, Storage}, + BridgeWestendParachains: pallet_bridge_parachains::::{Pallet, Call, Storage, Event}, // RialtoParachain bridge modules. - BridgeRialtoParachains: pallet_bridge_parachains::{Pallet, Call, Storage}, + BridgeRialtoParachains: pallet_bridge_parachains::{Pallet, Call, Storage, Event}, BridgeRialtoParachainMessages: pallet_bridge_messages::::{Pallet, Call, Storage, Event, Config}, // Pallet for sending XCM. diff --git a/bridges/modules/parachains/src/extension.rs b/bridges/modules/parachains/src/extension.rs index fb93e671e228c..05500cf41ddb3 100644 --- a/bridges/modules/parachains/src/extension.rs +++ b/bridges/modules/parachains/src/extension.rs @@ -17,7 +17,7 @@ use crate::{Config, Pallet, RelayBlockHash, RelayBlockHasher, RelayBlockNumber}; use bp_runtime::FilterCall; use frame_support::{dispatch::CallableCallFor, traits::IsSubType}; -use sp_runtime::transaction_validity::{TransactionValidity, ValidTransaction}; +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 @@ -57,13 +57,19 @@ where }; let maybe_stored_best_head = crate::ParasInfo::::get(parachain); - Self::validate_updated_parachain_head( + let is_valid = Self::validate_updated_parachain_head( parachain, &maybe_stored_best_head, updated_at_relay_block_number, parachain_head_hash, "Rejecting obsolete parachain-head transaction", - ) + ); + + if is_valid { + Ok(ValidTransaction::default()) + } else { + InvalidTransaction::Stale.into() + } } } diff --git a/bridges/modules/parachains/src/lib.rs b/bridges/modules/parachains/src/lib.rs index bf07cdc9e6730..944059d6d9e0e 100644 --- a/bridges/modules/parachains/src/lib.rs +++ b/bridges/modules/parachains/src/lib.rs @@ -77,6 +77,27 @@ pub mod pallet { /// Weight info of the given parachains pallet. pub type WeightInfoOf = >::WeightInfo; + #[pallet::event] + #[pallet::generate_deposit(pub(super) fn deposit_event)] + pub enum Event, I: 'static = ()> { + /// The caller has provided head of parachain that the pallet is not configured to track. + UntrackedParachainRejected { parachain: ParaId }, + /// The caller has declared that he has provided given parachain head, but it is missing + /// from the storage proof. + MissingParachainHead { parachain: ParaId }, + /// The caller has provided parachain head hash that is not matching the hash read from the + /// storage proof. + IncorrectParachainHeadHash { + parachain: ParaId, + parachain_head_hash: ParaHash, + actual_parachain_head_hash: ParaHash, + }, + /// The caller has provided obsolete parachain head, which is already known to the pallet. + RejectedObsoleteParachainHead { parachain: ParaId, parachain_head_hash: ParaHash }, + /// Parachain head has been updated. + UpdatedParachainHead { parachain: ParaId, parachain_head_hash: ParaHash }, + } + #[pallet::error] pub enum Error { /// Relay chain block hash is unknown to us. @@ -100,6 +121,8 @@ pub mod pallet { pub trait Config: pallet_bridge_grandpa::Config { + /// The overarching event type. + type Event: From> + IsType<::Event>; /// Benchmarks results from runtime we're plugged into. type WeightInfo: WeightInfoExt; @@ -248,6 +271,7 @@ pub mod pallet { "The head of parachain {:?} has been provided, but it is not tracked by the pallet", parachain, ); + Self::deposit_event(Event::UntrackedParachainRejected { parachain }); continue; } @@ -264,6 +288,7 @@ pub mod pallet { "Looks like it has been deregistered from the source relay chain" }, ); + Self::deposit_event(Event::MissingParachainHead { parachain }); continue; }, Err(e) => { @@ -273,6 +298,7 @@ pub mod pallet { parachain, e, ); + Self::deposit_event(Event::MissingParachainHead { parachain }); continue; }, }; @@ -288,6 +314,11 @@ pub mod pallet { parachain_head_hash, actual_parachain_head_hash, ); + Self::deposit_event(Event::IncorrectParachainHeadHash { + parachain, + parachain_head_hash, + actual_parachain_head_hash, + }); continue; } @@ -387,16 +418,20 @@ pub mod pallet { /// Check if para head has been already updated at better relay chain block. /// Without this check, we may import heads in random order. + /// + /// Returns `true` if the pallet is ready to import given parachain head. + /// Returns `false` if the pallet already knows the same or better parachain head. + #[must_use] pub fn validate_updated_parachain_head( parachain: ParaId, maybe_stored_best_head: &Option, updated_at_relay_block_number: RelayBlockNumber, updated_head_hash: ParaHash, err_log_prefix: &str, - ) -> TransactionValidity { + ) -> bool { let stored_best_head = match maybe_stored_best_head { Some(stored_best_head) => stored_best_head, - None => return Ok(ValidTransaction::default()), + None => return true, }; if stored_best_head.best_head_hash.at_relay_block_number >= @@ -410,7 +445,7 @@ pub mod pallet { stored_best_head.best_head_hash.at_relay_block_number, updated_at_relay_block_number ); - return InvalidTransaction::Stale.into() + return false } if stored_best_head.best_head_hash.head_hash == updated_head_hash { @@ -423,10 +458,10 @@ pub mod pallet { stored_best_head.best_head_hash.at_relay_block_number, updated_at_relay_block_number ); - return InvalidTransaction::Stale.into() + return false } - Ok(ValidTransaction::default()) + true } /// Try to update parachain head. @@ -439,14 +474,20 @@ pub mod pallet { ) -> Result { // check if head has been already updated at better relay chain block. Without this // check, we may import heads in random order - Self::validate_updated_parachain_head( + let is_valid = Self::validate_updated_parachain_head( parachain, &stored_best_head, updated_at_relay_block_number, updated_head_hash, "The parachain head can't be updated", - ) - .map_err(|_| ())?; + ); + if !is_valid { + Self::deposit_event(Event::RejectedObsoleteParachainHead { + parachain, + parachain_head_hash: updated_head_hash, + }); + return Err(()) + } let next_imported_hash_position = stored_best_head .map_or(0, |stored_best_head| stored_best_head.next_imported_hash_position); @@ -485,6 +526,10 @@ pub mod pallet { ); ImportedParaHeads::::remove(parachain, head_hash_to_prune); } + Self::deposit_event(Event::UpdatedParachainHead { + parachain, + parachain_head_hash: updated_head_hash, + }); Ok(UpdateParachainHeadArtifacts { best_head: updated_best_para_head, prune_happened }) } @@ -526,7 +571,8 @@ pub mod pallet { mod tests { use super::*; use crate::mock::{ - run_test, test_relay_header, Origin, TestRuntime, PARAS_PALLET_NAME, UNTRACKED_PARACHAIN_ID, + run_test, test_relay_header, Event as TestEvent, Origin, TestRuntime, PARAS_PALLET_NAME, + UNTRACKED_PARACHAIN_ID, }; use codec::Encode; @@ -545,6 +591,7 @@ mod tests { traits::{Get, OnInitialize}, weights::Weight, }; + use frame_system::{EventRecord, Pallet as System, Phase}; use sp_runtime::DispatchError; use sp_trie::{trie_types::TrieDBMutBuilderV1, LayoutV1, MemoryDB, Recorder, TrieMut}; @@ -733,6 +780,28 @@ mod tests { ImportedParaHeads::::get(ParaId(3), head_hash(3, 10)), Some(head_data(3, 10)) ); + + assert_eq!( + System::::events(), + vec![ + EventRecord { + phase: Phase::Initialization, + event: TestEvent::Parachains(Event::UpdatedParachainHead { + parachain: ParaId(1), + parachain_head_hash: initial_best_head(1).best_head_hash.head_hash, + }), + topics: vec![], + }, + EventRecord { + phase: Phase::Initialization, + event: TestEvent::Parachains(Event::UpdatedParachainHead { + parachain: ParaId(3), + parachain_head_hash: head_data(3, 10).hash(), + }), + topics: vec![], + } + ], + ); }); } @@ -764,6 +833,17 @@ mod tests { ImportedParaHeads::::get(ParaId(1), head_data(1, 10).hash()), None ); + assert_eq!( + System::::events(), + vec![EventRecord { + phase: Phase::Initialization, + event: TestEvent::Parachains(Event::UpdatedParachainHead { + parachain: ParaId(1), + parachain_head_hash: head_data(1, 5).hash(), + }), + topics: vec![], + }], + ); // import head#10 of parachain#1 at relay block #1 proceed(1, state_root_10); @@ -786,6 +866,27 @@ mod tests { ImportedParaHeads::::get(ParaId(1), head_data(1, 10).hash()), Some(head_data(1, 10)) ); + assert_eq!( + System::::events(), + vec![ + EventRecord { + phase: Phase::Initialization, + event: TestEvent::Parachains(Event::UpdatedParachainHead { + parachain: ParaId(1), + parachain_head_hash: head_data(1, 5).hash(), + }), + topics: vec![], + }, + EventRecord { + phase: Phase::Initialization, + event: TestEvent::Parachains(Event::UpdatedParachainHead { + parachain: ParaId(1), + parachain_head_hash: head_data(1, 10).hash(), + }), + topics: vec![], + } + ], + ); }); } @@ -834,6 +935,34 @@ mod tests { next_imported_hash_position: 1, }) ); + assert_eq!( + System::::events(), + vec![ + EventRecord { + phase: Phase::Initialization, + event: TestEvent::Parachains(Event::UpdatedParachainHead { + parachain: ParaId(1), + parachain_head_hash: head_data(1, 5).hash(), + }), + topics: vec![], + }, + EventRecord { + phase: Phase::Initialization, + event: TestEvent::Parachains(Event::UntrackedParachainRejected { + parachain: ParaId(UNTRACKED_PARACHAIN_ID), + }), + topics: vec![], + }, + EventRecord { + phase: Phase::Initialization, + event: TestEvent::Parachains(Event::UpdatedParachainHead { + parachain: ParaId(2), + parachain_head_hash: head_data(1, 5).hash(), + }), + topics: vec![], + } + ], + ); }); } @@ -846,12 +975,44 @@ mod tests { initialize(state_root); assert_ok!(import_parachain_1_head(0, state_root, parachains.clone(), proof.clone())); assert_eq!(ParasInfo::::get(ParaId(1)), Some(initial_best_head(1))); + assert_eq!( + System::::events(), + vec![EventRecord { + phase: Phase::Initialization, + event: TestEvent::Parachains(Event::UpdatedParachainHead { + parachain: ParaId(1), + parachain_head_hash: initial_best_head(1).best_head_hash.head_hash, + }), + topics: vec![], + }], + ); // 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, parachains, proof)); assert_eq!(ParasInfo::::get(ParaId(1)), Some(initial_best_head(1))); + assert_eq!( + System::::events(), + vec![ + EventRecord { + phase: Phase::Initialization, + event: TestEvent::Parachains(Event::UpdatedParachainHead { + parachain: ParaId(1), + parachain_head_hash: initial_best_head(1).best_head_hash.head_hash, + }), + topics: vec![], + }, + EventRecord { + phase: Phase::Initialization, + event: TestEvent::Parachains(Event::RejectedObsoleteParachainHead { + parachain: ParaId(1), + parachain_head_hash: initial_best_head(1).best_head_hash.head_hash, + }), + topics: vec![], + } + ], + ); }); } @@ -878,6 +1039,17 @@ mod tests { next_imported_hash_position: 1, }) ); + assert_eq!( + System::::events(), + vec![EventRecord { + phase: Phase::Initialization, + event: TestEvent::Parachains(Event::UpdatedParachainHead { + parachain: ParaId(1), + parachain_head_hash: head_data(1, 10).hash(), + }), + topics: vec![], + }], + ); // now try to import head#5 at relay block#0 // => nothing is changed, because better head has already been imported @@ -892,6 +1064,27 @@ mod tests { next_imported_hash_position: 1, }) ); + assert_eq!( + System::::events(), + vec![ + EventRecord { + phase: Phase::Initialization, + event: TestEvent::Parachains(Event::UpdatedParachainHead { + parachain: ParaId(1), + parachain_head_hash: head_data(1, 10).hash(), + }), + topics: vec![], + }, + EventRecord { + phase: Phase::Initialization, + event: TestEvent::Parachains(Event::RejectedObsoleteParachainHead { + parachain: ParaId(1), + parachain_head_hash: head_data(1, 5).hash(), + }), + topics: vec![], + } + ], + ); }); } @@ -1047,5 +1240,57 @@ mod tests { ); } + #[test] + fn ignores_parachain_head_if_it_is_missing_from_storage_proof() { + let (state_root, proof, _) = prepare_parachain_heads_proof(vec![(1, head_data(1, 0))]); + let parachains = vec![(ParaId(2), Default::default())]; + run_test(|| { + initialize(state_root); + assert_ok!(Pallet::::submit_parachain_heads( + Origin::signed(1), + (0, test_relay_header(0, state_root).hash()), + parachains, + proof, + )); + assert_eq!( + System::::events(), + vec![EventRecord { + phase: Phase::Initialization, + event: TestEvent::Parachains(Event::MissingParachainHead { + parachain: ParaId(2), + }), + topics: vec![], + }], + ); + }); + } + + #[test] + fn ignores_parachain_head_if_parachain_head_hash_is_wrong() { + let (state_root, proof, _) = prepare_parachain_heads_proof(vec![(1, head_data(1, 0))]); + let parachains = vec![(ParaId(1), head_data(1, 10).hash())]; + run_test(|| { + initialize(state_root); + assert_ok!(Pallet::::submit_parachain_heads( + Origin::signed(1), + (0, test_relay_header(0, state_root).hash()), + parachains, + proof, + )); + assert_eq!( + System::::events(), + vec![EventRecord { + phase: Phase::Initialization, + event: TestEvent::Parachains(Event::IncorrectParachainHeadHash { + parachain: ParaId(1), + parachain_head_hash: head_data(1, 10).hash(), + actual_parachain_head_hash: head_data(1, 0).hash(), + }), + topics: vec![], + }], + ); + }); + } + generate_owned_bridge_module_tests!(BasicOperatingMode::Normal, BasicOperatingMode::Halted); } diff --git a/bridges/modules/parachains/src/mock.rs b/bridges/modules/parachains/src/mock.rs index eaf5d3b4fdd91..292856f35a766 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::{Call, Pallet}, + Parachains: pallet_bridge_parachains::{Call, Pallet, Event}, } } @@ -67,7 +67,7 @@ impl frame_system::Config for TestRuntime { type AccountId = AccountId; type Lookup = IdentityLookup; type Header = Header; - type Event = (); + type Event = Event; type BlockHashCount = BlockHashCount; type Version = (); type PalletInfo = PalletInfo; @@ -112,6 +112,7 @@ parameter_types! { } impl pallet_bridge_parachains::Config for TestRuntime { + type Event = Event; type WeightInfo = (); type BridgesGrandpaPalletInstance = pallet_bridge_grandpa::Instance1; type ParasPalletName = ParasPalletName; @@ -166,7 +167,11 @@ impl Chain for OtherBridgedChain { } pub fn run_test(test: impl FnOnce() -> T) -> T { - sp_io::TestExternalities::new(Default::default()).execute_with(test) + sp_io::TestExternalities::new(Default::default()).execute_with(|| { + System::set_block_number(1); + System::reset_events(); + test() + }) } pub fn test_relay_header(