diff --git a/bridges/bin/millau/runtime/src/lib.rs b/bridges/bin/millau/runtime/src/lib.rs index 743bab6d13..3b4b06fbc1 100644 --- a/bridges/bin/millau/runtime/src/lib.rs +++ b/bridges/bin/millau/runtime/src/lib.rs @@ -520,6 +520,7 @@ pub type WitRialtoParachainsInstance = (); impl pallet_bridge_parachains::Config for Runtime { type BridgesGrandpaPalletInstance = RialtoGrandpaInstance; type ParasPalletName = RialtoParasPalletName; + type TrackedParachains = frame_support::traits::Everything; type HeadsToKeep = HeadersToKeep; } diff --git a/bridges/modules/grandpa/src/lib.rs b/bridges/modules/grandpa/src/lib.rs index 4184464231..8fa0ed89cc 100644 --- a/bridges/modules/grandpa/src/lib.rs +++ b/bridges/modules/grandpa/src/lib.rs @@ -93,6 +93,8 @@ pub mod pallet { /// The setting is there to prevent growing the on-chain state indefinitely. Note /// the setting does not relate to block numbers - we will simply keep as much items /// in the storage, so it doesn't guarantee any fixed timeframe for finality headers. + /// + /// Incautious change of this constant may lead to orphan entries in the runtime storage. #[pallet::constant] type HeadersToKeep: Get; diff --git a/bridges/modules/messages/src/mock.rs b/bridges/modules/messages/src/mock.rs index a4ecb0467b..33c6eae6eb 100644 --- a/bridges/modules/messages/src/mock.rs +++ b/bridges/modules/messages/src/mock.rs @@ -149,7 +149,7 @@ parameter_types! { pub const MaxUnrewardedRelayerEntriesAtInboundLane: u64 = 16; pub const MaxUnconfirmedMessagesAtInboundLane: u64 = 32; pub storage TokenConversionRate: FixedU128 = 1.into(); - pub const TestBridgedChainId: bp_runtime::ChainId = *b"test"; + pub const TestBridgedChainId: bp_runtime::ChainId = *b"test"; } #[derive(Debug, Clone, Encode, Decode, PartialEq, Eq, TypeInfo)] diff --git a/bridges/modules/parachains/src/lib.rs b/bridges/modules/parachains/src/lib.rs index d7a74656c0..50bfc45676 100644 --- a/bridges/modules/parachains/src/lib.rs +++ b/bridges/modules/parachains/src/lib.rs @@ -25,8 +25,9 @@ use bp_parachains::parachain_head_storage_key_at_source; use bp_polkadot_core::parachains::{ParaHash, ParaHasher, ParaHead, ParaHeadsProof, ParaId}; +use bp_runtime::StorageProofError; use codec::{Decode, Encode}; -use frame_support::RuntimeDebug; +use frame_support::{traits::Contains, RuntimeDebug}; use scale_info::TypeInfo; use sp_runtime::traits::Header as HeaderT; use sp_std::vec::Vec; @@ -90,11 +91,20 @@ pub mod pallet { #[pallet::constant] type ParasPalletName: Get<&'static str>; + /// Set of parachains that are tracked by this pallet. + /// + /// The set may be extended easily, without requiring any runtime upgrades. Removing tracked + /// parachain requires special handling - pruning existing heads and cleaning related data + /// structures. + type TrackedParachains: Contains; + /// Maximal number of single parachain heads to keep in the storage. /// /// The setting is there to prevent growing the on-chain state indefinitely. Note /// the setting does not relate to parachain block numbers - we will simply keep as much /// items in the storage, so it doesn't guarantee any fixed timeframe for heads. + /// + /// Incautious change of this constant may lead to orphan entries in the runtime storage. #[pallet::constant] type HeadsToKeep: Get; } @@ -156,17 +166,40 @@ pub mod pallet { sp_trie::StorageProof::new(parachain_heads_proof), move |storage| { for parachain in parachains { - // TODO: https://github.com/paritytech/parity-bridges-common/issues/1393 + // if we're not tracking this parachain, we'll just ignore its head proof here + if !T::TrackedParachains::contains(¶chain) { + log::trace!( + target: "runtime::bridge-parachains", + "The head of parachain {:?} has been provided, but it is not tracked by the pallet", + parachain, + ); + continue; + } + let parachain_head = match Pallet::::read_parachain_head(&storage, parachain) { - Some(parachain_head) => parachain_head, - None => { + Ok(Some(parachain_head)) => parachain_head, + Ok(None) => { log::trace!( target: "runtime::bridge-parachains", - "The head of parachain {:?} has been declared, but is missing from the proof", + "The head of parachain {:?} is None. {}", parachain, + if BestParaHeads::::contains_key(¶chain) { + "Looks like it is not yet registered at the source relay chain" + } else { + "Looks like it has been deregistered from the source relay chain" + }, ); continue; - } + }, + Err(e) => { + log::trace!( + target: "runtime::bridge-parachains", + "The read of head of parachain {:?} has failed: {:?}", + parachain, + e, + ); + continue; + }, }; let _: Result<_, ()> = BestParaHeads::::try_mutate(parachain, |stored_best_head| { @@ -183,14 +216,6 @@ pub mod pallet { ) .map_err(|_| Error::::InvalidStorageProof)?; - // TODO: there may be parachains we are not interested in - so we only need to accept - // intersection of `parachains-interesting-to-us` and `parachains` - // https://github.com/paritytech/parity-bridges-common/issues/1392 - - // TODO: if some parachain is no more interesting to us, we should start pruning its - // heads - // https://github.com/paritytech/parity-bridges-common/issues/1392 - Ok(()) } } @@ -232,12 +257,10 @@ pub mod pallet { fn read_parachain_head( storage: &bp_runtime::StorageProofChecker, parachain: ParaId, - ) -> Option { + ) -> Result, StorageProofError> { let parachain_head_key = parachain_head_storage_key_at_source(T::ParasPalletName::get(), parachain); - let parachain_head = storage.read_value(parachain_head_key.0.as_ref()).ok()??; - let parachain_head = ParaHead::decode(&mut ¶chain_head[..]).ok()?; - Some(parachain_head) + storage.read_and_decode_value(parachain_head_key.0.as_ref()) } /// Try to update parachain head. @@ -327,7 +350,9 @@ pub mod pallet { #[cfg(test)] mod tests { use super::*; - use crate::mock::{run_test, test_relay_header, Origin, TestRuntime, PARAS_PALLET_NAME}; + use crate::mock::{ + run_test, test_relay_header, Origin, TestRuntime, PARAS_PALLET_NAME, UNTRACKED_PARACHAIN_ID, + }; use bp_test_utils::{authority_list, make_default_justification}; use frame_support::{assert_noop, assert_ok, traits::OnInitialize}; @@ -510,6 +535,43 @@ mod tests { }); } + #[test] + fn ignores_untracked_parachain() { + let (state_root, proof) = prepare_parachain_heads_proof(vec![ + (1, head_data(1, 5)), + (UNTRACKED_PARACHAIN_ID, head_data(1, 5)), + (2, head_data(1, 5)), + ]); + run_test(|| { + // start with relay block #0 and try to import head#5 of parachain#1 and untracked + // parachain + initialize(state_root); + assert_ok!(Pallet::::submit_parachain_heads( + Origin::signed(1), + test_relay_header(0, state_root).hash(), + vec![ParaId(1), ParaId(UNTRACKED_PARACHAIN_ID), ParaId(2)], + proof, + )); + assert_eq!( + BestParaHeads::::get(ParaId(1)), + Some(BestParaHead { + at_relay_block_number: 0, + head_hash: head_data(1, 5).hash(), + next_imported_hash_position: 1, + }) + ); + assert_eq!(BestParaHeads::::get(ParaId(UNTRACKED_PARACHAIN_ID)), None,); + assert_eq!( + BestParaHeads::::get(ParaId(2)), + Some(BestParaHead { + at_relay_block_number: 0, + head_hash: head_data(1, 5).hash(), + next_imported_hash_position: 1, + }) + ); + }); + } + #[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))]); diff --git a/bridges/modules/parachains/src/mock.rs b/bridges/modules/parachains/src/mock.rs index 506783cdb4..c314fe7ab1 100644 --- a/bridges/modules/parachains/src/mock.rs +++ b/bridges/modules/parachains/src/mock.rs @@ -14,8 +14,9 @@ // You should have received a copy of the GNU General Public License // along with Parity Bridges Common. If not, see . +use bp_polkadot_core::parachains::ParaId; use bp_runtime::Chain; -use frame_support::{construct_runtime, parameter_types, weights::Weight}; +use frame_support::{construct_runtime, parameter_types, traits::IsInVec, weights::Weight}; use sp_runtime::{ testing::{Header, H256}, traits::{BlakeTwo256, Header as HeaderT, IdentityLookup}, @@ -34,6 +35,7 @@ type Block = frame_system::mocking::MockBlock; type UncheckedExtrinsic = frame_system::mocking::MockUncheckedExtrinsic; pub const PARAS_PALLET_NAME: &str = "Paras"; +pub const UNTRACKED_PARACHAIN_ID: u32 = 10; construct_runtime! { pub enum TestRuntime where @@ -106,11 +108,13 @@ impl pallet_bridge_grandpa::Config for TestRun parameter_types! { pub const HeadsToKeep: u32 = 4; pub const ParasPalletName: &'static str = PARAS_PALLET_NAME; + pub GetTenFirstParachains: Vec = (0..10).map(ParaId).collect(); } impl pallet_bridge_parachains::Config for TestRuntime { type BridgesGrandpaPalletInstance = pallet_bridge_grandpa::Instance1; type ParasPalletName = ParasPalletName; + type TrackedParachains = IsInVec; type HeadsToKeep = HeadsToKeep; } diff --git a/bridges/primitives/runtime/src/storage_proof.rs b/bridges/primitives/runtime/src/storage_proof.rs index 30976ff7fb..24b7dac97e 100644 --- a/bridges/primitives/runtime/src/storage_proof.rs +++ b/bridges/primitives/runtime/src/storage_proof.rs @@ -16,6 +16,7 @@ //! Logic for checking Substrate storage proofs. +use codec::Decode; use hash_db::{HashDB, Hasher, EMPTY_PREFIX}; use sp_runtime::RuntimeDebug; use sp_std::vec::Vec; @@ -50,18 +51,29 @@ where } /// Reads a value from the available subset of storage. If the value cannot be read due to an - /// incomplete or otherwise invalid proof, this returns an error. + /// incomplete or otherwise invalid proof, this function returns an error. pub fn read_value(&self, key: &[u8]) -> Result>, Error> { // LayoutV1 or LayoutV0 is identical for proof that only read values. read_trie_value::, _>(&self.db, &self.root, key) .map_err(|_| Error::StorageValueUnavailable) } + + /// Reads and decodes a value from the available subset of storage. If the value cannot be read + /// due to an incomplete or otherwise invalid proof, this function returns an error. If value is + /// read, but decoding fails, this function returns an error. + pub fn read_and_decode_value(&self, key: &[u8]) -> Result, Error> { + self.read_value(key).and_then(|v| { + v.map(|v| T::decode(&mut &v[..]).map_err(Error::StorageValueDecodeFailed)) + .transpose() + }) + } } #[derive(Eq, RuntimeDebug, PartialEq)] pub enum Error { StorageRootMismatch, StorageValueUnavailable, + StorageValueDecodeFailed(codec::Error), } /// Return valid storage proof and state root. @@ -69,6 +81,7 @@ pub enum Error { /// NOTE: This should only be used for **testing**. #[cfg(feature = "std")] pub fn craft_valid_storage_proof() -> (sp_core::H256, StorageProof) { + use codec::Encode; use sp_state_machine::{backend::Backend, prove_read, InMemoryBackend}; let state_version = sp_runtime::StateVersion::default(); @@ -79,6 +92,7 @@ pub fn craft_valid_storage_proof() -> (sp_core::H256, StorageProof) { (None, vec![(b"key1".to_vec(), Some(b"value1".to_vec()))]), (None, vec![(b"key2".to_vec(), Some(b"value2".to_vec()))]), (None, vec![(b"key3".to_vec(), Some(b"value3".to_vec()))]), + (None, vec![(b"key4".to_vec(), Some((42u64, 42u32, 42u16, 42u8).encode()))]), // Value is too big to fit in a branch node (None, vec![(b"key11".to_vec(), Some(vec![0u8; 32]))]), ], @@ -86,7 +100,7 @@ pub fn craft_valid_storage_proof() -> (sp_core::H256, StorageProof) { )); let root = backend.storage_root(std::iter::empty(), state_version).0; let proof = StorageProof::new( - prove_read(backend, &[&b"key1"[..], &b"key2"[..], &b"key22"[..]]) + prove_read(backend, &[&b"key1"[..], &b"key2"[..], &b"key4"[..], &b"key22"[..]]) .unwrap() .iter_nodes(), ); @@ -97,6 +111,7 @@ pub fn craft_valid_storage_proof() -> (sp_core::H256, StorageProof) { #[cfg(test)] pub mod tests { use super::*; + use codec::Encode; #[test] fn storage_proof_check() { @@ -107,8 +122,14 @@ pub mod tests { >::new(root, proof.clone()).unwrap(); assert_eq!(checker.read_value(b"key1"), Ok(Some(b"value1".to_vec()))); assert_eq!(checker.read_value(b"key2"), Ok(Some(b"value2".to_vec()))); + assert_eq!(checker.read_value(b"key4"), Ok(Some((42u64, 42u32, 42u16, 42u8).encode()))); assert_eq!(checker.read_value(b"key11111"), Err(Error::StorageValueUnavailable)); assert_eq!(checker.read_value(b"key22"), Ok(None)); + assert_eq!(checker.read_and_decode_value(b"key4"), Ok(Some((42u64, 42u32, 42u16, 42u8))),); + assert!(matches!( + checker.read_and_decode_value::<[u8; 64]>(b"key4"), + Err(Error::StorageValueDecodeFailed(_)), + )); // checking proof against invalid commitment fails assert_eq!(