omni-node: --dev sets manual seal and allows --chain to be set #6646
Conversation
Signed-off-by: Iulian Barbu <[email protected]>
iulianbarbu
added
T9-cumulus
Signed-off-by: Iulian Barbu <[email protected]>
Signed-off-by: Iulian Barbu <[email protected]>
Signed-off-by: Iulian Barbu <[email protected]>
Signed-off-by: Iulian Barbu <[email protected]>
Signed-off-by: Iulian Barbu <[email protected]>
iulianbarbu
changed the title
Signed-off-by: Iulian Barbu <[email protected]>
Signed-off-by: Iulian Barbu <[email protected]>
Signed-off-by: Iulian Barbu <[email protected]>
Signed-off-by: Iulian Barbu <[email protected]>
templates/parachain/README.md
Outdated
|
|
||
| ### Start `chopsticks` with the chain spec | ||
|
|
||
| We'll also need to set `--allow-unresolved-imports=true` because the `parachain-template-runtime` is not built |
There was a problem hiding this comment.
From what I understand it is the otherway round. You tried to use chopsticks earlier with a runtime which was built using --features runtime-benchmarks. This makes the runtime require additional host functions. You are telling chopsticks to ignore these hostfunctions by using this flag.
So if users build their runtimes without --features runtime-benchmarks they should not require this flag.
There was a problem hiding this comment.
Ah good catch, thanks! Actually just tested and we don't need the --allow-unresolved-imports flag because on normal build the parachain-template-runtime doesn't build with --features runtime-benchmarks. I built the runtime before with the runtime-bnechmarks feature and thought it is the std/non-benchmarks runtime. 😅
There was a problem hiding this comment.
updated the readme here: f696909
| #[arg(long)] | ||
| /// a block each `dev_block_time` ms, as if it was part of a parachain. Default to 3000ms if | ||
| /// not set when using `--dev` flag. | ||
| #[arg(long, requires = "dev")] |
There was a problem hiding this comment.
Hmm why does this require --dev? If I just specify --dev-block-time 500 it could just work like before no? Basically --dev just being a shortcut.
There was a problem hiding this comment.
It is because using just --dev-block-time <number_ms> isn't equivalent to --dev --dev-block-time <number_ms>. Missing --dev will not set a bunch of flags accordingly: --rpc-cors=all, fore-authoring, --alice and --tmp. Not sure how important they are (as in I haven't explored precisely to what end they serve), but thought about keeping the --dev experience complete.
There was a problem hiding this comment.
Maybe I am missing something, but --dev can keep doing this stuff. But I could still be able to specify --dev-block-time on its own like before.
There was a problem hiding this comment.
Maybe I am missing something, but
--devcan keep doing this stuff. But I could still be able to specify--dev-block-timeon its own like before.
Yes, it can still do its stuff, but that stuff wouldn't be activated if we're not setting the --dev flag, right? I mean, we can also set it artificially like below:
cli.run.base.shared_params.dev |= cli.dev_block_time.is_some();Is this what you think about?
There was a problem hiding this comment.
Yes exactly. I mean this is what I meant by "old behaviour". The check was changed in this PR, before we were checking for dev_block_time already.
So basically my idea is:
--dev: Activates manual seal with default 3000ms block time. And all the other flags you mentioned, basically easy dev setup.--dev-block-time 1000: Just manual seal with 1000ms, no extra hidden flags activated.--dev --dev-block-time 1000: The easy dev setup but with 1000ms block times.
Let me know if you think it makes sense.
There was a problem hiding this comment.
Yes, I am ok with this. Changed it back here: 06bbd5e, and adjusted a few more things to reflect that --dev isn't mandatory.
Signed-off-by: Iulian Barbu <[email protected]>
Signed-off-by: Iulian Barbu <[email protected]>
Description
This PR changes a few things:
--devflag will not conflict with--chainanymore, but if--chainis not given will set--chain=dev.--dev-block-timeis optional and it defaults to 3000ms if not set after setting--dev.--dev.Closes: #6537
Integration
Relevant for node/runtime developers that use OmniNode lib, including
polkadot-omni-nodebinary, although the recommended way for runtime development is to usechopsticks.Review Notes
parachain-template-nodeas is (meaning--devisn't usable and testing a runtime with theparachain-template-nodestill needs a relay chain here). I am doing this because I think we want either way to phase outparachain-template-nodeand adding manual seal support for it is wasted effort. We might add support though if the demand is forparachain-template-node.--dev-block-time. Also, would want first to align & merge on runtime metadata checks we added in Omni Node here: omni-node: add metadata checks for runtime/parachain compatibility #6450 before starting to infer AURA config slot duration via the same way.--devnow.