From 1617852a2f1caec796f2b218aecae2facaeacad6 Mon Sep 17 00:00:00 2001 From: Andrei Eres Date: Tue, 1 Oct 2024 14:39:01 +0200 Subject: [PATCH] Remove ValidateFromChainState (#5707) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Description This PR removes the `CandidateValidationMessage::ValidateFromChainState`, which was previously used by backing, but is no longer relevant since initial async backing implementation https://github.com/paritytech/polkadot/pull/5557. Fixes https://github.com/paritytech/polkadot-sdk/issues/5643 ## Integration This change should not affect downstream projects since `ValidateFromChainState` was already unused. ## Review Notes - Removed all occurrences of `ValidateFromChainState`. - Moved utility functions, previously used in candidate validation tests and malus, exclusively to candidate validation tests as they are no longer used in malus. - Deleted the `polkadot_parachain_candidate_validation_validate_from_chain_state` metric from Prometheus. - Removed `Spawner` from `ReplaceValidationResult` in malus’ interceptors. - `fake_validation_error` was only used for `ValidateFromChainState` handling, while other cases directly used `InvalidCandidate::InvalidOutputs`. It has been replaced with `fake_validation_error`, with a fallback to `InvalidCandidate::InvalidOutputs`. - Updated overseer’s minimal example to replace `ValidateFromChainState` with `ValidateFromExhaustive`. --- .../node/core/candidate-validation/src/lib.rs | 195 +----------------- .../core/candidate-validation/src/metrics.rs | 15 -- .../core/candidate-validation/src/tests.rs | 55 ++++- .../src/variants/back_garbage_candidate.rs | 2 - polkadot/node/malus/src/variants/common.rs | 155 +------------- .../src/variants/dispute_valid_candidates.rs | 2 - .../src/variants/suggest_garbage_candidate.rs | 1 - .../node/overseer/examples/minimal-example.rs | 10 +- polkadot/node/overseer/src/tests.rs | 12 +- polkadot/node/subsystem-types/src/messages.rs | 22 -- .../src/types/overseer-protocol.md | 16 -- prdoc/pr_5707.prdoc | 19 ++ 12 files changed, 97 insertions(+), 407 deletions(-) create mode 100644 prdoc/pr_5707.prdoc diff --git a/polkadot/node/core/candidate-validation/src/lib.rs b/polkadot/node/core/candidate-validation/src/lib.rs index a9732e934414..50505d733916 100644 --- a/polkadot/node/core/candidate-validation/src/lib.rs +++ b/polkadot/node/core/candidate-validation/src/lib.rs @@ -46,8 +46,8 @@ use polkadot_primitives::{ DEFAULT_LENIENT_PREPARATION_TIMEOUT, DEFAULT_PRECHECK_PREPARATION_TIMEOUT, }, AuthorityDiscoveryId, CandidateCommitments, CandidateDescriptor, CandidateEvent, - CandidateReceipt, ExecutorParams, Hash, OccupiedCoreAssumption, PersistedValidationData, - PvfExecKind, PvfPrepKind, SessionIndex, ValidationCode, ValidationCodeHash, ValidatorId, + CandidateReceipt, ExecutorParams, Hash, PersistedValidationData, PvfExecKind, PvfPrepKind, + SessionIndex, ValidationCode, ValidationCodeHash, ValidatorId, }; use sp_application_crypto::{AppCrypto, ByteArray}; use sp_keystore::KeystorePtr; @@ -83,8 +83,7 @@ const PVF_APPROVAL_EXECUTION_RETRY_DELAY: Duration = Duration::from_secs(3); const PVF_APPROVAL_EXECUTION_RETRY_DELAY: Duration = Duration::from_millis(200); // The task queue size is chosen to be somewhat bigger than the PVF host incoming queue size -// to allow exhaustive validation messages to fall through in case the tasks are clogged with -// `ValidateFromChainState` messages awaiting data from the runtime +// to allow exhaustive validation messages to fall through in case the tasks are clogged const TASK_LIMIT: usize = 30; /// Configuration for the candidate validation subsystem @@ -155,30 +154,6 @@ where S: SubsystemSender, { match msg { - CandidateValidationMessage::ValidateFromChainState { - candidate_receipt, - pov, - executor_params, - exec_kind, - response_sender, - .. - } => async move { - let _timer = metrics.time_validate_from_chain_state(); - let res = validate_from_chain_state( - &mut sender, - validation_host, - candidate_receipt, - pov, - executor_params, - exec_kind, - &metrics, - ) - .await; - - metrics.on_validation_event(&res); - let _ = response_sender.send(res); - } - .boxed(), CandidateValidationMessage::ValidateFromExhaustive { validation_data, validation_code, @@ -657,170 +632,6 @@ where } } -#[derive(Debug)] -enum AssumptionCheckOutcome { - Matches(PersistedValidationData, ValidationCode), - DoesNotMatch, - BadRequest, -} - -async fn check_assumption_validation_data( - sender: &mut Sender, - descriptor: &CandidateDescriptor, - assumption: OccupiedCoreAssumption, -) -> AssumptionCheckOutcome -where - Sender: SubsystemSender, -{ - let validation_data = { - let (tx, rx) = oneshot::channel(); - let d = runtime_api_request( - sender, - descriptor.relay_parent, - RuntimeApiRequest::PersistedValidationData(descriptor.para_id, assumption, tx), - rx, - ) - .await; - - match d { - Ok(None) | Err(RuntimeRequestFailed) => return AssumptionCheckOutcome::BadRequest, - Ok(Some(d)) => d, - } - }; - - let persisted_validation_data_hash = validation_data.hash(); - - 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(RuntimeRequestFailed) => AssumptionCheckOutcome::BadRequest, - Ok(Some(v)) => AssumptionCheckOutcome::Matches(validation_data, v), - } - } else { - AssumptionCheckOutcome::DoesNotMatch - } -} - -async fn find_assumed_validation_data( - sender: &mut Sender, - descriptor: &CandidateDescriptor, -) -> AssumptionCheckOutcome -where - 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 - // relay-parent. We can fetch these values by getting the persisted validation data - // based on the different `OccupiedCoreAssumption`s. - - const ASSUMPTIONS: &[OccupiedCoreAssumption] = &[ - OccupiedCoreAssumption::Included, - OccupiedCoreAssumption::TimedOut, - // `TimedOut` and `Free` both don't perform any speculation and therefore should be the - // same for our purposes here. In other words, if `TimedOut` matched then the `Free` must - // be matched as well. - ]; - - // Consider running these checks in parallel to reduce validation latency. - for assumption in ASSUMPTIONS { - let outcome = check_assumption_validation_data(sender, descriptor, *assumption).await; - - match outcome { - AssumptionCheckOutcome::Matches(_, _) => return outcome, - AssumptionCheckOutcome::BadRequest => return outcome, - AssumptionCheckOutcome::DoesNotMatch => continue, - } - } - - AssumptionCheckOutcome::DoesNotMatch -} - -/// Returns validation data for a given candidate. -pub async fn find_validation_data( - sender: &mut Sender, - descriptor: &CandidateDescriptor, -) -> Result, ValidationFailed> -where - Sender: SubsystemSender, -{ - match find_assumed_validation_data(sender, &descriptor).await { - AssumptionCheckOutcome::Matches(validation_data, validation_code) => - Ok(Some((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. - Ok(None) - }, - AssumptionCheckOutcome::BadRequest => - Err(ValidationFailed("Assumption Check: Bad request".into())), - } -} - -async fn validate_from_chain_state( - sender: &mut Sender, - validation_host: ValidationHost, - candidate_receipt: CandidateReceipt, - pov: Arc, - executor_params: ExecutorParams, - exec_kind: PvfExecKind, - metrics: &Metrics, -) -> Result -where - Sender: SubsystemSender, -{ - let mut new_sender = sender.clone(); - let (validation_data, validation_code) = - match find_validation_data(&mut new_sender, &candidate_receipt.descriptor).await? { - Some((validation_data, validation_code)) => (validation_data, validation_code), - None => return Ok(ValidationResult::Invalid(InvalidCandidate::BadParent)), - }; - - let validation_result = validate_candidate_exhaustive( - validation_host, - validation_data, - validation_code, - candidate_receipt.clone(), - pov, - executor_params, - exec_kind, - metrics, - ) - .await; - - if let Ok(ValidationResult::Valid(ref outputs, _)) = validation_result { - let (tx, rx) = oneshot::channel(); - match runtime_api_request( - sender, - candidate_receipt.descriptor.relay_parent, - RuntimeApiRequest::CheckValidationOutputs( - candidate_receipt.descriptor.para_id, - outputs.clone(), - tx, - ), - rx, - ) - .await - { - Ok(true) => {}, - Ok(false) => return Ok(ValidationResult::Invalid(InvalidCandidate::InvalidOutputs)), - Err(RuntimeRequestFailed) => - return Err(ValidationFailed("Check Validation Outputs: Bad request".into())), - } - } - - validation_result -} - async fn validate_candidate_exhaustive( mut validation_backend: impl ValidationBackend + Send, persisted_validation_data: PersistedValidationData, diff --git a/polkadot/node/core/candidate-validation/src/metrics.rs b/polkadot/node/core/candidate-validation/src/metrics.rs index 1459907aa599..76ccd56555f9 100644 --- a/polkadot/node/core/candidate-validation/src/metrics.rs +++ b/polkadot/node/core/candidate-validation/src/metrics.rs @@ -20,7 +20,6 @@ use polkadot_node_metrics::metrics::{self, prometheus}; #[derive(Clone)] pub(crate) struct MetricsInner { pub(crate) validation_requests: prometheus::CounterVec, - pub(crate) validate_from_chain_state: prometheus::Histogram, pub(crate) validate_from_exhaustive: prometheus::Histogram, pub(crate) validate_candidate_exhaustive: prometheus::Histogram, } @@ -46,13 +45,6 @@ impl Metrics { } } - /// Provide a timer for `validate_from_chain_state` which observes on drop. - pub fn time_validate_from_chain_state( - &self, - ) -> Option { - self.0.as_ref().map(|metrics| metrics.validate_from_chain_state.start_timer()) - } - /// Provide a timer for `validate_from_exhaustive` which observes on drop. pub fn time_validate_from_exhaustive( &self, @@ -83,13 +75,6 @@ impl metrics::Metrics for Metrics { )?, registry, )?, - validate_from_chain_state: prometheus::register( - prometheus::Histogram::with_opts(prometheus::HistogramOpts::new( - "polkadot_parachain_candidate_validation_validate_from_chain_state", - "Time spent within `candidate_validation::validate_from_chain_state`", - ))?, - registry, - )?, validate_from_exhaustive: prometheus::register( prometheus::Histogram::with_opts(prometheus::HistogramOpts::new( "polkadot_parachain_candidate_validation_validate_from_exhaustive", diff --git a/polkadot/node/core/candidate-validation/src/tests.rs b/polkadot/node/core/candidate-validation/src/tests.rs index 0dcd84bab6cf..17ca67889f6b 100644 --- a/polkadot/node/core/candidate-validation/src/tests.rs +++ b/polkadot/node/core/candidate-validation/src/tests.rs @@ -25,7 +25,8 @@ use polkadot_node_subsystem::messages::AllMessages; use polkadot_node_subsystem_util::reexports::SubsystemContext; use polkadot_overseer::ActivatedLeaf; use polkadot_primitives::{ - CoreIndex, GroupIndex, HeadData, Id as ParaId, SessionInfo, UpwardMessage, ValidatorId, + CoreIndex, GroupIndex, HeadData, Id as ParaId, OccupiedCoreAssumption, SessionInfo, + UpwardMessage, ValidatorId, }; use polkadot_primitives_test_helpers::{ dummy_collator, dummy_collator_signature, dummy_hash, make_valid_candidate_descriptor, @@ -34,6 +35,58 @@ use sp_core::{sr25519::Public, testing::TaskExecutor}; use sp_keyring::Sr25519Keyring; use sp_keystore::{testing::MemoryKeystore, Keystore}; +#[derive(Debug)] +enum AssumptionCheckOutcome { + Matches(PersistedValidationData, ValidationCode), + DoesNotMatch, + BadRequest, +} + +async fn check_assumption_validation_data( + sender: &mut Sender, + descriptor: &CandidateDescriptor, + assumption: OccupiedCoreAssumption, +) -> AssumptionCheckOutcome +where + Sender: SubsystemSender, +{ + let validation_data = { + let (tx, rx) = oneshot::channel(); + let d = runtime_api_request( + sender, + descriptor.relay_parent, + RuntimeApiRequest::PersistedValidationData(descriptor.para_id, assumption, tx), + rx, + ) + .await; + + match d { + Ok(None) | Err(RuntimeRequestFailed) => return AssumptionCheckOutcome::BadRequest, + Ok(Some(d)) => d, + } + }; + + let persisted_validation_data_hash = validation_data.hash(); + + 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(RuntimeRequestFailed) => AssumptionCheckOutcome::BadRequest, + Ok(Some(v)) => AssumptionCheckOutcome::Matches(validation_data, v), + } + } else { + AssumptionCheckOutcome::DoesNotMatch + } +} + #[test] fn correctly_checks_included_assumption() { let validation_data: PersistedValidationData = Default::default(); diff --git a/polkadot/node/malus/src/variants/back_garbage_candidate.rs b/polkadot/node/malus/src/variants/back_garbage_candidate.rs index b939a2151e23..d6f1353a46a8 100644 --- a/polkadot/node/malus/src/variants/back_garbage_candidate.rs +++ b/polkadot/node/malus/src/variants/back_garbage_candidate.rs @@ -67,12 +67,10 @@ impl OverseerGen for BackGarbageCandidates { RuntimeClient: RuntimeApiSubsystemClient + ChainApiBackend + AuxStore + 'static, Spawner: 'static + SpawnNamed + Clone + Unpin, { - let spawner = args.spawner.clone(); let validation_filter = ReplaceValidationResult::new( FakeCandidateValidation::BackingAndApprovalValid, FakeCandidateValidationError::InvalidOutputs, f64::from(self.percentage), - SpawnGlue(spawner), ); validator_overseer_builder( diff --git a/polkadot/node/malus/src/variants/common.rs b/polkadot/node/malus/src/variants/common.rs index eb6988f81811..50a5af63db44 100644 --- a/polkadot/node/malus/src/variants/common.rs +++ b/polkadot/node/malus/src/variants/common.rs @@ -21,7 +21,6 @@ use crate::{ shared::{MALICIOUS_POV, MALUS}, }; -use polkadot_node_core_candidate_validation::find_validation_data; use polkadot_node_primitives::{InvalidCandidate, ValidationResult}; use polkadot_primitives::{ @@ -149,59 +148,21 @@ impl Into for FakeCandidateValidationError { #[derive(Clone, Debug)] /// An interceptor which fakes validation result with a preconfigured result. /// Replaces `CandidateValidationSubsystem`. -pub struct ReplaceValidationResult { +pub struct ReplaceValidationResult { fake_validation: FakeCandidateValidation, fake_validation_error: FakeCandidateValidationError, distribution: Bernoulli, - spawner: Spawner, } -impl ReplaceValidationResult -where - Spawner: overseer::gen::Spawner, -{ +impl ReplaceValidationResult { pub fn new( fake_validation: FakeCandidateValidation, fake_validation_error: FakeCandidateValidationError, percentage: f64, - spawner: Spawner, ) -> Self { let distribution = Bernoulli::new(percentage / 100.0) .expect("Invalid probability! Percentage must be in range [0..=100]."); - Self { fake_validation, fake_validation_error, distribution, spawner } - } - - /// Creates and sends the validation response for a given candidate. Queries the runtime to - /// obtain the validation data for the given candidate. - pub fn send_validation_response( - &self, - candidate_descriptor: CandidateDescriptor, - subsystem_sender: Sender, - response_sender: oneshot::Sender>, - ) where - Sender: overseer::CandidateValidationSenderTrait + Clone + Send + 'static, - { - let _candidate_descriptor = candidate_descriptor.clone(); - let mut subsystem_sender = subsystem_sender.clone(); - let (sender, receiver) = std::sync::mpsc::channel(); - self.spawner.spawn_blocking( - "malus-get-validation-data", - Some("malus"), - Box::pin(async move { - match find_validation_data(&mut subsystem_sender, &_candidate_descriptor).await { - Ok(Some((validation_data, validation_code))) => { - sender - .send((validation_data, validation_code)) - .expect("channel is still open"); - }, - _ => { - panic!("Unable to fetch validation data"); - }, - } - }), - ); - let (validation_data, _) = receiver.recv().unwrap(); - create_validation_response(validation_data, candidate_descriptor, response_sender); + Self { fake_validation, fake_validation_error, distribution } } } @@ -251,10 +212,9 @@ fn create_validation_response( response_sender.send(result).unwrap(); } -impl MessageInterceptor for ReplaceValidationResult +impl MessageInterceptor for ReplaceValidationResult where Sender: overseer::CandidateValidationSenderTrait + Clone + Send + 'static, - Spawner: overseer::gen::Spawner + Clone + 'static, { type Message = CandidateValidationMessage; @@ -262,7 +222,7 @@ where // configuration fail them. fn intercept_incoming( &self, - subsystem_sender: &mut Sender, + _subsystem_sender: &mut Sender, msg: FromOrchestra, ) -> Option> { match msg { @@ -343,7 +303,7 @@ where match behave_maliciously { true => { let validation_result = - ValidationResult::Invalid(InvalidCandidate::InvalidOutputs); + ValidationResult::Invalid(self.fake_validation_error.into()); gum::info!( target: MALUS, @@ -390,109 +350,6 @@ where }), } }, - // Behaviour related to the backing subsystem - FromOrchestra::Communication { - msg: - CandidateValidationMessage::ValidateFromChainState { - candidate_receipt, - pov, - executor_params, - exec_kind, - response_sender, - .. - }, - } => { - match self.fake_validation { - x if x.misbehaves_valid() && x.should_misbehave(exec_kind) => { - // Behave normally if the `PoV` is not known to be malicious. - if pov.block_data.0.as_slice() != MALICIOUS_POV { - return Some(FromOrchestra::Communication { - msg: CandidateValidationMessage::ValidateFromChainState { - candidate_receipt, - pov, - executor_params, - exec_kind, - response_sender, - }, - }) - } - // If the `PoV` is malicious, back the candidate with some probability `p`, - // where 'p' defaults to 100% for suggest-garbage-candidate variant. - let behave_maliciously = self.distribution.sample(&mut rand::thread_rng()); - match behave_maliciously { - true => { - gum::info!( - target: MALUS, - ?behave_maliciously, - "😈 Backing candidate with malicious PoV.", - ); - - self.send_validation_response( - candidate_receipt.descriptor, - subsystem_sender.clone(), - response_sender, - ); - None - }, - // If the `PoV` is malicious, we behave normally with some probability - // `(1-p)` - false => Some(FromOrchestra::Communication { - msg: CandidateValidationMessage::ValidateFromChainState { - candidate_receipt, - pov, - executor_params, - exec_kind, - response_sender, - }, - }), - } - }, - x if x.misbehaves_invalid() && x.should_misbehave(exec_kind) => { - // Maliciously set the validation result to invalid for a valid candidate - // with probability `p` - let behave_maliciously = self.distribution.sample(&mut rand::thread_rng()); - match behave_maliciously { - true => { - let validation_result = - ValidationResult::Invalid(self.fake_validation_error.into()); - gum::info!( - target: MALUS, - para_id = ?candidate_receipt.descriptor.para_id, - "😈 Maliciously sending invalid validation result: {:?}.", - &validation_result, - ); - // We're not even checking the candidate, this makes us appear - // faster than honest validators. - response_sender.send(Ok(validation_result)).unwrap(); - None - }, - // With some probability `(1-p)` we behave normally - false => { - gum::info!(target: MALUS, "😈 'Decided' to not act maliciously.",); - - Some(FromOrchestra::Communication { - msg: CandidateValidationMessage::ValidateFromChainState { - candidate_receipt, - pov, - executor_params, - exec_kind, - response_sender, - }, - }) - }, - } - }, - _ => Some(FromOrchestra::Communication { - msg: CandidateValidationMessage::ValidateFromChainState { - candidate_receipt, - pov, - executor_params, - exec_kind, - response_sender, - }, - }), - } - }, msg => Some(msg), } } diff --git a/polkadot/node/malus/src/variants/dispute_valid_candidates.rs b/polkadot/node/malus/src/variants/dispute_valid_candidates.rs index a50fdce16e4e..5422167545ce 100644 --- a/polkadot/node/malus/src/variants/dispute_valid_candidates.rs +++ b/polkadot/node/malus/src/variants/dispute_valid_candidates.rs @@ -84,12 +84,10 @@ impl OverseerGen for DisputeValidCandidates { RuntimeClient: RuntimeApiSubsystemClient + ChainApiBackend + AuxStore + 'static, Spawner: 'static + SpawnNamed + Clone + Unpin, { - let spawner = args.spawner.clone(); let validation_filter = ReplaceValidationResult::new( self.fake_validation, self.fake_validation_error, f64::from(self.percentage), - SpawnGlue(spawner.clone()), ); validator_overseer_builder( diff --git a/polkadot/node/malus/src/variants/suggest_garbage_candidate.rs b/polkadot/node/malus/src/variants/suggest_garbage_candidate.rs index 6921352cdfc2..ab2d380fbaf4 100644 --- a/polkadot/node/malus/src/variants/suggest_garbage_candidate.rs +++ b/polkadot/node/malus/src/variants/suggest_garbage_candidate.rs @@ -315,7 +315,6 @@ impl OverseerGen for SuggestGarbageCandidates { FakeCandidateValidation::BackingAndApprovalValid, FakeCandidateValidationError::InvalidOutputs, fake_valid_probability, - SpawnGlue(args.spawner.clone()), ); validator_overseer_builder( diff --git a/polkadot/node/overseer/examples/minimal-example.rs b/polkadot/node/overseer/examples/minimal-example.rs index 86a1801a5f2d..c2cb1817312c 100644 --- a/polkadot/node/overseer/examples/minimal-example.rs +++ b/polkadot/node/overseer/examples/minimal-example.rs @@ -31,8 +31,10 @@ use polkadot_overseer::{ gen::{FromOrchestra, SpawnedSubsystem}, HeadSupportsParachains, SubsystemError, }; -use polkadot_primitives::{CandidateReceipt, Hash, PvfExecKind}; -use polkadot_primitives_test_helpers::{dummy_candidate_descriptor, dummy_hash}; +use polkadot_primitives::{CandidateReceipt, Hash, PersistedValidationData, PvfExecKind}; +use polkadot_primitives_test_helpers::{ + dummy_candidate_descriptor, dummy_hash, dummy_validation_code, +}; struct AlwaysSupportsParachains; @@ -73,7 +75,9 @@ impl Subsystem1 { commitments_hash: Hash::zero(), }; - let msg = CandidateValidationMessage::ValidateFromChainState { + let msg = CandidateValidationMessage::ValidateFromExhaustive { + validation_data: PersistedValidationData { ..Default::default() }, + validation_code: dummy_validation_code(), candidate_receipt, pov: PoV { block_data: BlockData(Vec::new()) }.into(), executor_params: Default::default(), diff --git a/polkadot/node/overseer/src/tests.rs b/polkadot/node/overseer/src/tests.rs index cb0add03e2e7..47c26186b518 100644 --- a/polkadot/node/overseer/src/tests.rs +++ b/polkadot/node/overseer/src/tests.rs @@ -29,10 +29,10 @@ use polkadot_node_subsystem_types::messages::{ }; use polkadot_primitives::{ CandidateHash, CandidateReceipt, CollatorPair, Id as ParaId, InvalidDisputeStatementKind, - PvfExecKind, SessionIndex, ValidDisputeStatementKind, ValidatorIndex, + PersistedValidationData, PvfExecKind, SessionIndex, ValidDisputeStatementKind, ValidatorIndex, }; use polkadot_primitives_test_helpers::{ - dummy_candidate_descriptor, dummy_candidate_receipt, dummy_hash, + dummy_candidate_descriptor, dummy_candidate_receipt, dummy_hash, dummy_validation_code, }; use crate::{ @@ -104,7 +104,9 @@ where }; let (tx, _) = oneshot::channel(); - ctx.send_message(CandidateValidationMessage::ValidateFromChainState { + ctx.send_message(CandidateValidationMessage::ValidateFromExhaustive { + validation_data: PersistedValidationData { ..Default::default() }, + validation_code: dummy_validation_code(), candidate_receipt, pov: PoV { block_data: BlockData(Vec::new()) }.into(), executor_params: Default::default(), @@ -802,7 +804,9 @@ fn test_candidate_validation_msg() -> CandidateValidationMessage { commitments_hash: Hash::zero(), }; - CandidateValidationMessage::ValidateFromChainState { + CandidateValidationMessage::ValidateFromExhaustive { + validation_data: PersistedValidationData { ..Default::default() }, + validation_code: dummy_validation_code(), candidate_receipt, pov, executor_params: Default::default(), diff --git a/polkadot/node/subsystem-types/src/messages.rs b/polkadot/node/subsystem-types/src/messages.rs index fafc700e739f..a520e85f52a9 100644 --- a/polkadot/node/subsystem-types/src/messages.rs +++ b/polkadot/node/subsystem-types/src/messages.rs @@ -142,28 +142,6 @@ pub enum PreCheckOutcome { /// or `Ok(ValidationResult::Invalid)`. #[derive(Debug)] pub enum CandidateValidationMessage { - /// Validate a candidate with provided parameters using relay-chain state. - /// - /// This will implicitly attempt to gather the `PersistedValidationData` and `ValidationCode` - /// from the runtime API of the chain, based on the `relay_parent` - /// of the `CandidateReceipt`. - /// - /// This will also perform checking of validation outputs against the acceptance criteria. - /// - /// If there is no state available which can provide this data or the core for - /// the para is not free at the relay-parent, an error is returned. - ValidateFromChainState { - /// The candidate receipt - candidate_receipt: CandidateReceipt, - /// The proof-of-validity - pov: Arc, - /// Session's executor parameters - executor_params: ExecutorParams, - /// Execution kind, used for timeouts and retries (backing/approvals) - exec_kind: PvfExecKind, - /// The sending side of the response channel - response_sender: oneshot::Sender>, - }, /// Validate a candidate with provided, exhaustive parameters for validation. /// /// Explicitly provide the `PersistedValidationData` and `ValidationCode` so this can do full diff --git a/polkadot/roadmap/implementers-guide/src/types/overseer-protocol.md b/polkadot/roadmap/implementers-guide/src/types/overseer-protocol.md index 317f339ddd4e..6e24d969dde4 100644 --- a/polkadot/roadmap/implementers-guide/src/types/overseer-protocol.md +++ b/polkadot/roadmap/implementers-guide/src/types/overseer-protocol.md @@ -901,22 +901,6 @@ const APPROVAL_EXECUTION_TIMEOUT: Duration = 6 seconds; /// or `Ok(ValidationResult::Invalid)`. #[derive(Debug)] pub enum CandidateValidationMessage { - /// Validate a candidate with provided parameters using relay-chain state. - /// - /// This will implicitly attempt to gather the `PersistedValidationData` and `ValidationCode` - /// from the runtime API of the chain, based on the `relay_parent` - /// of the `CandidateDescriptor`. - /// - /// This will also perform checking of validation outputs against the acceptance criteria. - /// - /// If there is no state available which can provide this data or the core for - /// the para is not free at the relay-parent, an error is returned. - ValidateFromChainState( - CandidateDescriptor, - Arc, - Duration, // Execution timeout. - oneshot::Sender>, - ), /// Validate a candidate with provided, exhaustive parameters for validation. /// /// Explicitly provide the `PersistedValidationData` and `ValidationCode` so this can do full diff --git a/prdoc/pr_5707.prdoc b/prdoc/pr_5707.prdoc new file mode 100644 index 000000000000..11136b3c3626 --- /dev/null +++ b/prdoc/pr_5707.prdoc @@ -0,0 +1,19 @@ +# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0 +# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json + +title: Remove ValidateFromChainState + +doc: + - audience: Node Dev + description: | + Removed the `CandidateValidationMessage::ValidateFromChainState`, which was previously used by backing, but is no longer relevant since initial async backing implementation + +crates: + - name: polkadot-node-subsystem-types + bump: major + - name: polkadot-node-core-candidate-validation + bump: major + - name: polkadot + bump: patch + - name: polkadot-overseer + bump: patch