diff --git a/node/malus/src/variants/back_garbage_candidate.rs b/node/malus/src/variants/back_garbage_candidate.rs index 0b57cd827cef..2ca98576eda0 100644 --- a/node/malus/src/variants/back_garbage_candidate.rs +++ b/node/malus/src/variants/back_garbage_candidate.rs @@ -111,7 +111,7 @@ where pov, _duration, response_sender, - candidate_commitments, + _candidate_commitments, ), } if pov.block_data.0.as_slice() == MALICIOUS_POV => { Self::let_pass( @@ -130,7 +130,7 @@ where pov, _duration, response_sender, - commitments_hash, + _commitments_hash, ), } if pov.block_data.0.as_slice() == MALICIOUS_POV => { if let Some(candidate_receipt) = diff --git a/node/malus/src/variants/common.rs b/node/malus/src/variants/common.rs index b11e68714d78..71bc28633106 100644 --- a/node/malus/src/variants/common.rs +++ b/node/malus/src/variants/common.rs @@ -23,22 +23,18 @@ use crate::{ interceptor::*, shared::{MALICIOUS_POV, MALUS}, }; -use std::collections::HashMap; use polkadot_node_core_candidate_validation::find_validation_data; use polkadot_node_primitives::{InvalidCandidate, PoV, ValidationResult}; use polkadot_node_subsystem::messages::{CandidateValidationMessage, ValidationFailed}; -use polkadot_primitives::v2::{ - BlockNumber, CandidateCommitments, CandidateDescriptor, Hash, PersistedValidationData, - ValidationCode, -}; +use polkadot_primitives::v2::{CandidateCommitments, CandidateDescriptor, PersistedValidationData}; use polkadot_cli::service::SpawnNamed; use clap::ArgEnum; use futures::channel::oneshot; -use std::sync::{Arc, Mutex}; +use std::sync::Arc; #[derive(ArgEnum, Clone, Copy, Debug, PartialEq)] #[clap(rename_all = "kebab-case")] @@ -118,8 +114,6 @@ pub struct ReplaceValidationResult { fake_validation: FakeCandidateValidation, fake_validation_error: FakeCandidateValidationError, spawner: Spawner, - // We use the para head hash as key. - validation_result: Arc>>, } impl ReplaceValidationResult @@ -131,12 +125,7 @@ where fake_validation_error: FakeCandidateValidationError, spawner: Spawner, ) -> Self { - Self { - fake_validation, - fake_validation_error, - spawner, - validation_result: Arc::new(Mutex::new(HashMap::new())), - } + Self { fake_validation, fake_validation_error, spawner } } /// Creates and sends the validation response for a given candidate. Queries the runtime to obtain the validation data for the @@ -163,7 +152,9 @@ where 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)); + sender + .send((validation_data, validation_code)) + .expect("channel is still open"); }, _ => { panic!("Unable to fetch validation data"); @@ -171,14 +162,8 @@ where } }), ); - let (validation_data, validation_code) = receiver.recv().unwrap(); - create_validation_response( - validation_data, - Some(validation_code), - candidate_descriptor, - pov, - response_sender, - ); + let (validation_data, _) = receiver.recv().unwrap(); + create_validation_response(validation_data, candidate_descriptor, pov, response_sender); } } @@ -198,7 +183,6 @@ pub fn create_fake_candidate_commitments( // Create and send validation response. This function needs the persistent validation data. fn create_validation_response( persisted_validation_data: PersistedValidationData, - validation_code: Option, _candidate_descriptor: CandidateDescriptor, _pov: Arc, response_sender: oneshot::Sender>, @@ -229,74 +213,6 @@ where subsystem_sender: &mut Sender, msg: FromOverseer, ) -> Option> { - if self.fake_validation == FakeCandidateValidation::Disabled { - // Proxy messages and cache validation results. - match msg { - FromOverseer::Communication { - msg: - CandidateValidationMessage::ValidateFromChainState( - descriptor, - pov, - timeout, - response_sender, - commitments_hash, - ), - } => { - // This is our request, let it pass to original subsystem. - if timeout == std::time::Duration::from_secs(13) { - return Some(FromOverseer::Communication { - msg: CandidateValidationMessage::ValidateFromChainState( - descriptor, - pov, - timeout, - response_sender, - commitments_hash, - ), - }) - } - - let (tx, rx) = oneshot::channel(); - let mut subsystem_sender = subsystem_sender.clone(); - - gum::info!(target: MALUS, "Proxying validation request",); - - let validation_result = self.validation_result.clone(); - self.spawner.spawn( - "malus-validation-proxy", - Some("malus"), - Box::pin(async move { - subsystem_sender - .send_message(CandidateValidationMessage::ValidateFromChainState( - descriptor.clone(), - pov, - std::time::Duration::from_secs(13), - tx, - commitments_hash, - )) - .await; - let result = rx.await.unwrap(); - gum::info!(target: MALUS, "Proxied validation result {:?}", &result); - match result.unwrap() { - ValidationResult::Valid(commitments, validation_data) => { - validation_result.lock().expect("bad lock").insert( - descriptor.para_head, - (commitments.clone(), validation_data.clone()), - ); - response_sender.send(Ok(ValidationResult::Valid( - commitments, - validation_data, - ))); - }, - _ => panic!("Bad things can happen."), - } - }), - ); - return None - }, - _ => return Some(msg), - } - } - match msg { FromOverseer::Communication { msg: @@ -328,13 +244,7 @@ where match self.fake_validation { FakeCandidateValidation::ApprovalValid | FakeCandidateValidation::BackingAndApprovalValid => { - create_validation_response( - validation_data, - Some(validation_code), - descriptor, - pov, - sender, - ); + create_validation_response(validation_data, descriptor, pov, sender); None }, FakeCandidateValidation::ApprovalInvalid | diff --git a/node/malus/src/variants/suggest_garbage_candidate.rs b/node/malus/src/variants/suggest_garbage_candidate.rs index 0922d22ff7ca..1f893621c1ee 100644 --- a/node/malus/src/variants/suggest_garbage_candidate.rs +++ b/node/malus/src/variants/suggest_garbage_candidate.rs @@ -32,7 +32,7 @@ use polkadot_cli::{ }; use polkadot_node_core_candidate_validation::find_validation_data; use polkadot_node_primitives::{AvailableData, BlockData, PoV}; -use polkadot_primitives::v2::{CandidateCommitments, CandidateDescriptor, CandidateHash}; +use polkadot_primitives::v2::{CandidateDescriptor, CandidateHash}; use polkadot_node_subsystem_util::request_validators; @@ -80,7 +80,7 @@ where { type Message = CandidateBackingMessage; - /// Cache and forward `CandidateBackingMessage::Second`. This is called by collator protocol (validator side). + /// Intercept incoming `Second` requests from the `collator-protocol` subsystem. We take fn intercept_incoming( &self, subsystem_sender: &mut Sender, @@ -117,7 +117,9 @@ where match find_validation_data(&mut new_sender, &_candidate.descriptor()).await { Ok(Some((validation_data, validation_code))) => { - sender.send((validation_data, validation_code, n_validators)); + sender + .send((validation_data, validation_code, n_validators)) + .expect("channel is still open"); }, _ => { panic!("Unable to fetch validation data"); @@ -198,11 +200,14 @@ where "Created malicious candidate" ); + // Map malicious candidate to the original one. We need this mapping to send back the correct seconded statement + // to the collators. self.inner .lock() .expect("bad lock") .map .insert(malicious_candidate_hash, candidate.hash()); + let message = FromOverseer::Communication { msg: CandidateBackingMessage::Second(relay_parent, malicious_candidate, pov), }; @@ -216,18 +221,14 @@ where fn intercept_outgoing(&self, msg: AllMessages) -> Option { let msg = match msg { - // TODO: Send the Seconded statement for the original candidate. AllMessages::CollatorProtocol(CollatorProtocolMessage::Seconded( relay_parent, statement, )) => { - // match self.inner.lock().expect("bad lock").map.entry(statement.payload().candidate_hash()) { - // Entry::Occupied(original_candidate) => { - // // Create new statement - // statement.payload_ut(). - - // } - // } + // `parachain::collator-protocol: received an unexpected `CollationSeconded`: unknown statement statement=...` + // TODO: Fix this error. We get this on colaltors because `malicious backing` creates a candidate that gets backed/included. + // It is harmless for test parachain collators, but it will prevent cumulus based collators to make progress + // as they wait for the relay chain to confirm the seconding of the collation. AllMessages::CollatorProtocol(CollatorProtocolMessage::Seconded( relay_parent, statement,