Fix nothing scheduled on session boundary (#1403)

* Fix scheduled state at session boundaries.

* Cleanup + better docs.

* More cleanup and fixes.

* Remove 12s hack.

* Add dep.

* Make clippy happy

---------

Co-authored-by: eskimor <[email protected]>

polkadot/primitives/src/lib.rs

@@ -41,8 +41,8 @@ pub use v5::{
 	BackedCandidate, Balance, BlakeTwo256, Block, BlockId, BlockNumber, CandidateCommitments,
 	CandidateDescriptor, CandidateEvent, CandidateHash, CandidateIndex, CandidateReceipt,
 	CheckedDisputeStatementSet, CheckedMultiDisputeStatementSet, CollatorId, CollatorSignature,
-	CommittedCandidateReceipt, CompactStatement, ConsensusLog, CoreIndex, CoreOccupied, CoreState,
-	DisputeState, DisputeStatement, DisputeStatementSet, DownwardMessage, EncodeAs, ExecutorParam,
+	CommittedCandidateReceipt, CompactStatement, ConsensusLog, CoreIndex, CoreState, DisputeState,
+	DisputeStatement, DisputeStatementSet, DownwardMessage, EncodeAs, ExecutorParam,
 	ExecutorParams, ExecutorParamsHash, ExplicitDisputeStatement, GroupIndex, GroupRotationInfo,
 	Hash, HashT, HeadData, Header, HrmpChannelId, Id, InboundDownwardMessage, InboundHrmpMessage,
 	IndexedVec, InherentData, InvalidDisputeStatementKind, Moment, MultiDisputeStatementSet, Nonce,

polkadot/primitives/src/v5/mod.rs

@@ -830,60 +830,6 @@ pub struct ParathreadEntry {
 	pub retries: u32,
 }
 
-/// An assignment for a parachain scheduled to be backed and included in a relay chain block.
-#[derive(Clone, Encode, Decode, PartialEq, TypeInfo, RuntimeDebug)]
-pub struct Assignment {
-	/// Assignment's ParaId
-	pub para_id: Id,
-}
-
-impl Assignment {
-	/// Create a new `Assignment`.
-	pub fn new(para_id: Id) -> Self {
-		Self { para_id }
-	}
-}
-
-/// An entry tracking a paras
-#[derive(Clone, Encode, Decode, TypeInfo, PartialEq, RuntimeDebug)]
-pub struct ParasEntry<N = BlockNumber> {
-	/// The `Assignment`
-	pub assignment: Assignment,
-	/// The number of times the entry has timed out in availability.
-	pub availability_timeouts: u32,
-	/// The block height where this entry becomes invalid.
-	pub ttl: N,
-}
-
-impl<N> ParasEntry<N> {
-	/// Return `Id` from the underlying `Assignment`.
-	pub fn para_id(&self) -> Id {
-		self.assignment.para_id
-	}
-
-	/// Create a new `ParasEntry`.
-	pub fn new(assignment: Assignment, now: N) -> Self {
-		ParasEntry { assignment, availability_timeouts: 0, ttl: now }
-	}
-}
-
-/// What is occupying a specific availability core.
-#[derive(Clone, Encode, Decode, TypeInfo, RuntimeDebug)]
-#[cfg_attr(feature = "std", derive(PartialEq))]
-pub enum CoreOccupied<N> {
-	/// The core is not occupied.
-	Free,
-	/// A paras.
-	Paras(ParasEntry<N>),
-}
-
-impl<N> CoreOccupied<N> {
-	/// Is core free?
-	pub fn is_free(&self) -> bool {
-		matches!(self, Self::Free)
-	}
-}
-
 /// A helper data-type for tracking validator-group rotations.
 #[derive(Clone, Encode, Decode, TypeInfo, RuntimeDebug)]
 #[cfg_attr(feature = "std", derive(PartialEq))]

polkadot/runtime/parachains/Cargo.toml

@@ -50,6 +50,7 @@ rand_chacha = { version = "0.3.1", default-features = false }
 static_assertions = { version = "1.1.0", optional = true }
 polkadot-parachain-primitives = { path = "../../parachain", default-features = false }
 polkadot-runtime-metrics = { path = "../metrics", default-features = false}
+polkadot-core-primitives = { path = "../../core-primitives", default-features = false }
 
 [dev-dependencies]
 futures = "0.3.21"

polkadot/runtime/parachains/src/assigner.rs

@@ -17,11 +17,11 @@
 //! The Polkadot multiplexing assignment provider.
 //! Provides blockspace assignments for both bulk and on demand parachains.
 use frame_system::pallet_prelude::BlockNumberFor;
-use primitives::{v5::Assignment, CoreIndex, Id as ParaId};
+use primitives::{CoreIndex, Id as ParaId};
 
 use crate::{
 	configuration, paras,
-	scheduler::common::{AssignmentProvider, AssignmentProviderConfig},
+	scheduler::common::{Assignment, AssignmentProvider, AssignmentProviderConfig},
 };
 
 pub use pallet::*;

polkadot/runtime/parachains/src/assigner_on_demand/mod.rs

@@ -34,7 +34,7 @@ mod tests;
 
 use crate::{
 	configuration, paras,
-	scheduler::common::{AssignmentProvider, AssignmentProviderConfig},
+	scheduler::common::{Assignment, AssignmentProvider, AssignmentProviderConfig},
 };
 
 use frame_support::{
@@ -46,7 +46,7 @@ use frame_support::{
 	},
 };
 use frame_system::pallet_prelude::*;
-use primitives::{v5::Assignment, CoreIndex, Id as ParaId};
+use primitives::{CoreIndex, Id as ParaId};
 use sp_runtime::{
 	traits::{One, SaturatedConversion},
 	FixedPointNumber, FixedPointOperand, FixedU128, Perbill, Saturating,
@@ -606,7 +606,6 @@ impl<T: Config> AssignmentProvider<BlockNumberFor<T>> for Pallet<T> {
 	fn get_provider_config(_core_idx: CoreIndex) -> AssignmentProviderConfig<BlockNumberFor<T>> {
 		let config = <configuration::Pallet<T>>::config();
 		AssignmentProviderConfig {
-			availability_period: config.paras_availability_period,
 			max_availability_timeouts: config.on_demand_retries,
 			ttl: config.on_demand_ttl,
 		}

polkadot/runtime/parachains/src/assigner_on_demand/tests.rs

@@ -24,13 +24,11 @@ use crate::{
 		System, Test,
 	},
 	paras::{ParaGenesisArgs, ParaKind},
+	scheduler::common::Assignment,
 };
 use frame_support::{assert_noop, assert_ok, error::BadOrigin};
 use pallet_balances::Error as BalancesError;
-use primitives::{
-	v5::{Assignment, ValidationCode},
-	BlockNumber, SessionIndex,
-};
+use primitives::{v5::ValidationCode, BlockNumber, SessionIndex};
 use sp_std::collections::btree_map::BTreeMap;
 
 fn schedule_blank_para(id: ParaId, parakind: ParaKind) {

polkadot/runtime/parachains/src/assigner_parachains.rs

@@ -19,11 +19,11 @@
 
 use crate::{
 	configuration, paras,
-	scheduler::common::{AssignmentProvider, AssignmentProviderConfig},
+	scheduler::common::{Assignment, AssignmentProvider, AssignmentProviderConfig},
 };
 use frame_system::pallet_prelude::BlockNumberFor;
 pub use pallet::*;
-use primitives::{v5::Assignment, CoreIndex, Id as ParaId};
+use primitives::{CoreIndex, Id as ParaId};
 
 #[frame_support::pallet]
 pub mod pallet {
@@ -57,9 +57,7 @@ impl<T: Config> AssignmentProvider<BlockNumberFor<T>> for Pallet<T> {
 	fn push_assignment_for_core(_: CoreIndex, _: Assignment) {}
 
 	fn get_provider_config(_core_idx: CoreIndex) -> AssignmentProviderConfig<BlockNumberFor<T>> {
-		let config = <configuration::Pallet<T>>::config();
 		AssignmentProviderConfig {
-			availability_period: config.paras_availability_period,
 			// The next assignment already goes to the same [`ParaId`], no timeout tracking needed.
 			max_availability_timeouts: 0,
 			// The next assignment already goes to the same [`ParaId`], this can be any number

polkadot/runtime/parachains/src/builder.rs

@@ -18,18 +18,20 @@ use crate::{
 	configuration, inclusion, initializer, paras,
 	paras::ParaKind,
 	paras_inherent,
-	scheduler::{self, common::AssignmentProviderConfig},
+	scheduler::{
+		self,
+		common::{Assignment, AssignmentProviderConfig},
+		CoreOccupied, ParasEntry,
+	},
 	session_info, shared,
 };
 use bitvec::{order::Lsb0 as BitOrderLsb0, vec::BitVec};
 use frame_support::pallet_prelude::*;
 use frame_system::pallet_prelude::*;
 use primitives::{
-	collator_signature_payload,
-	v5::{Assignment, ParasEntry},
-	AvailabilityBitfield, BackedCandidate, CandidateCommitments, CandidateDescriptor,
-	CandidateHash, CollatorId, CollatorSignature, CommittedCandidateReceipt, CompactStatement,
-	CoreIndex, CoreOccupied, DisputeStatement, DisputeStatementSet, GroupIndex, HeadData,
+	collator_signature_payload, AvailabilityBitfield, BackedCandidate, CandidateCommitments,
+	CandidateDescriptor, CandidateHash, CollatorId, CollatorSignature, CommittedCandidateReceipt,
+	CompactStatement, CoreIndex, DisputeStatement, DisputeStatementSet, GroupIndex, HeadData,
 	Id as ParaId, IndexedVec, InherentData as ParachainsInherentData, InvalidDisputeStatementKind,
 	PersistedValidationData, SessionIndex, SigningContext, UncheckedSigned,
 	ValidDisputeStatementKind, ValidationCode, ValidatorId, ValidatorIndex, ValidityAttestation,

polkadot/runtime/parachains/src/configuration.rs

@@ -190,11 +190,20 @@ pub struct HostConfiguration<BlockNumber> {
 	///
 	/// Must be non-zero.
 	pub group_rotation_frequency: BlockNumber,
-	/// The availability period, in blocks. This is the amount of blocks
-	/// after inclusion that validators have to make the block available and signal its
-	/// availability to the chain.
+	/// The minimum availability period, in blocks.
 	///
-	/// Must be at least 1.
+	/// This is the minimum amount of blocks after a core became occupied that validators have time
+	/// to make the block available.
+	///
+	/// This value only has effect on group rotations. If backers backed something at the end of
+	/// their rotation, the occupied core affects the backing group that comes afterwards. We limit
+	/// the effect one backing group can have on the next to `paras_availability_period` blocks.
+	///
+	/// Within a group rotation there is no timeout as backers are only affecting themselves.
+	///
+	/// Must be at least 1. With a value of 1, the previous group will not be able to negatively
+	/// affect the following group at the expense of a tight availability timeline at group
+	/// rotation boundaries.
 	pub paras_availability_period: BlockNumber,
 	/// The amount of blocks ahead to schedule paras.
 	pub scheduling_lookahead: u32,