diff --git a/polkadot/node/core/candidate-validation/src/lib.rs b/polkadot/node/core/candidate-validation/src/lib.rs index 406a37cb08..760cf2f69b 100644 --- a/polkadot/node/core/candidate-validation/src/lib.rs +++ b/polkadot/node/core/candidate-validation/src/lib.rs @@ -24,7 +24,7 @@ #![warn(missing_docs)] use polkadot_node_core_pvf::{ - InvalidCandidate as WasmInvalidCandidate, Pvf, ValidationError, ValidationHost, + InvalidCandidate as WasmInvalidCandidate, PrepareError, Pvf, ValidationError, ValidationHost, }; use polkadot_node_primitives::{ BlockData, InvalidCandidate, PoV, ValidationResult, POV_BOMB_LIMIT, VALIDATION_CODE_BOMB_LIMIT, @@ -32,7 +32,8 @@ use polkadot_node_primitives::{ use polkadot_node_subsystem::{ errors::RuntimeApiError, messages::{ - CandidateValidationMessage, RuntimeApiMessage, RuntimeApiRequest, ValidationFailed, + CandidateValidationMessage, PreCheckOutcome, RuntimeApiMessage, RuntimeApiRequest, + ValidationFailed, }, overseer, FromOverseer, OverseerSignal, SpawnedSubsystem, SubsystemContext, SubsystemError, SubsystemResult, SubsystemSender, @@ -194,6 +195,30 @@ where ctx.spawn("validate-from-exhaustive", bg.boxed())?; }, + CandidateValidationMessage::PreCheck( + relay_parent, + validation_code_hash, + response_sender, + ) => { + let bg = { + let mut sender = ctx.sender().clone(); + let validation_host = validation_host.clone(); + + async move { + let precheck_result = precheck_pvf( + &mut sender, + validation_host, + relay_parent, + validation_code_hash, + ) + .await; + + let _ = response_sender.send(precheck_result); + } + }; + + ctx.spawn("candidate-validation-pre-check", bg.boxed())?; + }, }, } } @@ -235,6 +260,73 @@ where }) } +async fn request_validation_code_by_hash( + sender: &mut Sender, + relay_parent: Hash, + validation_code_hash: ValidationCodeHash, +) -> Result, RuntimeRequestFailed> +where + Sender: SubsystemSender, +{ + let (tx, rx) = oneshot::channel(); + runtime_api_request( + sender, + relay_parent, + RuntimeApiRequest::ValidationCodeByHash(validation_code_hash, tx), + rx, + ) + .await +} + +async fn precheck_pvf( + sender: &mut Sender, + mut validation_backend: impl ValidationBackend, + relay_parent: Hash, + validation_code_hash: ValidationCodeHash, +) -> PreCheckOutcome +where + Sender: SubsystemSender, +{ + let validation_code = + match request_validation_code_by_hash(sender, relay_parent, validation_code_hash).await { + Ok(Some(code)) => code, + _ => { + // The reasoning why this is "failed" and not invalid is because we assume that + // during pre-checking voting the relay-chain will pin the code. In case the code + // actually is not there, we issue failed since this looks more like a bug. This + // leads to us abstaining. + tracing::warn!( + target: LOG_TARGET, + ?relay_parent, + ?validation_code_hash, + "precheck: requested validation code is not found on-chain!", + ); + return PreCheckOutcome::Failed + }, + }; + + let validation_code = match sp_maybe_compressed_blob::decompress( + &validation_code.0, + VALIDATION_CODE_BOMB_LIMIT, + ) { + Ok(code) => Pvf::from_code(code.into_owned()), + Err(e) => { + tracing::debug!(target: LOG_TARGET, err=?e, "precheck: cannot decompress validation code"); + return PreCheckOutcome::Invalid + }, + }; + + match validation_backend.precheck_pvf(validation_code).await { + Ok(_) => PreCheckOutcome::Valid, + Err(prepare_err) => match prepare_err { + PrepareError::Prevalidation(_) | + PrepareError::Preparation(_) | + PrepareError::Panic(_) => PreCheckOutcome::Invalid, + PrepareError::TimedOut | PrepareError::DidNotMakeIt => PreCheckOutcome::Failed, + }, + } +} + #[derive(Debug)] enum AssumptionCheckOutcome { Matches(PersistedValidationData, ValidationCode), @@ -487,6 +579,8 @@ trait ValidationBackend { timeout: Duration, params: ValidationParams, ) -> Result; + + async fn precheck_pvf(&mut self, pvf: Pvf) -> Result<(), PrepareError>; } #[async_trait] @@ -520,6 +614,17 @@ impl ValidationBackend for ValidationHost { validation_result } + + async fn precheck_pvf(&mut self, pvf: Pvf) -> Result<(), PrepareError> { + let (tx, rx) = oneshot::channel(); + if let Err(_) = self.precheck_pvf(pvf, tx).await { + return Err(PrepareError::DidNotMakeIt) + } + + let precheck_result = rx.await.or(Err(PrepareError::DidNotMakeIt))?; + + precheck_result + } } /// Does basic checks of a candidate. Provide the encoded PoV-block. Returns `Ok` if basic checks diff --git a/polkadot/node/core/candidate-validation/src/tests.rs b/polkadot/node/core/candidate-validation/src/tests.rs index a1e4a9d150..74d6fe13db 100644 --- a/polkadot/node/core/candidate-validation/src/tests.rs +++ b/polkadot/node/core/candidate-validation/src/tests.rs @@ -17,6 +17,7 @@ use super::*; use assert_matches::assert_matches; use futures::executor; +use polkadot_node_core_pvf::PrepareError; use polkadot_node_subsystem::messages::AllMessages; use polkadot_node_subsystem_test_helpers as test_helpers; use polkadot_node_subsystem_util::reexports::SubsystemContext; @@ -326,18 +327,18 @@ fn check_does_not_match() { executor::block_on(test_fut); } -struct MockValidatorBackend { +struct MockValidateCandidateBackend { result: Result, } -impl MockValidatorBackend { +impl MockValidateCandidateBackend { fn with_hardcoded_result(result: Result) -> Self { Self { result } } } #[async_trait] -impl ValidationBackend for MockValidatorBackend { +impl ValidationBackend for MockValidateCandidateBackend { async fn validate_candidate( &mut self, _raw_validation_code: Vec, @@ -346,6 +347,10 @@ impl ValidationBackend for MockValidatorBackend { ) -> Result { self.result.clone() } + + async fn precheck_pvf(&mut self, _pvf: Pvf) -> Result<(), PrepareError> { + unreachable!() + } } #[test] @@ -380,7 +385,7 @@ fn candidate_validation_ok_is_ok() { }; let v = executor::block_on(validate_candidate_exhaustive( - MockValidatorBackend::with_hardcoded_result(Ok(validation_result)), + MockValidateCandidateBackend::with_hardcoded_result(Ok(validation_result)), validation_data.clone(), validation_code, descriptor, @@ -421,9 +426,9 @@ fn candidate_validation_bad_return_is_invalid() { assert!(check.is_ok()); let v = executor::block_on(validate_candidate_exhaustive( - MockValidatorBackend::with_hardcoded_result(Err(ValidationError::InvalidCandidate( - WasmInvalidCandidate::AmbiguousWorkerDeath, - ))), + MockValidateCandidateBackend::with_hardcoded_result(Err( + ValidationError::InvalidCandidate(WasmInvalidCandidate::AmbiguousWorkerDeath), + )), validation_data, validation_code, descriptor, @@ -457,9 +462,9 @@ fn candidate_validation_timeout_is_internal_error() { assert!(check.is_ok()); let v = executor::block_on(validate_candidate_exhaustive( - MockValidatorBackend::with_hardcoded_result(Err(ValidationError::InvalidCandidate( - WasmInvalidCandidate::HardTimeout, - ))), + MockValidateCandidateBackend::with_hardcoded_result(Err( + ValidationError::InvalidCandidate(WasmInvalidCandidate::HardTimeout), + )), validation_data, validation_code, descriptor, @@ -492,9 +497,9 @@ fn candidate_validation_code_mismatch_is_invalid() { assert_matches!(check, Err(InvalidCandidate::CodeHashMismatch)); let v = executor::block_on(validate_candidate_exhaustive( - MockValidatorBackend::with_hardcoded_result(Err(ValidationError::InvalidCandidate( - WasmInvalidCandidate::HardTimeout, - ))), + MockValidateCandidateBackend::with_hardcoded_result(Err( + ValidationError::InvalidCandidate(WasmInvalidCandidate::HardTimeout), + )), validation_data, validation_code, descriptor, @@ -534,7 +539,7 @@ fn compressed_code_works() { }; let v = executor::block_on(validate_candidate_exhaustive( - MockValidatorBackend::with_hardcoded_result(Ok(validation_result)), + MockValidateCandidateBackend::with_hardcoded_result(Ok(validation_result)), validation_data, validation_code, descriptor, @@ -574,7 +579,7 @@ fn code_decompression_failure_is_invalid() { }; let v = executor::block_on(validate_candidate_exhaustive( - MockValidatorBackend::with_hardcoded_result(Ok(validation_result)), + MockValidateCandidateBackend::with_hardcoded_result(Ok(validation_result)), validation_data, validation_code, descriptor, @@ -615,7 +620,7 @@ fn pov_decompression_failure_is_invalid() { }; let v = executor::block_on(validate_candidate_exhaustive( - MockValidatorBackend::with_hardcoded_result(Ok(validation_result)), + MockValidateCandidateBackend::with_hardcoded_result(Ok(validation_result)), validation_data, validation_code, descriptor, @@ -626,3 +631,166 @@ fn pov_decompression_failure_is_invalid() { assert_matches!(v, Ok(ValidationResult::Invalid(InvalidCandidate::PoVDecompressionFailure))); } + +struct MockPreCheckBackend { + result: Result<(), PrepareError>, +} + +impl MockPreCheckBackend { + fn with_hardcoded_result(result: Result<(), PrepareError>) -> Self { + Self { result } + } +} + +#[async_trait] +impl ValidationBackend for MockPreCheckBackend { + async fn validate_candidate( + &mut self, + _raw_validation_code: Vec, + _timeout: Duration, + _params: ValidationParams, + ) -> Result { + unreachable!() + } + + async fn precheck_pvf(&mut self, _pvf: Pvf) -> Result<(), PrepareError> { + self.result.clone() + } +} + +#[test] +fn precheck_works() { + let relay_parent = [3; 32].into(); + let validation_code = ValidationCode(vec![3; 16]); + let validation_code_hash = validation_code.hash(); + + let pool = TaskExecutor::new(); + let (mut ctx, mut ctx_handle) = + test_helpers::make_subsystem_context::(pool.clone()); + + let (check_fut, check_result) = precheck_pvf( + ctx.sender(), + MockPreCheckBackend::with_hardcoded_result(Ok(())), + relay_parent, + validation_code_hash, + ) + .remote_handle(); + + let test_fut = async move { + assert_matches!( + ctx_handle.recv().await, + AllMessages::RuntimeApi(RuntimeApiMessage::Request( + rp, + RuntimeApiRequest::ValidationCodeByHash( + vch, + tx + ), + )) => { + assert_eq!(vch, validation_code_hash); + assert_eq!(rp, relay_parent); + + let _ = tx.send(Ok(Some(validation_code.clone()))); + } + ); + assert_matches!(check_result.await, PreCheckOutcome::Valid); + }; + + let test_fut = future::join(test_fut, check_fut); + executor::block_on(test_fut); +} + +#[test] +fn precheck_invalid_pvf_blob_compression() { + let relay_parent = [3; 32].into(); + + let raw_code = vec![2u8; VALIDATION_CODE_BOMB_LIMIT + 1]; + let validation_code = + sp_maybe_compressed_blob::compress(&raw_code, VALIDATION_CODE_BOMB_LIMIT + 1) + .map(ValidationCode) + .unwrap(); + let validation_code_hash = validation_code.hash(); + + let pool = TaskExecutor::new(); + let (mut ctx, mut ctx_handle) = + test_helpers::make_subsystem_context::(pool.clone()); + + let (check_fut, check_result) = precheck_pvf( + ctx.sender(), + MockPreCheckBackend::with_hardcoded_result(Ok(())), + relay_parent, + validation_code_hash, + ) + .remote_handle(); + + let test_fut = async move { + assert_matches!( + ctx_handle.recv().await, + AllMessages::RuntimeApi(RuntimeApiMessage::Request( + rp, + RuntimeApiRequest::ValidationCodeByHash( + vch, + tx + ), + )) => { + assert_eq!(vch, validation_code_hash); + assert_eq!(rp, relay_parent); + + let _ = tx.send(Ok(Some(validation_code.clone()))); + } + ); + assert_matches!(check_result.await, PreCheckOutcome::Invalid); + }; + + let test_fut = future::join(test_fut, check_fut); + executor::block_on(test_fut); +} + +#[test] +fn precheck_properly_classifies_outcomes() { + let inner = |prepare_result, precheck_outcome| { + let relay_parent = [3; 32].into(); + let validation_code = ValidationCode(vec![3; 16]); + let validation_code_hash = validation_code.hash(); + + let pool = TaskExecutor::new(); + let (mut ctx, mut ctx_handle) = + test_helpers::make_subsystem_context::(pool.clone()); + + let (check_fut, check_result) = precheck_pvf( + ctx.sender(), + MockPreCheckBackend::with_hardcoded_result(prepare_result), + relay_parent, + validation_code_hash, + ) + .remote_handle(); + + let test_fut = async move { + assert_matches!( + ctx_handle.recv().await, + AllMessages::RuntimeApi(RuntimeApiMessage::Request( + rp, + RuntimeApiRequest::ValidationCodeByHash( + vch, + tx + ), + )) => { + assert_eq!(vch, validation_code_hash); + assert_eq!(rp, relay_parent); + + let _ = tx.send(Ok(Some(validation_code.clone()))); + } + ); + assert_eq!(check_result.await, precheck_outcome); + }; + + let test_fut = future::join(test_fut, check_fut); + executor::block_on(test_fut); + }; + + inner(Err(PrepareError::Prevalidation("foo".to_owned())), PreCheckOutcome::Invalid); + inner(Err(PrepareError::Preparation("bar".to_owned())), PreCheckOutcome::Invalid); + inner(Err(PrepareError::Panic("baz".to_owned())), PreCheckOutcome::Invalid); + + inner(Err(PrepareError::TimedOut), PreCheckOutcome::Failed); + inner(Err(PrepareError::DidNotMakeIt), PreCheckOutcome::Failed); +} diff --git a/polkadot/node/subsystem-types/src/messages.rs b/polkadot/node/subsystem-types/src/messages.rs index 48baf44aac..6be22c2b5b 100644 --- a/polkadot/node/subsystem-types/src/messages.rs +++ b/polkadot/node/subsystem-types/src/messages.rs @@ -93,6 +93,26 @@ impl BoundToRelayParent for CandidateBackingMessage { #[error("Validation failed with {0:?}")] pub struct ValidationFailed(pub String); +/// The outcome of the candidate-validation's PVF pre-check request. +#[derive(Debug, PartialEq)] +pub enum PreCheckOutcome { + /// The PVF has been compiled successfully within the given constraints. + Valid, + /// The PVF could not be compiled. This variant is used when the candidate-validation subsystem + /// can be sure that the PVF is invalid. To give a couple of examples: a PVF that cannot be + /// decompressed or that does not represent a structurally valid WebAssembly file. + Invalid, + /// This variant is used when the PVF cannot be compiled but for other reasons that are not + /// included into [`PreCheckOutcome::Invalid`]. This variant can indicate that the PVF in + /// question is invalid, however it is not necessary that PVF that received this judgement + /// is invalid. + /// + /// For example, if during compilation the preparation worker was killed we cannot be sure why + /// it happened: because the PVF was malicious made the worker to use too much memory or its + /// because the host machine is under severe memory pressure and it decided to kill the worker. + Failed, +} + /// Messages received by the Validation subsystem. /// /// ## Validation Requests @@ -137,6 +157,17 @@ pub enum CandidateValidationMessage { Duration, oneshot::Sender>, ), + /// Try to compile the given validation code and send back + /// the outcome. + /// + /// The validation code is specified by the hash and will be queried from the runtime API at the + /// given relay-parent. + PreCheck( + // Relay-parent + Hash, + ValidationCodeHash, + oneshot::Sender, + ), } impl CandidateValidationMessage { @@ -145,6 +176,7 @@ impl CandidateValidationMessage { match self { Self::ValidateFromChainState(_, _, _, _) => None, Self::ValidateFromExhaustive(_, _, _, _, _, _) => None, + Self::PreCheck(relay_parent, _, _) => Some(*relay_parent), } } } diff --git a/polkadot/scripts/gitlab/lingua.dic b/polkadot/scripts/gitlab/lingua.dic index 44c0d230f1..14d85a6d3e 100644 --- a/polkadot/scripts/gitlab/lingua.dic +++ b/polkadot/scripts/gitlab/lingua.dic @@ -124,6 +124,7 @@ isolate/BG iterable jaeger/MS js +judgement keccak256/M keypair/MS keystore/MS