Skip building on blocks on relay parents in old session

cumulus/client/consensus/aura/src/collators/slot_based/block_builder_task.rs

@@ -201,7 +201,7 @@ where
 				continue;
 			};
 
-			let Ok(rp_data) = offset_relay_parent_find_descendants(
+			let Ok(Some(rp_data)) = offset_relay_parent_find_descendants(
 				&mut relay_chain_data_cache,
 				relay_best_hash,
 				relay_parent_offset,
@@ -467,7 +467,7 @@ pub(crate) async fn offset_relay_parent_find_descendants<RelayClient>(
 	relay_chain_data_cache: &mut RelayChainDataCache<RelayClient>,
 	relay_best_block: RelayHash,
 	relay_parent_offset: u32,
-) -> Result<RelayParentData, ()>
+) -> Result<Option<RelayParentData>, ()>
 where
 	RelayClient: RelayChainInterface + Clone + 'static,
 {
@@ -481,7 +481,12 @@ where
 	};
 
 	if relay_parent_offset == 0 {
-		return Ok(RelayParentData::new(relay_header));
+		return Ok(Some(RelayParentData::new(relay_header)));
+	}
+
+	if sc_consensus_babe::contains_epoch_change::<RelayBlock>(&relay_header) {
+		tracing::debug!(target: LOG_TARGET, ?relay_best_block, relay_best_block_number = relay_header.number(), "Relay parent is in previous session.");
+		return Ok(None);
 	}
 
 	let mut required_ancestors: VecDeque<RelayHeader> = Default::default();
@@ -492,6 +497,10 @@ where
 			.await?
 			.relay_parent_header
 			.clone();
+		if sc_consensus_babe::contains_epoch_change::<RelayBlock>(&next_header) {
+			tracing::debug!(target: LOG_TARGET, ?relay_best_block, ancestor = %next_header.hash(), ancestor_block_number = next_header.number(), "Ancestor of best block is in previous session.");
+			return Ok(None);
+		}
 		required_ancestors.push_front(next_header.clone());
 		relay_header = next_header;
 	}
@@ -510,7 +519,7 @@ where
 		"Relay parent descendants."
 	);
 
-	Ok(RelayParentData::new_with_descendants(relay_parent, required_ancestors.into()))
+	Ok(Some(RelayParentData::new_with_descendants(relay_parent, required_ancestors.into())))
 }
 
 /// Return value of [`determine_core`].

cumulus/client/consensus/aura/src/collators/slot_based/tests.rs

@@ -20,6 +20,7 @@ use super::{
 	relay_chain_data_cache::{RelayChainData, RelayChainDataCache},
 };
 use async_trait::async_trait;
+use codec::Encode;
 use cumulus_primitives_core::{ClaimQueueOffset, CoreInfo, CoreSelector, CumulusDigestItem};
 use cumulus_relay_chain_interface::*;
 use futures::Stream;
@@ -28,6 +29,11 @@ use polkadot_primitives::{
 	CandidateEvent, CommittedCandidateReceiptV2, CoreIndex, Hash as RelayHash,
 	Header as RelayHeader, Id as ParaId,
 };
+use rstest::rstest;
+use sc_consensus_babe::{
+	AuthorityId, ConsensusLog as BabeConsensusLog, NextEpochDescriptor, BABE_ENGINE_ID,
+};
+use sp_core::sr25519;
 use sp_runtime::{generic::BlockId, testing::Header as TestHeader, traits::Header};
 use sp_version::RuntimeVersion;
 use std::{
@@ -45,7 +51,7 @@ async fn offset_test_zero_offset() {
 
 	let result = offset_relay_parent_find_descendants(&mut cache, best_hash, 0).await;
 	assert!(result.is_ok());
-	let data = result.unwrap();
+	let data = result.unwrap().unwrap();
 	assert_eq!(data.descendants_len(), 0);
 	assert_eq!(data.relay_parent().hash(), best_hash);
 	assert!(data.into_inherent_descendant_list().is_empty());
@@ -61,7 +67,7 @@ async fn offset_test_two_offset() {
 
 	let result = offset_relay_parent_find_descendants(&mut cache, best_hash, 2).await;
 	assert!(result.is_ok());
-	let data = result.unwrap();
+	let data = result.unwrap().unwrap();
 	assert_eq!(data.descendants_len(), 2);
 	assert_eq!(*data.relay_parent().number(), 98);
 	let descendant_list = data.into_inherent_descendant_list();
@@ -80,7 +86,7 @@ async fn offset_test_five_offset() {
 
 	let result = offset_relay_parent_find_descendants(&mut cache, best_hash, 5).await;
 	assert!(result.is_ok());
-	let data = result.unwrap();
+	let data = result.unwrap().unwrap();
 	assert_eq!(data.descendants_len(), 5);
 	assert_eq!(*data.relay_parent().number(), 95);
 	let descendant_list = data.into_inherent_descendant_list();
@@ -104,6 +110,33 @@ async fn offset_test_too_long() {
 	assert!(result.is_err());
 }
 
+#[derive(PartialEq)]
+enum HasEpochChange {
+	Yes,
+	No,
+}
+
+#[rstest]
+#[case::in_best(
+	&[HasEpochChange::No, HasEpochChange::No, HasEpochChange::Yes],
+)]
+#[case::in_first_ancestor(
+	&[HasEpochChange::No, HasEpochChange::Yes, HasEpochChange::No],
+)]
+#[case::in_second_ancestor(
+	&[HasEpochChange::Yes, HasEpochChange::No, HasEpochChange::No],
+)]
+#[tokio::test]
+async fn offset_returns_none_when_epoch_change_encountered(#[case] flags: &[HasEpochChange]) {
+	let (headers, best_hash) = build_headers_with_epoch_flags(flags);
+	let client = TestRelayClient::new(headers);
+	let mut cache = RelayChainDataCache::new(client, 1.into());
+
+	let result = offset_relay_parent_find_descendants(&mut cache, best_hash, 3).await;
+	assert!(result.is_ok());
+	assert!(result.unwrap().is_none());
+}
+
 #[tokio::test]
 async fn determine_core_new_relay_parent() {
 	let (headers, _best_hash) = create_header_chain();
@@ -593,6 +626,50 @@ impl RelayChainInterface for TestRelayClient {
 	}
 }
 
