From 91ed4a500e920d8252a067341b0b120bcc3e7b85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Mon, 9 Jan 2023 11:17:57 +0100 Subject: [PATCH 1/5] Refactor `validate_block` This pull request changes the `validate_block` implementation. One of the key changes are that we free data structures as early as possible. The memory while validating the block is scarce and we need to give as much as possible to the actual execution of the block. Besides that the pr moves the validation of the `validation_data` into the `validate_block` implementation completely instead of using this machinery with putting the data into some global variable that would then be read while executing the block. There are also some new docs to explain the internals of `validate_block`. --- .../parachain-system/proc-macro/src/lib.rs | 6 +- pallets/parachain-system/src/lib.rs | 30 ---- .../src/validate_block/implementation.rs | 167 +++++++++++++----- .../src/validate_block/mod.rs | 24 +-- 4 files changed, 126 insertions(+), 101 deletions(-) diff --git a/pallets/parachain-system/proc-macro/src/lib.rs b/pallets/parachain-system/proc-macro/src/lib.rs index 855d7fed484..7a74199b075 100644 --- a/pallets/parachain-system/proc-macro/src/lib.rs +++ b/pallets/parachain-system/proc-macro/src/lib.rs @@ -114,12 +114,16 @@ pub fn register_validate_block(input: proc_macro::TokenStream) -> proc_macro::To use super::*; #[no_mangle] - unsafe fn validate_block(arguments: *const u8, arguments_len: usize) -> u64 { + unsafe fn validate_block(arguments: *mut u8, arguments_len: usize) -> u64 { let params = #crate_::validate_block::polkadot_parachain::load_params( arguments, arguments_len, ); + // Memory is scarce resource, so let's free the `arguments` as we don't + // need them anymore. + #crate_::validate_block::sp_io::allocator::free(arguments); + let res = #crate_::validate_block::implementation::validate_block::< <#runtime as #crate_::validate_block::GetRuntimeBlockType>::RuntimeBlock, #block_executor, diff --git a/pallets/parachain-system/src/lib.rs b/pallets/parachain-system/src/lib.rs index 743c0c25372..be2a1426691 100644 --- a/pallets/parachain-system/src/lib.rs +++ b/pallets/parachain-system/src/lib.rs @@ -355,8 +355,6 @@ pub mod pallet { horizontal_messages, } = data; - Self::validate_validation_data(&vfp); - // Check that the associated relay chain block number is as expected. T::CheckAssociatedRelayNumber::check_associated_relay_number( vfp.relay_parent_number, @@ -769,34 +767,6 @@ impl GetChannelInfo for Pallet { } impl Pallet { - /// Validate the given [`PersistedValidationData`] against the - /// [`ValidationParams`](polkadot_parachain::primitives::ValidationParams). - /// - /// This check will only be executed when the block is currently being executed in the context - /// of [`validate_block`]. If this is being executed in the context of block building or block - /// import, this is a no-op. - /// - /// # Panics - /// - /// Panics while validating the `PoV` on the relay chain if the [`PersistedValidationData`] - /// passed by the block author was incorrect. - fn validate_validation_data(validation_data: &PersistedValidationData) { - validate_block::with_validation_params(|params| { - assert_eq!( - params.parent_head, validation_data.parent_head, - "Parent head doesn't match" - ); - assert_eq!( - params.relay_parent_number, validation_data.relay_parent_number, - "Relay parent number doesn't match", - ); - assert_eq!( - params.relay_parent_storage_root, validation_data.relay_parent_storage_root, - "Relay parent storage root doesn't match", - ); - }); - } - /// Process all inbound downward messages relayed by the collator. /// /// Checks if the sequence of the messages is valid, dispatches them and communicates the diff --git a/pallets/parachain-system/src/validate_block/implementation.rs b/pallets/parachain-system/src/validate_block/implementation.rs index 8c03f012be2..983f58dbedf 100644 --- a/pallets/parachain-system/src/validate_block/implementation.rs +++ b/pallets/parachain-system/src/validate_block/implementation.rs @@ -19,15 +19,21 @@ use frame_support::traits::{ExecuteBlock, ExtrinsicCall, Get, IsSubType}; use sp_runtime::traits::{Block as BlockT, Extrinsic, HashFor, Header as HeaderT}; -use sp_io::KillStorageResult; -use sp_std::prelude::*; +use cumulus_primitives_core::{ + relay_chain::v2::Hash as RHash, ParachainBlockData, PersistedValidationData, +}; +use cumulus_primitives_parachain_inherent::ParachainInherentData; -use polkadot_parachain::primitives::{HeadData, ValidationParams, ValidationResult}; +use polkadot_parachain::primitives::{ + HeadData, RelayChainBlockNumber, ValidationParams, ValidationResult, +}; use codec::{Decode, Encode}; use sp_core::storage::{ChildInfo, StateVersion}; use sp_externalities::{set_and_run_with_externalities, Externalities}; +use sp_io::KillStorageResult; +use sp_std::{mem, prelude::*}; use sp_trie::MemoryDB; type TrieBackend = sp_state_machine::TrieBackend>, HashFor>; @@ -38,7 +44,32 @@ fn with_externalities R, R>(f: F) -> R { sp_externalities::with_externalities(f).expect("Environmental externalities not set.") } -/// Validate a given parachain block on a validator. +/// Validate the given parachain block. +/// +/// This function is doing roughly the following: +/// +/// 1. We decode the [`ParachainBlockData`] from the `block_data` in `params`. +/// +/// 2. We are doing some security checks like checking that the `parent_head` in `params` +/// is the parent of the block we are going to check. We also ensure that the `set_validation_data` +/// inherent is present in the block and that the validation data matches the values in `params`. +/// +/// 3. We construct the sparse in-memory database from the storage proof inside the block data and +/// then ensure that the storage root matches the storage root in the `parent_head`. +/// +/// 4. We replace all the storage related host functions with functions inside the wasm blob. +/// This means instead of calling into the host, we will stay inside the wasm execution. This is +/// very important as the relay chain validator hasn't the state required to verify the block. But +/// we have the in-memory database that contains all the values from the state of the parachain +/// that we require to verify the block. +/// +/// 5. We are going to run `check_inherents`. This is important to check stuff like the timestamp +/// matching the real world time. +/// +/// 6. The last step is to execute the entire block in the machinery we just have setup. Executing +/// the blocks include running all transactions in the block against our in-memory database and +/// ensuring that the final storage root matches the storage root in the header of the block. In the +/// end we return back the [`ValidationResult`] with all the required information for the validator. #[doc(hidden)] pub fn validate_block< B: BlockT, @@ -52,19 +83,30 @@ where B::Extrinsic: ExtrinsicCall, ::Call: IsSubType>, { - let block_data = - cumulus_primitives_core::ParachainBlockData::::decode(&mut ¶ms.block_data.0[..]) - .expect("Invalid parachain block data"); + let block_data = ParachainBlockData::::decode(&mut ¶ms.block_data.0[..]) + .expect("Invalid parachain block data"); let parent_head = B::Header::decode(&mut ¶ms.parent_head.0[..]).expect("Invalid parent head"); + // We decoded the block data and don't need it anymore. So, we release it to free the memory. + mem::drop(params.block_data); + let (header, extrinsics, storage_proof) = block_data.deconstruct(); let head_data = HeadData(header.encode()); let block = B::new(header, extrinsics); - assert!(parent_head.hash() == *block.header().parent_hash(), "Invalid parent hash",); + assert!(parent_head.hash() == *block.header().parent_hash(), "Invalid parent hash"); + + let inherent_data = extract_parachain_inherent_data(&block); + + validate_validation_data( + &inherent_data.validation_data, + params.relay_parent_number, + params.relay_parent_storage_root, + params.parent_head, + ); // Create the db let db = match storage_proof.to_memory_db(Some(parent_head.state_root())) { @@ -74,6 +116,8 @@ where sp_std::mem::drop(storage_proof); + // We use the storage root of the `parent_head` to ensure that it is the correct root. + // This is already being done above while creating the in-memory db, but let's be paranoid!! let backend = sp_state_machine::TrieBackendBuilder::new(db, *parent_head.state_root()).build(); let _guard = ( @@ -115,22 +159,6 @@ where sp_io::offchain_index::host_clear.replace_implementation(host_offchain_index_clear), ); - let inherent_data = block - .extrinsics() - .iter() - // Inherents are at the front of the block and are unsigned. - // - // If `is_signed` is returning `None`, we keep it safe and assume that it is "signed". - // We are searching for unsigned transactions anyway. - .take_while(|e| !e.is_signed().unwrap_or(true)) - .filter_map(|e| e.call().is_sub_type()) - .find_map(|c| match c { - crate::Call::set_validation_data { data: validation_data } => - Some(validation_data.clone()), - _ => None, - }) - .expect("Could not find `set_validation_data` inherent"); - run_with_externalities::(&backend, || { let relay_chain_proof = crate::RelayChainStateProof::new( PSC::SelfParaId::get(), @@ -153,34 +181,75 @@ where }); run_with_externalities::(&backend, || { - super::set_and_run_with_validation_params(params, || { - E::execute_block(block); - - let new_validation_code = crate::NewValidationCode::::get(); - let upward_messages = crate::UpwardMessages::::get(); - let processed_downward_messages = crate::ProcessedDownwardMessages::::get(); - let horizontal_messages = crate::HrmpOutboundMessages::::get(); - let hrmp_watermark = crate::HrmpWatermark::::get(); - - let head_data = - if let Some(custom_head_data) = crate::CustomValidationHeadData::::get() { - HeadData(custom_head_data) - } else { - head_data - }; - - ValidationResult { - head_data, - new_validation_code: new_validation_code.map(Into::into), - upward_messages, - processed_downward_messages, - horizontal_messages, - hrmp_watermark, - } - }) + E::execute_block(block); + + let new_validation_code = crate::NewValidationCode::::get(); + let upward_messages = crate::UpwardMessages::::get(); + let processed_downward_messages = crate::ProcessedDownwardMessages::::get(); + let horizontal_messages = crate::HrmpOutboundMessages::::get(); + let hrmp_watermark = crate::HrmpWatermark::::get(); + + let head_data = + if let Some(custom_head_data) = crate::CustomValidationHeadData::::get() { + HeadData(custom_head_data) + } else { + head_data + }; + + ValidationResult { + head_data, + new_validation_code: new_validation_code.map(Into::into), + upward_messages, + processed_downward_messages, + horizontal_messages, + hrmp_watermark, + } }) } +/// Extract the [`ParachainInherentData`]. +fn extract_parachain_inherent_data( + block: &B, +) -> ParachainInherentData +where + B::Extrinsic: ExtrinsicCall, + ::Call: IsSubType>, +{ + block + .extrinsics() + .iter() + // Inherents are at the front of the block and are unsigned. + // + // If `is_signed` is returning `None`, we keep it safe and assume that it is "signed". + // We are searching for unsigned transactions anyway. + .take_while(|e| !e.is_signed().unwrap_or(true)) + .filter_map(|e| e.call().is_sub_type()) + .find_map(|c| match c { + crate::Call::set_validation_data { data: validation_data } => + Some(validation_data.clone()), + _ => None, + }) + .expect("Could not find `set_validation_data` inherent") +} + +/// Validate the given [`PersistedValidationData`] against the [`ValidationParams`]. +fn validate_validation_data( + validation_data: &PersistedValidationData, + relay_parent_number: RelayChainBlockNumber, + relay_parent_storage_root: RHash, + parent_head: HeadData, +) { + assert_eq!(parent_head, validation_data.parent_head, "Parent head doesn't match"); + assert_eq!( + relay_parent_number, validation_data.relay_parent_number, + "Relay parent number doesn't match", + ); + assert_eq!( + relay_parent_storage_root, validation_data.relay_parent_storage_root, + "Relay parent storage root doesn't match", + ); +} + /// Run the given closure with the externalities set. fn run_with_externalities R>( backend: &TrieBackend, diff --git a/pallets/parachain-system/src/validate_block/mod.rs b/pallets/parachain-system/src/validate_block/mod.rs index 44886efd4fe..ab921ffc611 100644 --- a/pallets/parachain-system/src/validate_block/mod.rs +++ b/pallets/parachain-system/src/validate_block/mod.rs @@ -16,8 +16,6 @@ //! A module that enables a runtime to work as parachain. -use polkadot_parachain::primitives::ValidationParams; - #[cfg(not(feature = "std"))] #[doc(hidden)] pub mod implementation; @@ -29,23 +27,7 @@ mod tests; pub use polkadot_parachain; #[cfg(not(feature = "std"))] #[doc(hidden)] -pub use sp_runtime::traits::GetRuntimeBlockType; - -// Stores the [`ValidationParams`] that are being passed to `validate_block`. -// -// This value will only be set when a parachain validator validates a given `PoV`. -environmental::environmental!(VALIDATION_PARAMS: ValidationParams); - -/// Execute the given closure with the [`ValidationParams`]. -/// -/// Returns `None` if the [`ValidationParams`] are not set, because the code is currently not being -/// executed in the context of `validate_block`. -pub(crate) fn with_validation_params(f: impl FnOnce(&ValidationParams) -> R) -> Option { - VALIDATION_PARAMS::with(|v| f(v)) -} - -/// Set the [`ValidationParams`] for the local context and execute the given closure in this context. +pub use sp_io; #[cfg(not(feature = "std"))] -fn set_and_run_with_validation_params(mut params: ValidationParams, f: impl FnOnce() -> R) -> R { - VALIDATION_PARAMS::using(&mut params, f) -} +#[doc(hidden)] +pub use sp_runtime::traits::GetRuntimeBlockType; From 6b443eb385355202046dbe0e69e37d9bf204d984 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Mon, 9 Jan 2023 16:52:33 +0100 Subject: [PATCH 2/5] No clone wars!! --- .../parachain-system/src/validate_block/implementation.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/pallets/parachain-system/src/validate_block/implementation.rs b/pallets/parachain-system/src/validate_block/implementation.rs index 983f58dbedf..ccb30c848b1 100644 --- a/pallets/parachain-system/src/validate_block/implementation.rs +++ b/pallets/parachain-system/src/validate_block/implementation.rs @@ -210,7 +210,7 @@ where /// Extract the [`ParachainInherentData`]. fn extract_parachain_inherent_data( block: &B, -) -> ParachainInherentData +) -> &ParachainInherentData where B::Extrinsic: ExtrinsicCall, ::Call: IsSubType>, @@ -225,8 +225,7 @@ where .take_while(|e| !e.is_signed().unwrap_or(true)) .filter_map(|e| e.call().is_sub_type()) .find_map(|c| match c { - crate::Call::set_validation_data { data: validation_data } => - Some(validation_data.clone()), + crate::Call::set_validation_data { data: validation_data } => Some(validation_data), _ => None, }) .expect("Could not find `set_validation_data` inherent") From 7eda556daa2c3d4950bbac2eaedc0b8b2ede6b10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Tue, 10 Jan 2023 11:41:54 +0100 Subject: [PATCH 3/5] Integrate more feedback --- .../parachain-system/proc-macro/src/lib.rs | 17 +++++--- .../src/validate_block/implementation.rs | 39 ++++++++++--------- .../src/validate_block/mod.rs | 27 ++++++++++++- .../src/validate_block/tests.rs | 34 +++++++++++++++- 4 files changed, 91 insertions(+), 26 deletions(-) diff --git a/pallets/parachain-system/proc-macro/src/lib.rs b/pallets/parachain-system/proc-macro/src/lib.rs index 7a74199b075..490b9c17c44 100644 --- a/pallets/parachain-system/proc-macro/src/lib.rs +++ b/pallets/parachain-system/proc-macro/src/lib.rs @@ -115,14 +115,19 @@ pub fn register_validate_block(input: proc_macro::TokenStream) -> proc_macro::To #[no_mangle] unsafe fn validate_block(arguments: *mut u8, arguments_len: usize) -> u64 { - let params = #crate_::validate_block::polkadot_parachain::load_params( - arguments, - arguments_len, + // We convert the `arguments` into a boxed slice and then into `Bytes`. + let args = #crate_::validate_block::sp_std::boxed::Box::from_raw( + #crate_::validate_block::sp_std::slice::from_raw_parts_mut( + arguments, + arguments_len, + ) ); + let args = #crate_::validate_block::bytes::Bytes::from(args); - // Memory is scarce resource, so let's free the `arguments` as we don't - // need them anymore. - #crate_::validate_block::sp_io::allocator::free(arguments); + // Then we decode from these bytes the `MemoryOptimizedValidationParams`. + let params = #crate_::validate_block::decode_from_bytes::< + #crate_::validate_block::MemoryOptimizedValidationParams + >(args).expect("Invalid arguments to `validate_block`."); let res = #crate_::validate_block::implementation::validate_block::< <#runtime as #crate_::validate_block::GetRuntimeBlockType>::RuntimeBlock, diff --git a/pallets/parachain-system/src/validate_block/implementation.rs b/pallets/parachain-system/src/validate_block/implementation.rs index ccb30c848b1..f7a967498c5 100644 --- a/pallets/parachain-system/src/validate_block/implementation.rs +++ b/pallets/parachain-system/src/validate_block/implementation.rs @@ -16,9 +16,7 @@ //! The actual implementation of the validate block functionality. -use frame_support::traits::{ExecuteBlock, ExtrinsicCall, Get, IsSubType}; -use sp_runtime::traits::{Block as BlockT, Extrinsic, HashFor, Header as HeaderT}; - +use super::MemoryOptimizedValidationParams; use cumulus_primitives_core::{ relay_chain::v2::Hash as RHash, ParachainBlockData, PersistedValidationData, }; @@ -30,9 +28,11 @@ use polkadot_parachain::primitives::{ use codec::{Decode, Encode}; +use frame_support::traits::{ExecuteBlock, ExtrinsicCall, Get, IsSubType}; use sp_core::storage::{ChildInfo, StateVersion}; use sp_externalities::{set_and_run_with_externalities, Externalities}; use sp_io::KillStorageResult; +use sp_runtime::traits::{Block as BlockT, Extrinsic, HashFor, Header as HeaderT}; use sp_std::{mem, prelude::*}; use sp_trie::MemoryDB; @@ -77,39 +77,41 @@ pub fn validate_block< PSC: crate::Config, CI: crate::CheckInherents, >( - params: ValidationParams, + MemoryOptimizedValidationParams { + block_data, + parent_head, + relay_parent_number, + relay_parent_storage_root, + }: MemoryOptimizedValidationParams, ) -> ValidationResult where B::Extrinsic: ExtrinsicCall, ::Call: IsSubType>, { - let block_data = ParachainBlockData::::decode(&mut ¶ms.block_data.0[..]) + let block_data = codec::decode_from_bytes::>(block_data) .expect("Invalid parachain block data"); - let parent_head = - B::Header::decode(&mut ¶ms.parent_head.0[..]).expect("Invalid parent head"); - - // We decoded the block data and don't need it anymore. So, we release it to free the memory. - mem::drop(params.block_data); + let parent_header = + codec::decode_from_bytes::(parent_head.clone()).expect("Invalid parent head"); let (header, extrinsics, storage_proof) = block_data.deconstruct(); let head_data = HeadData(header.encode()); let block = B::new(header, extrinsics); - assert!(parent_head.hash() == *block.header().parent_hash(), "Invalid parent hash"); + assert!(parent_header.hash() == *block.header().parent_hash(), "Invalid parent hash"); let inherent_data = extract_parachain_inherent_data(&block); validate_validation_data( &inherent_data.validation_data, - params.relay_parent_number, - params.relay_parent_storage_root, - params.parent_head, + relay_parent_number, + relay_parent_storage_root, + parent_head, ); // Create the db - let db = match storage_proof.to_memory_db(Some(parent_head.state_root())) { + let db = match storage_proof.to_memory_db(Some(parent_header.state_root())) { Ok((db, _)) => db, Err(_) => panic!("Compact proof decoding failure."), }; @@ -118,7 +120,8 @@ where // We use the storage root of the `parent_head` to ensure that it is the correct root. // This is already being done above while creating the in-memory db, but let's be paranoid!! - let backend = sp_state_machine::TrieBackendBuilder::new(db, *parent_head.state_root()).build(); + let backend = + sp_state_machine::TrieBackendBuilder::new(db, *parent_header.state_root()).build(); let _guard = ( // Replace storage calls with our own implementations @@ -236,9 +239,9 @@ fn validate_validation_data( validation_data: &PersistedValidationData, relay_parent_number: RelayChainBlockNumber, relay_parent_storage_root: RHash, - parent_head: HeadData, + parent_head: bytes::Bytes, ) { - assert_eq!(parent_head, validation_data.parent_head, "Parent head doesn't match"); + assert_eq!(parent_head, validation_data.parent_head.0, "Parent head doesn't match"); assert_eq!( relay_parent_number, validation_data.relay_parent_number, "Relay parent number doesn't match", diff --git a/pallets/parachain-system/src/validate_block/mod.rs b/pallets/parachain-system/src/validate_block/mod.rs index ab921ffc611..162f181a665 100644 --- a/pallets/parachain-system/src/validate_block/mod.rs +++ b/pallets/parachain-system/src/validate_block/mod.rs @@ -27,7 +27,32 @@ mod tests; pub use polkadot_parachain; #[cfg(not(feature = "std"))] #[doc(hidden)] -pub use sp_io; +pub use sp_std; +#[cfg(not(feature = "std"))] +#[doc(hidden)] +pub use bytes; +#[cfg(not(feature = "std"))] +#[doc(hidden)] +pub use codec::decode_from_bytes; #[cfg(not(feature = "std"))] #[doc(hidden)] pub use sp_runtime::traits::GetRuntimeBlockType; + +/// Basically the same as [`ValidationParams`](polkadot_parachain::primitives::ValidationParams), +/// but a little bit optimized for our use case here. +/// +/// `block_data` and `head_data` are represented as [`bytes::Bytes`] to make them reuse +/// the memory of the input parameter of the exported `validate_blocks` function. +/// +/// The layout of this type must match exactly the layout of +/// [`ValidationParams`](polkadot_parachain::primitives::ValidationParams) to have the same +/// SCALE encoding. +#[derive(codec::Decode)] +#[cfg_attr(feature = "std", derive(codec::Encode))] +#[doc(hidden)] +pub struct MemoryOptimizedValidationParams { + pub parent_head: bytes::Bytes, + pub block_data: bytes::Bytes, + pub relay_parent_number: cumulus_primitives_core::relay_chain::v2::BlockNumber, + pub relay_parent_storage_root: cumulus_primitives_core::relay_chain::v2::Hash, +} diff --git a/pallets/parachain-system/src/validate_block/tests.rs b/pallets/parachain-system/src/validate_block/tests.rs index 116ee86edad..b9b3ca26dc7 100644 --- a/pallets/parachain-system/src/validate_block/tests.rs +++ b/pallets/parachain-system/src/validate_block/tests.rs @@ -14,7 +14,7 @@ // You should have received a copy of the GNU General Public License // along with Cumulus. If not, see . -use codec::{Decode, Encode}; +use codec::{Decode, Encode, DecodeAll}; use cumulus_primitives_core::{ParachainBlockData, PersistedValidationData}; use cumulus_test_client::{ generate_extrinsic, @@ -27,6 +27,8 @@ use sp_keyring::AccountKeyring::*; use sp_runtime::traits::Header as HeaderT; use std::{env, process::Command}; +use crate::validate_block::MemoryOptimizedValidationParams; + fn call_validate_block_encoded_header( parent_head: Header, block_data: ParachainBlockData, @@ -289,3 +291,33 @@ fn check_inherents_are_unsigned_and_before_all_other_extrinsics() { .contains("Could not find `set_validation_data` inherent")); } } + +/// Test that ensures that `ValidationParams` and `MemoryOptimizedValidationParams` +/// are encoding/decoding. +#[test] +fn validation_params_and_memory_optimized_validation_params_encode_and_decode() { + const BLOCK_DATA: &[u8] = &[1, 2, 3, 4, 5]; + const PARENT_HEAD: &[u8] = &[1, 3, 4, 5, 6, 7, 9]; + + + let validation_params = + ValidationParams { + block_data: BlockData(BLOCK_DATA.encode()), + parent_head: HeadData(PARENT_HEAD.encode()), + relay_parent_number: 1, + relay_parent_storage_root: Hash::random(), + }; + + let encoded = validation_params.encode(); + + let decoded = MemoryOptimizedValidationParams::decode_all(&mut &encoded[..]).unwrap(); + assert_eq!(decoded.relay_parent_number, validation_params.relay_parent_number); + assert_eq!(decoded.relay_parent_storage_root, validation_params.relay_parent_storage_root); + assert_eq!(decoded.block_data, validation_params.block_data.0); + assert_eq!(decoded.parent_head, validation_params.parent_head.0); + + let encoded = decoded.encode(); + + let decoded = ValidationParams::decode_all(&mut &encoded[..]).unwrap(); + assert_eq!(decoded, validation_params); +} From 178a45c51b0fbcf64c128d7601f5d123b9302009 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Tue, 10 Jan 2023 11:44:35 +0100 Subject: [PATCH 4/5] FMT --- .../parachain-system/src/validate_block/mod.rs | 10 +++++----- .../parachain-system/src/validate_block/tests.rs | 16 +++++++--------- 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/pallets/parachain-system/src/validate_block/mod.rs b/pallets/parachain-system/src/validate_block/mod.rs index 162f181a665..8138f2843b3 100644 --- a/pallets/parachain-system/src/validate_block/mod.rs +++ b/pallets/parachain-system/src/validate_block/mod.rs @@ -24,19 +24,19 @@ mod tests; #[cfg(not(feature = "std"))] #[doc(hidden)] -pub use polkadot_parachain; +pub use bytes; #[cfg(not(feature = "std"))] #[doc(hidden)] -pub use sp_std; +pub use codec::decode_from_bytes; #[cfg(not(feature = "std"))] #[doc(hidden)] -pub use bytes; +pub use polkadot_parachain; #[cfg(not(feature = "std"))] #[doc(hidden)] -pub use codec::decode_from_bytes; +pub use sp_runtime::traits::GetRuntimeBlockType; #[cfg(not(feature = "std"))] #[doc(hidden)] -pub use sp_runtime::traits::GetRuntimeBlockType; +pub use sp_std; /// Basically the same as [`ValidationParams`](polkadot_parachain::primitives::ValidationParams), /// but a little bit optimized for our use case here. diff --git a/pallets/parachain-system/src/validate_block/tests.rs b/pallets/parachain-system/src/validate_block/tests.rs index b9b3ca26dc7..2c39188a085 100644 --- a/pallets/parachain-system/src/validate_block/tests.rs +++ b/pallets/parachain-system/src/validate_block/tests.rs @@ -14,7 +14,7 @@ // You should have received a copy of the GNU General Public License // along with Cumulus. If not, see . -use codec::{Decode, Encode, DecodeAll}; +use codec::{Decode, DecodeAll, Encode}; use cumulus_primitives_core::{ParachainBlockData, PersistedValidationData}; use cumulus_test_client::{ generate_extrinsic, @@ -299,14 +299,12 @@ fn validation_params_and_memory_optimized_validation_params_encode_and_decode() const BLOCK_DATA: &[u8] = &[1, 2, 3, 4, 5]; const PARENT_HEAD: &[u8] = &[1, 3, 4, 5, 6, 7, 9]; - - let validation_params = - ValidationParams { - block_data: BlockData(BLOCK_DATA.encode()), - parent_head: HeadData(PARENT_HEAD.encode()), - relay_parent_number: 1, - relay_parent_storage_root: Hash::random(), - }; + let validation_params = ValidationParams { + block_data: BlockData(BLOCK_DATA.encode()), + parent_head: HeadData(PARENT_HEAD.encode()), + relay_parent_number: 1, + relay_parent_storage_root: Hash::random(), + }; let encoded = validation_params.encode(); From d519aca365e010b0d15d1d383b6fde4665635b3f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Tue, 10 Jan 2023 17:06:07 +0100 Subject: [PATCH 5/5] Delay the header encoding --- pallets/parachain-system/src/validate_block/implementation.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pallets/parachain-system/src/validate_block/implementation.rs b/pallets/parachain-system/src/validate_block/implementation.rs index f7a967498c5..25615ecb94b 100644 --- a/pallets/parachain-system/src/validate_block/implementation.rs +++ b/pallets/parachain-system/src/validate_block/implementation.rs @@ -96,8 +96,6 @@ where let (header, extrinsics, storage_proof) = block_data.deconstruct(); - let head_data = HeadData(header.encode()); - let block = B::new(header, extrinsics); assert!(parent_header.hash() == *block.header().parent_hash(), "Invalid parent hash"); @@ -184,6 +182,8 @@ where }); run_with_externalities::(&backend, || { + let head_data = HeadData(block.header().encode()); + E::execute_block(block); let new_validation_code = crate::NewValidationCode::::get();