paritytech / polkadot-sdk

The Parity Polkadot Blockchain SDK
https://polkadot.com/
1.91k stars 704 forks source link

Improve parachains node `--dev` flag experience #6537

Open iulianbarbu opened 3 days ago

iulianbarbu commented 3 days ago

Is there an existing issue?

Experiencing problems? Have you tried our Stack Exchange first?

Motivation

There are a few issues with the current flow of how --dev flag works with parachain development:

  1. Starting omni node in development mode with --dev isn't supported because --dev and --chain can not be both provided at the same time to the node.
  2. Starting parachain-template-node with --dev requires polkadot-sdk to be built with a dependency on polkadot-cli?/rococo-native, since the dev chain spec of the polkadot-template-node uses rococo-local as relay chain.
  3. My understanding (based on a quick experiment) is also that polkadot-omni-node and polkadot-template-node would require a working relay chain network (1 validator at least? to be clarified), which if not created before the parachain node startup will result in block authoring/finalization issues.

Request

Unify & simplify the experience of how --dev flag works for OmniNode and regular parachain development - for what regular means we can refer to parachain-template-node and using it with --dev flag.

parachain-template-node might phase out with time, and only OmniNode usage might be relevant, but I think fixing/simplifying the --dev outcome for the parachain-template-node would be useful to ensure that all substrate/cumulus/omni-node/templates built-in behaviors converge when using --dev flag.

Solution

  1. Enable both --dev & --chain flags to work together for OmniNode, keeping in mind the experience shouldn't be very different from what we settle on as parachain-template-node --dev behavior and side effects, after we clarify how to approach point 3.
  2. Make polkadot-sdk (umbrella crate) std feature depend on polkadot-cli?/rococo-native, to enable the --dev flag usage with parachain-template-node.
  3. Not very sure about this one yet, but an opinion I've seen here is to defer to using a manual seal when passing --dev. Manual seal requires setting a block time though, and we could hardcode it to 3000ms (as I've seen in practice used in a few places). Open to other suggestions though. Other opinions I've seen were to depend on zombienet to handle --dev flag accordingly (as in detecting that parachain node has such a flag and setup a relaychain network accordingly), or make a similar thing within the node itself, or mock relay chain network dependencies (which needs more scoping).

I will follow up with a more concise plan here once we'll confirm the problem statement looks right, and the solutions are sufficiently scoped for a decision.

Are you willing to help with this request?

Yes!

bkchr commented 2 days ago

2. Make polkadot-sdk (umbrella crate) std feature depend on polkadot-cli?/rococo-native, to enable the --dev flag usage with parachain-template-node.

This should not be required when you use manual-seal, as you will not require the relay chain.

3. Manual seal requires setting a block time though, and we could hardcode it to 3000ms (as I've seen in practice used in a few places)

Not sure for what you need a block time, because manual seal only does something when you ask it to do something. At some point we had an issue that manual seal actually build blocks automatically when there isn't a tx send for some time, but we never implemented it. If you need some block time, you could try to fetch it from Aura_slot_duration.

DrW3RK commented 2 days ago

For point 3:

Ideally, there should be a flag to see the parachain in action, like a solochain, and test the runtime features that have been developed. A parachain runtime developer could use this and just focus on the runtime development and testing, without running any validator in the background.

xlc commented 1 day ago

runtime devs can just use chopsticks

DrW3RK commented 1 day ago

@xlc Fair point. It would be nice to integrate Chopsticks into the node repo and be invoked with a flag.

iulianbarbu commented 1 day ago

This should not be required when you use manual-seal, as you will not require the relay chain.

I am fine with scratching 2 in the context of enabling manual seal on parachain-template-node side when used with --dev.

Not sure for what you need a block time, because manual seal only does something when you ask it to do something.

Yes, for OmniNode and minimal-template-node we tell it to do something with a timer which triggers the block sealing continuously at certain intervals.

If you need some block time, you could try to fetch it from Aura_slot_duration

I will check it out, we should be able to assume that OmniNode works with runtimes that use AURA only (for the time being) and parachain-template-runtime already uses it. If AURA isn't configured, we could emit a warning and default to a value (but we can also standardize determining block times based on common usecases other than AURA, and the default choice - or configurable block time, which I wouldn't worry for right now - would be for niche use cases). WDYT @bkchr ?

bkchr commented 1 day ago

runtime devs can just use chopsticks

I'm telling this to people as well ;)

If AURA isn't configured, we could emit a warning and default to a value (but we can also standardize determining block times based on common usecases other than AURA, and the default choice - or configurable block time, which I wouldn't worry for right now - would be for niche use cases). WDYT @bkchr ?

Sounds good to me

iulianbarbu commented 1 day ago

@xlc @DrW3RK I am not super familiar with chopsticks. My understanding is that it needs a live chain and it will fork its current state and work with it locally. In the case of non-live chain, I think we should start a local chain/node based on the runtime we want to test, and then point chopsticks to that (and then, to start the local node, we use --dev, which behind the scenes uses manual seal). LMK if such setup makes sense to you at least for the start. If it makes sense, I can focus on this DX for the scope of this issue.

It would be nice to integrate Chopsticks into the node repo and be invoked with a flag.

Integrating chopsticks more into the node, by instructing it via a flag to start in --dev mode and also start chopsticks with the correct configuration that points to it can be another follow up. I would focus in this issue on fixing --dev to some extent for both OmniNode and parachain-template-node (and possibly how it is perceived in substrate/cumulus), and optionally add chopsticks docs for usage with a local node started with --dev in the parachain-template. WDYT?

xlc commented 1 day ago

you can run a dev chain from chain spec using chopsticks. we may need to improve the docs but there isnt much reason to use omni node when developing runtime

iulianbarbu commented 1 day ago

you can run a dev chain from chain spec using chopsticks. we may need to improve the docs but there isnt much reason to use omni node when developing runtime

I see. In this case I am not super sure why --dev is needed. It might be for some non-runtime development related reason. @bkchr any idea?

iulianbarbu commented 2 hours ago

you can run a dev chain from chain spec using chopsticks. we may need to improve the docs but there isnt much reason to use omni node when developing runtime

I see. In this case I am not super sure why --dev is needed. It might be for some non-runtime development related reason. @bkchr any idea?

Leaving here a brain dump with my perspective over --dev, which I am hoping it will also help with alignment:

Regular OmniNode usage with --dev

  1. If people use the shipped polkadot-omni-node binary as is they are probably more interested in runtime development, and for this case chopsticks makes a better debugging companion. We should direct people to chopsticks in this case.

  2. polkadot-omni-node has the --dev-block-time flag which makes the node start with manual seal. This overlaps with our intention of making --dev start the node with manual seal. To not have two flags doing the same thing I am thinking about deprecating --dev-block-time and rely on --dev to achieve the same thing but slightly better (since we'd use AURA config to determine the block-time that is used to set the intervals when block sealing happens, and if not present, set a default for it, and possibly in the future if needed, make this default configurable by passing it in the chain-spec file).

  3. Then there is polkadot-omni-node-lib case, where people develop certain nodes when polkadot-omni-node isn't enough, so in this case --dev makes more sense, and if used, it should act similarly to how polkadot-omni-node acts, but it could be extended by the node devs as they need.

Other --dev usage (e.g. parachain-template-node)

--dev is part of SharedParams struct in substrate/client/cli/config::CliConfiguration, and is used across multiple places. We can understand its scope by following & understanding its usage in these multiple locations. I think we can document this, and even go for a node/runtime development workflows kind of doc, at least in the templates.