Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Adjust GenesisBuilder naming #150

Closed
michalkucharczyk opened this issue Jul 24, 2023 · 5 comments · Fixed by #2714
Closed

Adjust GenesisBuilder naming #150

michalkucharczyk opened this issue Jul 24, 2023 · 5 comments · Fixed by #2714
Assignees
@michalkucharczyk
Labels
I4-refactor Code needs refactoring. T1-FRAME This PR/Issue is related to core FRAME, the framework.

Comments

@michalkucharczyk
Copy link
Contributor

michalkucharczyk commented Jul 24, 2023 •
edited
Loading

GenesisBuilder trait needs some naming adjustments.

          LGTM other than a grumble about naming which I hope to see fixed before this is stabilized.

Originally posted by @kianenigma in paritytech/substrate#14310 (review)

Refer also to: paritytech/substrate#14131 (review)

part of: #25
@michalkucharczyk michalkucharczyk mentioned this issue Jul 24, 2023
Merged
@michalkucharczyk michalkucharczyk self-assigned this Jul 24, 2023
@juangirini juangirini transferred this issue from paritytech/substrate Aug 24, 2023
@the-right-joyce the-right-joyce added I4-refactor Code needs refactoring. T1-FRAME This PR/Issue is related to core FRAME, the framework. and removed I7-refactor labels Aug 25, 2023
@kianenigma
Copy link
Contributor

To re-iterate, I have two points:

  1. We are using the word "config", where it is meant to just represent "state". For example fn build_config(config: Vec<u8>, param: Vec<u8>) -> sp_genesis_builder::Result which is now part of all runtimes is doing nothing other than building the initial "state". Why are we calling this "config"? there is no "configuration" involved.
  2. And, the former point is important, because is FRAME Config means something specific, and I worried that people would mix the two.

@JoshOrndorff
Copy link
Contributor

This is relevant to what we are doing in Off-Narrative-Labs/Tuxedo#127 (cc @muraca) and my question at https://substrate.stackexchange.com/questions/10105/extrinsics-in-genesis-block

IDK if it is in scope for this issue, but we are wanting to allow chain launchers to specify genesis transactions (yes transactions, not just state) in their chain specs. But have so far relied on a storage hack.

@michalkucharczyk
Copy link
Contributor Author

michalkucharczyk commented Oct 18, 2023 •
edited
Loading

IDK if it is in scope for this issue, but we are wanting to allow chain launchers to specify genesis transactions (yes transactions, not just state) in their chain specs. But have so far relied on a storage hack.

