Skip to content

Commit

Permalink
Merge branch 'master' into wk-231018-prdoc-new-schema
Browse files Browse the repository at this point in the history
  • Loading branch information
chevdor authored Nov 21, 2023
2 parents 148175f + 552be48 commit 36547bd
Show file tree
Hide file tree
Showing 32 changed files with 494 additions and 598 deletions.
4 changes: 0 additions & 4 deletions .gitlab-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ variables:
RUSTY_CACHIER_COMPRESSION_METHOD: zstd
NEXTEST_FAILURE_OUTPUT: immediate-final
NEXTEST_SUCCESS_OUTPUT: final
ZOMBIENET_IMAGE: "docker.io/paritytech/zombienet:v1.3.80"
DOCKER_IMAGES_VERSION: "${CI_COMMIT_SHA}"

default:
Expand Down Expand Up @@ -199,9 +198,6 @@ default:
- if: $CI_COMMIT_REF_NAME =~ /^[0-9]+$/ # PRs
- if: $CI_COMMIT_REF_NAME =~ /^gh-readonly-queue.*$/ # merge queues

.zombienet-refs:
extends: .build-refs

include:
# check jobs
- .gitlab/pipeline/check.yml
Expand Down
5 changes: 5 additions & 0 deletions .gitlab/pipeline/zombienet.yml
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
.zombienet-refs:
extends: .build-refs
variables:
ZOMBIENET_IMAGE: "docker.io/paritytech/zombienet:v1.3.82"

include:
# substrate tests
- .gitlab/pipeline/zombienet/substrate.yml
Expand Down
15 changes: 7 additions & 8 deletions .gitlab/pipeline/zombienet/substrate.yml
Original file line number Diff line number Diff line change
Expand Up @@ -40,14 +40,13 @@
tags:
- zombienet-polkadot-integration-test

# Skip this one until PolkadotJS includes `SkipCheckIfFeeless` extension
# zombienet-substrate-0000-block-building:
# extends:
# - .zombienet-substrate-common
# script:
# - /home/nonroot/zombie-net/scripts/ci/run-test-local-env-manager.sh
# --local-dir="${LOCAL_DIR}/0000-block-building"
# --test="block-building.zndsl"
zombienet-substrate-0000-block-building:
extends:
- .zombienet-substrate-common
script:
- /home/nonroot/zombie-net/scripts/ci/run-test-local-env-manager.sh
--local-dir="${LOCAL_DIR}/0000-block-building"
--test="block-building.zndsl"

zombienet-substrate-0001-basic-warp-sync:
extends:
Expand Down
7 changes: 0 additions & 7 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

60 changes: 35 additions & 25 deletions polkadot/node/core/candidate-validation/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@
#![warn(missing_docs)]

