From 094d1647415f82c0a79c6496cb9041cae23a8019 Mon Sep 17 00:00:00 2001 From: Chris Sosnin Date: Thu, 19 Aug 2021 17:44:02 +0300 Subject: [PATCH] Reliable error handling --- node/core/candidate-validation/src/lib.rs | 107 ++++++++++++-------- node/core/candidate-validation/src/tests.rs | 28 +++++ node/core/pvf/src/lib.rs | 2 +- 3 files changed, 91 insertions(+), 46 deletions(-) diff --git a/node/core/candidate-validation/src/lib.rs b/node/core/candidate-validation/src/lib.rs index 9555b546d6cf..13a17af3e1f5 100644 --- a/node/core/candidate-validation/src/lib.rs +++ b/node/core/candidate-validation/src/lib.rs @@ -152,6 +152,7 @@ where let _timer = metrics.time_validate_from_exhaustive(); let res = validate_candidate_exhaustive( + &mut ctx, &mut validation_host, persisted_validation_data, Some(validation_code), @@ -314,41 +315,17 @@ where return Ok(Err(ValidationFailed("Assumption Check: Bad request".into()))), }; - let mut validation_result = validate_candidate_exhaustive( - &mut validation_host.clone(), - validation_data.clone(), + let validation_result = validate_candidate_exhaustive( + ctx, + validation_host, + validation_data, None, descriptor.clone(), - pov.clone(), + pov, metrics, ) .await; - // In case preimage for the supplied code hash was not found by the - // validation host, request the code from Runtime API and try again. - if let Ok(Err(ref err)) = validation_result { - if err.0.as_str() == "Code not found" { - let validation_code = match request_validation_code_by_hash(ctx, &descriptor).await? { - Ok(Some(validation_code)) => validation_code, - Ok(None) => - return Ok(Err(ValidationFailed( - "Runtime API didn't return validation code".into(), - ))), - Err(err) => return Ok(Err(ValidationFailed(err.to_string()))), - }; - - validation_result = validate_candidate_exhaustive( - validation_host, - validation_data, - Some(validation_code), - descriptor.clone(), - pov, - metrics, - ) - .await; - } - } - if let Ok(Ok(ValidationResult::Valid(ref outputs, _))) = validation_result { let (tx, rx) = oneshot::channel(); match runtime_api_request( @@ -366,27 +343,22 @@ where } } - if let Ok(Err(ref err)) = validation_result { - if err.0.as_str() == "Code not found" { - tracing::error!( - target: LOG_TARGET, - para_id = ?descriptor.para_id, - "Failed to validate candidate: validation code not found in cache" - ); - } - } - validation_result } -async fn validate_candidate_exhaustive( +async fn validate_candidate_exhaustive( + ctx: &mut Context, mut validation_backend: impl ValidationBackend, persisted_validation_data: PersistedValidationData, validation_code: Option, descriptor: CandidateDescriptor, pov: Arc, metrics: &Metrics, -) -> SubsystemResult> { +) -> SubsystemResult> +where + Context: SubsystemContext, + Context: overseer::SubsystemContext, +{ let _timer = metrics.time_validate_candidate_exhaustive(); let validation_code_hash = validation_code.as_ref().map(ValidationCode::hash); @@ -445,7 +417,7 @@ async fn validate_candidate_exhaustive( relay_parent_storage_root: persisted_validation_data.relay_parent_storage_root, }; - let result = validation_backend.validate_candidate(validation_code, params).await; + let mut result = validation_backend.validate_candidate(validation_code, params.clone()).await; if let Err(ref e) = result { tracing::debug!( @@ -453,6 +425,43 @@ async fn validate_candidate_exhaustive( error = ?e, "Failed to validate candidate", ); + + // In case preimage for the supplied code hash was not found by the + // validation host, request the code from Runtime API and try again. + if let &ValidationError::ArtifactNotFound = e { + tracing::debug!( + target: LOG_TARGET, + "Validation host failed to find artifact by provided hash", + ); + + let validation_code = match request_validation_code_by_hash(ctx, &descriptor).await? { + Ok(Some(validation_code)) => validation_code, + Ok(None) => + return Ok(Err(ValidationFailed( + "Runtime API didn't return validation code by hash".into(), + ))), + Err(err) => return Ok(Err(ValidationFailed(err.to_string()))), + }; + + let validation_code = Pvf::from_code( + match sp_maybe_compressed_blob::decompress( + &validation_code.0, + VALIDATION_CODE_BOMB_LIMIT, + ) { + Ok(code) => code, + Err(e) => { + tracing::debug!(target: LOG_TARGET, err=?e, "Invalid validation code"); + + // If the validation code is invalid, the candidate certainly is. + return Ok(Ok(ValidationResult::Invalid( + InvalidCandidate::CodeDecompressionFailure, + ))) + }, + } + .to_vec(), + ); + result = validation_backend.validate_candidate(validation_code, params).await; + } } let result = match result { @@ -466,9 +475,17 @@ async fn validate_candidate_exhaustive( Ok(ValidationResult::Invalid(InvalidCandidate::ExecutionError( "ambigious worker death".to_string(), ))), - Err(ValidationError::ArtifactNotFound) => - Err(ValidationFailed("Code not found".to_string())), - + Err(ValidationError::ArtifactNotFound) => { + // The code was supplied on the second attempt, this + // error should be unreachable. + tracing::error!( + target: LOG_TARGET, + "Unexpected error received from the validation host" + ); + Err(ValidationFailed( + "Validation host failed to find artifact even though it was supplied".to_string(), + )) + }, Ok(res) => if res.head_data.hash() != descriptor.para_head { Ok(ValidationResult::Invalid(InvalidCandidate::ParaHeadHashMismatch)) diff --git a/node/core/candidate-validation/src/tests.rs b/node/core/candidate-validation/src/tests.rs index 326d6550425b..b0ca40c1d079 100644 --- a/node/core/candidate-validation/src/tests.rs +++ b/node/core/candidate-validation/src/tests.rs @@ -279,6 +279,9 @@ impl ValidationBackend for MockValidatorBackend { #[test] fn candidate_validation_ok_is_ok() { + let pool = TaskExecutor::new(); + let (mut ctx, mut _ctx_handle) = test_helpers::make_subsystem_context(pool.clone()); + let validation_data = PersistedValidationData { max_pov_size: 1024, ..Default::default() }; let pov = PoV { block_data: BlockData(vec![1; 32]) }; @@ -309,6 +312,7 @@ fn candidate_validation_ok_is_ok() { }; let v = executor::block_on(validate_candidate_exhaustive( + &mut ctx, MockValidatorBackend::with_hardcoded_result(Ok(validation_result)), validation_data.clone(), Some(validation_code), @@ -331,6 +335,9 @@ fn candidate_validation_ok_is_ok() { #[test] fn candidate_validation_bad_return_is_invalid() { + let pool = TaskExecutor::new(); + let (mut ctx, mut _ctx_handle) = test_helpers::make_subsystem_context(pool.clone()); + let validation_data = PersistedValidationData { max_pov_size: 1024, ..Default::default() }; let pov = PoV { block_data: BlockData(vec![1; 32]) }; @@ -350,6 +357,7 @@ fn candidate_validation_bad_return_is_invalid() { assert!(check.is_ok()); let v = executor::block_on(validate_candidate_exhaustive( + &mut ctx, MockValidatorBackend::with_hardcoded_result(Err(ValidationError::InvalidCandidate( WasmInvalidCandidate::AmbigiousWorkerDeath, ))), @@ -367,6 +375,9 @@ fn candidate_validation_bad_return_is_invalid() { #[test] fn candidate_validation_timeout_is_internal_error() { + let pool = TaskExecutor::new(); + let (mut ctx, mut _ctx_handle) = test_helpers::make_subsystem_context(pool.clone()); + let validation_data = PersistedValidationData { max_pov_size: 1024, ..Default::default() }; let pov = PoV { block_data: BlockData(vec![1; 32]) }; @@ -386,6 +397,7 @@ fn candidate_validation_timeout_is_internal_error() { assert!(check.is_ok()); let v = executor::block_on(validate_candidate_exhaustive( + &mut ctx, MockValidatorBackend::with_hardcoded_result(Err(ValidationError::InvalidCandidate( WasmInvalidCandidate::HardTimeout, ))), @@ -402,6 +414,9 @@ fn candidate_validation_timeout_is_internal_error() { #[test] fn candidate_validation_code_mismatch_is_invalid() { + let pool = TaskExecutor::new(); + let (mut ctx, mut _ctx_handle) = test_helpers::make_subsystem_context(pool.clone()); + let validation_data = PersistedValidationData { max_pov_size: 1024, ..Default::default() }; let pov = PoV { block_data: BlockData(vec![1; 32]) }; @@ -421,6 +436,7 @@ fn candidate_validation_code_mismatch_is_invalid() { assert_matches!(check, Err(InvalidCandidate::CodeHashMismatch)); let v = executor::block_on(validate_candidate_exhaustive( + &mut ctx, MockValidatorBackend::with_hardcoded_result(Err(ValidationError::InvalidCandidate( WasmInvalidCandidate::HardTimeout, ))), @@ -438,6 +454,9 @@ fn candidate_validation_code_mismatch_is_invalid() { #[test] fn compressed_code_works() { + let pool = TaskExecutor::new(); + let (mut ctx, mut _ctx_handle) = test_helpers::make_subsystem_context(pool.clone()); + let validation_data = PersistedValidationData { max_pov_size: 1024, ..Default::default() }; let pov = PoV { block_data: BlockData(vec![1; 32]) }; let head_data = HeadData(vec![1, 1, 1]); @@ -463,6 +482,7 @@ fn compressed_code_works() { }; let v = executor::block_on(validate_candidate_exhaustive( + &mut ctx, MockValidatorBackend::with_hardcoded_result(Ok(validation_result)), validation_data, Some(validation_code), @@ -477,6 +497,9 @@ fn compressed_code_works() { #[test] fn code_decompression_failure_is_invalid() { + let pool = TaskExecutor::new(); + let (mut ctx, mut _ctx_handle) = test_helpers::make_subsystem_context(pool.clone()); + let validation_data = PersistedValidationData { max_pov_size: 1024, ..Default::default() }; let pov = PoV { block_data: BlockData(vec![1; 32]) }; let head_data = HeadData(vec![1, 1, 1]); @@ -503,6 +526,7 @@ fn code_decompression_failure_is_invalid() { }; let v = executor::block_on(validate_candidate_exhaustive( + &mut ctx, MockValidatorBackend::with_hardcoded_result(Ok(validation_result)), validation_data, Some(validation_code), @@ -517,6 +541,9 @@ fn code_decompression_failure_is_invalid() { #[test] fn pov_decompression_failure_is_invalid() { + let pool = TaskExecutor::new(); + let (mut ctx, mut _ctx_handle) = test_helpers::make_subsystem_context(pool.clone()); + let validation_data = PersistedValidationData { max_pov_size: POV_BOMB_LIMIT as u32, ..Default::default() }; let head_data = HeadData(vec![1, 1, 1]); @@ -544,6 +571,7 @@ fn pov_decompression_failure_is_invalid() { }; let v = executor::block_on(validate_candidate_exhaustive( + &mut ctx, MockValidatorBackend::with_hardcoded_result(Ok(validation_result)), validation_data, Some(validation_code), diff --git a/node/core/pvf/src/lib.rs b/node/core/pvf/src/lib.rs index bd72a5e1ed08..08c4f2617f66 100644 --- a/node/core/pvf/src/lib.rs +++ b/node/core/pvf/src/lib.rs @@ -93,7 +93,7 @@ pub use sp_tracing; pub use error::{InvalidCandidate, ValidationError}; pub use priority::Priority; -pub use pvf::Pvf; +pub use pvf::{Pvf, PvfPreimage}; pub use host::{start, Config, ValidationHost};