This issue is only about re-naming the components that are already in place (e.g : GenesisBuilder API), which were implemented to add support for GenesisConfig in wasm runtimes. (refer to: #25).

Adding support for genesis extrinsic is definitely out of scope of this ticket.

@bkchr
Copy link
Member

bkchr commented Oct 19, 2023

IDK if it is in scope for this issue, but we are wanting to allow chain launchers to specify genesis transactions (yes transactions, not just state) in their chain specs. But have so far relied on a storage hack.

This is already supported, as I have answered you on SE.

@michalkucharczyk michalkucharczyk mentioned this issue Dec 14, 2023
Merged
@michalkucharczyk
Copy link
Contributor Author

New naming proposed in #2714 (cc: @kianenigma)

github-merge-queue bot pushed a commit that referenced this issue Apr 4, 2024
@michalkucharczyk @skunert
@kianenigma @ggwpez
GenesisConfig presets for runtime (#2714)
f910a15 
The runtime now can provide a number of predefined presets of
`RuntimeGenesisConfig` struct. This presets are intended to be used in
different deployments, e.g.: `local`, `staging`, etc, and should be
included into the corresponding chain-specs.

Having `GenesisConfig` presets in runtime allows to fully decouple node
from runtime types (the problem is described in #1984).

**Summary of changes:**
- The `GenesisBuilder` API was adjusted to enable this functionality
(and provide better naming - #150):
   ```rust
    fn preset_names() -> Vec<PresetId>;
fn get_preset(id: Option<PresetId>) -> Option<serde_json::Value>;
//`None` means default
    fn build_state(value: serde_json::Value);
    pub struct PresetId(Vec<u8>);
   ```

- **Breaking change**: Old `create_default_config` method was removed,
`build_config` was renamed to `build_state`. As a consequence a node
won't be able to interact with genesis config for older runtimes. The
cleanup was made for sake of API simplicity. Also IMO maintaining
compatibility with old API is not so crucial.
- Reference implementation was provided for `substrate-test-runtime` and
`rococo` runtimes. For rococo new
[`genesis_configs_presets`](https://github.com/paritytech/polkadot-sdk/blob/3b41d66b97c5ff0ec4a1989da5ffd8b9f3f588e3/polkadot/runtime/rococo/src/genesis_config_presets.rs#L530)
module was added and is used in `GenesisBuilder`
[_presets-related_](https://github.com/paritytech/polkadot-sdk/blob/3b41d66b97c5ff0ec4a1989da5ffd8b9f3f588e3/polkadot/runtime/rococo/src/lib.rs#L2462-L2485)
methods.

- The `chain-spec-builder` util was also improved and allows to
([_doc_](https://github.com/paritytech/polkadot-sdk/blob/3b41d66b97c5ff0ec4a1989da5ffd8b9f3f588e3/substrate/bin/utils/chain-spec-builder/src/lib.rs#L19)):
   - list presets provided by given runtime (`list-presets`),
- display preset or default config provided by the runtime
(`display-preset`),
   - build chain-spec using named preset (`create ... named-preset`),


- The `ChainSpecBuilder` is extended with
[`with_genesis_config_preset_name`](https://github.com/paritytech/polkadot-sdk/blob/3b41d66b97c5ff0ec4a1989da5ffd8b9f3f588e3/substrate/client/chain-spec/src/chain_spec.rs#L447)
method which allows to build chain-spec using named preset provided by
the runtime. Sample usage on the node side
[here](https://github.com/paritytech/polkadot-sdk/blob/2caffaae803e08a3d5b46c860e8016da023ff4ce/polkadot/node/service/src/chain_spec.rs#L404).

Implementation of #1984.
fixes: #150
part of: #25

---------

Co-authored-by: Sebastian Kunert <skunert49@gmail.com>
Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Ank4n pushed a commit that referenced this issue Apr 9, 2024
@michalkucharczyk @skunert
@kianenigma @ggwpez @Ank4n
GenesisConfig presets for runtime (#2714)
b159df1 
The runtime now can provide a number of predefined presets of
`RuntimeGenesisConfig` struct. This presets are intended to be used in
different deployments, e.g.: `local`, `staging`, etc, and should be
included into the corresponding chain-specs.

Having `GenesisConfig` presets in runtime allows to fully decouple node
from runtime types (the problem is described in #1984).

**Summary of changes:**
- The `GenesisBuilder` API was adjusted to enable this functionality
(and provide better naming - #150):
   ```rust
    fn preset_names() -> Vec<PresetId>;
fn get_preset(id: Option<PresetId>) -> Option<serde_json::Value>;
//`None` means default
    fn build_state(value: serde_json::Value);
    pub struct PresetId(Vec<u8>);
   ```

- **Breaking change**: Old `create_default_config` method was removed,
`build_config` was renamed to `build_state`. As a consequence a node
won't be able to interact with genesis config for older runtimes. The
cleanup was made for sake of API simplicity. Also IMO maintaining
compatibility with old API is not so crucial.
- Reference implementation was provided for `substrate-test-runtime` and
`rococo` runtimes. For rococo new
[`genesis_configs_presets`](https://github.com/paritytech/polkadot-sdk/blob/3b41d66b97c5ff0ec4a1989da5ffd8b9f3f588e3/polkadot/runtime/rococo/src/genesis_config_presets.rs#L530)
module was added and is used in `GenesisBuilder`
[_presets-related_](https://github.com/paritytech/polkadot-sdk/blob/3b41d66b97c5ff0ec4a1989da5ffd8b9f3f588e3/polkadot/runtime/rococo/src/lib.rs#L2462-L2485)
methods.

- The `chain-spec-builder` util was also improved and allows to
([_doc_](https://github.com/paritytech/polkadot-sdk/blob/3b41d66b97c5ff0ec4a1989da5ffd8b9f3f588e3/substrate/bin/utils/chain-spec-builder/src/lib.rs#L19)):
   - list presets provided by given runtime (`list-presets`),
- display preset or default config provided by the runtime
(`display-preset`),
   - build chain-spec using named preset (`create ... named-preset`),


- The `ChainSpecBuilder` is extended with
[`with_genesis_config_preset_name`](https://github.com/paritytech/polkadot-sdk/blob/3b41d66b97c5ff0ec4a1989da5ffd8b9f3f588e3/substrate/client/chain-spec/src/chain_spec.rs#L447)
method which allows to build chain-spec using named preset provided by
the runtime. Sample usage on the node side
[here](https://github.com/paritytech/polkadot-sdk/blob/2caffaae803e08a3d5b46c860e8016da023ff4ce/polkadot/node/service/src/chain_spec.rs#L404).

Implementation of #1984.
fixes: #150
part of: #25

---------

Co-authored-by: Sebastian Kunert <skunert49@gmail.com>
Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
dharjeezy pushed a commit to dharjeezy/polkadot-sdk that referenced this issue Apr 9, 2024
@michalkucharczyk @skunert
@kianenigma @ggwpez @dharjeezy
GenesisConfig presets for runtime (paritytech#2714)
0adbbf4 
The runtime now can provide a number of predefined presets of
`RuntimeGenesisConfig` struct. This presets are intended to be used in
different deployments, e.g.: `local`, `staging`, etc, and should be
included into the corresponding chain-specs.

Having `GenesisConfig` presets in runtime allows to fully decouple node
from runtime types (the problem is described in paritytech#1984).

**Summary of changes:**
- The `GenesisBuilder` API was adjusted to enable this functionality
(and provide better naming - paritytech#150):
   ```rust
    fn preset_names() -> Vec<PresetId>;
fn get_preset(id: Option<PresetId>) -> Option<serde_json::Value>;
//`None` means default
    fn build_state(value: serde_json::Value);
    pub struct PresetId(Vec<u8>);
   ```

- **Breaking change**: Old `create_default_config` method was removed,
`build_config` was renamed to `build_state`. As a consequence a node
won't be able to interact with genesis config for older runtimes. The
cleanup was made for sake of API simplicity. Also IMO maintaining
compatibility with old API is not so crucial.
- Reference implementation was provided for `substrate-test-runtime` and
`rococo` runtimes. For rococo new
[`genesis_configs_presets`](https://github.com/paritytech/polkadot-sdk/blob/3b41d66b97c5ff0ec4a1989da5ffd8b9f3f588e3/polkadot/runtime/rococo/src/genesis_config_presets.rs#L530)
module was added and is used in `GenesisBuilder`
[_presets-related_](https://github.com/paritytech/polkadot-sdk/blob/3b41d66b97c5ff0ec4a1989da5ffd8b9f3f588e3/polkadot/runtime/rococo/src/lib.rs#L2462-L2485)
methods.

- The `chain-spec-builder` util was also improved and allows to
([_doc_](https://github.com/paritytech/polkadot-sdk/blob/3b41d66b97c5ff0ec4a1989da5ffd8b9f3f588e3/substrate/bin/utils/chain-spec-builder/src/lib.rs#L19)):
   - list presets provided by given runtime (`list-presets`),
- display preset or default config provided by the runtime
(`display-preset`),
   - build chain-spec using named preset (`create ... named-preset`),


- The `ChainSpecBuilder` is extended with
[`with_genesis_config_preset_name`](https://github.com/paritytech/polkadot-sdk/blob/3b41d66b97c5ff0ec4a1989da5ffd8b9f3f588e3/substrate/client/chain-spec/src/chain_spec.rs#L447)
method which allows to build chain-spec using named preset provided by
the runtime. Sample usage on the node side
[here](https://github.com/paritytech/polkadot-sdk/blob/2caffaae803e08a3d5b46c860e8016da023ff4ce/polkadot/node/service/src/chain_spec.rs#L404).

Implementation of paritytech#1984.
fixes: paritytech#150
part of: paritytech#25

---------

Co-authored-by: Sebastian Kunert <skunert49@gmail.com>
Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@michalkucharczyk michalkucharczyk

Labels
I4-refactor Code needs refactoring. T1-FRAME This PR/Issue is related to core FRAME, the framework.
Projects
Runtime / FRAME
Status: Done
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

GenesisConfig presets for runtime paritytech/polkadot-sdk
7 participants
@michalkucharczyk @JoshOrndorff @juangirini @kianenigma @bkchr @ggwpez @the-right-joyce