revert to old handle_new_activations logic in some cases (#7514)

* revert to old `handle_new_activations` logic * gate sending messages on scheduled cores to max_depth >= 2 * fmt * 2->1
paritytech · Jul 28, 2023 · f2aa3b3 · f2aa3b3
1 parent 637f0c5
commit f2aa3b3
Show file tree

Hide file tree

Showing 2 changed files with 64 additions and 11 deletions.
diff --git a/node/collation-generation/src/lib.rs b/node/collation-generation/src/lib.rs
@@ -41,8 +41,9 @@ use polkadot_node_subsystem::{
 	SubsystemContext, SubsystemError, SubsystemResult,
 };
 use polkadot_node_subsystem_util::{
-	request_availability_cores, request_persisted_validation_data, request_validation_code,
-	request_validation_code_hash, request_validators,
+	request_availability_cores, request_persisted_validation_data,
+	request_staging_async_backing_params, request_validation_code, request_validation_code_hash,
+	request_validators,
 };
 use polkadot_primitives::{
 	collator_signature_payload, CandidateCommitments, CandidateDescriptor, CandidateReceipt,
@@ -201,29 +202,46 @@ async fn handle_new_activations<Context>(
 	for relay_parent in activated {
 		let _relay_parent_timer = metrics.time_new_activations_relay_parent();
 
-		let (availability_cores, validators) = join!(
+		let (availability_cores, validators, async_backing_params) = join!(
 			request_availability_cores(relay_parent, ctx.sender()).await,
 			request_validators(relay_parent, ctx.sender()).await,
+			request_staging_async_backing_params(relay_parent, ctx.sender()).await,
 		);
 
 		let availability_cores = availability_cores??;
 		let n_validators = validators??.len();
+		let async_backing_params = async_backing_params?.ok();
 
 		for (core_idx, core) in availability_cores.into_iter().enumerate() {
 			let _availability_core_timer = metrics.time_new_activations_availability_core();
 
 			let (scheduled_core, assumption) = match core {
 				CoreState::Scheduled(scheduled_core) =>
 					(scheduled_core, OccupiedCoreAssumption::Free),
-				CoreState::Occupied(occupied_core) => {
-					// TODO [now]: this assumes that next up == current.
-					// in practice we should only set `OccupiedCoreAssumption::Included`
-					// when the candidate occupying the core is also of the same para.
-					if let Some(scheduled) = occupied_core.next_up_on_available {
-						(scheduled, OccupiedCoreAssumption::Included)
-					} else {
+				CoreState::Occupied(occupied_core) => match async_backing_params {
+					Some(params) if params.max_candidate_depth >= 1 => {
+						// maximum candidate depth when building on top of a block
+						// pending availability is necessarily 1 - the depth of the
+						// pending block is 0 so the child has depth 1.
+
+						// TODO [now]: this assumes that next up == current.
+						// in practice we should only set `OccupiedCoreAssumption::Included`
+						// when the candidate occupying the core is also of the same para.
+						if let Some(scheduled) = occupied_core.next_up_on_available {
+							(scheduled, OccupiedCoreAssumption::Included)
+						} else {
+							continue
+						}
+					},
+					_ => {
+						gum::trace!(
+							target: LOG_TARGET,
+							core_idx = %core_idx,
+							relay_parent = ?relay_parent,
+							"core is occupied. Keep going.",
+						);
 						continue
-					}
+					},
 				},
 				CoreState::Free => {
 					gum::trace!(

diff --git a/node/collation-generation/src/tests.rs b/node/collation-generation/src/tests.rs
@@ -151,6 +151,14 @@ fn requests_availability_per_relay_parent() {
 				Some(AllMessages::RuntimeApi(RuntimeApiMessage::Request(_hash, RuntimeApiRequest::Validators(tx)))) => {
 					tx.send(Ok(vec![dummy_validator(); 3])).unwrap();
 				}
+				Some(AllMessages::RuntimeApi(RuntimeApiMessage::Request(
+					_hash,
+					RuntimeApiRequest::StagingAsyncBackingParams(
+						tx,
+					),
+				))) => {
+					tx.send(Err(RuntimeApiError::NotSupported { runtime_api_name: "doesnt_matter" })).unwrap();
+				},
 				Some(msg) => panic!("didn't expect any other overseer requests given no availability cores; got {:?}", msg),
 			}
 		}
@@ -225,6 +233,15 @@ fn requests_validation_data_for_scheduled_matches() {
 				))) => {
 					tx.send(Ok(vec![dummy_validator(); 3])).unwrap();
 				},
+				Some(AllMessages::RuntimeApi(RuntimeApiMessage::Request(
+					_hash,
+					RuntimeApiRequest::StagingAsyncBackingParams(tx),
+				))) => {
+					tx.send(Err(RuntimeApiError::NotSupported {
+						runtime_api_name: "doesnt_matter",
+					}))
+					.unwrap();
+				},
 				Some(msg) => {
 					panic!("didn't expect any other overseer requests; got {:?}", msg)
 				},
@@ -313,6 +330,15 @@ fn sends_distribute_collation_message() {
 				))) => {
 					tx.send(Ok(Some(ValidationCode(vec![1, 2, 3]).hash()))).unwrap();
 				},
+				Some(AllMessages::RuntimeApi(RuntimeApiMessage::Request(
+					_hash,
+					RuntimeApiRequest::StagingAsyncBackingParams(tx),
+				))) => {
+					tx.send(Err(RuntimeApiError::NotSupported {
+						runtime_api_name: "doesnt_matter",
+					}))
+					.unwrap();
+				},
 				Some(msg @ AllMessages::CollatorProtocol(_)) => {
 					inner_to_collator_protocol.lock().await.push(msg);
 				},
@@ -466,6 +492,15 @@ fn fallback_when_no_validation_code_hash_api() {
 				))) => {
 					tx.send(Ok(Some(ValidationCode(vec![1, 2, 3])))).unwrap();
 				},
+				Some(AllMessages::RuntimeApi(RuntimeApiMessage::Request(
+					_hash,
+					RuntimeApiRequest::StagingAsyncBackingParams(tx),
+				))) => {
+					tx.send(Err(RuntimeApiError::NotSupported {
+						runtime_api_name: "doesnt_matter",
+					}))
+					.unwrap();
+				},
 				Some(msg @ AllMessages::CollatorProtocol(_)) => {
 					inner_to_collator_protocol.lock().await.push(msg);
 				},