remove retry from backers on failed candidate validation (#2182)

Hey guys, as discussed I've changed the name to a more general one `PvfExecKind`, is this good or too general? Creating this as a draft, I still have to fix the tests. Closes #1585 Kusama address: FkB6QEo8VnV3oifugNj5NeVG3Mvq1zFbrUu4P5YwRoe5mQN --------- Co-authored-by: command-bot <> Co-authored-by: Marcin S <[email protected]>
paritytech · Nov 20, 2023 · ede4a36 · ede4a36
1 parent b35300c
commit ede4a36
Show file tree

Hide file tree

Showing 18 changed files with 271 additions and 243 deletions.
diff --git a/polkadot/node/core/approval-voting/src/lib.rs b/polkadot/node/core/approval-voting/src/lib.rs
@@ -54,7 +54,7 @@ use polkadot_node_subsystem_util::{
 };
 use polkadot_primitives::{
 	ApprovalVote, BlockNumber, CandidateHash, CandidateIndex, CandidateReceipt, DisputeStatement,
-	ExecutorParams, GroupIndex, Hash, PvfExecTimeoutKind, SessionIndex, SessionInfo,
+	ExecutorParams, GroupIndex, Hash, PvfExecKind, SessionIndex, SessionInfo,
 	ValidDisputeStatementKind, ValidatorId, ValidatorIndex, ValidatorPair, ValidatorSignature,
 };
 use sc_keystore::LocalKeystore;
@@ -2867,7 +2867,7 @@ async fn launch_approval<Context>(
 				candidate_receipt: candidate.clone(),
 				pov: available_data.pov,
 				executor_params,
-				exec_timeout_kind: PvfExecTimeoutKind::Approval,
+				exec_kind: PvfExecKind::Approval,
 				response_sender: val_tx,
 			})
 			.await;

diff --git a/polkadot/node/core/approval-voting/src/tests.rs b/polkadot/node/core/approval-voting/src/tests.rs
@@ -2705,10 +2705,10 @@ async fn handle_double_assignment_import(
 	assert_matches!(
 		overseer_recv(virtual_overseer).await,
 		AllMessages::CandidateValidation(CandidateValidationMessage::ValidateFromExhaustive {
-			exec_timeout_kind,
+			exec_kind,
 			response_sender,
 			..
-		}) if exec_timeout_kind == PvfExecTimeoutKind::Approval => {
+		}) if exec_kind == PvfExecKind::Approval => {
 			response_sender.send(Ok(ValidationResult::Valid(Default::default(), Default::default())))
 				.unwrap();
 		}

diff --git a/polkadot/node/core/backing/src/lib.rs b/polkadot/node/core/backing/src/lib.rs
@@ -106,7 +106,7 @@ use polkadot_node_subsystem_util::{
 use polkadot_primitives::{
 	BackedCandidate, CandidateCommitments, CandidateHash, CandidateReceipt,
 	CommittedCandidateReceipt, CoreIndex, CoreState, ExecutorParams, Hash, Id as ParaId,
-	PersistedValidationData, PvfExecTimeoutKind, SigningContext, ValidationCode, ValidatorId,
+	PersistedValidationData, PvfExecKind, SigningContext, ValidationCode, ValidatorId,
 	ValidatorIndex, ValidatorSignature, ValidityAttestation,
 };
 use sp_keystore::KeystorePtr;
@@ -566,7 +566,7 @@ async fn request_candidate_validation(
 			candidate_receipt,
 			pov,
 			executor_params,
-			exec_timeout_kind: PvfExecTimeoutKind::Backing,
+			exec_kind: PvfExecKind::Backing,
 			response_sender: tx,
 		})
 		.await;

diff --git a/polkadot/node/core/backing/src/tests/mod.rs b/polkadot/node/core/backing/src/tests/mod.rs
@@ -33,7 +33,7 @@ use polkadot_node_subsystem::{
 };
 use polkadot_node_subsystem_test_helpers as test_helpers;
 use polkadot_primitives::{
-	CandidateDescriptor, GroupRotationInfo, HeadData, PersistedValidationData, PvfExecTimeoutKind,
+	CandidateDescriptor, GroupRotationInfo, HeadData, PersistedValidationData, PvfExecKind,
 	ScheduledCore, SessionIndex, LEGACY_MIN_BACKING_VOTES,
 };
 use sp_application_crypto::AppCrypto;
@@ -344,14 +344,14 @@ async fn assert_validate_from_exhaustive(
 				validation_data,
 				validation_code,
 				candidate_receipt,
-				exec_timeout_kind,
+				exec_kind,
 				response_sender,
 				..
 			},
 		) if validation_data == *assert_pvd &&
 			validation_code == *assert_validation_code &&
 			*pov == *assert_pov && &candidate_receipt.descriptor == assert_candidate.descriptor() &&
