Malus: improvements in dispute ancestor and suggest garbage candidate implementation (#5011)

* Implement fake validation results Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * refactor Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * cargo lock Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * spell check Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * spellcheck Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * typos Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * Review feedback Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * move stuff around Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * chores Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * Impl valid - still wip Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * fixes Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * fmt Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * Pull Ladi's implementation: https://github.com/paritytech/polkadot/pull/4711 Co-authored-by: Lldenaurois <Ljdenaurois@gmail.com> Co-authored-by: Andrei Sandu <andrei-mihail@parity.io> Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * Fix build Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * Logs and comments Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * WIP: suggest garbage candidate + implement validation result caching Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * fix Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * Do commitment hash checks in candidate validation Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * Minor refactor in approval, backing, dispute-coord Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * Working version of suggest garbage candidate Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * Dedup Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * cleanup #1 Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * Fix tests Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * remove debug leftovers Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * fmt Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * Accidentally commited some local test Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * spellcheck Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * some more fixes Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * Refactor and fix it Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * review feedback Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * typo Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * tests review feedback Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * refactor disputer Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * fix tests Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * Fix zombienet disputes test Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * spellcheck Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * fix Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * Fix ui tests Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * fix typo Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> Co-authored-by: Lldenaurois <Ljdenaurois@gmail.com>
2026-06-13 00:01:09 +00:00 · 2022-04-13 16:45:39 +03:00
parent a46237cebb
commit cddd5749d3
23 changed files with 921 additions and 529 deletions
@@ -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>(
+	sender: &mut Sender,
+	descriptor: &CandidateDescriptor,
+) -> Result<Option<(PersistedValidationData, ValidationCode)>, 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>(
 	sender: &mut Sender,
 	validation_host: ValidationHost,
-	descriptor: CandidateDescriptor,
+	candidate_receipt: CandidateReceipt,
 	pov: Arc<PoV>,
 	timeout: Duration,
 	metrics: &Metrics,
@@ -424,25 +446,18 @@ async fn validate_from_chain_state<Sender>(
 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<PoV>,
 	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))
+				}
 			},
 	}
 }
@@ -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(),