-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Undo v4 changes, move them to vstaging #7103
base: alex/parathreads_review
Are you sure you want to change the base?
Changes from 22 commits
b6b9f8e
c76ce40
e2d5869
fc276d6
7baa04f
94d24d5
f943c1a
59f6bc2
d65d96e
cbfa7a1
2471334
678b1d8
13fa655
2cf57a8
4d01e65
af57dd0
b413cc7
6fae5a3
a665df0
42bc527
ea8a0c6
a198b95
918243e
49ebc7d
c49582c
6273880
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -112,7 +112,7 @@ | |
|
||
use crate::{ | ||
vstaging, BlockNumber, CandidateCommitments, CandidateEvent, CandidateHash, | ||
CommittedCandidateReceipt, CoreState, DisputeState, ExecutorParams, GroupRotationInfo, | ||
CommittedCandidateReceipt, DisputeState, ExecutorParams, GroupRotationInfo, | ||
OccupiedCoreAssumption, PersistedValidationData, PvfCheckStatement, ScrapedOnChainVotes, | ||
SessionIndex, SessionInfo, ValidatorId, ValidatorIndex, ValidatorSignature, | ||
}; | ||
|
@@ -135,7 +135,12 @@ sp_api::decl_runtime_apis! { | |
|
||
/// Yields information on all availability cores as relevant to the child block. | ||
/// Cores are either free or occupied. Free cores can have paras assigned to them. | ||
fn availability_cores() -> Vec<CoreState<H, N>>; | ||
fn availability_cores() -> Vec<crate::v4::CoreState<H, N>>; | ||
|
||
/// Yields information on all availability cores as relevant to the child block. | ||
/// Cores are either free or occupied. Free cores can have paras assigned to them. | ||
#[api_version(5)] | ||
fn availability_cores_on_demand() -> Vec<vstaging::CoreState<H, N>>; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So this only gets us on-demand availability cores? I guess not, from the docs and from my understanding. So where does the name come from? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's supposed to be the |
||
|
||
/// Yields the persisted validation data for the given `ParaId` along with an assumption that | ||
/// should be used if the para currently occupies a core. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,7 +20,6 @@ use bitvec::vec::BitVec; | |
use parity_scale_codec::{Decode, Encode}; | ||
use scale_info::TypeInfo; | ||
use sp_std::{ | ||
collections::btree_set::BTreeSet, | ||
marker::PhantomData, | ||
prelude::*, | ||
slice::{Iter, IterMut}, | ||
|
@@ -29,7 +28,7 @@ use sp_std::{ | |
|
||
use application_crypto::KeyTypeId; | ||
use inherents::InherentIdentifier; | ||
use primitives::{OpaquePeerId, RuntimeDebug}; | ||
use primitives::RuntimeDebug; | ||
use runtime_primitives::traits::{AppVerify, Header as HeaderT}; | ||
use sp_arithmetic::traits::{BaseArithmetic, Saturating}; | ||
|
||
|
@@ -50,9 +49,6 @@ pub use polkadot_parachain::primitives::{ | |
|
||
use serde::{Deserialize, Serialize}; | ||
|
||
#[cfg(feature = "std")] | ||
use sc_network::PeerId; | ||
|
||
pub use sp_authority_discovery::AuthorityId as AuthorityDiscoveryId; | ||
pub use sp_consensus_slots::Slot; | ||
pub use sp_staking::SessionIndex; | ||
|
@@ -818,121 +814,14 @@ pub struct ParathreadEntry { | |
pub retries: u32, | ||
} | ||
|
||
/// An Assignemnt for a paras going to produce a paras block. | ||
#[derive(Clone, Encode, Decode, PartialEq, TypeInfo, RuntimeDebug)] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I remember these are actually not part of v4 at the moment and you are undoing previous work done in alex/parathreads_review, right? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Exactly! At the end of the day, we'll need to make sure v4 is just as it is on |
||
pub struct Assignment { | ||
/// Assignment's ParaId | ||
pub para_id: Id, | ||
/// Assignment's CollatorRestrictions | ||
pub collator_restrictions: CollatorRestrictions, | ||
} | ||
|
||
impl Assignment { | ||
/// Create a new `Assignment`. | ||
pub fn new(para_id: Id, collator_restrictions: CollatorRestrictions) -> Self { | ||
Assignment { para_id, collator_restrictions } | ||
} | ||
} | ||
|
||
/// Restrictions on collators for a specific paras block. | ||
#[derive(Clone, Encode, Decode, PartialEq, TypeInfo, RuntimeDebug)] | ||
pub struct CollatorRestrictions { | ||
/// Collators to prefer/allow. | ||
/// Empty set means no restrictions. | ||
collator_peer_ids: BTreeSet<OpaquePeerId>, | ||
restriction_kind: CollatorRestrictionKind, | ||
} | ||
|
||
impl CollatorRestrictions { | ||
/// Specialised new function for parachains. | ||
pub fn none() -> Self { | ||
CollatorRestrictions { | ||
collator_peer_ids: BTreeSet::new(), | ||
restriction_kind: CollatorRestrictionKind::Preferred, | ||
} | ||
} | ||
|
||
/// Create a new `CollatorRestrictions`. | ||
pub fn new( | ||
collator_peer_ids: BTreeSet<OpaquePeerId>, | ||
restriction_kind: CollatorRestrictionKind, | ||
) -> Self { | ||
CollatorRestrictions { collator_peer_ids, restriction_kind } | ||
} | ||
|
||
/// Is peer_id allowed to collate? | ||
#[cfg(feature = "std")] | ||
pub fn can_collate(&self, peer_id: &PeerId) -> bool { | ||
self.collator_peer_ids.is_empty() || | ||
match self.restriction_kind { | ||
CollatorRestrictionKind::Preferred => true, | ||
CollatorRestrictionKind::Required => { | ||
let peer_id = OpaquePeerId(peer_id.to_bytes()); | ||
self.collator_peer_ids.contains(&peer_id) | ||
}, | ||
} | ||
} | ||
} | ||
|
||
/// How to apply the collator restrictions. | ||
#[derive(Clone, Encode, Decode, PartialEq, TypeInfo, RuntimeDebug)] | ||
pub enum CollatorRestrictionKind { | ||
/// peer ids mentioned will be preferred in connections, but others are still allowed. | ||
Preferred, | ||
/// Any collator with a `PeerId` not in the set of `CollatorRestrictions` will be rejected. | ||
Required, | ||
} | ||
|
||
/// An entry tracking a paras | ||
#[derive(Clone, Encode, Decode, TypeInfo, PartialEq, RuntimeDebug)] | ||
pub struct ParasEntry { | ||
/// The `Assignment` | ||
pub assignment: Assignment, | ||
/// Number of times this has been retried. | ||
pub retries: u32, | ||
} | ||
|
||
impl From<Assignment> for ParasEntry { | ||
fn from(assignment: Assignment) -> Self { | ||
ParasEntry { assignment, retries: 0 } | ||
} | ||
} | ||
|
||
impl ParasEntry { | ||
/// Create a new `ParasEntry`. | ||
pub fn new(assignment: Assignment) -> Self { | ||
ParasEntry { assignment, retries: 0 } | ||
} | ||
|
||
/// Return `Id` from the underlying `Assignment`. | ||
pub fn para_id(&self) -> Id { | ||
self.assignment.para_id | ||
} | ||
|
||
/// Return `CollatorRestrictions` from the underlying `Assignment`. | ||
pub fn collator_restrictions(&self) -> &CollatorRestrictions { | ||
&self.assignment.collator_restrictions | ||
} | ||
} | ||
|
||
/// What is occupying a specific availability core. | ||
#[derive(Clone, Encode, Decode, TypeInfo, RuntimeDebug)] | ||
#[cfg_attr(feature = "std", derive(PartialEq))] | ||
pub enum CoreOccupied { | ||
/// The core is not occupied. | ||
Free, | ||
/// A paras. | ||
Paras(ParasEntry), | ||
} | ||
|
||
impl CoreOccupied { | ||
/// Is core free? | ||
pub fn is_free(&self) -> bool { | ||
match self { | ||
Self::Free => true, | ||
Self::Paras(_) => false, | ||
} | ||
} | ||
/// A parathread. | ||
Parathread(ParathreadEntry), | ||
/// A parachain. | ||
Parachain, | ||
} | ||
|
||
/// A helper data-type for tracking validator-group rotations. | ||
|
@@ -1062,11 +951,10 @@ impl<H, N> OccupiedCore<H, N> { | |
#[derive(Clone, Encode, Decode, TypeInfo, RuntimeDebug)] | ||
#[cfg_attr(feature = "std", derive(PartialEq))] | ||
pub struct ScheduledCore { | ||
// TODO: Is the same as Assignment | ||
/// The ID of a para scheduled. | ||
pub para_id: Id, | ||
/// The collator restrictions. | ||
pub collator_restrictions: CollatorRestrictions, | ||
/// The collator required to author the block, if any. | ||
pub collator: Option<CollatorId>, | ||
} | ||
|
||
/// The state of a particular availability core. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We discussed somewhere that such transformations in the client are not encouraged and we prefer doing this in the subsystem. Unfortunately I don't remember the exact reasons.
For your particular case I think it's okay though. Depending on the runtime api you call either the new or the old version of the function. And you do this workaround to avoid the
changed_in
issues with the runtime api.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Which file would this ideally go to then? What exactly is the subsystem again? I'm only aware of the node/client and runtime distinction.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Subsystem is a module (lets say) from the node - e.g.
approval-voting
,dispute-coordinator
etc.You can put a function in any helpers module (e.g. https://github.com/paritytech/polkadot/blob/dbae30efe080a1d41fe54ef4da8af47614c9ca93/node/subsystem-util/src/runtime/mod.rs) and call it each time you need the availability_cores.
But I'd suggest not to touch it for now.