From c71487f2d9f8798831c6a6e9614b3028adbb60aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Wed, 16 Oct 2024 22:41:41 +0200 Subject: [PATCH 01/34] Support multiple blocks in `ParachainBlockData` --- Cargo.lock | 1 + cumulus/client/collator/src/lib.rs | 15 +- cumulus/client/collator/src/service.rs | 6 +- cumulus/client/consensus/aura/src/collator.rs | 20 +- .../consensus/aura/src/collators/basic.rs | 7 +- .../consensus/aura/src/collators/lookahead.rs | 15 +- .../collators/slot_based/collation_task.rs | 8 +- cumulus/client/pov-recovery/src/lib.rs | 48 ++++- cumulus/client/pov-recovery/src/tests.rs | 33 ++- .../src/validate_block/implementation.rs | 201 ++++++++++-------- .../src/validate_block/tests.rs | 46 ++-- cumulus/primitives/core/Cargo.toml | 2 + cumulus/primitives/core/src/lib.rs | 58 +++-- cumulus/test/client/src/block_builder.rs | 8 +- cumulus/test/client/src/lib.rs | 37 ++-- .../test/service/benches/validate_block.rs | 9 +- .../service/benches/validate_block_glutton.rs | 21 +- 17 files changed, 295 insertions(+), 240 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1a1c10b9570a..837e16909af8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4512,6 +4512,7 @@ dependencies = [ "sp-runtime 31.0.1", "sp-trie 29.0.0", "staging-xcm", + "tracing", ] [[package]] diff --git a/cumulus/client/collator/src/lib.rs b/cumulus/client/collator/src/lib.rs index 91ff913f263d..951929671aef 100644 --- a/cumulus/client/collator/src/lib.rs +++ b/cumulus/client/collator/src/lib.rs @@ -31,7 +31,7 @@ use polkadot_node_subsystem::messages::{CollationGenerationMessage, CollatorProt use polkadot_overseer::Handle as OverseerHandle; use polkadot_primitives::{CollatorPair, Id as ParaId}; -use codec::{Decode, Encode}; +use codec::Decode; use futures::prelude::*; use std::sync::Arc; @@ -120,13 +120,7 @@ where let (collation, b) = self.service.build_collation(&last_head, block_hash, candidate)?; - tracing::info!( - target: LOG_TARGET, - "PoV size {{ header: {}kb, extrinsics: {}kb, storage_proof: {}kb }}", - b.header().encode().len() as f64 / 1024f64, - b.extrinsics().encode().len() as f64 / 1024f64, - b.storage_proof().encode().len() as f64 / 1024f64, - ); + b.log_size_info(); if let MaybeCompressedPoV::Compressed(ref pov) = collation.proof_of_validity { tracing::info!( @@ -336,6 +330,7 @@ pub fn start_collator_sync( mod tests { use super::*; use async_trait::async_trait; + use codec::Encode; use cumulus_client_consensus_common::ParachainCandidate; use cumulus_primitives_core::ParachainBlockData; use cumulus_test_client::{ @@ -458,10 +453,10 @@ mod tests { let block = ParachainBlockData::::decode(&mut &decompressed[..]).expect("Is a valid block"); - assert_eq!(1, *block.header().number()); + assert_eq!(1, *block.blocks().nth(0).unwrap().header().number()); // Ensure that we did not include `:code` in the proof. - let proof = block.storage_proof(); + let proof = block.proofs().nth(0).unwrap().clone(); let backend = sp_state_machine::create_proof_check_backend::( *header.state_root(), diff --git a/cumulus/client/collator/src/service.rs b/cumulus/client/collator/src/service.rs index c06be006fc17..a121ac226a5b 100644 --- a/cumulus/client/collator/src/service.rs +++ b/cumulus/client/collator/src/service.rs @@ -219,7 +219,7 @@ where block_hash: Block::Hash, candidate: ParachainCandidate, ) -> Option<(Collation, ParachainBlockData)> { - let (header, extrinsics) = candidate.block.deconstruct(); + let block = candidate.block; let compact_proof = match candidate .proof @@ -234,7 +234,7 @@ where // Create the parachain block data for the validators. let collation_info = self - .fetch_collation_info(block_hash, &header) + .fetch_collation_info(block_hash, block.header()) .map_err(|e| { tracing::error!( target: LOG_TARGET, @@ -245,7 +245,7 @@ where .ok() .flatten()?; - let block_data = ParachainBlockData::::new(header, extrinsics, compact_proof); + let block_data = ParachainBlockData::::new(vec![(block, compact_proof)]); let pov = polkadot_node_primitives::maybe_compress_pov(PoV { block_data: BlockData(block_data.encode()), diff --git a/cumulus/client/consensus/aura/src/collator.rs b/cumulus/client/consensus/aura/src/collator.rs index dc830e463a4f..84ac905407e8 100644 --- a/cumulus/client/consensus/aura/src/collator.rs +++ b/cumulus/client/consensus/aura/src/collator.rs @@ -24,7 +24,7 @@ //! This module also exposes some standalone functions for common operations when building //! aura-based collators. -use codec::{Codec, Encode}; +use codec::Codec; use cumulus_client_collator::service::ServiceInterface as CollatorServiceInterface; use cumulus_client_consensus_common::{ self as consensus_common, ParachainBlockImportMarker, ParachainCandidate, @@ -228,10 +228,7 @@ where inherent_data: (ParachainInherentData, InherentData), proposal_duration: Duration, max_pov_size: usize, - ) -> Result< - Option<(Collation, ParachainBlockData, Block::Hash)>, - Box, - > { + ) -> Result)>, Box> { let maybe_candidate = self .build_block_and_import( parent_header, @@ -249,13 +246,7 @@ where if let Some((collation, block_data)) = self.collator_service.build_collation(parent_header, hash, candidate) { - tracing::info!( - target: crate::LOG_TARGET, - "PoV size {{ header: {}kb, extrinsics: {}kb, storage_proof: {}kb }}", - block_data.header().encode().len() as f64 / 1024f64, - block_data.extrinsics().encode().len() as f64 / 1024f64, - block_data.storage_proof().encode().len() as f64 / 1024f64, - ); + block_data.log_size_info(); if let MaybeCompressedPoV::Compressed(ref pov) = collation.proof_of_validity { tracing::info!( @@ -265,10 +256,9 @@ where ); } - Ok(Some((collation, block_data, hash))) + Ok(Some((collation, block_data))) } else { - Err(Box::::from("Unable to produce collation") - as Box) + Err(Box::::from("Unable to produce collation")) } } diff --git a/cumulus/client/consensus/aura/src/collators/basic.rs b/cumulus/client/consensus/aura/src/collators/basic.rs index d843483b79fa..7c8c2ccea448 100644 --- a/cumulus/client/consensus/aura/src/collators/basic.rs +++ b/cumulus/client/consensus/aura/src/collators/basic.rs @@ -260,9 +260,12 @@ where .await ); - if let Some((collation, _, post_hash)) = maybe_collation { + if let Some((collation, block_data)) = maybe_collation { + let Some(block_hash) = block_data.blocks().nth(0).map(|b| b.hash()) else { + continue + }; let result_sender = - Some(collator.collator_service().announce_with_barrier(post_hash)); + Some(collator.collator_service().announce_with_barrier(block_hash)); request.complete(Some(CollationResult { collation, result_sender })); } else { request.complete(None); diff --git a/cumulus/client/consensus/aura/src/collators/lookahead.rs b/cumulus/client/consensus/aura/src/collators/lookahead.rs index 8ac43fbd116e..de8d0f65f73f 100644 --- a/cumulus/client/consensus/aura/src/collators/lookahead.rs +++ b/cumulus/client/consensus/aura/src/collators/lookahead.rs @@ -436,7 +436,16 @@ where ) .await { - Ok(Some((collation, block_data, new_block_hash))) => { + Ok(Some((collation, block_data))) => { + let Some(new_block_header) = + block_data.blocks().nth(0).map(|b| b.header().clone()) + else { + tracing::error!(target: crate::LOG_TARGET, "Produced PoV doesn't contain any blocks"); + break + }; + + let new_block_hash = new_block_header.hash(); + // Here we are assuming that the import logic protects against equivocations // and provides sybil-resistance, as it should. collator.collator_service().announce_block(new_block_hash, None); @@ -446,7 +455,7 @@ where export_pov.clone(), collation.proof_of_validity.clone().into_compressed(), new_block_hash, - *block_data.header().number(), + *new_block_header.number(), parent_header.clone(), *relay_parent_header.state_root(), *relay_parent_header.number(), @@ -475,7 +484,7 @@ where .await; parent_hash = new_block_hash; - parent_header = block_data.into_header(); + parent_header = new_block_header; }, Ok(None) => { tracing::debug!(target: crate::LOG_TARGET, "No block proposal"); diff --git a/cumulus/client/consensus/aura/src/collators/slot_based/collation_task.rs b/cumulus/client/consensus/aura/src/collators/slot_based/collation_task.rs index 5b8151f6302c..83df8876859c 100644 --- a/cumulus/client/consensus/aura/src/collators/slot_based/collation_task.rs +++ b/cumulus/client/consensus/aura/src/collators/slot_based/collation_task.rs @@ -107,13 +107,7 @@ async fn handle_collation_message( }, }; - tracing::info!( - target: LOG_TARGET, - "PoV size {{ header: {:.2}kB, extrinsics: {:.2}kB, storage_proof: {:.2}kB }}", - block_data.header().encoded_size() as f64 / 1024f64, - block_data.extrinsics().encoded_size() as f64 / 1024f64, - block_data.storage_proof().encoded_size() as f64 / 1024f64, - ); + block_data.log_size_info(); if let MaybeCompressedPoV::Compressed(ref pov) = collation.proof_of_validity { tracing::info!( diff --git a/cumulus/client/pov-recovery/src/lib.rs b/cumulus/client/pov-recovery/src/lib.rs index 043cba12d193..2ec68eb5195a 100644 --- a/cumulus/client/pov-recovery/src/lib.rs +++ b/cumulus/client/pov-recovery/src/lib.rs @@ -398,9 +398,33 @@ where }, }; - let block = block_data.into_block(); + let blocks_and_proofs = block_data.into_inner(); - let parent = *block.header().parent_hash(); + if let Some((block, _)) = blocks_and_proofs.last() { + let last_block_hash = block.hash(); + if last_block_hash != block_hash { + tracing::debug!( + target: LOG_TARGET, + expected_block_hash = ?block_hash, + got_block_hash = ?last_block_hash, + "Recovered candidate doesn't contain the expected block.", + ); + + self.reset_candidate(block_hash); + return; + } + } + + let Some(parent) = blocks_and_proofs.first().map(|(b, _)| *b.header().parent_hash()) else { + tracing::debug!( + target: LOG_TARGET, + ?block_hash, + "Recovered candidate doesn't contain any blocks.", + ); + + self.reset_candidate(block_hash); + return; + }; match self.parachain_client.block_status(parent) { Ok(BlockStatus::Unknown) => { @@ -418,7 +442,12 @@ where "Waiting for recovery of parent.", ); - self.waiting_for_parent.entry(parent).or_default().push(block); + blocks_and_proofs.into_iter().for_each(|(b, _)| { + self.waiting_for_parent + .entry(*b.header().parent_hash()) + .or_default() + .push(b); + }); return } else { tracing::debug!( @@ -447,17 +476,16 @@ where _ => (), } - self.import_block(block); + self.import_blocks(blocks_and_proofs.into_iter().map(|d| d.0)); } - /// Import the given `block`. + /// Import the given `blocks`. /// /// This will also recursively drain `waiting_for_parent` and import them as well. - fn import_block(&mut self, block: Block) { - let mut blocks = VecDeque::new(); + fn import_blocks(&mut self, blocks: impl Iterator) { + let mut blocks = VecDeque::from_iter(blocks); - tracing::debug!(target: LOG_TARGET, block_hash = ?block.hash(), "Importing block retrieved using pov_recovery"); - blocks.push_back(block); + tracing::trace!(target: LOG_TARGET, blocks = ?blocks.iter().map(|b| b.hash()), "Importing blocks retrieved using pov_recovery"); let mut incoming_blocks = Vec::new(); @@ -586,7 +614,7 @@ where if let Some(waiting_blocks) = self.waiting_for_parent.remove(&imported.hash) { for block in waiting_blocks { tracing::debug!(target: LOG_TARGET, block_hash = ?block.hash(), resolved_parent = ?imported.hash, "Found new waiting child block during import, queuing."); - self.import_block(block); + self.import_blocks(std::iter::once(block)); } }; diff --git a/cumulus/client/pov-recovery/src/tests.rs b/cumulus/client/pov-recovery/src/tests.rs index 94dec32485cc..b32731776a99 100644 --- a/cumulus/client/pov-recovery/src/tests.rs +++ b/cumulus/client/pov-recovery/src/tests.rs @@ -688,9 +688,7 @@ async fn single_pending_candidate_recovery_success(#[case] runtime_version: u32) AvailableData { pov: Arc::new(PoV { block_data: ParachainBlockData::::new( - header.clone(), - vec![], - CompactProof {encoded_nodes: vec![]} + vec![(Block::new(header.clone(), vec![]), CompactProof { encoded_nodes: vec![] })] ).encode().into() }), validation_data: dummy_pvd(), @@ -792,9 +790,7 @@ async fn single_pending_candidate_recovery_retry_succeeds() { AvailableData { pov: Arc::new(PoV { block_data: ParachainBlockData::::new( - header.clone(), - vec![], - CompactProof {encoded_nodes: vec![]} + vec![(Block::new(header.clone(), Vec::new()), CompactProof { encoded_nodes: vec![] })] ).encode().into() }), validation_data: dummy_pvd(), @@ -1098,11 +1094,10 @@ async fn candidate_is_imported_while_awaiting_recovery() { recovery_response_tx .send(Ok(AvailableData { pov: Arc::new(PoV { - block_data: ParachainBlockData::::new( - header.clone(), - vec![], + block_data: ParachainBlockData::::new(vec![( + Block::new(header.clone(), vec![]), CompactProof { encoded_nodes: vec![] }, - ) + )]) .encode() .into(), }), @@ -1196,11 +1191,10 @@ async fn candidate_is_finalized_while_awaiting_recovery() { recovery_response_tx .send(Ok(AvailableData { pov: Arc::new(PoV { - block_data: ParachainBlockData::::new( - header.clone(), - vec![], + block_data: ParachainBlockData::::new(vec![( + Block::new(header.clone(), vec![]), CompactProof { encoded_nodes: vec![] }, - ) + )]) .encode() .into(), }), @@ -1285,9 +1279,7 @@ async fn chained_recovery_success() { .send(Ok(AvailableData { pov: Arc::new(PoV { block_data: ParachainBlockData::::new( - header.clone(), - vec![], - CompactProof { encoded_nodes: vec![] }, + vec![(Block::new(header.clone(), vec![]), CompactProof { encoded_nodes: vec![] })] ) .encode() .into(), @@ -1401,11 +1393,10 @@ async fn chained_recovery_child_succeeds_before_parent() { recovery_response_sender .send(Ok(AvailableData { pov: Arc::new(PoV { - block_data: ParachainBlockData::::new( - header.clone(), - vec![], + block_data: ParachainBlockData::::new(vec![( + Block::new(header.clone(), vec![]), CompactProof { encoded_nodes: vec![] }, - ) + )]) .encode() .into(), }), diff --git a/cumulus/pallets/parachain-system/src/validate_block/implementation.rs b/cumulus/pallets/parachain-system/src/validate_block/implementation.rs index c4c8440e5187..020eab5b7f59 100644 --- a/cumulus/pallets/parachain-system/src/validate_block/implementation.rs +++ b/cumulus/pallets/parachain-system/src/validate_block/implementation.rs @@ -29,7 +29,10 @@ use polkadot_parachain_primitives::primitives::{ use alloc::vec::Vec; use codec::Encode; -use frame_support::traits::{ExecuteBlock, ExtrinsicCall, Get, IsSubType}; +use frame_support::{ + traits::{ExecuteBlock, ExtrinsicCall, Get, IsSubType}, + BoundedVec, +}; use sp_core::storage::{ChildInfo, StateVersion}; use sp_externalities::{set_and_run_with_externalities, Externalities}; use sp_io::KillStorageResult; @@ -89,7 +92,7 @@ pub fn validate_block< >( MemoryOptimizedValidationParams { block_data, - parent_head, + parent_head: parachain_head, relay_parent_number, relay_parent_storage_root, }: MemoryOptimizedValidationParams, @@ -98,46 +101,6 @@ where B::Extrinsic: ExtrinsicCall, ::Call: IsSubType>, { - let block_data = codec::decode_from_bytes::>(block_data) - .expect("Invalid parachain 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 block = B::new(header, extrinsics); - 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, - relay_parent_number, - relay_parent_storage_root, - parent_head, - ); - - // Create the db - let db = match storage_proof.to_memory_db(Some(parent_header.state_root())) { - Ok((db, _)) => db, - Err(_) => panic!("Compact proof decoding failure."), - }; - - core::mem::drop(storage_proof); - - let mut recorder = SizeOnlyRecorderProvider::new(); - let cache_provider = trie_cache::CacheProvider::new(); - // 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_with_cache( - db, - *parent_header.state_root(), - cache_provider, - ) - .with_recorder(recorder.clone()) - .build(); - let _guard = ( // Replace storage calls with our own implementations sp_io::storage::host_read.replace_implementation(host_storage_read), @@ -179,59 +142,117 @@ where .replace_implementation(host_storage_proof_size), ); - run_with_externalities_and_recorder::(&backend, &mut recorder, || { - let relay_chain_proof = crate::RelayChainStateProof::new( - PSC::SelfParaId::get(), - inherent_data.validation_data.relay_parent_storage_root, - inherent_data.relay_chain_state.clone(), - ) - .expect("Invalid relay chain state proof"); + let block_data = codec::decode_from_bytes::>(block_data) + .expect("Invalid parachain block data"); - #[allow(deprecated)] - let res = CI::check_inherents(&block, &relay_chain_proof); + let mut parent_header = + codec::decode_from_bytes::(parachain_head.clone()).expect("Invalid parent head"); - if !res.ok() { - if log::log_enabled!(log::Level::Error) { - res.into_errors().for_each(|e| { - log::error!("Checking inherent with identifier `{:?}` failed", e.0) - }); - } + let blocks_and_proofs = block_data.into_inner(); - panic!("Checking inherents failed"); - } - }); + assert_eq!( + *blocks_and_proofs + .first() + .expect("BlockData should have at least one block") + .0 + .header() + .parent_hash(), + parent_header.hash(), + "Parachain head needs to be the parent of the first block" + ); - run_with_externalities_and_recorder::(&backend, &mut recorder, || { - let head_data = HeadData(block.header().encode()); + let mut processed_downward_messages = 0; + let mut upward_messages = BoundedVec::default(); + let mut horizontal_messages = BoundedVec::default(); + let mut hrmp_watermark = Default::default(); + let mut head_data = None; + let mut new_validation_code = None; + + for (block, storage_proof) in blocks_and_proofs { + let inherent_data = extract_parachain_inherent_data(&block); + + validate_validation_data( + &inherent_data.validation_data, + relay_parent_number, + relay_parent_storage_root, + ¶chain_head, + ); - E::execute_block(block); + // Create the db + let db = match storage_proof.to_memory_db(Some(parent_header.state_root())) { + Ok((db, _)) => db, + Err(_) => panic!("Compact proof decoding failure."), + }; + + core::mem::drop(storage_proof); + + let mut recorder = SizeOnlyRecorderProvider::new(); + let cache_provider = trie_cache::CacheProvider::new(); + // 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_with_cache( + db, + *parent_header.state_root(), + cache_provider, + ) + .with_recorder(recorder.clone()) + .build(); + + run_with_externalities_and_recorder::(&backend, &mut recorder, || { + let relay_chain_proof = crate::RelayChainStateProof::new( + PSC::SelfParaId::get(), + inherent_data.validation_data.relay_parent_storage_root, + inherent_data.relay_chain_state.clone(), + ) + .expect("Invalid relay chain state proof"); + + #[allow(deprecated)] + let res = CI::check_inherents(&block, &relay_chain_proof); + + if !res.ok() { + if log::log_enabled!(log::Level::Error) { + res.into_errors().for_each(|e| { + log::error!("Checking inherent with identifier `{:?}` failed", e.0) + }); + } + + panic!("Checking inherents failed"); + } + }); + + run_with_externalities_and_recorder::(&backend, &mut recorder, || { + parent_header = block.header().clone(); + + E::execute_block(block); + + new_validation_code = + new_validation_code.take().or(crate::NewValidationCode::::get()); + upward_messages + .try_extend(crate::UpwardMessages::::get().into_iter()) + .expect( + "Number of upward messages should not be greater than `MAX_UPWARD_MESSAGE_NUM`", + ); + processed_downward_messages += crate::ProcessedDownwardMessages::::get(); + horizontal_messages.try_extend(crate::HrmpOutboundMessages::::get().into_iter()).expect( + "Number of horizontal messages should not be greater than `MAX_HORIZONTAL_MESSAGE_NUM`", + ); + hrmp_watermark = crate::HrmpWatermark::::get(); + + head_data = Some( + crate::CustomValidationHeadData::::get() + .map_or_else(|| HeadData(parent_header.encode()), HeadData), + ); + }) + } - let new_validation_code = crate::NewValidationCode::::get(); - let upward_messages = crate::UpwardMessages::::get().try_into().expect( - "Number of upward messages should not be greater than `MAX_UPWARD_MESSAGE_NUM`", - ); - let processed_downward_messages = crate::ProcessedDownwardMessages::::get(); - let horizontal_messages = crate::HrmpOutboundMessages::::get().try_into().expect( - "Number of horizontal messages should not be greater than `MAX_HORIZONTAL_MESSAGE_NUM`", - ); - 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, - } - }) + ValidationResult { + head_data: head_data.expect("HeadData not set"), + new_validation_code: new_validation_code.map(Into::into), + upward_messages, + processed_downward_messages, + horizontal_messages, + hrmp_watermark, + } } /// Extract the [`ParachainInherentData`]. @@ -263,7 +284,7 @@ fn validate_validation_data( validation_data: &PersistedValidationData, relay_parent_number: RelayChainBlockNumber, relay_parent_storage_root: RHash, - parent_head: bytes::Bytes, + parent_head: &[u8], ) { assert_eq!(parent_head, validation_data.parent_head.0, "Parent head doesn't match"); assert_eq!( diff --git a/cumulus/pallets/parachain-system/src/validate_block/tests.rs b/cumulus/pallets/parachain-system/src/validate_block/tests.rs index 871ce5c1710e..bff491a71332 100644 --- a/cumulus/pallets/parachain-system/src/validate_block/tests.rs +++ b/cumulus/pallets/parachain-system/src/validate_block/tests.rs @@ -115,7 +115,7 @@ fn validate_block_works() { build_block_with_witness(&client, Vec::new(), parent_head.clone(), Default::default()); let block = seal_block(block, slot, &client); - let header = block.header().clone(); + let header = block.blocks().nth(0).unwrap().header().clone(); let res_header = call_validate_block(parent_head, block, validation_data.relay_parent_storage_root) .expect("Calls `validate_block`"); @@ -140,7 +140,7 @@ fn validate_block_with_extra_extrinsics() { Default::default(), ); let block = seal_block(block, slot, &client); - let header = block.header().clone(); + let header = block.blocks().nth(0).unwrap().header().clone(); let res_header = call_validate_block(parent_head, block, validation_data.relay_parent_storage_root) @@ -173,7 +173,7 @@ fn validate_block_returns_custom_head_data() { parent_head.clone(), Default::default(), ); - let header = block.header().clone(); + let header = block.blocks().nth(0).unwrap().header().clone(); assert_ne!(expected_header, header.encode()); let block = seal_block(block, slot, &client); @@ -192,13 +192,16 @@ fn validate_block_invalid_parent_hash() { if env::var("RUN_TEST").is_ok() { let (client, parent_head) = create_test_client(); - let TestBlockData { block, validation_data, .. } = + let TestBlockData { mut block, validation_data, .. } = build_block_with_witness(&client, Vec::new(), parent_head.clone(), Default::default()); - let (mut header, extrinsics, witness) = block.deconstruct(); - header.set_parent_hash(Hash::from_low_u64_be(1)); + block + .blocks_mut() + .nth(0) + .unwrap() + .header + .set_parent_hash(Hash::from_low_u64_be(1)); - let block_data = ParachainBlockData::new(header, extrinsics, witness); - call_validate_block(parent_head, block_data, validation_data.relay_parent_storage_root) + call_validate_block(parent_head, block, validation_data.relay_parent_storage_root) .unwrap_err(); } else { let output = Command::new(env::current_exe().unwrap()) @@ -208,7 +211,8 @@ fn validate_block_invalid_parent_hash() { .expect("Runs the test"); assert!(output.status.success()); - assert!(dbg!(String::from_utf8(output.stderr).unwrap()).contains("Invalid parent hash")); + assert!(dbg!(String::from_utf8(output.stderr).unwrap()) + .contains("Parachain head needs to be the parent of the first block")); } } @@ -242,19 +246,18 @@ fn check_inherents_are_unsigned_and_before_all_other_extrinsics() { if env::var("RUN_TEST").is_ok() { let (client, parent_head) = create_test_client(); - let TestBlockData { block, validation_data, .. } = + let TestBlockData { mut block, validation_data, .. } = build_block_with_witness(&client, Vec::new(), parent_head.clone(), Default::default()); - let (header, mut extrinsics, proof) = block.deconstruct(); - - extrinsics.insert(0, transfer(&client, Alice, Bob, 69)); + block + .blocks_mut() + .nth(0) + .unwrap() + .extrinsics + .insert(0, transfer(&client, Alice, Bob, 69)); - call_validate_block( - parent_head, - ParachainBlockData::new(header, extrinsics, proof), - validation_data.relay_parent_storage_root, - ) - .unwrap_err(); + call_validate_block(parent_head, block, validation_data.relay_parent_storage_root) + .unwrap_err(); } else { let output = Command::new(env::current_exe().unwrap()) .args([ @@ -319,7 +322,8 @@ fn validate_block_works_with_child_tries() { parent_head.clone(), Default::default(), ); - let block = block.into_block(); + + let block = block.blocks().nth(0).unwrap().clone(); futures::executor::block_on(client.import(BlockOrigin::Own, block.clone())).unwrap(); @@ -333,7 +337,7 @@ fn validate_block_works_with_child_tries() { ); let block = seal_block(block, slot, &client); - let header = block.header().clone(); + let header = block.blocks().nth(0).unwrap().header().clone(); let res_header = call_validate_block(parent_head, block, validation_data.relay_parent_storage_root) .expect("Calls `validate_block`"); diff --git a/cumulus/primitives/core/Cargo.toml b/cumulus/primitives/core/Cargo.toml index 533d368d3b00..1ab0d448b9f4 100644 --- a/cumulus/primitives/core/Cargo.toml +++ b/cumulus/primitives/core/Cargo.toml @@ -12,6 +12,7 @@ workspace = true [dependencies] codec = { features = ["derive"], workspace = true } scale-info = { features = ["derive"], workspace = true } +tracing.workspace = true # Substrate sp-api = { workspace = true } @@ -35,6 +36,7 @@ std = [ "sp-api/std", "sp-runtime/std", "sp-trie/std", + "tracing/std", "xcm/std", ] runtime-benchmarks = [ diff --git a/cumulus/primitives/core/src/lib.rs b/cumulus/primitives/core/src/lib.rs index dfb574ef3301..c371fac60ed8 100644 --- a/cumulus/primitives/core/src/lib.rs +++ b/cumulus/primitives/core/src/lib.rs @@ -42,6 +42,7 @@ pub use sp_runtime::{ ConsensusEngineId, }; +use sp_trie::CompactProof; pub use xcm::latest::prelude::*; /// A module that re-exports relevant relay chain definitions. @@ -201,53 +202,46 @@ pub enum ServiceQuality { /// This is send as PoV (proof of validity block) to the relay-chain validators. There it will be /// passed to the parachain validation Wasm blob to be validated. #[derive(codec::Encode, codec::Decode, Clone)] -pub struct ParachainBlockData { - /// The header of the parachain block. - header: B::Header, - /// The extrinsics of the parachain block. - extrinsics: alloc::vec::Vec, - /// The data that is required to emulate the storage accesses executed by all extrinsics. - storage_proof: sp_trie::CompactProof, +pub struct ParachainBlockData { + blocks: Vec<(Block, CompactProof)>, } -impl ParachainBlockData { +impl ParachainBlockData { /// Creates a new instance of `Self`. - pub fn new( - header: ::Header, - extrinsics: alloc::vec::Vec<::Extrinsic>, - storage_proof: sp_trie::CompactProof, - ) -> Self { - Self { header, extrinsics, storage_proof } + pub fn new(blocks: Vec<(Block, CompactProof)>) -> Self { + Self { blocks } } - /// Convert `self` into the stored block. - pub fn into_block(self) -> B { - B::new(self.header, self.extrinsics) + pub fn blocks(&self) -> impl Iterator { + self.blocks.iter().map(|e| &e.0) } - /// Convert `self` into the stored header. - pub fn into_header(self) -> B::Header { - self.header + pub fn blocks_mut(&mut self) -> impl Iterator { + self.blocks.iter_mut().map(|e| &mut e.0) } - /// Returns the header. - pub fn header(&self) -> &B::Header { - &self.header + pub fn into_blocks(self) -> impl Iterator { + self.blocks.into_iter().map(|d| d.0) } - /// Returns the extrinsics. - pub fn extrinsics(&self) -> &[B::Extrinsic] { - &self.extrinsics + pub fn proofs(&self) -> impl Iterator { + self.blocks.iter().map(|d| &d.1) } - /// Returns the [`CompactProof`](sp_trie::CompactProof). - pub fn storage_proof(&self) -> &sp_trie::CompactProof { - &self.storage_proof + /// Deconstruct into the inner parts. + pub fn into_inner(self) -> Vec<(Block, CompactProof)> { + self.blocks } - /// Deconstruct into the inner parts. - pub fn deconstruct(self) -> (B::Header, alloc::vec::Vec, sp_trie::CompactProof) { - (self.header, self.extrinsics, self.storage_proof) + /// Log the size of the individual components (header, extrinsics, storage proof) as info. + pub fn log_size_info(&self) { + tracing::info!( + target: "cumulus", + "PoV size {{ header: {}kb, extrinsics: {}kb, storage_proof: {}kb }}", + self.blocks().map(|b| b.header().encoded_size()).sum::() as f64 / 1024f64, + self.blocks().map(|b| b.extrinsics().encoded_size()).sum::() as f64 / 1024f64, + self.proofs().map(|p| p.encoded_size()).sum::() as f64 / 1024f64, + ); } } diff --git a/cumulus/test/client/src/block_builder.rs b/cumulus/test/client/src/block_builder.rs index c2e5a69dd9b5..82b1ffcbb6e5 100644 --- a/cumulus/test/client/src/block_builder.rs +++ b/cumulus/test/client/src/block_builder.rs @@ -24,10 +24,7 @@ use polkadot_primitives::{BlockNumber as PBlockNumber, Hash as PHash}; use sc_block_builder::BlockBuilderBuilder; use sp_api::ProvideRuntimeApi; use sp_consensus_aura::Slot; -use sp_runtime::{ - traits::{Block as BlockT, Header as HeaderT}, - Digest, DigestItem, -}; +use sp_runtime::{traits::Header as HeaderT, Digest, DigestItem}; /// A struct containing a block builder and support data required to build test scenarios. pub struct BlockBuilderAndSupportData<'a> { @@ -195,7 +192,6 @@ impl<'a> BuildParachainBlockData for sc_block_builder::BlockBuilder<'a, Block, C .into_compact_proof::<
::Hashing>(parent_state_root) .expect("Creates the compact proof"); - let (header, extrinsics) = built_block.block.deconstruct(); - ParachainBlockData::new(header, extrinsics, storage_proof) + ParachainBlockData::new(vec![(built_block.block, storage_proof)]) } } diff --git a/cumulus/test/client/src/lib.rs b/cumulus/test/client/src/lib.rs index f26413e441e7..5bf64c0a9dbd 100644 --- a/cumulus/test/client/src/lib.rs +++ b/cumulus/test/client/src/lib.rs @@ -238,19 +238,28 @@ pub fn seal_block( parachain_slot: Slot, client: &Client, ) -> ParachainBlockData { - let parent_hash = block.header().parent_hash; - let authorities = client.runtime_api().authorities(parent_hash).unwrap(); - let expected_author = slot_author::<::Pair>(parachain_slot, &authorities) - .expect("Should be able to find author"); - - let (mut header, extrinsics, proof) = block.deconstruct(); - let keystore = get_keystore(); - let seal_digest = seal::<_, sp_consensus_aura::sr25519::AuthorityPair>( - &header.hash(), - expected_author, - &keystore, + ParachainBlockData::new( + block + .into_inner() + .into_iter() + .map(|(mut block, proof)| { + let parent_hash = block.header.parent_hash; + let authorities = client.runtime_api().authorities(parent_hash).unwrap(); + let expected_author = + slot_author::<::Pair>(parachain_slot, &authorities) + .expect("Should be able to find author"); + + let keystore = get_keystore(); + let seal_digest = seal::<_, sp_consensus_aura::sr25519::AuthorityPair>( + &block.header.hash(), + expected_author, + &keystore, + ) + .expect("Should be able to create seal"); + block.header.digest_mut().push(seal_digest); + + (block, proof) + }) + .collect::>(), ) - .expect("Should be able to create seal"); - header.digest_mut().push(seal_digest); - ParachainBlockData::new(header, extrinsics, proof) } diff --git a/cumulus/test/service/benches/validate_block.rs b/cumulus/test/service/benches/validate_block.rs index 34b09d99ce98..9ebfa47013b9 100644 --- a/cumulus/test/service/benches/validate_block.rs +++ b/cumulus/test/service/benches/validate_block.rs @@ -116,7 +116,8 @@ fn benchmark_block_validation(c: &mut Criterion) { let parachain_block = block_builder.build_parachain_block(*parent_header.state_root()); - let proof_size_in_kb = parachain_block.storage_proof().encode().len() as f64 / 1024f64; + let proof_size_in_kb = + parachain_block.proofs().map(|p| p.encoded_size()).sum::() as f64 / 1024f64; let runtime = utils::get_wasm_module(); let (relay_parent_storage_root, _) = sproof_builder.into_state_root_and_proof(); @@ -131,7 +132,11 @@ fn benchmark_block_validation(c: &mut Criterion) { // This is not strictly necessary for this benchmark, but // let us make sure that the result of `validate_block` is what // we expect. - verify_expected_result(&runtime, &encoded_params, parachain_block.into_block()); + verify_expected_result( + &runtime, + &encoded_params, + parachain_block.blocks().nth(0).unwrap().clone(), + ); let mut group = c.benchmark_group("Block validation"); group.sample_size(20); diff --git a/cumulus/test/service/benches/validate_block_glutton.rs b/cumulus/test/service/benches/validate_block_glutton.rs index 6fe26519a3eb..05ed11e3d672 100644 --- a/cumulus/test/service/benches/validate_block_glutton.rs +++ b/cumulus/test/service/benches/validate_block_glutton.rs @@ -78,7 +78,11 @@ fn benchmark_block_validation(c: &mut Criterion) { set_glutton_parameters(&client, is_first, compute_ratio, storage_ratio); is_first = false; - runtime.block_on(import_block(&client, parachain_block.clone().into_block(), false)); + runtime.block_on(import_block( + &client, + parachain_block.blocks().nth(0).unwrap().clone(), + false, + )); // Build benchmark block let parent_hash = client.usage_info().chain.best_hash; @@ -92,8 +96,13 @@ fn benchmark_block_validation(c: &mut Criterion) { client.init_block_builder(Some(validation_data), Default::default()); let parachain_block = block_builder.build_parachain_block(*parent_header.state_root()); - let proof_size_in_kb = parachain_block.storage_proof().encode().len() as f64 / 1024f64; - runtime.block_on(import_block(&client, parachain_block.clone().into_block(), false)); + let proof_size_in_kb = + parachain_block.proofs().map(|p| p.encoded_size()).sum::() as f64 / 1024f64; + runtime.block_on(import_block( + &client, + parachain_block.blocks().nth(0).unwrap().clone(), + false, + )); let runtime = utils::get_wasm_module(); let sproof_builder: RelayStateSproofBuilder = Default::default(); @@ -109,7 +118,11 @@ fn benchmark_block_validation(c: &mut Criterion) { // This is not strictly necessary for this benchmark, but // let us make sure that the result of `validate_block` is what // we expect. - verify_expected_result(&runtime, &encoded_params, parachain_block.into_block()); + verify_expected_result( + &runtime, + &encoded_params, + parachain_block.blocks().nth(0).unwrap().clone(), + ); group.bench_function( format!( From 0cdb13d29b1a80dc491f6912f157cfafcab0be3f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Wed, 16 Oct 2024 22:49:33 +0200 Subject: [PATCH 02/34] Comments --- cumulus/primitives/core/src/lib.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/cumulus/primitives/core/src/lib.rs b/cumulus/primitives/core/src/lib.rs index c371fac60ed8..ba46179a3ec0 100644 --- a/cumulus/primitives/core/src/lib.rs +++ b/cumulus/primitives/core/src/lib.rs @@ -212,18 +212,22 @@ impl ParachainBlockData { Self { blocks } } + /// Returns an iterator yielding references to the stored blocks. pub fn blocks(&self) -> impl Iterator { self.blocks.iter().map(|e| &e.0) } + /// Returns an iterator yielding mutable references to the stored blocks. pub fn blocks_mut(&mut self) -> impl Iterator { self.blocks.iter_mut().map(|e| &mut e.0) } + /// Returns an iterator yielding the stored blocks. pub fn into_blocks(self) -> impl Iterator { self.blocks.into_iter().map(|d| d.0) } + /// Returns an iterator yielding references to the stored proofs. pub fn proofs(&self) -> impl Iterator { self.blocks.iter().map(|d| &d.1) } From 441553044feaab5c1e245ee4532db21d622188f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Fri, 18 Oct 2024 10:59:56 +0200 Subject: [PATCH 03/34] Add a test --- Cargo.lock | 1 - cumulus/pallets/parachain-system/Cargo.toml | 1 - .../src/validate_block/tests.rs | 142 ++++++++++++++++-- cumulus/test/client/src/block_builder.rs | 28 ++-- cumulus/test/client/src/lib.rs | 21 ++- 5 files changed, 157 insertions(+), 36 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 837e16909af8..02f6cb68ec6e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4364,7 +4364,6 @@ dependencies = [ "rand", "sc-client-api", "scale-info", - "sp-consensus-slots", "sp-core 28.0.0", "sp-crypto-hashing 0.1.0", "sp-externalities 0.25.0", diff --git a/cumulus/pallets/parachain-system/Cargo.toml b/cumulus/pallets/parachain-system/Cargo.toml index 3cb0394c4b95..c2af4b9b40a8 100644 --- a/cumulus/pallets/parachain-system/Cargo.toml +++ b/cumulus/pallets/parachain-system/Cargo.toml @@ -59,7 +59,6 @@ sp-keyring = { workspace = true, default-features = true } sp-crypto-hashing = { workspace = true, default-features = true } sp-tracing = { workspace = true, default-features = true } sp-version = { workspace = true, default-features = true } -sp-consensus-slots = { workspace = true, default-features = true } # Cumulus cumulus-test-client = { workspace = true } diff --git a/cumulus/pallets/parachain-system/src/validate_block/tests.rs b/cumulus/pallets/parachain-system/src/validate_block/tests.rs index bff491a71332..0bf21c6de07b 100644 --- a/cumulus/pallets/parachain-system/src/validate_block/tests.rs +++ b/cumulus/pallets/parachain-system/src/validate_block/tests.rs @@ -27,7 +27,6 @@ use cumulus_test_client::{ TestClientBuilder, TestClientBuilderExt, ValidationParams, }; use cumulus_test_relay_sproof_builder::RelayStateSproofBuilder; -use sp_consensus_slots::Slot; use sp_runtime::traits::{Block as BlockT, Header as HeaderT}; use std::{env, process::Command}; @@ -35,6 +34,7 @@ use std::{env, process::Command}; use crate::validate_block::MemoryOptimizedValidationParams; fn call_validate_block_encoded_header( + validation_code: &[u8], parent_head: Header, block_data: ParachainBlockData, relay_parent_storage_root: Hash, @@ -46,7 +46,7 @@ fn call_validate_block_encoded_header( relay_parent_number: 1, relay_parent_storage_root, }, - WASM_BINARY.expect("You need to build the WASM binaries to run the tests!"), + validation_code, ) .map(|v| v.head_data.0) } @@ -56,8 +56,29 @@ fn call_validate_block( block_data: ParachainBlockData, relay_parent_storage_root: Hash, ) -> cumulus_test_client::ExecutorResult
{ - call_validate_block_encoded_header(parent_head, block_data, relay_parent_storage_root) - .map(|v| Header::decode(&mut &v[..]).expect("Decodes `Header`.")) + call_validate_block_encoded_header( + WASM_BINARY.expect("You need to build the WASM binaries to run the tests!"), + parent_head, + block_data, + relay_parent_storage_root, + ) + .map(|v| Header::decode(&mut &v[..]).expect("Decodes `Header`.")) +} + +/// Call `validate_block` in the runtime with `elastic-scaling` activated. +fn call_validate_block_elastic_scaling( + parent_head: Header, + block_data: ParachainBlockData, + relay_parent_storage_root: Hash, +) -> cumulus_test_client::ExecutorResult
{ + call_validate_block_encoded_header( + test_runtime::elastic_scaling::WASM_BINARY + .expect("You need to build the WASM binaries to run the tests!"), + parent_head, + block_data, + relay_parent_storage_root, + ) + .map(|v| Header::decode(&mut &v[..]).expect("Decodes `Header`.")) } fn create_test_client() -> (Client, Header) { @@ -72,10 +93,28 @@ fn create_test_client() -> (Client, Header) { (client, genesis_header) } +/// Create test client using the runtime with `elastic-scaling` feature enabled. +fn create_elastic_scaling_test_client() -> (Client, Header) { + let mut builder = TestClientBuilder::new(); + builder.genesis_init_mut().wasm = Some( + test_runtime::elastic_scaling::WASM_BINARY + .expect("You need to build the WASM binaries to run the tests!") + .to_vec(), + ); + let client = builder.enable_import_proof_recording().build(); + + let genesis_header = client + .header(client.chain_info().genesis_hash) + .ok() + .flatten() + .expect("Genesis header exists; qed"); + + (client, genesis_header) +} + struct TestBlockData { block: ParachainBlockData, validation_data: PersistedValidationData, - slot: Slot, } fn build_block_with_witness( @@ -96,14 +135,67 @@ fn build_block_with_witness( let cumulus_test_client::BlockBuilderAndSupportData { mut block_builder, persisted_validation_data, - slot, } = client.init_block_builder(Some(validation_data), sproof_builder); extra_extrinsics.into_iter().for_each(|e| block_builder.push(e).unwrap()); let block = block_builder.build_parachain_block(*parent_head.state_root()); - TestBlockData { block, validation_data: persisted_validation_data, slot } + TestBlockData { block, validation_data: persisted_validation_data } +} + +fn build_multiple_blocks_with_witness( + client: &Client, + mut parent_head: Header, + mut sproof_builder: RelayStateSproofBuilder, + num_blocks: usize, +) -> TestBlockData { + sproof_builder.para_id = test_runtime::PARACHAIN_ID.into(); + sproof_builder.included_para_head = Some(HeadData(parent_head.encode())); + sproof_builder.current_slot = (std::time::SystemTime::now() + .duration_since(std::time::SystemTime::UNIX_EPOCH) + .expect("Time is always after UNIX_EPOCH; qed") + .as_millis() as u64 / + 6000) + .into(); + + let validation_data = PersistedValidationData { + relay_parent_number: 1, + parent_head: parent_head.encode().into(), + ..Default::default() + }; + + let mut persisted_validation_data = None; + let mut blocks = Vec::new(); + + for _ in 0..num_blocks { + let cumulus_test_client::BlockBuilderAndSupportData { + block_builder, + persisted_validation_data: p_v_data, + } = client.init_block_builder(Some(validation_data.clone()), sproof_builder.clone()); + + persisted_validation_data = Some(p_v_data); + + blocks.extend( + block_builder + .build_parachain_block(*parent_head.state_root()) + .into_inner() + .into_iter() + .inspect(|d| { + futures::executor::block_on( + client.import_as_best(BlockOrigin::Own, d.0.clone()), + ) + .unwrap(); + + parent_head = d.0.header.clone(); + }), + ); + } + + TestBlockData { + block: ParachainBlockData::new(blocks), + validation_data: persisted_validation_data.unwrap(), + } } #[test] @@ -111,10 +203,10 @@ fn validate_block_works() { sp_tracing::try_init_simple(); let (client, parent_head) = create_test_client(); - let TestBlockData { block, validation_data, slot } = + let TestBlockData { block, validation_data } = build_block_with_witness(&client, Vec::new(), parent_head.clone(), Default::default()); - let block = seal_block(block, slot, &client); + let block = seal_block(block, &client); let header = block.blocks().nth(0).unwrap().header().clone(); let res_header = call_validate_block(parent_head, block, validation_data.relay_parent_storage_root) @@ -122,6 +214,25 @@ fn validate_block_works() { assert_eq!(header, res_header); } +#[test] +fn validate_multiple_blocks_work() { + sp_tracing::try_init_simple(); + + let (client, parent_head) = create_elastic_scaling_test_client(); + let TestBlockData { block, validation_data } = + build_multiple_blocks_with_witness(&client, parent_head.clone(), Default::default(), 4); + + let block = seal_block(block, &client); + let header = block.blocks().last().unwrap().header().clone(); + let res_header = call_validate_block_elastic_scaling( + parent_head, + block, + validation_data.relay_parent_storage_root, + ) + .expect("Calls `validate_block`"); + assert_eq!(header, res_header); +} + #[test] fn validate_block_with_extra_extrinsics() { sp_tracing::try_init_simple(); @@ -133,13 +244,13 @@ fn validate_block_with_extra_extrinsics() { transfer(&client, Charlie, Alice, 500), ]; - let TestBlockData { block, validation_data, slot } = build_block_with_witness( + let TestBlockData { block, validation_data } = build_block_with_witness( &client, extra_extrinsics, parent_head.clone(), Default::default(), ); - let block = seal_block(block, slot, &client); + let block = seal_block(block, &client); let header = block.blocks().nth(0).unwrap().header().clone(); let res_header = @@ -167,7 +278,7 @@ fn validate_block_returns_custom_head_data() { transfer(&client, Bob, Charlie, 100), ]; - let TestBlockData { block, validation_data, slot } = build_block_with_witness( + let TestBlockData { block, validation_data } = build_block_with_witness( &client, extra_extrinsics, parent_head.clone(), @@ -176,8 +287,9 @@ fn validate_block_returns_custom_head_data() { let header = block.blocks().nth(0).unwrap().header().clone(); assert_ne!(expected_header, header.encode()); - let block = seal_block(block, slot, &client); + let block = seal_block(block, &client); let res_header = call_validate_block_encoded_header( + WASM_BINARY.expect("You need to build the WASM binaries to run the tests!"), parent_head, block, validation_data.relay_parent_storage_root, @@ -329,14 +441,14 @@ fn validate_block_works_with_child_tries() { let parent_head = block.header().clone(); - let TestBlockData { block, validation_data, slot } = build_block_with_witness( + let TestBlockData { block, validation_data } = build_block_with_witness( &client, vec![generate_extrinsic(&client, Alice, TestPalletCall::read_and_write_child_tries {})], parent_head.clone(), Default::default(), ); - let block = seal_block(block, slot, &client); + let block = seal_block(block, &client); let header = block.blocks().nth(0).unwrap().header().clone(); let res_header = call_validate_block(parent_head, block, validation_data.relay_parent_storage_root) diff --git a/cumulus/test/client/src/block_builder.rs b/cumulus/test/client/src/block_builder.rs index 82b1ffcbb6e5..75b8d6932cdf 100644 --- a/cumulus/test/client/src/block_builder.rs +++ b/cumulus/test/client/src/block_builder.rs @@ -23,14 +23,13 @@ use cumulus_test_runtime::{Block, GetLastTimestamp, Hash, Header}; use polkadot_primitives::{BlockNumber as PBlockNumber, Hash as PHash}; use sc_block_builder::BlockBuilderBuilder; use sp_api::ProvideRuntimeApi; -use sp_consensus_aura::Slot; +use sp_consensus_aura::{AuraApi, Slot}; use sp_runtime::{traits::Header as HeaderT, Digest, DigestItem}; /// A struct containing a block builder and support data required to build test scenarios. pub struct BlockBuilderAndSupportData<'a> { pub block_builder: sc_block_builder::BlockBuilder<'a, Block, Client>, pub persisted_validation_data: PersistedValidationData, - pub slot: Slot, } /// An extension for the Cumulus test client to init a block builder. @@ -83,9 +82,12 @@ fn init_block_builder( mut relay_sproof_builder: RelayStateSproofBuilder, timestamp: u64, ) -> BlockBuilderAndSupportData<'_> { - // This slot will be used for both relay chain and parachain - let slot: Slot = (timestamp / cumulus_test_runtime::SLOT_DURATION).into(); - relay_sproof_builder.current_slot = slot; + let slot: Slot = + (timestamp / client.runtime_api().slot_duration(at).unwrap().as_millis()).into(); + + if relay_sproof_builder.current_slot == 0u64 { + relay_sproof_builder.current_slot = (timestamp / 6_000).into(); + } let aura_pre_digest = Digest { logs: vec![DigestItem::PreRuntime(sp_consensus_aura::AURA_ENGINE_ID, slot.encode())], @@ -130,7 +132,7 @@ fn init_block_builder( .into_iter() .for_each(|ext| block_builder.push(ext).expect("Pushes inherent")); - BlockBuilderAndSupportData { block_builder, persisted_validation_data: validation_data, slot } + BlockBuilderAndSupportData { block_builder, persisted_validation_data: validation_data } } impl InitBlockBuilder for Client { @@ -152,12 +154,16 @@ impl InitBlockBuilder for Client { let last_timestamp = self.runtime_api().get_last_timestamp(at).expect("Get last timestamp"); let timestamp = if last_timestamp == 0 { - std::time::SystemTime::now() - .duration_since(std::time::SystemTime::UNIX_EPOCH) - .expect("Time is always after UNIX_EPOCH; qed") - .as_millis() as u64 + if relay_sproof_builder.current_slot != 0u64 { + *relay_sproof_builder.current_slot * 6_000 + } else { + std::time::SystemTime::now() + .duration_since(std::time::SystemTime::UNIX_EPOCH) + .expect("Time is always after UNIX_EPOCH; qed") + .as_millis() as u64 + } } else { - last_timestamp + cumulus_test_runtime::SLOT_DURATION + last_timestamp + self.runtime_api().slot_duration(at).unwrap().as_millis() }; init_block_builder(self, at, validation_data, relay_sproof_builder, timestamp) diff --git a/cumulus/test/client/src/lib.rs b/cumulus/test/client/src/lib.rs index 5bf64c0a9dbd..a3d5982bac80 100644 --- a/cumulus/test/client/src/lib.rs +++ b/cumulus/test/client/src/lib.rs @@ -28,14 +28,17 @@ use runtime::{ Balance, Block, BlockHashCount, Runtime, RuntimeCall, Signature, SignedExtra, SignedPayload, UncheckedExtrinsic, VERSION, }; -use sc_consensus_aura::standalone::{seal, slot_author}; +use sc_consensus_aura::{ + find_pre_digest, + standalone::{seal, slot_author}, +}; pub use sc_executor::error::Result as ExecutorResult; use sc_executor::HeapAllocStrategy; use sc_executor_common::runtime_blob::RuntimeBlob; use sp_api::ProvideRuntimeApi; use sp_application_crypto::AppCrypto; use sp_blockchain::HeaderBackend; -use sp_consensus_aura::{AuraApi, Slot}; +use sp_consensus_aura::AuraApi; use sp_core::Pair; use sp_io::TestExternalities; use sp_keystore::testing::MemoryKeystore; @@ -72,6 +75,7 @@ pub type Client = client::Client; #[derive(Default)] pub struct GenesisParameters { pub endowed_accounts: Vec, + pub wasm: Option>, } impl substrate_test_client::GenesisInit for GenesisParameters { @@ -79,7 +83,9 @@ impl substrate_test_client::GenesisInit for GenesisParameters { cumulus_test_service::chain_spec::get_chain_spec_with_extra_endowed( None, self.endowed_accounts.clone(), - cumulus_test_runtime::WASM_BINARY.expect("WASM binary not compiled!"), + self.wasm.as_deref().unwrap_or_else(|| { + cumulus_test_runtime::WASM_BINARY.expect("WASM binary not compiled!") + }), ) .build_storage() .expect("Builds test runtime genesis storage") @@ -233,16 +239,15 @@ fn get_keystore() -> sp_keystore::KeystorePtr { /// Given parachain block data and a slot, seal the block with an aura seal. Assumes that the /// authorities of the test runtime are present in the keyring. -pub fn seal_block( - block: ParachainBlockData, - parachain_slot: Slot, - client: &Client, -) -> ParachainBlockData { +pub fn seal_block(block: ParachainBlockData, client: &Client) -> ParachainBlockData { ParachainBlockData::new( block .into_inner() .into_iter() .map(|(mut block, proof)| { + let parachain_slot = + find_pre_digest::::Signature>(&block.header) + .unwrap(); let parent_hash = block.header.parent_hash; let authorities = client.runtime_api().authorities(parent_hash).unwrap(); let expected_author = From 109b3f2163d35f853c4d3d3510306eff267438ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Fri, 18 Oct 2024 12:00:52 +0200 Subject: [PATCH 04/34] Only set the head on the last block --- .../src/validate_block/implementation.rs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/cumulus/pallets/parachain-system/src/validate_block/implementation.rs b/cumulus/pallets/parachain-system/src/validate_block/implementation.rs index 020eab5b7f59..8f913779293b 100644 --- a/cumulus/pallets/parachain-system/src/validate_block/implementation.rs +++ b/cumulus/pallets/parachain-system/src/validate_block/implementation.rs @@ -167,8 +167,9 @@ where let mut hrmp_watermark = Default::default(); let mut head_data = None; let mut new_validation_code = None; + let num_blocks = blocks_and_proofs.len(); - for (block, storage_proof) in blocks_and_proofs { + for (block_index, (block, storage_proof)) in blocks_and_proofs.into_iter().enumerate() { let inherent_data = extract_parachain_inherent_data(&block); validate_validation_data( @@ -238,10 +239,12 @@ where ); hrmp_watermark = crate::HrmpWatermark::::get(); - head_data = Some( - crate::CustomValidationHeadData::::get() - .map_or_else(|| HeadData(parent_header.encode()), HeadData), - ); + if block_index + 1 == num_blocks { + head_data = Some( + crate::CustomValidationHeadData::::get() + .map_or_else(|| HeadData(parent_header.encode()), HeadData), + ); + } }) } From 6f7667d33d72a000e57ae0bea4d327850775df83 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Fri, 18 Oct 2024 13:49:47 +0200 Subject: [PATCH 05/34] Handle versioning in collation generation --- cumulus/client/collator/src/service.rs | 26 +++-- cumulus/primitives/core/src/lib.rs | 64 ++--------- .../core/src/parachain_block_data.rs | 102 ++++++++++++++++++ 3 files changed, 130 insertions(+), 62 deletions(-) create mode 100644 cumulus/primitives/core/src/parachain_block_data.rs diff --git a/cumulus/client/collator/src/service.rs b/cumulus/client/collator/src/service.rs index a121ac226a5b..c6365184f0a9 100644 --- a/cumulus/client/collator/src/service.rs +++ b/cumulus/client/collator/src/service.rs @@ -175,13 +175,14 @@ where /// Fetch the collation info from the runtime. /// - /// Returns `Ok(Some(_))` on success, `Err(_)` on error or `Ok(None)` if the runtime api isn't - /// implemented by the runtime. + /// Returns `Ok(Some((CollationInfo, ApiVersion)))` on success, `Err(_)` on error or `Ok(None)` + /// if the runtime api isn't implemented by the runtime. `ApiVersion` being the version of the + /// [`CollectCollectionInfo`] runtime api. pub fn fetch_collation_info( &self, block_hash: Block::Hash, header: &Block::Header, - ) -> Result, sp_api::ApiError> { + ) -> Result, sp_api::ApiError> { let runtime_api = self.runtime_api.runtime_api(); let api_version = @@ -205,7 +206,7 @@ where runtime_api.collect_collation_info(block_hash, header)? }; - Ok(Some(collation_info)) + Ok(Some((collation_info, api_version))) } /// Build a full [`Collation`] from a given [`ParachainCandidate`]. This requires @@ -233,7 +234,7 @@ where }; // Create the parachain block data for the validators. - let collation_info = self + let (collation_info, api_version) = self .fetch_collation_info(block_hash, block.header()) .map_err(|e| { tracing::error!( @@ -248,7 +249,20 @@ where let block_data = ParachainBlockData::::new(vec![(block, compact_proof)]); let pov = polkadot_node_primitives::maybe_compress_pov(PoV { - block_data: BlockData(block_data.encode()), + block_data: BlockData(if api_version >= 3 { + block_data.encode() + } else { + let block_data = block_data.as_v0(); + + if block_data.is_none() { + tracing::error!( + target: LOG_TARGET, + "Trying to submit a collation with multiple blocks is not supported by the current runtime." + ); + } + + block_data?.encode() + }), }); let upward_messages = collation_info diff --git a/cumulus/primitives/core/src/lib.rs b/cumulus/primitives/core/src/lib.rs index ba46179a3ec0..04c68d847b1f 100644 --- a/cumulus/primitives/core/src/lib.rs +++ b/cumulus/primitives/core/src/lib.rs @@ -26,6 +26,9 @@ use polkadot_parachain_primitives::primitives::HeadData; use scale_info::TypeInfo; use sp_runtime::RuntimeDebug; +pub mod parachain_block_data; + +pub use parachain_block_data::ParachainBlockData; pub use polkadot_core_primitives::InboundDownwardMessage; pub use polkadot_parachain_primitives::primitives::{ DmpMessageHandler, Id as ParaId, IsSystem, UpwardMessage, ValidationParams, XcmpMessageFormat, @@ -35,14 +38,11 @@ pub use polkadot_primitives::{ vstaging::{ClaimQueueOffset, CoreSelector}, AbridgedHostConfiguration, AbridgedHrmpChannel, PersistedValidationData, }; - pub use sp_runtime::{ generic::{Digest, DigestItem}, traits::Block as BlockT, ConsensusEngineId, }; - -use sp_trie::CompactProof; pub use xcm::latest::prelude::*; /// A module that re-exports relevant relay chain definitions. @@ -197,58 +197,6 @@ pub enum ServiceQuality { Fast, } -/// The parachain block that is created by a collator. -/// -/// This is send as PoV (proof of validity block) to the relay-chain validators. There it will be -/// passed to the parachain validation Wasm blob to be validated. -#[derive(codec::Encode, codec::Decode, Clone)] -pub struct ParachainBlockData { - blocks: Vec<(Block, CompactProof)>, -} - -impl ParachainBlockData { - /// Creates a new instance of `Self`. - pub fn new(blocks: Vec<(Block, CompactProof)>) -> Self { - Self { blocks } - } - - /// Returns an iterator yielding references to the stored blocks. - pub fn blocks(&self) -> impl Iterator { - self.blocks.iter().map(|e| &e.0) - } - - /// Returns an iterator yielding mutable references to the stored blocks. - pub fn blocks_mut(&mut self) -> impl Iterator { - self.blocks.iter_mut().map(|e| &mut e.0) - } - - /// Returns an iterator yielding the stored blocks. - pub fn into_blocks(self) -> impl Iterator { - self.blocks.into_iter().map(|d| d.0) - } - - /// Returns an iterator yielding references to the stored proofs. - pub fn proofs(&self) -> impl Iterator { - self.blocks.iter().map(|d| &d.1) - } - - /// Deconstruct into the inner parts. - pub fn into_inner(self) -> Vec<(Block, CompactProof)> { - self.blocks - } - - /// Log the size of the individual components (header, extrinsics, storage proof) as info. - pub fn log_size_info(&self) { - tracing::info!( - target: "cumulus", - "PoV size {{ header: {}kb, extrinsics: {}kb, storage_proof: {}kb }}", - self.blocks().map(|b| b.header().encoded_size()).sum::() as f64 / 1024f64, - self.blocks().map(|b| b.extrinsics().encoded_size()).sum::() as f64 / 1024f64, - self.proofs().map(|p| p.encoded_size()).sum::() as f64 / 1024f64, - ); - } -} - /// A consensus engine ID indicating that this is a Cumulus Parachain. pub const CUMULUS_CONSENSUS_ID: ConsensusEngineId = *b"CMLS"; @@ -387,7 +335,11 @@ pub struct CollationInfo { sp_api::decl_runtime_apis! { /// Runtime api to collect information about a collation. - #[api_version(2)] + /// + /// Version history: + /// - Version 2: Changed [`Self::collect_collation_info`] signature + /// - Version 3: Signals to the node to use version 1 of [`ParachainBlockData`]. + #[api_version(3)] pub trait CollectCollationInfo { /// Collect information about a collation. #[changed_in(2)] diff --git a/cumulus/primitives/core/src/parachain_block_data.rs b/cumulus/primitives/core/src/parachain_block_data.rs new file mode 100644 index 000000000000..acf3a51b3e57 --- /dev/null +++ b/cumulus/primitives/core/src/parachain_block_data.rs @@ -0,0 +1,102 @@ +// Copyright (C) Parity Technologies (UK) Ltd. +// This file is part of Cumulus. + +// Cumulus is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// Cumulus is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with Cumulus. If not, see . + +//! Provides [`ParachainBlockData`] and its historical versions. + +use alloc::vec::Vec; +use codec::Encode; +use sp_runtime::traits::Block as BlockT; +use sp_trie::CompactProof; + +pub mod v0 { + use super::*; + + #[derive(codec::Encode, codec::Decode, Clone)] + pub struct ParachainBlockData { + /// The header of the parachain block. + pub header: B::Header, + /// The extrinsics of the parachain block. + pub extrinsics: alloc::vec::Vec, + /// The data that is required to emulate the storage accesses executed by all extrinsics. + pub storage_proof: sp_trie::CompactProof, + } +} + +/// The parachain block that is created by a collator. +/// +/// This is send as PoV (proof of validity block) to the relay-chain validators. There it will be +/// passed to the parachain validation Wasm blob to be validated. +#[derive(codec::Encode, codec::Decode, Clone)] +pub struct ParachainBlockData { + blocks: Vec<(Block, CompactProof)>, +} + +impl ParachainBlockData { + /// Creates a new instance of `Self`. + pub fn new(blocks: Vec<(Block, CompactProof)>) -> Self { + Self { blocks } + } + + /// Returns an iterator yielding references to the stored blocks. + pub fn blocks(&self) -> impl Iterator { + self.blocks.iter().map(|e| &e.0) + } + + /// Returns an iterator yielding mutable references to the stored blocks. + pub fn blocks_mut(&mut self) -> impl Iterator { + self.blocks.iter_mut().map(|e| &mut e.0) + } + + /// Returns an iterator yielding the stored blocks. + pub fn into_blocks(self) -> impl Iterator { + self.blocks.into_iter().map(|d| d.0) + } + + /// Returns an iterator yielding references to the stored proofs. + pub fn proofs(&self) -> impl Iterator { + self.blocks.iter().map(|d| &d.1) + } + + /// Deconstruct into the inner parts. + pub fn into_inner(self) -> Vec<(Block, CompactProof)> { + self.blocks + } + + /// Log the size of the individual components (header, extrinsics, storage proof) as info. + pub fn log_size_info(&self) { + tracing::info!( + target: "cumulus", + "PoV size {{ header: {}kb, extrinsics: {}kb, storage_proof: {}kb }}", + self.blocks().map(|b| b.header().encoded_size()).sum::() as f64 / 1024f64, + self.blocks().map(|b| b.extrinsics().encoded_size()).sum::() as f64 / 1024f64, + self.proofs().map(|p| p.encoded_size()).sum::() as f64 / 1024f64, + ); + } + + /// Converts into [`v0::ParachainBlockData`]. + /// + /// Returns `None` if there is not exactly one block. + pub fn as_v0(&self) -> Option> { + if self.blocks.len() != 1 { + return None + } + + self.blocks.first().map(|(block, storage_proof)| { + let (header, extrinsics) = block.clone().deconstruct(); + v0::ParachainBlockData { header, extrinsics, storage_proof: storage_proof.clone() } + }) + } +} From a8a64bc0dd942bb3588b2346b62d3a5f1a5e0cc2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Fri, 18 Oct 2024 23:24:09 +0200 Subject: [PATCH 06/34] Write tests for pov-recovery --- cumulus/client/pov-recovery/src/lib.rs | 70 ++++++----- cumulus/client/pov-recovery/src/tests.rs | 118 +++++++++++++++++- .../core/src/parachain_block_data.rs | 9 ++ cumulus/test/runtime/build.rs | 4 +- 4 files changed, 167 insertions(+), 34 deletions(-) diff --git a/cumulus/client/pov-recovery/src/lib.rs b/cumulus/client/pov-recovery/src/lib.rs index 2ec68eb5195a..3acd4408b2ed 100644 --- a/cumulus/client/pov-recovery/src/lib.rs +++ b/cumulus/client/pov-recovery/src/lib.rs @@ -62,7 +62,7 @@ use polkadot_primitives::{ use cumulus_primitives_core::ParachainBlockData; use cumulus_relay_chain_interface::{RelayChainInterface, RelayChainResult}; -use codec::Decode; +use codec::{Decode, DecodeAll}; use futures::{ channel::mpsc::Receiver, select, stream::FuturesUnordered, Future, FutureExt, Stream, StreamExt, }; @@ -349,6 +349,43 @@ where self.clear_waiting_recovery(&hash); } + /// Try to decode [`ParachainBlockData`] from `data`. + /// + /// Internally it will handle the decoding of the different versions. + fn decode_parachain_block_data( + data: &[u8], + expected_block_hash: Block::Hash, + ) -> Option> { + if let Ok(block_data) = ParachainBlockData::::decode_all(&mut &data[..]) { + if block_data.blocks().last().map_or(false, |b| b.hash() == expected_block_hash) { + return Some(block_data) + } + + tracing::debug!( + target: LOG_TARGET, + ?expected_block_hash, + "Could not find the expected block hash as latest block in `ParachainBlockData`" + ); + } + + if let Ok(block_data) = + cumulus_primitives_core::parachain_block_data::v0::ParachainBlockData::::decode_all( + &mut &data[..], + ) { + if block_data.header.hash() == expected_block_hash { + return Some(block_data.into()) + } + } + + tracing::warn!( + target: LOG_TARGET, + ?expected_block_hash, + "Could not decode `ParachainBlockData` from recovered PoV", + ); + + None + } + /// Handle a recovered candidate. async fn handle_candidate_recovered(&mut self, block_hash: Block::Hash, pov: Option<&PoV>) { let pov = match pov { @@ -384,37 +421,14 @@ where }, }; - let block_data = match ParachainBlockData::::decode(&mut &raw_block_data[..]) { - Ok(d) => d, - Err(error) => { - tracing::warn!( - target: LOG_TARGET, - ?error, - "Failed to decode parachain block data from recovered PoV", - ); - - self.reset_candidate(block_hash); - return - }, + let Some(block_data) = Self::decode_parachain_block_data(&raw_block_data, block_hash) + else { + self.reset_candidate(block_hash); + return }; let blocks_and_proofs = block_data.into_inner(); - if let Some((block, _)) = blocks_and_proofs.last() { - let last_block_hash = block.hash(); - if last_block_hash != block_hash { - tracing::debug!( - target: LOG_TARGET, - expected_block_hash = ?block_hash, - got_block_hash = ?last_block_hash, - "Recovered candidate doesn't contain the expected block.", - ); - - self.reset_candidate(block_hash); - return; - } - } - let Some(parent) = blocks_and_proofs.first().map(|(b, _)| *b.header().parent_hash()) else { tracing::debug!( target: LOG_TARGET, diff --git a/cumulus/client/pov-recovery/src/tests.rs b/cumulus/client/pov-recovery/src/tests.rs index b32731776a99..9568ccba5cb1 100644 --- a/cumulus/client/pov-recovery/src/tests.rs +++ b/cumulus/client/pov-recovery/src/tests.rs @@ -629,7 +629,10 @@ async fn pending_candidate_height_lower_than_latest_finalized() { #[case(RuntimeApiRequest::CANDIDATES_PENDING_AVAILABILITY_RUNTIME_REQUIREMENT)] #[case(10)] #[tokio::test] -async fn single_pending_candidate_recovery_success(#[case] runtime_version: u32) { +async fn single_pending_candidate_recovery_success( + #[case] runtime_version: u32, + #[values(true, false)] latest_block_data: bool, +) { sp_tracing::init_for_tests(); let (recovery_subsystem_tx, mut recovery_subsystem_rx) = @@ -687,9 +690,15 @@ async fn single_pending_candidate_recovery_success(#[case] runtime_version: u32) Ok( AvailableData { pov: Arc::new(PoV { - block_data: ParachainBlockData::::new( + block_data: if latest_block_data { ParachainBlockData::::new( vec![(Block::new(header.clone(), vec![]), CompactProof { encoded_nodes: vec![] })] - ).encode().into() + ).encode()} else { + cumulus_primitives_core::parachain_block_data::v0::ParachainBlockData:: { + header: header.clone(), + extrinsics: Vec::new(), + storage_proof: CompactProof { encoded_nodes: Vec::new() } + }.encode() + }.into() }), validation_data: dummy_pvd(), } @@ -1418,3 +1427,106 @@ async fn chained_recovery_child_succeeds_before_parent() { // No more import requests received assert_matches!(import_requests_rx.next().timeout(Duration::from_millis(100)).await, None); } + +#[tokio::test] +async fn recovery_multiple_blocks_per_candidate() { + sp_tracing::init_for_tests(); + + let (recovery_subsystem_tx, mut recovery_subsystem_rx) = + AvailabilityRecoverySubsystemHandle::new(); + let recovery_delay_range = + RecoveryDelayRange { min: Duration::from_millis(0), max: Duration::from_millis(0) }; + let (_explicit_recovery_chan_tx, explicit_recovery_chan_rx) = mpsc::channel(10); + let candidates = make_candidate_chain(1..4); + let candidate = candidates.last().clone().unwrap(); + let headers = candidates + .iter() + .map(|c| Header::decode(&mut &c.commitments.head_data.0[..]).unwrap()) + .collect::>(); + let header = headers.last().unwrap(); + + let relay_chain_client = Relaychain::new(vec![( + PHeader { + parent_hash: PHash::from_low_u64_be(0), + number: 1, + state_root: PHash::random(), + extrinsics_root: PHash::random(), + digest: Default::default(), + }, + vec![candidate.clone()], + )]); + let mut known_blocks = HashMap::new(); + known_blocks.insert(GENESIS_HASH, BlockStatus::InChainWithState); + let known_blocks = Arc::new(Mutex::new(known_blocks)); + let (parachain_client, import_notifications_tx, _finality_notifications_tx) = + ParachainClient::new(vec![dummy_usage_info(0)], known_blocks.clone()); + let (parachain_import_queue, mut import_requests_rx) = ParachainImportQueue::new(); + + let pov_recovery = PoVRecovery::::new( + Box::new(recovery_subsystem_tx), + recovery_delay_range, + Arc::new(parachain_client), + Box::new(parachain_import_queue), + relay_chain_client, + ParaId::new(1000), + explicit_recovery_chan_rx, + Arc::new(DummySyncOracle::default()), + ); + + task::spawn(pov_recovery.run()); + + // Candidates are recovered in the right order. + assert_matches!( + recovery_subsystem_rx.next().await, + Some(AvailabilityRecoveryMessage::RecoverAvailableData( + receipt, + session_index, + None, + None, + response_tx + )) => { + assert_eq!(receipt.hash(), candidate.hash()); + assert_eq!(session_index, TEST_SESSION_INDEX); + response_tx + .send(Ok(AvailableData { + pov: Arc::new(PoV { + block_data: ParachainBlockData::::new( + headers.iter().map(|h| (Block::new(h.clone(), vec![]), CompactProof { encoded_nodes: vec![] })).collect() + ) + .encode() + .into(), + }), + validation_data: dummy_pvd(), + })) + .unwrap(); + } + ); + + assert_matches!(import_requests_rx.next().await, Some(incoming_blocks) => { + assert_eq!(incoming_blocks.len(), 3); + assert_eq!(incoming_blocks.iter().map(|b| b.header.clone().unwrap()).collect::>(), headers); + }); + + known_blocks + .lock() + .expect("Poisoned lock") + .insert(header.hash(), BlockStatus::InChainWithState); + + let (unpin_sender, _unpin_receiver) = sc_utils::mpsc::tracing_unbounded("test_unpin", 10); + import_notifications_tx + .unbounded_send(BlockImportNotification::new( + header.hash(), + BlockOrigin::ConsensusBroadcast, + header.clone(), + false, + None, + unpin_sender, + )) + .unwrap(); + + // No more recovery messages received. + assert_matches!(recovery_subsystem_rx.next().timeout(Duration::from_millis(100)).await, None); + + // No more import requests received + assert_matches!(import_requests_rx.next().timeout(Duration::from_millis(100)).await, None); +} diff --git a/cumulus/primitives/core/src/parachain_block_data.rs b/cumulus/primitives/core/src/parachain_block_data.rs index acf3a51b3e57..00b24875567c 100644 --- a/cumulus/primitives/core/src/parachain_block_data.rs +++ b/cumulus/primitives/core/src/parachain_block_data.rs @@ -33,6 +33,15 @@ pub mod v0 { /// The data that is required to emulate the storage accesses executed by all extrinsics. pub storage_proof: sp_trie::CompactProof, } + + impl From> for super::ParachainBlockData { + fn from(block_data: ParachainBlockData) -> Self { + Self::new(alloc::vec![( + Block::new(block_data.header, block_data.extrinsics), + block_data.storage_proof, + )]) + } + } } /// The parachain block that is created by a collator. diff --git a/cumulus/test/runtime/build.rs b/cumulus/test/runtime/build.rs index 7a7fe8ffaa82..fa1f9eba2f58 100644 --- a/cumulus/test/runtime/build.rs +++ b/cumulus/test/runtime/build.rs @@ -25,10 +25,8 @@ fn main() { .set_file_name("wasm_binary_spec_version_incremented.rs") .build(); - WasmBuilder::new() - .with_current_project() + WasmBuilder::init_with_defaults() .enable_feature("elastic-scaling") - .import_memory() .set_file_name("wasm_binary_elastic_scaling.rs") .build(); } From 3c0f51a74963d8b83564841a20b90b3cabe37302 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Fri, 18 Oct 2024 23:39:24 +0200 Subject: [PATCH 07/34] Use enum to be future proof --- .../core/src/parachain_block_data.rs | 49 +++++++++++++------ 1 file changed, 34 insertions(+), 15 deletions(-) diff --git a/cumulus/primitives/core/src/parachain_block_data.rs b/cumulus/primitives/core/src/parachain_block_data.rs index 00b24875567c..482bc48a0cd3 100644 --- a/cumulus/primitives/core/src/parachain_block_data.rs +++ b/cumulus/primitives/core/src/parachain_block_data.rs @@ -49,39 +49,50 @@ pub mod v0 { /// This is send as PoV (proof of validity block) to the relay-chain validators. There it will be /// passed to the parachain validation Wasm blob to be validated. #[derive(codec::Encode, codec::Decode, Clone)] -pub struct ParachainBlockData { - blocks: Vec<(Block, CompactProof)>, +pub enum ParachainBlockData { + #[codec(index = 1)] + V1(Vec<(Block, CompactProof)>), } impl ParachainBlockData { /// Creates a new instance of `Self`. pub fn new(blocks: Vec<(Block, CompactProof)>) -> Self { - Self { blocks } + Self::V1(blocks) } /// Returns an iterator yielding references to the stored blocks. pub fn blocks(&self) -> impl Iterator { - self.blocks.iter().map(|e| &e.0) + match self { + Self::V1(blocks) => blocks.iter().map(|e| &e.0), + } } /// Returns an iterator yielding mutable references to the stored blocks. pub fn blocks_mut(&mut self) -> impl Iterator { - self.blocks.iter_mut().map(|e| &mut e.0) + match self { + Self::V1(blocks) => blocks.iter_mut().map(|e| &mut e.0), + } } /// Returns an iterator yielding the stored blocks. pub fn into_blocks(self) -> impl Iterator { - self.blocks.into_iter().map(|d| d.0) + match self { + Self::V1(blocks) => blocks.into_iter().map(|e| e.0), + } } /// Returns an iterator yielding references to the stored proofs. pub fn proofs(&self) -> impl Iterator { - self.blocks.iter().map(|d| &d.1) + match self { + Self::V1(blocks) => blocks.iter().map(|e| &e.1), + } } /// Deconstruct into the inner parts. pub fn into_inner(self) -> Vec<(Block, CompactProof)> { - self.blocks + match self { + Self::V1(blocks) => blocks, + } } /// Log the size of the individual components (header, extrinsics, storage proof) as info. @@ -99,13 +110,21 @@ impl ParachainBlockData { /// /// Returns `None` if there is not exactly one block. pub fn as_v0(&self) -> Option> { - if self.blocks.len() != 1 { - return None + match self { + Self::V1(blocks) => { + if blocks.len() != 1 { + return None + } + + blocks.first().map(|(block, storage_proof)| { + let (header, extrinsics) = block.clone().deconstruct(); + v0::ParachainBlockData { + header, + extrinsics, + storage_proof: storage_proof.clone(), + } + }) + }, } - - self.blocks.first().map(|(block, storage_proof)| { - let (header, extrinsics) = block.clone().deconstruct(); - v0::ParachainBlockData { header, extrinsics, storage_proof: storage_proof.clone() } - }) } } From acb64aeecf05497e43f92bfa92a10783095faa78 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Fri, 15 Nov 2024 16:12:56 +0100 Subject: [PATCH 08/34] Use scale --- cumulus/pallets/parachain-system/src/lib.rs | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/cumulus/pallets/parachain-system/src/lib.rs b/cumulus/pallets/parachain-system/src/lib.rs index 39fc8321a072..48d1388d7e59 100644 --- a/cumulus/pallets/parachain-system/src/lib.rs +++ b/cumulus/pallets/parachain-system/src/lib.rs @@ -55,7 +55,6 @@ use frame_system::{ensure_none, ensure_root, pallet_prelude::HeaderFor}; use polkadot_parachain_primitives::primitives::RelayChainBlockNumber; use polkadot_runtime_parachains::FeeTracker; use scale_info::TypeInfo; -use sp_core::U256; use sp_runtime::{ traits::{Block as BlockT, BlockNumberProvider, Hash, One}, BoundedSlice, FixedU128, RuntimeDebug, Saturating, @@ -204,15 +203,16 @@ pub struct DefaultCoreSelector(PhantomData); impl SelectCore for DefaultCoreSelector { fn selected_core() -> (CoreSelector, ClaimQueueOffset) { - let core_selector: U256 = frame_system::Pallet::::block_number().into(); + let core_selector = frame_system::Pallet::::block_number().using_encoded(|b| b[0]); - (CoreSelector(core_selector.byte(0)), ClaimQueueOffset(DEFAULT_CLAIM_QUEUE_OFFSET)) + (CoreSelector(core_selector), ClaimQueueOffset(DEFAULT_CLAIM_QUEUE_OFFSET)) } fn select_next_core() -> (CoreSelector, ClaimQueueOffset) { - let core_selector: U256 = (frame_system::Pallet::::block_number() + One::one()).into(); + let core_selector = + (frame_system::Pallet::::block_number() + One::one()).using_encoded(|b| b[0]); - (CoreSelector(core_selector.byte(0)), ClaimQueueOffset(DEFAULT_CLAIM_QUEUE_OFFSET)) + (CoreSelector(core_selector), ClaimQueueOffset(DEFAULT_CLAIM_QUEUE_OFFSET)) } } @@ -221,15 +221,16 @@ pub struct LookaheadCoreSelector(PhantomData); impl SelectCore for LookaheadCoreSelector { fn selected_core() -> (CoreSelector, ClaimQueueOffset) { - let core_selector: U256 = frame_system::Pallet::::block_number().into(); + let core_selector = frame_system::Pallet::::block_number().using_encoded(|b| b[0]); - (CoreSelector(core_selector.byte(0)), ClaimQueueOffset(1)) + (CoreSelector(core_selector), ClaimQueueOffset(1)) } fn select_next_core() -> (CoreSelector, ClaimQueueOffset) { - let core_selector: U256 = (frame_system::Pallet::::block_number() + One::one()).into(); + let core_selector = + (frame_system::Pallet::::block_number() + One::one()).using_encoded(|b| b[0]); - (CoreSelector(core_selector.byte(0)), ClaimQueueOffset(1)) + (CoreSelector(core_selector), ClaimQueueOffset(1)) } } From 70e7d50aed0a05eb8f36868197b3161b6cf42c00 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Fri, 15 Nov 2024 23:14:54 +0100 Subject: [PATCH 09/34] Handle UMPSignals --- .../src/validate_block/implementation.rs | 61 +++++++++++++++++-- polkadot/primitives/src/vstaging/mod.rs | 4 +- 2 files changed, 57 insertions(+), 8 deletions(-) diff --git a/cumulus/pallets/parachain-system/src/validate_block/implementation.rs b/cumulus/pallets/parachain-system/src/validate_block/implementation.rs index 802a8fc75097..eedabb44393f 100644 --- a/cumulus/pallets/parachain-system/src/validate_block/implementation.rs +++ b/cumulus/pallets/parachain-system/src/validate_block/implementation.rs @@ -27,8 +27,9 @@ use polkadot_parachain_primitives::primitives::{ }; use alloc::vec::Vec; -use codec::Encode; +use codec::{Decode, Encode}; +use cumulus_primitives_core::relay_chain::vstaging::{UMPSignal, UMP_SEPARATOR}; use frame_support::{ traits::{ExecuteBlock, ExtrinsicCall, Get, IsSubType}, BoundedVec, @@ -163,6 +164,7 @@ where let mut processed_downward_messages = 0; let mut upward_messages = BoundedVec::default(); + let mut upward_message_signals = Vec::>::new(); let mut horizontal_messages = BoundedVec::default(); let mut hrmp_watermark = Default::default(); let mut head_data = None; @@ -228,11 +230,33 @@ where new_validation_code = new_validation_code.take().or(crate::NewValidationCode::::get()); - upward_messages - .try_extend(crate::UpwardMessages::::get().into_iter()) - .expect( - "Number of upward messages should not be greater than `MAX_UPWARD_MESSAGE_NUM`", - ); + + let mut found_separator = false; + crate::UpwardMessages::::get() + .into_iter() + .filter_map(|m| { + if cfg!(feature = "experimental-ump-signals") { + if m == UMP_SEPARATOR { + found_separator = true; + None + } else if found_separator { + if upward_message_signals.iter().any(|s| *s != m) { + upward_message_signals.push(m); + } + None + } else { + Some(m) + } + } else { + Some(m) + } + }) + .for_each(|m| { + upward_messages.try_push(m) + .expect( + "Number of upward messages should not be greater than `MAX_UPWARD_MESSAGE_NUM`", + ) + }); processed_downward_messages += crate::ProcessedDownwardMessages::::get(); horizontal_messages.try_extend(crate::HrmpOutboundMessages::::get().into_iter()).expect( "Number of horizontal messages should not be greater than `MAX_HORIZONTAL_MESSAGE_NUM`", @@ -248,6 +272,31 @@ where }) } + if !upward_message_signals.is_empty() { + let mut selected_core = None; + + upward_message_signals.iter().for_each(|s| { + if let Ok(UMPSignal::SelectCore(selector, offset)) = UMPSignal::decode(&mut &s[..]) { + match &selected_core { + Some(selected_core) if *selected_core != (selector, offset) => { + panic!("All `SelectCore` signals need to select the same core") + }, + Some(_) => {}, + None => { + selected_core = Some((selector, offset)); + }, + } + } + }); + + upward_messages + .try_push(UMP_SEPARATOR) + .expect("UMPSignals does not fit in UMPMessages"); + upward_messages + .try_extend(upward_message_signals.into_iter()) + .expect("UMPSignals does not fit in UMPMessages"); + } + ValidationResult { head_data: head_data.expect("HeadData not set"), new_validation_code: new_validation_code.map(Into::into), diff --git a/polkadot/primitives/src/vstaging/mod.rs b/polkadot/primitives/src/vstaging/mod.rs index 271f78efe090..91ebae3e8448 100644 --- a/polkadot/primitives/src/vstaging/mod.rs +++ b/polkadot/primitives/src/vstaging/mod.rs @@ -416,11 +416,11 @@ impl From> for super::v8::CandidateReceipt { /// A strictly increasing sequence number, typically this would be the least significant byte of the /// block number. -#[derive(PartialEq, Eq, Clone, Encode, Decode, TypeInfo, RuntimeDebug)] +#[derive(PartialEq, Eq, Clone, Encode, Decode, TypeInfo, RuntimeDebug, Copy)] pub struct CoreSelector(pub u8); /// An offset in the relay chain claim queue. -#[derive(PartialEq, Eq, Clone, Encode, Decode, TypeInfo, RuntimeDebug)] +#[derive(PartialEq, Eq, Clone, Encode, Decode, TypeInfo, RuntimeDebug, Copy)] pub struct ClaimQueueOffset(pub u8); /// Signals that a parachain can send to the relay chain via the UMP queue. From e5277ffeb6df5d463baf2d4bd398a66b699e38a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Fri, 31 Jan 2025 23:18:09 +0100 Subject: [PATCH 10/34] Ensure the blocks match --- .../parachain-system/src/validate_block/implementation.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/cumulus/pallets/parachain-system/src/validate_block/implementation.rs b/cumulus/pallets/parachain-system/src/validate_block/implementation.rs index eedabb44393f..86f67e511435 100644 --- a/cumulus/pallets/parachain-system/src/validate_block/implementation.rs +++ b/cumulus/pallets/parachain-system/src/validate_block/implementation.rs @@ -181,6 +181,13 @@ where ¶chain_head, ); + assert_eq!( + parent_header.hash(), + *block.header().parent_hash(), + "Invalid parent header hash: {:?}", + block.header().hash() + ); + // Create the db let db = match storage_proof.to_memory_db(Some(parent_header.state_root())) { Ok((db, _)) => db, From 7f20108d8a9cf2144aff40fc29be9c68addbc0d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Fri, 21 Mar 2025 23:39:28 +0100 Subject: [PATCH 11/34] Only one proof --- cumulus/client/collator/src/service.rs | 2 +- .../consensus/aura/src/collators/basic.rs | 2 +- .../consensus/aura/src/collators/lookahead.rs | 2 +- .../collators/slot_based/collation_task.rs | 22 +- cumulus/client/pov-recovery/src/lib.rs | 13 +- .../src/validate_block/implementation.rs | 219 +++++++++--------- .../core/src/parachain_block_data.rs | 56 +++-- 7 files changed, 161 insertions(+), 155 deletions(-) diff --git a/cumulus/client/collator/src/service.rs b/cumulus/client/collator/src/service.rs index 44cdc895f921..41cfc6b866ec 100644 --- a/cumulus/client/collator/src/service.rs +++ b/cumulus/client/collator/src/service.rs @@ -247,7 +247,7 @@ where .ok() .flatten()?; - let block_data = ParachainBlockData::::new(vec![(block, compact_proof)]); + let block_data = ParachainBlockData::::new(vec![block], compact_proof); let pov = polkadot_node_primitives::maybe_compress_pov(PoV { block_data: BlockData(if api_version >= 3 { diff --git a/cumulus/client/consensus/aura/src/collators/basic.rs b/cumulus/client/consensus/aura/src/collators/basic.rs index 0ed140147f4e..72558a93c9f9 100644 --- a/cumulus/client/consensus/aura/src/collators/basic.rs +++ b/cumulus/client/consensus/aura/src/collators/basic.rs @@ -262,7 +262,7 @@ where ); if let Some((collation, block_data)) = maybe_collation { - let Some(block_hash) = block_data.blocks().nth(0).map(|b| b.hash()) else { + let Some(block_hash) = block_data.blocks().first().map(|b| b.hash()) else { continue }; let result_sender = diff --git a/cumulus/client/consensus/aura/src/collators/lookahead.rs b/cumulus/client/consensus/aura/src/collators/lookahead.rs index da52b5294139..5c98689a03d2 100644 --- a/cumulus/client/consensus/aura/src/collators/lookahead.rs +++ b/cumulus/client/consensus/aura/src/collators/lookahead.rs @@ -397,7 +397,7 @@ where { Ok(Some((collation, block_data))) => { let Some(new_block_header) = - block_data.blocks().nth(0).map(|b| b.header().clone()) + block_data.blocks().first().map(|b| b.header().clone()) else { tracing::error!(target: crate::LOG_TARGET, "Produced PoV doesn't contain any blocks"); break diff --git a/cumulus/client/consensus/aura/src/collators/slot_based/collation_task.rs b/cumulus/client/consensus/aura/src/collators/slot_based/collation_task.rs index 819d58c9599c..0414ebf2e118 100644 --- a/cumulus/client/consensus/aura/src/collators/slot_based/collation_task.rs +++ b/cumulus/client/consensus/aura/src/collators/slot_based/collation_task.rs @@ -147,16 +147,18 @@ async fn handle_collation_message( - pov_path.clone(), - pov.clone(), - block_data.header().hash(), - *block_data.header().number(), - parent_header.clone(), - relay_parent_header.state_root, - relay_parent_header.number, - max_pov_size, - ); + if let Some(header) = block_data.blocks().first().map(|b| b.header()) { + export_pov_to_path::( + pov_path.clone(), + pov.clone(), + header.hash(), + *header.number(), + parent_header.clone(), + relay_parent_header.state_root, + relay_parent_header.number, + max_pov_size, + ); + } } else { tracing::error!(target: LOG_TARGET, "Failed to get relay parent header from hash: {relay_parent:?}"); } diff --git a/cumulus/client/pov-recovery/src/lib.rs b/cumulus/client/pov-recovery/src/lib.rs index 4f708d8e1fdd..e6a0ddb8c537 100644 --- a/cumulus/client/pov-recovery/src/lib.rs +++ b/cumulus/client/pov-recovery/src/lib.rs @@ -1,6 +1,5 @@ // Copyright (C) Parity Technologies (UK) Ltd. // This file is part of Cumulus. -// SPDX-License-Identifier: GPL-3.0-or-later WITH Classpath-exception-2.0 // Cumulus is free software: you can redistribute it and/or modify // it under the terms of the GNU General Public License as published by @@ -9,11 +8,11 @@ // Cumulus is distributed in the hope that it will be useful, // but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the // GNU General Public License for more details. // You should have received a copy of the GNU General Public License -// along with Cumulus. If not, see . +// along with Cumulus. If not, see . //! Parachain PoV recovery //! @@ -432,9 +431,9 @@ where return }; - let blocks_and_proofs = block_data.into_inner(); + let blocks = block_data.into_blocks(); - let Some(parent) = blocks_and_proofs.first().map(|(b, _)| *b.header().parent_hash()) else { + let Some(parent) = blocks.first().map(|b| *b.header().parent_hash()) else { tracing::debug!( target: LOG_TARGET, ?block_hash, @@ -461,7 +460,7 @@ where "Waiting for recovery of parent.", ); - blocks_and_proofs.into_iter().for_each(|(b, _)| { + blocks.into_iter().for_each(|b| { self.waiting_for_parent .entry(*b.header().parent_hash()) .or_default() @@ -495,7 +494,7 @@ where _ => (), } - self.import_blocks(blocks_and_proofs.into_iter().map(|d| d.0)); + self.import_blocks(blocks.into_iter()); } /// Import the given `blocks`. diff --git a/cumulus/pallets/parachain-system/src/validate_block/implementation.rs b/cumulus/pallets/parachain-system/src/validate_block/implementation.rs index 3b3f1a1dca8a..f25de4a69cf8 100644 --- a/cumulus/pallets/parachain-system/src/validate_block/implementation.rs +++ b/cumulus/pallets/parachain-system/src/validate_block/implementation.rs @@ -1,18 +1,18 @@ // Copyright (C) Parity Technologies (UK) Ltd. // This file is part of Cumulus. -// SPDX-License-Identifier: Apache-2.0 - -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. + +// Cumulus is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// Cumulus is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with Cumulus. If not, see . //! The actual implementation of the validate block functionality. @@ -38,6 +38,7 @@ 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, ExtrinsicLike, HashingFor, Header as HeaderT}; +use sp_state_machine::OverlayedChanges; use sp_trie::{MemoryDB, ProofSizeProvider}; use trie_recorder::SizeOnlyRecorderProvider; @@ -149,13 +150,12 @@ where let mut parent_header = codec::decode_from_bytes::(parachain_head.clone()).expect("Invalid parent head"); - let blocks_and_proofs = block_data.into_inner(); + let (blocks, proof) = block_data.into_inner(); assert_eq!( - *blocks_and_proofs + *blocks .first() .expect("BlockData should have at least one block") - .0 .header() .parent_hash(), parent_header.hash(), @@ -169,9 +169,35 @@ where let mut hrmp_watermark = Default::default(); let mut head_data = None; let mut new_validation_code = None; - let num_blocks = blocks_and_proofs.len(); - - for (block_index, (block, storage_proof)) in blocks_and_proofs.into_iter().enumerate() { + let num_blocks = blocks.len(); + + // Create the db + let mut db = match proof.to_memory_db(Some(parent_header.state_root())) { + Ok((db, _)) => db, + Err(_) => panic!("Compact proof decoding failure."), + }; + + core::mem::drop(proof); + + // We use the same recorder across all blocks. Each node only contributed once to the total size + // of the storage proof. + let mut recorder = SizeOnlyRecorderProvider::new(); + let cache_provider = trie_cache::CacheProvider::new(); + // 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_with_cache( + db, + *parent_header.state_root(), + cache_provider, + ) + .with_recorder(recorder.clone()) + .build(); + + // We let all blocks contribute to the same overlay. Data written by a previous block will be + // directly accessible without going to the db. + let mut overlay = OverlayedChanges::default(); + + for (block_index, block) in blocks.into_iter().enumerate() { let inherent_data = extract_parachain_inherent_data(&block); validate_validation_data( @@ -181,102 +207,86 @@ where ¶chain_head, ); - assert_eq!( - parent_header.hash(), - *block.header().parent_hash(), - "Invalid parent header hash: {:?}", - block.header().hash() - ); + run_with_externalities_and_recorder::( + &backend, + &mut recorder, + &mut Default::default(), + || { + let relay_chain_proof = crate::RelayChainStateProof::new( + PSC::SelfParaId::get(), + inherent_data.validation_data.relay_parent_storage_root, + inherent_data.relay_chain_state.clone(), + ) + .expect("Invalid relay chain state proof"); + + #[allow(deprecated)] + let res = CI::check_inherents(&block, &relay_chain_proof); + + if !res.ok() { + if log::log_enabled!(log::Level::Error) { + res.into_errors().for_each(|e| { + log::error!("Checking inherent with identifier `{:?}` failed", e.0) + }); + } - // Create the db - let db = match storage_proof.to_memory_db(Some(parent_header.state_root())) { - Ok((db, _)) => db, - Err(_) => panic!("Compact proof decoding failure."), - }; - - core::mem::drop(storage_proof); - - let mut recorder = SizeOnlyRecorderProvider::new(); - let cache_provider = trie_cache::CacheProvider::new(); - // 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_with_cache( - db, - *parent_header.state_root(), - cache_provider, - ) - .with_recorder(recorder.clone()) - .build(); - - run_with_externalities_and_recorder::(&backend, &mut recorder, || { - let relay_chain_proof = crate::RelayChainStateProof::new( - PSC::SelfParaId::get(), - inherent_data.validation_data.relay_parent_storage_root, - inherent_data.relay_chain_state.clone(), - ) - .expect("Invalid relay chain state proof"); - - #[allow(deprecated)] - let res = CI::check_inherents(&block, &relay_chain_proof); - - if !res.ok() { - if log::log_enabled!(log::Level::Error) { - res.into_errors().for_each(|e| { - log::error!("Checking inherent with identifier `{:?}` failed", e.0) - }); + panic!("Checking inherents failed"); } + }, + ); - panic!("Checking inherents failed"); - } - }); - - run_with_externalities_and_recorder::(&backend, &mut recorder, || { - parent_header = block.header().clone(); - - E::execute_block(block); - - new_validation_code = - new_validation_code.take().or(crate::NewValidationCode::::get()); - - let mut found_separator = false; - crate::UpwardMessages::::get() - .into_iter() - .filter_map(|m| { - if cfg!(feature = "experimental-ump-signals") { - if m == UMP_SEPARATOR { - found_separator = true; - None - } else if found_separator { - if upward_message_signals.iter().any(|s| *s != m) { - upward_message_signals.push(m); + run_with_externalities_and_recorder::( + &backend, + &mut recorder, + &mut overlay, + || { + parent_header = block.header().clone(); + + E::execute_block(block); + + new_validation_code = + new_validation_code.take().or(crate::NewValidationCode::::get()); + + let mut found_separator = false; + crate::UpwardMessages::::get() + .into_iter() + .filter_map(|m| { + if cfg!(feature = "experimental-ump-signals") { + if m == UMP_SEPARATOR { + found_separator = true; + None + } else if found_separator { + if upward_message_signals.iter().any(|s| *s != m) { + upward_message_signals.push(m); + } + None + } else { + Some(m) } - None } else { Some(m) } - } else { - Some(m) - } - }) - .for_each(|m| { - upward_messages.try_push(m) + }) + .for_each(|m| { + upward_messages.try_push(m) .expect( "Number of upward messages should not be greater than `MAX_UPWARD_MESSAGE_NUM`", ) - }); - processed_downward_messages += crate::ProcessedDownwardMessages::::get(); - horizontal_messages.try_extend(crate::HrmpOutboundMessages::::get().into_iter()).expect( + }); + + processed_downward_messages += crate::ProcessedDownwardMessages::::get(); + horizontal_messages.try_extend(crate::HrmpOutboundMessages::::get().into_iter()).expect( "Number of horizontal messages should not be greater than `MAX_HORIZONTAL_MESSAGE_NUM`", ); - hrmp_watermark = crate::HrmpWatermark::::get(); + hrmp_watermark = crate::HrmpWatermark::::get(); - if block_index + 1 == num_blocks { - head_data = Some( - crate::CustomValidationHeadData::::get() - .map_or_else(|| HeadData(parent_header.encode()), HeadData), - ); - } - }) + if block_index + 1 == num_blocks { + head_data = Some( + crate::CustomValidationHeadData::::get() + .map_or_else(|| HeadData(parent_header.encode()), HeadData), + ); + } + }, + ) } if !upward_message_signals.is_empty() { @@ -357,11 +367,10 @@ fn validate_validation_data( fn run_with_externalities_and_recorder R>( backend: &TrieBackend, recorder: &mut SizeOnlyRecorderProvider>, + overlay: &mut OverlayedChanges>, execute: F, ) -> R { - let mut overlay = sp_state_machine::OverlayedChanges::default(); - let mut ext = Ext::::new(&mut overlay, backend); - recorder.reset(); + let mut ext = Ext::::new(overlay, backend); recorder::using(recorder, || set_and_run_with_externalities(&mut ext, || execute())) } diff --git a/cumulus/primitives/core/src/parachain_block_data.rs b/cumulus/primitives/core/src/parachain_block_data.rs index 482bc48a0cd3..ce9901778f40 100644 --- a/cumulus/primitives/core/src/parachain_block_data.rs +++ b/cumulus/primitives/core/src/parachain_block_data.rs @@ -36,10 +36,10 @@ pub mod v0 { impl From> for super::ParachainBlockData { fn from(block_data: ParachainBlockData) -> Self { - Self::new(alloc::vec![( - Block::new(block_data.header, block_data.extrinsics), + Self::new( + alloc::vec![Block::new(block_data.header, block_data.extrinsics)], block_data.storage_proof, - )]) + ) } } } @@ -51,47 +51,47 @@ pub mod v0 { #[derive(codec::Encode, codec::Decode, Clone)] pub enum ParachainBlockData { #[codec(index = 1)] - V1(Vec<(Block, CompactProof)>), + V1 { blocks: Vec, proof: CompactProof }, } impl ParachainBlockData { /// Creates a new instance of `Self`. - pub fn new(blocks: Vec<(Block, CompactProof)>) -> Self { - Self::V1(blocks) + pub fn new(blocks: Vec, proof: CompactProof) -> Self { + Self::V1 { blocks, proof } } - /// Returns an iterator yielding references to the stored blocks. - pub fn blocks(&self) -> impl Iterator { + /// Returns references to the stored blocks. + pub fn blocks(&self) -> &[Block] { match self { - Self::V1(blocks) => blocks.iter().map(|e| &e.0), + Self::V1 { blocks, .. } => &blocks, } } - /// Returns an iterator yielding mutable references to the stored blocks. - pub fn blocks_mut(&mut self) -> impl Iterator { + /// Returns mutable references to the stored blocks. + pub fn blocks_mut(&mut self) -> &mut [Block] { match self { - Self::V1(blocks) => blocks.iter_mut().map(|e| &mut e.0), + Self::V1 { ref mut blocks, .. } => blocks, } } - /// Returns an iterator yielding the stored blocks. - pub fn into_blocks(self) -> impl Iterator { + /// Returns the stored blocks. + pub fn into_blocks(self) -> Vec { match self { - Self::V1(blocks) => blocks.into_iter().map(|e| e.0), + Self::V1 { blocks, .. } => blocks, } } - /// Returns an iterator yielding references to the stored proofs. - pub fn proofs(&self) -> impl Iterator { + /// Returns a reference to the stored proof. + pub fn proof(&self) -> &CompactProof { match self { - Self::V1(blocks) => blocks.iter().map(|e| &e.1), + Self::V1 { proof, .. } => proof, } } /// Deconstruct into the inner parts. - pub fn into_inner(self) -> Vec<(Block, CompactProof)> { + pub fn into_inner(self) -> (Vec, CompactProof) { match self { - Self::V1(blocks) => blocks, + Self::V1 { blocks, proof } => (blocks, proof), } } @@ -100,9 +100,9 @@ impl ParachainBlockData { tracing::info!( target: "cumulus", "PoV size {{ header: {}kb, extrinsics: {}kb, storage_proof: {}kb }}", - self.blocks().map(|b| b.header().encoded_size()).sum::() as f64 / 1024f64, - self.blocks().map(|b| b.extrinsics().encoded_size()).sum::() as f64 / 1024f64, - self.proofs().map(|p| p.encoded_size()).sum::() as f64 / 1024f64, + self.blocks().iter().map(|b| b.header().encoded_size()).sum::() as f64 / 1024f64, + self.blocks().iter().map(|b| b.extrinsics().encoded_size()).sum::() as f64 / 1024f64, + self.proof().encoded_size() as f64 / 1024f64, ); } @@ -111,18 +111,14 @@ impl ParachainBlockData { /// Returns `None` if there is not exactly one block. pub fn as_v0(&self) -> Option> { match self { - Self::V1(blocks) => { + Self::V1 { blocks, proof } => { if blocks.len() != 1 { return None } - blocks.first().map(|(block, storage_proof)| { + blocks.first().map(|block| { let (header, extrinsics) = block.clone().deconstruct(); - v0::ParachainBlockData { - header, - extrinsics, - storage_proof: storage_proof.clone(), - } + v0::ParachainBlockData { header, extrinsics, storage_proof: proof.clone() } }) }, } From ce343a6062f803d5d097e402777eb91926d894f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Sun, 23 Mar 2025 22:59:47 +0100 Subject: [PATCH 12/34] Fix warnings --- .../parachain-system/src/validate_block/implementation.rs | 2 +- .../parachain-system/src/validate_block/trie_recorder.rs | 7 ------- 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/cumulus/pallets/parachain-system/src/validate_block/implementation.rs b/cumulus/pallets/parachain-system/src/validate_block/implementation.rs index f25de4a69cf8..5fc086587e59 100644 --- a/cumulus/pallets/parachain-system/src/validate_block/implementation.rs +++ b/cumulus/pallets/parachain-system/src/validate_block/implementation.rs @@ -172,7 +172,7 @@ where let num_blocks = blocks.len(); // Create the db - let mut db = match proof.to_memory_db(Some(parent_header.state_root())) { + let db = match proof.to_memory_db(Some(parent_header.state_root())) { Ok((db, _)) => db, Err(_) => panic!("Compact proof decoding failure."), }; diff --git a/cumulus/pallets/parachain-system/src/validate_block/trie_recorder.rs b/cumulus/pallets/parachain-system/src/validate_block/trie_recorder.rs index 09acedf1d983..c03c8c272cef 100644 --- a/cumulus/pallets/parachain-system/src/validate_block/trie_recorder.rs +++ b/cumulus/pallets/parachain-system/src/validate_block/trie_recorder.rs @@ -105,13 +105,6 @@ impl SizeOnlyRecorderProvider { recorded_keys: Default::default(), } } - - /// Reset the internal state. - pub fn reset(&self) { - self.seen_nodes.borrow_mut().clear(); - *self.encoded_size.borrow_mut() = 0; - self.recorded_keys.borrow_mut().clear(); - } } impl sp_trie::TrieRecorderProvider for SizeOnlyRecorderProvider { From 44ab2887f8130d47432538dfc3605cf54d4f7a16 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Mon, 24 Mar 2025 22:07:51 +0100 Subject: [PATCH 13/34] Fix compilation errors --- cumulus/client/collator/src/lib.rs | 4 +- cumulus/client/pov-recovery/src/tests.rs | 27 +++++----- .../src/validate_block/tests.rs | 54 ++++++++----------- .../src/validate_block/trie_recorder.rs | 3 -- cumulus/test/client/src/block_builder.rs | 2 +- cumulus/test/client/src/lib.rs | 10 ++-- 6 files changed, 45 insertions(+), 55 deletions(-) diff --git a/cumulus/client/collator/src/lib.rs b/cumulus/client/collator/src/lib.rs index e45e7d3cb1d7..9cd08bb06c3a 100644 --- a/cumulus/client/collator/src/lib.rs +++ b/cumulus/client/collator/src/lib.rs @@ -454,10 +454,10 @@ mod tests { let block = ParachainBlockData::::decode(&mut &decompressed[..]).expect("Is a valid block"); - assert_eq!(1, *block.blocks().nth(0).unwrap().header().number()); + assert_eq!(1, *block.blocks()[0].header().number()); // Ensure that we did not include `:code` in the proof. - let proof = block.proofs().nth(0).unwrap().clone(); + let proof = block.proof().clone(); let backend = sp_state_machine::create_proof_check_backend::( *header.state_root(), diff --git a/cumulus/client/pov-recovery/src/tests.rs b/cumulus/client/pov-recovery/src/tests.rs index 53362a312130..0a3d13dc0e41 100644 --- a/cumulus/client/pov-recovery/src/tests.rs +++ b/cumulus/client/pov-recovery/src/tests.rs @@ -697,7 +697,7 @@ async fn single_pending_candidate_recovery_success( AvailableData { pov: Arc::new(PoV { block_data: if latest_block_data { ParachainBlockData::::new( - vec![(Block::new(header.clone(), vec![]), CompactProof { encoded_nodes: vec![] })] + vec![Block::new(header.clone(), vec![])], CompactProof { encoded_nodes: vec![] } ).encode()} else { cumulus_primitives_core::parachain_block_data::v0::ParachainBlockData:: { header: header.clone(), @@ -805,7 +805,7 @@ async fn single_pending_candidate_recovery_retry_succeeds() { AvailableData { pov: Arc::new(PoV { block_data: ParachainBlockData::::new( - vec![(Block::new(header.clone(), Vec::new()), CompactProof { encoded_nodes: vec![] })] + vec![Block::new(header.clone(), Vec::new())], CompactProof { encoded_nodes: vec![] } ).encode().into() }), validation_data: dummy_pvd(), @@ -1109,10 +1109,10 @@ async fn candidate_is_imported_while_awaiting_recovery() { recovery_response_tx .send(Ok(AvailableData { pov: Arc::new(PoV { - block_data: ParachainBlockData::::new(vec![( - Block::new(header.clone(), vec![]), + block_data: ParachainBlockData::::new( + vec![Block::new(header.clone(), vec![])], CompactProof { encoded_nodes: vec![] }, - )]) + ) .encode() .into(), }), @@ -1206,10 +1206,10 @@ async fn candidate_is_finalized_while_awaiting_recovery() { recovery_response_tx .send(Ok(AvailableData { pov: Arc::new(PoV { - block_data: ParachainBlockData::::new(vec![( - Block::new(header.clone(), vec![]), + block_data: ParachainBlockData::::new( + vec![Block::new(header.clone(), vec![])], CompactProof { encoded_nodes: vec![] }, - )]) + ) .encode() .into(), }), @@ -1294,7 +1294,7 @@ async fn chained_recovery_success() { .send(Ok(AvailableData { pov: Arc::new(PoV { block_data: ParachainBlockData::::new( - vec![(Block::new(header.clone(), vec![]), CompactProof { encoded_nodes: vec![] })] + vec![Block::new(header.clone(), vec![])], CompactProof { encoded_nodes: vec![] } ) .encode() .into(), @@ -1408,10 +1408,10 @@ async fn chained_recovery_child_succeeds_before_parent() { recovery_response_sender .send(Ok(AvailableData { pov: Arc::new(PoV { - block_data: ParachainBlockData::::new(vec![( - Block::new(header.clone(), vec![]), + block_data: ParachainBlockData::::new( + vec![Block::new(header.clone(), vec![])], CompactProof { encoded_nodes: vec![] }, - )]) + ) .encode() .into(), }), @@ -1497,7 +1497,8 @@ async fn recovery_multiple_blocks_per_candidate() { .send(Ok(AvailableData { pov: Arc::new(PoV { block_data: ParachainBlockData::::new( - headers.iter().map(|h| (Block::new(h.clone(), vec![]), CompactProof { encoded_nodes: vec![] })).collect() + headers.iter().map(|h| Block::new(h.clone(), vec![])).collect(), + CompactProof { encoded_nodes: vec![] }, ) .encode() .into(), diff --git a/cumulus/pallets/parachain-system/src/validate_block/tests.rs b/cumulus/pallets/parachain-system/src/validate_block/tests.rs index dc5d56a26d3e..16a087eddcde 100644 --- a/cumulus/pallets/parachain-system/src/validate_block/tests.rs +++ b/cumulus/pallets/parachain-system/src/validate_block/tests.rs @@ -167,6 +167,8 @@ fn build_multiple_blocks_with_witness( let mut persisted_validation_data = None; let mut blocks = Vec::new(); + //TODO: Fix this, not correct. + let mut proof = None; for _ in 0..num_blocks { let cumulus_test_client::BlockBuilderAndSupportData { @@ -176,24 +178,21 @@ fn build_multiple_blocks_with_witness( persisted_validation_data = Some(p_v_data); - blocks.extend( - block_builder - .build_parachain_block(*parent_head.state_root()) - .into_inner() - .into_iter() - .inspect(|d| { - futures::executor::block_on( - client.import_as_best(BlockOrigin::Own, d.0.clone()), - ) - .unwrap(); - - parent_head = d.0.header.clone(); - }), - ); + let (build_blocks, build_proof) = + block_builder.build_parachain_block(*parent_head.state_root()).into_inner(); + + proof.get_or_insert_with(|| build_proof); + + blocks.extend(build_blocks.into_iter().inspect(|b| { + futures::executor::block_on(client.import_as_best(BlockOrigin::Own, b.clone())) + .unwrap(); + + parent_head = b.header.clone(); + })); } TestBlockData { - block: ParachainBlockData::new(blocks), + block: ParachainBlockData::new(blocks, proof.unwrap()), validation_data: persisted_validation_data.unwrap(), } } @@ -207,7 +206,7 @@ fn validate_block_works() { build_block_with_witness(&client, Vec::new(), parent_head.clone(), Default::default()); let block = seal_block(block, &client); - let header = block.blocks().nth(0).unwrap().header().clone(); + let header = block.blocks()[0].header().clone(); let res_header = call_validate_block(parent_head, block, validation_data.relay_parent_storage_root) .expect("Calls `validate_block`"); @@ -215,6 +214,7 @@ fn validate_block_works() { } #[test] +#[ignore = "Needs another pr to work"] fn validate_multiple_blocks_work() { sp_tracing::try_init_simple(); @@ -251,7 +251,7 @@ fn validate_block_with_extra_extrinsics() { Default::default(), ); let block = seal_block(block, &client); - let header = block.blocks().nth(0).unwrap().header().clone(); + let header = block.blocks()[0].header().clone(); let res_header = call_validate_block(parent_head, block, validation_data.relay_parent_storage_root) @@ -284,7 +284,7 @@ fn validate_block_returns_custom_head_data() { parent_head.clone(), Default::default(), ); - let header = block.blocks().nth(0).unwrap().header().clone(); + let header = block.blocks()[0].header().clone(); assert_ne!(expected_header, header.encode()); let block = seal_block(block, &client); @@ -306,12 +306,7 @@ fn validate_block_invalid_parent_hash() { let (client, parent_head) = create_test_client(); let TestBlockData { mut block, validation_data, .. } = build_block_with_witness(&client, Vec::new(), parent_head.clone(), Default::default()); - block - .blocks_mut() - .nth(0) - .unwrap() - .header - .set_parent_hash(Hash::from_low_u64_be(1)); + block.blocks_mut()[0].header.set_parent_hash(Hash::from_low_u64_be(1)); call_validate_block(parent_head, block, validation_data.relay_parent_storage_root) .unwrap_err(); @@ -361,12 +356,7 @@ fn check_inherents_are_unsigned_and_before_all_other_extrinsics() { let TestBlockData { mut block, validation_data, .. } = build_block_with_witness(&client, Vec::new(), parent_head.clone(), Default::default()); - block - .blocks_mut() - .nth(0) - .unwrap() - .extrinsics - .insert(0, transfer(&client, Alice, Bob, 69)); + block.blocks_mut()[0].extrinsics.insert(0, transfer(&client, Alice, Bob, 69)); call_validate_block(parent_head, block, validation_data.relay_parent_storage_root) .unwrap_err(); @@ -435,7 +425,7 @@ fn validate_block_works_with_child_tries() { Default::default(), ); - let block = block.blocks().nth(0).unwrap().clone(); + let block = block.blocks()[0].clone(); futures::executor::block_on(client.import(BlockOrigin::Own, block.clone())).unwrap(); @@ -449,7 +439,7 @@ fn validate_block_works_with_child_tries() { ); let block = seal_block(block, &client); - let header = block.blocks().nth(0).unwrap().header().clone(); + let header = block.blocks()[0].header().clone(); let res_header = call_validate_block(parent_head, block, validation_data.relay_parent_storage_root) .expect("Calls `validate_block`"); diff --git a/cumulus/pallets/parachain-system/src/validate_block/trie_recorder.rs b/cumulus/pallets/parachain-system/src/validate_block/trie_recorder.rs index c03c8c272cef..ec748d90e826 100644 --- a/cumulus/pallets/parachain-system/src/validate_block/trie_recorder.rs +++ b/cumulus/pallets/parachain-system/src/validate_block/trie_recorder.rs @@ -285,9 +285,6 @@ mod tests { reference_recorder.estimate_encoded_size(), recorder_for_test.estimate_encoded_size() ); - - recorder_for_test.reset(); - assert_eq!(recorder_for_test.estimate_encoded_size(), 0) } } } diff --git a/cumulus/test/client/src/block_builder.rs b/cumulus/test/client/src/block_builder.rs index 75b8d6932cdf..63796a665c7d 100644 --- a/cumulus/test/client/src/block_builder.rs +++ b/cumulus/test/client/src/block_builder.rs @@ -198,6 +198,6 @@ impl<'a> BuildParachainBlockData for sc_block_builder::BlockBuilder<'a, Block, C .into_compact_proof::<
::Hashing>(parent_state_root) .expect("Creates the compact proof"); - ParachainBlockData::new(vec![(built_block.block, storage_proof)]) + ParachainBlockData::new(vec![built_block.block], storage_proof) } } diff --git a/cumulus/test/client/src/lib.rs b/cumulus/test/client/src/lib.rs index 054b63de3c73..580f7d21f0f6 100644 --- a/cumulus/test/client/src/lib.rs +++ b/cumulus/test/client/src/lib.rs @@ -240,11 +240,12 @@ fn get_keystore() -> sp_keystore::KeystorePtr { /// Given parachain block data and a slot, seal the block with an aura seal. Assumes that the /// authorities of the test runtime are present in the keyring. pub fn seal_block(block: ParachainBlockData, client: &Client) -> ParachainBlockData { + let (blocks, proof) = block.into_inner(); + ParachainBlockData::new( - block - .into_inner() + blocks .into_iter() - .map(|(mut block, proof)| { + .map(|mut block| { let parachain_slot = find_pre_digest::::Signature>(&block.header) .unwrap(); @@ -263,8 +264,9 @@ pub fn seal_block(block: ParachainBlockData, client: &Client) -> ParachainBlockD .expect("Should be able to create seal"); block.header.digest_mut().push(seal_digest); - (block, proof) + block }) .collect::>(), + proof, ) } From 8e46176367b24f55ca65db2e26d37f5c89fc3642 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Mon, 24 Mar 2025 22:44:01 +0100 Subject: [PATCH 14/34] More fixes --- cumulus/test/service/benches/validate_block.rs | 12 ++++-------- .../test/service/benches/validate_block_glutton.rs | 10 +++++----- 2 files changed, 9 insertions(+), 13 deletions(-) diff --git a/cumulus/test/service/benches/validate_block.rs b/cumulus/test/service/benches/validate_block.rs index 0e5156653d24..ecfc824b571f 100644 --- a/cumulus/test/service/benches/validate_block.rs +++ b/cumulus/test/service/benches/validate_block.rs @@ -91,7 +91,8 @@ fn benchmark_block_validation(c: &mut Criterion) { let para_id = ParaId::from(cumulus_test_runtime::PARACHAIN_ID); let mut test_client_builder = TestClientBuilder::with_default_backend(); let genesis_init = test_client_builder.genesis_init_mut(); - *genesis_init = cumulus_test_client::GenesisParameters { endowed_accounts: account_ids }; + *genesis_init = + cumulus_test_client::GenesisParameters { endowed_accounts: account_ids, wasm: None }; let client = test_client_builder.build_with_native_executor(None).0; let (max_transfer_count, extrinsics) = create_extrinsics(&client, &src_accounts, &dst_accounts); @@ -119,8 +120,7 @@ fn benchmark_block_validation(c: &mut Criterion) { let parachain_block = block_builder.build_parachain_block(*parent_header.state_root()); - let proof_size_in_kb = - parachain_block.proofs().map(|p| p.encoded_size()).sum::() as f64 / 1024f64; + let proof_size_in_kb = parachain_block.proof().encoded_size() as f64 / 1024f64; let runtime = utils::get_wasm_module(); let (relay_parent_storage_root, _) = sproof_builder.into_state_root_and_proof(); @@ -135,11 +135,7 @@ fn benchmark_block_validation(c: &mut Criterion) { // This is not strictly necessary for this benchmark, but // let us make sure that the result of `validate_block` is what // we expect. - verify_expected_result( - &runtime, - &encoded_params, - parachain_block.blocks().nth(0).unwrap().clone(), - ); + verify_expected_result(&runtime, &encoded_params, parachain_block.blocks()[0].clone()); let mut group = c.benchmark_group("Block validation"); group.sample_size(20); diff --git a/cumulus/test/service/benches/validate_block_glutton.rs b/cumulus/test/service/benches/validate_block_glutton.rs index 05ed11e3d672..05c422f84dec 100644 --- a/cumulus/test/service/benches/validate_block_glutton.rs +++ b/cumulus/test/service/benches/validate_block_glutton.rs @@ -63,7 +63,7 @@ fn benchmark_block_validation(c: &mut Criterion) { let endowed_accounts = vec![AccountId::from(Alice.public())]; let mut test_client_builder = TestClientBuilder::with_default_backend(); let genesis_init = test_client_builder.genesis_init_mut(); - *genesis_init = cumulus_test_client::GenesisParameters { endowed_accounts }; + *genesis_init = cumulus_test_client::GenesisParameters { endowed_accounts, wasm: None }; let client = test_client_builder.build_with_native_executor(None).0; @@ -80,7 +80,7 @@ fn benchmark_block_validation(c: &mut Criterion) { runtime.block_on(import_block( &client, - parachain_block.blocks().nth(0).unwrap().clone(), + parachain_block.blocks()[0].clone(), false, )); @@ -97,10 +97,10 @@ fn benchmark_block_validation(c: &mut Criterion) { let parachain_block = block_builder.build_parachain_block(*parent_header.state_root()); let proof_size_in_kb = - parachain_block.proofs().map(|p| p.encoded_size()).sum::() as f64 / 1024f64; + parachain_block.proof().encoded_size() as f64 / 1024f64; runtime.block_on(import_block( &client, - parachain_block.blocks().nth(0).unwrap().clone(), + parachain_block.blocks()[0].clone(), false, )); let runtime = utils::get_wasm_module(); @@ -121,7 +121,7 @@ fn benchmark_block_validation(c: &mut Criterion) { verify_expected_result( &runtime, &encoded_params, - parachain_block.blocks().nth(0).unwrap().clone(), + parachain_block.blocks()[0].clone(), ); group.bench_function( From b0b041dbf15b0a38228889b567cd1ed3e5070577 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Mon, 24 Mar 2025 22:50:25 +0100 Subject: [PATCH 15/34] FMT.. --- .../service/benches/validate_block_glutton.rs | 21 ++++--------------- 1 file changed, 4 insertions(+), 17 deletions(-) diff --git a/cumulus/test/service/benches/validate_block_glutton.rs b/cumulus/test/service/benches/validate_block_glutton.rs index 05c422f84dec..06ad73996514 100644 --- a/cumulus/test/service/benches/validate_block_glutton.rs +++ b/cumulus/test/service/benches/validate_block_glutton.rs @@ -78,11 +78,7 @@ fn benchmark_block_validation(c: &mut Criterion) { set_glutton_parameters(&client, is_first, compute_ratio, storage_ratio); is_first = false; - runtime.block_on(import_block( - &client, - parachain_block.blocks()[0].clone(), - false, - )); + runtime.block_on(import_block(&client, parachain_block.blocks()[0].clone(), false)); // Build benchmark block let parent_hash = client.usage_info().chain.best_hash; @@ -96,13 +92,8 @@ fn benchmark_block_validation(c: &mut Criterion) { client.init_block_builder(Some(validation_data), Default::default()); let parachain_block = block_builder.build_parachain_block(*parent_header.state_root()); - let proof_size_in_kb = - parachain_block.proof().encoded_size() as f64 / 1024f64; - runtime.block_on(import_block( - &client, - parachain_block.blocks()[0].clone(), - false, - )); + let proof_size_in_kb = parachain_block.proof().encoded_size() as f64 / 1024f64; + runtime.block_on(import_block(&client, parachain_block.blocks()[0].clone(), false)); let runtime = utils::get_wasm_module(); let sproof_builder: RelayStateSproofBuilder = Default::default(); @@ -118,11 +109,7 @@ fn benchmark_block_validation(c: &mut Criterion) { // This is not strictly necessary for this benchmark, but // let us make sure that the result of `validate_block` is what // we expect. - verify_expected_result( - &runtime, - &encoded_params, - parachain_block.blocks()[0].clone(), - ); + verify_expected_result(&runtime, &encoded_params, parachain_block.blocks()[0].clone()); group.bench_function( format!( From b6cfcda9821b9021627e7257b6b7b0fd2f5adbe9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Mon, 24 Mar 2025 22:50:47 +0100 Subject: [PATCH 16/34] More --- cumulus/bin/pov-validator/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cumulus/bin/pov-validator/Cargo.toml b/cumulus/bin/pov-validator/Cargo.toml index d7af29a6bcb2..a919e3f68eac 100644 --- a/cumulus/bin/pov-validator/Cargo.toml +++ b/cumulus/bin/pov-validator/Cargo.toml @@ -19,8 +19,8 @@ sc-executor.workspace = true sp-core.workspace = true sp-io.workspace = true sp-maybe-compressed-blob.workspace = true -tracing-subscriber.workspace = true tracing.workspace = true +tracing-subscriber.workspace = true [lints] workspace = true From 05096afaf6c2fe0196fbcd8396e8c8196b5740fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Tue, 25 Mar 2025 09:33:38 +0100 Subject: [PATCH 17/34] MIGHTY CLIPPY ACCEPT MY SACRIFICE --- cumulus/client/pov-recovery/src/tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cumulus/client/pov-recovery/src/tests.rs b/cumulus/client/pov-recovery/src/tests.rs index 0a3d13dc0e41..78884691cc38 100644 --- a/cumulus/client/pov-recovery/src/tests.rs +++ b/cumulus/client/pov-recovery/src/tests.rs @@ -1444,7 +1444,7 @@ async fn recovery_multiple_blocks_per_candidate() { RecoveryDelayRange { min: Duration::from_millis(0), max: Duration::from_millis(0) }; let (_explicit_recovery_chan_tx, explicit_recovery_chan_rx) = mpsc::channel(10); let candidates = make_candidate_chain(1..4); - let candidate = candidates.last().clone().unwrap(); + let candidate = candidates.last().unwrap(); let headers = candidates .iter() .map(|c| Header::decode(&mut &c.commitments.head_data.0[..]).unwrap()) From fa0889883f424856cc2fb7f8acd54285678932a7 Mon Sep 17 00:00:00 2001 From: "cmd[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue, 25 Mar 2025 08:38:46 +0000 Subject: [PATCH 18/34] Update from github-actions[bot] running command 'prdoc --bump major --audience node_dev' --- prdoc/pr_6137.prdoc | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 prdoc/pr_6137.prdoc diff --git a/prdoc/pr_6137.prdoc b/prdoc/pr_6137.prdoc new file mode 100644 index 000000000000..5f7a40f13766 --- /dev/null +++ b/prdoc/pr_6137.prdoc @@ -0,0 +1,22 @@ +title: 'cumulus: `ParachainBlockData` support multiple blocks' +doc: +- audience: Node Dev + description: |- + This pull request adds support to `ParachainBlockData` to support multiple blocks at once. This basically means that cumulus based Parachains could start packaging multiple blocks into one `PoV`. From the relay chain POV nothing changes and these `PoV`s appear like any other `PoV`. Internally this `PoV` then executes the blocks sequentially. However, all these blocks together can use the same amount of resources like a single `PoV`. This pull request is basically a preparation to support running parachains with a faster block time than the relay chain. + + This breaks the encoding of `ParachainBlockData`. It requires that the collators upgrade first before the runtime requiring the new `ParachainBlockData` is enacted. The collators will decide based on the api version of `CollectCollationInfo`, which `ParachainBlockData` format they will send to the relay chain so that the validation code can interpret it correctly. +crates: +- name: cumulus-client-collator + bump: major +- name: cumulus-client-consensus-aura + bump: major +- name: cumulus-client-pov-recovery + bump: major +- name: cumulus-pallet-parachain-system + bump: major +- name: cumulus-primitives-core + bump: major +- name: polkadot-primitives + bump: major +- name: cumulus-pov-validator + bump: major From 4f5ffb35fe545fe4d1f745d30ddb6ed618d25c86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Tue, 25 Mar 2025 10:51:05 +0100 Subject: [PATCH 19/34] Fix doc issue --- cumulus/client/collator/src/service.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cumulus/client/collator/src/service.rs b/cumulus/client/collator/src/service.rs index 41cfc6b866ec..921f1890f783 100644 --- a/cumulus/client/collator/src/service.rs +++ b/cumulus/client/collator/src/service.rs @@ -178,7 +178,7 @@ where /// /// Returns `Ok(Some((CollationInfo, ApiVersion)))` on success, `Err(_)` on error or `Ok(None)` /// if the runtime api isn't implemented by the runtime. `ApiVersion` being the version of the - /// [`CollectCollectionInfo`] runtime api. + /// [`CollectCollationInfo`] runtime api. pub fn fetch_collation_info( &self, block_hash: Block::Hash, From 99f814c7588b75de50bd3d25af4ba0bdab97c485 Mon Sep 17 00:00:00 2001 From: "cmd[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue, 25 Mar 2025 10:06:12 +0000 Subject: [PATCH 20/34] Update from github-actions[bot] running command 'fmt' --- cumulus/bin/pov-validator/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cumulus/bin/pov-validator/Cargo.toml b/cumulus/bin/pov-validator/Cargo.toml index a919e3f68eac..d7af29a6bcb2 100644 --- a/cumulus/bin/pov-validator/Cargo.toml +++ b/cumulus/bin/pov-validator/Cargo.toml @@ -19,8 +19,8 @@ sc-executor.workspace = true sp-core.workspace = true sp-io.workspace = true sp-maybe-compressed-blob.workspace = true -tracing.workspace = true tracing-subscriber.workspace = true +tracing.workspace = true [lints] workspace = true From 146f29de32b725017463c92fb0a6b3efda498824 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Tue, 25 Mar 2025 11:07:32 +0100 Subject: [PATCH 21/34] Fix some issues --- cumulus/client/pov-recovery/src/lib.rs | 5 ++-- .../src/validate_block/implementation.rs | 26 +++++++++---------- .../core/src/parachain_block_data.rs | 26 +++++++++---------- prdoc/pr_6137.prdoc | 8 ++++-- 4 files changed, 35 insertions(+), 30 deletions(-) diff --git a/cumulus/client/pov-recovery/src/lib.rs b/cumulus/client/pov-recovery/src/lib.rs index e6a0ddb8c537..b9ad5b3f91dc 100644 --- a/cumulus/client/pov-recovery/src/lib.rs +++ b/cumulus/client/pov-recovery/src/lib.rs @@ -1,5 +1,6 @@ // Copyright (C) Parity Technologies (UK) Ltd. // This file is part of Cumulus. +// SPDX-License-Identifier: GPL-3.0-or-later WITH Classpath-exception-2.0 // Cumulus is free software: you can redistribute it and/or modify // it under the terms of the GNU General Public License as published by @@ -8,11 +9,11 @@ // Cumulus is distributed in the hope that it will be useful, // but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the // GNU General Public License for more details. // You should have received a copy of the GNU General Public License -// along with Cumulus. If not, see . +// along with Cumulus. If not, see . //! Parachain PoV recovery //! diff --git a/cumulus/pallets/parachain-system/src/validate_block/implementation.rs b/cumulus/pallets/parachain-system/src/validate_block/implementation.rs index 5fc086587e59..74a6f23809be 100644 --- a/cumulus/pallets/parachain-system/src/validate_block/implementation.rs +++ b/cumulus/pallets/parachain-system/src/validate_block/implementation.rs @@ -1,18 +1,18 @@ // Copyright (C) Parity Technologies (UK) Ltd. // This file is part of Cumulus. - -// Cumulus is free software: you can redistribute it and/or modify -// it under the terms of the GNU General Public License as published by -// the Free Software Foundation, either version 3 of the License, or -// (at your option) any later version. - -// Cumulus is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. - -// You should have received a copy of the GNU General Public License -// along with Cumulus. If not, see . +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. //! The actual implementation of the validate block functionality. diff --git a/cumulus/primitives/core/src/parachain_block_data.rs b/cumulus/primitives/core/src/parachain_block_data.rs index ce9901778f40..7e56124922da 100644 --- a/cumulus/primitives/core/src/parachain_block_data.rs +++ b/cumulus/primitives/core/src/parachain_block_data.rs @@ -1,18 +1,18 @@ // Copyright (C) Parity Technologies (UK) Ltd. // This file is part of Cumulus. - -// Cumulus is free software: you can redistribute it and/or modify -// it under the terms of the GNU General Public License as published by -// the Free Software Foundation, either version 3 of the License, or -// (at your option) any later version. - -// Cumulus is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. - -// You should have received a copy of the GNU General Public License -// along with Cumulus. If not, see . +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. //! Provides [`ParachainBlockData`] and its historical versions. diff --git a/prdoc/pr_6137.prdoc b/prdoc/pr_6137.prdoc index 5f7a40f13766..60f88966042c 100644 --- a/prdoc/pr_6137.prdoc +++ b/prdoc/pr_6137.prdoc @@ -2,9 +2,13 @@ title: 'cumulus: `ParachainBlockData` support multiple blocks' doc: - audience: Node Dev description: |- - This pull request adds support to `ParachainBlockData` to support multiple blocks at once. This basically means that cumulus based Parachains could start packaging multiple blocks into one `PoV`. From the relay chain POV nothing changes and these `PoV`s appear like any other `PoV`. Internally this `PoV` then executes the blocks sequentially. However, all these blocks together can use the same amount of resources like a single `PoV`. This pull request is basically a preparation to support running parachains with a faster block time than the relay chain. + This pull request adds support to `ParachainBlockData` to support multiple blocks at once. This basically means that cumulus based Parachains could start packaging multiple blocks into one `PoV`. + From the relay chain PoV nothing changes and these `PoV`s appear like any other `PoV`. Internally this `PoV` then executes the blocks sequentially. However, all these blocks together can use the same amount of resources like a single `PoV`. + This pull request is basically a preparation to support running parachains with a faster block time than the relay chain. + + This breaks the encoding of `ParachainBlockData`. It requires that the collators upgrade first before the runtime requiring the new `ParachainBlockData` is enacted. + The collators will decide based on the api version of `CollectCollationInfo`, which `ParachainBlockData` format they will send to the relay chain so that the validation code can interpret it correctly. - This breaks the encoding of `ParachainBlockData`. It requires that the collators upgrade first before the runtime requiring the new `ParachainBlockData` is enacted. The collators will decide based on the api version of `CollectCollationInfo`, which `ParachainBlockData` format they will send to the relay chain so that the validation code can interpret it correctly. crates: - name: cumulus-client-collator bump: major From 06dba39529727e21977287b371f946cc78b24868 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Tue, 25 Mar 2025 22:37:48 +0100 Subject: [PATCH 22/34] Fix bug --- .../src/validate_block/implementation.rs | 12 ++-- .../src/validate_block/tests.rs | 60 ++++++++++++++++--- polkadot/node/collation-generation/src/lib.rs | 11 +++- .../node/core/candidate-validation/src/lib.rs | 16 ++++- 4 files changed, 80 insertions(+), 19 deletions(-) diff --git a/cumulus/pallets/parachain-system/src/validate_block/implementation.rs b/cumulus/pallets/parachain-system/src/validate_block/implementation.rs index 74a6f23809be..81e60a891b03 100644 --- a/cumulus/pallets/parachain-system/src/validate_block/implementation.rs +++ b/cumulus/pallets/parachain-system/src/validate_block/implementation.rs @@ -255,7 +255,7 @@ where found_separator = true; None } else if found_separator { - if upward_message_signals.iter().any(|s| *s != m) { + if upward_message_signals.iter().all(|s| *s != m) { upward_message_signals.push(m); } None @@ -268,15 +268,15 @@ where }) .for_each(|m| { upward_messages.try_push(m) - .expect( - "Number of upward messages should not be greater than `MAX_UPWARD_MESSAGE_NUM`", - ) + .expect( + "Number of upward messages should not be greater than `MAX_UPWARD_MESSAGE_NUM`", + ) }); processed_downward_messages += crate::ProcessedDownwardMessages::::get(); horizontal_messages.try_extend(crate::HrmpOutboundMessages::::get().into_iter()).expect( - "Number of horizontal messages should not be greater than `MAX_HORIZONTAL_MESSAGE_NUM`", - ); + "Number of horizontal messages should not be greater than `MAX_HORIZONTAL_MESSAGE_NUM`", + ); hrmp_watermark = crate::HrmpWatermark::::get(); if block_index + 1 == num_blocks { diff --git a/cumulus/pallets/parachain-system/src/validate_block/tests.rs b/cumulus/pallets/parachain-system/src/validate_block/tests.rs index 16a087eddcde..4d9abcc2b39f 100644 --- a/cumulus/pallets/parachain-system/src/validate_block/tests.rs +++ b/cumulus/pallets/parachain-system/src/validate_block/tests.rs @@ -14,6 +14,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +use crate::*; use codec::{Decode, DecodeAll, Encode}; use cumulus_primitives_core::{ParachainBlockData, PersistedValidationData}; use cumulus_test_client::{ @@ -27,18 +28,21 @@ use cumulus_test_client::{ TestClientBuilder, TestClientBuilderExt, ValidationParams, }; use cumulus_test_relay_sproof_builder::RelayStateSproofBuilder; +use polkadot_parachain_primitives::primitives::ValidationResult; +#[cfg(feature = "experimental-ump-signals")] +use relay_chain::vstaging::{UMPSignal, UMP_SEPARATOR}; use sp_runtime::traits::{Block as BlockT, Header as HeaderT}; use std::{env, process::Command}; use crate::validate_block::MemoryOptimizedValidationParams; -fn call_validate_block_encoded_header( +fn call_validate_block_validation_result( validation_code: &[u8], parent_head: Header, block_data: ParachainBlockData, relay_parent_storage_root: Hash, -) -> cumulus_test_client::ExecutorResult> { +) -> cumulus_test_client::ExecutorResult { cumulus_test_client::validate_block( ValidationParams { block_data: BlockData(block_data.encode()), @@ -48,7 +52,6 @@ fn call_validate_block_encoded_header( }, validation_code, ) - .map(|v| v.head_data.0) } fn call_validate_block( @@ -56,13 +59,13 @@ fn call_validate_block( block_data: ParachainBlockData, relay_parent_storage_root: Hash, ) -> cumulus_test_client::ExecutorResult
{ - call_validate_block_encoded_header( + call_validate_block_validation_result( WASM_BINARY.expect("You need to build the WASM binaries to run the tests!"), parent_head, block_data, relay_parent_storage_root, ) - .map(|v| Header::decode(&mut &v[..]).expect("Decodes `Header`.")) + .map(|v| Header::decode(&mut &v.head_data.0[..]).expect("Decodes `Header`.")) } /// Call `validate_block` in the runtime with `elastic-scaling` activated. @@ -71,14 +74,14 @@ fn call_validate_block_elastic_scaling( block_data: ParachainBlockData, relay_parent_storage_root: Hash, ) -> cumulus_test_client::ExecutorResult
{ - call_validate_block_encoded_header( + call_validate_block_validation_result( test_runtime::elastic_scaling::WASM_BINARY .expect("You need to build the WASM binaries to run the tests!"), parent_head, block_data, relay_parent_storage_root, ) - .map(|v| Header::decode(&mut &v[..]).expect("Decodes `Header`.")) + .map(|v| Header::decode(&mut &v.head_data.0[..]).expect("Decodes `Header`.")) } fn create_test_client() -> (Client, Header) { @@ -288,13 +291,15 @@ fn validate_block_returns_custom_head_data() { assert_ne!(expected_header, header.encode()); let block = seal_block(block, &client); - let res_header = call_validate_block_encoded_header( + let res_header = call_validate_block_validation_result( WASM_BINARY.expect("You need to build the WASM binaries to run the tests!"), parent_head, block, validation_data.relay_parent_storage_root, ) - .expect("Calls `validate_block`"); + .expect("Calls `validate_block`") + .head_data + .0; assert_eq!(expected_header, res_header); } @@ -445,3 +450,40 @@ fn validate_block_works_with_child_tries() { .expect("Calls `validate_block`"); assert_eq!(header, res_header); } + +#[test] +#[cfg(feature = "experimental-ump-signals")] +fn validate_block_handles_ump_signal() { + sp_tracing::try_init_simple(); + + let (client, parent_head) = create_elastic_scaling_test_client(); + let extra_extrinsics = + vec![transfer(&client, Alice, Bob, 69), transfer(&client, Bob, Charlie, 100)]; + + let TestBlockData { block, validation_data } = build_block_with_witness( + &client, + extra_extrinsics, + parent_head.clone(), + Default::default(), + ); + + let block = seal_block(block, &client); + let upward_messages = call_validate_block_validation_result( + test_runtime::elastic_scaling::WASM_BINARY + .expect("You need to build the WASM binaries to run the tests!"), + parent_head, + block, + validation_data.relay_parent_storage_root, + ) + .expect("Calls `validate_block`") + .upward_messages; + + assert_eq!( + upward_messages, + vec![ + UMP_SEPARATOR, + UMPSignal::SelectCore(CoreSelector(1), ClaimQueueOffset(DEFAULT_CLAIM_QUEUE_OFFSET)) + .encode() + ] + ); +} diff --git a/polkadot/node/collation-generation/src/lib.rs b/polkadot/node/collation-generation/src/lib.rs index b4b3e009a6bd..fd672e7160b1 100644 --- a/polkadot/node/collation-generation/src/lib.rs +++ b/polkadot/node/collation-generation/src/lib.rs @@ -612,7 +612,7 @@ async fn construct_and_distribute_receipt( commitments.head_data.hash(), validation_code_hash, ), - commitments, + commitments: commitments.clone(), }; ccr.check_core_index(&transposed_claim_queue) @@ -653,8 +653,15 @@ async fn construct_and_distribute_receipt( ?relay_parent, para_id = %para_id, ?core_index, - "candidate is generated", + "Candidate generated", ); + gum::trace!( + target: LOG_TARGET, + ?commitments, + candidate_hash = ?receipt.hash(), + "Candidate commitments", + ); + metrics.on_collation_generated(); sender diff --git a/polkadot/node/core/candidate-validation/src/lib.rs b/polkadot/node/core/candidate-validation/src/lib.rs index a40db8439f3a..dec7cded89f9 100644 --- a/polkadot/node/core/candidate-validation/src/lib.rs +++ b/polkadot/node/core/candidate-validation/src/lib.rs @@ -865,10 +865,12 @@ async fn validate_candidate_exhaustive( let validation_code_hash = validation_code.hash(); let relay_parent = candidate_receipt.descriptor.relay_parent(); let para_id = candidate_receipt.descriptor.para_id(); + let candidate_hash = candidate_receipt.hash(); gum::debug!( target: LOG_TARGET, ?validation_code_hash, + ?candidate_hash, ?para_id, "About to validate a candidate.", ); @@ -888,7 +890,7 @@ async fn validate_candidate_exhaustive( &pov, &validation_code_hash, ) { - gum::info!(target: LOG_TARGET, ?para_id, "Invalid candidate (basic checks)"); + gum::debug!(target: LOG_TARGET, ?para_id, ?candidate_hash, "Invalid candidate (basic checks)"); return Ok(ValidationResult::Invalid(e)) } @@ -935,7 +937,7 @@ async fn validate_candidate_exhaustive( }; if let Err(ref error) = result { - gum::info!(target: LOG_TARGET, ?para_id, ?error, "Failed to validate candidate"); + gum::info!(target: LOG_TARGET, ?para_id, ?candidate_hash, ?error, "Failed to validate candidate"); } match result { @@ -943,6 +945,7 @@ async fn validate_candidate_exhaustive( gum::warn!( target: LOG_TARGET, ?para_id, + ?candidate_hash, ?e, "An internal error occurred during validation, will abstain from voting", ); @@ -1008,9 +1011,18 @@ async fn validate_candidate_exhaustive( gum::info!( target: LOG_TARGET, ?para_id, + ?candidate_hash, "Invalid candidate (commitments hash)" ); + gum::trace!( + target: LOG_TARGET, + ?para_id, + ?candidate_hash, + produced_commitments = ?committed_candidate_receipt.commitments, + "Invalid candidate commitments" + ); + // If validation produced a new set of commitments, we treat the candidate as // invalid. Ok(ValidationResult::Invalid(InvalidCandidate::CommitmentsHashMismatch)) From cedb8e268426aa4f2dad2a270d581e9a4903ce61 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Wed, 26 Mar 2025 15:47:39 +0100 Subject: [PATCH 23/34] Fix more zombienet tests --- cumulus/client/pov-recovery/src/lib.rs | 6 +++++- cumulus/zombienet/tests/0002-pov_recovery.zndsl | 12 ++++++------ .../zombienet/tests/0009-elastic_pov_recovery.zndsl | 2 +- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/cumulus/client/pov-recovery/src/lib.rs b/cumulus/client/pov-recovery/src/lib.rs index b9ad5b3f91dc..2ffff3a9d0d4 100644 --- a/cumulus/client/pov-recovery/src/lib.rs +++ b/cumulus/client/pov-recovery/src/lib.rs @@ -504,7 +504,11 @@ where fn import_blocks(&mut self, blocks: impl Iterator) { let mut blocks = VecDeque::from_iter(blocks); - tracing::trace!(target: LOG_TARGET, blocks = ?blocks.iter().map(|b| b.hash()), "Importing blocks retrieved using pov_recovery"); + tracing::trace!( + target: LOG_TARGET, + blocks = ?blocks.iter().map(|b| b.hash()), + "Importing blocks retrieved using pov_recovery", + ); let mut incoming_blocks = Vec::new(); diff --git a/cumulus/zombienet/tests/0002-pov_recovery.zndsl b/cumulus/zombienet/tests/0002-pov_recovery.zndsl index dc7095ced252..5cc615e70d20 100644 --- a/cumulus/zombienet/tests/0002-pov_recovery.zndsl +++ b/cumulus/zombienet/tests/0002-pov_recovery.zndsl @@ -19,9 +19,9 @@ two: reports block height is at least 20 within 800 seconds # three: reports block height is at least 20 within 800 seconds eve: reports block height is at least 20 within 800 seconds -one: count of log lines containing "Importing block retrieved using pov_recovery" is greater than 19 within 10 seconds -two: count of log lines containing "Importing block retrieved using pov_recovery" is greater than 19 within 10 seconds -three: count of log lines containing "Importing block retrieved using pov_recovery" is greater than 19 within 10 seconds -eve: count of log lines containing "Importing block retrieved using pov_recovery" is greater than 19 within 10 seconds -charlie: count of log lines containing "Importing block retrieved using pov_recovery" is greater than 19 within 10 seconds -alice: count of log lines containing "Importing block retrieved using pov_recovery" is greater than 19 within 10 seconds +one: count of log lines containing "Importing blocks retrieved using pov_recovery" is greater than 19 within 10 seconds +two: count of log lines containing "Importing blocks retrieved using pov_recovery" is greater than 19 within 10 seconds +three: count of log lines containing "Importing blocks retrieved using pov_recovery" is greater than 19 within 10 seconds +eve: count of log lines containing "Importing blocks retrieved using pov_recovery" is greater than 19 within 10 seconds +charlie: count of log lines containing "Importing blocks retrieved using pov_recovery" is greater than 19 within 10 seconds +alice: count of log lines containing "Importing blocks retrieved using pov_recovery" is greater than 19 within 10 seconds diff --git a/cumulus/zombienet/tests/0009-elastic_pov_recovery.zndsl b/cumulus/zombienet/tests/0009-elastic_pov_recovery.zndsl index 5cca6120ff3a..2614084a5a46 100644 --- a/cumulus/zombienet/tests/0009-elastic_pov_recovery.zndsl +++ b/cumulus/zombienet/tests/0009-elastic_pov_recovery.zndsl @@ -21,4 +21,4 @@ alice: parachain 2100 is registered within 300 seconds collator-elastic: reports block height is at least 40 within 225 seconds collator-elastic: count of log lines containing "set_validation_data inherent needs to be present in every block" is 0 within 10 seconds -recovery-target: count of log lines containing "Importing block retrieved using pov_recovery" is greater than 35 within 10 seconds +recovery-target: count of log lines containing "Importing blocks retrieved using pov_recovery" is greater than 35 within 10 seconds From d79ebcf70ec88a0d6b33305e325db6af30846eda Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Wed, 26 Mar 2025 22:42:16 +0100 Subject: [PATCH 24/34] Let's sleep longer --- .github/workflows/zombienet-reusable-preflight.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/zombienet-reusable-preflight.yml b/.github/workflows/zombienet-reusable-preflight.yml index 68c39cdebd95..2158fef4e4e5 100644 --- a/.github/workflows/zombienet-reusable-preflight.yml +++ b/.github/workflows/zombienet-reusable-preflight.yml @@ -245,7 +245,7 @@ jobs: echo "::warning::No CI workflow runs found for this commit" exit 1 fi - sleep 10 + sleep 60 done #check if the build succeeded From aae6db9c56c5947ac4f1da9b55736bc0d6d87cc2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Thu, 27 Mar 2025 12:27:59 +0100 Subject: [PATCH 25/34] Use debug for logging --- cumulus/client/pov-recovery/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cumulus/client/pov-recovery/src/lib.rs b/cumulus/client/pov-recovery/src/lib.rs index 2ffff3a9d0d4..e8f2e636249f 100644 --- a/cumulus/client/pov-recovery/src/lib.rs +++ b/cumulus/client/pov-recovery/src/lib.rs @@ -504,7 +504,7 @@ where fn import_blocks(&mut self, blocks: impl Iterator) { let mut blocks = VecDeque::from_iter(blocks); - tracing::trace!( + tracing::debug!( target: LOG_TARGET, blocks = ?blocks.iter().map(|b| b.hash()), "Importing blocks retrieved using pov_recovery", From 9f70dac4a6d85005b7085e7dafd0901dbe56d6af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Mon, 31 Mar 2025 21:33:23 +0200 Subject: [PATCH 26/34] Update cumulus/primitives/core/Cargo.toml Co-authored-by: Michal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com> --- cumulus/primitives/core/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cumulus/primitives/core/Cargo.toml b/cumulus/primitives/core/Cargo.toml index 4ebf8517281e..592026b82412 100644 --- a/cumulus/primitives/core/Cargo.toml +++ b/cumulus/primitives/core/Cargo.toml @@ -14,7 +14,7 @@ workspace = true [dependencies] codec = { features = ["derive"], workspace = true } scale-info = { features = ["derive"], workspace = true } -tracing.workspace = true +tracing = { workspace = true } # Substrate sp-api = { workspace = true } From d4904ecfaeb46baa1606bf888e8c6dd6916a9290 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Thu, 3 Apr 2025 22:11:29 +0200 Subject: [PATCH 27/34] Review comments --- .../src/validate_block/implementation.rs | 13 +++++++++---- cumulus/primitives/core/src/parachain_block_data.rs | 8 ++++---- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/cumulus/pallets/parachain-system/src/validate_block/implementation.rs b/cumulus/pallets/parachain-system/src/validate_block/implementation.rs index 81e60a891b03..225c0d7b5f55 100644 --- a/cumulus/pallets/parachain-system/src/validate_block/implementation.rs +++ b/cumulus/pallets/parachain-system/src/validate_block/implementation.rs @@ -250,6 +250,7 @@ where crate::UpwardMessages::::get() .into_iter() .filter_map(|m| { + // Filter out the `UMP_SEPARATOR` and the `UMPSignals`. if cfg!(feature = "experimental-ump-signals") { if m == UMP_SEPARATOR { found_separator = true; @@ -260,6 +261,7 @@ where } None } else { + // No signal or separator Some(m) } } else { @@ -293,16 +295,19 @@ where let mut selected_core = None; upward_message_signals.iter().for_each(|s| { - if let Ok(UMPSignal::SelectCore(selector, offset)) = UMPSignal::decode(&mut &s[..]) { - match &selected_core { + match UMPSignal::decode(&mut &s[..]).expect("Failed to decode `UMPSignal`") { + UMPSignal::SelectCore(selector, offset) => match &selected_core { Some(selected_core) if *selected_core != (selector, offset) => { - panic!("All `SelectCore` signals need to select the same core") + panic!( + "All `SelectCore` signals need to select the same core: {selected_core:?} vs {:?}", + (selector, offset), + ) }, Some(_) => {}, None => { selected_core = Some((selector, offset)); }, - } + }, } }); diff --git a/cumulus/primitives/core/src/parachain_block_data.rs b/cumulus/primitives/core/src/parachain_block_data.rs index 7e56124922da..e5357125e080 100644 --- a/cumulus/primitives/core/src/parachain_block_data.rs +++ b/cumulus/primitives/core/src/parachain_block_data.rs @@ -99,10 +99,10 @@ impl ParachainBlockData { pub fn log_size_info(&self) { tracing::info!( target: "cumulus", - "PoV size {{ header: {}kb, extrinsics: {}kb, storage_proof: {}kb }}", - self.blocks().iter().map(|b| b.header().encoded_size()).sum::() as f64 / 1024f64, - self.blocks().iter().map(|b| b.extrinsics().encoded_size()).sum::() as f64 / 1024f64, - self.proof().encoded_size() as f64 / 1024f64, + header_kb = %self.blocks().iter().map(|b| b.header().encoded_size()).sum::() as f64 / 1024f64, + extrinsics_kb = %self.blocks().iter().map(|b| b.extrinsics().encoded_size()).sum::() as f64 / 1024f64, + storage_proof_kb = %self.proof().encoded_size() as f64 / 1024f64, + "PoV size", ); } From 583efc253a50e8833de56771fb34f20ad0c3fead Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Fri, 4 Apr 2025 18:27:17 +0200 Subject: [PATCH 28/34] Make it backwards and forwards compatible --- cumulus/client/pov-recovery/src/lib.rs | 43 ++--- cumulus/client/pov-recovery/src/tests.rs | 19 +- .../core/src/parachain_block_data.rs | 182 +++++++++++++++--- prdoc/pr_6137.prdoc | 5 +- 4 files changed, 188 insertions(+), 61 deletions(-) diff --git a/cumulus/client/pov-recovery/src/lib.rs b/cumulus/client/pov-recovery/src/lib.rs index e8f2e636249f..089ad08367a7 100644 --- a/cumulus/client/pov-recovery/src/lib.rs +++ b/cumulus/client/pov-recovery/src/lib.rs @@ -361,33 +361,28 @@ where data: &[u8], expected_block_hash: Block::Hash, ) -> Option> { - if let Ok(block_data) = ParachainBlockData::::decode_all(&mut &data[..]) { - if block_data.blocks().last().map_or(false, |b| b.hash() == expected_block_hash) { - return Some(block_data) - } - - tracing::debug!( - target: LOG_TARGET, - ?expected_block_hash, - "Could not find the expected block hash as latest block in `ParachainBlockData`" - ); - } + match ParachainBlockData::::decode_all(&mut &data[..]) { + Ok(block_data) => { + if block_data.blocks().last().map_or(false, |b| b.hash() == expected_block_hash) { + return Some(block_data) + } - if let Ok(block_data) = - cumulus_primitives_core::parachain_block_data::v0::ParachainBlockData::::decode_all( - &mut &data[..], - ) { - if block_data.header.hash() == expected_block_hash { - return Some(block_data.into()) - } + tracing::debug!( + target: LOG_TARGET, + ?expected_block_hash, + "Could not find the expected block hash as latest block in `ParachainBlockData`" + ); + }, + Err(error) => { + tracing::debug!( + target: LOG_TARGET, + ?expected_block_hash, + ?error, + "Could not decode `ParachainBlockData` from recovered PoV", + ); + }, } - tracing::warn!( - target: LOG_TARGET, - ?expected_block_hash, - "Could not decode `ParachainBlockData` from recovered PoV", - ); - None } diff --git a/cumulus/client/pov-recovery/src/tests.rs b/cumulus/client/pov-recovery/src/tests.rs index 78884691cc38..e6f2e2e2e34e 100644 --- a/cumulus/client/pov-recovery/src/tests.rs +++ b/cumulus/client/pov-recovery/src/tests.rs @@ -692,19 +692,20 @@ async fn single_pending_candidate_recovery_success( )) => { assert_eq!(receipt.hash(), candidate_hash); assert_eq!(session_index, TEST_SESSION_INDEX); + let block_data = + ParachainBlockData::::new( + vec![Block::new(header.clone(), vec![])], CompactProof { encoded_nodes: vec![] } + ); + response_tx.send( Ok( AvailableData { pov: Arc::new(PoV { - block_data: if latest_block_data { ParachainBlockData::::new( - vec![Block::new(header.clone(), vec![])], CompactProof { encoded_nodes: vec![] } - ).encode()} else { - cumulus_primitives_core::parachain_block_data::v0::ParachainBlockData:: { - header: header.clone(), - extrinsics: Vec::new(), - storage_proof: CompactProof { encoded_nodes: Vec::new() } - }.encode() - }.into() + block_data: if latest_block_data { + block_data + } else { + block_data.as_v0().unwrap() + }.encode().into() }), validation_data: dummy_pvd(), } diff --git a/cumulus/primitives/core/src/parachain_block_data.rs b/cumulus/primitives/core/src/parachain_block_data.rs index e5357125e080..0b48a571acfc 100644 --- a/cumulus/primitives/core/src/parachain_block_data.rs +++ b/cumulus/primitives/core/src/parachain_block_data.rs @@ -17,29 +17,48 @@ //! Provides [`ParachainBlockData`] and its historical versions. use alloc::vec::Vec; -use codec::Encode; +use codec::{Decode, Encode}; use sp_runtime::traits::Block as BlockT; use sp_trie::CompactProof; -pub mod v0 { - use super::*; +/// Special prefix used by [`ParachainBlockData`] from version 1 and upwards to distinguish from the +/// unversioned legacy/v0 version. +const VERSIONED_PARACHAIN_BLOCK_DATA_PREFIX: &[u8] = b"VERSIONEDPBD"; - #[derive(codec::Encode, codec::Decode, Clone)] - pub struct ParachainBlockData { - /// The header of the parachain block. - pub header: B::Header, - /// The extrinsics of the parachain block. - pub extrinsics: alloc::vec::Vec, - /// The data that is required to emulate the storage accesses executed by all extrinsics. - pub storage_proof: sp_trie::CompactProof, +// Struct which allows prepending bytes after reading from an input. +pub(crate) struct PrependBytesInput<'a, I> { + prepend: &'a [u8], + read: usize, + inner: &'a mut I, +} + +impl<'a, I: codec::Input> codec::Input for PrependBytesInput<'a, I> { + fn remaining_len(&mut self) -> Result, codec::Error> { + let remaining_compact = self.prepend.len().saturating_sub(self.read); + Ok(self.inner.remaining_len()?.map(|len| len.saturating_add(remaining_compact))) } - impl From> for super::ParachainBlockData { - fn from(block_data: ParachainBlockData) -> Self { - Self::new( - alloc::vec![Block::new(block_data.header, block_data.extrinsics)], - block_data.storage_proof, - ) + fn read(&mut self, into: &mut [u8]) -> Result<(), codec::Error> { + if into.is_empty() { + return Ok(()); + } + + let remaining_compact = self.prepend.len().saturating_sub(self.read); + if remaining_compact > 0 { + let to_read = into.len().min(remaining_compact); + into[..to_read].copy_from_slice(&self.prepend[self.read..][..to_read]); + self.read += to_read; + + if to_read < into.len() { + // Buffer not full, keep reading the inner. + self.inner.read(&mut into[to_read..]) + } else { + // Buffer was filled by the bytes. + Ok(()) + } + } else { + // Prepended bytes has been read, just read from inner. + self.inner.read(into) } } } @@ -48,12 +67,54 @@ pub mod v0 { /// /// This is send as PoV (proof of validity block) to the relay-chain validators. There it will be /// passed to the parachain validation Wasm blob to be validated. -#[derive(codec::Encode, codec::Decode, Clone)] +#[derive(Clone)] pub enum ParachainBlockData { - #[codec(index = 1)] + V0 { block: [Block; 1], proof: CompactProof }, V1 { blocks: Vec, proof: CompactProof }, } +impl Encode for ParachainBlockData { + fn encode(&self) -> Vec { + match self { + Self::V0 { block, proof } => + (block[0].header(), block[0].extrinsics(), &proof).encode(), + Self::V1 { blocks, proof } => { + let mut res = VERSIONED_PARACHAIN_BLOCK_DATA_PREFIX.to_vec(); + 1u8.encode_to(&mut res); + blocks.encode_to(&mut res); + proof.encode_to(&mut res); + res + }, + } + } +} + +impl Decode for ParachainBlockData { + fn decode(input: &mut I) -> Result { + let mut prefix = [0u8; VERSIONED_PARACHAIN_BLOCK_DATA_PREFIX.len()]; + input.read(&mut prefix)?; + + if prefix == VERSIONED_PARACHAIN_BLOCK_DATA_PREFIX { + match input.read_byte()? { + 1 => { + let blocks = Vec::::decode(input)?; + let proof = CompactProof::decode(input)?; + + Ok(Self::V1 { blocks, proof }) + }, + _ => Err("Unknown `ParachainBlockData` version".into()), + } + } else { + let mut input = PrependBytesInput { prepend: &prefix, read: 0, inner: input }; + let header = Block::Header::decode(&mut input)?; + let extrinsics = Vec::::decode(&mut input)?; + let proof = CompactProof::decode(&mut input)?; + + Ok(Self::V0 { block: [Block::new(header, extrinsics)], proof }) + } + } +} + impl ParachainBlockData { /// Creates a new instance of `Self`. pub fn new(blocks: Vec, proof: CompactProof) -> Self { @@ -63,6 +124,7 @@ impl ParachainBlockData { /// Returns references to the stored blocks. pub fn blocks(&self) -> &[Block] { match self { + Self::V0 { block, .. } => &block[..], Self::V1 { blocks, .. } => &blocks, } } @@ -70,6 +132,7 @@ impl ParachainBlockData { /// Returns mutable references to the stored blocks. pub fn blocks_mut(&mut self) -> &mut [Block] { match self { + Self::V0 { ref mut block, .. } => block, Self::V1 { ref mut blocks, .. } => blocks, } } @@ -77,6 +140,7 @@ impl ParachainBlockData { /// Returns the stored blocks. pub fn into_blocks(self) -> Vec { match self { + Self::V0 { block, .. } => block.into_iter().collect(), Self::V1 { blocks, .. } => blocks, } } @@ -84,6 +148,7 @@ impl ParachainBlockData { /// Returns a reference to the stored proof. pub fn proof(&self) -> &CompactProof { match self { + Self::V0 { proof, .. } => &proof, Self::V1 { proof, .. } => proof, } } @@ -91,6 +156,7 @@ impl ParachainBlockData { /// Deconstruct into the inner parts. pub fn into_inner(self) -> (Vec, CompactProof) { match self { + Self::V0 { block, proof } => (block.into_iter().collect(), proof), Self::V1 { blocks, proof } => (blocks, proof), } } @@ -109,18 +175,86 @@ impl ParachainBlockData { /// Converts into [`v0::ParachainBlockData`]. /// /// Returns `None` if there is not exactly one block. - pub fn as_v0(&self) -> Option> { + pub fn as_v0(&self) -> Option { match self { + Self::V0 { .. } => Some(self.clone()), Self::V1 { blocks, proof } => { if blocks.len() != 1 { return None } - blocks.first().map(|block| { - let (header, extrinsics) = block.clone().deconstruct(); - v0::ParachainBlockData { header, extrinsics, storage_proof: proof.clone() } - }) + blocks + .first() + .map(|block| Self::V0 { block: [block.clone()], proof: proof.clone() }) }, } } } + +#[cfg(test)] +mod tests { + use super::*; + use sp_runtime::testing::*; + + #[derive(codec::Encode, codec::Decode, Clone, PartialEq, Debug)] + struct ParachainBlockDataV0 { + /// The header of the parachain block. + pub header: B::Header, + /// The extrinsics of the parachain block. + pub extrinsics: alloc::vec::Vec, + /// The data that is required to emulate the storage accesses executed by all extrinsics. + pub storage_proof: sp_trie::CompactProof, + } + + type TestExtrinsic = TestXt; + type TestBlock = Block; + + #[test] + fn decoding_encoding_v0_works() { + let v0 = ParachainBlockDataV0:: { + header: Header::new_from_number(10), + extrinsics: vec![ + TestExtrinsic::new_bare(MockCallU64(10)), + TestExtrinsic::new_bare(MockCallU64(100)), + ], + storage_proof: CompactProof { encoded_nodes: vec![vec![10u8; 200], vec![20u8; 30]] }, + }; + + let encoded = v0.encode(); + let decoded = ParachainBlockData::::decode(&mut &encoded[..]).unwrap(); + + match &decoded { + ParachainBlockData::V0 { block, proof } => { + assert_eq!(v0.header, block[0].header); + assert_eq!(v0.extrinsics, block[0].extrinsics); + assert_eq!(&v0.storage_proof, proof); + }, + _ => panic!("Invalid decoding"), + } + + let encoded = decoded.as_v0().unwrap().encode(); + + let decoded = ParachainBlockDataV0::::decode(&mut &encoded[..]).unwrap(); + assert_eq!(decoded, v0); + } + + #[test] + fn decoding_encoding_v1_works() { + let v1 = ParachainBlockData::::V1 { + blocks: vec![TestBlock::new( + Header::new_from_number(10), + vec![ + TestExtrinsic::new_bare(MockCallU64(10)), + TestExtrinsic::new_bare(MockCallU64(100)), + ], + )], + proof: CompactProof { encoded_nodes: vec![vec![10u8; 200], vec![20u8; 30]] }, + }; + + let encoded = v1.encode(); + let decoded = ParachainBlockData::::decode(&mut &encoded[..]).unwrap(); + + assert_eq!(v1.blocks(), decoded.blocks()); + assert_eq!(v1.proof(), decoded.proof()); + } +} diff --git a/prdoc/pr_6137.prdoc b/prdoc/pr_6137.prdoc index 60f88966042c..6a5930ba12fe 100644 --- a/prdoc/pr_6137.prdoc +++ b/prdoc/pr_6137.prdoc @@ -5,10 +5,7 @@ doc: This pull request adds support to `ParachainBlockData` to support multiple blocks at once. This basically means that cumulus based Parachains could start packaging multiple blocks into one `PoV`. From the relay chain PoV nothing changes and these `PoV`s appear like any other `PoV`. Internally this `PoV` then executes the blocks sequentially. However, all these blocks together can use the same amount of resources like a single `PoV`. This pull request is basically a preparation to support running parachains with a faster block time than the relay chain. - - This breaks the encoding of `ParachainBlockData`. It requires that the collators upgrade first before the runtime requiring the new `ParachainBlockData` is enacted. - The collators will decide based on the api version of `CollectCollationInfo`, which `ParachainBlockData` format they will send to the relay chain so that the validation code can interpret it correctly. - + crates: - name: cumulus-client-collator bump: major From 16d95c919fbab261c7d01f0a32e6890a4df2157b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Fri, 4 Apr 2025 18:52:32 +0200 Subject: [PATCH 29/34] Update pr_6137.prdoc --- prdoc/pr_6137.prdoc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/prdoc/pr_6137.prdoc b/prdoc/pr_6137.prdoc index 6a5930ba12fe..13076fed7d1c 100644 --- a/prdoc/pr_6137.prdoc +++ b/prdoc/pr_6137.prdoc @@ -5,6 +5,8 @@ doc: This pull request adds support to `ParachainBlockData` to support multiple blocks at once. This basically means that cumulus based Parachains could start packaging multiple blocks into one `PoV`. From the relay chain PoV nothing changes and these `PoV`s appear like any other `PoV`. Internally this `PoV` then executes the blocks sequentially. However, all these blocks together can use the same amount of resources like a single `PoV`. This pull request is basically a preparation to support running parachains with a faster block time than the relay chain. + + This changes the encoding of ParachainBlockData. However, encoding and decoding is made in a backwards and forwards compatible way. This means that there is no dependency between the collator and runtime upgrade. crates: - name: cumulus-client-collator From a303b8c359b7cd1f47bd80b435454fb1820fe3ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Mon, 7 Apr 2025 16:30:46 +0200 Subject: [PATCH 30/34] Update cumulus/primitives/core/src/parachain_block_data.rs --- cumulus/primitives/core/src/parachain_block_data.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cumulus/primitives/core/src/parachain_block_data.rs b/cumulus/primitives/core/src/parachain_block_data.rs index 0b48a571acfc..2e8373207771 100644 --- a/cumulus/primitives/core/src/parachain_block_data.rs +++ b/cumulus/primitives/core/src/parachain_block_data.rs @@ -172,7 +172,7 @@ impl ParachainBlockData { ); } - /// Converts into [`v0::ParachainBlockData`]. + /// Converts into [`ParachainBlockData::V0`]. /// /// Returns `None` if there is not exactly one block. pub fn as_v0(&self) -> Option { From c7c5b064c0909f28bb5a5466a8549bb097ded2e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Tue, 8 Apr 2025 22:00:08 +0200 Subject: [PATCH 31/34] Update cumulus/pallets/parachain-system/src/validate_block/implementation.rs --- .../parachain-system/src/validate_block/implementation.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cumulus/pallets/parachain-system/src/validate_block/implementation.rs b/cumulus/pallets/parachain-system/src/validate_block/implementation.rs index 225c0d7b5f55..5967a68b7d7d 100644 --- a/cumulus/pallets/parachain-system/src/validate_block/implementation.rs +++ b/cumulus/pallets/parachain-system/src/validate_block/implementation.rs @@ -209,7 +209,7 @@ where run_with_externalities_and_recorder::( &backend, - &mut recorder, + &mut SizeOnlyRecorderProvider::new(), &mut Default::default(), || { let relay_chain_proof = crate::RelayChainStateProof::new( From 1e130c6ea559bc524e5cc6f21ca732b18b9db764 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Wed, 9 Apr 2025 14:04:20 +0200 Subject: [PATCH 32/34] Ensure we don't share the `recorder` --- cumulus/pallets/parachain-system/src/lib.rs | 18 +++++++ .../src/validate_block/implementation.rs | 48 +++++++++++++------ .../src/validate_block/trie_cache.rs | 20 ++++++++ .../src/validate_block/trie_recorder.rs | 9 ++-- 4 files changed, 75 insertions(+), 20 deletions(-) diff --git a/cumulus/pallets/parachain-system/src/lib.rs b/cumulus/pallets/parachain-system/src/lib.rs index 2af6dbe91a55..3f2acf8a4f2a 100644 --- a/cumulus/pallets/parachain-system/src/lib.rs +++ b/cumulus/pallets/parachain-system/src/lib.rs @@ -486,6 +486,9 @@ pub mod pallet { // like for example from scheduler, we only kill the storage entry if it was not yet // updated in the current block. if !>::get() { + // NOTE: Killing here is required to at least include the trie nodes down to the key + // in the proof. Because this value will be read in `validate_block` and thus, + // needs to be reachable by the proof. NewValidationCode::::kill(); weight += T::DbWeight::get().writes(1); } @@ -508,10 +511,25 @@ pub mod pallet { // Remove the validation from the old block. ValidationData::::kill(); + // NOTE: Killing here is required to at least include the trie nodes down to the key in + // the proof. Because this value will be read in `validate_block` and thus, needs to + // be reachable by the proof. ProcessedDownwardMessages::::kill(); + // NOTE: Killing here is required to at least include the trie nodes down to the key in + // the proof. Because this value will be read in `validate_block` and thus, needs to + // be reachable by the proof. HrmpWatermark::::kill(); + // NOTE: Killing here is required to at least include the trie nodes down to the key in + // the proof. Because this value will be read in `validate_block` and thus, needs to + // be reachable by the proof. UpwardMessages::::kill(); + // NOTE: Killing here is required to at least include the trie nodes down to the key in + // the proof. Because this value will be read in `validate_block` and thus, needs to + // be reachable by the proof. HrmpOutboundMessages::::kill(); + // NOTE: Killing here is required to at least include the trie nodes down to the key in + // the proof. Because this value will be read in `validate_block` and thus, needs to + // be reachable by the proof. CustomValidationHeadData::::kill(); weight += T::DbWeight::get().writes(6); diff --git a/cumulus/pallets/parachain-system/src/validate_block/implementation.rs b/cumulus/pallets/parachain-system/src/validate_block/implementation.rs index 5967a68b7d7d..2aaa4fecef78 100644 --- a/cumulus/pallets/parachain-system/src/validate_block/implementation.rs +++ b/cumulus/pallets/parachain-system/src/validate_block/implementation.rs @@ -49,7 +49,7 @@ type TrieBackend = sp_state_machine::TrieBackend< SizeOnlyRecorderProvider>, >; -type Ext<'a, B> = sp_state_machine::Ext<'a, HashingFor, TrieBackend>; +type Ext<'a, Block, Backend> = sp_state_machine::Ext<'a, HashingFor, Backend>; fn with_externalities R, R>(f: F) -> R { sp_externalities::with_externalities(f).expect("Environmental externalities not set.") @@ -179,9 +179,6 @@ where core::mem::drop(proof); - // We use the same recorder across all blocks. Each node only contributed once to the total size - // of the storage proof. - let mut recorder = SizeOnlyRecorderProvider::new(); let cache_provider = trie_cache::CacheProvider::new(); // 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!! @@ -190,14 +187,23 @@ where *parent_header.state_root(), cache_provider, ) - .with_recorder(recorder.clone()) .build(); + // We use the same recorder when executing all blocks. So, each node only contributes once to + // the total size of the storage proof. This recorder should only be used for `execute_block`. + let mut execute_recorder = SizeOnlyRecorderProvider::default(); + // `backend` with the `execute_recorder`. As the `execute_recorder`, this should only be used + // for `execute_block`. + let execute_backend = sp_state_machine::TrieBackendBuilder::wrap(&backend) + .with_recorder(execute_recorder.clone()) + .build(); + // We let all blocks contribute to the same overlay. Data written by a previous block will be // directly accessible without going to the db. let mut overlay = OverlayedChanges::default(); for (block_index, block) in blocks.into_iter().enumerate() { + parent_header = block.header().clone(); let inherent_data = extract_parachain_inherent_data(&block); validate_validation_data( @@ -207,9 +213,10 @@ where ¶chain_head, ); + // We don't need the recorder or the overlay in here. run_with_externalities_and_recorder::( &backend, - &mut SizeOnlyRecorderProvider::new(), + &mut Default::default(), &mut Default::default(), || { let relay_chain_proof = crate::RelayChainStateProof::new( @@ -235,14 +242,25 @@ where ); run_with_externalities_and_recorder::( - &backend, - &mut recorder, + &execute_backend, + // Here is the only place where we want to use the recorder. + // We want to ensure that we not accidentally read something from the proof, that was + // not yet read and thus, alter the proof size. Otherwise we end up with mismatches in + // later blocks. + &mut execute_recorder, &mut overlay, || { - parent_header = block.header().clone(); - E::execute_block(block); + }, + ); + run_with_externalities_and_recorder::( + &backend, + &mut Default::default(), + // We are only reading here, but need to know what the old block has written. Thus, we + // are passing here the overlay. + &mut overlay, + || { new_validation_code = new_validation_code.take().or(crate::NewValidationCode::::get()); @@ -369,13 +387,13 @@ fn validate_validation_data( } /// Run the given closure with the externalities and recorder set. -fn run_with_externalities_and_recorder R>( - backend: &TrieBackend, - recorder: &mut SizeOnlyRecorderProvider>, - overlay: &mut OverlayedChanges>, +fn run_with_externalities_and_recorder R>( + backend: &impl sp_state_machine::Backend>, + recorder: &mut SizeOnlyRecorderProvider>, + overlay: &mut OverlayedChanges>, execute: F, ) -> R { - let mut ext = Ext::::new(overlay, backend); + let mut ext = Ext::::new(overlay, backend); recorder::using(recorder, || set_and_run_with_externalities(&mut ext, || execute())) } diff --git a/cumulus/pallets/parachain-system/src/validate_block/trie_cache.rs b/cumulus/pallets/parachain-system/src/validate_block/trie_cache.rs index 36efd3decf77..9590af993e9f 100644 --- a/cumulus/pallets/parachain-system/src/validate_block/trie_cache.rs +++ b/cumulus/pallets/parachain-system/src/validate_block/trie_cache.rs @@ -84,6 +84,26 @@ impl CacheProvider { } } +impl TrieCacheProvider for &CacheProvider { + type Cache<'a> + = TrieCache<'a, H> + where + Self: 'a, + H: 'a; + + fn as_trie_db_cache(&self, storage_root: ::Out) -> Self::Cache<'_> { + TrieCacheProvider::::as_trie_db_cache(*self, storage_root) + } + + fn as_trie_db_mut_cache(&self) -> Self::Cache<'_> { + TrieCacheProvider::::as_trie_db_mut_cache(*self) + } + + fn merge<'a>(&'a self, other: Self::Cache<'a>, new_root: ::Out) { + TrieCacheProvider::merge(*self, other, new_root) + } +} + impl TrieCacheProvider for CacheProvider { type Cache<'a> = TrieCache<'a, H> diff --git a/cumulus/pallets/parachain-system/src/validate_block/trie_recorder.rs b/cumulus/pallets/parachain-system/src/validate_block/trie_recorder.rs index ec748d90e826..9f84843c5626 100644 --- a/cumulus/pallets/parachain-system/src/validate_block/trie_recorder.rs +++ b/cumulus/pallets/parachain-system/src/validate_block/trie_recorder.rs @@ -96,9 +96,8 @@ pub(crate) struct SizeOnlyRecorderProvider { recorded_keys: Rc, RecordedForKey>>>, } -impl SizeOnlyRecorderProvider { - /// Create a new instance of [`SizeOnlyRecorderProvider`] - pub fn new() -> Self { +impl Default for SizeOnlyRecorderProvider { + fn default() -> Self { Self { seen_nodes: Default::default(), encoded_size: Default::default(), @@ -197,7 +196,7 @@ mod tests { for _ in 1..10 { let reference_recorder = Recorder::default(); let recorder_for_test: SizeOnlyRecorderProvider = - SizeOnlyRecorderProvider::new(); + SizeOnlyRecorderProvider::default(); let reference_cache: SharedTrieCache = SharedTrieCache::new(CacheSize::new(1024 * 5)); let cache_for_test: SharedTrieCache = @@ -252,7 +251,7 @@ mod tests { for _ in 1..10 { let reference_recorder = Recorder::default(); let recorder_for_test: SizeOnlyRecorderProvider = - SizeOnlyRecorderProvider::new(); + SizeOnlyRecorderProvider::default(); { let mut reference_trie_recorder = reference_recorder.as_trie_recorder(root); let reference_trie = From 1ef54b53ef3cdb662aac271dff1fcbe30f21add9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Wed, 9 Apr 2025 15:08:42 +0200 Subject: [PATCH 33/34] Fix warning --- .../parachain-system/src/validate_block/implementation.rs | 7 ------- 1 file changed, 7 deletions(-) diff --git a/cumulus/pallets/parachain-system/src/validate_block/implementation.rs b/cumulus/pallets/parachain-system/src/validate_block/implementation.rs index 2aaa4fecef78..02cfea9e1dff 100644 --- a/cumulus/pallets/parachain-system/src/validate_block/implementation.rs +++ b/cumulus/pallets/parachain-system/src/validate_block/implementation.rs @@ -42,13 +42,6 @@ use sp_state_machine::OverlayedChanges; use sp_trie::{MemoryDB, ProofSizeProvider}; use trie_recorder::SizeOnlyRecorderProvider; -type TrieBackend = sp_state_machine::TrieBackend< - MemoryDB>, - HashingFor, - trie_cache::CacheProvider>, - SizeOnlyRecorderProvider>, ->; - type Ext<'a, Block, Backend> = sp_state_machine::Ext<'a, HashingFor, Backend>; fn with_externalities R, R>(f: F) -> R { From 225dd99be969b7066a5ec5b8775afef427cc826c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Wed, 9 Apr 2025 16:21:09 +0200 Subject: [PATCH 34/34] Remove more warnings --- .../parachain-system/src/validate_block/implementation.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cumulus/pallets/parachain-system/src/validate_block/implementation.rs b/cumulus/pallets/parachain-system/src/validate_block/implementation.rs index 02cfea9e1dff..c062454ba784 100644 --- a/cumulus/pallets/parachain-system/src/validate_block/implementation.rs +++ b/cumulus/pallets/parachain-system/src/validate_block/implementation.rs @@ -39,7 +39,7 @@ use sp_externalities::{set_and_run_with_externalities, Externalities}; use sp_io::KillStorageResult; use sp_runtime::traits::{Block as BlockT, ExtrinsicLike, HashingFor, Header as HeaderT}; use sp_state_machine::OverlayedChanges; -use sp_trie::{MemoryDB, ProofSizeProvider}; +use sp_trie::ProofSizeProvider; use trie_recorder::SizeOnlyRecorderProvider; type Ext<'a, Block, Backend> = sp_state_machine::Ext<'a, HashingFor, Backend>;