-
Couldn't load subscription status.
- Fork 2.7k
chain-spec: support for json config/patch (and GenesisBuilder API)
#14562
base: master
Are you sure you want to change the base?
chain-spec: support for json config/patch (and GenesisBuilder API)
#14562
Conversation
- json test added (wip)
- ChainSpecBuilder used - tests using RuntimeGenesisConfig added to verify patching approach
18330f8 to
fc55a8d
Compare
|
bot clean |
| /// accounts. Only works for kitchen-sink runtime | ||
| #[derive(Parser, Debug)] | ||
| #[command(rename_all = "kebab-case")] | ||
| pub struct NewCmd { |
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.
IMO we could remove it as it works only with kitchensink runtime.
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.
I think that can be removed.
If I understood correctly the same result can be accomplished by:
- using the
generatecommand to build the patch - then using the
runtimeto patch the kitchensink wasm (passed from the command line)
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.
But I also noted that the GenerateCmd also uses generate_chain_spec. And thus also uses the kitchensink runtime.
What abut NOT using the kitchensink runtime at all here? I.e. use this Generate command just to build a patch without the code?
But this also requires the ChainSpecBuilder::with_code to be optional (optional code field for patches)
Co-authored-by: Sebastian Kunert <[email protected]>
|
bot fmt |
|
bot rebase |
…ort-for-genesis-builder-api
|
bot clean |
| }, | ||
| balances: BalancesConfig { | ||
| ) -> serde_json::Value { | ||
| serde_json::json!({ |
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.
By using serde_json::Value we are loosing the static type checking advantages we can have when using the types defined by the native runtime (i.e. RuntimeGenesisConfig).
(Maybe I'm going to say something stupid, not 100% confident with metadata) but isn't possible to have some sanity check using the runtime metadata?
Maybe for this we have to investigate (scale_info::meta_type::<Runtime>())
bin/node-template/runtime/Cargo.toml
Outdated
| ] | ||
|
|
||
| #Enabling this flag will disable GenesisBuilder API implementation in runtime. | ||
| disable-genesis-builder = [] |
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.
Just an idea to open a discussion (not saying this is better).
What about instead using a feature called genesis-builder that is part of the default = ["std", "genesis-builder"]?
Then define genesis-builder feature to also enable serde_json and sp-genesis-builder which may be set as optional dependencies.
| /// accounts. Only works for kitchen-sink runtime | ||
| #[derive(Parser, Debug)] | ||
| #[command(rename_all = "kebab-case")] | ||
| pub struct NewCmd { |
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.
I think that can be removed.
If I understood correctly the same result can be accomplished by:
- using the
generatecommand to build the patch - then using the
runtimeto patch the kitchensink wasm (passed from the command line)
| /// into `RawGenesis` as `genesis::top::raw::0x3a636f6465` (which is | ||
| /// [`sp_core::storage::well_known_keys::CODE`]). If the spec is already in `raw` format, and | ||
| /// contains `genesis::top::raw::0x3a636f6465` field it will be updated with content of `code` | ||
| /// field (if present). |
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.
Dumb question. Do we care somewhere about preserving field order in the json? Looks not... but I just wanted to open the discussion
|
|
||
| [dependencies] | ||
| codec = { package = "parity-scale-codec", version = "3.6.1", default-features = false, features = ["derive"] } | ||
| json-patch = { version = "1.0.0", default-features = false } |
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.
I'd just add this few lines function in chain-spec/lib.rs and remove the dependency.
IMO adding a dependency to use a trivial function is not good (again, this is my opinion).
| <UpgradedToTripleRefCount<T>>::put(true); | ||
|
|
||
| sp_io::storage::set(well_known_keys::CODE, &self.code); | ||
| sp_io::storage::set(well_known_keys::EXTRINSIC_INDEX, &0u32.encode()); |
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.
Looking at this now I wonder what is the point of setting to 0 the well known EXTRINSIC_INDEX during build.
- maybe we can modify this with an
unwrap_or_default()(thus not returning anOption). - On block initialization this is always set to 0
This PR:
GenesisConfigtoChainSpecallowing interaction with runtimeGenesisBuilderAPI.chain-spec-buildercommand line util,GenesisBuilderAPI in kitchensink runtimecodefromsystem_palletcodeto theChainSpecChainSpec::from_genesis, but also changes the signature of this method extending it withcodeargument.ChainSpec::builder()should be used instead.GenesisBuilderAPI fornode-template-runtimeandkitchensink-runtime.codemoved intoChainSpecHaving the explicit
codefield inChainSpecwould allow for duplication of the wasm code within the raw version of the ChainSpec (as the raw storage may contain wasm code undergenesis::top::raw::0x3a636f6465key).To avoid this redundancy/ambiguity following measure was implemented. When re-exporting the raw version of
ChainSpecthecodefield will be converted intogenesis::top::raw::0x3a636f6465. It will be removed from theChainSpec. If the rawChainSpecalready containsgenesis::top::raw::0x3a636f6465entry, it will be overwritten with the value of thecodefield (if present).Similarly, when building genesis state (actually calling
ChainSpec::assimilate_storage) the existing0x3a636f6465will also be overwritten withcodefield (if present).Example:
{ "name": "TestName", "id": "test_id", "chainType": "Local", ... "genesis": { "raw": { "top": { "0x3a636f6465": "0x010101" }, "childrenDefault": {} } }, "code": "0x060708" }is equivalent of:
{ "name": "TestName", "id": "test_id", "chainType": "Local", ... "genesis": { "raw": { "top": { "0x3a636f6465": "0x060708" }, "childrenDefault": {} } } }JSON based GenesisConfig in
ChainSpec:Patch
The
ChainSpeccan now be built using genesis config JSON patch (which contains some key-value pairs meant to override runtime's genesis config default values). This can be achieved withwith_genesis_patchmethod of the builder:Resulting
ChainSpecinstance can be converted to raw version of chain spec JSON file. This was not changed and can be done withchain_spec.as_json(true)method. Sample output is here. The runtime'sGenesisBuilder::build_configAPI is called during this conversion.The
ChainSpecinstance can also be written to chain spec JSON file in human readable form. The resulting chain spec file will contain the genesis config patch (partial genesis config). Sample output is hereFull Config
It is also possible to build
ChainSpecusing full genesis config JSON (containing all the genesis config key-value pairs). No defaults will be used in this approach. The sample code is as follows:Again, resulting
ChainSpecinstance can be converted to the raw version of chain spec JSON file (which involves callingGenesisBuilder::build_configruntime method) .It can be also stored in human readable version, sample output here.
chain-spec-builderutilNew commands allowing to interact with arbitrary WASM runtime blob were added. Use cases are as follows:
Get default config from runtime
Queries the default genesis config from the provided
runtime.wasmand uses it in the chain spec. Can also store runtime's default genesis config in given file (-d):Note:
GenesisBuilder::create_default_configruntime function is called.Generate raw storage chain spec using genesis config patch
Patches the runtime's default genesis config with provided
patch.jsonand generates raw storage (-s) version of chain spec:Note:
GenesisBuilder::build_configruntime function is called.Generate raw storage chain spec using full genesis config
Build the chain spec using provided full genesis config json file. No defaults will be used:
Note:
GenesisBuilder::build_configruntime function is called.Generate human readable chain spec using genesis config patch
Note: No runtime API is called.
Generate human readable chain spec using full genesis config
Note: No runtime API is called.
Some extra utils:
verify: allows to verify if human readable chain spec is valid (precisely: all required fields in genesis config are initialized),edit, allows to:Some open questions/issues:
naming.with_no_genesis_defaults+ in chain spec json keys:JsonPatch/JsonFull,.GenesisSourcesource for patch and full configNew/Generatecommands in `chain-spec-builder? (IMO we can remove them).Step towards: paritytech/polkadot-sdk#25
polkadot companion: paritytech/polkadot#7508
cumulus companion: paritytech/cumulus#2936