Skip to content

Commit

Permalink
Fix nothing scheduled on session boundary (#1403)
Browse files Browse the repository at this point in the history
* 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 <eskimor@no-such-url.com>
  • Loading branch information
2 people authored and Daanvdplas committed Sep 11, 2023
1 parent 031187b commit c99c00d
Show file tree
Hide file tree
Showing 19 changed files with 492 additions and 646 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions polkadot/primitives/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
54 changes: 0 additions & 54 deletions polkadot/primitives/src/v5/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))]
Expand Down
1 change: 1 addition & 0 deletions polkadot/runtime/parachains/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
4 changes: 2 additions & 2 deletions polkadot/runtime/parachains/src/assigner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::*;
Expand Down
5 changes: 2 additions & 3 deletions polkadot/runtime/parachains/src/assigner_on_demand/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ mod tests;

use crate::{
configuration, paras,
scheduler::common::{AssignmentProvider, AssignmentProviderConfig},
scheduler::common::{Assignment, AssignmentProvider, AssignmentProviderConfig},
};

use frame_support::{
Expand All @@ -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,
Expand Down Expand Up @@ -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,
}
Expand Down
6 changes: 2 additions & 4 deletions polkadot/runtime/parachains/src/assigner_on_demand/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
6 changes: 2 additions & 4 deletions polkadot/runtime/parachains/src/assigner_parachains.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down
14 changes: 8 additions & 6 deletions polkadot/runtime/parachains/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
17 changes: 13 additions & 4 deletions polkadot/runtime/parachains/src/configuration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Loading

0 comments on commit c99c00d

Please sign in to comment.