Remove parachain-id cli command

[bkchr]

This was never planned to be a permanent feature. This was mainly added for testing purposes, but
now was copied by everybody. The users should be more specific about the para id and set this
properly in the chain spec.

[xlc]

This is still a useful feature for testing. e.g. when doing e2e XCM testing, we need multiple instances of a parachain with different parachain id.

Maybe rename it to test-only-parachain-id or something?

[riusricardo]

This is still a useful feature for testing. e.g. when doing e2e XCM testing, we need multiple instances of a parachain with different parachain id.

Maybe rename it to test-only-parachain-id or something?

I see the reasoning to keep this for testing purposes. The name change might be enough to break functionality and we update the docs to prevent people from using it in production.

[riusricardo]

LGTM and I support the change to prevent more issues with incorrect parachain IDs.
@xlc has a good point but I'm not sure if this is the best way to handle a test case.

[bkchr]

This is still a useful feature for testing. e.g. when doing e2e XCM testing, we need multiple instances of a parachain with different parachain id.

Maybe rename it to test-only-parachain-id or something?

If you need this in your parachain, you can just create your own RunCmd etc that does the same as we are doing here with the #[flatten] attribute. Then you can add whatever cli args you want.

[bkchr]

What I mean is that you can just wrap this another time:

cumulus/client/cli/src/lib.rs

Line 123 in 0b492ba

pub struct RunCmd {

[h4x3rotab]

Generally speaking, it's not a good idea to use --parachain-id at all. We can have some alternative, and the one Khala is using is pretty wise IMO.

The parachain id is burned into the chain-spec, as a part of the genesis storage. For production parachains, the id should definitely be fixed in the chain spec generation code, as well as the raw chain spec file embedded in the node client. For test parachains, it still holds: the parachain id should still be tied to the chain spec once it's generated.

Therefore I made this PR to skip the parachain id parameter and read the id from the storage whenever possible:
paritytech/polkadot-launch#102

The case @xlc mentioned is mainly for the on-the-fly chain-spec generation for local testnet. A lot of parachains used to have this parameter, passing to the chain spec generation function to write the parachain id to the storage. However this is problematic. In Substrate a lot of code assumes each chain spec should have an unique name, and if you give a different --parachain-id you just shot yourself in the foot.

So what to do instead for the on-the-fly chain-spec generation? My suggestion is to allow a dynamic chain name. At Phala, we allow to specify a chain name like "khala-dev-2004". The "-2004" prefix indicating the parachain id for this chain spec instance. In the code, it parses the parachain id from the name, and generate the chain spec based on it. In this way, it's very easy to distinguish the parachain instances with different parachain id.

Sample code here:

https://github.com/Phala-Network/khala-parachain/blob/3c5de0a5035b512136e53b1afb812b9288d36e02/node/src/command.rs#L36-L92