Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Commit

Permalink
Reliable error handling
Browse files Browse the repository at this point in the history
  • Loading branch information
@slumber
slumber committed Aug 19, 2021
1 parent 7311c27 commit 094d164
Show file tree
Hide file tree
Showing 3 changed files with 91 additions and 46 deletions.
107 changes: 62 additions & 45 deletions node/core/candidate-validation/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -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(
Expand All @@ -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<Context>(
ctx: &mut Context,
mut validation_backend: impl ValidationBackend,
persisted_validation_data: PersistedValidationData,
validation_code: Option<ValidationCode>,
descriptor: CandidateDescriptor,
pov: Arc<PoV>,
metrics: &Metrics,
) -> SubsystemResult<Result<ValidationResult, ValidationFailed>> {
) -> SubsystemResult<Result<ValidationResult, ValidationFailed>>
where
Context: SubsystemContext<Message = CandidateValidationMessage>,
Context: overseer::SubsystemContext<Message = CandidateValidationMessage>,
{
let _timer = metrics.time_validate_candidate_exhaustive();

let validation_code_hash = validation_code.as_ref().map(ValidationCode::hash);
Expand Down Expand Up @@ -445,14 +417,51 @@ 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!(
target: LOG_TARGET,
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 {
Expand All @@ -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))
Expand Down
28 changes: 28 additions & 0 deletions node/core/candidate-validation/src/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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]) };
Expand Down Expand Up @@ -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),
Expand All @@ -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]) };
Expand All @@ -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,
))),
Expand All @@ -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]) };
Expand All @@ -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,
))),
Expand All @@ -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]) };
Expand All @@ -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,
))),
Expand All @@ -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]);
Expand All @@ -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),
Expand All @@ -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]);
Expand All @@ -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),
Expand All @@ -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]);
Expand Down Expand Up @@ -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),
Expand Down
2 changes: 1 addition & 1 deletion node/core/pvf/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down

0 comments on commit 094d164

Please sign in to comment.