Deprecate para_id() from CoreState in polkadot primitives
#3979
Conversation
tdimitrov
added
the
T8-polkadot
prdoc/pr_3979.prdoc
Outdated
|
|
||
| crates: | ||
| - name: polkadot-primitives | ||
| bump: major |
There was a problem hiding this comment.
Not sure about this one. We are removing a pub method so it's should be a major bump?
There was a problem hiding this comment.
I think so, it is a breaking change
There was a problem hiding this comment.
Good point. Let's deprecate it first then. Like: Don't remove now, just deprecated, thus minor bump.
There was a problem hiding this comment.
But it can cause problems if it is used incorrectly? Isn't this worse than breaking other projects because of a potential bug?
There was a problem hiding this comment.
And just in case people are actually using it, let's also fix it. (Returning the scheduled para_id also on an occupied core.) This should not break any code, as up until now this was the same thing.
There was a problem hiding this comment.
@sandreim could you please have another look at the prdoc? Does the updated description make sense?
Co-authored-by: Andrei Sandu <[email protected]>
tdimitrov
changed the title
para_id() from CoreState in polkadot primitivespara_id() from CoreState in polkadot primitives
prdoc/pr_3979.prdoc
Outdated
| doc: | ||
| - audience: "Node Dev" | ||
| description: | | ||
| The motivation is that the old implementation of `para_id()` can be misused with Coretime which allows two |
There was a problem hiding this comment.
Given that we just fixed it, it can not actually be misused anymore. So the only reason for deprecation is that claim queue is replacing it.
With Coretime enabled we can no longer assume there is a static 1:1 mapping between core index and para id. This mapping should be obtained from the scheduler/claimqueue on block by block basis. This PR modifies `para_id()` (from `CoreState`) to return the scheduled `ParaId` for occupied cores and removes its usages in the code. Closes #3948 --------- Co-authored-by: Andrei Sandu <[email protected]>
With Coretime enabled we can no longer assume there is a static 1:1 mapping between core index and para id. This mapping should be obtained from the scheduler/claimqueue on block by block basis. This PR modifies `para_id()` (from `CoreState`) to return the scheduled `ParaId` for occupied cores and removes its usages in the code. Closes #3948 --------- Co-authored-by: Andrei Sandu <[email protected]>
…tech#3979) With Coretime enabled we can no longer assume there is a static 1:1 mapping between core index and para id. This mapping should be obtained from the scheduler/claimqueue on block by block basis. This PR modifies `para_id()` (from `CoreState`) to return the scheduled `ParaId` for occupied cores and removes its usages in the code. Closes paritytech#3948 --------- Co-authored-by: Andrei Sandu <[email protected]>
With Coretime enabled we can no longer assume there is a static 1:1 mapping between core index and para id. This mapping should be obtained from the scheduler/claimqueue on block by block basis.
This PR modifies
para_id()(fromCoreState) to return the scheduledParaIdfor occupied cores and removes its usages in the code.Closes #3948