Improve parachain liveness by reducing required number of backing vot…

…es (#5016) * Let node side take advantage reduced requirements in the runtime and hopefully improve liveness of parachains by means of that. * Fix tests.
paritytech · Mar 8, 2022 · c1a8115 · c1a8115
1 parent 9162387
commit c1a8115
Show file tree

Hide file tree

Showing 3 changed files with 86 additions and 59 deletions.
diff --git a/node/core/backing/src/lib.rs b/node/core/backing/src/lib.rs
@@ -191,15 +191,10 @@ struct AttestingData {
 }
 
 /// How many votes we need to consider a candidate backed.
+///
+/// WARNING: This has to be kept in sync with the runtime check in the inclusion module.
 fn minimum_votes(n_validators: usize) -> usize {
-	// Runtime change going live, see: https://github.com/paritytech/polkadot/pull/4437
-	let old_runtime_value = n_validators / 2 + 1;
-	let new_runtime_value = std::cmp::min(2, n_validators);
-
-	// Until new runtime is live everywhere and we don't yet have
-	// https://github.com/paritytech/polkadot/issues/4576, we want to err on the higher value for
-	// secured block production:
-	std::cmp::max(old_runtime_value, new_runtime_value)
+	std::cmp::min(2, n_validators)
 }
 
 #[derive(Default)]

diff --git a/node/core/backing/src/tests.rs b/node/core/backing/src/tests.rs
@@ -526,6 +526,18 @@ fn backing_works() {
 		)
 		.await;
 
+		assert_matches!(
+			virtual_overseer.recv().await,
+			AllMessages::Provisioner(
+				ProvisionerMessage::ProvisionableData(
+					_,
+					ProvisionableData::BackedCandidate(candidate_receipt)
+				)
+			) => {
+				assert_eq!(candidate_receipt, candidate_a.to_plain());
+			}
+		);
+
 		assert_matches!(
 			virtual_overseer.recv().await,
 			AllMessages::StatementDistribution(
@@ -548,18 +560,6 @@ fn backing_works() {
 		)
 		.await;
 
-		assert_matches!(
-			virtual_overseer.recv().await,
-			AllMessages::Provisioner(
-				ProvisionerMessage::ProvisionableData(
-					_,
-					ProvisionableData::BackedCandidate(candidate_receipt)
-				)
-			) => {
-				assert_eq!(candidate_receipt, candidate_a.to_plain());
-			}
-		);
-
 		virtual_overseer
 			.send(FromOverseer::Signal(OverseerSignal::ActiveLeaves(
 				ActiveLeavesUpdate::stop_work(test_state.relay_parent),
@@ -701,16 +701,11 @@ fn backing_works_while_validation_ongoing() {
 
 		virtual_overseer.send(FromOverseer::Communication { msg: statement }).await;
 
-		let statement =
-			CandidateBackingMessage::Statement(test_state.relay_parent, signed_c.clone());
-
-		virtual_overseer.send(FromOverseer::Communication { msg: statement }).await;
-
 		test_dispute_coordinator_notifications(
 			&mut virtual_overseer,
 			candidate_a.hash(),
 			test_state.session(),
-			vec![ValidatorIndex(5), ValidatorIndex(3)],
+			vec![ValidatorIndex(5)],
 		)
 		.await;
 
@@ -728,6 +723,19 @@ fn backing_works_while_validation_ongoing() {
 			) if descriptor == candidate_a.descriptor
 		);
 
+		let statement =
+			CandidateBackingMessage::Statement(test_state.relay_parent, signed_c.clone());
+
+		virtual_overseer.send(FromOverseer::Communication { msg: statement }).await;
+
+		test_dispute_coordinator_notifications(
+			&mut virtual_overseer,
+			candidate_a.hash(),
+			test_state.session(),
+			vec![ValidatorIndex(3)],
+		)
+		.await;
+
 		let (tx, rx) = oneshot::channel();
 		let msg = CandidateBackingMessage::GetBackedCandidates(
 			test_state.relay_parent,
@@ -886,6 +894,19 @@ fn backing_misbehavior_works() {
 		)
 		.await;
 
+		assert_matches!(
+			virtual_overseer.recv().await,
+			AllMessages::Provisioner(
+				ProvisionerMessage::ProvisionableData(
+					_,
+					ProvisionableData::BackedCandidate(CandidateReceipt {
+						descriptor,
+						..
+					})
+				)
+			) if descriptor == candidate_a.descriptor
+		);
+
 		assert_matches!(
 			virtual_overseer.recv().await,
 			AllMessages::StatementDistribution(
@@ -1618,18 +1639,25 @@ fn retry_works() {
 		)
 		.await;
 
-		assert_matches!(
-			virtual_overseer.recv().await,
-			AllMessages::AvailabilityDistribution(
-				AvailabilityDistributionMessage::FetchPoV {
-					relay_parent,
-					tx,
-					..
-				}
+		// Not deterministic which message comes first:
+		for _ in 0u32..2 {
+			match virtual_overseer.recv().await {
+				AllMessages::Provisioner(ProvisionerMessage::ProvisionableData(
+					_,
+					ProvisionableData::BackedCandidate(CandidateReceipt { descriptor, .. }),
+				)) => {
+					assert_eq!(descriptor, candidate.descriptor);
+				},
+				AllMessages::AvailabilityDistribution(
+					AvailabilityDistributionMessage::FetchPoV { relay_parent, tx, .. },
 				) if relay_parent == test_state.relay_parent => {
-				std::mem::drop(tx);
+					std::mem::drop(tx);
+				},
+				msg => {
+					assert!(false, "Unexpected message: {:?}", msg);
+				},
 			}
-		);
+		}
 
 		let statement =
 			CandidateBackingMessage::Statement(test_state.relay_parent, signed_c.clone());
@@ -1643,27 +1671,20 @@ fn retry_works() {
 		)
 		.await;
 
-		// Not deterministic which message comes first:
-		for _ in 0u32..2 {
-			match virtual_overseer.recv().await {
-				AllMessages::Provisioner(ProvisionerMessage::ProvisionableData(
-					_,
-					ProvisionableData::BackedCandidate(CandidateReceipt { descriptor, .. }),
-				)) => {
-					assert_eq!(descriptor, candidate.descriptor);
-				},
+		assert_matches!(
+			virtual_overseer.recv().await,
+			AllMessages::AvailabilityDistribution(
+				AvailabilityDistributionMessage::FetchPoV {
+					relay_parent,
+					tx,
+					..
+				}
 				// Subsystem requests PoV and requests validation.
 				// Now we pass.
-				AllMessages::AvailabilityDistribution(
-					AvailabilityDistributionMessage::FetchPoV { relay_parent, tx, .. },
 				) if relay_parent == test_state.relay_parent => {
 					tx.send(pov.clone()).unwrap();
-				},
-				msg => {
-					assert!(false, "Unexpected message: {:?}", msg);
-				},
-			}
-		}
+				}
+		);
 
 		assert_matches!(
 			virtual_overseer.recv().await,
@@ -1774,16 +1795,11 @@ fn observes_backing_even_if_not_validator() {
 
 		virtual_overseer.send(FromOverseer::Communication { msg: statement }).await;
 
-		let statement =
-			CandidateBackingMessage::Statement(test_state.relay_parent, signed_c.clone());
-
-		virtual_overseer.send(FromOverseer::Communication { msg: statement }).await;
-
 		test_dispute_coordinator_notifications(
 			&mut virtual_overseer,
 			candidate_a_hash,
 			test_state.session(),
-			vec![ValidatorIndex(0), ValidatorIndex(5), ValidatorIndex(2)],
+			vec![ValidatorIndex(0), ValidatorIndex(5)],
 		)
 		.await;
 
@@ -1799,6 +1815,19 @@ fn observes_backing_even_if_not_validator() {
 			}
 		);
 
+		let statement =
+			CandidateBackingMessage::Statement(test_state.relay_parent, signed_c.clone());
+
+		virtual_overseer.send(FromOverseer::Communication { msg: statement }).await;
+
+		test_dispute_coordinator_notifications(
+			&mut virtual_overseer,
+			candidate_a_hash,
+			test_state.session(),
+			vec![ValidatorIndex(2)],
+		)
+		.await;
+
 		virtual_overseer
 			.send(FromOverseer::Signal(OverseerSignal::ActiveLeaves(
 				ActiveLeavesUpdate::stop_work(test_state.relay_parent),

diff --git a/runtime/parachains/src/inclusion/mod.rs b/runtime/parachains/src/inclusion/mod.rs
@@ -168,6 +168,9 @@ impl<H> Default for ProcessedCandidates<H> {
 }
 
 /// Number of backing votes we need for a valid backing.
+///
+/// WARNING: This check has to be kept in sync with the node side check in the backing
+/// subsystem.
 pub fn minimum_backing_votes(n_validators: usize) -> usize {
 	// For considerations on this value see:
 	// https://github.com/paritytech/polkadot/pull/1656#issuecomment-999734650