This repository has been archived by the owner on Nov 15, 2023. It is now read-only.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
GenesisBuilder
runtime API #14131GenesisBuilder
runtime API #14131Changes from 11 commits
926f828
f5b4cbe
d24fbba
139341a
587770b
78f467b
75c1f73
5756776
1b3f8ab
0b3e084
fcd2ee9
bed8d05
5ae93f6
77c6504
cc91d46
679a970
aa1f77c
43a5d41
82bb745
8ca9b44
f9926bd
706f3f2
b35c443
14a251b
2435e42
8b62fcf
2547761
a1b64de
750ddb2
cfbd0d2
4c615f0
b02e21e
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Naming is still not really good, but yeah... We also need to supply some kind of name or whatever we want to call it to distinguish which genesis config we want to generate. Here we also have different kind of genesis configs: https://github.com/paritytech/substrate/tree/2c3b923423fac829b02842fbb9a0016b55c417df/bin/node/cli/src/chain_spec.rs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe I am missing something, but I don't see the need for extra parameter to construct default
GenesisConfig
.GenesisConfig
of the runtime is just a struct containing allGenesisConfig
s pallets, so all required parametrization is already self-contained in config (AFAIU it).e.g. the generated
GenesisConfig
forkitchensink-runtime
:What I see in chain_spec.rs are different
ChainSpec
s: development_config, local_testnet_config.Each of them has specific
genesis
: local_testnet_genesis, development_config_genesis. Those specific gensis are just differently parametrizedGenesisConfig
instances ofkitchensink_runtime
created in shared function testnet_genesis.With new
GenesisBuilder
API approach, that customGenesisConfig
will be embedded in the form of pre-configured json files customized on top of thejson
for defaultGensisConfig
.If I missed something, would you please be more precise on where the extra parameter is actually required?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you give an example of this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Examples for
test-runtime
.What I mean by default
GenesisConfig
:substrate/test-utils/runtime/src/lib.rs
Line 1340 in e147d24
What I mean by
GenesisConfig
customised on top of default:substrate/test-utils/runtime/src/test_json/default_genesis_config.json
Line 1 in e147d24
Since we won't have
GenesisConfig
on the native side of the node (at least this is my current understanding), we will not be able to hard-code theGenesisConfig
in the form of rust variable. The alternative for this will be json string.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
substrate/bin/node/cli/src/chain_spec.rs
Lines 297 to 379 in 9d0200b
This is the stuff I'm "worried" about.
But I think we should be able to solve this with some tricks. However this would require to write there some
json
code, but I think this is doable.I imagine something like this:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BTW this
json
macro already exists: https://docs.rs/serde_json/latest/serde_json/macro.json.htmlThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added in: 679a970
We would not need to put this functionality into the runtime, we can easily implement it outside the runtime, using functions already defined:
However I do understand that
build_with_patch
is a nice convenient function.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.