attempt to fix zombienet disputes (#7618)

* update metric name

* update some metric names

* avoid cycles when creating fake candidates

* make undying collator more friendly to malformed parents

* fix a bug in malus

node/malus/integrationtests/0001-dispute-valid-block.zndsl

@@ -16,14 +16,14 @@ 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
+alice: reports polkadot_parachain_candidate_disputes_total is at least 1 within 250 seconds
+bob: reports polkadot_parachain_candidate_disputes_total is at least 1 within 90 seconds
+charlie: reports polkadot_parachain_candidate_disputes_total is at least 1 within 90 seconds
+alice: reports polkadot_parachain_candidate_dispute_votes{validity="valid"} is at least 1 within 90 seconds
+bob: reports polkadot_parachain_candidate_dispute_votes{validity="valid"} is at least 2 within 90 seconds
+charlie: reports polkadot_parachain_candidate_dispute_votes{validity="valid"} is at least 2 within 90 seconds
+alice: reports polkadot_parachain_candidate_dispute_concluded{validity="valid"} is at least 1 within 90 seconds
+alice: reports polkadot_parachain_candidate_dispute_concluded{validity="invalid"} is 0 within 90 seconds
+bob: reports polkadot_parachain_candidate_dispute_concluded{validity="valid"} is at least 1 within 90 seconds
+charlie: reports polkadot_parachain_candidate_dispute_concluded{validity="valid"} is at least 1 within 90 seconds
+charlie: reports polkadot_parachain_candidate_dispute_concluded{validity="valid"} is at least 1 within 90 seconds

node/malus/src/variants/common.rs

@@ -30,6 +30,7 @@ use polkadot_node_subsystem::{
 
 use polkadot_primitives::{
 	CandidateCommitments, CandidateDescriptor, CandidateReceipt, PersistedValidationData,
+	PvfExecTimeoutKind,
 };
 
 use futures::channel::oneshot;
@@ -48,6 +49,55 @@ pub enum FakeCandidateValidation {
 	BackingAndApprovalValid,
 }
 
+impl FakeCandidateValidation {
+	fn misbehaves_valid(&self) -> bool {
+		use FakeCandidateValidation::*;
+
+		match *self {
+			BackingValid | ApprovalValid | BackingAndApprovalValid => true,
+			_ => false,
+		}
+	}
+
+	fn misbehaves_invalid(&self) -> bool {
+		use FakeCandidateValidation::*;
+
+		match *self {
+			BackingInvalid | ApprovalInvalid | BackingAndApprovalInvalid => true,
+			_ => false,
+		}
+	}
+
+	fn includes_backing(&self) -> bool {
+		use FakeCandidateValidation::*;
+
+		match *self {
+			BackingInvalid | BackingAndApprovalInvalid | BackingValid | BackingAndApprovalValid =>
+				true,
+			_ => false,
+		}
+	}
+
+	fn includes_approval(&self) -> bool {
+		use FakeCandidateValidation::*;
+
+		match *self {
+			ApprovalInvalid |
+			BackingAndApprovalInvalid |
+			ApprovalValid |
+			BackingAndApprovalValid => true,
+			_ => false,
+		}
+	}
+
+	fn should_misbehave(&self, timeout: PvfExecTimeoutKind) -> bool {
+		match timeout {
+			PvfExecTimeoutKind::Backing => self.includes_backing(),
+			PvfExecTimeoutKind::Approval => self.includes_approval(),
+		}
+	}
+}
+
 /// Candidate invalidity details
 #[derive(clap::ValueEnum, Clone, Copy, Debug, PartialEq)]
 #[value(rename_all = "kebab-case")]
@@ -162,11 +212,20 @@ where
 pub fn create_fake_candidate_commitments(
 	persisted_validation_data: &PersistedValidationData,
 ) -> CandidateCommitments {
+	// Backing rejects candidates which output the same head as the parent,
+	// therefore we must create a new head which is not equal to the parent.
+	let mut head_data = persisted_validation_data.parent_head.clone();
+	if head_data.0.is_empty() {
+		head_data.0.push(0);
+	} else {
+		head_data.0[0] = head_data.0[0].wrapping_add(1);
+	};
+
 	CandidateCommitments {
 		upward_messages: Default::default(),
 		horizontal_messages: Default::default(),
 		new_validation_code: None,
-		head_data: persisted_validation_data.parent_head.clone(),
+		head_data,
 		processed_downward_messages: 0,
 		hrmp_watermark: persisted_validation_data.relay_parent_number,
 	}
@@ -224,8 +283,7 @@ where
 					),
 			} => {
 				match self.fake_validation {
-					FakeCandidateValidation::ApprovalValid |
-					FakeCandidateValidation::BackingAndApprovalValid => {
+					x if x.misbehaves_valid() && x.should_misbehave(timeout) => {
 						// Behave normally if the `PoV` is not known to be malicious.
 						if pov.block_data.0.as_slice() != MALICIOUS_POV {
 							return Some(FromOrchestra::Communication {
@@ -278,8 +336,7 @@ where
 							},
 						}
 					},
-					FakeCandidateValidation::ApprovalInvalid |
-					FakeCandidateValidation::BackingAndApprovalInvalid => {
+					x if x.misbehaves_invalid() && x.should_misbehave(timeout) => {
 						// Set the validation result to invalid with probability `p` and trigger a
 						// dispute
 						let behave_maliciously = self.distribution.sample(&mut rand::thread_rng());
@@ -342,8 +399,7 @@ where
 					),
 			} => {
 				match self.fake_validation {
-					FakeCandidateValidation::BackingValid |
-					FakeCandidateValidation::BackingAndApprovalValid => {
+					x if x.misbehaves_valid() && x.should_misbehave(timeout) => {
 						// Behave normally if the `PoV` is not known to be malicious.
 						if pov.block_data.0.as_slice() != MALICIOUS_POV {
 							return Some(FromOrchestra::Communication {
@@ -385,8 +441,7 @@ where
 							}),
 						}
 					},
-					FakeCandidateValidation::BackingInvalid |
-					FakeCandidateValidation::BackingAndApprovalInvalid => {
+					x if x.misbehaves_invalid() && x.should_misbehave(timeout) => {
 						// Maliciously set the validation result to invalid for a valid candidate
 						// with probability `p`
 						let behave_maliciously = self.distribution.sample(&mut rand::thread_rng());

parachain/test-parachains/undying/collator/src/lib.rs

@@ -33,7 +33,9 @@ use std::{
 	},
 	time::Duration,
 };
-use test_parachain_undying::{execute, hash_state, BlockData, GraveyardState, HeadData};
+use test_parachain_undying::{
+	execute, hash_state, BlockData, GraveyardState, HeadData, StateMismatch,
+};
 
 /// Default PoV size which also drives state size.
 const DEFAULT_POV_SIZE: usize = 1000;
@@ -45,7 +47,7 @@ fn calculate_head_and_state_for_number(
 	number: u64,
 	graveyard_size: usize,
 	pvf_complexity: u32,
-) -> (HeadData, GraveyardState) {
+) -> Result<(HeadData, GraveyardState), StateMismatch> {
 	let index = 0u64;
 	let mut graveyard = vec![0u8; graveyard_size * graveyard_size];
 	let zombies = 0;
@@ -62,13 +64,12 @@ fn calculate_head_and_state_for_number(
 
 	while head.number < number {
 		let block = BlockData { state, tombstones: 1_000, iterations: pvf_complexity };
-		let (new_head, new_state) =
-			execute(head.hash(), head.clone(), block).expect("Produces valid block");
+		let (new_head, new_state) = execute(head.hash(), head.clone(), block)?;
 		head = new_head;
 		state = new_state;
 	}
 
-	(head, state)
+	Ok((head, state))
 }
 
 /// The state of the undying parachain.
@@ -122,39 +123,35 @@ impl State {
 	/// Advance the state and produce a new block based on the given `parent_head`.
 	///
 	/// Returns the new [`BlockData`] and the new [`HeadData`].
-	fn advance(&mut self, parent_head: HeadData) -> (BlockData, HeadData) {
+	fn advance(&mut self, parent_head: HeadData) -> Result<(BlockData, HeadData), StateMismatch> {
 		self.best_block = parent_head.number;
 
-		let state = if let Some(head_data) = self.number_to_head.get(&self.best_block) {
-			self.head_to_state.get(head_data).cloned().unwrap_or_else(|| {
-				calculate_head_and_state_for_number(
-					parent_head.number,
-					self.graveyard_size,
-					self.pvf_complexity,
-				)
-				.1
-			})
+		let state = if let Some(state) = self
+			.number_to_head
+			.get(&self.best_block)
+			.and_then(|head_data| self.head_to_state.get(head_data).cloned())
+		{
+			state
 		} else {
 			let (_, state) = calculate_head_and_state_for_number(
 				parent_head.number,
 				self.graveyard_size,
 				self.pvf_complexity,
-			);
+			)?;
 			state
 		};
 
 		// Start with prev state and transaction to execute (place 1000 tombstones).
 		let block = BlockData { state, tombstones: 1000, iterations: self.pvf_complexity };
 
-		let (new_head, new_state) =
-			execute(parent_head.hash(), parent_head, block.clone()).expect("Produces valid block");
+		let (new_head, new_state) = execute(parent_head.hash(), parent_head, block.clone())?;
 
 		let new_head_arc = Arc::new(new_head.clone());
 
 		self.head_to_state.insert(new_head_arc.clone(), new_state);
 		self.number_to_head.insert(new_head.number, new_head_arc);
 
-		(block, new_head)
+		Ok((block, new_head))
 	}
 }
 
@@ -233,10 +230,21 @@ impl Collator {
 		let seconded_collations = self.seconded_collations.clone();
 
 		Box::new(move |relay_parent, validation_data| {
-			let parent = HeadData::decode(&mut &validation_data.parent_head.0[..])
-				.expect("Decodes parent head");
+			let parent = match HeadData::decode(&mut &validation_data.parent_head.0[..]) {
+				Err(err) => {
+					log::error!("Requested to build on top of malformed head-data: {:?}", err);
+					return futures::future::ready(None).boxed()
+				},
+				Ok(p) => p,
+			};
 
-			let (block_data, head_data) = state.lock().unwrap().advance(parent);
+			let (block_data, head_data) = match state.lock().unwrap().advance(parent.clone()) {
+				Err(err) => {
+					log::error!("Unable to build on top of {:?}: {:?}", parent, err);
+					return futures::future::ready(None).boxed()
+				},
+				Ok(x) => x,
+			};
 
 			log::info!(
 				"created a new collation on relay-parent({}): {:?}",
@@ -280,7 +288,6 @@ impl Collator {
 								"Seconded statement should match our collation: {:?}",
 								res.statement.payload()
 							);
-							std::process::exit(-1);
 						}
 
 						seconded_collations.fetch_add(1, Ordering::Relaxed);
@@ -394,10 +401,10 @@ mod tests {
 		let collator = Collator::new(1_000, 1);
 		let graveyard_size = collator.state.lock().unwrap().graveyard_size;
 
-		let mut head = calculate_head_and_state_for_number(10, graveyard_size, 1).0;
+		let mut head = calculate_head_and_state_for_number(10, graveyard_size, 1).unwrap().0;
 
 		for i in 1..10 {
-			head = collator.state.lock().unwrap().advance(head).1;
+			head = collator.state.lock().unwrap().advance(head).unwrap().1;
 			assert_eq!(10 + i, head.number);
 		}
 
@@ -414,7 +421,7 @@ mod tests {
 			.clone();
 
 		for _ in 1..20 {
-			second_head = collator.state.lock().unwrap().advance(second_head.clone()).1;
+			second_head = collator.state.lock().unwrap().advance(second_head.clone()).unwrap().1;
 		}
 
 		assert_eq!(second_head, head);

zombienet_tests/functional/0002-parachains-disputes.zndsl

@@ -26,9 +26,9 @@ alice: parachain 2003 block height is at least 10 within 200 seconds
 
 # Check if disputes are initiated and concluded.
 # TODO: check if disputes are concluded faster than initiated.
-eve: reports parachain_candidate_disputes_total is at least 10 within 15 seconds
-eve: reports parachain_candidate_dispute_concluded{validity="valid"} is at least 10 within 15 seconds
-eve: reports parachain_candidate_dispute_concluded{validity="invalid"} is 0 within 15 seconds
+eve: reports polkadot_parachain_candidate_disputes_total is at least 10 within 15 seconds
+eve: reports polkadot_parachain_candidate_dispute_concluded{validity="valid"} is at least 10 within 15 seconds
+eve: reports polkadot_parachain_candidate_dispute_concluded{validity="invalid"} is 0 within 15 seconds
 
 # As of <https://github.com/paritytech/polkadot/pull/6249>, we don't slash on disputes
 # with `valid` outcome, so there is no offence reported.

zombienet_tests/functional/0003-parachains-garbage-candidate.zndsl

@@ -32,13 +32,13 @@ honest-validator-2: reports polkadot_parachain_disputes_finality_lag is lower th
 sleep 30 seconds
 
 # Check that garbage parachain blocks included by malicious validators are being disputed.
-honest-validator-0: reports parachain_candidate_disputes_total is at least 2 within 15 seconds
-honest-validator-1: reports parachain_candidate_disputes_total is at least 2 within 15 seconds
-honest-validator-2: reports parachain_candidate_disputes_total is at least 2 within 15 seconds
+honest-validator-0: reports polkadot_parachain_candidate_disputes_total is at least 2 within 15 seconds
+honest-validator-1: reports polkadot_parachain_candidate_disputes_total is at least 2 within 15 seconds
+honest-validator-2: reports polkadot_parachain_candidate_disputes_total is at least 2 within 15 seconds
 
 # Disputes should always end as "invalid"
-honest-validator-0: reports parachain_candidate_dispute_concluded{validity="invalid"} is at least 2 within 15 seconds
-honest-validator-1: reports parachain_candidate_dispute_concluded{validity="valid"} is 0 within 15 seconds
+honest-validator-0: reports polkadot_parachain_candidate_dispute_concluded{validity="invalid"} is at least 2 within 15 seconds
+honest-validator-1: reports polkadot_parachain_candidate_dispute_concluded{validity="valid"} is 0 within 15 seconds
 
 # Check participating in the losing side of a dispute logged
 malus-validator: log line contains "Voted for a candidate that was concluded invalid." within 180 seconds

zombienet_tests/functional/0004-parachains-disputes-past-session.zndsl

@@ -14,7 +14,7 @@ honest-validator-0: parachain 1000 is registered within 100 seconds
 honest-validator-0: parachain 1000 block height is at least 1 within 300 seconds
 
 # There should be disputes initiated
-honest-validator-0: reports parachain_candidate_disputes_total is at least 2 within 200 seconds
+honest-validator-0: reports polkadot_parachain_candidate_disputes_total is at least 2 within 200 seconds
 
 # Stop issuing disputes
 malus-validator-0: pause
@@ -29,9 +29,9 @@ honest-validator-0: reports block height minus finalised block is at least 10 wi
 alice: resume
 
 # Disputes should start concluding now
-honest-validator-0: reports parachain_candidate_dispute_concluded{validity="invalid"} is at least 1 within 200 seconds
+honest-validator-0: reports polkadot_parachain_candidate_dispute_concluded{validity="invalid"} is at least 1 within 200 seconds
 # Disputes should always end as "invalid"
-honest-validator-0: reports parachain_candidate_dispute_concluded{validity="valid"} is 0
+honest-validator-0: reports polkadot_parachain_candidate_dispute_concluded{validity="valid"} is 0
 
 # Check an unsigned extrinsic is submitted
 honest-validator: log line contains "Successfully reported pending slash" within 180 seconds

zombienet_tests/misc/0001-paritydb.zndsl

@@ -55,14 +55,14 @@ validator-8: reports polkadot_parachain_approval_checking_finality_lag is 0
 validator-9: reports polkadot_parachain_approval_checking_finality_lag is 0
 
 # Check lag - dispute conclusion
-validator-0: reports parachain_candidate_disputes_total is 0
-validator-1: reports parachain_candidate_disputes_total is 0
-validator-2: reports parachain_candidate_disputes_total is 0
-validator-3: reports parachain_candidate_disputes_total is 0
-validator-4: reports parachain_candidate_disputes_total is 0
-validator-5: reports parachain_candidate_disputes_total is 0
-validator-6: reports parachain_candidate_disputes_total is 0
-validator-7: reports parachain_candidate_disputes_total is 0
-validator-8: reports parachain_candidate_disputes_total is 0
-validator-9: reports parachain_candidate_disputes_total is 0
+validator-0: reports polkadot_parachain_candidate_disputes_total is 0
+validator-1: reports polkadot_parachain_candidate_disputes_total is 0
+validator-2: reports polkadot_parachain_candidate_disputes_total is 0
+validator-3: reports polkadot_parachain_candidate_disputes_total is 0
+validator-4: reports polkadot_parachain_candidate_disputes_total is 0
+validator-5: reports polkadot_parachain_candidate_disputes_total is 0
+validator-6: reports polkadot_parachain_candidate_disputes_total is 0
+validator-7: reports polkadot_parachain_candidate_disputes_total is 0
+validator-8: reports polkadot_parachain_candidate_disputes_total is 0
+validator-9: reports polkadot_parachain_candidate_disputes_total is 0