From 0a1ace34afdd97805ec9e5d7e298e055b0308910 Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Thu, 4 Mar 2021 14:50:30 -0500 Subject: [PATCH] Unify Substrate Bridge Pallet with Finality Verifier (#783) * Add relevant storage items from Substrate pallet * Add function for importing finalized headers to storage * Remove unused getter functions * Add GenesisConfig to pallet * Add initialization extrinsic * Add operational extrinsic * Get existing finality verifier tests compiling again * Add tests for pallet initialization * Add tests related to pallet's operational status * Update tests which were using `pallet-substrate-bridge` * Add tests related to header imports * Use wrapper function when init-ing some tests * Add prefix to tests related to rate limiter * Fix failed compilation related to GenesisConfig * Add some documentation * Change some extrinsics to be Operational * Add public interface to pallet * Implement runtime APIs for finality-verifier pallet * Justify use of `expect` when importing headers * Reject headers with forced changes * Add weight to initialize extrinsic * Remove TODO which will be addressed later * Move succesful import log to correct location * Expand proof for when `best_finalized` is fetched * Move check for newer finalized blocks earlier in pipeline * Rename `ConflictingFork` error to be more generic * Only compute finality_target's hash once * Add missing documentation to Runtime APIs * Add TODO about using `set_id` from `ScheduledChange` digest --- bridges/bin/millau/runtime/src/lib.rs | 11 + bridges/bin/rialto/runtime/src/lib.rs | 11 + bridges/modules/finality-verifier/Cargo.toml | 2 + bridges/modules/finality-verifier/src/lib.rs | 605 ++++++++++++++++-- bridges/modules/finality-verifier/src/mock.rs | 8 +- bridges/primitives/millau/src/lib.rs | 18 +- bridges/primitives/rialto/src/lib.rs | 18 +- 7 files changed, 630 insertions(+), 43 deletions(-) diff --git a/bridges/bin/millau/runtime/src/lib.rs b/bridges/bin/millau/runtime/src/lib.rs index 89440184002f..090257419d54 100644 --- a/bridges/bin/millau/runtime/src/lib.rs +++ b/bridges/bin/millau/runtime/src/lib.rs @@ -559,6 +559,17 @@ impl_runtime_apis! { } } + impl bp_rialto::RialtoFinalityApi for Runtime { + fn best_finalized() -> (bp_rialto::BlockNumber, bp_rialto::Hash) { + let header = BridgeFinalityVerifier::best_finalized(); + (header.number, header.hash()) + } + + fn is_known_header(hash: bp_rialto::Hash) -> bool { + BridgeFinalityVerifier::is_known_header(hash) + } + } + impl bp_rialto::ToRialtoOutboundLaneApi for Runtime { fn estimate_message_delivery_and_dispatch_fee( _lane_id: bp_message_lane::LaneId, diff --git a/bridges/bin/rialto/runtime/src/lib.rs b/bridges/bin/rialto/runtime/src/lib.rs index 6bb73ef037aa..eed087bdf82c 100644 --- a/bridges/bin/rialto/runtime/src/lib.rs +++ b/bridges/bin/rialto/runtime/src/lib.rs @@ -642,6 +642,17 @@ impl_runtime_apis! { } } + impl bp_millau::MillauFinalityApi for Runtime { + fn best_finalized() -> (bp_millau::BlockNumber, bp_millau::Hash) { + let header = BridgeFinalityVerifier::best_finalized(); + (header.number, header.hash()) + } + + fn is_known_header(hash: bp_millau::Hash) -> bool { + BridgeFinalityVerifier::is_known_header(hash) + } + } + impl bp_currency_exchange::RialtoCurrencyExchangeApi for Runtime { fn filter_transaction_proof(proof: exchange::EthereumTransactionInclusionProof) -> bool { BridgeRialtoCurrencyExchange::filter_transaction_proof(&proof) diff --git a/bridges/modules/finality-verifier/Cargo.toml b/bridges/modules/finality-verifier/Cargo.toml index 8afc30cd30ed..79279f038bd2 100644 --- a/bridges/modules/finality-verifier/Cargo.toml +++ b/bridges/modules/finality-verifier/Cargo.toml @@ -21,6 +21,7 @@ bp-header-chain = { path = "../../primitives/header-chain", default-features = f frame-support = { git = "https://github.com/paritytech/substrate.git", branch = "master" , default-features = false } frame-system = { git = "https://github.com/paritytech/substrate.git", branch = "master" , default-features = false } +sp-finality-grandpa = { git = "https://github.com/paritytech/substrate.git", branch = "master" , default-features = false } sp-runtime = { git = "https://github.com/paritytech/substrate.git", branch = "master" , default-features = false } sp-std = { git = "https://github.com/paritytech/substrate.git", branch = "master" , default-features = false } @@ -40,6 +41,7 @@ std = [ "frame-support/std", "frame-system/std", "serde", + "sp-finality-grandpa/std", "sp-runtime/std", "sp-std/std", ] diff --git a/bridges/modules/finality-verifier/src/lib.rs b/bridges/modules/finality-verifier/src/lib.rs index 3db6cb7bf7cd..8d0aa7b8cacd 100644 --- a/bridges/modules/finality-verifier/src/lib.rs +++ b/bridges/modules/finality-verifier/src/lib.rs @@ -33,11 +33,16 @@ #![allow(clippy::large_enum_variant)] use bp_header_chain::{justification::verify_justification, AncestryChecker, HeaderChain}; -use bp_runtime::{Chain, HeaderOf}; +use bp_runtime::{BlockNumberOf, Chain, HashOf, HasherOf, HeaderOf}; +use codec::{Decode, Encode}; use finality_grandpa::voter_set::VoterSet; use frame_support::{dispatch::DispatchError, ensure}; -use frame_system::ensure_signed; -use sp_runtime::traits::Header as HeaderT; +use frame_system::{ensure_signed, RawOrigin}; +#[cfg(feature = "std")] +use serde::{Deserialize, Serialize}; +use sp_finality_grandpa::{ConsensusLog, GRANDPA_ENGINE_ID}; +use sp_runtime::traits::{BadOrigin, Header as HeaderT, Zero}; +use sp_runtime::RuntimeDebug; use sp_std::vec::Vec; #[cfg(test)] @@ -46,15 +51,21 @@ mod mock; // Re-export in crate namespace for `construct_runtime!` pub use pallet::*; +/// Block number of the bridged chain. +pub type BridgedBlockNumber = BlockNumberOf<::BridgedChain>; +/// Block hash of the bridged chain. +pub type BridgedBlockHash = HashOf<::BridgedChain>; +/// Hasher of the bridged chain. +pub type _BridgedBlockHasher = HasherOf<::BridgedChain>; +/// Header of the bridged chain. +pub type BridgedHeader = HeaderOf<::BridgedChain>; + #[frame_support::pallet] pub mod pallet { use super::*; use frame_support::pallet_prelude::*; use frame_system::pallet_prelude::*; - /// Header of the bridged chain. - pub(crate) type BridgedHeader = HeaderOf<::BridgedChain>; - #[pallet::config] pub trait Config: frame_system::Config { /// The chain we are bridging to here. @@ -113,6 +124,7 @@ pub mod pallet { justification: Vec, ancestry_proof: T::AncestryProof, ) -> DispatchResultWithPostInfo { + ensure_operational::()?; let _ = ensure_signed(origin)?; ensure!( @@ -120,14 +132,24 @@ pub mod pallet { >::TooManyRequests ); + let (hash, number) = (finality_target.hash(), finality_target.number()); frame_support::debug::trace!("Going to try and finalize header {:?}", finality_target); - let authority_set = T::HeaderChain::authority_set(); + let best_finalized = >::get(>::get()).expect( + "In order to reach this point the bridge must have been initialized. Afterwards, + every time `BestFinalized` is updated `ImportedHeaders` is also updated. Therefore + `ImportedHeaders` must contain an entry for `BestFinalized`.", + ); + + // We do a quick check here to ensure that our header chain is making progress and isn't + // "travelling back in time" (which could be indicative of something bad, e.g a hard-fork). + ensure!(best_finalized.number() < number, >::OldHeader); + + let authority_set = >::get(); let voter_set = VoterSet::new(authority_set.authorities).ok_or(>::InvalidAuthoritySet)?; let set_id = authority_set.set_id; - let (hash, number) = (finality_target.hash(), *finality_target.number()); - verify_justification::>((hash, number), set_id, voter_set, &justification).map_err( + verify_justification::>((hash, *number), set_id, voter_set, &justification).map_err( |e| { frame_support::debug::error!("Received invalid justification for {:?}: {:?}", finality_target, e); >::InvalidJustification @@ -142,11 +164,85 @@ pub mod pallet { >::InvalidAncestryProof ); - let _ = T::HeaderChain::append_header(finality_target)?; - frame_support::debug::info!("Succesfully imported finalized header with hash {:?}!", hash); + let _ = T::HeaderChain::append_header(finality_target.clone())?; + import_header::(hash, finality_target)?; >::mutate(|count| *count += 1); + frame_support::debug::info!("Succesfully imported finalized header with hash {:?}!", hash); + + Ok(().into()) + } + + /// Bootstrap the bridge pallet with an initial header and authority set from which to sync. + /// + /// The initial configuration provided does not need to be the genesis header of the bridged + /// chain, it can be any arbirary header. You can also provide the next scheduled set change + /// if it is already know. + /// + /// This function is only allowed to be called from a trusted origin and writes to storage + /// with practically no checks in terms of the validity of the data. It is important that + /// you ensure that valid data is being passed in. + #[pallet::weight((T::DbWeight::get().reads_writes(2, 5), DispatchClass::Operational))] + pub fn initialize( + origin: OriginFor, + init_data: super::InitializationData>, + ) -> DispatchResultWithPostInfo { + ensure_owner_or_root::(origin)?; + + let init_allowed = !>::exists(); + ensure!(init_allowed, >::AlreadyInitialized); + initialize_bridge::(init_data.clone()); + + frame_support::debug::info!( + "Pallet has been initialized with the following parameters: {:?}", + init_data + ); + + Ok(().into()) + } + + /// Change `ModuleOwner`. + /// + /// May only be called either by root, or by `ModuleOwner`. + #[pallet::weight((T::DbWeight::get().reads_writes(1, 1), DispatchClass::Operational))] + pub fn set_owner(origin: OriginFor, new_owner: Option) -> DispatchResultWithPostInfo { + ensure_owner_or_root::(origin)?; + match new_owner { + Some(new_owner) => { + ModuleOwner::::put(&new_owner); + frame_support::debug::info!("Setting pallet Owner to: {:?}", new_owner); + } + None => { + ModuleOwner::::kill(); + frame_support::debug::info!("Removed Owner of pallet."); + } + } + + Ok(().into()) + } + + /// Halt all pallet operations. Operations may be resumed using `resume_operations` call. + /// + /// May only be called either by root, or by `ModuleOwner`. + #[pallet::weight((T::DbWeight::get().reads_writes(1, 1), DispatchClass::Operational))] + pub fn halt_operations(origin: OriginFor) -> DispatchResultWithPostInfo { + ensure_owner_or_root::(origin)?; + >::put(true); + frame_support::debug::warn!("Stopping pallet operations."); + + Ok(().into()) + } + + /// Resume all pallet operations. May be called even if pallet is halted. + /// + /// May only be called either by root, or by `ModuleOwner`. + #[pallet::weight((T::DbWeight::get().reads_writes(1, 1), DispatchClass::Operational))] + pub fn resume_operations(origin: OriginFor) -> DispatchResultWithPostInfo { + ensure_owner_or_root::(origin)?; + >::put(false); + frame_support::debug::info!("Resuming pallet operations."); + Ok(().into()) } } @@ -162,6 +258,68 @@ pub mod pallet { #[pallet::getter(fn request_count)] pub(super) type RequestCount = StorageValue<_, u32, ValueQuery>; + /// Hash of the header used to bootstrap the pallet. + #[pallet::storage] + pub(super) type InitialHash = StorageValue<_, BridgedBlockHash, ValueQuery>; + + /// Hash of the best finalized header. + #[pallet::storage] + pub(super) type BestFinalized = StorageValue<_, BridgedBlockHash, ValueQuery>; + + /// Headers which have been imported into the pallet. + #[pallet::storage] + pub(super) type ImportedHeaders = StorageMap<_, Identity, BridgedBlockHash, BridgedHeader>; + + /// The current GRANDPA Authority set. + #[pallet::storage] + pub(super) type CurrentAuthoritySet = StorageValue<_, bp_header_chain::AuthoritySet, ValueQuery>; + + /// Optional pallet owner. + /// + /// Pallet owner has a right to halt all pallet operations and then resume it. If it is + /// `None`, then there are no direct ways to halt/resume pallet operations, but other + /// runtime methods may still be used to do that (i.e. democracy::referendum to update halt + /// flag directly or call the `halt_operations`). + #[pallet::storage] + pub(super) type ModuleOwner = StorageValue<_, T::AccountId, OptionQuery>; + + /// If true, all pallet transactions are failed immediately. + #[pallet::storage] + pub(super) type IsHalted = StorageValue<_, bool, ValueQuery>; + + #[pallet::genesis_config] + pub struct GenesisConfig { + owner: Option, + init_data: Option>>, + } + + #[cfg(feature = "std")] + impl Default for GenesisConfig { + fn default() -> Self { + Self { + owner: None, + init_data: None, + } + } + } + + #[pallet::genesis_build] + impl GenesisBuild for GenesisConfig { + fn build(&self) { + if let Some(ref owner) = self.owner { + >::put(owner); + } + + if let Some(init_data) = self.init_data.clone() { + initialize_bridge::(init_data); + } else { + // Since the bridge hasn't been initialized we shouldn't allow anyone to perform + // transactions. + >::put(true); + } + } + } + #[pallet::error] pub enum Error { /// The given justification is invalid for the given header. @@ -175,32 +333,199 @@ pub mod pallet { FailedToWriteHeader, /// There are too many requests for the current window to handle. TooManyRequests, + /// The header being imported is older than the best finalized header known to the pallet. + OldHeader, + /// The scheduled authority set change found in the header is unsupported by the pallet. + /// + /// This is the case for non-standard (e.g forced) authority set changes. + UnsupportedScheduledChange, + /// The pallet has already been initialized. + AlreadyInitialized, + /// All pallet operations are halted. + Halted, + } + + /// Import the given header to the pallet's storage. + /// + /// This function will also check if the header schedules and enacts authority set changes, + /// updating the current authority set accordingly. + /// + /// Note: This function assumes that the given header has already been proven to be valid and + /// finalized. Using this assumption it will write them to storage with minimal checks. That + /// means it's of great importance that this function *not* called with any headers whose + /// finality has not been checked, otherwise you risk bricking your bridge. + pub(crate) fn import_header( + hash: BridgedBlockHash, + header: BridgedHeader, + ) -> Result<(), sp_runtime::DispatchError> { + // We don't support forced changes - at that point governance intervention is required. + ensure!( + super::find_forced_change(&header).is_none(), + >::UnsupportedScheduledChange + ); + + if let Some(change) = super::find_scheduled_change(&header) { + // GRANDPA only includes a `delay` for forced changes, so this isn't valid. + ensure!(change.delay == Zero::zero(), >::UnsupportedScheduledChange); + + // TODO [#788]: Stop manually increasing the `set_id` here. + let next_authorities = bp_header_chain::AuthoritySet { + authorities: change.next_authorities, + set_id: >::get().set_id + 1, + }; + + // Since our header schedules a change and we know the delay is 0, it must also enact + // the change. + >::put(next_authorities); + }; + + >::put(hash); + >::insert(hash, header); + + Ok(()) + } + + /// Since this writes to storage with no real checks this should only be used in functions that + /// were called by a trusted origin. + fn initialize_bridge(init_params: super::InitializationData>) { + let super::InitializationData { + header, + authority_list, + set_id, + is_halted, + } = init_params; + + let initial_hash = header.hash(); + >::put(initial_hash); + >::put(initial_hash); + >::insert(initial_hash, header); + + let authority_set = bp_header_chain::AuthoritySet::new(authority_list, set_id); + >::put(authority_set); + + >::put(is_halted); + } + + /// Ensure that the origin is either root, or `ModuleOwner`. + fn ensure_owner_or_root(origin: T::Origin) -> Result<(), BadOrigin> { + match origin.into() { + Ok(RawOrigin::Root) => Ok(()), + Ok(RawOrigin::Signed(ref signer)) if Some(signer) == >::get().as_ref() => Ok(()), + _ => Err(BadOrigin), + } + } + + /// Ensure that the pallet is in operational mode (not halted). + fn ensure_operational() -> Result<(), Error> { + if >::get() { + Err(>::Halted) + } else { + Ok(()) + } } } +impl Pallet { + /// Get the best finalized header the pallet knows of. + /// + /// Returns a dummy header if there is no best header. This can only happen + /// if the pallet has not been initialized yet. + pub fn best_finalized() -> BridgedHeader { + let hash = >::get(); + >::get(hash).unwrap_or_else(|| { + >::new( + Default::default(), + Default::default(), + Default::default(), + Default::default(), + Default::default(), + ) + }) + } + + /// Check if a particular header is known to the bridge pallet. + pub fn is_known_header(hash: BridgedBlockHash) -> bool { + >::contains_key(hash) + } +} + +/// Data required for initializing the bridge pallet. +/// +/// The bridge needs to know where to start its sync from, and this provides that initial context. +#[derive(Default, Encode, Decode, RuntimeDebug, PartialEq, Clone)] +#[cfg_attr(feature = "std", derive(Serialize, Deserialize))] +pub struct InitializationData { + /// The header from which we should start syncing. + pub header: H, + /// The initial authorities of the pallet. + pub authority_list: sp_finality_grandpa::AuthorityList, + /// The ID of the initial authority set. + pub set_id: sp_finality_grandpa::SetId, + /// Should the pallet block transaction immediately after initialization. + pub is_halted: bool, +} + +pub(crate) fn find_scheduled_change(header: &H) -> Option> { + use sp_runtime::generic::OpaqueDigestItemId; + + let id = OpaqueDigestItemId::Consensus(&GRANDPA_ENGINE_ID); + + let filter_log = |log: ConsensusLog| match log { + ConsensusLog::ScheduledChange(change) => Some(change), + _ => None, + }; + + // find the first consensus digest with the right ID which converts to + // the right kind of consensus log. + header.digest().convert_first(|l| l.try_to(id).and_then(filter_log)) +} + +/// Checks the given header for a consensus digest signalling a **forced** scheduled change and +/// extracts it. +pub(crate) fn find_forced_change( + header: &H, +) -> Option<(H::Number, sp_finality_grandpa::ScheduledChange)> { + use sp_runtime::generic::OpaqueDigestItemId; + + let id = OpaqueDigestItemId::Consensus(&GRANDPA_ENGINE_ID); + + let filter_log = |log: ConsensusLog| match log { + ConsensusLog::ForcedChange(delay, change) => Some((delay, change)), + _ => None, + }; + + // find the first consensus digest with the right ID which converts to + // the right kind of consensus log. + header.digest().convert_first(|l| l.try_to(id).and_then(filter_log)) +} + #[cfg(test)] mod tests { use super::*; - use crate::mock::{run_test, test_header, Origin, TestRuntime}; - use bp_test_utils::{authority_list, make_justification_for_header}; + use crate::mock::{run_test, test_header, Origin, TestHash, TestHeader, TestNumber, TestRuntime}; + use bp_test_utils::{alice, authority_list, bob, make_justification_for_header}; use codec::Encode; - use frame_support::{assert_err, assert_ok}; + use frame_support::weights::PostDispatchInfo; + use frame_support::{assert_err, assert_noop, assert_ok}; + use sp_runtime::{Digest, DigestItem, DispatchError}; fn initialize_substrate_bridge() { + assert_ok!(init_with_origin(Origin::root())); + } + + fn init_with_origin( + origin: Origin, + ) -> Result, sp_runtime::DispatchErrorWithPostInfo> { let genesis = test_header(0); - let init_data = pallet_substrate_bridge::InitializationData { + let init_data = InitializationData { header: genesis, authority_list: authority_list(), set_id: 1, - scheduled_change: None, is_halted: false, }; - assert_ok!(pallet_substrate_bridge::Module::::initialize( - Origin::root(), - init_data - )); + Module::::initialize(origin, init_data.clone()).map(|_| init_data) } fn submit_finality_proof(child: u8, header: u8) -> frame_support::dispatch::DispatchResultWithPostInfo { @@ -223,6 +548,146 @@ mod tests { let _ = Module::::on_initialize(current_number); } + fn change_log(delay: u64) -> Digest { + let consensus_log = ConsensusLog::::ScheduledChange(sp_finality_grandpa::ScheduledChange { + next_authorities: vec![(alice(), 1), (bob(), 1)], + delay, + }); + + Digest:: { + logs: vec![DigestItem::Consensus(GRANDPA_ENGINE_ID, consensus_log.encode())], + } + } + + fn forced_change_log(delay: u64) -> Digest { + let consensus_log = ConsensusLog::::ForcedChange( + delay, + sp_finality_grandpa::ScheduledChange { + next_authorities: vec![(alice(), 1), (bob(), 1)], + delay, + }, + ); + + Digest:: { + logs: vec![DigestItem::Consensus(GRANDPA_ENGINE_ID, consensus_log.encode())], + } + } + + #[test] + fn init_root_or_owner_origin_can_initialize_pallet() { + run_test(|| { + assert_noop!(init_with_origin(Origin::signed(1)), DispatchError::BadOrigin); + assert_ok!(init_with_origin(Origin::root())); + + // Reset storage so we can initialize the pallet again + BestFinalized::::kill(); + ModuleOwner::::put(2); + assert_ok!(init_with_origin(Origin::signed(2))); + }) + } + + #[test] + fn init_storage_entries_are_correctly_initialized() { + run_test(|| { + assert_eq!( + BestFinalized::::get(), + BridgedBlockHash::::default() + ); + assert_eq!(Module::::best_finalized(), test_header(0)); + + let init_data = init_with_origin(Origin::root()).unwrap(); + + assert!(>::contains_key(init_data.header.hash())); + assert_eq!(BestFinalized::::get(), init_data.header.hash()); + assert_eq!( + CurrentAuthoritySet::::get().authorities, + init_data.authority_list + ); + assert_eq!(IsHalted::::get(), false); + }) + } + + #[test] + fn init_can_only_initialize_pallet_once() { + run_test(|| { + initialize_substrate_bridge(); + assert_noop!( + init_with_origin(Origin::root()), + >::AlreadyInitialized + ); + }) + } + + #[test] + fn pallet_owner_may_change_owner() { + run_test(|| { + ModuleOwner::::put(2); + + assert_ok!(Module::::set_owner(Origin::root(), Some(1))); + assert_noop!( + Module::::halt_operations(Origin::signed(2)), + DispatchError::BadOrigin, + ); + assert_ok!(Module::::halt_operations(Origin::root())); + + assert_ok!(Module::::set_owner(Origin::signed(1), None)); + assert_noop!( + Module::::resume_operations(Origin::signed(1)), + DispatchError::BadOrigin, + ); + assert_noop!( + Module::::resume_operations(Origin::signed(2)), + DispatchError::BadOrigin, + ); + assert_ok!(Module::::resume_operations(Origin::root())); + }); + } + + #[test] + fn pallet_may_be_halted_by_root() { + run_test(|| { + assert_ok!(Module::::halt_operations(Origin::root())); + assert_ok!(Module::::resume_operations(Origin::root())); + }); + } + + #[test] + fn pallet_may_be_halted_by_owner() { + run_test(|| { + ModuleOwner::::put(2); + + assert_ok!(Module::::halt_operations(Origin::signed(2))); + assert_ok!(Module::::resume_operations(Origin::signed(2))); + + assert_noop!( + Module::::halt_operations(Origin::signed(1)), + DispatchError::BadOrigin, + ); + assert_noop!( + Module::::resume_operations(Origin::signed(1)), + DispatchError::BadOrigin, + ); + + assert_ok!(Module::::halt_operations(Origin::signed(2))); + assert_noop!( + Module::::resume_operations(Origin::signed(1)), + DispatchError::BadOrigin, + ); + }); + } + + #[test] + fn pallet_rejects_transactions_if_halted() { + run_test(|| { + >::put(true); + + assert_noop!( + Module::::submit_finality_proof(Origin::signed(1), test_header(1), vec![], vec![]), + Error::::Halted, + ); + }) + } + #[test] fn succesfully_imports_header_with_valid_finality_and_ancestry_proofs() { run_test(|| { @@ -231,12 +696,8 @@ mod tests { assert_ok!(submit_finality_proof(1, 2)); let header = test_header(2); - assert_eq!( - pallet_substrate_bridge::Module::::best_headers(), - vec![(*header.number(), header.hash())] - ); - - assert_eq!(pallet_substrate_bridge::Module::::best_finalized(), header); + assert_eq!(>::get(), header.hash()); + assert!(>::contains_key(header.hash())); }) } @@ -309,18 +770,14 @@ mod tests { let genesis = test_header(0); let invalid_authority_list = vec![(alice(), u64::MAX), (bob(), u64::MAX)]; - let init_data = pallet_substrate_bridge::InitializationData { + let init_data = InitializationData { header: genesis, authority_list: invalid_authority_list, set_id: 1, - scheduled_change: None, is_halted: false, }; - assert_ok!(pallet_substrate_bridge::Module::::initialize( - Origin::root(), - init_data - )); + assert_ok!(Module::::initialize(Origin::root(), init_data)); let header = test_header(1); let justification = [1u8; 32].encode(); @@ -334,7 +791,81 @@ mod tests { } #[test] - fn disallows_imports_once_limit_is_hit_in_single_block() { + fn importing_header_ensures_that_chain_is_extended() { + run_test(|| { + initialize_substrate_bridge(); + + assert_ok!(submit_finality_proof(5, 6)); + assert_err!(submit_finality_proof(3, 4), Error::::OldHeader); + assert_ok!(submit_finality_proof(7, 8)); + }) + } + + #[test] + fn importing_header_enacts_new_authority_set() { + run_test(|| { + initialize_substrate_bridge(); + + let next_set_id = 2; + let next_authorities = vec![(alice(), 1), (bob(), 1)]; + + // Need to update the header digest to indicate that our header signals an authority set + // change. The change will be enacted when we import our header. + let mut header = test_header(2); + header.digest = change_log(0); + + // Let's import our test header + assert_ok!(pallet::import_header::(header.hash(), header.clone())); + + // Make sure that our header is the best finalized + assert_eq!(>::get(), header.hash()); + assert!(>::contains_key(header.hash())); + + // Make sure that the authority set actually changed upon importing our header + assert_eq!( + >::get(), + bp_header_chain::AuthoritySet::new(next_authorities, next_set_id), + ); + }) + } + + #[test] + fn importing_header_rejects_header_with_scheduled_change_delay() { + run_test(|| { + initialize_substrate_bridge(); + + // Need to update the header digest to indicate that our header signals an authority set + // change. However, the change doesn't happen until the next block. + let mut header = test_header(2); + header.digest = change_log(1); + + // Should not be allowed to import this header + assert_err!( + pallet::import_header::(header.hash(), header), + >::UnsupportedScheduledChange + ); + }) + } + + #[test] + fn importing_header_rejects_header_with_forced_changes() { + run_test(|| { + initialize_substrate_bridge(); + + // Need to update the header digest to indicate that it signals a forced authority set + // change. + let mut header = test_header(2); + header.digest = forced_change_log(0); + + // Should not be allowed to import this header + assert_err!( + pallet::import_header::(header.hash(), header), + >::UnsupportedScheduledChange + ); + }) + } + #[test] + fn rate_limiter_disallows_imports_once_limit_is_hit_in_single_block() { run_test(|| { initialize_substrate_bridge(); assert_ok!(submit_finality_proof(1, 2)); @@ -344,7 +875,7 @@ mod tests { } #[test] - fn invalid_requests_do_not_count_towards_request_count() { + fn rate_limiter_invalid_requests_do_not_count_towards_request_count() { run_test(|| { let submit_invalid_request = || { let child = test_header(1); @@ -376,7 +907,7 @@ mod tests { } #[test] - fn allows_request_after_new_block_has_started() { + fn rate_limiter_allows_request_after_new_block_has_started() { run_test(|| { initialize_substrate_bridge(); assert_ok!(submit_finality_proof(1, 2)); @@ -388,7 +919,7 @@ mod tests { } #[test] - fn disallows_imports_once_limit_is_hit_across_different_blocks() { + fn rate_limiter_disallows_imports_once_limit_is_hit_across_different_blocks() { run_test(|| { initialize_substrate_bridge(); assert_ok!(submit_finality_proof(1, 2)); @@ -401,7 +932,7 @@ mod tests { } #[test] - fn allows_max_requests_after_long_time_with_no_activity() { + fn rate_limiter_allows_max_requests_after_long_time_with_no_activity() { run_test(|| { initialize_substrate_bridge(); assert_ok!(submit_finality_proof(1, 2)); diff --git a/bridges/modules/finality-verifier/src/mock.rs b/bridges/modules/finality-verifier/src/mock.rs index 26890d9d23a1..dc2fc27394f8 100644 --- a/bridges/modules/finality-verifier/src/mock.rs +++ b/bridges/modules/finality-verifier/src/mock.rs @@ -17,8 +17,7 @@ // From construct_runtime macro #![allow(clippy::from_over_into)] -use crate::pallet::{BridgedHeader, Config}; -use bp_runtime::{BlockNumberOf, Chain}; +use bp_runtime::Chain; use frame_support::{construct_runtime, parameter_types, weights::Weight}; use sp_runtime::{ testing::{Header, H256}, @@ -27,8 +26,9 @@ use sp_runtime::{ }; pub type AccountId = u64; -pub type TestHeader = BridgedHeader; -pub type TestNumber = BlockNumberOf<::BridgedChain>; +pub type TestHeader = crate::BridgedHeader; +pub type TestNumber = crate::BridgedBlockNumber; +pub type TestHash = crate::BridgedBlockHash; type Block = frame_system::mocking::MockBlock; type UncheckedExtrinsic = frame_system::mocking::MockUncheckedExtrinsic; diff --git a/bridges/primitives/millau/src/lib.rs b/bridges/primitives/millau/src/lib.rs index 84096d116ef5..23acef4701a6 100644 --- a/bridges/primitives/millau/src/lib.rs +++ b/bridges/primitives/millau/src/lib.rs @@ -238,6 +238,11 @@ pub const IS_KNOWN_MILLAU_BLOCK_METHOD: &str = "MillauHeaderApi_is_known_block"; /// Name of the `MillauHeaderApi::incomplete_headers` runtime method. pub const INCOMPLETE_MILLAU_HEADERS_METHOD: &str = "MillauHeaderApi_incomplete_headers"; +/// Name of the `RialtoFinalityApi::best_finalized` runtime method. +pub const BEST_FINALIZED_MILLAU_HEADER_METHOD: &str = "MillauFinalityApi_best_finalized"; +/// Name of the `RialtoFinalityApi::is_known_header` runtime method. +pub const IS_KNOW_MILLAU_HEADER_METHOD: &str = "MillauFinalityApi_is_known_header"; + /// Name of the `ToMillauOutboundLaneApi::estimate_message_delivery_and_dispatch_fee` runtime method. pub const TO_MILLAU_ESTIMATE_MESSAGE_FEE_METHOD: &str = "ToMillauOutboundLaneApi_estimate_message_delivery_and_dispatch_fee"; @@ -258,7 +263,7 @@ pub const FROM_MILLAU_UNREWARDED_RELAYERS_STATE: &str = "FromMillauInboundLaneAp sp_api::decl_runtime_apis! { /// API for querying information about Millau headers from the Bridge Pallet instance. /// - /// This API is implemented by runtimes that are bridging with Millau chain, not the + /// This API is implemented by runtimes that are bridging with the Millau chain, not the /// Millau runtime itself. pub trait MillauHeaderApi { /// Returns number and hash of the best blocks known to the bridge module. @@ -281,6 +286,17 @@ sp_api::decl_runtime_apis! { fn is_finalized_block(hash: Hash) -> bool; } + /// API for querying information about the finalized Millau headers. + /// + /// This API is implemented by runtimes that are bridging with the Millau chain, not the + /// Millau runtime itself. + pub trait MillauFinalityApi { + /// Returns number and hash of the best finalized header known to the bridge module. + fn best_finalized() -> (BlockNumber, Hash); + /// Returns true if the header is known to the runtime. + fn is_known_header(hash: Hash) -> bool; + } + /// Outbound message lane API for messages that are sent to Millau chain. /// /// This API is implemented by runtimes that are sending messages to Millau chain, not the diff --git a/bridges/primitives/rialto/src/lib.rs b/bridges/primitives/rialto/src/lib.rs index 706e2f27854d..856dd8b7fbfe 100644 --- a/bridges/primitives/rialto/src/lib.rs +++ b/bridges/primitives/rialto/src/lib.rs @@ -199,6 +199,11 @@ pub const IS_KNOWN_RIALTO_BLOCK_METHOD: &str = "RialtoHeaderApi_is_known_block"; /// Name of the `RialtoHeaderApi::incomplete_headers` runtime method. pub const INCOMPLETE_RIALTO_HEADERS_METHOD: &str = "RialtoHeaderApi_incomplete_headers"; +/// Name of the `RialtoFinalityApi::best_finalized` runtime method. +pub const BEST_FINALIZED_RIALTO_HEADER_METHOD: &str = "RialtoFinalityApi_best_finalized"; +/// Name of the `RialtoFinalityApi::is_known_header` runtime method. +pub const IS_KNOW_RIALTO_HEADER_METHOD: &str = "RialtoFinalityApi_is_known_header"; + /// Name of the `ToRialtoOutboundLaneApi::estimate_message_delivery_and_dispatch_fee` runtime method. pub const TO_RIALTO_ESTIMATE_MESSAGE_FEE_METHOD: &str = "ToRialtoOutboundLaneApi_estimate_message_delivery_and_dispatch_fee"; @@ -219,7 +224,7 @@ pub const FROM_RIALTO_UNREWARDED_RELAYERS_STATE: &str = "FromRialtoInboundLaneAp sp_api::decl_runtime_apis! { /// API for querying information about Rialto headers from the Bridge Pallet instance. /// - /// This API is implemented by runtimes that are bridging with Rialto chain, not the + /// This API is implemented by runtimes that are bridging with the Rialto chain, not the /// Rialto runtime itself. pub trait RialtoHeaderApi { /// Returns number and hash of the best blocks known to the bridge module. @@ -242,6 +247,17 @@ sp_api::decl_runtime_apis! { fn is_finalized_block(hash: Hash) -> bool; } + /// API for querying information about the finalized Rialto headers. + /// + /// This API is implemented by runtimes that are bridging with the Rialto chain, not the + /// Millau runtime itself. + pub trait RialtoFinalityApi { + /// Returns number and hash of the best finalized header known to the bridge module. + fn best_finalized() -> (BlockNumber, Hash); + /// Returns true if the header is known to the runtime. + fn is_known_header(hash: Hash) -> bool; + } + /// Outbound message lane API for messages that are sent to Rialto chain. /// /// This API is implemented by runtimes that are sending messages to Rialto chain, not the