This repository was archived by the owner on Nov 15, 2023. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Undo v4 changes, move them to vstaging #7103
Open
ghost
wants to merge
26
commits into
alex/parathreads_review
Choose a base branch
from
alex/parathreads_vstaging
base: alex/parathreads_review
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
26 commits
Select commit
Hold shift + click to select a range
b6b9f8e
Undo v4 changes, move them to vstaging
c76ce40
Merge branch 'alex/parathreads_review' into alex/parathreads_vstaging
e2d5869
minor
fc276d6
Merge branch 'alex/parathreads_review' into alex/parathreads_vstaging
7baa04f
Merge branch 'alex/parathreads_review' into alex/parathreads_vstaging
94d24d5
Fix imports
f943c1a
Merge branch 'alex/parathreads_review' into alex/parathreads_vstaging
59f6bc2
Merge branch 'alex/parathreads_review' into alex/parathreads_vstaging
d65d96e
include version dispatch on availablity_cores() call
cbfa7a1
ifmt
2471334
Fix test-runtime
678b1d8
fix node runtime api test
13fa655
Undo RuntimeDebug -> Debug changes in v4
2cf57a8
runtime-api tests fix
4d01e65
Merge branch 'alex/parathreads_review' into alex/parathreads_vstaging
af57dd0
Fix removing api version by accident
b413cc7
Merge branch 'alex/parathreads_review' into alex/parathreads_vstaging
6fae5a3
Remove naked unwrap()s
a665df0
Add availability_cores_on_demand to mock and use that in tests
42bc527
Merge branch 'alex/parathreads_review' into alex/parathreads_vstaging
ea8a0c6
Merge branch 'alex/parathreads_review' into alex/parathreads_vstaging
a198b95
Merge branch 'alex/parathreads_review' into alex/parathreads_vstaging
918243e
Address PR comments
49ebc7d
Rename availability_cores_on_demand to ..._vstaging
c49582c
pebkac
6273880
minor refactor
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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)] | ||
|
Contributor
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?
Author
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. | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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_inissues 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-coordinatoretc.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.