Fix people chain spec parsing in chainspec generator #305
Conversation
chain-spec-generator/src/common.rs
Outdated
| @@ -90,6 +90,8 @@ pub fn from_json_file(filepath: &str, supported: String) -> Result<Box<dyn Chain | |||
| Ok(Box::new(GluttonKusamaChainSpec::from_json_file(path)?)), | |||
| x if x.starts_with("encointer-kusama") => | |||
| Ok(Box::new(EncointerKusamaChainSpec::from_json_file(path)?)), | |||
| x if x.starts_with("people-kusama") => | |||
There was a problem hiding this comment.
Hmm, I just wonder why coretime-kusama is also missing, could you add it also?
There was a problem hiding this comment.
I think we could extract these IFs for chain-id matching out of the common.rs (it is not really common) to the main.rs and e.g. join it with supported networks cfg here: https://github.com/polkadot-fellows/runtimes/blob/main/chain-spec-generator/src/main.rs#L39
|
slightly unrelated, why do we even need to maintain the chain spec generator in the runtime repo? it is only going to be used once (for production runtimes). for dev/testing purpose, we will use the westend/rococo runtime in the polkadot-sdk repo besides, why do we need another chain spec generator at all? to my understanding, the polkadot-sdk already provides a chain spec generator cli and it should work. there will be additional chain spec editing needed but it can be just a simple script |
Maybe I am missing something, but I think we won't need it, but we need to bump sp-genesis-builder with 'get_preset' feature first, which I also need here: #298 |
|
I only spend 2 minutes reading the related code so my understanding could be wrong. But the preset feature appears to be overengineered to me. Why the runtime needs to know some network preset? If you check my poc runtime, it is using a version of polkadot-sdk without the preset feature. All I need to do is use jq or whatever tool to edit the chainspec json to do the modification needed and that's it. Yeah we don't get nice Rust type check but does it matter? again, it is one off thing and the chainspec generator can validate the chainspec json. |
That's not a universal solution. I'm currently doing experiments with |
|
we have a chain spec generator in polkadot-sdk and I see no reason why it wouldn't be enough |
|
Review required! Latest push from author must always be reviewed |
|
@bkontur you mean something like this? (see the last changes) |
Yes, nice, exactly. With actual chain-spec-generator setup, at least we won't forget to add this stuff when adding new chains. |
Yes, the chain spec generator here was introduced when the feature wasn't ready yet in the SDK repo. As the chain spec generator there is now ready, we could start using it here.
Because people can do modifications and start the runtime easily using a given genesis. It is basically the same feature as before when running a node with |
I think it was just forgotten when people chain was added.