From bdc73c78c8f72144d813739b588e9d8fa9920df4 Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Fri, 28 Feb 2025 12:30:42 +0200 Subject: [PATCH 01/15] add runtime API Signed-off-by: Andrei Sandu --- polkadot/node/primitives/src/lib.rs | 4 ++++ polkadot/node/service/src/fake_runtime_api.rs | 4 ++++ polkadot/node/subsystem-types/src/messages.rs | 6 ++++++ polkadot/primitives/src/runtime_api.rs | 4 ++++ polkadot/runtime/parachains/src/configuration.rs | 3 +++ .../runtime/parachains/src/runtime_api_impl/vstaging.rs | 5 +++++ polkadot/runtime/rococo/src/lib.rs | 4 ++++ polkadot/runtime/test-runtime/src/lib.rs | 4 ++++ polkadot/runtime/westend/src/lib.rs | 4 ++++ 9 files changed, 38 insertions(+) diff --git a/polkadot/node/primitives/src/lib.rs b/polkadot/node/primitives/src/lib.rs index 845daa2679850..8e2ae6e30a57b 100644 --- a/polkadot/node/primitives/src/lib.rs +++ b/polkadot/node/primitives/src/lib.rs @@ -69,6 +69,10 @@ const MERKLE_NODE_MAX_SIZE: usize = 512 + 100; const MERKLE_PROOF_MAX_DEPTH: usize = 8; /// The bomb limit for decompressing code blobs. +#[deprecated( + note = "`VALIDATION_CODE_BOMB_LIMIT` will be removed. Use `validation_code_bomb_limit` + runtime API to retrieve the value from the runtime" +)] pub const VALIDATION_CODE_BOMB_LIMIT: usize = (MAX_CODE_SIZE * 4u32) as usize; /// The bomb limit for decompressing PoV blobs. diff --git a/polkadot/node/service/src/fake_runtime_api.rs b/polkadot/node/service/src/fake_runtime_api.rs index 4e31c72d334f7..526936a33d393 100644 --- a/polkadot/node/service/src/fake_runtime_api.rs +++ b/polkadot/node/service/src/fake_runtime_api.rs @@ -237,6 +237,10 @@ sp_api::impl_runtime_apis! { ) -> Option<()> { unimplemented!() } + + fn validation_code_bomb_limit() -> u32 { + unimplemented!() + } } impl sp_consensus_beefy::BeefyApi for Runtime { diff --git a/polkadot/node/subsystem-types/src/messages.rs b/polkadot/node/subsystem-types/src/messages.rs index 4d4fc89a6addc..ed9300be249e1 100644 --- a/polkadot/node/subsystem-types/src/messages.rs +++ b/polkadot/node/subsystem-types/src/messages.rs @@ -778,6 +778,9 @@ pub enum RuntimeApiRequest { /// Get the lookahead from the scheduler params. /// `V12` SchedulingLookahead(SessionIndex, RuntimeApiSender), + /// Get the maximum uncompressed code size. + /// `V12` + ValidationCodeBombLimit(SessionIndex, RuntimeApiSender), } impl RuntimeApiRequest { @@ -824,6 +827,9 @@ impl RuntimeApiRequest { /// `SchedulingLookahead` pub const SCHEDULING_LOOKAHEAD_RUNTIME_REQUIREMENT: u32 = 12; + + /// `ValidationCodeBombLimit` + pub const VALIDATION_CODE_BOMB_LIMIT_RUNTIME_REQUIREMENT: u32 = 12; } /// A message to the Runtime API subsystem. diff --git a/polkadot/primitives/src/runtime_api.rs b/polkadot/primitives/src/runtime_api.rs index e0516a2f77e42..b68ecdd986949 100644 --- a/polkadot/primitives/src/runtime_api.rs +++ b/polkadot/primitives/src/runtime_api.rs @@ -308,5 +308,9 @@ sp_api::decl_runtime_apis! { /// Retrieve the scheduling lookahead #[api_version(12)] fn scheduling_lookahead() -> u32; + + /***** Added in v12 *****/ + /// Retrieve the maximum uncompressed code size. + fn validation_code_bomb_limit() -> u32; } } diff --git a/polkadot/runtime/parachains/src/configuration.rs b/polkadot/runtime/parachains/src/configuration.rs index e5cf7c4d276e8..8c63fb620418b 100644 --- a/polkadot/runtime/parachains/src/configuration.rs +++ b/polkadot/runtime/parachains/src/configuration.rs @@ -50,6 +50,9 @@ const LOG_TARGET: &str = "runtime::configuration"; // `polkadot_node_network_protocol::POV_RESPONSE_SIZE`. const POV_SIZE_HARD_LIMIT: u32 = 16 * 1024 * 1024; +// The maximum compression ratio that we use to compute the maximum uncompressed code size. +pub(crate) const MAX_VALIDATION_CODE_COMPRESSION_RATIO: u32 = 10; + /// All configuration of the runtime with respect to paras. #[derive( Clone, diff --git a/polkadot/runtime/parachains/src/runtime_api_impl/vstaging.rs b/polkadot/runtime/parachains/src/runtime_api_impl/vstaging.rs index 5a77af0d79731..799f00726f84d 100644 --- a/polkadot/runtime/parachains/src/runtime_api_impl/vstaging.rs +++ b/polkadot/runtime/parachains/src/runtime_api_impl/vstaging.rs @@ -50,3 +50,8 @@ pub fn backing_constraints( pub fn scheduling_lookahead() -> u32 { configuration::ActiveConfig::::get().scheduler_params.lookahead } + +/// Implementation for `validation_code_bomb_limit` function from the runtime API +pub fn validation_code_bomb_limit() -> u32 { + configuration::ActiveConfig::::get().max_code_size * configuration::MAX_VALIDATION_CODE_COMPRESSION_RATIO +} \ No newline at end of file diff --git a/polkadot/runtime/rococo/src/lib.rs b/polkadot/runtime/rococo/src/lib.rs index 0a6f52890b272..4961d7bbad296 100644 --- a/polkadot/runtime/rococo/src/lib.rs +++ b/polkadot/runtime/rococo/src/lib.rs @@ -2166,6 +2166,10 @@ sp_api::impl_runtime_apis! { fn scheduling_lookahead() -> u32 { parachains_staging_runtime_api_impl::scheduling_lookahead::() } + + fn validation_code_bomb_limit() -> u32 { + parachains_staging_runtime_api_impl::validation_code_bomb_limit::() + } } #[api_version(5)] diff --git a/polkadot/runtime/test-runtime/src/lib.rs b/polkadot/runtime/test-runtime/src/lib.rs index fc489e3bc685e..59126308acff6 100644 --- a/polkadot/runtime/test-runtime/src/lib.rs +++ b/polkadot/runtime/test-runtime/src/lib.rs @@ -1109,6 +1109,10 @@ sp_api::impl_runtime_apis! { fn scheduling_lookahead() -> u32 { staging_runtime_impl::scheduling_lookahead::() } + + fn validation_code_bomb_limit() -> u32 { + parachains_staging_runtime_api_impl::validation_code_bomb_limit::() + } } impl sp_consensus_beefy::BeefyApi for Runtime { diff --git a/polkadot/runtime/westend/src/lib.rs b/polkadot/runtime/westend/src/lib.rs index ddc26f4e645b6..03ac12a24e6b1 100644 --- a/polkadot/runtime/westend/src/lib.rs +++ b/polkadot/runtime/westend/src/lib.rs @@ -2211,6 +2211,10 @@ sp_api::impl_runtime_apis! { fn scheduling_lookahead() -> u32 { parachains_staging_runtime_api_impl::scheduling_lookahead::() } + + fn validation_code_bomb_limit() -> u32 { + parachains_staging_runtime_api_impl::validation_code_bomb_limit::() + } } #[api_version(5)] From 566a4f22d04f5b4eb223abde170c7775cc20576b Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Fri, 28 Feb 2025 13:03:25 +0200 Subject: [PATCH 02/15] node side runtime api support Signed-off-by: Andrei Sandu --- polkadot/node/core/runtime-api/src/cache.rs | 13 +++++++++ polkadot/node/core/runtime-api/src/lib.rs | 19 ++++++++++++ .../subsystem-types/src/runtime_client.rs | 8 +++++ .../node/subsystem-util/src/runtime/mod.rs | 29 +++++++++++++++++++ 4 files changed, 69 insertions(+) diff --git a/polkadot/node/core/runtime-api/src/cache.rs b/polkadot/node/core/runtime-api/src/cache.rs index 4ed42626d88ee..4e16264145b06 100644 --- a/polkadot/node/core/runtime-api/src/cache.rs +++ b/polkadot/node/core/runtime-api/src/cache.rs @@ -77,6 +77,7 @@ pub(crate) struct RequestResultCache { claim_queue: LruMap>>, backing_constraints: LruMap<(Hash, ParaId), Option>, scheduling_lookahead: LruMap, + validation_code_bomb_limits: LruMap, } impl Default for RequestResultCache { @@ -116,6 +117,7 @@ impl Default for RequestResultCache { claim_queue: LruMap::new(ByLength::new(DEFAULT_CACHE_CAP)), backing_constraints: LruMap::new(ByLength::new(DEFAULT_CACHE_CAP)), scheduling_lookahead: LruMap::new(ByLength::new(DEFAULT_CACHE_CAP)), + validation_code_bomb_limits: LruMap::new(ByLength::new(DEFAULT_CACHE_CAP)), } } } @@ -590,6 +592,16 @@ impl RequestResultCache { ) { self.scheduling_lookahead.insert(session_index, scheduling_lookahead); } + + /// Cache the validation code bomb limit for a session + pub(crate) fn cache_validation_code_bomb_limit(&mut self, session: SessionIndex, limit: u32) { + self.validation_code_bomb_limits.insert(session, limit); + } + + /// Get the validation code bomb limit for a session if cached + pub(crate) fn validation_code_bomb_limit(&mut self, session: SessionIndex) -> Option { + self.validation_code_bomb_limits.get(&session).copied() + } } pub(crate) enum RequestResult { @@ -642,4 +654,5 @@ pub(crate) enum RequestResult { CandidatesPendingAvailability(Hash, ParaId, Vec), BackingConstraints(Hash, ParaId, Option), SchedulingLookahead(SessionIndex, u32), + ValidationCodeBombLimit(SessionIndex, u32), } diff --git a/polkadot/node/core/runtime-api/src/lib.rs b/polkadot/node/core/runtime-api/src/lib.rs index 2d864c8cf2f4c..1270be8b15021 100644 --- a/polkadot/node/core/runtime-api/src/lib.rs +++ b/polkadot/node/core/runtime-api/src/lib.rs @@ -189,6 +189,9 @@ where SchedulingLookahead(session_index, scheduling_lookahead) => self .requests_cache .cache_scheduling_lookahead(session_index, scheduling_lookahead), + ValidationCodeBombLimit(session_index, limit) => self + .requests_cache + .cache_validation_code_bomb_limit(session_index, limit), } } @@ -357,6 +360,15 @@ where Some(Request::SchedulingLookahead(index, sender)) } }, + Request::ValidationCodeBombLimit(index, sender) => { + if let Some(value) = self.requests_cache.validation_code_bomb_limit(index) { + self.metrics.on_cached_request(); + let _ = sender.send(Ok(value)); + None + } else { + Some(Request::ValidationCodeBombLimit(index, sender)) + } + }, } } @@ -684,5 +696,12 @@ where sender, result = (index) ), + Request::ValidationCodeBombLimit(index, sender) => query!( + ValidationCodeBombLimit, + validation_code_bomb_limit(), + ver = Request::VALIDATION_CODE_BOMB_LIMIT_RUNTIME_REQUIREMENT, + sender, + result = (index) + ), } } diff --git a/polkadot/node/subsystem-types/src/runtime_client.rs b/polkadot/node/subsystem-types/src/runtime_client.rs index 7e3849c20694d..dd5f61b3bc7f0 100644 --- a/polkadot/node/subsystem-types/src/runtime_client.rs +++ b/polkadot/node/subsystem-types/src/runtime_client.rs @@ -353,6 +353,10 @@ pub trait RuntimeApiSubsystemClient { // === v12 === /// Fetch the scheduling lookahead value async fn scheduling_lookahead(&self, at: Hash) -> Result; + + // === v12 === + /// Fetch the maximum uncompressed code size. + async fn validation_code_bomb_limit(&self, at: Hash) -> Result; } /// Default implementation of [`RuntimeApiSubsystemClient`] using the client. @@ -642,6 +646,10 @@ where async fn scheduling_lookahead(&self, at: Hash) -> Result { self.client.runtime_api().scheduling_lookahead(at) } + + async fn validation_code_bomb_limit(&self, at: Hash) -> Result { + self.client.runtime_api().validation_code_bomb_limit(at) + } } impl HeaderBackend for DefaultSubsystemClient diff --git a/polkadot/node/subsystem-util/src/runtime/mod.rs b/polkadot/node/subsystem-util/src/runtime/mod.rs index dd843cfb01fa9..46a009ae5c817 100644 --- a/polkadot/node/subsystem-util/src/runtime/mod.rs +++ b/polkadot/node/subsystem-util/src/runtime/mod.rs @@ -637,3 +637,32 @@ pub async fn fetch_scheduling_lookahead( res } } + +/// Fetch the validation code bomb limit from the runtime. +pub async fn fetch_validation_code_bomb_limit( + parent: Hash, + session_index: SessionIndex, + sender: &mut impl overseer::SubsystemSender, +) -> Result { + let res = recv_runtime( + request_from_runtime(parent, sender, |tx| { + RuntimeApiRequest::ValidationCodeBombLimit(session_index, tx) + }) + .await, + ) + .await; + + if let Err(Error::RuntimeRequest(RuntimeApiError::NotSupported { .. })) = res { + gum::trace!( + target: LOG_TARGET, + ?parent, + "Querying the validation code bomb limit from the runtime is not supported by the current Runtime API", + ); + + // TODO: Remove this once runtime API version 12 is released. + #[allow(deprecated)] + Ok(polkadot_node_primitives::VALIDATION_CODE_BOMB_LIMIT as u32) + } else { + res + } +} From 81bcebc1e13b160ae7d26125264a6690d84ae938 Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Fri, 28 Feb 2025 14:09:22 +0200 Subject: [PATCH 03/15] use runtime provided bomb limit in candidate validation and pvf worker Signed-off-by: Andrei Sandu --- .../node/core/candidate-validation/src/lib.rs | 94 ++++++++++++++++--- polkadot/node/core/pvf/common/src/pvf.rs | 11 ++- .../node/core/pvf/prepare-worker/src/lib.rs | 3 +- 3 files changed, 91 insertions(+), 17 deletions(-) diff --git a/polkadot/node/core/candidate-validation/src/lib.rs b/polkadot/node/core/candidate-validation/src/lib.rs index 24590fe0c90ea..76742eba26ea6 100644 --- a/polkadot/node/core/candidate-validation/src/lib.rs +++ b/polkadot/node/core/candidate-validation/src/lib.rs @@ -195,9 +195,34 @@ where let relay_parent = candidate_receipt.descriptor.relay_parent(); let maybe_claim_queue = claim_queue(relay_parent, &mut sender).await; + let Some(session_index) = get_session_index(&mut sender, relay_parent).await else { + let error = "cannot fetch session index from the runtime"; + gum::warn!( + target: LOG_TARGET, + ?relay_parent, + error, + ); + + let _ = response_sender.send(Err(ValidationFailed("Session index not found".to_string()))); + return + }; + + // This will return a default value for the limit if runtime API is not available. + // however we still error out if there is a weird runtime API error. + let Ok(validation_code_bomb_limit) = util::runtime::fetch_validation_code_bomb_limit(relay_parent, session_index, &mut sender).await else { + let error = "cannot fetch validation code bomb limit from the runtime"; + gum::warn!( + target: LOG_TARGET, + ?relay_parent, + error, + ); + + let _ = response_sender.send(Err(ValidationFailed("Validation code bomb limit not available".to_string()))); + return + }; let res = validate_candidate_exhaustive( - get_session_index(&mut sender, relay_parent).await, + session_index, validation_host, validation_data, validation_code, @@ -207,6 +232,7 @@ where exec_kind, &metrics, maybe_claim_queue, + validation_code_bomb_limit, ) .await; @@ -220,8 +246,34 @@ where response_sender, .. } => async move { + let Some(session_index) = get_session_index(&mut sender, relay_parent).await else { + let error = "cannot fetch session index from the runtime"; + gum::warn!( + target: LOG_TARGET, + ?relay_parent, + error, + ); + + let _ = response_sender.send(PreCheckOutcome::Failed); + return + }; + + // This will return a default value for the limit if runtime API is not available. + // however we still error out if there is a weird runtime API error. + let Ok(validation_code_bomb_limit) = util::runtime::fetch_validation_code_bomb_limit(relay_parent, session_index, &mut sender).await else { + let error = "cannot fetch validation code bomb limit from the runtime"; + gum::warn!( + target: LOG_TARGET, + ?relay_parent, + error, + ); + + let _ = response_sender.send(PreCheckOutcome::Failed); + return + }; + let precheck_result = - precheck_pvf(&mut sender, validation_host, relay_parent, validation_code_hash) + precheck_pvf(&mut sender, validation_host, relay_parent, validation_code_hash, validation_code_bomb_limit) .await; let _ = response_sender.send(precheck_result); @@ -533,11 +585,29 @@ where continue; }; + let Some(session_index) = get_session_index(sender, relay_parent).await else { + continue; + }; + + let validation_code_bomb_limit = match util::runtime::fetch_validation_code_bomb_limit(relay_parent, session_index, sender).await { + Ok(limit) => limit, + Err(err) => { + gum::warn!( + target: LOG_TARGET, + ?relay_parent, + ?err, + "cannot fetch validation code bomb limit from runtime API", + ); + continue; + } + }; + let pvf = PvfPrepData::from_code( validation_code.0, executor_params.clone(), timeout, PrepareJobKind::Prechecking, + validation_code_bomb_limit, ); active_pvfs.push(pvf); @@ -690,6 +760,7 @@ async fn precheck_pvf( mut validation_backend: impl ValidationBackend, relay_parent: Hash, validation_code_hash: ValidationCodeHash, + validation_code_bomb_limit: u32, ) -> PreCheckOutcome where Sender: SubsystemSender, @@ -739,6 +810,7 @@ where executor_params, timeout, PrepareJobKind::Prechecking, + validation_code_bomb_limit, ); match validation_backend.precheck_pvf(pvf).await { @@ -753,7 +825,7 @@ where } async fn validate_candidate_exhaustive( - maybe_expected_session_index: Option, + expected_session_index: SessionIndex, mut validation_backend: impl ValidationBackend + Send, persisted_validation_data: PersistedValidationData, validation_code: ValidationCode, @@ -763,6 +835,7 @@ async fn validate_candidate_exhaustive( exec_kind: PvfExecKind, metrics: &Metrics, maybe_claim_queue: Option, + validation_code_bomb_limit: u32, ) -> Result { let _timer = metrics.time_validate_candidate_exhaustive(); let validation_code_hash = validation_code.hash(); @@ -779,17 +852,6 @@ async fn validate_candidate_exhaustive( // We only check the session index for backing. match (exec_kind, candidate_receipt.descriptor.session_index()) { (PvfExecKind::Backing(_) | PvfExecKind::BackingSystemParas(_), Some(session_index)) => { - let Some(expected_session_index) = maybe_expected_session_index else { - let error = "cannot fetch session index from the runtime"; - gum::warn!( - target: LOG_TARGET, - ?relay_parent, - error, - ); - - return Err(ValidationFailed(error.into())) - }; - if session_index != expected_session_index { return Ok(ValidationResult::Invalid(InvalidCandidate::InvalidSessionIndex)) } @@ -819,6 +881,7 @@ async fn validate_candidate_exhaustive( executor_params, prep_timeout, PrepareJobKind::Compilation, + validation_code_bomb_limit, ); validation_backend @@ -843,6 +906,7 @@ async fn validate_candidate_exhaustive( PVF_APPROVAL_EXECUTION_RETRY_DELAY, exec_kind.into(), exec_kind, + validation_code_bomb_limit, ) .await, }; @@ -1005,6 +1069,7 @@ trait ValidationBackend { prepare_priority: polkadot_node_core_pvf::Priority, // The kind for the execution job. exec_kind: PvfExecKind, + validation_code_bomb_limit: u32, ) -> Result { let prep_timeout = pvf_prep_timeout(&executor_params, PvfPrepKind::Prepare); // Construct the PVF a single time, since it is an expensive operation. Cloning it is cheap. @@ -1013,6 +1078,7 @@ trait ValidationBackend { executor_params, prep_timeout, PrepareJobKind::Compilation, + validation_code_bomb_limit, ); // We keep track of the total time that has passed and stop retrying if we are taking too // long. diff --git a/polkadot/node/core/pvf/common/src/pvf.rs b/polkadot/node/core/pvf/common/src/pvf.rs index 4019a8d8b0d00..338e2074009a7 100644 --- a/polkadot/node/core/pvf/common/src/pvf.rs +++ b/polkadot/node/core/pvf/common/src/pvf.rs @@ -28,6 +28,8 @@ use std::{fmt, sync::Arc, time::Duration}; pub struct PvfPrepData { /// Wasm code (maybe compressed) maybe_compressed_code: Arc>, + /// Maximum uncompressed code size. + validation_code_bomb_limit: u32, /// Wasm code hash. code_hash: ValidationCodeHash, /// Executor environment parameters for the session for which artifact is prepared @@ -45,11 +47,12 @@ impl PvfPrepData { executor_params: ExecutorParams, prep_timeout: Duration, prep_kind: PrepareJobKind, + validation_code_bomb_limit: u32, ) -> Self { let maybe_compressed_code = Arc::new(code); let code_hash = sp_crypto_hashing::blake2_256(&maybe_compressed_code).into(); let executor_params = Arc::new(executor_params); - Self { maybe_compressed_code, code_hash, executor_params, prep_timeout, prep_kind } + Self { maybe_compressed_code, code_hash, executor_params, prep_timeout, prep_kind, validation_code_bomb_limit } } /// Returns validation code hash @@ -77,6 +80,11 @@ impl PvfPrepData { self.prep_kind } + /// Returns validation code bomb limit. + pub fn validation_code_bomb_limit(&self) -> u32 { + self.validation_code_bomb_limit + } + /// Creates a structure for tests. #[cfg(feature = "test-utils")] pub fn from_discriminator_and_timeout(num: u32, timeout: Duration) -> Self { @@ -86,6 +94,7 @@ impl PvfPrepData { ExecutorParams::default(), timeout, PrepareJobKind::Compilation, + 30*1024*1024, ) } diff --git a/polkadot/node/core/pvf/prepare-worker/src/lib.rs b/polkadot/node/core/pvf/prepare-worker/src/lib.rs index f8ebb6effcecd..ec68ffb8eb71a 100644 --- a/polkadot/node/core/pvf/prepare-worker/src/lib.rs +++ b/polkadot/node/core/pvf/prepare-worker/src/lib.rs @@ -38,7 +38,6 @@ use polkadot_node_core_pvf_common::{ executor_interface::{prepare, prevalidate}, worker::{pipe2_cloexec, PipeFd, WorkerInfo}, }; -use polkadot_node_primitives::VALIDATION_CODE_BOMB_LIMIT; use codec::{Decode, Encode}; use polkadot_node_core_pvf_common::{ @@ -304,7 +303,7 @@ pub fn worker_entrypoint( fn prepare_artifact(pvf: PvfPrepData) -> Result { let maybe_compressed_code = pvf.maybe_compressed_code(); let raw_validation_code = - sp_maybe_compressed_blob::decompress(&maybe_compressed_code, VALIDATION_CODE_BOMB_LIMIT) + sp_maybe_compressed_blob::decompress(&maybe_compressed_code, pvf.validation_code_bomb_limit() as usize) .map_err(|e| PrepareError::CouldNotDecompressCodeBlob(e.to_string()))?; let observed_wasm_code_len = raw_validation_code.len() as u32; From 16b35e735a4e358eb7d849dd52efbff0f668d2e8 Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Fri, 28 Feb 2025 17:08:41 +0200 Subject: [PATCH 04/15] fix tests Signed-off-by: Andrei Sandu --- .../node/core/candidate-validation/src/lib.rs | 57 +++++-- .../core/candidate-validation/src/tests.rs | 161 +++++++++++++----- polkadot/node/core/pvf/tests/it/main.rs | 16 +- 3 files changed, 172 insertions(+), 62 deletions(-) diff --git a/polkadot/node/core/candidate-validation/src/lib.rs b/polkadot/node/core/candidate-validation/src/lib.rs index 76742eba26ea6..a40db8439f3ad 100644 --- a/polkadot/node/core/candidate-validation/src/lib.rs +++ b/polkadot/node/core/candidate-validation/src/lib.rs @@ -203,13 +203,20 @@ where error, ); - let _ = response_sender.send(Err(ValidationFailed("Session index not found".to_string()))); + let _ = response_sender + .send(Err(ValidationFailed("Session index not found".to_string()))); return }; - + // This will return a default value for the limit if runtime API is not available. // however we still error out if there is a weird runtime API error. - let Ok(validation_code_bomb_limit) = util::runtime::fetch_validation_code_bomb_limit(relay_parent, session_index, &mut sender).await else { + let Ok(validation_code_bomb_limit) = util::runtime::fetch_validation_code_bomb_limit( + relay_parent, + session_index, + &mut sender, + ) + .await + else { let error = "cannot fetch validation code bomb limit from the runtime"; gum::warn!( target: LOG_TARGET, @@ -217,7 +224,9 @@ where error, ); - let _ = response_sender.send(Err(ValidationFailed("Validation code bomb limit not available".to_string()))); + let _ = response_sender.send(Err(ValidationFailed( + "Validation code bomb limit not available".to_string(), + ))); return }; @@ -257,10 +266,16 @@ where let _ = response_sender.send(PreCheckOutcome::Failed); return }; - + // This will return a default value for the limit if runtime API is not available. // however we still error out if there is a weird runtime API error. - let Ok(validation_code_bomb_limit) = util::runtime::fetch_validation_code_bomb_limit(relay_parent, session_index, &mut sender).await else { + let Ok(validation_code_bomb_limit) = util::runtime::fetch_validation_code_bomb_limit( + relay_parent, + session_index, + &mut sender, + ) + .await + else { let error = "cannot fetch validation code bomb limit from the runtime"; gum::warn!( target: LOG_TARGET, @@ -272,9 +287,14 @@ where return }; - let precheck_result = - precheck_pvf(&mut sender, validation_host, relay_parent, validation_code_hash, validation_code_bomb_limit) - .await; + let precheck_result = precheck_pvf( + &mut sender, + validation_host, + relay_parent, + validation_code_hash, + validation_code_bomb_limit, + ) + .await; let _ = response_sender.send(precheck_result); } @@ -585,11 +605,15 @@ where continue; }; - let Some(session_index) = get_session_index(sender, relay_parent).await else { - continue; - }; + let Some(session_index) = get_session_index(sender, relay_parent).await else { continue }; - let validation_code_bomb_limit = match util::runtime::fetch_validation_code_bomb_limit(relay_parent, session_index, sender).await { + let validation_code_bomb_limit = match util::runtime::fetch_validation_code_bomb_limit( + relay_parent, + session_index, + sender, + ) + .await + { Ok(limit) => limit, Err(err) => { gum::warn!( @@ -599,7 +623,7 @@ where "cannot fetch validation code bomb limit from runtime API", ); continue; - } + }, }; let pvf = PvfPrepData::from_code( @@ -851,11 +875,10 @@ async fn validate_candidate_exhaustive( // We only check the session index for backing. match (exec_kind, candidate_receipt.descriptor.session_index()) { - (PvfExecKind::Backing(_) | PvfExecKind::BackingSystemParas(_), Some(session_index)) => { + (PvfExecKind::Backing(_) | PvfExecKind::BackingSystemParas(_), Some(session_index)) => if session_index != expected_session_index { return Ok(ValidationResult::Invalid(InvalidCandidate::InvalidSessionIndex)) - } - }, + }, (_, _) => {}, }; diff --git a/polkadot/node/core/candidate-validation/src/tests.rs b/polkadot/node/core/candidate-validation/src/tests.rs index ee72daa1f86eb..a6938df7f0148 100644 --- a/polkadot/node/core/candidate-validation/src/tests.rs +++ b/polkadot/node/core/candidate-validation/src/tests.rs @@ -24,7 +24,7 @@ use crate::PvfExecKind; use assert_matches::assert_matches; use futures::executor; use polkadot_node_core_pvf::PrepareError; -use polkadot_node_primitives::{BlockData, VALIDATION_CODE_BOMB_LIMIT}; +use polkadot_node_primitives::BlockData; use polkadot_node_subsystem::messages::AllMessages; use polkadot_node_subsystem_test_helpers::{make_subsystem_context, TestSubsystemContextHandle}; use polkadot_node_subsystem_util::reexports::SubsystemContext; @@ -46,6 +46,8 @@ use sp_core::{sr25519::Public, testing::TaskExecutor}; use sp_keyring::Sr25519Keyring; use sp_keystore::{testing::MemoryKeystore, Keystore}; +const VALIDATION_CODE_BOMB_LIMIT: u32 = 30 * 1024 * 1024; + #[derive(Debug)] enum AssumptionCheckOutcome { Matches(PersistedValidationData, ValidationCode), @@ -518,7 +520,7 @@ fn session_index_checked_only_in_backing() { // The session index is invalid let v = executor::block_on(validate_candidate_exhaustive( - Some(1), + 1, MockValidateCandidateBackend::with_hardcoded_result(Ok(validation_result.clone())), validation_data.clone(), validation_code.clone(), @@ -528,6 +530,7 @@ fn session_index_checked_only_in_backing() { PvfExecKind::Backing(dummy_hash()), &Default::default(), Default::default(), + VALIDATION_CODE_BOMB_LIMIT, )) .unwrap(); @@ -535,7 +538,7 @@ fn session_index_checked_only_in_backing() { // Approval doesn't fail since the check is ommited. let v = executor::block_on(validate_candidate_exhaustive( - Some(1), + 1, MockValidateCandidateBackend::with_hardcoded_result(Ok(validation_result.clone())), validation_data.clone(), validation_code.clone(), @@ -545,6 +548,7 @@ fn session_index_checked_only_in_backing() { PvfExecKind::Approval, &Default::default(), Default::default(), + VALIDATION_CODE_BOMB_LIMIT, )) .unwrap(); @@ -559,7 +563,7 @@ fn session_index_checked_only_in_backing() { // Approval doesn't fail since the check is ommited. let v = executor::block_on(validate_candidate_exhaustive( - Some(1), + 1, MockValidateCandidateBackend::with_hardcoded_result(Ok(validation_result)), validation_data.clone(), validation_code, @@ -569,6 +573,7 @@ fn session_index_checked_only_in_backing() { PvfExecKind::Dispute, &Default::default(), Default::default(), + VALIDATION_CODE_BOMB_LIMIT, )) .unwrap(); @@ -657,7 +662,7 @@ fn candidate_validation_ok_is_ok(#[case] v2_descriptor: bool) { let _ = cq.insert(CoreIndex(1), vec![1.into(), 1.into()].into()); let v = executor::block_on(validate_candidate_exhaustive( - Some(1), + 1, MockValidateCandidateBackend::with_hardcoded_result(Ok(validation_result)), validation_data.clone(), validation_code, @@ -667,6 +672,7 @@ fn candidate_validation_ok_is_ok(#[case] v2_descriptor: bool) { PvfExecKind::Backing(dummy_hash()), &Default::default(), Some(ClaimQueueSnapshot(cq)), + VALIDATION_CODE_BOMB_LIMIT, )) .unwrap(); @@ -735,7 +741,7 @@ fn invalid_session_or_core_index() { CandidateReceipt { descriptor, commitments_hash: commitments.hash() }; let err = executor::block_on(validate_candidate_exhaustive( - Some(1), + 1, MockValidateCandidateBackend::with_hardcoded_result(Ok(validation_result.clone())), validation_data.clone(), validation_code.clone(), @@ -745,13 +751,14 @@ fn invalid_session_or_core_index() { PvfExecKind::Backing(dummy_hash()), &Default::default(), Default::default(), + VALIDATION_CODE_BOMB_LIMIT, )) .unwrap(); assert_matches!(err, ValidationResult::Invalid(InvalidCandidate::InvalidSessionIndex)); let err = executor::block_on(validate_candidate_exhaustive( - Some(1), + 1, MockValidateCandidateBackend::with_hardcoded_result(Ok(validation_result.clone())), validation_data.clone(), validation_code.clone(), @@ -761,6 +768,7 @@ fn invalid_session_or_core_index() { PvfExecKind::BackingSystemParas(dummy_hash()), &Default::default(), Default::default(), + VALIDATION_CODE_BOMB_LIMIT, )) .unwrap(); @@ -769,7 +777,7 @@ fn invalid_session_or_core_index() { candidate_receipt.descriptor.set_session_index(1); let result = executor::block_on(validate_candidate_exhaustive( - Some(1), + 1, MockValidateCandidateBackend::with_hardcoded_result(Ok(validation_result.clone())), validation_data.clone(), validation_code.clone(), @@ -779,12 +787,13 @@ fn invalid_session_or_core_index() { PvfExecKind::Backing(dummy_hash()), &Default::default(), Some(Default::default()), + VALIDATION_CODE_BOMB_LIMIT, )) .unwrap(); assert_matches!(result, ValidationResult::Invalid(InvalidCandidate::InvalidCoreIndex)); let result = executor::block_on(validate_candidate_exhaustive( - Some(1), + 1, MockValidateCandidateBackend::with_hardcoded_result(Ok(validation_result.clone())), validation_data.clone(), validation_code.clone(), @@ -794,12 +803,13 @@ fn invalid_session_or_core_index() { PvfExecKind::BackingSystemParas(dummy_hash()), &Default::default(), Some(Default::default()), + VALIDATION_CODE_BOMB_LIMIT, )) .unwrap(); assert_matches!(result, ValidationResult::Invalid(InvalidCandidate::InvalidCoreIndex)); let v = executor::block_on(validate_candidate_exhaustive( - Some(1), + 1, MockValidateCandidateBackend::with_hardcoded_result(Ok(validation_result.clone())), validation_data.clone(), validation_code.clone(), @@ -809,6 +819,7 @@ fn invalid_session_or_core_index() { PvfExecKind::Approval, &Default::default(), Default::default(), + VALIDATION_CODE_BOMB_LIMIT, )) .unwrap(); @@ -824,7 +835,7 @@ fn invalid_session_or_core_index() { // Dispute check passes because we don't check core or session index let v = executor::block_on(validate_candidate_exhaustive( - Some(1), + 1, MockValidateCandidateBackend::with_hardcoded_result(Ok(validation_result.clone())), validation_data.clone(), validation_code.clone(), @@ -834,6 +845,7 @@ fn invalid_session_or_core_index() { PvfExecKind::Dispute, &Default::default(), Default::default(), + VALIDATION_CODE_BOMB_LIMIT, )) .unwrap(); @@ -853,7 +865,7 @@ fn invalid_session_or_core_index() { let _ = cq.insert(CoreIndex(1), vec![1.into(), 2.into()].into()); let v = executor::block_on(validate_candidate_exhaustive( - Some(1), + 1, MockValidateCandidateBackend::with_hardcoded_result(Ok(validation_result.clone())), validation_data.clone(), validation_code.clone(), @@ -863,6 +875,7 @@ fn invalid_session_or_core_index() { PvfExecKind::Backing(dummy_hash()), &Default::default(), Some(ClaimQueueSnapshot(cq.clone())), + VALIDATION_CODE_BOMB_LIMIT, )) .unwrap(); @@ -876,7 +889,7 @@ fn invalid_session_or_core_index() { }); let v = executor::block_on(validate_candidate_exhaustive( - Some(1), + 1, MockValidateCandidateBackend::with_hardcoded_result(Ok(validation_result.clone())), validation_data.clone(), validation_code.clone(), @@ -886,6 +899,7 @@ fn invalid_session_or_core_index() { PvfExecKind::BackingSystemParas(dummy_hash()), &Default::default(), Some(ClaimQueueSnapshot(cq)), + VALIDATION_CODE_BOMB_LIMIT, )) .unwrap(); @@ -920,7 +934,7 @@ fn invalid_session_or_core_index() { [PvfExecKind::Backing(dummy_hash()), PvfExecKind::BackingSystemParas(dummy_hash())] { let result = executor::block_on(validate_candidate_exhaustive( - Some(1), + 1, MockValidateCandidateBackend::with_hardcoded_result(Ok(validation_result.clone())), validation_data.clone(), validation_code.clone(), @@ -930,6 +944,7 @@ fn invalid_session_or_core_index() { exec_kind, &Default::default(), Some(Default::default()), + VALIDATION_CODE_BOMB_LIMIT, )) .unwrap(); assert_matches!(result, ValidationResult::Invalid(InvalidCandidate::InvalidCoreIndex)); @@ -938,7 +953,7 @@ fn invalid_session_or_core_index() { // Validation doesn't fail for approvals and disputes, core/session index is not checked. for exec_kind in [PvfExecKind::Approval, PvfExecKind::Dispute] { let v = executor::block_on(validate_candidate_exhaustive( - Some(1), + 1, MockValidateCandidateBackend::with_hardcoded_result(Ok(validation_result.clone())), validation_data.clone(), validation_code.clone(), @@ -948,6 +963,7 @@ fn invalid_session_or_core_index() { exec_kind, &Default::default(), Default::default(), + VALIDATION_CODE_BOMB_LIMIT, )) .unwrap(); @@ -992,7 +1008,7 @@ fn candidate_validation_bad_return_is_invalid() { let candidate_receipt = CandidateReceipt { descriptor, commitments_hash: Hash::zero() }; let v = executor::block_on(validate_candidate_exhaustive( - Some(1), + 1, MockValidateCandidateBackend::with_hardcoded_result(Err(ValidationError::Invalid( WasmInvalidCandidate::HardTimeout, ))), @@ -1004,6 +1020,7 @@ fn candidate_validation_bad_return_is_invalid() { PvfExecKind::Backing(dummy_hash()), &Default::default(), Default::default(), + VALIDATION_CODE_BOMB_LIMIT, )) .unwrap(); @@ -1075,7 +1092,7 @@ fn candidate_validation_one_ambiguous_error_is_valid() { let candidate_receipt = CandidateReceipt { descriptor, commitments_hash: commitments.hash() }; let v = executor::block_on(validate_candidate_exhaustive( - Some(1), + 1, MockValidateCandidateBackend::with_hardcoded_result_list(vec![ Err(ValidationError::PossiblyInvalid(PossiblyInvalidError::AmbiguousWorkerDeath)), Ok(validation_result), @@ -1088,6 +1105,7 @@ fn candidate_validation_one_ambiguous_error_is_valid() { PvfExecKind::Approval, &Default::default(), Default::default(), + VALIDATION_CODE_BOMB_LIMIT, )) .unwrap(); @@ -1118,7 +1136,7 @@ fn candidate_validation_multiple_ambiguous_errors_is_invalid() { let candidate_receipt = CandidateReceipt { descriptor, commitments_hash: Hash::zero() }; let v = executor::block_on(validate_candidate_exhaustive( - Some(1), + 1, MockValidateCandidateBackend::with_hardcoded_result_list(vec![ Err(ValidationError::PossiblyInvalid(PossiblyInvalidError::AmbiguousWorkerDeath)), Err(ValidationError::PossiblyInvalid(PossiblyInvalidError::AmbiguousWorkerDeath)), @@ -1131,6 +1149,7 @@ fn candidate_validation_multiple_ambiguous_errors_is_invalid() { PvfExecKind::Approval, &Default::default(), Default::default(), + VALIDATION_CODE_BOMB_LIMIT, )) .unwrap(); @@ -1238,7 +1257,7 @@ fn candidate_validation_retry_on_error_helper( let candidate_receipt = CandidateReceipt { descriptor, commitments_hash: Hash::zero() }; return executor::block_on(validate_candidate_exhaustive( - Some(1), + 1, MockValidateCandidateBackend::with_hardcoded_result_list(mock_errors), validation_data, validation_code, @@ -1248,6 +1267,7 @@ fn candidate_validation_retry_on_error_helper( exec_kind, &Default::default(), Default::default(), + VALIDATION_CODE_BOMB_LIMIT, )) } @@ -1281,7 +1301,7 @@ fn candidate_validation_timeout_is_internal_error() { let candidate_receipt = CandidateReceipt { descriptor, commitments_hash: Hash::zero() }; let v = executor::block_on(validate_candidate_exhaustive( - Some(1), + 1, MockValidateCandidateBackend::with_hardcoded_result(Err(ValidationError::Invalid( WasmInvalidCandidate::HardTimeout, ))), @@ -1293,6 +1313,7 @@ fn candidate_validation_timeout_is_internal_error() { PvfExecKind::Backing(dummy_hash()), &Default::default(), Default::default(), + VALIDATION_CODE_BOMB_LIMIT, )); assert_matches!(v, Ok(ValidationResult::Invalid(InvalidCandidate::Timeout))); @@ -1332,7 +1353,7 @@ fn candidate_validation_commitment_hash_mismatch_is_invalid() { }; let result = executor::block_on(validate_candidate_exhaustive( - Some(1), + 1, MockValidateCandidateBackend::with_hardcoded_result(Ok(validation_result)), validation_data, validation_code, @@ -1342,6 +1363,7 @@ fn candidate_validation_commitment_hash_mismatch_is_invalid() { PvfExecKind::Backing(dummy_hash()), &Default::default(), Default::default(), + VALIDATION_CODE_BOMB_LIMIT, )) .unwrap(); @@ -1382,7 +1404,7 @@ fn candidate_validation_code_mismatch_is_invalid() { let (_ctx, _ctx_handle) = make_subsystem_context::(pool.clone()); let v = executor::block_on(validate_candidate_exhaustive( - Some(1), + 1, MockValidateCandidateBackend::with_hardcoded_result(Err(ValidationError::Invalid( WasmInvalidCandidate::HardTimeout, ))), @@ -1394,6 +1416,7 @@ fn candidate_validation_code_mismatch_is_invalid() { PvfExecKind::Backing(dummy_hash()), &Default::default(), Default::default(), + VALIDATION_CODE_BOMB_LIMIT, )) .unwrap(); @@ -1407,9 +1430,10 @@ fn compressed_code_works() { let head_data = HeadData(vec![1, 1, 1]); let raw_code = vec![2u8; 16]; - let validation_code = sp_maybe_compressed_blob::compress(&raw_code, VALIDATION_CODE_BOMB_LIMIT) - .map(ValidationCode) - .unwrap(); + let validation_code = + sp_maybe_compressed_blob::compress(&raw_code, VALIDATION_CODE_BOMB_LIMIT as usize) + .map(ValidationCode) + .unwrap(); let descriptor = make_valid_candidate_descriptor( ParaId::from(1_u32), @@ -1444,7 +1468,7 @@ fn compressed_code_works() { let candidate_receipt = CandidateReceipt { descriptor, commitments_hash: commitments.hash() }; let v = executor::block_on(validate_candidate_exhaustive( - Some(1), + 1, MockValidateCandidateBackend::with_hardcoded_result(Ok(validation_result)), validation_data, validation_code, @@ -1454,6 +1478,7 @@ fn compressed_code_works() { PvfExecKind::Backing(dummy_hash()), &Default::default(), Some(Default::default()), + VALIDATION_CODE_BOMB_LIMIT, )); assert_matches!(v, Ok(ValidationResult::Valid(_, _))); @@ -1514,6 +1539,7 @@ fn precheck_works() { MockPreCheckBackend::with_hardcoded_result(Ok(())), relay_parent, validation_code_hash, + VALIDATION_CODE_BOMB_LIMIT, ) .remote_handle(); @@ -1571,6 +1597,7 @@ fn precheck_properly_classifies_outcomes() { MockPreCheckBackend::with_hardcoded_result(prepare_result), relay_parent, validation_code_hash, + VALIDATION_CODE_BOMB_LIMIT, ) .remote_handle(); @@ -1821,6 +1848,21 @@ fn maybe_prepare_validation_golden_path() { let _ = tx.send(Ok(Some(ValidationCode(Vec::new())))); } ); + + assert_matches!( + ctx_handle.recv().await, + AllMessages::RuntimeApi(RuntimeApiMessage::Request(_, RuntimeApiRequest::SessionIndexForChild(tx))) => { + let _ = tx.send(Ok(1)); + } + ); + + assert_matches!( + ctx_handle.recv().await, + AllMessages::RuntimeApi(RuntimeApiMessage::Request(_, RuntimeApiRequest::ValidationCodeBombLimit(session, tx))) => { + assert_eq!(session, 1); + let _ = tx.send(Ok(VALIDATION_CODE_BOMB_LIMIT)); + } + ); }; let test_fut = future::join(test_fut, check_fut); @@ -1980,6 +2022,21 @@ fn maybe_prepare_validation_does_not_prepare_pvfs_if_no_new_session_but_a_valida let _ = tx.send(Ok(Some(ValidationCode(Vec::new())))); } ); + + assert_matches!( + ctx_handle.recv().await, + AllMessages::RuntimeApi(RuntimeApiMessage::Request(_, RuntimeApiRequest::SessionIndexForChild(tx))) => { + let _ = tx.send(Ok(1)); + } + ); + + assert_matches!( + ctx_handle.recv().await, + AllMessages::RuntimeApi(RuntimeApiMessage::Request(_, RuntimeApiRequest::ValidationCodeBombLimit(session, tx))) => { + assert_eq!(session, 1); + let _ = tx.send(Ok(VALIDATION_CODE_BOMB_LIMIT)); + } + ); }; let test_fut = future::join(test_fut, check_fut); @@ -2131,21 +2188,30 @@ fn maybe_prepare_validation_prepares_a_limited_number_of_pvfs() { } ); - assert_matches!( - ctx_handle.recv().await, - AllMessages::RuntimeApi(RuntimeApiMessage::Request(_, RuntimeApiRequest::ValidationCodeByHash(hash, tx))) => { - assert_eq!(hash, ValidationCode(vec![0; 16]).hash()); - let _ = tx.send(Ok(Some(ValidationCode(Vec::new())))); - } - ); + for c in 0..2 { + assert_matches!( + ctx_handle.recv().await, + AllMessages::RuntimeApi(RuntimeApiMessage::Request(_, RuntimeApiRequest::ValidationCodeByHash(hash, tx))) => { + assert_eq!(hash, ValidationCode(vec![c; 16]).hash()); + let _ = tx.send(Ok(Some(ValidationCode(Vec::new())))); + } + ); - assert_matches!( - ctx_handle.recv().await, - AllMessages::RuntimeApi(RuntimeApiMessage::Request(_, RuntimeApiRequest::ValidationCodeByHash(hash, tx))) => { - assert_eq!(hash, ValidationCode(vec![1; 16]).hash()); - let _ = tx.send(Ok(Some(ValidationCode(Vec::new())))); - } - ); + assert_matches!( + ctx_handle.recv().await, + AllMessages::RuntimeApi(RuntimeApiMessage::Request(_, RuntimeApiRequest::SessionIndexForChild(tx))) => { + let _ = tx.send(Ok(1)); + } + ); + + assert_matches!( + ctx_handle.recv().await, + AllMessages::RuntimeApi(RuntimeApiMessage::Request(_, RuntimeApiRequest::ValidationCodeBombLimit(session, tx))) => { + assert_eq!(session, 1); + let _ = tx.send(Ok(VALIDATION_CODE_BOMB_LIMIT)); + } + ); + } }; let test_fut = future::join(test_fut, check_fut); @@ -2216,6 +2282,21 @@ fn maybe_prepare_validation_does_not_prepare_already_prepared_pvfs() { let _ = tx.send(Ok(Some(ValidationCode(Vec::new())))); } ); + + assert_matches!( + ctx_handle.recv().await, + AllMessages::RuntimeApi(RuntimeApiMessage::Request(_, RuntimeApiRequest::SessionIndexForChild(tx))) => { + let _ = tx.send(Ok(1)); + } + ); + + assert_matches!( + ctx_handle.recv().await, + AllMessages::RuntimeApi(RuntimeApiMessage::Request(_, RuntimeApiRequest::ValidationCodeBombLimit(session, tx))) => { + assert_eq!(session, 1); + let _ = tx.send(Ok(VALIDATION_CODE_BOMB_LIMIT)); + } + ); }; let test_fut = future::join(test_fut, check_fut); diff --git a/polkadot/node/core/pvf/tests/it/main.rs b/polkadot/node/core/pvf/tests/it/main.rs index cfb78fd530d23..9b24e7b64c89c 100644 --- a/polkadot/node/core/pvf/tests/it/main.rs +++ b/polkadot/node/core/pvf/tests/it/main.rs @@ -24,7 +24,7 @@ use polkadot_node_core_pvf::{ PossiblyInvalidError, PrepareError, PrepareJobKind, PvfPrepData, ValidationError, ValidationHost, JOB_TIMEOUT_WALL_CLOCK_FACTOR, }; -use polkadot_node_primitives::{PoV, POV_BOMB_LIMIT, VALIDATION_CODE_BOMB_LIMIT}; +use polkadot_node_primitives::{PoV, POV_BOMB_LIMIT}; use polkadot_node_subsystem::messages::PvfExecKind; use polkadot_parachain_primitives::primitives::{BlockData, ValidationResult}; use polkadot_primitives::{ @@ -33,6 +33,8 @@ use polkadot_primitives::{ }; use sp_core::H256; +const VALIDATION_CODE_BOMB_LIMIT: u32 = 30 * 1024 * 1024; + use std::{io::Write, sync::Arc, time::Duration}; use tokio::sync::Mutex; @@ -94,6 +96,7 @@ impl TestHost { executor_params, TEST_PREPARATION_TIMEOUT, PrepareJobKind::Prechecking, + VALIDATION_CODE_BOMB_LIMIT, ), result_tx, ) @@ -121,6 +124,7 @@ impl TestHost { executor_params, TEST_PREPARATION_TIMEOUT, PrepareJobKind::Compilation, + VALIDATION_CODE_BOMB_LIMIT, ), TEST_EXECUTION_TIMEOUT, Arc::new(pvd), @@ -682,9 +686,10 @@ async fn artifact_does_reprepare_on_meaningful_exec_parameter_change() { #[tokio::test] async fn invalid_compressed_code_fails_prechecking() { let host = TestHost::new().await; - let raw_code = vec![2u8; VALIDATION_CODE_BOMB_LIMIT + 1]; + let raw_code = vec![2u8; VALIDATION_CODE_BOMB_LIMIT as usize + 1]; let validation_code = - sp_maybe_compressed_blob::compress(&raw_code, VALIDATION_CODE_BOMB_LIMIT + 1).unwrap(); + sp_maybe_compressed_blob::compress(&raw_code, VALIDATION_CODE_BOMB_LIMIT as usize + 1) + .unwrap(); let res = host.precheck_pvf(&validation_code, Default::default()).await; @@ -703,9 +708,10 @@ async fn invalid_compressed_code_fails_validation() { }; let pov = PoV { block_data: BlockData(Vec::new()) }; - let raw_code = vec![2u8; VALIDATION_CODE_BOMB_LIMIT + 1]; + let raw_code = vec![2u8; VALIDATION_CODE_BOMB_LIMIT as usize + 1]; let validation_code = - sp_maybe_compressed_blob::compress(&raw_code, VALIDATION_CODE_BOMB_LIMIT + 1).unwrap(); + sp_maybe_compressed_blob::compress(&raw_code, VALIDATION_CODE_BOMB_LIMIT as usize + 1) + .unwrap(); let result = host .validate_candidate(&validation_code, pvd, pov, Default::default(), H256::default()) From 26a879d778add8a6595de68b445fc26be72f1018 Mon Sep 17 00:00:00 2001 From: "cmd[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 5 Mar 2025 10:02:23 +0000 Subject: [PATCH 05/15] Update from sandreim running command 'prdoc --audience runtime_dev --bump minor' --- prdoc/pr_7760.prdoc | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) create mode 100644 prdoc/pr_7760.prdoc diff --git a/prdoc/pr_7760.prdoc b/prdoc/pr_7760.prdoc new file mode 100644 index 0000000000000..b29d9da870ad5 --- /dev/null +++ b/prdoc/pr_7760.prdoc @@ -0,0 +1,34 @@ +title: Dynamic uncompressed code size limit +doc: +- audience: Runtime Dev + description: |- + Fixes https://github.com/paritytech/polkadot-sdk/issues/7664 + + The discussion in https://github.com/paritytech/polkadot-sdk/pull/7710 should provide the context for why this solution was picked. +crates: +- name: polkadot-node-primitives + bump: minor +- name: polkadot-service + bump: minor +- name: polkadot-node-subsystem-types + bump: minor +- name: polkadot-primitives + bump: minor +- name: polkadot-runtime-parachains + bump: minor +- name: rococo-runtime + bump: minor +- name: westend-runtime + bump: minor +- name: polkadot-node-core-runtime-api + bump: minor +- name: polkadot-node-subsystem-util + bump: minor +- name: polkadot-node-core-candidate-validation + bump: minor +- name: polkadot-node-core-pvf-common + bump: minor +- name: polkadot-node-core-pvf-prepare-worker + bump: minor +- name: polkadot-node-core-pvf + bump: minor From 0d01a3b9f4c19bc451efbb549ac92cff4b9067a5 Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Wed, 5 Mar 2025 12:23:17 +0200 Subject: [PATCH 06/15] missed version Signed-off-by: Andrei Sandu --- polkadot/primitives/src/runtime_api.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/polkadot/primitives/src/runtime_api.rs b/polkadot/primitives/src/runtime_api.rs index b68ecdd986949..2471dc1fc0738 100644 --- a/polkadot/primitives/src/runtime_api.rs +++ b/polkadot/primitives/src/runtime_api.rs @@ -311,6 +311,7 @@ sp_api::decl_runtime_apis! { /***** Added in v12 *****/ /// Retrieve the maximum uncompressed code size. + #[api_version(12)] fn validation_code_bomb_limit() -> u32; } } From 6ae7abd60e76953f913fee861b3f752d8420c951 Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Wed, 5 Mar 2025 12:28:38 +0200 Subject: [PATCH 07/15] update prdoc Signed-off-by: Andrei Sandu --- prdoc/pr_7760.prdoc | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/prdoc/pr_7760.prdoc b/prdoc/pr_7760.prdoc index b29d9da870ad5..3511a57a87e9f 100644 --- a/prdoc/pr_7760.prdoc +++ b/prdoc/pr_7760.prdoc @@ -2,16 +2,17 @@ title: Dynamic uncompressed code size limit doc: - audience: Runtime Dev description: |- - Fixes https://github.com/paritytech/polkadot-sdk/issues/7664 - - The discussion in https://github.com/paritytech/polkadot-sdk/pull/7710 should provide the context for why this solution was picked. + Deprecates node constant `VALIDATION_CODE_BOMB_LIMIT` and introduces + `validation_code_bomb_limit` runtime API that computes the maximum + uncompressed code size as the maximum code size multiplied by a + compression ratio of 10. crates: - name: polkadot-node-primitives bump: minor - name: polkadot-service - bump: minor + bump: patch - name: polkadot-node-subsystem-types - bump: minor + bump: major - name: polkadot-primitives bump: minor - name: polkadot-runtime-parachains @@ -21,14 +22,14 @@ crates: - name: westend-runtime bump: minor - name: polkadot-node-core-runtime-api - bump: minor + bump: patch - name: polkadot-node-subsystem-util bump: minor - name: polkadot-node-core-candidate-validation - bump: minor + bump: patch - name: polkadot-node-core-pvf-common - bump: minor + bump: major - name: polkadot-node-core-pvf-prepare-worker - bump: minor + bump: patch - name: polkadot-node-core-pvf - bump: minor + bump: patch From 4925be62ba8593e9d4af01b81dafa1255145d00a Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Wed, 5 Mar 2025 12:39:44 +0200 Subject: [PATCH 08/15] fmt Signed-off-by: Andrei Sandu --- polkadot/node/core/pvf/common/src/pvf.rs | 11 +++++++++-- polkadot/node/core/pvf/prepare-worker/src/lib.rs | 8 +++++--- polkadot/node/core/runtime-api/src/lib.rs | 5 ++--- .../parachains/src/runtime_api_impl/vstaging.rs | 5 +++-- 4 files changed, 19 insertions(+), 10 deletions(-) diff --git a/polkadot/node/core/pvf/common/src/pvf.rs b/polkadot/node/core/pvf/common/src/pvf.rs index 338e2074009a7..6bfe9331224fb 100644 --- a/polkadot/node/core/pvf/common/src/pvf.rs +++ b/polkadot/node/core/pvf/common/src/pvf.rs @@ -52,7 +52,14 @@ impl PvfPrepData { let maybe_compressed_code = Arc::new(code); let code_hash = sp_crypto_hashing::blake2_256(&maybe_compressed_code).into(); let executor_params = Arc::new(executor_params); - Self { maybe_compressed_code, code_hash, executor_params, prep_timeout, prep_kind, validation_code_bomb_limit } + Self { + maybe_compressed_code, + code_hash, + executor_params, + prep_timeout, + prep_kind, + validation_code_bomb_limit, + } } /// Returns validation code hash @@ -94,7 +101,7 @@ impl PvfPrepData { ExecutorParams::default(), timeout, PrepareJobKind::Compilation, - 30*1024*1024, + 30 * 1024 * 1024, ) } diff --git a/polkadot/node/core/pvf/prepare-worker/src/lib.rs b/polkadot/node/core/pvf/prepare-worker/src/lib.rs index ec68ffb8eb71a..533abe414a0a9 100644 --- a/polkadot/node/core/pvf/prepare-worker/src/lib.rs +++ b/polkadot/node/core/pvf/prepare-worker/src/lib.rs @@ -302,9 +302,11 @@ pub fn worker_entrypoint( fn prepare_artifact(pvf: PvfPrepData) -> Result { let maybe_compressed_code = pvf.maybe_compressed_code(); - let raw_validation_code = - sp_maybe_compressed_blob::decompress(&maybe_compressed_code, pvf.validation_code_bomb_limit() as usize) - .map_err(|e| PrepareError::CouldNotDecompressCodeBlob(e.to_string()))?; + let raw_validation_code = sp_maybe_compressed_blob::decompress( + &maybe_compressed_code, + pvf.validation_code_bomb_limit() as usize, + ) + .map_err(|e| PrepareError::CouldNotDecompressCodeBlob(e.to_string()))?; let observed_wasm_code_len = raw_validation_code.len() as u32; let blob = match prevalidate(&raw_validation_code) { diff --git a/polkadot/node/core/runtime-api/src/lib.rs b/polkadot/node/core/runtime-api/src/lib.rs index 1270be8b15021..eefc1be106d30 100644 --- a/polkadot/node/core/runtime-api/src/lib.rs +++ b/polkadot/node/core/runtime-api/src/lib.rs @@ -189,9 +189,8 @@ where SchedulingLookahead(session_index, scheduling_lookahead) => self .requests_cache .cache_scheduling_lookahead(session_index, scheduling_lookahead), - ValidationCodeBombLimit(session_index, limit) => self - .requests_cache - .cache_validation_code_bomb_limit(session_index, limit), + ValidationCodeBombLimit(session_index, limit) => + self.requests_cache.cache_validation_code_bomb_limit(session_index, limit), } } diff --git a/polkadot/runtime/parachains/src/runtime_api_impl/vstaging.rs b/polkadot/runtime/parachains/src/runtime_api_impl/vstaging.rs index 799f00726f84d..e79f1bb0f283f 100644 --- a/polkadot/runtime/parachains/src/runtime_api_impl/vstaging.rs +++ b/polkadot/runtime/parachains/src/runtime_api_impl/vstaging.rs @@ -53,5 +53,6 @@ pub fn scheduling_lookahead() -> u32 { /// Implementation for `validation_code_bomb_limit` function from the runtime API pub fn validation_code_bomb_limit() -> u32 { - configuration::ActiveConfig::::get().max_code_size * configuration::MAX_VALIDATION_CODE_COMPRESSION_RATIO -} \ No newline at end of file + configuration::ActiveConfig::::get().max_code_size * + configuration::MAX_VALIDATION_CODE_COMPRESSION_RATIO +} From 89566025869da330e3ef7e93a7c0500d6805fd6b Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Wed, 5 Mar 2025 15:02:29 +0200 Subject: [PATCH 09/15] remove deprecated Signed-off-by: Andrei Sandu --- cumulus/bin/pov-validator/src/main.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/cumulus/bin/pov-validator/src/main.rs b/cumulus/bin/pov-validator/src/main.rs index 1c08f218f6b8a..f04c5938f631b 100644 --- a/cumulus/bin/pov-validator/src/main.rs +++ b/cumulus/bin/pov-validator/src/main.rs @@ -18,7 +18,7 @@ use clap::Parser; use codec::{Decode, Encode}; -use polkadot_node_primitives::{BlockData, PoV, POV_BOMB_LIMIT, VALIDATION_CODE_BOMB_LIMIT}; +use polkadot_node_primitives::{BlockData, PoV, POV_BOMB_LIMIT}; use polkadot_parachain_primitives::primitives::ValidationParams; use polkadot_primitives::{BlockNumber as RBlockNumber, Hash as RHash, HeadData}; use sc_executor::WasmExecutor; @@ -26,6 +26,10 @@ use sp_core::traits::{CallContext, CodeExecutor, RuntimeCode, WrappedRuntimeCode use std::{fs, path::PathBuf, time::Instant}; use tracing::level_filters::LevelFilter; +// This is now determined by the chain, call `validation_code_bomb_limit` API. +// max_code_size * 10 = 30MB currently. Update constant if needed. +const VALIDATION_CODE_BOMB_LIMIT: usize = 30 * 1024 * 1024; + /// Tool for validating a `PoV` locally. #[derive(Parser)] struct Cli { From 11cd37647fb8d5dd7cdaf37f9e7724b893b7f11f Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Wed, 5 Mar 2025 16:27:06 +0200 Subject: [PATCH 10/15] unneeded Signed-off-by: Andrei Sandu --- polkadot/node/service/src/fake_runtime_api.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/polkadot/node/service/src/fake_runtime_api.rs b/polkadot/node/service/src/fake_runtime_api.rs index 526936a33d393..4e31c72d334f7 100644 --- a/polkadot/node/service/src/fake_runtime_api.rs +++ b/polkadot/node/service/src/fake_runtime_api.rs @@ -237,10 +237,6 @@ sp_api::impl_runtime_apis! { ) -> Option<()> { unimplemented!() } - - fn validation_code_bomb_limit() -> u32 { - unimplemented!() - } } impl sp_consensus_beefy::BeefyApi for Runtime { From 78297bb67b47b3d98cba1a4a1e6343e1879353db Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Wed, 5 Mar 2025 16:52:46 +0200 Subject: [PATCH 11/15] fix build Signed-off-by: Andrei Sandu --- .../relay-chain-minimal-node/src/blockchain_rpc_client.rs | 4 ++++ .../client/relay-chain-rpc-interface/src/rpc_client.rs | 8 ++++++++ polkadot/node/core/runtime-api/src/tests.rs | 4 ++++ polkadot/runtime/test-runtime/src/lib.rs | 2 +- 4 files changed, 17 insertions(+), 1 deletion(-) diff --git a/cumulus/client/relay-chain-minimal-node/src/blockchain_rpc_client.rs b/cumulus/client/relay-chain-minimal-node/src/blockchain_rpc_client.rs index a30608224ab9e..313568745dbc3 100644 --- a/cumulus/client/relay-chain-minimal-node/src/blockchain_rpc_client.rs +++ b/cumulus/client/relay-chain-minimal-node/src/blockchain_rpc_client.rs @@ -469,6 +469,10 @@ impl RuntimeApiSubsystemClient for BlockChainRpcClient { async fn scheduling_lookahead(&self, at: Hash) -> Result { Ok(self.rpc_client.parachain_host_scheduling_lookahead(at).await?) } + + async fn validation_code_bomb_limit(&self, at: Hash) -> Result { + Ok(self.rpc_client.parachain_host_validation_code_bomb_limit(at).await?) + } } #[async_trait::async_trait] diff --git a/cumulus/client/relay-chain-rpc-interface/src/rpc_client.rs b/cumulus/client/relay-chain-rpc-interface/src/rpc_client.rs index b2fd5a4e6089c..c03b7dc738753 100644 --- a/cumulus/client/relay-chain-rpc-interface/src/rpc_client.rs +++ b/cumulus/client/relay-chain-rpc-interface/src/rpc_client.rs @@ -715,6 +715,14 @@ impl RelayChainRpcClient { .await } + pub async fn parachain_host_validation_code_bomb_limit( + &self, + at: RelayHash, + ) -> Result { + self.call_remote_runtime_function("ParachainHost_validation_code_bomb_limit", at, None::<()>) + .await + } + pub async fn validation_code_hash( &self, at: RelayHash, diff --git a/polkadot/node/core/runtime-api/src/tests.rs b/polkadot/node/core/runtime-api/src/tests.rs index bbc5801290022..bb75417a278fb 100644 --- a/polkadot/node/core/runtime-api/src/tests.rs +++ b/polkadot/node/core/runtime-api/src/tests.rs @@ -319,6 +319,10 @@ impl RuntimeApiSubsystemClient for MockSubsystemClient { ) -> Result, ApiError> { todo!("Not required for tests") } + + async fn validation_code_bomb_limit(&self, _: Hash) -> Result { + todo!("Not required for tests") + } } #[test] diff --git a/polkadot/runtime/test-runtime/src/lib.rs b/polkadot/runtime/test-runtime/src/lib.rs index 59126308acff6..2a9a5a4a75384 100644 --- a/polkadot/runtime/test-runtime/src/lib.rs +++ b/polkadot/runtime/test-runtime/src/lib.rs @@ -1111,7 +1111,7 @@ sp_api::impl_runtime_apis! { } fn validation_code_bomb_limit() -> u32 { - parachains_staging_runtime_api_impl::validation_code_bomb_limit::() + staging_runtime_impl::validation_code_bomb_limit::() } } From bb2b604aa6002c8877e7d7ea519da90c4f527175 Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Wed, 5 Mar 2025 17:36:00 +0200 Subject: [PATCH 12/15] fmt Signed-off-by: Andrei Sandu --- .../client/relay-chain-rpc-interface/src/rpc_client.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/cumulus/client/relay-chain-rpc-interface/src/rpc_client.rs b/cumulus/client/relay-chain-rpc-interface/src/rpc_client.rs index c03b7dc738753..113bc557499f0 100644 --- a/cumulus/client/relay-chain-rpc-interface/src/rpc_client.rs +++ b/cumulus/client/relay-chain-rpc-interface/src/rpc_client.rs @@ -719,8 +719,12 @@ impl RelayChainRpcClient { &self, at: RelayHash, ) -> Result { - self.call_remote_runtime_function("ParachainHost_validation_code_bomb_limit", at, None::<()>) - .await + self.call_remote_runtime_function( + "ParachainHost_validation_code_bomb_limit", + at, + None::<()>, + ) + .await } pub async fn validation_code_hash( From fa084471144b25017c2e5c792eddf8f1cebcdf77 Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Wed, 5 Mar 2025 17:49:04 +0200 Subject: [PATCH 13/15] fix node bench Signed-off-by: Andrei Sandu --- .../core/pvf/prepare-worker/benches/prepare_rococo_runtime.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/polkadot/node/core/pvf/prepare-worker/benches/prepare_rococo_runtime.rs b/polkadot/node/core/pvf/prepare-worker/benches/prepare_rococo_runtime.rs index 49b30dc33ceb7..9e6947655365e 100644 --- a/polkadot/node/core/pvf/prepare-worker/benches/prepare_rococo_runtime.rs +++ b/polkadot/node/core/pvf/prepare-worker/benches/prepare_rococo_runtime.rs @@ -47,6 +47,7 @@ fn prepare_rococo_runtime(c: &mut Criterion) { ExecutorParams::default(), Duration::from_secs(360), PrepareJobKind::Compilation, + 64 * 1024 * 1024, ), Err(e) => { panic!("Cannot decompress blob: {:?}", e); From 1ad717e90cc8a67d440c3c4d063785faf933914b Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Wed, 5 Mar 2025 18:29:17 +0200 Subject: [PATCH 14/15] more fixing Signed-off-by: Andrei Sandu --- polkadot/node/core/pvf/benches/host_prepare_rococo_runtime.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/polkadot/node/core/pvf/benches/host_prepare_rococo_runtime.rs b/polkadot/node/core/pvf/benches/host_prepare_rococo_runtime.rs index 342128b7cca21..05becf87a3251 100644 --- a/polkadot/node/core/pvf/benches/host_prepare_rococo_runtime.rs +++ b/polkadot/node/core/pvf/benches/host_prepare_rococo_runtime.rs @@ -77,6 +77,7 @@ impl TestHost { executor_params, TEST_PREPARATION_TIMEOUT, PrepareJobKind::Prechecking, + 16 * 1024 * 1024, ), result_tx, ) @@ -98,6 +99,7 @@ fn host_prepare_rococo_runtime(c: &mut Criterion) { ExecutorParams::default(), Duration::from_secs(360), PrepareJobKind::Compilation, + 64 * 1024 * 1024, ), Err(e) => { panic!("Cannot decompress blob: {:?}", e); From f029e4e009a73762395651e24b0bc1658d472d3a Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Thu, 6 Mar 2025 11:57:14 +0200 Subject: [PATCH 15/15] semver fix Signed-off-by: Andrei Sandu --- prdoc/pr_7760.prdoc | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/prdoc/pr_7760.prdoc b/prdoc/pr_7760.prdoc index 3511a57a87e9f..53df66430f86f 100644 --- a/prdoc/pr_7760.prdoc +++ b/prdoc/pr_7760.prdoc @@ -8,8 +8,6 @@ doc: compression ratio of 10. crates: - name: polkadot-node-primitives - bump: minor -- name: polkadot-service bump: patch - name: polkadot-node-subsystem-types bump: major @@ -33,3 +31,9 @@ crates: bump: patch - name: polkadot-node-core-pvf bump: patch +- name: cumulus-relay-chain-minimal-node + bump: minor +- name: cumulus-relay-chain-rpc-interface + bump: minor +- name: cumulus-pov-validator + bump: minor