From f95180714fa0c3413214c16700fc2d1d4eaad892 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 4 Oct 2021 11:54:09 +0200 Subject: [PATCH 1/5] Bump syn from 1.0.77 to 1.0.78 (#4006) Bumps [syn](https://github.com/dtolnay/syn) from 1.0.77 to 1.0.78. - [Release notes](https://github.com/dtolnay/syn/releases) - [Commits](https://github.com/dtolnay/syn/compare/1.0.77...1.0.78) --- updated-dependencies: - dependency-name: syn dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- Cargo.lock | 4 ++-- node/overseer/overseer-gen/proc-macro/Cargo.toml | 2 +- xcm/procedural/Cargo.toml | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 59d46b7b8c3a..bf94de083777 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -10408,9 +10408,9 @@ checksum = "6bdef32e8150c2a081110b42772ffe7d7c9032b606bc226c8260fd97e0976601" [[package]] name = "syn" -version = "1.0.77" +version = "1.0.78" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5239bc68e0fef57495900cfea4e8dc75596d9a319d7e16b1e0a440d24e6fe0a0" +checksum = "a4eac2e6c19f5c3abc0c229bea31ff0b9b091c7b14990e8924b92902a303a0c0" dependencies = [ "proc-macro2", "quote", diff --git a/node/overseer/overseer-gen/proc-macro/Cargo.toml b/node/overseer/overseer-gen/proc-macro/Cargo.toml index aa972b0e6a56..9495fb5a8678 100644 --- a/node/overseer/overseer-gen/proc-macro/Cargo.toml +++ b/node/overseer/overseer-gen/proc-macro/Cargo.toml @@ -12,7 +12,7 @@ targets = ["x86_64-unknown-linux-gnu"] proc-macro = true [dependencies] -syn = { version = "1.0.77", features = ["full", "extra-traits"] } +syn = { version = "1.0.78", features = ["full", "extra-traits"] } quote = "1.0.9" proc-macro2 = "1.0.26" proc-macro-crate = "1.1.0" diff --git a/xcm/procedural/Cargo.toml b/xcm/procedural/Cargo.toml index fde65dcdd51a..03b5275c04f6 100644 --- a/xcm/procedural/Cargo.toml +++ b/xcm/procedural/Cargo.toml @@ -10,4 +10,4 @@ proc-macro = true [dependencies] proc-macro2 = "1.0.28" quote = "1.0.9" -syn = "1.0.77" +syn = "1.0.78" From 6cb31e50f54c6d30290f97d4d22f590645184ae0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Mon, 4 Oct 2021 13:14:03 +0200 Subject: [PATCH 2/5] Fix unoccupied bitfields (#4004) * Fix unoccupied bitfields If there is an unoccupied bitfield set, we should just ignore it and not keep it for the rest of the logic in `process_bitfields`. * Bring back test, but this time corrected * Remove incorrect code --- runtime/parachains/src/inclusion.rs | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/runtime/parachains/src/inclusion.rs b/runtime/parachains/src/inclusion.rs index 1c37bce3bd27..7150dbbfb798 100644 --- a/runtime/parachains/src/inclusion.rs +++ b/runtime/parachains/src/inclusion.rs @@ -180,8 +180,6 @@ pub mod pallet { NotCollatorSigned, /// The validation data hash does not match expected. ValidationDataHashMismatch, - /// Internal error only returned when compiled with debug assertions. - InternalError, /// The downward message queue is not processed correctly. IncorrectDownwardMessageHandling, /// At least one upward message sent does not pass the acceptance criteria. @@ -328,8 +326,6 @@ impl Pallet { candidate_pending_availability.availability_votes.get_mut(val_idx) }) { *bit = true; - } else if cfg!(debug_assertions) { - ensure!(false, Error::::InternalError); } } @@ -1412,13 +1408,14 @@ mod tests { bare_bitfield, &signing_context, )); - - assert!(ParaInclusion::process_bitfields( - expected_bits(), - vec![signed.into()], - &core_lookup, - ) - .is_err()); + assert_eq!( + ParaInclusion::process_bitfields( + expected_bits(), + vec![signed.into()], + &core_lookup, + ), + Ok(vec![]) + ); } // empty bitfield signed: always OK, but kind of useless. From 3477414e077cb21167cb23b75f11d5a8f2af922e Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Mon, 4 Oct 2021 14:25:17 +0200 Subject: [PATCH 3/5] Use background tasks properly in candidate-validation (#4002) * refactor: candidate-validation background tasks * fix tests * fmt --- node/core/candidate-validation/src/lib.rs | 237 +++++++++++--------- node/core/candidate-validation/src/tests.rs | 86 ++++--- 2 files changed, 176 insertions(+), 147 deletions(-) diff --git a/node/core/candidate-validation/src/lib.rs b/node/core/candidate-validation/src/lib.rs index 2862d4c3f30e..9d2852efcc59 100644 --- a/node/core/candidate-validation/src/lib.rs +++ b/node/core/candidate-validation/src/lib.rs @@ -35,7 +35,7 @@ use polkadot_node_subsystem::{ CandidateValidationMessage, RuntimeApiMessage, RuntimeApiRequest, ValidationFailed, }, overseer, FromOverseer, OverseerSignal, SpawnedSubsystem, SubsystemContext, SubsystemError, - SubsystemResult, + SubsystemResult, SubsystemSender, }; use polkadot_node_subsystem_util::metrics::{self, prometheus}; use polkadot_parachain::primitives::{ValidationParams, ValidationResult as WasmValidationResult}; @@ -120,7 +120,7 @@ where Context: SubsystemContext, Context: overseer::SubsystemContext, { - let (mut validation_host, task) = polkadot_node_core_pvf::start( + let (validation_host, task) = polkadot_node_core_pvf::start( polkadot_node_core_pvf::Config::new(cache_path, program_path), pvf_metrics, ); @@ -137,24 +137,28 @@ where pov, response_sender, ) => { - let _timer = metrics.time_validate_from_chain_state(); - - let res = spawn_validate_from_chain_state( - &mut ctx, - &mut validation_host, - descriptor, - pov, - &metrics, - ) - .await; - - match res { - Ok(x) => { - metrics.on_validation_event(&x); - let _ = response_sender.send(x); - }, - Err(e) => return Err(e), - } + let bg = { + let mut sender = ctx.sender().clone(); + let metrics = metrics.clone(); + let validation_host = validation_host.clone(); + + async move { + let _timer = metrics.time_validate_from_chain_state(); + let res = validate_from_chain_state( + &mut sender, + validation_host, + descriptor, + pov, + &metrics, + ) + .await; + + metrics.on_validation_event(&res); + let _ = response_sender.send(res); + } + }; + + ctx.spawn("validate-from-chain-state", bg.boxed())?; }, CandidateValidationMessage::ValidateFromExhaustive( persisted_validation_data, @@ -163,50 +167,68 @@ where pov, response_sender, ) => { - let _timer = metrics.time_validate_from_exhaustive(); - - let res = validate_candidate_exhaustive( - &mut validation_host, - persisted_validation_data, - validation_code, - descriptor, - pov, - &metrics, - ) - .await; - - match res { - Ok(x) => { - metrics.on_validation_event(&x); - - if let Err(_e) = response_sender.send(x) { - tracing::warn!( - target: LOG_TARGET, - "Requester of candidate validation dropped", - ) - } - }, - Err(e) => return Err(e), - } + let bg = { + let metrics = metrics.clone(); + let validation_host = validation_host.clone(); + + async move { + let _timer = metrics.time_validate_from_exhaustive(); + let res = validate_candidate_exhaustive( + validation_host, + persisted_validation_data, + validation_code, + descriptor, + pov, + &metrics, + ) + .await; + + metrics.on_validation_event(&res); + let _ = response_sender.send(res); + } + }; + + ctx.spawn("validate-from-exhaustive", bg.boxed())?; }, }, } } } -async fn runtime_api_request( - ctx: &mut Context, +struct RuntimeRequestFailed; + +async fn runtime_api_request( + sender: &mut Sender, relay_parent: Hash, request: RuntimeApiRequest, receiver: oneshot::Receiver>, -) -> SubsystemResult> +) -> Result where - Context: SubsystemContext, - Context: overseer::SubsystemContext, + Sender: SubsystemSender, { - ctx.send_message(RuntimeApiMessage::Request(relay_parent, request)).await; + sender + .send_message(RuntimeApiMessage::Request(relay_parent, request).into()) + .await; - receiver.await.map_err(Into::into) + receiver + .await + .map_err(|_| { + tracing::debug!(target: LOG_TARGET, ?relay_parent, "Runtime API request dropped"); + + RuntimeRequestFailed + }) + .and_then(|res| { + res.map_err(|e| { + tracing::debug!( + target: LOG_TARGET, + ?relay_parent, + err = ?e, + "Runtime API request internal error" + ); + + RuntimeRequestFailed + }) + }) } #[derive(Debug)] @@ -216,61 +238,57 @@ enum AssumptionCheckOutcome { BadRequest, } -async fn check_assumption_validation_data( - ctx: &mut Context, +async fn check_assumption_validation_data( + sender: &mut Sender, descriptor: &CandidateDescriptor, assumption: OccupiedCoreAssumption, -) -> SubsystemResult +) -> AssumptionCheckOutcome where - Context: SubsystemContext, - Context: overseer::SubsystemContext, + Sender: SubsystemSender, { let validation_data = { let (tx, rx) = oneshot::channel(); let d = runtime_api_request( - ctx, + sender, descriptor.relay_parent, RuntimeApiRequest::PersistedValidationData(descriptor.para_id, assumption, tx), rx, ) - .await?; + .await; match d { - Ok(None) | Err(_) => return Ok(AssumptionCheckOutcome::BadRequest), + Ok(None) | Err(RuntimeRequestFailed) => return AssumptionCheckOutcome::BadRequest, Ok(Some(d)) => d, } }; let persisted_validation_data_hash = validation_data.hash(); - SubsystemResult::Ok( - if descriptor.persisted_validation_data_hash == persisted_validation_data_hash { - let (code_tx, code_rx) = oneshot::channel(); - let validation_code = runtime_api_request( - ctx, - descriptor.relay_parent, - RuntimeApiRequest::ValidationCode(descriptor.para_id, assumption, code_tx), - code_rx, - ) - .await?; + if descriptor.persisted_validation_data_hash == persisted_validation_data_hash { + let (code_tx, code_rx) = oneshot::channel(); + let validation_code = runtime_api_request( + sender, + descriptor.relay_parent, + RuntimeApiRequest::ValidationCode(descriptor.para_id, assumption, code_tx), + code_rx, + ) + .await; - match validation_code { - Ok(None) | Err(_) => AssumptionCheckOutcome::BadRequest, - Ok(Some(v)) => AssumptionCheckOutcome::Matches(validation_data, v), - } - } else { - AssumptionCheckOutcome::DoesNotMatch - }, - ) + match validation_code { + Ok(None) | Err(RuntimeRequestFailed) => AssumptionCheckOutcome::BadRequest, + Ok(Some(v)) => AssumptionCheckOutcome::Matches(validation_data, v), + } + } else { + AssumptionCheckOutcome::DoesNotMatch + } } -async fn find_assumed_validation_data( - ctx: &mut Context, +async fn find_assumed_validation_data( + sender: &mut Sender, descriptor: &CandidateDescriptor, -) -> SubsystemResult +) -> AssumptionCheckOutcome where - Context: SubsystemContext, - Context: overseer::SubsystemContext, + Sender: SubsystemSender, { // The candidate descriptor has a `persisted_validation_data_hash` which corresponds to // one of up to two possible values that we can derive from the state of the @@ -287,41 +305,40 @@ where // Consider running these checks in parallel to reduce validation latency. for assumption in ASSUMPTIONS { - let outcome = check_assumption_validation_data(ctx, descriptor, *assumption).await?; + let outcome = check_assumption_validation_data(sender, descriptor, *assumption).await; match outcome { - AssumptionCheckOutcome::Matches(_, _) => return Ok(outcome), - AssumptionCheckOutcome::BadRequest => return Ok(outcome), + AssumptionCheckOutcome::Matches(_, _) => return outcome, + AssumptionCheckOutcome::BadRequest => return outcome, AssumptionCheckOutcome::DoesNotMatch => continue, } } - Ok(AssumptionCheckOutcome::DoesNotMatch) + AssumptionCheckOutcome::DoesNotMatch } -async fn spawn_validate_from_chain_state( - ctx: &mut Context, - validation_host: &mut ValidationHost, +async fn validate_from_chain_state( + sender: &mut Sender, + validation_host: ValidationHost, descriptor: CandidateDescriptor, pov: Arc, metrics: &Metrics, -) -> SubsystemResult> +) -> Result where - Context: SubsystemContext, - Context: overseer::SubsystemContext, + Sender: SubsystemSender, { let (validation_data, validation_code) = - match find_assumed_validation_data(ctx, &descriptor).await? { + match find_assumed_validation_data(sender, &descriptor).await { AssumptionCheckOutcome::Matches(validation_data, validation_code) => (validation_data, validation_code), AssumptionCheckOutcome::DoesNotMatch => { // If neither the assumption of the occupied core having the para included or the assumption // of the occupied core timing out are valid, then the persisted_validation_data_hash in the descriptor // is not based on the relay parent and is thus invalid. - return Ok(Ok(ValidationResult::Invalid(InvalidCandidate::BadParent))) + return Ok(ValidationResult::Invalid(InvalidCandidate::BadParent)) }, AssumptionCheckOutcome::BadRequest => - return Ok(Err(ValidationFailed("Assumption Check: Bad request".into()))), + return Err(ValidationFailed("Assumption Check: Bad request".into())), }; let validation_result = validate_candidate_exhaustive( @@ -334,20 +351,20 @@ where ) .await; - if let Ok(Ok(ValidationResult::Valid(ref outputs, _))) = validation_result { + if let Ok(ValidationResult::Valid(ref outputs, _)) = validation_result { let (tx, rx) = oneshot::channel(); match runtime_api_request( - ctx, + sender, descriptor.relay_parent, RuntimeApiRequest::CheckValidationOutputs(descriptor.para_id, outputs.clone(), tx), rx, ) - .await? + .await { Ok(true) => {}, - Ok(false) => return Ok(Ok(ValidationResult::Invalid(InvalidCandidate::InvalidOutputs))), - Err(_) => - return Ok(Err(ValidationFailed("Check Validation Outputs: Bad request".into()))), + Ok(false) => return Ok(ValidationResult::Invalid(InvalidCandidate::InvalidOutputs)), + Err(RuntimeRequestFailed) => + return Err(ValidationFailed("Check Validation Outputs: Bad request".into())), } } @@ -361,7 +378,7 @@ async fn validate_candidate_exhaustive( descriptor: CandidateDescriptor, pov: Arc, metrics: &Metrics, -) -> SubsystemResult> { +) -> Result { let _timer = metrics.time_validate_candidate_exhaustive(); let validation_code_hash = validation_code.hash(); @@ -378,7 +395,7 @@ async fn validate_candidate_exhaustive( &*pov, &validation_code_hash, ) { - return Ok(Ok(ValidationResult::Invalid(e))) + return Ok(ValidationResult::Invalid(e)) } let raw_validation_code = match sp_maybe_compressed_blob::decompress( @@ -390,7 +407,7 @@ async fn validate_candidate_exhaustive( 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))) + return Ok(ValidationResult::Invalid(InvalidCandidate::CodeDecompressionFailure)) }, }; @@ -401,7 +418,7 @@ async fn validate_candidate_exhaustive( tracing::debug!(target: LOG_TARGET, err=?e, "Invalid PoV code"); // If the PoV is invalid, the candidate certainly is. - return Ok(Ok(ValidationResult::Invalid(InvalidCandidate::PoVDecompressionFailure))) + return Ok(ValidationResult::Invalid(InvalidCandidate::PoVDecompressionFailure)) }, }; @@ -424,7 +441,7 @@ async fn validate_candidate_exhaustive( ); } - let result = match result { + match result { Err(ValidationError::InternalError(e)) => Err(ValidationFailed(e)), Err(ValidationError::InvalidCandidate(WasmInvalidCandidate::HardTimeout)) => @@ -450,9 +467,7 @@ async fn validate_candidate_exhaustive( }; Ok(ValidationResult::Valid(outputs, persisted_validation_data)) }, - }; - - Ok(result) + } } #[async_trait] @@ -465,7 +480,7 @@ trait ValidationBackend { } #[async_trait] -impl ValidationBackend for &'_ mut ValidationHost { +impl ValidationBackend for ValidationHost { async fn validate_candidate( &mut self, raw_validation_code: Vec, diff --git a/node/core/candidate-validation/src/tests.rs b/node/core/candidate-validation/src/tests.rs index 15314c35ed2f..f067ead6cd3c 100644 --- a/node/core/candidate-validation/src/tests.rs +++ b/node/core/candidate-validation/src/tests.rs @@ -19,6 +19,7 @@ use assert_matches::assert_matches; use futures::executor; use polkadot_node_subsystem::messages::AllMessages; use polkadot_node_subsystem_test_helpers as test_helpers; +use polkadot_node_subsystem_util::reexports::SubsystemContext; use polkadot_primitives::v1::{HeadData, UpwardMessage}; use sp_core::testing::TaskExecutor; use sp_keyring::Sr25519Keyring; @@ -52,11 +53,15 @@ fn correctly_checks_included_assumption() { candidate.para_id = para_id; let pool = TaskExecutor::new(); - let (mut ctx, mut ctx_handle) = test_helpers::make_subsystem_context(pool.clone()); + let (mut ctx, mut ctx_handle) = + test_helpers::make_subsystem_context::(pool.clone()); - let (check_fut, check_result) = - check_assumption_validation_data(&mut ctx, &candidate, OccupiedCoreAssumption::Included) - .remote_handle(); + let (check_fut, check_result) = check_assumption_validation_data( + ctx.sender(), + &candidate, + OccupiedCoreAssumption::Included, + ) + .remote_handle(); let test_fut = async move { assert_matches!( @@ -89,7 +94,7 @@ fn correctly_checks_included_assumption() { } ); - assert_matches!(check_result.await.unwrap(), AssumptionCheckOutcome::Matches(o, v) => { + assert_matches!(check_result.await, AssumptionCheckOutcome::Matches(o, v) => { assert_eq!(o, validation_data); assert_eq!(v, validation_code); }); @@ -114,11 +119,15 @@ fn correctly_checks_timed_out_assumption() { candidate.para_id = para_id; let pool = TaskExecutor::new(); - let (mut ctx, mut ctx_handle) = test_helpers::make_subsystem_context(pool.clone()); + let (mut ctx, mut ctx_handle) = + test_helpers::make_subsystem_context::(pool.clone()); - let (check_fut, check_result) = - check_assumption_validation_data(&mut ctx, &candidate, OccupiedCoreAssumption::TimedOut) - .remote_handle(); + let (check_fut, check_result) = check_assumption_validation_data( + ctx.sender(), + &candidate, + OccupiedCoreAssumption::TimedOut, + ) + .remote_handle(); let test_fut = async move { assert_matches!( @@ -151,7 +160,7 @@ fn correctly_checks_timed_out_assumption() { } ); - assert_matches!(check_result.await.unwrap(), AssumptionCheckOutcome::Matches(o, v) => { + assert_matches!(check_result.await, AssumptionCheckOutcome::Matches(o, v) => { assert_eq!(o, validation_data); assert_eq!(v, validation_code); }); @@ -174,11 +183,15 @@ fn check_is_bad_request_if_no_validation_data() { candidate.para_id = para_id; let pool = TaskExecutor::new(); - let (mut ctx, mut ctx_handle) = test_helpers::make_subsystem_context(pool.clone()); + let (mut ctx, mut ctx_handle) = + test_helpers::make_subsystem_context::(pool.clone()); - let (check_fut, check_result) = - check_assumption_validation_data(&mut ctx, &candidate, OccupiedCoreAssumption::Included) - .remote_handle(); + let (check_fut, check_result) = check_assumption_validation_data( + ctx.sender(), + &candidate, + OccupiedCoreAssumption::Included, + ) + .remote_handle(); let test_fut = async move { assert_matches!( @@ -198,7 +211,7 @@ fn check_is_bad_request_if_no_validation_data() { } ); - assert_matches!(check_result.await.unwrap(), AssumptionCheckOutcome::BadRequest); + assert_matches!(check_result.await, AssumptionCheckOutcome::BadRequest); }; let test_fut = future::join(test_fut, check_fut); @@ -218,11 +231,15 @@ fn check_is_bad_request_if_no_validation_code() { candidate.para_id = para_id; let pool = TaskExecutor::new(); - let (mut ctx, mut ctx_handle) = test_helpers::make_subsystem_context(pool.clone()); + let (mut ctx, mut ctx_handle) = + test_helpers::make_subsystem_context::(pool.clone()); - let (check_fut, check_result) = - check_assumption_validation_data(&mut ctx, &candidate, OccupiedCoreAssumption::TimedOut) - .remote_handle(); + let (check_fut, check_result) = check_assumption_validation_data( + ctx.sender(), + &candidate, + OccupiedCoreAssumption::TimedOut, + ) + .remote_handle(); let test_fut = async move { assert_matches!( @@ -255,7 +272,7 @@ fn check_is_bad_request_if_no_validation_code() { } ); - assert_matches!(check_result.await.unwrap(), AssumptionCheckOutcome::BadRequest); + assert_matches!(check_result.await, AssumptionCheckOutcome::BadRequest); }; let test_fut = future::join(test_fut, check_fut); @@ -274,11 +291,15 @@ fn check_does_not_match() { candidate.para_id = para_id; let pool = TaskExecutor::new(); - let (mut ctx, mut ctx_handle) = test_helpers::make_subsystem_context(pool.clone()); + let (mut ctx, mut ctx_handle) = + test_helpers::make_subsystem_context::(pool.clone()); - let (check_fut, check_result) = - check_assumption_validation_data(&mut ctx, &candidate, OccupiedCoreAssumption::Included) - .remote_handle(); + let (check_fut, check_result) = check_assumption_validation_data( + ctx.sender(), + &candidate, + OccupiedCoreAssumption::Included, + ) + .remote_handle(); let test_fut = async move { assert_matches!( @@ -298,7 +319,7 @@ fn check_does_not_match() { } ); - assert_matches!(check_result.await.unwrap(), AssumptionCheckOutcome::DoesNotMatch); + assert_matches!(check_result.await, AssumptionCheckOutcome::DoesNotMatch); }; let test_fut = future::join(test_fut, check_fut); @@ -365,7 +386,6 @@ fn candidate_validation_ok_is_ok() { Arc::new(pov), &Default::default(), )) - .unwrap() .unwrap(); assert_matches!(v, ValidationResult::Valid(outputs, used_validation_data) => { @@ -408,7 +428,6 @@ fn candidate_validation_bad_return_is_invalid() { Arc::new(pov), &Default::default(), )) - .unwrap() .unwrap(); assert_matches!(v, ValidationResult::Invalid(InvalidCandidate::ExecutionError(_))); @@ -443,8 +462,7 @@ fn candidate_validation_timeout_is_internal_error() { descriptor, Arc::new(pov), &Default::default(), - )) - .unwrap(); + )); assert_matches!(v, Ok(ValidationResult::Invalid(InvalidCandidate::Timeout))); } @@ -479,7 +497,6 @@ fn candidate_validation_code_mismatch_is_invalid() { Arc::new(pov), &Default::default(), )) - .unwrap() .unwrap(); assert_matches!(v, ValidationResult::Invalid(InvalidCandidate::CodeHashMismatch)); @@ -518,8 +535,7 @@ fn compressed_code_works() { descriptor, Arc::new(pov), &Default::default(), - )) - .unwrap(); + )); assert_matches!(v, Ok(ValidationResult::Valid(_, _))); } @@ -558,8 +574,7 @@ fn code_decompression_failure_is_invalid() { descriptor, Arc::new(pov), &Default::default(), - )) - .unwrap(); + )); assert_matches!(v, Ok(ValidationResult::Invalid(InvalidCandidate::CodeDecompressionFailure))); } @@ -599,8 +614,7 @@ fn pov_decompression_failure_is_invalid() { descriptor, Arc::new(pov), &Default::default(), - )) - .unwrap(); + )); assert_matches!(v, Ok(ValidationResult::Invalid(InvalidCandidate::PoVDecompressionFailure))); } From d9dc3d85c434bbc154796f411b17ec49255da1a3 Mon Sep 17 00:00:00 2001 From: Bernhard Schuster Date: Mon, 4 Oct 2021 14:36:54 +0200 Subject: [PATCH 4/5] chore: ci list files that spellcheck finds (#3992) * chore: ci list files that spellcheck finds Avoid -r flag for false positives * avoid master * improve --- .gitlab-ci.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 01aed3c06ce3..6a7d381c0722 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -242,8 +242,9 @@ spellcheck: - cargo spellcheck --version # compare with the commit parent to the PR, given it's from a default branch - git fetch origin +${CI_DEFAULT_BRANCH}:${CI_DEFAULT_BRANCH} + - cargo spellcheck list-files -vvv $(git diff --diff-filter=AM --name-only $(git merge-base ${CI_COMMIT_SHA} ${CI_DEFAULT_BRANCH})) - time cargo spellcheck check -vvv --cfg=scripts/gitlab/spellcheck.toml --checkers hunspell --code 1 - -r $(git diff --name-only ${CI_COMMIT_SHA} $(git merge-base ${CI_COMMIT_SHA} ${CI_DEFAULT_BRANCH})) + $(git diff --diff-filter=AM --name-only $(git merge-base ${CI_COMMIT_SHA} ${CI_DEFAULT_BRANCH})) allow_failure: true build-adder-collator: From 16d66072576d603e9c1fce934853fa093669ef60 Mon Sep 17 00:00:00 2001 From: Chevdor Date: Mon, 4 Oct 2021 16:06:26 +0200 Subject: [PATCH 5/5] Add extrinsic ordering filtering (#3631) --- .../extrinsic-ordering-check-from-bin.yml | 21 +++++-- scripts/github/extrinsic-ordering-filter.sh | 55 +++++++++++++++++++ 2 files changed, 71 insertions(+), 5 deletions(-) create mode 100755 scripts/github/extrinsic-ordering-filter.sh diff --git a/.github/workflows/extrinsic-ordering-check-from-bin.yml b/.github/workflows/extrinsic-ordering-check-from-bin.yml index 0d2ffc6f2b18..199b3be6fe66 100644 --- a/.github/workflows/extrinsic-ordering-check-from-bin.yml +++ b/.github/workflows/extrinsic-ordering-check-from-bin.yml @@ -6,7 +6,7 @@ on: inputs: reference_url: description: The WebSocket url of the reference node - default: wss://rpc.polkadot.io + default: wss://kusama-rpc.polkadot.io required: true binary_url: description: A url to a Linux binary for the node containing the runtime to test @@ -14,7 +14,7 @@ on: required: true chain: description: The name of the chain under test. Usually, you would pass a local chain - default: polkadot-local + default: kusama-local required: true jobs: @@ -27,6 +27,8 @@ jobs: REF_URL: ${{github.event.inputs.reference_url}} steps: + - uses: actions/checkout@v2 + - name: Fetch binary run: | echo Fetching $BIN_URL @@ -46,17 +48,26 @@ jobs: echo "Date: $(date)" >> output.txt echo "Reference: $REF_URL" >> output.txt echo "Target version: $VERSION" >> output.txt - echo "-------------------------------------------" >> output.txt + echo "Chain: $CHAIN" >> output.txt + echo "----------------------------------------------------------------------" >> output.txt + + - name: Pull polkadot-js-tools image + run: docker pull jacogr/polkadot-js-tools - name: Compare the metadata run: | - CMD="docker run --network host jacogr/polkadot-js-tools metadata $REF_URL ws://localhost:9944" + CMD="docker run --pull always --network host jacogr/polkadot-js-tools metadata $REF_URL ws://localhost:9944" echo -e "Running:\n$CMD" $CMD >> output.txt sed -z -i 's/\n\n/\n/g' output.txt + cat output.txt | egrep -n -i '' + SUMMARY=$(./scripts/github/extrinsic-ordering-filter.sh output.txt) + echo -e $SUMMARY + echo -e $SUMMARY >> output.txt - name: Show result - run: cat output.txt + run: | + cat output.txt - name: Stop our local node run: pkill polkadot diff --git a/scripts/github/extrinsic-ordering-filter.sh b/scripts/github/extrinsic-ordering-filter.sh new file mode 100755 index 000000000000..4fd3337f64a6 --- /dev/null +++ b/scripts/github/extrinsic-ordering-filter.sh @@ -0,0 +1,55 @@ +#!/usr/bin/env bash +# This script is used in a Github Workflow. It helps filtering out what is interesting +# when comparing metadata and spot what would require a tx version bump. + +# shellcheck disable=SC2002,SC2086 + +FILE=$1 + +# Higlight indexes that were deleted +function find_deletions() { + echo "\n## Deletions\n" + RES=$(cat "$FILE" | grep -n '\[\-\]' | tr -s " ") + if [ "$RES" ]; then + echo "$RES" | awk '{ printf "%s\\n", $0 }' + else + echo "n/a" + fi +} + +# Highlight indexes that have been deleted +function find_index_changes() { + echo "\n## Index changes\n" + RES=$(cat "$FILE" | grep -E -n -i 'idx:\s*([0-9]+)\s*(->)\s*([0-9]+)' | tr -s " ") + if [ "$RES" ]; then + echo "$RES" | awk '{ printf "%s\\n", $0 }' + else + echo "n/a" + fi +} + +# Highlight values that decreased +function find_decreases() { + echo "\n## Decreases\n" + OUT=$(cat "$FILE" | grep -E -i -o '([0-9]+)\s*(->)\s*([0-9]+)' | awk '$1 > $3 { printf "%s;", $0 }') + IFS=$';' LIST=("$OUT") + unset RES + for line in "${LIST[@]}"; do + RES="$RES\n$(cat "$FILE" | grep -E -i -n \"$line\" | tr -s " ")" + done + + if [ "$RES" ]; then + echo "$RES" | awk '{ printf "%s\\n", $0 }' | sort -u -g | uniq + else + echo "n/a" + fi +} + +echo "\n------------------------------ SUMMARY -------------------------------" +echo "\n⚠️ This filter is here to help spotting changes that should be reviewed carefully." +echo "\n⚠️ It catches only index changes, deletions and value decreases". + +find_deletions "$FILE" +find_index_changes "$FILE" +find_decreases "$FILE" +echo "\n----------------------------------------------------------------------\n"