diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 0e8c64dc074f..b63a301c3660 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -727,7 +727,7 @@ zombienet-tests-parachains-disputes: tags: - zombienet-polkadot-integration-test -zombienet-tests-malus-dispute-valid: +zombienet-tests-bft-disputes-lag: stage: stage3 image: "${ZOMBIENET_IMAGE}" <<: *kubernetes-env @@ -737,21 +737,21 @@ zombienet-tests-malus-dispute-valid: - job: publish-malus-image - job: publish-test-collators-image variables: - GH_DIR: "https://github.com/paritytech/polkadot/tree/${CI_COMMIT_SHORT_SHA}/node/malus/integrationtests" + GH_DIR: "https://github.com/paritytech/polkadot/tree/${CI_COMMIT_SHORT_SHA}/zombienet_tests/functional" before_script: - echo "Zombie-net Tests Config" - - echo "${ZOMBIENET_IMAGE_NAME}" + - echo "${ZOMBIENET_IMAGE}" - echo "${PARACHAINS_IMAGE_NAME} ${PARACHAINS_IMAGE_TAG}" - - echo "${MALUS_IMAGE_NAME} ${MALUS_IMAGE_TAG}" + - echo "COL_IMAGE=${COLLATOR_IMAGE_NAME}:${COLLATOR_IMAGE_TAG}" - echo "${GH_DIR}" - - export DEBUG=zombie* + - export DEBUG=zombie,zombie::network-node - export ZOMBIENET_INTEGRATION_TEST_IMAGE=${PARACHAINS_IMAGE_NAME}:${PARACHAINS_IMAGE_TAG} - export MALUS_IMAGE=${MALUS_IMAGE_NAME}:${MALUS_IMAGE_TAG} - export COL_IMAGE=${COLLATOR_IMAGE_NAME}:${COLLATOR_IMAGE_TAG} script: - /home/nonroot/zombie-net/scripts/ci/run-test-env-manager.sh --github-remote-dir="${GH_DIR}" - --test="0001-dispute-valid-block.feature" + --test="0003-parachains-bft-disputes.feature" allow_failure: false retry: 2 tags: diff --git a/Cargo.lock b/Cargo.lock index 4e989fd4502b..4d0dbe4b3fdf 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7624,6 +7624,7 @@ dependencies = [ "futures-timer", "parity-util-mem", "polkadot-cli", + "polkadot-erasure-coding", "polkadot-node-core-backing", "polkadot-node-core-candidate-validation", "polkadot-node-core-dispute-coordinator", diff --git a/cli/src/cli.rs b/cli/src/cli.rs index 916c0b83ade8..95c296c00a32 100644 --- a/cli/src/cli.rs +++ b/cli/src/cli.rs @@ -81,6 +81,7 @@ pub struct ValidationWorkerCommand { #[allow(missing_docs)] #[derive(Debug, Parser)] +#[cfg_attr(feature = "malus", derive(Clone))] pub struct RunCmd { #[allow(missing_docs)] #[clap(flatten)] diff --git a/node/core/approval-voting/src/lib.rs b/node/core/approval-voting/src/lib.rs index 6037abd2a66a..eba4c21e7113 100644 --- a/node/core/approval-voting/src/lib.rs +++ b/node/core/approval-voting/src/lib.rs @@ -2273,7 +2273,7 @@ async fn launch_approval( CandidateValidationMessage::ValidateFromExhaustive( available_data.validation_data, validation_code, - candidate.descriptor.clone(), + candidate.clone(), available_data.pov, APPROVAL_EXECUTION_TIMEOUT, val_tx, diff --git a/node/core/backing/src/lib.rs b/node/core/backing/src/lib.rs index 2f85b5ed43aa..9e65e6e1ab98 100644 --- a/node/core/backing/src/lib.rs +++ b/node/core/backing/src/lib.rs @@ -31,8 +31,8 @@ use futures::{ }; use polkadot_node_primitives::{ - AvailableData, PoV, SignedDisputeStatement, SignedFullStatement, Statement, ValidationResult, - BACKING_EXECUTION_TIMEOUT, + AvailableData, InvalidCandidate, PoV, SignedDisputeStatement, SignedFullStatement, Statement, + ValidationResult, BACKING_EXECUTION_TIMEOUT, }; use polkadot_node_subsystem_util::{ self as util, @@ -41,8 +41,8 @@ use polkadot_node_subsystem_util::{ request_validators, FromJobCommand, JobSender, Validator, }; use polkadot_primitives::v2::{ - BackedCandidate, CandidateCommitments, CandidateDescriptor, CandidateHash, CandidateReceipt, - CollatorId, CommittedCandidateReceipt, CoreIndex, CoreState, Hash, Id as ParaId, SessionIndex, + BackedCandidate, CandidateCommitments, CandidateHash, CandidateReceipt, CollatorId, + CommittedCandidateReceipt, CoreIndex, CoreState, Hash, Id as ParaId, SessionIndex, SigningContext, ValidatorId, ValidatorIndex, ValidatorSignature, ValidityAttestation, }; use polkadot_subsystem::{ @@ -378,14 +378,14 @@ async fn request_pov( async fn request_candidate_validation( sender: &mut JobSender, - candidate: CandidateDescriptor, + candidate_receipt: CandidateReceipt, pov: Arc, ) -> Result { let (tx, rx) = oneshot::channel(); sender .send_message(CandidateValidationMessage::ValidateFromChainState( - candidate, + candidate_receipt, pov, BACKING_EXECUTION_TIMEOUT, tx, @@ -456,11 +456,9 @@ async fn validate_and_make_available( .with_pov(&pov) .with_para_id(candidate.descriptor().para_id) }); - request_candidate_validation(&mut sender, candidate.descriptor.clone(), pov.clone()).await? + request_candidate_validation(&mut sender, candidate.clone(), pov.clone()).await? }; - let expected_commitments_hash = candidate.commitments_hash; - let res = match v { ValidationResult::Valid(commitments, validation_data) => { gum::debug!( @@ -469,41 +467,39 @@ async fn validate_and_make_available( "Validation successful", ); - // If validation produces a new set of commitments, we vote the candidate as invalid. - if commitments.hash() != expected_commitments_hash { - gum::debug!( - target: LOG_TARGET, - candidate_hash = ?candidate.hash(), - actual_commitments = ?commitments, - "Commitments obtained with validation don't match the announced by the candidate receipt", - ); - Err(candidate) - } else { - let erasure_valid = make_pov_available( - &mut sender, - n_validators, - pov.clone(), - candidate.hash(), - validation_data, - candidate.descriptor.erasure_root, - span.as_ref(), - ) - .await?; + let erasure_valid = make_pov_available( + &mut sender, + n_validators, + pov.clone(), + candidate.hash(), + validation_data, + candidate.descriptor.erasure_root, + span.as_ref(), + ) + .await?; - match erasure_valid { - Ok(()) => Ok((candidate, commitments, pov.clone())), - Err(InvalidErasureRoot) => { - gum::debug!( - target: LOG_TARGET, - candidate_hash = ?candidate.hash(), - actual_commitments = ?commitments, - "Erasure root doesn't match the announced by the candidate receipt", - ); - Err(candidate) - }, - } + match erasure_valid { + Ok(()) => Ok((candidate, commitments, pov.clone())), + Err(InvalidErasureRoot) => { + gum::debug!( + target: LOG_TARGET, + candidate_hash = ?candidate.hash(), + actual_commitments = ?commitments, + "Erasure root doesn't match the announced by the candidate receipt", + ); + Err(candidate) + }, } }, + ValidationResult::Invalid(InvalidCandidate::CommitmentsHashMismatch) => { + // If validation produces a new set of commitments, we vote the candidate as invalid. + gum::warn!( + target: LOG_TARGET, + candidate_hash = ?candidate.hash(), + "Validation yielded different commitments", + ); + Err(candidate) + }, ValidationResult::Invalid(reason) => { gum::debug!( target: LOG_TARGET, diff --git a/node/core/backing/src/tests.rs b/node/core/backing/src/tests.rs index 47b78bfc281b..eab8b68b3309 100644 --- a/node/core/backing/src/tests.rs +++ b/node/core/backing/src/tests.rs @@ -24,7 +24,8 @@ use futures::{future, Future}; use polkadot_node_primitives::{BlockData, InvalidCandidate}; use polkadot_node_subsystem_test_helpers as test_helpers; use polkadot_primitives::v2::{ - CollatorId, GroupRotationInfo, HeadData, PersistedValidationData, ScheduledCore, + CandidateDescriptor, CollatorId, GroupRotationInfo, HeadData, PersistedValidationData, + ScheduledCore, }; use polkadot_subsystem::{ messages::{ @@ -332,12 +333,12 @@ fn backing_second_works() { virtual_overseer.recv().await, AllMessages::CandidateValidation( CandidateValidationMessage::ValidateFromChainState( - c, + candidate_receipt, pov, timeout, tx, ) - ) if pov == pov && &c == candidate.descriptor() && timeout == BACKING_EXECUTION_TIMEOUT => { + ) if pov == pov && &candidate_receipt.descriptor == candidate.descriptor() && timeout == BACKING_EXECUTION_TIMEOUT && candidate.commitments.hash() == candidate_receipt.commitments_hash => { tx.send(Ok( ValidationResult::Valid(CandidateCommitments { head_data: expected_head_data.clone(), @@ -419,6 +420,8 @@ fn backing_works() { .build(); let candidate_a_hash = candidate_a.hash(); + let candidate_a_commitments_hash = candidate_a.commitments.hash(); + let public1 = CryptoStore::sr25519_generate_new( &*test_state.keystore, ValidatorId::ID, @@ -497,7 +500,7 @@ fn backing_works() { timeout, tx, ) - ) if pov == pov && &c == candidate_a.descriptor() && timeout == BACKING_EXECUTION_TIMEOUT => { + ) if pov == pov && c.descriptor() == candidate_a.descriptor() && timeout == BACKING_EXECUTION_TIMEOUT && c.commitments_hash == candidate_a_commitments_hash=> { tx.send(Ok( ValidationResult::Valid(CandidateCommitments { head_data: expected_head_data.clone(), @@ -594,6 +597,8 @@ fn backing_works_while_validation_ongoing() { .build(); let candidate_a_hash = candidate_a.hash(); + let candidate_a_commitments_hash = candidate_a.commitments.hash(); + let public1 = CryptoStore::sr25519_generate_new( &*test_state.keystore, ValidatorId::ID, @@ -691,7 +696,7 @@ fn backing_works_while_validation_ongoing() { timeout, tx, ) - ) if pov == pov && &c == candidate_a.descriptor() && timeout == BACKING_EXECUTION_TIMEOUT => { + ) if pov == pov && c.descriptor() == candidate_a.descriptor() && timeout == BACKING_EXECUTION_TIMEOUT && candidate_a_commitments_hash == c.commitments_hash => { // we never validate the candidate. our local node // shouldn't issue any statements. std::mem::forget(tx); @@ -799,6 +804,8 @@ fn backing_misbehavior_works() { .build(); let candidate_a_hash = candidate_a.hash(); + let candidate_a_commitments_hash = candidate_a.commitments.hash(); + let public2 = CryptoStore::sr25519_generate_new( &*test_state.keystore, ValidatorId::ID, @@ -865,7 +872,7 @@ fn backing_misbehavior_works() { timeout, tx, ) - ) if pov == pov && &c == candidate_a.descriptor() && timeout == BACKING_EXECUTION_TIMEOUT => { + ) if pov == pov && c.descriptor() == candidate_a.descriptor() && timeout == BACKING_EXECUTION_TIMEOUT && candidate_a_commitments_hash == c.commitments_hash => { tx.send(Ok( ValidationResult::Valid(CandidateCommitments { head_data: expected_head_data.clone(), @@ -1025,7 +1032,7 @@ fn backing_dont_second_invalid() { timeout, tx, ) - ) if pov == pov && &c == candidate_a.descriptor() && timeout == BACKING_EXECUTION_TIMEOUT => { + ) if pov == pov && c.descriptor() == candidate_a.descriptor() && timeout == BACKING_EXECUTION_TIMEOUT => { tx.send(Ok(ValidationResult::Invalid(InvalidCandidate::BadReturn))).unwrap(); } ); @@ -1054,7 +1061,7 @@ fn backing_dont_second_invalid() { timeout, tx, ) - ) if pov == pov && &c == candidate_b.descriptor() && timeout == BACKING_EXECUTION_TIMEOUT => { + ) if pov == pov && c.descriptor() == candidate_b.descriptor() && timeout == BACKING_EXECUTION_TIMEOUT => { tx.send(Ok( ValidationResult::Valid(CandidateCommitments { head_data: expected_head_data.clone(), @@ -1185,7 +1192,7 @@ fn backing_second_after_first_fails_works() { timeout, tx, ) - ) if pov == pov && &c == candidate.descriptor() && timeout == BACKING_EXECUTION_TIMEOUT => { + ) if pov == pov && c.descriptor() == candidate.descriptor() && timeout == BACKING_EXECUTION_TIMEOUT && c.commitments_hash == candidate.commitments.hash() => { tx.send(Ok(ValidationResult::Invalid(InvalidCandidate::BadReturn))).unwrap(); } ); @@ -1319,7 +1326,7 @@ fn backing_works_after_failed_validation() { timeout, tx, ) - ) if pov == pov && &c == candidate.descriptor() && timeout == BACKING_EXECUTION_TIMEOUT => { + ) if pov == pov && c.descriptor() == candidate.descriptor() && timeout == BACKING_EXECUTION_TIMEOUT && c.commitments_hash == candidate.commitments.hash() => { tx.send(Err(ValidationFailed("Internal test error".into()))).unwrap(); } ); @@ -1696,7 +1703,7 @@ fn retry_works() { timeout, _tx, ) - ) if pov == pov && &c == candidate.descriptor() && timeout == BACKING_EXECUTION_TIMEOUT + ) if pov == pov && c.descriptor() == candidate.descriptor() && timeout == BACKING_EXECUTION_TIMEOUT && c.commitments_hash == candidate.commitments.hash() ); virtual_overseer }); diff --git a/node/core/candidate-validation/src/lib.rs b/node/core/candidate-validation/src/lib.rs index 8256690bac47..74e49990ba16 100644 --- a/node/core/candidate-validation/src/lib.rs +++ b/node/core/candidate-validation/src/lib.rs @@ -41,7 +41,7 @@ use polkadot_node_subsystem::{ use polkadot_node_subsystem_util::metrics::{self, prometheus}; use polkadot_parachain::primitives::{ValidationParams, ValidationResult as WasmValidationResult}; use polkadot_primitives::v2::{ - CandidateCommitments, CandidateDescriptor, Hash, OccupiedCoreAssumption, + CandidateCommitments, CandidateDescriptor, CandidateReceipt, Hash, OccupiedCoreAssumption, PersistedValidationData, ValidationCode, ValidationCodeHash, }; @@ -134,7 +134,7 @@ where FromOverseer::Signal(OverseerSignal::Conclude) => return Ok(()), FromOverseer::Communication { msg } => match msg { CandidateValidationMessage::ValidateFromChainState( - descriptor, + candidate_receipt, pov, timeout, response_sender, @@ -149,7 +149,7 @@ where let res = validate_from_chain_state( &mut sender, validation_host, - descriptor, + candidate_receipt, pov, timeout, &metrics, @@ -166,7 +166,7 @@ where CandidateValidationMessage::ValidateFromExhaustive( persisted_validation_data, validation_code, - descriptor, + candidate_receipt, pov, timeout, response_sender, @@ -181,7 +181,7 @@ where validation_host, persisted_validation_data, validation_code, - descriptor, + candidate_receipt, pov, timeout, &metrics, @@ -413,10 +413,32 @@ where 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, - descriptor: CandidateDescriptor, + candidate_receipt: CandidateReceipt, pov: Arc, timeout: Duration, metrics: &Metrics, @@ -424,25 +446,18 @@ async fn validate_from_chain_state( where Sender: SubsystemSender, { + let mut new_sender = sender.clone(); let (validation_data, validation_code) = - 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(ValidationResult::Invalid(InvalidCandidate::BadParent)) - }, - AssumptionCheckOutcome::BadRequest => - return Err(ValidationFailed("Assumption Check: Bad request".into())), + 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, - descriptor.clone(), + candidate_receipt.clone(), pov, timeout, metrics, @@ -450,11 +465,20 @@ where .await; if let Ok(ValidationResult::Valid(ref outputs, _)) = validation_result { + // If validation produces new commitments we consider the candidate invalid. + if candidate_receipt.commitments_hash != outputs.hash() { + return Ok(ValidationResult::Invalid(InvalidCandidate::CommitmentsHashMismatch)) + } + let (tx, rx) = oneshot::channel(); match runtime_api_request( sender, - descriptor.relay_parent, - RuntimeApiRequest::CheckValidationOutputs(descriptor.para_id, outputs.clone(), tx), + candidate_receipt.descriptor.relay_parent, + RuntimeApiRequest::CheckValidationOutputs( + candidate_receipt.descriptor.para_id, + outputs.clone(), + tx, + ), rx, ) .await @@ -473,7 +497,7 @@ async fn validate_candidate_exhaustive( mut validation_backend: impl ValidationBackend, persisted_validation_data: PersistedValidationData, validation_code: ValidationCode, - descriptor: CandidateDescriptor, + candidate_receipt: CandidateReceipt, pov: Arc, timeout: Duration, metrics: &Metrics, @@ -484,12 +508,12 @@ async fn validate_candidate_exhaustive( gum::debug!( target: LOG_TARGET, ?validation_code_hash, - para_id = ?descriptor.para_id, + para_id = ?candidate_receipt.descriptor.para_id, "About to validate a candidate.", ); if let Err(e) = perform_basic_checks( - &descriptor, + &candidate_receipt.descriptor, persisted_validation_data.max_pov_size, &*pov, &validation_code_hash, @@ -555,7 +579,7 @@ async fn validate_candidate_exhaustive( Ok(ValidationResult::Invalid(InvalidCandidate::ExecutionError(e))), Ok(res) => - if res.head_data.hash() != descriptor.para_head { + if res.head_data.hash() != candidate_receipt.descriptor.para_head { Ok(ValidationResult::Invalid(InvalidCandidate::ParaHeadHashMismatch)) } else { let outputs = CandidateCommitments { @@ -566,7 +590,12 @@ async fn validate_candidate_exhaustive( processed_downward_messages: res.processed_downward_messages, hrmp_watermark: res.hrmp_watermark, }; - Ok(ValidationResult::Valid(outputs, persisted_validation_data)) + if candidate_receipt.commitments_hash != outputs.hash() { + // If validation produced a new set of commitments, we treat the candidate as invalid. + Ok(ValidationResult::Invalid(InvalidCandidate::CommitmentsHashMismatch)) + } else { + Ok(ValidationResult::Valid(outputs, persisted_validation_data)) + } }, } } diff --git a/node/core/candidate-validation/src/tests.rs b/node/core/candidate-validation/src/tests.rs index 29c4185ed2cb..b896a9f7f3d1 100644 --- a/node/core/candidate-validation/src/tests.rs +++ b/node/core/candidate-validation/src/tests.rs @@ -406,11 +406,22 @@ fn candidate_validation_ok_is_ok() { hrmp_watermark: 0, }; + let commitments = CandidateCommitments { + head_data: validation_result.head_data.clone(), + upward_messages: validation_result.upward_messages.clone(), + horizontal_messages: validation_result.horizontal_messages.clone(), + new_validation_code: validation_result.new_validation_code.clone(), + processed_downward_messages: validation_result.processed_downward_messages, + hrmp_watermark: validation_result.hrmp_watermark, + }; + + let candidate_receipt = CandidateReceipt { descriptor, commitments_hash: commitments.hash() }; + let v = executor::block_on(validate_candidate_exhaustive( MockValidateCandidateBackend::with_hardcoded_result(Ok(validation_result)), validation_data.clone(), validation_code, - descriptor, + candidate_receipt, Arc::new(pov), Duration::from_secs(0), &Default::default(), @@ -453,13 +464,15 @@ fn candidate_validation_bad_return_is_invalid() { ); assert!(check.is_ok()); + 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::AmbiguousWorkerDeath), )), validation_data, validation_code, - descriptor, + candidate_receipt, Arc::new(pov), Duration::from_secs(0), &Default::default(), @@ -495,13 +508,15 @@ fn candidate_validation_timeout_is_internal_error() { ); assert!(check.is_ok()); + 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), )), validation_data, validation_code, - descriptor, + candidate_receipt, Arc::new(pov), Duration::from_secs(0), &Default::default(), @@ -510,6 +525,52 @@ fn candidate_validation_timeout_is_internal_error() { assert_matches!(v, Ok(ValidationResult::Invalid(InvalidCandidate::Timeout))); } +#[test] +fn candidate_validation_commitment_hash_mismatch_is_invalid() { + let validation_data = PersistedValidationData { max_pov_size: 1024, ..Default::default() }; + let pov = PoV { block_data: BlockData(vec![0xff; 32]) }; + let validation_code = ValidationCode(vec![0xff; 16]); + let head_data = HeadData(vec![1, 1, 1]); + + let candidate_receipt = CandidateReceipt { + descriptor: make_valid_candidate_descriptor( + 1.into(), + validation_data.parent_head.hash(), + validation_data.hash(), + pov.hash(), + validation_code.hash(), + head_data.hash(), + dummy_hash(), + Sr25519Keyring::Alice, + ), + commitments_hash: Hash::zero(), + }; + + // This will result in different commitments for this candidate. + let validation_result = WasmValidationResult { + head_data, + new_validation_code: None, + upward_messages: Vec::new(), + horizontal_messages: Vec::new(), + processed_downward_messages: 0, + hrmp_watermark: 12345, + }; + + let result = executor::block_on(validate_candidate_exhaustive( + MockValidateCandidateBackend::with_hardcoded_result(Ok(validation_result)), + validation_data, + validation_code, + candidate_receipt, + Arc::new(pov), + Duration::from_secs(0), + &Default::default(), + )) + .unwrap(); + + // Ensure `post validation` check on the commitments hash works as expected. + assert_matches!(result, ValidationResult::Invalid(InvalidCandidate::CommitmentsHashMismatch)); +} + #[test] fn candidate_validation_code_mismatch_is_invalid() { let validation_data = PersistedValidationData { max_pov_size: 1024, ..Default::default() }; @@ -536,13 +597,15 @@ fn candidate_validation_code_mismatch_is_invalid() { ); assert_matches!(check, Err(InvalidCandidate::CodeHashMismatch)); + 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), )), validation_data, validation_code, - descriptor, + candidate_receipt, Arc::new(pov), Duration::from_secs(0), &Default::default(), @@ -583,11 +646,22 @@ fn compressed_code_works() { hrmp_watermark: 0, }; + let commitments = CandidateCommitments { + head_data: validation_result.head_data.clone(), + upward_messages: validation_result.upward_messages.clone(), + horizontal_messages: validation_result.horizontal_messages.clone(), + new_validation_code: validation_result.new_validation_code.clone(), + processed_downward_messages: validation_result.processed_downward_messages, + hrmp_watermark: validation_result.hrmp_watermark, + }; + + let candidate_receipt = CandidateReceipt { descriptor, commitments_hash: commitments.hash() }; + let v = executor::block_on(validate_candidate_exhaustive( MockValidateCandidateBackend::with_hardcoded_result(Ok(validation_result)), validation_data, validation_code, - descriptor, + candidate_receipt, Arc::new(pov), Duration::from_secs(0), &Default::default(), @@ -628,11 +702,13 @@ fn code_decompression_failure_is_invalid() { hrmp_watermark: 0, }; + let candidate_receipt = CandidateReceipt { descriptor, commitments_hash: Hash::zero() }; + let v = executor::block_on(validate_candidate_exhaustive( MockValidateCandidateBackend::with_hardcoded_result(Ok(validation_result)), validation_data, validation_code, - descriptor, + candidate_receipt, Arc::new(pov), Duration::from_secs(0), &Default::default(), @@ -674,11 +750,13 @@ fn pov_decompression_failure_is_invalid() { hrmp_watermark: 0, }; + let candidate_receipt = CandidateReceipt { descriptor, commitments_hash: Hash::zero() }; + let v = executor::block_on(validate_candidate_exhaustive( MockValidateCandidateBackend::with_hardcoded_result(Ok(validation_result)), validation_data, validation_code, - descriptor, + candidate_receipt, Arc::new(pov), Duration::from_secs(0), &Default::default(), diff --git a/node/core/dispute-coordinator/src/real/participation/mod.rs b/node/core/dispute-coordinator/src/real/participation/mod.rs index 4ed4015cd6db..7f43a22b11cb 100644 --- a/node/core/dispute-coordinator/src/real/participation/mod.rs +++ b/node/core/dispute-coordinator/src/real/participation/mod.rs @@ -362,7 +362,7 @@ async fn participate( CandidateValidationMessage::ValidateFromExhaustive( available_data.validation_data, validation_code, - req.candidate_receipt().descriptor.clone(), + req.candidate_receipt().clone(), available_data.pov, APPROVAL_EXECUTION_TIMEOUT, validation_tx, @@ -393,6 +393,7 @@ async fn participate( send_result(&mut result_sender, req, ParticipationOutcome::Invalid).await; }, + Ok(Ok(ValidationResult::Invalid(invalid))) => { gum::warn!( target: LOG_TARGET, @@ -403,19 +404,8 @@ async fn participate( send_result(&mut result_sender, req, ParticipationOutcome::Invalid).await; }, - Ok(Ok(ValidationResult::Valid(commitments, _))) => { - if commitments.hash() != req.candidate_receipt().commitments_hash { - gum::warn!( - target: LOG_TARGET, - expected = ?req.candidate_receipt().commitments_hash, - got = ?commitments.hash(), - "Candidate is valid but commitments hash doesn't match", - ); - - send_result(&mut result_sender, req, ParticipationOutcome::Invalid).await; - } else { - send_result(&mut result_sender, req, ParticipationOutcome::Valid).await; - } + Ok(Ok(ValidationResult::Valid(_, _))) => { + send_result(&mut result_sender, req, ParticipationOutcome::Valid).await; }, } } diff --git a/node/core/dispute-coordinator/src/real/participation/tests.rs b/node/core/dispute-coordinator/src/real/participation/tests.rs index bbb83e9fcda0..d0b38eccbcbd 100644 --- a/node/core/dispute-coordinator/src/real/participation/tests.rs +++ b/node/core/dispute-coordinator/src/real/participation/tests.rs @@ -108,7 +108,10 @@ async fn activate_leaf( } /// Full participation happy path as seen via the overseer. -pub async fn participation_full_happy_path(ctx_handle: &mut VirtualOverseer) { +pub async fn participation_full_happy_path( + ctx_handle: &mut VirtualOverseer, + expected_commitments_hash: Hash, +) { recover_available_data(ctx_handle).await; fetch_validation_code(ctx_handle).await; store_available_data(ctx_handle, true).await; @@ -116,9 +119,13 @@ pub async fn participation_full_happy_path(ctx_handle: &mut VirtualOverseer) { assert_matches!( ctx_handle.recv().await, AllMessages::CandidateValidation( - CandidateValidationMessage::ValidateFromExhaustive(_, _, _, _, timeout, tx) + CandidateValidationMessage::ValidateFromExhaustive(_, _, candidate_receipt, _, timeout, tx) ) if timeout == APPROVAL_EXECUTION_TIMEOUT => { - tx.send(Ok(ValidationResult::Valid(dummy_candidate_commitments(None), PersistedValidationData::default()))).unwrap(); + if expected_commitments_hash != candidate_receipt.commitments_hash { + tx.send(Ok(ValidationResult::Invalid(InvalidCandidate::CommitmentsHashMismatch))).unwrap(); + } else { + tx.send(Ok(ValidationResult::Valid(dummy_candidate_commitments(None), PersistedValidationData::default()))).unwrap(); + } }, "overseer did not receive candidate validation message", ); @@ -438,7 +445,7 @@ fn cast_invalid_vote_if_validation_fails_or_is_invalid() { } #[test] -fn cast_invalid_vote_if_validation_passes_but_commitments_dont_match() { +fn cast_invalid_vote_if_commitments_dont_match() { futures::executor::block_on(async { let (mut ctx, mut ctx_handle) = make_our_subsystem_context(TaskExecutor::new()); @@ -459,11 +466,7 @@ fn cast_invalid_vote_if_validation_passes_but_commitments_dont_match() { AllMessages::CandidateValidation( CandidateValidationMessage::ValidateFromExhaustive(_, _, _, _, timeout, tx) ) if timeout == APPROVAL_EXECUTION_TIMEOUT => { - let mut commitments = CandidateCommitments::default(); - // this should lead to a commitments hash mismatch - commitments.processed_downward_messages = 42; - - tx.send(Ok(ValidationResult::Valid(commitments, PersistedValidationData::default()))).unwrap(); + tx.send(Ok(ValidationResult::Invalid(InvalidCandidate::CommitmentsHashMismatch))).unwrap(); }, "overseer did not receive candidate validation message", ); diff --git a/node/core/dispute-coordinator/src/real/tests.rs b/node/core/dispute-coordinator/src/real/tests.rs index bfed38265513..54b952fd9e2b 100644 --- a/node/core/dispute-coordinator/src/real/tests.rs +++ b/node/core/dispute-coordinator/src/real/tests.rs @@ -407,8 +407,9 @@ where async fn participation_with_distribution( virtual_overseer: &mut VirtualOverseer, candidate_hash: &CandidateHash, + expected_commitments_hash: Hash, ) { - participation_full_happy_path(virtual_overseer).await; + participation_full_happy_path(virtual_overseer, expected_commitments_hash).await; assert_matches!( virtual_overseer.recv().await, AllMessages::DisputeDistribution( @@ -426,7 +427,6 @@ fn make_valid_candidate_receipt() -> CandidateReceipt { } fn make_invalid_candidate_receipt() -> CandidateReceipt { - // Commitments hash will be 0, which is not correct: dummy_candidate_receipt_bad_sig(Default::default(), Some(Default::default())) } @@ -593,7 +593,12 @@ fn dispute_gets_confirmed_via_participation() { }) .await; - participation_with_distribution(&mut virtual_overseer, &candidate_hash1).await; + participation_with_distribution( + &mut virtual_overseer, + &candidate_hash1, + candidate_receipt1.commitments_hash, + ) + .await; { let (tx, rx) = oneshot::channel(); @@ -942,7 +947,12 @@ fn conflicting_votes_lead_to_dispute_participation() { }) .await; - participation_with_distribution(&mut virtual_overseer, &candidate_hash).await; + participation_with_distribution( + &mut virtual_overseer, + &candidate_hash, + candidate_receipt.commitments_hash, + ) + .await; { let (tx, rx) = oneshot::channel(); @@ -1224,7 +1234,12 @@ fn finality_votes_ignore_disputed_candidates() { }) .await; - participation_with_distribution(&mut virtual_overseer, &candidate_hash).await; + participation_with_distribution( + &mut virtual_overseer, + &candidate_hash, + candidate_receipt.commitments_hash, + ) + .await; { let (tx, rx) = oneshot::channel(); @@ -1322,7 +1337,12 @@ fn supermajority_valid_dispute_may_be_finalized() { }) .await; - participation_with_distribution(&mut virtual_overseer, &candidate_hash).await; + participation_with_distribution( + &mut virtual_overseer, + &candidate_hash, + candidate_receipt.commitments_hash, + ) + .await; let mut statements = Vec::new(); for i in (0..supermajority_threshold - 1).map(|i| i + 3) { @@ -1442,7 +1462,12 @@ fn concluded_supermajority_for_non_active_after_time() { }) .await; - participation_with_distribution(&mut virtual_overseer, &candidate_hash).await; + participation_with_distribution( + &mut virtual_overseer, + &candidate_hash, + candidate_receipt.commitments_hash, + ) + .await; let mut statements = Vec::new(); // -2: 1 for already imported vote and one for local vote (which is valid). @@ -1543,7 +1568,13 @@ fn concluded_supermajority_against_non_active_after_time() { ImportStatementsResult::ValidImport => {} ); - participation_with_distribution(&mut virtual_overseer, &candidate_hash).await; + // Use a different expected commitments hash to ensure the candidate validation returns invalid. + participation_with_distribution( + &mut virtual_overseer, + &candidate_hash, + CandidateCommitments::default().hash(), + ) + .await; let mut statements = Vec::new(); // minus 2, because of local vote and one previously imported invalid vote. @@ -1580,7 +1611,6 @@ fn concluded_supermajority_against_non_active_after_time() { .await; assert!(rx.await.unwrap().is_empty()); - let (tx, rx) = oneshot::channel(); virtual_overseer @@ -1672,7 +1702,12 @@ fn resume_dispute_without_local_statement() { let candidate_receipt = make_valid_candidate_receipt(); let candidate_hash = candidate_receipt.hash(); - participation_with_distribution(&mut virtual_overseer, &candidate_hash).await; + participation_with_distribution( + &mut virtual_overseer, + &candidate_hash, + candidate_receipt.commitments_hash, + ) + .await; let valid_vote0 = test_state .issue_explicit_statement_with_index(0, candidate_hash, session, true) diff --git a/node/malus/Cargo.toml b/node/malus/Cargo.toml index f6a02a2e99ed..74a5c3a7627a 100644 --- a/node/malus/Cargo.toml +++ b/node/malus/Cargo.toml @@ -28,10 +28,12 @@ color-eyre = { version = "0.6.1", default-features = false } assert_matches = "1.5" async-trait = "0.1.53" sp-keystore = { git = "https://github.com/paritytech/substrate", branch = "master" } +sp-core = { git = "https://github.com/paritytech/substrate", branch = "master" } clap = { version = "3.1", features = ["derive"] } futures = "0.3.21" futures-timer = "3.0.2" gum = { package = "tracing-gum", path = "../gum/" } +erasure = { package = "polkadot-erasure-coding", path = "../../erasure-coding" } [features] default = [] diff --git a/node/malus/integrationtests/0001-dispute-valid-block.feature b/node/malus/integrationtests/0001-dispute-valid-block.feature deleted file mode 100644 index fe0c14c791e4..000000000000 --- a/node/malus/integrationtests/0001-dispute-valid-block.feature +++ /dev/null @@ -1,29 +0,0 @@ -Description: Disputes -Network: ./0001-dispute-valid-block.toml -Creds: config - - -alice: is up -bob: is up -charlie: is up -david is up -alice: reports node_roles is 4 -bob: reports node_roles is 4 -alice: reports sub_libp2p_is_major_syncing is 0 -alice: reports block height is at least 2 within 15 seconds -alice: reports peers count is at least 2 -bob: reports block height is at least 2 -bob: reports peers count is at least 2 -charlie: reports block height is at least 2 -charlie: reports peers count is at least 2 -alice: reports parachain_candidate_disputes_total is at least 1 within 250 seconds -bob: reports parachain_candidate_disputes_total is at least 1 within 90 seconds -charlie: reports parachain_candidate_disputes_total is at least 1 within 90 seconds -alice: reports parachain_candidate_dispute_votes{validity="valid"} is at least 1 within 90 seconds -bob: reports parachain_candidate_dispute_votes{validity="valid"} is at least 2 within 90 seconds -charlie: reports parachain_candidate_dispute_votes{validity="valid"} is at least 2 within 90 seconds -alice: reports parachain_candidate_dispute_concluded{validity="valid"} is at least 1 within 90 seconds -alice: reports parachain_candidate_dispute_concluded{validity="invalid"} is 0 within 90 seconds -bob: reports parachain_candidate_dispute_concluded{validity="valid"} is at least 1 within 90 seconds -charlie: reports parachain_candidate_dispute_concluded{validity="valid"} is at least 1 within 90 seconds -charlie: reports parachain_candidate_dispute_concluded{validity="valid"} is at least 1 within 90 seconds diff --git a/node/malus/integrationtests/0001-dispute-valid-block.toml b/node/malus/integrationtests/0001-dispute-valid-block.toml deleted file mode 100644 index 9a4917841d30..000000000000 --- a/node/malus/integrationtests/0001-dispute-valid-block.toml +++ /dev/null @@ -1,39 +0,0 @@ -[settings] -timeout = 1000 - -[relaychain] -default_image = "{{ZOMBIENET_INTEGRATION_TEST_IMAGE}}" -chain = "wococo-local" -command = "polkadot" - - [[relaychain.nodes]] - name = "alice" - validator = true - extra_args = [ "--alice", "-lparachain=debug" ] - - [[relaychain.nodes]] - name = "bob" - validator = true - extra_args = [ "--bob", "-lparachain=debug" ] - - [[relaychain.nodes]] - name = "charlie" - validator = true - extra_args = [ "--charlie", "-lparachain=debug" ] - - [[relaychain.nodes]] - name = "dave" - validator = true - command = "/usr/local/bin/malus dispute-ancestor" - extra_args = ["--dave", "-lparachain=debug"] - image = "{{MALUS_IMAGE}}" - autoConnectApi = false - -[[parachains]] -id = 100 - - [parachains.collator] - name = "collator01" - image = "{{COL_IMAGE}}" - command = "/usr/local/bin/adder-collator" - args = ["-lparachain=debug"] diff --git a/node/malus/src/malus.rs b/node/malus/src/malus.rs index 11b50cb2c28c..88b3f8130abd 100644 --- a/node/malus/src/malus.rs +++ b/node/malus/src/malus.rs @@ -37,7 +37,7 @@ enum NemesisVariant { /// Back a candidate with a specifically crafted proof of validity. BackGarbageCandidate(RunCmd), /// Delayed disputing of ancestors that are perfectly fine. - DisputeAncestor(RunCmd), + DisputeAncestor(DisputeAncestorOptions), #[allow(missing_docs)] #[clap(name = "prepare-worker", hide = true)] @@ -66,9 +66,11 @@ impl MalusCli { NemesisVariant::BackGarbageCandidate(cmd) => polkadot_cli::run_node(run_cmd(cmd), BackGarbageCandidate)?, NemesisVariant::SuggestGarbageCandidate(cmd) => - polkadot_cli::run_node(run_cmd(cmd), SuggestGarbageCandidate)?, - NemesisVariant::DisputeAncestor(cmd) => - polkadot_cli::run_node(run_cmd(cmd), DisputeValidCandidates)?, + polkadot_cli::run_node(run_cmd(cmd), BackGarbageCandidateWrapper)?, + NemesisVariant::DisputeAncestor(opts) => polkadot_cli::run_node( + run_cmd(opts.clone().cmd), + DisputeValidCandidates::new(opts), + )?, NemesisVariant::PvfPrepareWorker(cmd) => { #[cfg(target_os = "android")] { @@ -120,7 +122,7 @@ mod tests { variant: NemesisVariant::DisputeAncestor(run), .. } => { - assert!(run.base.bob); + assert!(run.cmd.base.bob); }); } } diff --git a/node/malus/src/shared.rs b/node/malus/src/shared.rs index 3c1d55d0d88a..123497340ec5 100644 --- a/node/malus/src/shared.rs +++ b/node/malus/src/shared.rs @@ -17,7 +17,7 @@ use futures::prelude::*; use polkadot_node_primitives::SpawnNamed; -pub const MALUS: &str = "MALUS😈😈😈"; +pub const MALUS: &str = "MALUS"; #[allow(unused)] pub(crate) const MALICIOUS_POV: &[u8] = "😈😈pov_looks_valid_to_me😈😈".as_bytes(); diff --git a/node/malus/src/variants/back_garbage_candidate.rs b/node/malus/src/variants/back_garbage_candidate.rs index 5cca3144a8a1..5ece8cd4bd7b 100644 --- a/node/malus/src/variants/back_garbage_candidate.rs +++ b/node/malus/src/variants/back_garbage_candidate.rs @@ -14,10 +14,9 @@ // You should have received a copy of the GNU General Public License // along with Polkadot. If not, see . -//! A malicious overseer backing a particular candidate with a -//! malicious proof of validity that is received. - -#![allow(missing_docs)] +//! This variant of Malus backs/approves all malicious candidates crafted by +//! `suggest-garbage-candidate` variant and behaves honestly with other +//! candidates. use polkadot_cli::{ prepared_overseer_builder, @@ -28,170 +27,15 @@ use polkadot_cli::{ }, }; -// Import extra types relevant to the particular -// subsystem. -use polkadot_node_core_candidate_validation::CandidateValidationSubsystem; -use polkadot_node_subsystem::messages::{ - AvailabilityRecoveryMessage, CandidateValidationMessage, ValidationFailed, -}; -use polkadot_node_subsystem_util as util; - -// Filter wrapping related types. -use crate::{interceptor::*, shared::*}; -use polkadot_node_primitives::{PoV, ValidationResult}; - -use polkadot_primitives::v2::{ - CandidateCommitments, CandidateDescriptor, CandidateReceipt, PersistedValidationData, - ValidationCode, -}; - -use futures::channel::oneshot; -use std::{ - collections::HashMap, - sync::{Arc, Mutex}, +use crate::{ + interceptor::*, + variants::{FakeCandidateValidation, FakeCandidateValidationError, ReplaceValidationResult}, }; -#[derive(Clone, Debug)] -struct BribedPassageInner { - spawner: Spawner, - cache: HashMap, -} - -#[derive(Clone, Debug)] -struct BribedPassage { - inner: Arc>>, -} - -impl BribedPassage -where - Spawner: SpawnNamed, -{ - fn let_pass( - persisted_validation_data: PersistedValidationData, - validation_code: Option, - _candidate_descriptor: CandidateDescriptor, - _pov: Arc, - response_sender: oneshot::Sender>, - ) { - let candidate_commitmentments = CandidateCommitments { - head_data: persisted_validation_data.parent_head.clone(), - new_validation_code: validation_code, - ..Default::default() - }; - - response_sender - .send(Ok(ValidationResult::Valid(candidate_commitmentments, persisted_validation_data))) - .unwrap(); - } -} - -impl MessageInterceptor for BribedPassage -where - Sender: overseer::SubsystemSender - + overseer::SubsystemSender - + Clone - + Send - + 'static, - Spawner: SpawnNamed + Send + Clone + 'static, -{ - type Message = CandidateValidationMessage; - - fn intercept_incoming( - &self, - sender: &mut Sender, - msg: FromOverseer, - ) -> Option> { - match msg { - FromOverseer::Communication { - msg: - CandidateValidationMessage::ValidateFromExhaustive( - persisted_validation_data, - validation_code, - candidate_descriptor, - pov, - _duration, - response_sender, - ), - } if pov.block_data.0.as_slice() == MALICIOUS_POV => { - Self::let_pass( - persisted_validation_data, - Some(validation_code), - candidate_descriptor, - pov, - response_sender, - ); - None - }, - FromOverseer::Communication { - msg: - CandidateValidationMessage::ValidateFromChainState( - candidate_descriptor, - pov, - _duration, - response_sender, - ), - } if pov.block_data.0.as_slice() == MALICIOUS_POV => { - if let Some(candidate_receipt) = - self.inner.lock().unwrap().cache.get(&candidate_descriptor).cloned() - { - let mut subsystem_sender = sender.clone(); - let spawner = self.inner.lock().unwrap().spawner.clone(); - spawner.spawn( - "malus-back-garbage-adhoc", - Some("malus"), - Box::pin(async move { - let relay_parent = candidate_descriptor.relay_parent; - let session_index = util::request_session_index_for_child( - relay_parent, - &mut subsystem_sender, - ) - .await; - let session_index = session_index.await.unwrap().unwrap(); - - let (a_tx, a_rx) = oneshot::channel(); - - subsystem_sender - .send_message(AllMessages::from( - AvailabilityRecoveryMessage::RecoverAvailableData( - candidate_receipt, - session_index, - None, - a_tx, - ), - )) - .await; - - if let Ok(Ok(availability_data)) = a_rx.await { - Self::let_pass( - availability_data.validation_data, - None, - candidate_descriptor, - pov, - response_sender, - ); - } else { - gum::info!( - target: MALUS, - "Could not get availability data, can't back" - ); - } - }), - ); - } else { - gum::info!(target: MALUS, "No CandidateReceipt available to work with"); - } - None - }, - msg => Some(msg), - } - } - - fn intercept_outgoing(&self, msg: AllMessages) -> Option { - Some(msg) - } -} +use std::sync::Arc; -/// Generates an overseer that exposes bad behavior. +/// Generates an overseer that replaces the candidate validation subsystem with our malicious +/// variant. pub(crate) struct BackGarbageCandidate; impl OverseerGen for BackGarbageCandidate { @@ -205,24 +49,16 @@ impl OverseerGen for BackGarbageCandidate { RuntimeClient::Api: ParachainHost + BabeApi + AuthorityDiscoveryApi, Spawner: 'static + SpawnNamed + Clone + Unpin, { - let candidate_validation_config = args.candidate_validation_config.clone(); let spawner = args.spawner.clone(); + let validation_filter = ReplaceValidationResult::new( + FakeCandidateValidation::BackingAndApprovalValid, + FakeCandidateValidationError::InvalidOutputs, + spawner.clone(), + ); prepared_overseer_builder(args)? - .replace_candidate_validation(|cv| { - InterceptedSubsystem::new( - CandidateValidationSubsystem::with_config( - candidate_validation_config, - cv.metrics, - cv.pvf_metrics, - ), - BribedPassage:: { - inner: Arc::new(Mutex::new(BribedPassageInner { - spawner, - cache: Default::default(), - })), - }, - ) + .replace_candidate_validation(move |cv_subsystem| { + InterceptedSubsystem::new(cv_subsystem, validation_filter) }) .build_with_connector(connector) .map_err(|e| e.into()) diff --git a/node/malus/src/variants/common.rs b/node/malus/src/variants/common.rs new file mode 100644 index 000000000000..8bed137126d1 --- /dev/null +++ b/node/malus/src/variants/common.rs @@ -0,0 +1,342 @@ +// Copyright 2022 Parity Technologies (UK) Ltd. +// This file is part of Polkadot. + +// Polkadot is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// Polkadot is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with Polkadot. If not, see . + +//! Implements common code for nemesis. Currently, only `FakeValidationResult` +//! interceptor is implemented. +use crate::{ + interceptor::*, + shared::{MALICIOUS_POV, MALUS}, +}; + +use polkadot_node_core_candidate_validation::find_validation_data; +use polkadot_node_primitives::{InvalidCandidate, ValidationResult}; +use polkadot_node_subsystem::messages::{CandidateValidationMessage, ValidationFailed}; + +use polkadot_primitives::v2::{ + CandidateCommitments, CandidateDescriptor, CandidateReceipt, PersistedValidationData, +}; + +use polkadot_cli::service::SpawnNamed; + +use futures::channel::oneshot; + +#[derive(clap::ArgEnum, Clone, Copy, Debug, PartialEq)] +#[clap(rename_all = "kebab-case")] +#[non_exhaustive] +pub enum FakeCandidateValidation { + Disabled, + BackingInvalid, + ApprovalInvalid, + BackingAndApprovalInvalid, + BackingValid, + ApprovalValid, + BackingAndApprovalValid, +} + +/// Candidate invalidity details +#[derive(clap::ArgEnum, Clone, Copy, Debug, PartialEq)] +#[clap(rename_all = "kebab-case")] +pub enum FakeCandidateValidationError { + /// Validation outputs check doesn't pass. + InvalidOutputs, + /// Failed to execute.`validate_block`. This includes function panicking. + ExecutionError, + /// Execution timeout. + Timeout, + /// Validation input is over the limit. + ParamsTooLarge, + /// Code size is over the limit. + CodeTooLarge, + /// Code does not decompress correctly. + CodeDecompressionFailure, + /// PoV does not decompress correctly. + POVDecompressionFailure, + /// Validation function returned invalid data. + BadReturn, + /// Invalid relay chain parent. + BadParent, + /// POV hash does not match. + POVHashMismatch, + /// Bad collator signature. + BadSignature, + /// Para head hash does not match. + ParaHeadHashMismatch, + /// Validation code hash does not match. + CodeHashMismatch, +} + +impl Into for FakeCandidateValidationError { + fn into(self) -> InvalidCandidate { + match self { + FakeCandidateValidationError::ExecutionError => + InvalidCandidate::ExecutionError("Malus".into()), + FakeCandidateValidationError::InvalidOutputs => InvalidCandidate::InvalidOutputs, + FakeCandidateValidationError::Timeout => InvalidCandidate::Timeout, + FakeCandidateValidationError::ParamsTooLarge => InvalidCandidate::ParamsTooLarge(666), + FakeCandidateValidationError::CodeTooLarge => InvalidCandidate::CodeTooLarge(666), + FakeCandidateValidationError::CodeDecompressionFailure => + InvalidCandidate::CodeDecompressionFailure, + FakeCandidateValidationError::POVDecompressionFailure => + InvalidCandidate::PoVDecompressionFailure, + FakeCandidateValidationError::BadReturn => InvalidCandidate::BadReturn, + FakeCandidateValidationError::BadParent => InvalidCandidate::BadParent, + FakeCandidateValidationError::POVHashMismatch => InvalidCandidate::PoVHashMismatch, + FakeCandidateValidationError::BadSignature => InvalidCandidate::BadSignature, + FakeCandidateValidationError::ParaHeadHashMismatch => + InvalidCandidate::ParaHeadHashMismatch, + FakeCandidateValidationError::CodeHashMismatch => InvalidCandidate::CodeHashMismatch, + } + } +} + +#[derive(Clone, Debug)] +/// An interceptor which fakes validation result with a preconfigured result. +/// Replaces `CandidateValidationSubsystem`. +pub struct ReplaceValidationResult { + fake_validation: FakeCandidateValidation, + fake_validation_error: FakeCandidateValidationError, + spawner: Spawner, +} + +impl ReplaceValidationResult +where + Spawner: SpawnNamed, +{ + pub fn new( + fake_validation: FakeCandidateValidation, + fake_validation_error: FakeCandidateValidationError, + spawner: Spawner, + ) -> Self { + 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 + /// given candidate. + pub fn send_validation_response( + &self, + candidate_descriptor: CandidateDescriptor, + subsystem_sender: Sender, + response_sender: oneshot::Sender>, + ) where + Sender: overseer::SubsystemSender + + overseer::SubsystemSender + + 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); + } +} + +pub fn create_fake_candidate_commitments( + persisted_validation_data: &PersistedValidationData, +) -> CandidateCommitments { + CandidateCommitments { + upward_messages: Vec::new(), + horizontal_messages: Vec::new(), + new_validation_code: None, + head_data: persisted_validation_data.parent_head.clone(), + processed_downward_messages: 0, + hrmp_watermark: persisted_validation_data.relay_parent_number, + } +} + +// Create and send validation response. This function needs the persistent validation data. +fn create_validation_response( + persisted_validation_data: PersistedValidationData, + descriptor: CandidateDescriptor, + response_sender: oneshot::Sender>, +) { + let commitments = create_fake_candidate_commitments(&persisted_validation_data); + + // Craft the new malicious candidate. + let candidate_receipt = CandidateReceipt { descriptor, commitments_hash: commitments.hash() }; + + let result = Ok(ValidationResult::Valid(commitments, persisted_validation_data)); + + gum::debug!( + target: MALUS, + para_id = ?candidate_receipt.descriptor.para_id, + candidate_hash = ?candidate_receipt.hash(), + "ValidationResult: {:?}", + &result + ); + + response_sender.send(result).unwrap(); +} + +impl MessageInterceptor for ReplaceValidationResult +where + Sender: overseer::SubsystemSender + + overseer::SubsystemSender + + Clone + + Send + + 'static, + Spawner: SpawnNamed + Clone + 'static, +{ + type Message = CandidateValidationMessage; + + // Capture all candidate validation requests and depending on configuration fail them. + fn intercept_incoming( + &self, + subsystem_sender: &mut Sender, + msg: FromOverseer, + ) -> Option> { + match msg { + FromOverseer::Communication { + msg: + CandidateValidationMessage::ValidateFromExhaustive( + validation_data, + validation_code, + candidate_receipt, + pov, + timeout, + sender, + ), + } => { + match self.fake_validation { + FakeCandidateValidation::ApprovalValid | + FakeCandidateValidation::BackingAndApprovalValid => { + // Behave normally if the `PoV` is not known to be malicious. + if pov.block_data.0.as_slice() != MALICIOUS_POV { + return Some(FromOverseer::Communication { + msg: CandidateValidationMessage::ValidateFromExhaustive( + validation_data, + validation_code, + candidate_receipt, + pov, + timeout, + sender, + ), + }) + } + create_validation_response( + validation_data, + candidate_receipt.descriptor, + sender, + ); + None + }, + FakeCandidateValidation::ApprovalInvalid | + FakeCandidateValidation::BackingAndApprovalInvalid => { + let validation_result = + ValidationResult::Invalid(InvalidCandidate::InvalidOutputs); + + gum::debug!( + target: MALUS, + para_id = ?candidate_receipt.descriptor.para_id, + "ValidateFromExhaustive result: {:?}", + &validation_result + ); + // We're not even checking the candidate, this makes us appear faster than honest validators. + sender.send(Ok(validation_result)).unwrap(); + None + }, + _ => Some(FromOverseer::Communication { + msg: CandidateValidationMessage::ValidateFromExhaustive( + validation_data, + validation_code, + candidate_receipt, + pov, + timeout, + sender, + ), + }), + } + }, + FromOverseer::Communication { + msg: + CandidateValidationMessage::ValidateFromChainState( + candidate_receipt, + pov, + timeout, + response_sender, + ), + } => { + match self.fake_validation { + FakeCandidateValidation::BackingValid | + FakeCandidateValidation::BackingAndApprovalValid => { + // Behave normally if the `PoV` is not known to be malicious. + if pov.block_data.0.as_slice() != MALICIOUS_POV { + return Some(FromOverseer::Communication { + msg: CandidateValidationMessage::ValidateFromChainState( + candidate_receipt, + pov, + timeout, + response_sender, + ), + }) + } + self.send_validation_response( + candidate_receipt.descriptor, + subsystem_sender.clone(), + response_sender, + ); + None + }, + FakeCandidateValidation::BackingInvalid | + FakeCandidateValidation::BackingAndApprovalInvalid => { + let validation_result = + ValidationResult::Invalid(self.fake_validation_error.clone().into()); + gum::debug!( + target: MALUS, + para_id = ?candidate_receipt.descriptor.para_id, + "ValidateFromChainState 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 + }, + _ => Some(FromOverseer::Communication { + msg: CandidateValidationMessage::ValidateFromChainState( + candidate_receipt, + pov, + timeout, + response_sender, + ), + }), + } + }, + msg => Some(msg), + } + } + + fn intercept_outgoing(&self, msg: AllMessages) -> Option { + Some(msg) + } +} diff --git a/node/malus/src/variants/dispute_valid_candidates.rs b/node/malus/src/variants/dispute_valid_candidates.rs index aa5626c44bef..63a99e7df441 100644 --- a/node/malus/src/variants/dispute_valid_candidates.rs +++ b/node/malus/src/variants/dispute_valid_candidates.rs @@ -15,7 +15,8 @@ // along with Polkadot. If not, see . //! A malicious node that replaces approvals with invalid disputes -//! against valid candidates. +//! against valid candidates. Additionally, the malus node can be configured to +//! fake candidate validation and return a static result for candidate checking. //! //! Attention: For usage with `zombienet` only! @@ -28,71 +29,45 @@ use polkadot_cli::{ OverseerConnector, OverseerGen, OverseerGenArgs, OverseerHandle, ParachainHost, ProvideRuntimeApi, SpawnNamed, }, + RunCmd, }; // Filter wrapping related types. -use crate::interceptor::*; - -// Import extra types relevant to the particular -// subsystem. -use polkadot_node_core_backing::CandidateBackingSubsystem; -use polkadot_node_subsystem::messages::{ - ApprovalDistributionMessage, CandidateBackingMessage, DisputeCoordinatorMessage, -}; -use sp_keystore::SyncCryptoStorePtr; +use super::common::{FakeCandidateValidation, FakeCandidateValidationError}; +use crate::{interceptor::*, variants::ReplaceValidationResult}; use std::sync::Arc; -/// Replace outgoing approval messages with disputes. -#[derive(Clone, Debug)] -struct ReplaceApprovalsWithDisputes; - -impl MessageInterceptor for ReplaceApprovalsWithDisputes -where - Sender: overseer::SubsystemSender + Clone + Send + 'static, -{ - type Message = CandidateBackingMessage; +#[derive(Clone, Debug, clap::Parser)] +#[clap(rename_all = "kebab-case")] +#[allow(missing_docs)] +pub struct DisputeAncestorOptions { + /// Malicious candidate validation subsystem configuration. When enabled, node PVF execution is skipped + /// during backing and/or approval and it's result can by specified by this option and `--fake-validation-error` + /// for invalid candidate outcomes. + #[clap(long, arg_enum, ignore_case = true, default_value_t = FakeCandidateValidation::BackingAndApprovalInvalid)] + pub fake_validation: FakeCandidateValidation, + + /// Applies only when `--fake-validation` is configured to reject candidates as invalid. It allows + /// to specify the exact error to return from the malicious candidate validation subsystem. + #[clap(long, arg_enum, ignore_case = true, default_value_t = FakeCandidateValidationError::InvalidOutputs)] + pub fake_validation_error: FakeCandidateValidationError, + + #[clap(flatten)] + pub cmd: RunCmd, +} - fn intercept_incoming( - &self, - _sender: &mut Sender, - msg: FromOverseer, - ) -> Option> { - Some(msg) - } +pub(crate) struct DisputeValidCandidates { + /// Fake validation config (applies to disputes as well). + opts: DisputeAncestorOptions, +} - fn intercept_outgoing(&self, msg: AllMessages) -> Option { - match msg { - AllMessages::ApprovalDistribution(ApprovalDistributionMessage::DistributeApproval( - _, - )) => { - // drop the message on the floor - None - }, - AllMessages::DisputeCoordinator(DisputeCoordinatorMessage::ImportStatements { - candidate_hash, - candidate_receipt, - session, - .. - }) => { - // this would also dispute candidates we were not assigned to approve - Some(AllMessages::DisputeCoordinator( - DisputeCoordinatorMessage::IssueLocalStatement( - session, - candidate_hash, - candidate_receipt, - false, - ), - )) - }, - msg => Some(msg), - } +impl DisputeValidCandidates { + pub fn new(opts: DisputeAncestorOptions) -> Self { + Self { opts } } } -/// Generates an overseer that disputes instead of approving valid candidates. -pub(crate) struct DisputeValidCandidates; - impl OverseerGen for DisputeValidCandidates { fn generate<'a, Spawner, RuntimeClient>( &self, @@ -105,15 +80,15 @@ impl OverseerGen for DisputeValidCandidates { Spawner: 'static + SpawnNamed + Clone + Unpin, { let spawner = args.spawner.clone(); - let crypto_store_ptr = args.keystore.clone() as SyncCryptoStorePtr; - let filter = ReplaceApprovalsWithDisputes; + let validation_filter = ReplaceValidationResult::new( + self.opts.fake_validation, + self.opts.fake_validation_error, + spawner.clone(), + ); prepared_overseer_builder(args)? - .replace_candidate_backing(move |cb| { - InterceptedSubsystem::new( - CandidateBackingSubsystem::new(spawner, crypto_store_ptr, cb.params.metrics), - filter, - ) + .replace_candidate_validation(move |cv_subsystem| { + InterceptedSubsystem::new(cv_subsystem, validation_filter) }) .build_with_connector(connector) .map_err(|e| e.into()) diff --git a/node/malus/src/variants/mod.rs b/node/malus/src/variants/mod.rs index aab3203f5bf3..d57580fdf8d3 100644 --- a/node/malus/src/variants/mod.rs +++ b/node/malus/src/variants/mod.rs @@ -17,10 +17,13 @@ //! Collection of behavior variants. mod back_garbage_candidate; +mod common; mod dispute_valid_candidates; mod suggest_garbage_candidate; pub(crate) use self::{ - back_garbage_candidate::BackGarbageCandidate, dispute_valid_candidates::DisputeValidCandidates, - suggest_garbage_candidate::SuggestGarbageCandidate, + back_garbage_candidate::BackGarbageCandidate, + dispute_valid_candidates::{DisputeAncestorOptions, DisputeValidCandidates}, + suggest_garbage_candidate::BackGarbageCandidateWrapper, }; +pub(crate) use common::*; diff --git a/node/malus/src/variants/suggest_garbage_candidate.rs b/node/malus/src/variants/suggest_garbage_candidate.rs index 64e8629edc50..af8191fe10a5 100644 --- a/node/malus/src/variants/suggest_garbage_candidate.rs +++ b/node/malus/src/variants/suggest_garbage_candidate.rs @@ -14,11 +14,11 @@ // You should have received a copy of the GNU General Public License // along with Polkadot. If not, see . -//! A malicious overseer proposing a garbage block. +//! A malicious node that stores bogus availability chunks, preventing others from +//! doing approval voting. This should lead to disputes depending if the validator +//! has fetched a malicious chunk. //! -//! Supposed to be used with regular nodes or in conjunction -//! with [`malus-back-garbage-candidate.rs`](./malus-back-garbage-candidate.rs) -//! to simulate a coordinated attack. +//! Attention: For usage with `zombienet` only! #![allow(missing_docs)] @@ -30,73 +30,220 @@ use polkadot_cli::{ ProvideRuntimeApi, SpawnNamed, }, }; +use polkadot_node_core_candidate_validation::find_validation_data; +use polkadot_node_primitives::{AvailableData, BlockData, PoV}; +use polkadot_primitives::v2::{CandidateDescriptor, CandidateHash}; + +use polkadot_node_subsystem_util::request_validators; -// Import extra types relevant to the particular -// subsystem. -use polkadot_node_core_backing::CandidateBackingSubsystem; -use polkadot_node_primitives::Statement; -use polkadot_node_subsystem::{ - messages::{CandidateBackingMessage, StatementDistributionMessage}, - overseer::{self, SubsystemSender}, -}; -use polkadot_node_subsystem_util as util; // Filter wrapping related types. -use crate::interceptor::*; -use polkadot_primitives::v2::{ - CandidateCommitments, CandidateReceipt, CommittedCandidateReceipt, CompactStatement, Hash, - Signed, +use crate::{ + interceptor::*, + shared::{MALICIOUS_POV, MALUS}, + variants::{ + create_fake_candidate_commitments, FakeCandidateValidation, FakeCandidateValidationError, + ReplaceValidationResult, + }, }; -use sp_keystore::SyncCryptoStorePtr; -use util::metered; -use std::sync::Arc; +// Import extra types relevant to the particular +// subsystem. +use polkadot_node_subsystem::messages::{CandidateBackingMessage, CollatorProtocolMessage}; +use polkadot_primitives::v2::CandidateReceipt; -use crate::shared::*; +use std::{ + collections::HashMap, + sync::{Arc, Mutex}, +}; -/// Replaces the seconded PoV data -/// of outgoing messages by some garbage data. +struct Inner { + /// Maps malicious candidate hash to original candidate hash. + /// It is used to replace outgoing collator protocol seconded messages. + map: HashMap, +} + +/// Replace outgoing approval messages with disputes. #[derive(Clone)] -struct ReplacePoVBytes -where - Sender: Send, -{ - queue: metered::UnboundedMeteredSender<(Sender, Hash, CandidateReceipt)>, +struct NoteCandidate { + inner: Arc>, + spawner: Spawner, } -impl MessageInterceptor for ReplacePoVBytes +impl MessageInterceptor for NoteCandidate where - Sender: overseer::SubsystemSender + Clone + Send + 'static, + Sender: overseer::SubsystemSender + + overseer::SubsystemSender + + Clone + + Send + + 'static, + Spawner: SpawnNamed + Clone + 'static, { type Message = CandidateBackingMessage; + /// Intercept incoming `Second` requests from the `collator-protocol` subsystem. We take fn intercept_incoming( &self, - sender: &mut Sender, + subsystem_sender: &mut Sender, msg: FromOverseer, ) -> Option> { match msg { FromOverseer::Communication { - msg: CandidateBackingMessage::Second(hash, candidate_receipt, _pov), + msg: CandidateBackingMessage::Second(relay_parent, candidate, _pov), } => { - self.queue - .unbounded_send((sender.clone(), hash, candidate_receipt.clone())) - .unwrap(); + gum::debug!( + target: MALUS, + candidate_hash = ?candidate.hash(), + ?relay_parent, + "Received request to second candidate" + ); + + let pov = PoV { block_data: BlockData(MALICIOUS_POV.into()) }; + + let (sender, receiver) = std::sync::mpsc::channel(); + let mut new_sender = subsystem_sender.clone(); + let _candidate = candidate.clone(); + self.spawner.spawn_blocking( + "malus-get-validation-data", + Some("malus"), + Box::pin(async move { + gum::trace!(target: MALUS, "Requesting validators"); + let n_validators = request_validators(relay_parent, &mut new_sender) + .await + .await + .unwrap() + .unwrap() + .len(); + gum::trace!(target: MALUS, "Validators {}", n_validators); + match find_validation_data(&mut new_sender, &_candidate.descriptor()).await + { + Ok(Some((validation_data, validation_code))) => { + sender + .send((validation_data, validation_code, n_validators)) + .expect("channel is still open"); + }, + _ => { + panic!("Unable to fetch validation data"); + }, + } + }), + ); + + let (validation_data, validation_code, n_validators) = receiver.recv().unwrap(); + + let validation_data_hash = validation_data.hash(); + let validation_code_hash = validation_code.hash(); + let validation_data_relay_parent_number = validation_data.relay_parent_number; + + gum::trace!( + target: MALUS, + candidate_hash = ?candidate.hash(), + ?relay_parent, + ?n_validators, + ?validation_data_hash, + ?validation_code_hash, + ?validation_data_relay_parent_number, + "Fetched validation data." + ); + + let malicious_available_data = + AvailableData { pov: Arc::new(pov.clone()), validation_data }; + + let pov_hash = pov.hash(); + let erasure_root = { + let chunks = + erasure::obtain_chunks_v1(n_validators as usize, &malicious_available_data) + .unwrap(); - None + let branches = erasure::branches(chunks.as_ref()); + branches.root() + }; + + let (collator_id, collator_signature) = { + use polkadot_primitives::v2::CollatorPair; + use sp_core::crypto::Pair; + + let collator_pair = CollatorPair::generate().0; + let signature_payload = polkadot_primitives::v2::collator_signature_payload( + &relay_parent, + &candidate.descriptor().para_id, + &validation_data_hash, + &pov_hash, + &validation_code_hash, + ); + + (collator_pair.public(), collator_pair.sign(&signature_payload)) + }; + + let malicious_commitments = + create_fake_candidate_commitments(&malicious_available_data.validation_data); + + let malicious_candidate = CandidateReceipt { + descriptor: CandidateDescriptor { + para_id: candidate.descriptor().para_id, + relay_parent, + collator: collator_id, + persisted_validation_data_hash: validation_data_hash, + pov_hash, + erasure_root, + signature: collator_signature, + para_head: malicious_commitments.head_data.hash(), + validation_code_hash, + }, + commitments_hash: malicious_commitments.hash(), + }; + let malicious_candidate_hash = malicious_candidate.hash(); + + gum::debug!( + target: MALUS, + candidate_hash = ?candidate.hash(), + ?malicious_candidate_hash, + "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), + }; + + Some(message) }, - other => Some(other), + FromOverseer::Communication { msg } => Some(FromOverseer::Communication { msg }), + FromOverseer::Signal(signal) => Some(FromOverseer::Signal(signal)), } } fn intercept_outgoing(&self, msg: AllMessages) -> Option { + let msg = match msg { + AllMessages::CollatorProtocol(CollatorProtocolMessage::Seconded( + relay_parent, + statement, + )) => { + // `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, + )) + }, + msg => msg, + }; Some(msg) } } -/// Generates an overseer that exposes bad behavior. -pub(crate) struct SuggestGarbageCandidate; +/// Garbage candidate implementation wrapper which implements `OverseerGen` glue. +pub(crate) struct BackGarbageCandidateWrapper; -impl OverseerGen for SuggestGarbageCandidate { +impl OverseerGen for BackGarbageCandidateWrapper { fn generate<'a, Spawner, RuntimeClient>( &self, connector: OverseerConnector, @@ -107,65 +254,23 @@ impl OverseerGen for SuggestGarbageCandidate { RuntimeClient::Api: ParachainHost + BabeApi + AuthorityDiscoveryApi, Spawner: 'static + SpawnNamed + Clone + Unpin, { - let spawner = args.spawner.clone(); - let (sink, source) = metered::unbounded(); - let keystore = args.keystore.clone() as SyncCryptoStorePtr; + let inner = Inner { map: std::collections::HashMap::new() }; + let inner_mut = Arc::new(Mutex::new(inner)); + let note_candidate = + NoteCandidate { inner: inner_mut.clone(), spawner: args.spawner.clone() }; - let filter = ReplacePoVBytes { queue: sink }; - - let keystore2 = keystore.clone(); - let spawner2 = spawner.clone(); + let validation_filter = ReplaceValidationResult::new( + FakeCandidateValidation::BackingAndApprovalValid, + FakeCandidateValidationError::InvalidOutputs, + args.spawner.clone(), + ); - let result = prepared_overseer_builder(args)? - .replace_candidate_backing(move |cb| { - InterceptedSubsystem::new( - CandidateBackingSubsystem::new(spawner2, keystore2, cb.params.metrics), - filter, - ) + prepared_overseer_builder(args)? + .replace_candidate_backing(move |cb| InterceptedSubsystem::new(cb, note_candidate)) + .replace_candidate_validation(move |cb| { + InterceptedSubsystem::new(cb, validation_filter) }) .build_with_connector(connector) - .map_err(|e| e.into()); - - launch_processing_task( - &spawner, - source, - move |(mut subsystem_sender, hash, candidate_receipt): (_, Hash, CandidateReceipt)| { - let keystore = keystore.clone(); - async move { - gum::info!( - target: MALUS, - "Replacing seconded candidate pov with something else" - ); - - let committed_candidate_receipt = CommittedCandidateReceipt { - descriptor: candidate_receipt.descriptor.clone(), - commitments: CandidateCommitments::default(), - }; - - let statement = Statement::Seconded(committed_candidate_receipt); - - if let Ok(validator) = - util::Validator::new(hash, keystore.clone(), &mut subsystem_sender).await - { - let signed_statement: Signed = validator - .sign(keystore, statement) - .await - .expect("Signing works. qed") - .expect("Something must come out of this. qed"); - - subsystem_sender - .send_message(StatementDistributionMessage::Share( - hash, - signed_statement, - )) - .await; - } else { - gum::info!("We are not a validator. Not siging anything."); - } - } - }, - ); - - result + .map_err(|e| e.into()) } } diff --git a/node/overseer/examples/minimal-example.rs b/node/overseer/examples/minimal-example.rs index 20141fad8555..fc07672ef4a1 100644 --- a/node/overseer/examples/minimal-example.rs +++ b/node/overseer/examples/minimal-example.rs @@ -33,7 +33,7 @@ use polkadot_overseer::{ gen::{FromOverseer, SpawnedSubsystem}, AllMessages, HeadSupportsParachains, OverseerSignal, SubsystemError, }; -use polkadot_primitives::v2::Hash; +use polkadot_primitives::v2::{CandidateReceipt, Hash}; struct AlwaysSupportsParachains; impl HeadSupportsParachains for AlwaysSupportsParachains { @@ -73,8 +73,13 @@ impl Subsystem1 { Delay::new(Duration::from_secs(1)).await; let (tx, _) = oneshot::channel(); + let candidate_receipt = CandidateReceipt { + descriptor: dummy_candidate_descriptor(dummy_hash()), + commitments_hash: Hash::zero(), + }; + let msg = CandidateValidationMessage::ValidateFromChainState( - dummy_candidate_descriptor(dummy_hash()), + candidate_receipt, PoV { block_data: BlockData(Vec::new()) }.into(), Default::default(), tx, diff --git a/node/overseer/overseer-gen/tests/ui/err-01-duplicate-consumer.stderr b/node/overseer/overseer-gen/tests/ui/err-01-duplicate-consumer.stderr index 18ba674324b5..ea67ef7aad5b 100644 --- a/node/overseer/overseer-gen/tests/ui/err-01-duplicate-consumer.stderr +++ b/node/overseer/overseer-gen/tests/ui/err-01-duplicate-consumer.stderr @@ -1,21 +1,21 @@ -error[E0119]: conflicting implementations of trait `std::convert::From` for type `AllMessages` +error[E0119]: conflicting implementations of trait `polkadot_overseer_gen::SubsystemSender` for type `OverseerSubsystemSender` --> tests/ui/err-01-duplicate-consumer.rs:19:1 | 19 | #[overlord(signal=SigSigSig, event=Event, gen=AllMessages, error=OverseerError)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | | | first implementation here - | conflicting implementation for `AllMessages` + | conflicting implementation for `OverseerSubsystemSender` | = note: this error originates in the attribute macro `overlord` (in Nightly builds, run with -Z macro-backtrace for more info) -error[E0119]: conflicting implementations of trait `polkadot_overseer_gen::SubsystemSender` for type `OverseerSubsystemSender` +error[E0119]: conflicting implementations of trait `std::convert::From` for type `AllMessages` --> tests/ui/err-01-duplicate-consumer.rs:19:1 | 19 | #[overlord(signal=SigSigSig, event=Event, gen=AllMessages, error=OverseerError)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | | | first implementation here - | conflicting implementation for `OverseerSubsystemSender` + | conflicting implementation for `AllMessages` | = note: this error originates in the attribute macro `overlord` (in Nightly builds, run with -Z macro-backtrace for more info) diff --git a/node/overseer/src/tests.rs b/node/overseer/src/tests.rs index cc4bd38f05b0..2e3dca1cf110 100644 --- a/node/overseer/src/tests.rs +++ b/node/overseer/src/tests.rs @@ -29,8 +29,8 @@ use polkadot_node_subsystem_types::{ ActivatedLeaf, LeafStatus, }; use polkadot_primitives::v2::{ - CandidateHash, CollatorPair, InvalidDisputeStatementKind, ValidDisputeStatementKind, - ValidatorIndex, + CandidateHash, CandidateReceipt, CollatorPair, InvalidDisputeStatementKind, + ValidDisputeStatementKind, ValidatorIndex, }; use crate::{ @@ -108,9 +108,14 @@ where let mut c: usize = 0; loop { if c < 10 { + let candidate_receipt = CandidateReceipt { + descriptor: dummy_candidate_descriptor(dummy_hash()), + commitments_hash: Hash::zero(), + }; + let (tx, _) = oneshot::channel(); ctx.send_message(CandidateValidationMessage::ValidateFromChainState( - dummy_candidate_descriptor(dummy_hash()), + candidate_receipt, PoV { block_data: BlockData(Vec::new()) }.into(), Default::default(), tx, @@ -792,8 +797,13 @@ where fn test_candidate_validation_msg() -> CandidateValidationMessage { let (sender, _) = oneshot::channel(); let pov = Arc::new(PoV { block_data: BlockData(Vec::new()) }); + let candidate_receipt = CandidateReceipt { + descriptor: dummy_candidate_descriptor(dummy_hash()), + commitments_hash: Hash::zero(), + }; + CandidateValidationMessage::ValidateFromChainState( - dummy_candidate_descriptor(dummy_hash()), + candidate_receipt, pov, Duration::default(), sender, diff --git a/node/primitives/src/lib.rs b/node/primitives/src/lib.rs index 021d8e737432..6e28b1d34d9f 100644 --- a/node/primitives/src/lib.rs +++ b/node/primitives/src/lib.rs @@ -236,6 +236,8 @@ pub enum InvalidCandidate { ParaHeadHashMismatch, /// Validation code hash does not match. CodeHashMismatch, + /// Validation has generated different candidate commitments. + CommitmentsHashMismatch, } /// Result of the validation of the candidate. diff --git a/node/subsystem-types/src/messages.rs b/node/subsystem-types/src/messages.rs index 8657ec16283b..69b2baf0af12 100644 --- a/node/subsystem-types/src/messages.rs +++ b/node/subsystem-types/src/messages.rs @@ -39,13 +39,13 @@ use polkadot_node_primitives::{ SignedFullStatement, ValidationResult, }; use polkadot_primitives::v2::{ - AuthorityDiscoveryId, BackedCandidate, BlockNumber, CandidateDescriptor, CandidateEvent, - CandidateHash, CandidateIndex, CandidateReceipt, CollatorId, CommittedCandidateReceipt, - CoreState, GroupIndex, GroupRotationInfo, Hash, Header as BlockHeader, Id as ParaId, - InboundDownwardMessage, InboundHrmpMessage, MultiDisputeStatementSet, OccupiedCoreAssumption, - PersistedValidationData, PvfCheckStatement, SessionIndex, SessionInfo, - SignedAvailabilityBitfield, SignedAvailabilityBitfields, ValidationCode, ValidationCodeHash, - ValidatorId, ValidatorIndex, ValidatorSignature, + AuthorityDiscoveryId, BackedCandidate, BlockNumber, CandidateEvent, CandidateHash, + CandidateIndex, CandidateReceipt, CollatorId, CommittedCandidateReceipt, CoreState, GroupIndex, + GroupRotationInfo, Hash, Header as BlockHeader, Id as ParaId, InboundDownwardMessage, + InboundHrmpMessage, MultiDisputeStatementSet, OccupiedCoreAssumption, PersistedValidationData, + PvfCheckStatement, SessionIndex, SessionInfo, SignedAvailabilityBitfield, + SignedAvailabilityBitfields, ValidationCode, ValidationCodeHash, ValidatorId, ValidatorIndex, + ValidatorSignature, }; use polkadot_statement_table::v2::Misbehavior; use std::{ @@ -126,14 +126,14 @@ pub enum CandidateValidationMessage { /// /// 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`. + /// 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( - CandidateDescriptor, + CandidateReceipt, Arc, /// Execution timeout Duration, @@ -151,7 +151,7 @@ pub enum CandidateValidationMessage { ValidateFromExhaustive( PersistedValidationData, ValidationCode, - CandidateDescriptor, + CandidateReceipt, Arc, /// Execution timeout Duration, diff --git a/zombienet_tests/functional/0002-parachains-disputes.toml b/zombienet_tests/functional/0002-parachains-disputes.toml index a92ad71ca958..dc909726bdae 100644 --- a/zombienet_tests/functional/0002-parachains-disputes.toml +++ b/zombienet_tests/functional/0002-parachains-disputes.toml @@ -2,8 +2,8 @@ timeout = 1000 [relaychain.genesis.runtime.runtime_genesis_config.configuration.config] - max_validators_per_core = 2 - needed_approvals = 2 + max_validators_per_core = 5 + needed_approvals = 8 [relaychain] default_image = "{{ZOMBIENET_INTEGRATION_TEST_IMAGE}}" @@ -18,20 +18,20 @@ requests = { memory = "2G", cpu = "1" } [[relaychain.nodes]] image = "{{MALUS_IMAGE}}" name = "alice" - command = "malus dispute-ancestor" - args = [ "--alice", "-lparachain=debug" ] + command = "malus dispute-ancestor --fake-validation approval-invalid" + args = [ "--alice", " -lparachain=debug,MALUS=trace" ] [[relaychain.nodes]] image = "{{MALUS_IMAGE}}" name = "bob" - command = "malus dispute-ancestor" - args = [ "--bob", "-lparachain=debug"] - + command = "malus dispute-ancestor --fake-validation approval-invalid" + args = [ "--bob", "-lparachain=debug,MALUS=trace"] + [[relaychain.nodes]] image = "{{MALUS_IMAGE}}" name = "charlie" - command = "malus dispute-ancestor" - args = [ "--charlie", "-lparachain=debug" ] + command = "malus dispute-ancestor --fake-validation approval-invalid" + args = [ "--charlie", "-lparachain=debug,MALUS=trace" ] [[relaychain.nodes]] name = "dave" @@ -53,51 +53,21 @@ requests = { memory = "2G", cpu = "1" } name = "two" args = [ "--two", "-lparachain=debug"] +{% for id in range(2000,2004) %} [[parachains]] -id = 2000 +id = {{id}} addToGenesis = true -genesis_state_generator = "undying-collator export-genesis-state --pov-size=100000 --pvf-complexity=1" +genesis_state_generator = "undying-collator export-genesis-state --pov-size={{25000*(id-1999)}} --pvf-complexity={{id - 1999}}" [parachains.collator] image = "{{COL_IMAGE}}" - name = "collator01" + name = "collator" command = "undying-collator" - args = ["-lparachain=debug", "--pov-size=100000", "--pvf-complexity=1", "--parachain-id=2000"] - -[[parachains]] -id = 2001 -addToGenesis = true -genesis_state_generator = "undying-collator export-genesis-state --pov-size=100000 --pvf-complexity=2" - - [parachains.collator] - image = "{{COL_IMAGE}}" - name = "collator02" - command = "undying-collator" - args = ["-lparachain=debug", "--pov-size=100000", "--parachain-id=2001", "--pvf-complexity=2"] - -[[parachains]] -id = 2002 -addToGenesis = true -genesis_state_generator = "undying-collator export-genesis-state --pov-size=100000 --pvf-complexity=10" + args = ["-lparachain=debug", "--pov-size={{25000*(id-1999)}}", "--parachain-id={{id}}", "--pvf-complexity={{id - 1999}}"] - [parachains.collator] - image = "{{COL_IMAGE}}" - name = "collator03" - command = "undying-collator" - args = ["-lparachain=debug", "--pov-size=100000", "--parachain-id=2002", "--pvf-complexity=10"] - -[[parachains]] -id = 2003 -addToGenesis = true -genesis_state_generator = "undying-collator export-genesis-state --pov-size=20000 --pvf-complexity=1000" - - [parachains.collator] - image = "{{COL_IMAGE}}" - name = "collator04" - command = "undying-collator" - args = ["-lparachain=debug", "--pov-size=20000", "--parachain-id=2003", "--pvf-complexity=1000"] +{% endfor %} [types.Header] number = "u64" parent_hash = "Hash" -post_state = "Hash" \ No newline at end of file +post_state = "Hash" diff --git a/zombienet_tests/functional/0003-parachains-bft-disputes.feature b/zombienet_tests/functional/0003-parachains-bft-disputes.feature new file mode 100644 index 000000000000..f3ce05ad7958 --- /dev/null +++ b/zombienet_tests/functional/0003-parachains-bft-disputes.feature @@ -0,0 +1,54 @@ +Description: Test dispute finality lag at BFT threshold. Malus nodes are configured to back/approve a garbage candidate. +Network: ./0003-parachains-bft-disputes.toml +Creds: config + +honest-validator-0: is up +honest-validator-1: is up +honest-validator-2: is up +honest-validator-3: is up +honest-validator-4: is up +honest-validator-5: is up +honest-validator-6: is up +honest-validator-7: is up +honest-validator-8: is up +honest-validator-9: is up +malus-validator-0: is up +malus-validator-1: is up +malus-validator-2: is up + +# Check authority status. +honest-validator-0: reports node_roles is 4 +honest-validator-1: reports node_roles is 4 +honest-validator-2: reports node_roles is 4 +honest-validator-3: reports node_roles is 4 +honest-validator-4: reports node_roles is 4 +honest-validator-5: reports node_roles is 4 +honest-validator-6: reports node_roles is 4 +honest-validator-7: reports node_roles is 4 +honest-validator-8: reports node_roles is 4 +honest-validator-9: reports node_roles is 4 +malus-validator-0: reports node_roles is 4 +malus-validator-1: reports node_roles is 4 +malus-validator-2: reports node_roles is 4 + + +honest-validator-0: parachain 2000 block height is at least 4 within 180 seconds +honest-validator-1: parachain 2001 block height is at least 4 within 180 seconds +honest-validator-2: parachain 2002 block height is at least 4 within 180 seconds +honest-validator-3: parachain 2003 block height is at least 4 within 180 seconds + +honest-validator-3: log line contains "reverted due to a bad parachain block" within 180 seconds +sleep 60 seconds + +honest-validator-0: reports parachain_disputes_finality_lag is lower than 4 +honest-validator-1: reports parachain_disputes_finality_lag is lower than 4 +honest-validator-2: reports parachain_disputes_finality_lag is lower than 4 +honest-validator-3: reports parachain_disputes_finality_lag is lower than 4 +honest-validator-6: reports parachain_disputes_finality_lag is lower than 4 +honest-validator-7: reports parachain_disputes_finality_lag is lower than 4 +honest-validator-8: reports parachain_disputes_finality_lag is lower than 4 +honest-validator-9: reports parachain_disputes_finality_lag is lower than 4 + +honest-validator-0: reports parachain_candidate_disputes_total is at least 4 within 15 seconds +honest-validator-1: reports parachain_candidate_dispute_concluded{validity="invalid"} is at least 4 within 15 seconds +honest-validator-2: reports parachain_candidate_dispute_concluded{validity="valid"} is 0 within 15 seconds diff --git a/zombienet_tests/functional/0003-parachains-bft-disputes.toml b/zombienet_tests/functional/0003-parachains-bft-disputes.toml new file mode 100644 index 000000000000..3e1cc15e528e --- /dev/null +++ b/zombienet_tests/functional/0003-parachains-bft-disputes.toml @@ -0,0 +1,46 @@ +[settings] +timeout = 1000 +bootnode = true + +[relaychain.genesis.runtime.runtime_genesis_config.configuration.config] + max_validators_per_core = 3 + needed_approvals = 7 + +[relaychain] +default_image = "{{ZOMBIENET_INTEGRATION_TEST_IMAGE}}" +chain = "rococo-local" +chain_spec_command = "polkadot build-spec --chain rococo-local" +default_command = "polkadot" + +[relaychain.default_resources] +limits = { memory = "4G", cpu = "2" } +requests = { memory = "2G", cpu = "1" } + + [[relaychain.node_groups]] + name = "honest-validator" + count = 10 + args = ["-lparachain=debug,runtime=debug"] + + [[relaychain.node_groups]] + image = "{{MALUS_IMAGE}}" + name = "malus-validator" + command = "malus suggest-garbage-candidate" + args = ["-lparachain=debug,MALUS=trace"] + count = 3 + +{% for id in range(2000,2004) %} +[[parachains]] +id = {{id}} +addToGenesis = true +genesis_state_generator = "undying-collator export-genesis-state --pov-size={{10000*(id-1999)}} --pvf-complexity={{id - 1999}}" + [parachains.collator] + image = "{{COL_IMAGE}}" + name = "collator" + command = "undying-collator" + args = ["-lparachain=debug", "--pov-size={{10000*(id-1999)}}", "--parachain-id={{id}}", "--pvf-complexity={{id - 1999}}"] +{% endfor %} + +[types.Header] +number = "u64" +parent_hash = "Hash" +post_state = "Hash"