Revert loop prevention (#4472)

* Provisioner: Only include and sign bitfields on fresh leaves.

polkadot/node/core/backing/src/lib.rs

@@ -53,7 +53,7 @@ use polkadot_subsystem::{
 		DisputeCoordinatorMessage, ImportStatementsResult, ProvisionableData, ProvisionerMessage,
 		RuntimeApiRequest, StatementDistributionMessage, ValidationFailed,
 	},
-	overseer, PerLeafSpan, Stage, SubsystemSender,
+	overseer, ActivatedLeaf, PerLeafSpan, Stage, SubsystemSender,
 };
 use sp_keystore::SyncCryptoStorePtr;
 use statement_table::{
@@ -1180,13 +1180,13 @@ impl util::JobTrait for CandidateBackingJob {
 	const NAME: &'static str = "candidate-backing-job";
 
 	fn run<S: SubsystemSender>(
-		parent: Hash,
-		span: Arc<jaeger::Span>,
+		leaf: ActivatedLeaf,
 		keystore: SyncCryptoStorePtr,
 		metrics: Metrics,
 		rx_to: mpsc::Receiver<Self::ToJob>,
 		mut sender: JobSender<S>,
 	) -> Pin<Box<dyn Future<Output = Result<(), Self::Error>> + Send>> {
+		let parent = leaf.hash;
 		async move {
 			macro_rules! try_runtime_api {
 				($x: expr) => {
@@ -1208,7 +1208,7 @@ impl util::JobTrait for CandidateBackingJob {
 				}
 			}
 
-			let span = PerLeafSpan::new(span, "backing");
+			let span = PerLeafSpan::new(leaf.span, "backing");
 			let _span = span.child("runtime-apis");
 
 			let (validators, groups, session_index, cores) = futures::try_join!(

polkadot/node/core/bitfield-signing/src/lib.rs

@@ -34,7 +34,7 @@ use polkadot_node_subsystem::{
 		AvailabilityStoreMessage, BitfieldDistributionMessage, BitfieldSigningMessage,
 		RuntimeApiMessage, RuntimeApiRequest,
 	},
-	PerLeafSpan, SubsystemSender,
+	ActivatedLeaf, LeafStatus, PerLeafSpan, SubsystemSender,
 };
 use polkadot_node_subsystem_util::{
 	self as util,
@@ -43,7 +43,7 @@ use polkadot_node_subsystem_util::{
 };
 use polkadot_primitives::v1::{AvailabilityBitfield, CoreState, Hash, ValidatorIndex};
 use sp_keystore::{Error as KeystoreError, SyncCryptoStorePtr};
-use std::{iter::FromIterator, pin::Pin, sync::Arc, time::Duration};
+use std::{iter::FromIterator, pin::Pin, time::Duration};
 use wasm_timer::{Delay, Instant};
 
 #[cfg(test)]
@@ -237,8 +237,7 @@ impl JobTrait for BitfieldSigningJob {
 
 	/// Run a job for the parent block indicated
 	fn run<S: SubsystemSender>(
-		relay_parent: Hash,
-		span: Arc<jaeger::Span>,
+		leaf: ActivatedLeaf,
 		keystore: Self::RunArgs,
 		metrics: Self::Metrics,
 		_receiver: mpsc::Receiver<BitfieldSigningMessage>,
@@ -246,14 +245,23 @@ impl JobTrait for BitfieldSigningJob {
 	) -> Pin<Box<dyn Future<Output = Result<(), Self::Error>> + Send>> {
 		let metrics = metrics.clone();
 		async move {
-			let span = PerLeafSpan::new(span, "bitfield-signing");
+			if let LeafStatus::Stale = leaf.status {
+				tracing::debug!(
+					target: LOG_TARGET,
+					hash = ?leaf.hash,
+					block_number =  ?leaf.number,
+					"Stale leaf - don't sign bitfields."
+				);
+				return Ok(())
+			}
+
+			let span = PerLeafSpan::new(leaf.span, "bitfield-signing");
 			let _span = span.child("delay");
 			let wait_until = Instant::now() + JOB_DELAY;
 
 			// now do all the work we can before we need to wait for the availability store
 			// if we're not a validator, we can just succeed effortlessly
-			let validator = match Validator::new(relay_parent, keystore.clone(), &mut sender).await
-			{
+			let validator = match Validator::new(leaf.hash, keystore.clone(), &mut sender).await {
 				Ok(validator) => validator,
 				Err(util::Error::NotAValidator) => return Ok(()),
 				Err(err) => return Err(Error::Util(err)),
@@ -270,7 +278,7 @@ impl JobTrait for BitfieldSigningJob {
 			let span_availability = span.child("availability");
 
 			let bitfield = match construct_availability_bitfield(
-				relay_parent,
+				leaf.hash,
 				&span_availability,
 				validator.index(),
 				sender.subsystem_sender(),
@@ -311,7 +319,7 @@ impl JobTrait for BitfieldSigningJob {
 
 			sender
 				.send_message(BitfieldDistributionMessage::DistributeBitfield(
-					relay_parent,
+					leaf.hash,
 					signed_bitfield,
 				))
 				.await;

polkadot/node/core/provisioner/src/lib.rs

@@ -32,7 +32,7 @@ use polkadot_node_subsystem::{
 		CandidateBackingMessage, ChainApiMessage, DisputeCoordinatorMessage, ProvisionableData,
 		ProvisionerInherentData, ProvisionerMessage,
 	},
-	PerLeafSpan, SubsystemSender,
+	ActivatedLeaf, LeafStatus, PerLeafSpan, SubsystemSender,
 };
 use polkadot_node_subsystem_util::{
 	self as util, request_availability_cores, request_persisted_validation_data, JobSender,
@@ -43,7 +43,7 @@ use polkadot_primitives::v1::{
 	DisputeStatementSet, Hash, MultiDisputeStatementSet, OccupiedCoreAssumption,
 	SignedAvailabilityBitfield, ValidatorIndex,
 };
-use std::{collections::BTreeMap, pin::Pin, sync::Arc};
+use std::{collections::BTreeMap, pin::Pin};
 use thiserror::Error;
 
 mod metrics;
@@ -92,7 +92,7 @@ impl InherentAfter {
 
 /// A per-relay-parent job for the provisioning subsystem.
 pub struct ProvisioningJob {
-	relay_parent: Hash,
+	leaf: ActivatedLeaf,
 	receiver: mpsc::Receiver<ProvisionerMessage>,
 	backed_candidates: Vec<CandidateReceipt>,
 	signed_bitfields: Vec<SignedAvailabilityBitfield>,
@@ -156,15 +156,15 @@ impl JobTrait for ProvisioningJob {
 	//
 	// this function is in charge of creating and executing the job's main loop
 	fn run<S: SubsystemSender>(
-		relay_parent: Hash,
-		span: Arc<jaeger::Span>,
+		leaf: ActivatedLeaf,
 		_run_args: Self::RunArgs,
 		metrics: Self::Metrics,
 		receiver: mpsc::Receiver<ProvisionerMessage>,
 		mut sender: JobSender<S>,
 	) -> Pin<Box<dyn Future<Output = Result<(), Self::Error>> + Send>> {
 		async move {
-			let job = ProvisioningJob::new(relay_parent, metrics, receiver);
+			let span = leaf.span.clone();
+			let job = ProvisioningJob::new(leaf, metrics, receiver);
 
 			job.run_loop(sender.subsystem_sender(), PerLeafSpan::new(span, "provisioner"))
 				.await
@@ -175,12 +175,12 @@ impl JobTrait for ProvisioningJob {
 
 impl ProvisioningJob {
 	fn new(
-		relay_parent: Hash,
+		leaf: ActivatedLeaf,
 		metrics: Metrics,
 		receiver: mpsc::Receiver<ProvisionerMessage>,
 	) -> Self {
 		Self {
-			relay_parent,
+			leaf,
 			receiver,
 			backed_candidates: Vec::new(),
 			signed_bitfields: Vec::new(),
@@ -236,7 +236,7 @@ impl ProvisioningJob {
 		return_senders: Vec<oneshot::Sender<ProvisionerInherentData>>,
 	) {
 		if let Err(err) = send_inherent_data(
-			self.relay_parent,
+			&self.leaf,
 			&self.signed_bitfields,
 			&self.backed_candidates,
 			return_senders,
@@ -291,23 +291,27 @@ type CoreAvailability = BitVec<bitvec::order::Lsb0, u8>;
 /// maximize availability. So basically, include all bitfields. And then
 /// choose a coherent set of candidates along with that.
 async fn send_inherent_data(
-	relay_parent: Hash,
+	leaf: &ActivatedLeaf,
 	bitfields: &[SignedAvailabilityBitfield],
 	candidates: &[CandidateReceipt],
 	return_senders: Vec<oneshot::Sender<ProvisionerInherentData>>,
 	from_job: &mut impl SubsystemSender,
 	metrics: &Metrics,
 ) -> Result<(), Error> {
-	let availability_cores = request_availability_cores(relay_parent, from_job)
+	let availability_cores = request_availability_cores(leaf.hash, from_job)
 		.await
 		.await
 		.map_err(|err| Error::CanceledAvailabilityCores(err))??;
 
 	let disputes = select_disputes(from_job, metrics).await?;
-	let bitfields = select_availability_bitfields(&availability_cores, bitfields);
+	// Only include bitfields on fresh leaves. On chain reversions, we want to make sure that
+	// there will be at least one block, which cannot get disputed, so the chain can make progress.
+	let bitfields = match leaf.status {
+		LeafStatus::Fresh => select_availability_bitfields(&availability_cores, bitfields),
+		LeafStatus::Stale => Vec::new(),
+	};
 	let candidates =
-		select_candidates(&availability_cores, &bitfields, candidates, relay_parent, from_job)
-			.await?;
+		select_candidates(&availability_cores, &bitfields, candidates, leaf.hash, from_job).await?;
 
 	let inherent_data =
 		ProvisionerInherentData { bitfields, backed_candidates: candidates, disputes };