Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

omni-node: --dev sets manual seal and allows --chain to be set #6646

Open
wants to merge 19 commits into
base: master
Choose a base branch
from

Conversation

iulianbarbu
Copy link
Contributor

@iulianbarbu iulianbarbu commented Nov 26, 2024

Description

This PR changes a few things:

  • --dev flag will not conflict with --chain anymore, but if --chain is not given will set --chain=dev.
  • --dev-block-time is optional and it defaults to 3000ms if not set after setting --dev.
  • to start OmniNode with manual seal it is enough to pass just --dev.

Closes: #6537

Integration

Relevant for node/runtime developers that use OmniNode lib, including polkadot-omni-node binary, although the recommended way for runtime development is to use chopsticks.

Review Notes

  • Decided to focus only on OmniNode & templates docs in relation to it, and leave the parachain-template-node as is (meaning --dev isn't usable and testing a runtime with the parachain-template-node still needs a relay chain here). I am doing this because I think we want either way to phase out parachain-template-node and adding manual seal support for it is wasted effort. We might add support though if the demand is for parachain-template-node.
  • Decided to not infer the block time based on AURA config yet because there is still the option of setting a specific block time by using --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.
  • update the docs to mention --dev now.
  • mention about chopsticks in the context of runtime development

@iulianbarbu iulianbarbu added T9-cumulus This PR/Issue is related to cumulus. T17-Templates This PR/Issue is related to templates labels Nov 26, 2024
@iulianbarbu iulianbarbu self-assigned this Nov 26, 2024
Signed-off-by: Iulian Barbu <[email protected]>
Signed-off-by: Iulian Barbu <[email protected]>
@iulianbarbu iulianbarbu marked this pull request as draft November 26, 2024 10:48
@iulianbarbu iulianbarbu changed the title omni-node: --dev starts manual seal and allows --chain to be set omni-node: --dev sets manual seal and allows --chain to be set Nov 26, 2024
@iulianbarbu iulianbarbu marked this pull request as ready for review November 27, 2024 00:37
Signed-off-by: Iulian Barbu <[email protected]>
@iulianbarbu iulianbarbu requested review from michalkucharczyk and a team November 27, 2024 13:43
templates/parachain/README.md Outdated Show resolved Hide resolved
cumulus/polkadot-omni-node/lib/src/cli.rs Outdated Show resolved Hide resolved
@@ -128,7 +128,8 @@ pub struct Cli<Config: CliConfig> {
///
/// This is a dev option, and it won't result in starting or connecting to a parachain network.
/// The resulting node will work on its own, running the wasm blob and artificially producing
/// a block each `dev_block_time` ms, as if it was part of a parachain.
/// a block each `dev_block_time` ms, as if it was part of a parachain. Defaults to 3000ms if
/// not set and `--dev` is used.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would be more explicit here about enabling manual sealing and fact that --dev enables this.

/// Start a dev node that produces a block each `dev_block_time` ms.
///
/// This is a dev option. It enables a manual sealing, meaning blocks are produced manually 
/// rather than being part of an actual network consensus process. Using the option won't 
/// result in starting or connecting to a parachain network. The resulting node will work on
/// its own, running the wasm blob and artificially producing a block each `dev_block_time` ms, 
/// as if it was part of a parachain. 
///
/// The `--dev` flag sets the `dev_block_time` to a default value of 3000ms unless explicitly provided.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Used your phrasing here: d6faae7

#[arg(long, conflicts_with_all = &["chain"])]
/// This flag sets `--chain=dev`, `--force-authoring`, `--rpc-cors=all`, `--alice`, and `--tmp`
/// flags, unless explicitly overridden. It also disables local peer discovery (see `--no-mdns`
/// and `--discover-local`).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we mention manual seal here? (I know it does not enable it for every node, but I think it would be good to have this information in cli).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a note here: d6faae7.

Signed-off-by: Iulian Barbu <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T9-cumulus This PR/Issue is related to cumulus. T17-Templates This PR/Issue is related to templates
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve parachains node --dev flag experience
3 participants