Expose ClaimQueue via a runtime api and use it in collation-generation
#3580
Conversation
…rom the data provided with `request_availability_cores`
tdimitrov
added
the
T8-polkadot
| // TODO [now]: this assumes that next up == current. | ||
| // in practice we should only set `OccupiedCoreAssumption::Included` | ||
| // when the candidate occupying the core is also of the same para. |
There was a problem hiding this comment.
I think this TODO is being solved by this PR, isn't it? There's also a tracking issue here: #3327
There was a problem hiding this comment.
I guess you are right. But it is fixed thanks to the updated implementation of next_up_on_available?
Or I am missing something?
There was a problem hiding this comment.
oh, I think I was wrong actually. I think it should be fixed while implementing #3582
| if has_required_runtime( | ||
| sender, | ||
| relay_parent, | ||
| RuntimeApiRequest::CLAIM_QUEUE_RUNTIME_REQUIREMENT, | ||
| ) | ||
| .await |
There was a problem hiding this comment.
nit: instead of doing two requests (one for the version and another for the actual runtime API), we could directly try calling the runtime API and handle RuntimeApiError::NotSupported
There was a problem hiding this comment.
I see your point but I think it is more or less the same.
Each runtime call has got a version check before it (done by the runtime api subsystem). So in the current case we have got:
Version check (by has_required_runtime) -> Version check (by runtime api subsystem) -> Runtime call
If we call the new api directly on an old runtime and handle the error we will have the same:
Version check (by runtime api subsystem for the 'old call') -> Version check (by runtime api subsystem for the 'new call') -> Runtime call
So in terms of runtime calls - it's the same when we have got old runtime.
For a new runtime we do save one version check but (1) we will remove the version check once everything is released on Polkadot and (2) the runtime calls are cached and never hit the runtime.
Considering these - I'd stick with the current version because it is more explicit.
Another drawback of relying on NotSupported for version check is that the former happens in other weird cases - e.g. a runtime call being executed on pruned blocks. Having NotSupported errors polluting the logs won't be ideal especially when we chase bugs.
There was a problem hiding this comment.
For a new runtime we do save one version check but (1) we will remove the version check once everything is released on Polkadot and (2) the runtime calls are cached and never hit the runtime.
yes, that's exactly my point, saving one version check for a new runtime. We'd also save one round-trip to the runtime API subsystem, regardless of the runtime version.
anyway, I don't have very strong feelings about this
There was a problem hiding this comment.
Your point is valid. I've created an issue for this - #3756
|
The CI pipeline was cancelled due to failure one of the required jobs. |
…tion` (paritytech#3580) The PR adds two things: 1. Runtime API exposing the whole claim queue 2. Consumes the API in `collation-generation` to fetch the next scheduled `ParaEntry` for an occupied core. Related to paritytech#1797
The PR adds two things:
collation-generationto fetch the next scheduledParaEntryfor an occupied core.Related to #1797