Use para id in specs instead of flag

[joelamouche]

• Parachain id flag is not supported anymore and should be included in genesis specs
• Testing sequence will now use the specs to decide on chain id
• Added a test that isn't included in the ci with two parachains to test ids
• remove chain option from collator spawning and genesis export

tackles #151 #156

[joelamouche]

@shawntabrizi ready for review

[bkchr]

Imo we should entirely remove the paraid from here. Some projects may still support the cli argument or whatever. However, I'm also planing to remove the Para id by default from the chain spec.

[joelamouche]

However, I'm also planing to remove the Para id by default from the chain spec

@bkchr So how does one determine their parachain id?

[bkchr]

You hard code it in the runtime for example.

[riusricardo]

Why do we need to remove chain selection from the parachain parameters? It will cause issues when you export the genesis state.

[joelamouche]

According to a comment, it was't supported by cumulus anymore

[riusricardo]

I think this was a misunderstanding. The only thing that got removed was the parachain-id paritytech/cumulus#739
No need to remove the option to select a chain. It is very needed, in polkadot-collator we have several chains, the same stands for the polkadot binary.

[ghzlatarev]

Indeed. How was this resolved, perhaps I'm missing something @joelamouche ?

[joelamouche]

@ghzlatarev I left the --chain command. So if you specify the chain in the chain field of the parachain in hte config, it should work

[ghzlatarev]

@joelamouche I don't see it in startCollator or exportGenesisWasm , is it not needed there ?
Right now if I start a westmint-local network I get some default Local Testnet

[ghzlatarev]

@joelamouche i hit this code path https://github.com/paritytech/cumulus/blob/master/polkadot-parachains/src/command.rs#L119 -> https://github.com/paritytech/cumulus/blob/master/polkadot-parachains/src/chain_spec.rs#L71

[joelamouche]

You're right Im adding it back

[joelamouche]

You hard code it in the runtime for example.

@bkchr but what if I want to run the same runtime on 2 different ids?
In the moonbeam tests, we are running two copies of moonbase runtime. Would they automatically have different parachain ids if I removed the parachain id fro the specs?

[bkchr]

You hard code it in the runtime for example.

@bkchr but what if I want to run the same runtime on 2 different ids? In the moonbeam tests, we are running two copies of moonbase runtime. Would they automatically have different parachain ids if I removed the parachain id fro the specs?

This is completely up to the parachain developer. Aka if you want to support two different parachain ids, I see no reason why that should not be doable. However, the official code doesn't need to provide this, because it only confuses people.

[joelamouche]

OK that seems a pretty big refactor to let's leave it to a follow up PR @bkchr ?
I've started to try to implement that but the resolvedId is used in the addGenesisParachain function in spec.ts file and using an incremental index doesnt seem to work. Maybe you have an idea why @shawntabrizi

[shawntabrizi]

why are you passing an empty object to changeGenesisConfig?

[joelamouche]

Oh yeah I forgot to remove that :s

[shawntabrizi]

@joelamouche seems the code is just wrong in the case of a resolvedId? Otherwise, not sure why it wouldn't work.

[joelamouche]

@riusricardo this is the comment I was referring to. I'm going to put it back though, for collators that do need it

[joelamouche]

@shawntabrizi ready for review

[jimmychu0807]

hmmm.. didn't aware you guys are working on this...

I am working on #160

[shawntabrizi]

the config.json has a chain parameter that you specify for something like that