Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Cumulus Runtimes should check inherents and consensus seals in execute_block as well as validate_block #2436

Open
rphmeier opened this issue Apr 6, 2023 · 1 comment
Labels
I4-annoyance The node behaves within expectations, however this “expected behaviour” itself is at issue. I7-refactor Code needs refactoring.

Comments

@rphmeier
Copy link
Contributor

rphmeier commented Apr 6, 2023

Currently, Cumulus runtimes are designed to check inherents and consensus seals in the validate_block entry point used by relay-chain validators to execute the block. But the execute_block entry point used by full nodes when importing blocks doesn't do these checks, meaning that full nodes have to reimplement them on the client-side.

It should be standard practice and easy to set up a Cumulus runtime which always does all of these checks in both execute_block and validate_block. With that, import queues could be removed entirely from Cumulus collator consensus implementations.

ref #2301 (comment)

@rphmeier rphmeier added I4-annoyance The node behaves within expectations, however this “expected behaviour” itself is at issue. I7-refactor Code needs refactoring. labels Apr 6, 2023
@bkchr
Copy link
Member

bkchr commented Apr 11, 2023

For consensus seals we already got: paritytech/polkadot-sdk#63

For the inherent, I'm with you and we should probably remove check_inherent completely from parachains at all. The only inherent that can be checked in some reasonable way is currently the timestamp only and that will not change. I think in the future with async backing and building on prospective chains we can not provide a reasonable timestamp anymore. Thus, the timestamp should only be allowed to update when we are building on a fresh relay chain block. All parachain blocks that are building on the same relay chain block should be using the same timestamp. We should probably deprecate the usage of pallet-timestamp from parachains and just let parachains-system return the timestamp as Option that is only some if the parachain block was build on a new relay chain block as described above.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
I4-annoyance The node behaves within expectations, however this “expected behaviour” itself is at issue. I7-refactor Code needs refactoring.
Projects
None yet
Development

No branches or pull requests

2 participants