+/// Build a consecutive set of relay headers whose digest entries optionally carry a BABE
+/// epoch-change marker, returning the underlying map and the hash of the last header.
+fn build_headers_with_epoch_flags(
+	flags: &[HasEpochChange],
+) -> (HashMap<RelayHash, RelayHeader>, RelayHash) {
+	let mut headers = HashMap::new();
+	let mut parent_hash = RelayHash::default();
+	let mut last_hash = RelayHash::default();
+
+	for (index, has_epoch_change) in flags.iter().enumerate() {
+		let digest = if *has_epoch_change == HasEpochChange::Yes {
+			babe_epoch_change_digest()
+		} else {
+			Default::default()
+		};
+
+		let header = RelayHeader {
+			parent_hash,
+			number: (index as u32 + 1),
+			state_root: Default::default(),
+			extrinsics_root: Default::default(),
+			digest,
+		};
+
+		let hash = header.hash();
+		headers.insert(hash, header);
+		parent_hash = hash;
+		last_hash = hash;
+	}
+
+	(headers, last_hash)
+}
+
+/// Create a digest containing a single BABE `NextEpochData` item for use in tests.
+fn babe_epoch_change_digest() -> sp_runtime::generic::Digest {
+	let mut digest = sp_runtime::generic::Digest::default();
+	let authority_id = AuthorityId::from(sr25519::Public::from_raw([1u8; 32]));
+	let next_epoch =
+		NextEpochDescriptor { authorities: vec![(authority_id, 1u64)], randomness: [0u8; 32] };
+	let log = BabeConsensusLog::NextEpochData(next_epoch);
+	digest.push(sp_runtime::generic::DigestItem::Consensus(BABE_ENGINE_ID, log.encode()));
+	digest
+}
+
 fn create_header_chain() -> (HashMap<RelayHash, RelayHeader>, RelayHash) {
 	let mut headers = HashMap::new();
 	let mut current_parent = None;

cumulus/client/parachain-inherent/src/lib.rs

@@ -171,12 +171,10 @@ impl ParachainInherentDataProvider {
 	) -> Option<ParachainInherentData> {
 		// Only include next epoch authorities when the descendants include an epoch digest.
 		// Skip the first entry because this is the relay parent itself.
-		let include_next_authorities = relay_parent_descendants.iter().skip(1).any(|header| {
-			sc_consensus_babe::find_next_epoch_digest::<RelayBlock>(header)
-				.ok()
-				.flatten()
-				.is_some()
-		});
+		let include_next_authorities = relay_parent_descendants
+			.iter()
+			.skip(1)
+			.any(sc_consensus_babe::contains_epoch_change::<RelayBlock>);
 		let relay_chain_state = collect_relay_storage_proof(
 			relay_chain_interface,
 			para_id,

prdoc/pr_9990.prdoc

@@ -0,0 +1,16 @@
+title: '`cumulus`: Skip building on blocks on relay parents in old session'
+doc:
+- audience: Node Dev
+  description: |-
+    Collators building on older relay parents must skip building blocks when the chosen RP session is different than best block session.
+
+    The relay chain will not accept such candidate. We can see this happening on the Kusama Canary parachain at each session boundary.
+
+    Slot-based Aura has been modified in Cumulus to skip building blocks on relay parents in old session.
+crates:
+- name: cumulus-client-consensus-aura
+  bump: patch
+- name: cumulus-client-parachain-inherent
+  bump: patch
+- name: sc-consensus-babe
+  bump: minor

substrate/client/consensus/babe/src/lib.rs

@@ -913,6 +913,11 @@ pub fn find_pre_digest<B: BlockT>(header: &B::Header) -> Result<PreDigest, Error
 	pre_digest.ok_or_else(|| babe_err(Error::NoPreRuntimeDigest))
 }
 
+/// Check whether the given header contains a BABE epoch change digest.
+pub fn contains_epoch_change<B: BlockT>(header: &B::Header) -> bool {
+	find_next_epoch_digest::<B>(header).ok().flatten().is_some()
+}
+
 /// Extract the BABE epoch change digest from the given header, if it exists.
 pub fn find_next_epoch_digest<B: BlockT>(
 	header: &B::Header,