use polkadot_node_core_pvf::{
InternalValidationError, InvalidCandidate as WasmInvalidCandidate, PrepareError,
PrepareJobKind, PvfPrepData, ValidationError, ValidationHost,
InternalValidationError, InvalidCandidate as WasmInvalidCandidate, PossiblyInvalidError,
PrepareError, PrepareJobKind, PvfPrepData, ValidationError, ValidationHost,
};
use polkadot_node_primitives::{
BlockData, InvalidCandidate, PoV, ValidationResult, POV_BOMB_LIMIT, VALIDATION_CODE_BOMB_LIMIT,
Expand Down Expand Up @@ -642,7 +642,7 @@ async fn validate_candidate_exhaustive(
}

match result {
Err(ValidationError::InternalError(e)) => {
Err(ValidationError::Internal(e)) => {
gum::warn!(
target: LOG_TARGET,
?para_id,
Expand All @@ -651,34 +651,29 @@ async fn validate_candidate_exhaustive(
);
Err(ValidationFailed(e.to_string()))
},
Err(ValidationError::InvalidCandidate(WasmInvalidCandidate::HardTimeout)) =>
Err(ValidationError::Invalid(WasmInvalidCandidate::HardTimeout)) =>
Ok(ValidationResult::Invalid(InvalidCandidate::Timeout)),
Err(ValidationError::InvalidCandidate(WasmInvalidCandidate::WorkerReportedInvalid(e))) =>
Err(ValidationError::Invalid(WasmInvalidCandidate::WorkerReportedInvalid(e))) =>
Ok(ValidationResult::Invalid(InvalidCandidate::ExecutionError(e))),
Err(ValidationError::InvalidCandidate(WasmInvalidCandidate::AmbiguousWorkerDeath)) =>
Err(ValidationError::PossiblyInvalid(PossiblyInvalidError::AmbiguousWorkerDeath)) =>
Ok(ValidationResult::Invalid(InvalidCandidate::ExecutionError(
"ambiguous worker death".to_string(),
))),
Err(ValidationError::InvalidCandidate(WasmInvalidCandidate::JobError(err))) =>
Err(ValidationError::PossiblyInvalid(PossiblyInvalidError::JobError(err))) =>
Ok(ValidationResult::Invalid(InvalidCandidate::ExecutionError(err))),

Err(ValidationError::InvalidCandidate(WasmInvalidCandidate::AmbiguousJobDeath(err))) =>
Err(ValidationError::PossiblyInvalid(PossiblyInvalidError::AmbiguousJobDeath(err))) =>
Ok(ValidationResult::Invalid(InvalidCandidate::ExecutionError(format!(
"ambiguous job death: {err}"
)))),
Err(ValidationError::InvalidCandidate(WasmInvalidCandidate::PrepareError(e))) => {
// In principle if preparation of the `WASM` fails, the current candidate can not be the
// reason for that. So we can't say whether it is invalid or not. In addition, with
// pre-checking enabled only valid runtimes should ever get enacted, so we can be
// reasonably sure that this is some local problem on the current node. However, as this
// particular error *seems* to indicate a deterministic error, we raise a warning.
Err(ValidationError::Preparation(e)) => {
gum::warn!(
target: LOG_TARGET,
?para_id,
?e,
"Deterministic error occurred during preparation (should have been ruled out by pre-checking phase)",
);
Err(ValidationFailed(e))
Err(ValidationFailed(e.to_string()))
},
Ok(res) =>
if res.head_data.hash() != candidate_receipt.descriptor.para_head {
Expand Down Expand Up @@ -762,16 +757,31 @@ trait ValidationBackend {
}

match validation_result {
Err(ValidationError::InvalidCandidate(
WasmInvalidCandidate::AmbiguousWorkerDeath |
WasmInvalidCandidate::AmbiguousJobDeath(_),
)) if num_death_retries_left > 0 => num_death_retries_left -= 1,
Err(ValidationError::InvalidCandidate(WasmInvalidCandidate::JobError(_)))
if num_job_error_retries_left > 0 =>
num_job_error_retries_left -= 1,
Err(ValidationError::InternalError(_)) if num_internal_retries_left > 0 =>
num_internal_retries_left -= 1,
_ => break,
Err(ValidationError::PossiblyInvalid(
PossiblyInvalidError::AmbiguousWorkerDeath |
PossiblyInvalidError::AmbiguousJobDeath(_),
)) =>
if num_death_retries_left > 0 {
num_death_retries_left -= 1;
} else {
break;
},

Err(ValidationError::PossiblyInvalid(PossiblyInvalidError::JobError(_))) =>
if num_job_error_retries_left > 0 {
num_job_error_retries_left -= 1;
} else {
break;
},

Err(ValidationError::Internal(_)) =>
if num_internal_retries_left > 0 {
num_internal_retries_left -= 1;
} else {
break;
},

Ok(_) | Err(ValidationError::Invalid(_) | ValidationError::Preparation(_)) => break,
}

// If we got a possibly transient error, retry once after a brief delay, on the
Expand Down
40 changes: 20 additions & 20 deletions polkadot/node/core/candidate-validation/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -480,9 +480,9 @@ fn candidate_validation_bad_return_is_invalid() {
let candidate_receipt = CandidateReceipt { descriptor, commitments_hash: Hash::zero() };

let v = executor::block_on(validate_candidate_exhaustive(
MockValidateCandidateBackend::with_hardcoded_result(Err(
ValidationError::InvalidCandidate(WasmInvalidCandidate::HardTimeout),
)),
MockValidateCandidateBackend::with_hardcoded_result(Err(ValidationError::Invalid(
WasmInvalidCandidate::HardTimeout,
))),
validation_data,
validation_code,
candidate_receipt,
Expand Down Expand Up @@ -561,7 +561,7 @@ fn candidate_validation_one_ambiguous_error_is_valid() {

let v = executor::block_on(validate_candidate_exhaustive(
MockValidateCandidateBackend::with_hardcoded_result_list(vec![
Err(ValidationError::InvalidCandidate(WasmInvalidCandidate::AmbiguousWorkerDeath)),
Err(ValidationError::PossiblyInvalid(PossiblyInvalidError::AmbiguousWorkerDeath)),
Ok(validation_result),
]),
validation_data.clone(),
Expand Down Expand Up @@ -602,8 +602,8 @@ fn candidate_validation_multiple_ambiguous_errors_is_invalid() {

let v = executor::block_on(validate_candidate_exhaustive(
MockValidateCandidateBackend::with_hardcoded_result_list(vec![
Err(ValidationError::InvalidCandidate(WasmInvalidCandidate::AmbiguousWorkerDeath)),
Err(ValidationError::InvalidCandidate(WasmInvalidCandidate::AmbiguousWorkerDeath)),
Err(ValidationError::PossiblyInvalid(PossiblyInvalidError::AmbiguousWorkerDeath)),
Err(ValidationError::PossiblyInvalid(PossiblyInvalidError::AmbiguousWorkerDeath)),
]),
validation_data,
validation_code,
Expand All @@ -626,7 +626,7 @@ fn candidate_validation_retry_internal_errors() {
vec![
Err(InternalValidationError::HostCommunication("foo".into()).into()),
// Throw an AJD error, we should still retry again.
Err(ValidationError::InvalidCandidate(WasmInvalidCandidate::AmbiguousJobDeath(
Err(ValidationError::PossiblyInvalid(PossiblyInvalidError::AmbiguousJobDeath(
"baz".into(),
))),
// Throw another internal error.
Expand All @@ -644,7 +644,7 @@ fn candidate_validation_dont_retry_internal_errors() {
vec![
Err(InternalValidationError::HostCommunication("foo".into()).into()),
// Throw an AWD error, we should still retry again.
Err(ValidationError::InvalidCandidate(WasmInvalidCandidate::AmbiguousWorkerDeath)),
Err(ValidationError::PossiblyInvalid(PossiblyInvalidError::AmbiguousWorkerDeath)),
// Throw another internal error.
Err(InternalValidationError::HostCommunication("bar".into()).into()),
],
Expand All @@ -659,11 +659,11 @@ fn candidate_validation_retry_panic_errors() {
let v = candidate_validation_retry_on_error_helper(
PvfExecKind::Approval,
vec![
Err(ValidationError::InvalidCandidate(WasmInvalidCandidate::JobError("foo".into()))),
Err(ValidationError::PossiblyInvalid(PossiblyInvalidError::JobError("foo".into()))),
// Throw an AWD error, we should still retry again.
Err(ValidationError::InvalidCandidate(WasmInvalidCandidate::AmbiguousWorkerDeath)),
Err(ValidationError::PossiblyInvalid(PossiblyInvalidError::AmbiguousWorkerDeath)),
// Throw another panic error.
Err(ValidationError::InvalidCandidate(WasmInvalidCandidate::JobError("bar".into()))),
Err(ValidationError::PossiblyInvalid(PossiblyInvalidError::JobError("bar".into()))),
],
);

Expand All @@ -676,11 +676,11 @@ fn candidate_validation_dont_retry_panic_errors() {
let v = candidate_validation_retry_on_error_helper(
PvfExecKind::Backing,
vec![
Err(ValidationError::InvalidCandidate(WasmInvalidCandidate::JobError("foo".into()))),
Err(ValidationError::PossiblyInvalid(PossiblyInvalidError::JobError("foo".into()))),
// Throw an AWD error, we should still retry again.
Err(ValidationError::InvalidCandidate(WasmInvalidCandidate::AmbiguousWorkerDeath)),
Err(ValidationError::PossiblyInvalid(PossiblyInvalidError::AmbiguousWorkerDeath)),
// Throw another panic error.
Err(ValidationError::InvalidCandidate(WasmInvalidCandidate::JobError("bar".into()))),
Err(ValidationError::PossiblyInvalid(PossiblyInvalidError::JobError("bar".into()))),
],
);

Expand Down Expand Up @@ -758,9 +758,9 @@ fn candidate_validation_timeout_is_internal_error() {
let candidate_receipt = CandidateReceipt { descriptor, commitments_hash: Hash::zero() };

let v = executor::block_on(validate_candidate_exhaustive(
MockValidateCandidateBackend::with_hardcoded_result(Err(
ValidationError::InvalidCandidate(WasmInvalidCandidate::HardTimeout),
)),
MockValidateCandidateBackend::with_hardcoded_result(Err(ValidationError::Invalid(
WasmInvalidCandidate::HardTimeout,
))),
validation_data,
validation_code,
candidate_receipt,
Expand Down Expand Up @@ -852,9 +852,9 @@ fn candidate_validation_code_mismatch_is_invalid() {
let (_ctx, _ctx_handle) = test_helpers::make_subsystem_context::<AllMessages, _>(pool.clone());

let v = executor::block_on(validate_candidate_exhaustive(
MockValidateCandidateBackend::with_hardcoded_result(Err(
ValidationError::InvalidCandidate(WasmInvalidCandidate::HardTimeout),
)),
MockValidateCandidateBackend::with_hardcoded_result(Err(ValidationError::Invalid(
WasmInvalidCandidate::HardTimeout,
))),
validation_data,
validation_code,
candidate_receipt,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,7 @@
use criterion::{criterion_group, criterion_main, BatchSize, Criterion, SamplingMode};
use polkadot_node_core_pvf::{
start, testing, Config, Metrics, PrepareError, PrepareJobKind, PrepareStats, PvfPrepData,
ValidationHost,
start, testing, Config, Metrics, PrepareError, PrepareJobKind, PvfPrepData, ValidationHost,
};
use polkadot_primitives::ExecutorParams;
use rococo_runtime::WASM_BINARY;
Expand Down
7 changes: 4 additions & 3 deletions polkadot/node/core/pvf/common/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ pub enum PrepareError {
#[codec(index = 9)]
ClearWorkerDir(String),
/// The preparation job process died, due to OOM, a seccomp violation, or some other factor.
JobDied(String),
JobDied { err: String, job_pid: i32 },
#[codec(index = 10)]
/// Some error occurred when interfacing with the kernel.
#[codec(index = 11)]
Expand All @@ -96,7 +96,7 @@ impl PrepareError {
match self {
Prevalidation(_) | Preparation(_) | JobError(_) | OutOfMemory => true,
IoErr(_) |
JobDied(_) |
JobDied { .. } |
CreateTmpFile(_) |
RenameTmpFile { .. } |
ClearWorkerDir(_) |
Expand All @@ -119,7 +119,8 @@ impl fmt::Display for PrepareError {
JobError(err) => write!(f, "panic: {}", err),
TimedOut => write!(f, "prepare: timeout"),
IoErr(err) => write!(f, "prepare: io error while receiving response: {}", err),
JobDied(err) => write!(f, "prepare: prepare job died: {}", err),
JobDied { err, job_pid } =>
write!(f, "prepare: prepare job with pid {job_pid} died: {err}"),
CreateTmpFile(err) => write!(f, "prepare: error creating tmp file: {}", err),
RenameTmpFile { err, src, dest } =>
write!(f, "prepare: error renaming tmp file ({:?} -> {:?}): {}", src, dest, err),
Expand Down
2 changes: 1 addition & 1 deletion polkadot/node/core/pvf/common/src/execute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ pub enum WorkerResponse {
///
/// We cannot treat this as an internal error because malicious code may have killed the job.
/// We still retry it, because in the non-malicious case it is likely spurious.
JobDied(String),
JobDied { err: String, job_pid: i32 },
/// An unexpected error occurred in the job process, e.g. failing to spawn a thread, panic,
/// etc.
///
Expand Down
19 changes: 8 additions & 11 deletions polkadot/node/core/pvf/common/src/executor_intf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,22 +140,20 @@ pub unsafe fn create_runtime_from_artifact_bytes(
executor_params: &ExecutorParams,
) -> Result<WasmtimeRuntime, WasmError> {
let mut config = DEFAULT_CONFIG.clone();
config.semantics =
params_to_wasmtime_semantics(executor_params).map_err(|err| WasmError::Other(err))?;
config.semantics = params_to_wasmtime_semantics(executor_params);

sc_executor_wasmtime::create_runtime_from_artifact_bytes::<HostFunctions>(
compiled_artifact_blob,
config,
)
}

pub fn params_to_wasmtime_semantics(par: &ExecutorParams) -> Result<Semantics, String> {
pub fn params_to_wasmtime_semantics(par: &ExecutorParams) -> Semantics {
let mut sem = DEFAULT_CONFIG.semantics.clone();
let mut stack_limit = if let Some(stack_limit) = sem.deterministic_stack_limit.clone() {
stack_limit
} else {
return Err("No default stack limit set".to_owned())
};
let mut stack_limit = sem
.deterministic_stack_limit
.expect("There is a comment to not change the default stack limit; it should always be available; qed")
.clone();

for p in par.iter() {
match p {
Expand All @@ -172,7 +170,7 @@ pub fn params_to_wasmtime_semantics(par: &ExecutorParams) -> Result<Semantics, S
}
}
sem.deterministic_stack_limit = Some(stack_limit);
Ok(sem)
sem
}

/// Runs the prevalidation on the given code. Returns a [`RuntimeBlob`] if it succeeds.
Expand All @@ -191,8 +189,7 @@ pub fn prepare(
blob: RuntimeBlob,
executor_params: &ExecutorParams,
) -> Result<Vec<u8>, sc_executor_common::error::WasmError> {
let semantics = params_to_wasmtime_semantics(executor_params)
.map_err(|e| sc_executor_common::error::WasmError::Other(e))?;
let semantics = params_to_wasmtime_semantics(executor_params);
sc_executor_wasmtime::prepare_runtime_artifact(blob, &semantics)
}

Expand Down
2 changes: 1 addition & 1 deletion polkadot/node/core/pvf/common/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ pub struct SecurityStatus {
pub can_enable_landlock: bool,
/// Whether the seccomp features we use are fully available on this system.
pub can_enable_seccomp: bool,
// Whether we are able to unshare the user namespace and change the filesystem root.
/// Whether we are able to unshare the user namespace and change the filesystem root.
pub can_unshare_user_namespace_and_change_root: bool,
}

Expand Down
2 changes: 1 addition & 1 deletion polkadot/node/core/pvf/common/src/worker/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ pub fn run_worker<F>(
#[cfg_attr(not(target_os = "linux"), allow(unused_mut))] mut worker_dir_path: PathBuf,
node_version: Option<&str>,
worker_version: Option<&str>,
#[cfg_attr(not(target_os = "linux"), allow(unused_variables))] security_status: &SecurityStatus,
security_status: &SecurityStatus,
mut event_loop: F,
) where
F: FnMut(UnixStream, PathBuf) -> io::Result<Never>,
Expand Down
Loading

0 comments on commit 36547bd

Please sign in to comment.