-			exec_timeout_kind == PvfExecTimeoutKind::Backing &&
+			exec_kind == PvfExecKind::Backing &&
 			candidate_receipt.commitments_hash == assert_candidate.commitments.hash() =>
 		{
 			response_sender.send(Ok(ValidationResult::Valid(
@@ -550,14 +550,14 @@ fn backing_works() {
 					validation_code,
 					candidate_receipt,
 					pov,
-					exec_timeout_kind,
+					exec_kind,
 					response_sender,
 					..
 				},
 			) if validation_data == pvd_ab &&
 				validation_code == validation_code_ab &&
 				*pov == pov_ab && &candidate_receipt.descriptor == candidate_a.descriptor() &&
-				exec_timeout_kind == PvfExecTimeoutKind::Backing &&
+				exec_kind == PvfExecKind::Backing &&
 				candidate_receipt.commitments_hash == candidate_a_commitments_hash =>
 			{
 				response_sender.send(Ok(
@@ -729,14 +729,14 @@ fn backing_works_while_validation_ongoing() {
 					validation_code,
 					candidate_receipt,
 					pov,
-					exec_timeout_kind,
+					exec_kind,
 					response_sender,
 					..
 				},
 			) if validation_data == pvd_abc &&
 				validation_code == validation_code_abc &&
 				*pov == pov_abc && &candidate_receipt.descriptor == candidate_a.descriptor() &&
-				exec_timeout_kind == PvfExecTimeoutKind::Backing &&
+				exec_kind == PvfExecKind::Backing &&
 				candidate_a_commitments_hash == candidate_receipt.commitments_hash =>
 			{
 				// we never validate the candidate. our local node
@@ -890,14 +890,14 @@ fn backing_misbehavior_works() {
 					validation_code,
 					candidate_receipt,
 					pov,
-					exec_timeout_kind,
+					exec_kind,
 					response_sender,
 					..
 				},
 			) if validation_data == pvd_a &&
 				validation_code == validation_code_a &&
 				*pov == pov_a && &candidate_receipt.descriptor == candidate_a.descriptor() &&
-				exec_timeout_kind == PvfExecTimeoutKind::Backing &&
+				exec_kind == PvfExecKind::Backing &&
 				candidate_a_commitments_hash == candidate_receipt.commitments_hash =>
 			{
 				response_sender.send(Ok(
@@ -1057,14 +1057,14 @@ fn backing_dont_second_invalid() {
 					validation_code,
 					candidate_receipt,
 					pov,
-					exec_timeout_kind,
+					exec_kind,
 					response_sender,
 					..
 				},
 			) if validation_data == pvd_a &&
 				validation_code == validation_code_a &&
 				*pov == pov_block_a && &candidate_receipt.descriptor == candidate_a.descriptor() &&
-				exec_timeout_kind == PvfExecTimeoutKind::Backing &&
+				exec_kind == PvfExecKind::Backing &&
 				candidate_a.commitments.hash() == candidate_receipt.commitments_hash =>
 			{
 				response_sender.send(Ok(ValidationResult::Invalid(InvalidCandidate::BadReturn))).unwrap();
@@ -1097,14 +1097,14 @@ fn backing_dont_second_invalid() {
 					validation_code,
 					candidate_receipt,
 					pov,
-					exec_timeout_kind,
+					exec_kind,
 					response_sender,
 					..
 				},
 			) if validation_data == pvd_b &&
 				validation_code == validation_code_b &&
 				*pov == pov_block_b && &candidate_receipt.descriptor == candidate_b.descriptor() &&
-				exec_timeout_kind == PvfExecTimeoutKind::Backing &&
+				exec_kind == PvfExecKind::Backing &&
 				candidate_b.commitments.hash() == candidate_receipt.commitments_hash =>
 			{
 				response_sender.send(Ok(
@@ -1224,14 +1224,14 @@ fn backing_second_after_first_fails_works() {
 					validation_code,
 					candidate_receipt,
 					pov,
-					exec_timeout_kind,
+					exec_kind,
 					response_sender,
 					..
 				},
 			) if validation_data == pvd_a &&
 				validation_code == validation_code_a &&
 				*pov == pov_a && &candidate_receipt.descriptor == candidate.descriptor() &&
-				exec_timeout_kind == PvfExecTimeoutKind::Backing &&
+				exec_kind == PvfExecKind::Backing &&
 				candidate.commitments.hash() == candidate_receipt.commitments_hash =>
 			{
 				response_sender.send(Ok(ValidationResult::Invalid(InvalidCandidate::BadReturn))).unwrap();
@@ -1368,14 +1368,14 @@ fn backing_works_after_failed_validation() {
 					validation_code,
 					candidate_receipt,
 					pov,
-					exec_timeout_kind,
+					exec_kind,
 					response_sender,
 					..
 				},
 			) if validation_data == pvd_a &&
 				validation_code == validation_code_a &&
 				*pov == pov_a && &candidate_receipt.descriptor == candidate.descriptor() &&
-				exec_timeout_kind == PvfExecTimeoutKind::Backing &&
+				exec_kind == PvfExecKind::Backing &&
 				candidate.commitments.hash() == candidate_receipt.commitments_hash =>
 			{
 				response_sender.send(Err(ValidationFailed("Internal test error".into()))).unwrap();
@@ -1634,13 +1634,13 @@ fn retry_works() {
 					validation_code,
 					candidate_receipt,
 					pov,
-					exec_timeout_kind,
+					exec_kind,
 					..
 				},
 			) if validation_data == pvd_a &&
 				validation_code == validation_code_a &&
 				*pov == pov_a && &candidate_receipt.descriptor == candidate.descriptor() &&
-				exec_timeout_kind == PvfExecTimeoutKind::Backing &&
+				exec_kind == PvfExecKind::Backing &&
 				candidate.commitments.hash() == candidate_receipt.commitments_hash
 		);
 		virtual_overseer

diff --git a/polkadot/node/core/backing/src/tests/prospective_parachains.rs b/polkadot/node/core/backing/src/tests/prospective_parachains.rs
@@ -232,14 +232,14 @@ async fn assert_validate_seconded_candidate(
 			validation_code,
 			candidate_receipt,
 			pov,
-			exec_timeout_kind,
+			exec_kind,
 			response_sender,
 			..
 		}) if &validation_data == assert_pvd &&
 			&validation_code == assert_validation_code &&
 			&*pov == assert_pov &&
 			&candidate_receipt.descriptor == candidate.descriptor() &&
-			exec_timeout_kind == PvfExecTimeoutKind::Backing &&
+			exec_kind == PvfExecKind::Backing &&
 			candidate.commitments.hash() == candidate_receipt.commitments_hash =>
 		{
 			response_sender.send(Ok(ValidationResult::Valid(