Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

GenesisBuilder runtime API #14131

Merged
merged 32 commits into from
Jun 26, 2023
Merged

Conversation

michalkucharczyk
Copy link
Contributor

@michalkucharczyk michalkucharczyk commented May 12, 2023

Proposal of new Runtime API: GenesisBuilder.

It allows to:

  • create a default GenesisConfig and serialize it to a JSON blob,
  • deserialize the GenesisConfig from a given JSON blob, and put it into storage, (no defaults allowed)

Step towards: paritytech/polkadot-sdk#25


Required by : #14310

@michalkucharczyk michalkucharczyk changed the title GenesisConfigBuilder Runtime API: preliminary proposal GenesisBuilder Runtime API: preliminary proposal May 12, 2023
@michalkucharczyk michalkucharczyk added the A3-in_progress Pull request is in progress. No review needed at this stage. label May 12, 2023
@michalkucharczyk michalkucharczyk requested a review from a team May 12, 2023 18:52
@Polkadot-Forum
Copy link

This pull request has been mentioned on Polkadot Forum. There might be relevant details there:

https://forum.polkadot.network/t/genesisconfig-in-wasm-runtime-a-step-towards-a-non-native-future/2887/1

@michalkucharczyk michalkucharczyk added B0-silent Changes should not be mentioned in any release notes B1-note_worthy Changes should be noted in the release notes T1-runtime This PR/Issue is related to the topic “runtime”. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit and removed B0-silent Changes should not be mentioned in any release notes labels May 18, 2023
@michalkucharczyk michalkucharczyk added the C1-low PR touches the given topic and has a low impact on builders. label May 18, 2023
Copy link
Member

@bkchr bkchr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the naming I don't see any reason to always include genesis_config as this is already implied by GenesisBuilder.

Looking good so far I would say.

fn default_genesis_config_as_json() -> sp_std::vec::Vec<u8>;

/// Deserialize the `GenesisConfig` from given json blob and put it into the storage.
fn build_genesis_config_from_json(json: sp_std::vec::Vec<u8>);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
fn build_genesis_config_from_json(json: sp_std::vec::Vec<u8>);
fn build_from_json(json: sp_std::vec::Vec<u8>);

/// API to interact with GenesisConfig for the runtime
pub trait GenesisBuilder {
/// Instantiate default `GenesisConfig` and serializes it to json blob.
fn default_genesis_config_as_json() -> sp_std::vec::Vec<u8>;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
fn default_genesis_config_as_json() -> sp_std::vec::Vec<u8>;
fn generate_as_json(name: sp_runtime::RuntimeString) -> sp_std::vec::Vec<u8>;

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

Copy link
Contributor Author

@michalkucharczyk michalkucharczyk May 22, 2023

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 all GenesisConfigs pallets, so all required parametrization is already self-contained in config (AFAIU it).

e.g. the generated GenesisConfig for kitchensink-runtime:

pub struct GenesisConfig {
    pub system: SystemConfig,
    pub babe: BabeConfig,
    pub indices: IndicesConfig,
    pub balances: BalancesConfig,
    pub transaction_payment: TransactionPaymentConfig,
    pub staking: StakingConfig,
    pub session: SessionConfig,
    pub democracy: DemocracyConfig,
    pub council: CouncilConfig,
    pub technical_committee: TechnicalCommitteeConfig,
    pub elections: ElectionsConfig,
    pub technical_membership: TechnicalMembershipConfig,
    pub grandpa: GrandpaConfig,
    pub treasury: TreasuryConfig,
    pub sudo: SudoConfig,
    pub im_online: ImOnlineConfig,
    pub authority_discovery: AuthorityDiscoveryConfig,
    pub society: SocietyConfig,
    pub vesting: VestingConfig,
    pub assets: AssetsConfig,
    pub transaction_storage: TransactionStorageConfig,
    pub alliance_motion: AllianceMotionConfig,
    pub alliance: AllianceConfig,
    pub nomination_pools: NominationPoolsConfig,
}

What I see in chain_spec.rs are different ChainSpecs: development_config, local_testnet_config.

Each of them has specific genesis: local_testnet_genesis, development_config_genesis. Those specific gensis are just differently parametrized GenesisConfig instances of kitchensink_runtime created in shared function testnet_genesis.

With new GenesisBuilder API approach, that custom GenesisConfig will be embedded in the form of pre-configured json files customized on top of the json for default GensisConfig.

If I missed something, would you please be more precise on where the extra parameter is actually required?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With new GenesisBuilder API approach, that custom GenesisConfig will be embedded in the form of pre-configured json files customized on top of the json for default GensisConfig.

Could you give an example of this?

Copy link
Contributor Author

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:

let expected = r#"{"system":{"code":"0x"},"babe":{"authorities":[],"epochConfig":null},"substrateTest":{"authorities":[]},"balances":{"balances":[]}}"#;

What I mean by GenesisConfig customised on top of default:

{"system":{"code":"0x52"},"babe":{"authorities":[["5GrwvaEF5zXb26Fz9rcQpDWS57CtERHpNehXCPcNoHGKutQY",1],["5FHneW46xGXgs5mUiveU4sbTyGBzmstUspZC92UhjJM694ty",1],["5FLSigC9HGRKVhB9FiEo4Y3koPsNmBmLJbpXg2mp1hXcS59Y",1]],"epochConfig":{"c":[3,10],"allowed_slots":"PrimaryAndSecondaryPlainSlots"}},"substrateTest":{"authorities":["5GrwvaEF5zXb26Fz9rcQpDWS57CtERHpNehXCPcNoHGKutQY","5FHneW46xGXgs5mUiveU4sbTyGBzmstUspZC92UhjJM694ty","5FLSigC9HGRKVhB9FiEo4Y3koPsNmBmLJbpXg2mp1hXcS59Y"]},"balances":{"balances":[["5D34dL5prEUaGNQtPPZ3yN5Y6BnkfXunKXXz6fo7ZJbLwRRH",100000000000000000],["5GBNeWRhZc2jXu7D55rBimKYDk8PGk8itRYFTPfC8RJLKG5o",100000000000000000],["5Dfis6XL8J2P6JHUnUtArnFWndn62SydeP8ee8sG2ky9nfm9",100000000000000000],["5F4H97f7nQovyrbiq4ZetaaviNwThSVcFobcA5aGab6167dK",100000000000000000],["5DiDShBWa1fQx6gLzpf3SFBhMinCoyvHM1BWjPNsmXS8hkrW",100000000000000000],["5EFb84yH9tpcFuiKUcsmdoF7xeeY3ajG1ZLQimxQoFt9HMKR",100000000000000000],["5DZLHESsfGrJ5YzT3HuRPXsSNb589xQ4Unubh1mYLodzKdVY",100000000000000000],["5GHJzqvG6tXnngCpG7B12qjUvbo5e4e9z8Xjidk3CQZHxTPZ",100000000000000000],["5CUnSsgAyLND3bxxnfNhgWXSe9Wn676JzLpGLgyJv858qhoX",100000000000000000],["5CVKn7HAZW1Ky4r7Vkgsr7VEW88C2sHgUNDiwHY9Ct2hjU8q",100000000000000000],["5H673aukQ4PeDe1U2nuv1bi32xDEziimh3PZz7hDdYUB7TNz",100000000000000000],["5HTe9L15LJryjUAt1jZXZCBPnzbbGnpvFwbjE3NwCWaAqovf",100000000000000000],["5D7LFzGpMwHPyDBavkRbWSKWTtJhCaPPZ379wWLT23bJwXJz",100000000000000000],["5CLepMARnEgtVR1EkUuJVUvKh97gzergpSxUU3yKGx1v6EwC",100000000000000000],["5Chb2UhfvZpmjjEziHbFbotM4quX32ZscRV6QJBt1rUKzz51",100000000000000000],["5HmRp3i3ZZk7xsAvbi8hyXVP6whSMnBJGebVC4FsiZVhx52e",100000000000000000],["5GrwvaEF5zXb26Fz9rcQpDWS57CtERHpNehXCPcNoHGKutQY",100000000000000000],["5FHneW46xGXgs5mUiveU4sbTyGBzmstUspZC92UhjJM694ty",100000000000000000],["5FLSigC9HGRKVhB9FiEo4Y3koPsNmBmLJbpXg2mp1hXcS59Y",100000000000000000]]}}

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 the GenesisConfig in the form of rust variable. The alternative for this will be json string.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GenesisConfig {
system: SystemConfig { code: wasm_binary_unwrap().to_vec() },
balances: BalancesConfig {
balances: endowed_accounts.iter().cloned().map(|x| (x, ENDOWMENT)).collect(),
},
indices: IndicesConfig { indices: vec![] },
session: SessionConfig {
keys: initial_authorities
.iter()
.map(|x| {
(
x.0.clone(),
x.0.clone(),
session_keys(x.2.clone(), x.3.clone(), x.4.clone(), x.5.clone()),
)
})
.collect::<Vec<_>>(),
},
staking: StakingConfig {
validator_count: initial_authorities.len() as u32,
minimum_validator_count: initial_authorities.len() as u32,
invulnerables: initial_authorities.iter().map(|x| x.0.clone()).collect(),
slash_reward_fraction: Perbill::from_percent(10),
stakers,
..Default::default()
},
democracy: DemocracyConfig::default(),
elections: ElectionsConfig {
members: endowed_accounts
.iter()
.take((num_endowed_accounts + 1) / 2)
.cloned()
.map(|member| (member, STASH))
.collect(),
},
council: CouncilConfig::default(),
technical_committee: TechnicalCommitteeConfig {
members: endowed_accounts
.iter()
.take((num_endowed_accounts + 1) / 2)
.cloned()
.collect(),
phantom: Default::default(),
},
sudo: SudoConfig { key: Some(root_key) },
babe: BabeConfig {
authorities: vec![],
epoch_config: Some(kitchensink_runtime::BABE_GENESIS_EPOCH_CONFIG),
},
im_online: ImOnlineConfig { keys: vec![] },
authority_discovery: AuthorityDiscoveryConfig { keys: vec![] },
grandpa: GrandpaConfig { authorities: vec![] },
technical_membership: Default::default(),
treasury: Default::default(),
society: SocietyConfig {
members: endowed_accounts
.iter()
.take((num_endowed_accounts + 1) / 2)
.cloned()
.collect(),
pot: 0,
max_members: 999,
},
vesting: Default::default(),
assets: pallet_assets::GenesisConfig {
// This asset is used by the NIS pallet as counterpart currency.
assets: vec![(9, get_account_id_from_seed::<sr25519::Public>("Alice"), true, 1)],
..Default::default()
},
transaction_storage: Default::default(),
transaction_payment: Default::default(),
alliance: Default::default(),
alliance_motion: Default::default(),
nomination_pools: NominationPoolsConfig {
min_create_bond: 10 * DOLLARS,
min_join_bond: 1 * DOLLARS,
..Default::default()
},
glutton: GluttonConfig {
compute: Default::default(),
storage: Default::default(),
trash_data_count: Default::default(),
},

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:

let config = json! {{
     "session": {
         "keys":  initial_authorities.map().collect(),
     },
      "staking": {
          "validator_count": validator_count,
      },
}};

// It should be able to handle that we only pass parts of the config and the rest should be filled by the default values.
Runtime::build_from_json(config);

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

@michalkucharczyk michalkucharczyk Jun 6, 2023

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:

let json_str = Runtime::default_as_json();
let mut json: Value = serde_json::from_str(json_str).unwrap();
merge(&mut json, json!({
     "session": {
         "keys":  initial_authorities.map().collect(),
     },
      "staking": {
          "validator_count": validator_count,
      },
});
Runtime::build_from_json(json.to_string());

However I do understand that build_with_patch is a nice convenient function.

@michalkucharczyk michalkucharczyk requested a review from a team June 7, 2023 11:21
primitives/genesis-builder/Cargo.toml Outdated Show resolved Hide resolved
primitives/genesis-builder/src/lib.rs Outdated Show resolved Hide resolved
primitives/genesis-builder/src/lib.rs Outdated Show resolved Hide resolved
primitives/genesis-builder/src/lib.rs Outdated Show resolved Hide resolved
primitives/genesis-builder/src/lib.rs Outdated Show resolved Hide resolved
michalkucharczyk and others added 4 commits June 7, 2023 15:37
Co-authored-by: Davide Galassi <davxy@datawok.net>
Co-authored-by: Davide Galassi <davxy@datawok.net>
Co-authored-by: Davide Galassi <davxy@datawok.net>
@michalkucharczyk michalkucharczyk requested a review from a team June 7, 2023 16:49
@michalkucharczyk
Copy link
Contributor Author

I’ve just realized that we don’t need two functions: build_from_json, build_with_patch. Building from full config is just a special case of patching.

Copy link
Contributor

@skunert skunert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks nice!

Copy link
Member

@bkchr bkchr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good % nits

primitives/genesis-builder/src/lib.rs Outdated Show resolved Hide resolved
primitives/genesis-builder/src/lib.rs Outdated Show resolved Hide resolved
@michalkucharczyk
Copy link
Contributor Author

bot rebase

@paritytech-processbot
Copy link

Rebased

Copy link
Member

@bkchr bkchr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the current function naming is confusing.

If we think about what we need, then we need to get the default config as json. We need to be able to patch the default config to create configs for dev networks etc. Lastly, we need to be able to build the config. Maybe using here build is a too confusing term, not sure, but given the internals it makes sense.

With these three functions we should be able to implement all possibles functionality as it exists right now.

(docs then also require updating)

///
/// This function instantiates the default `GenesisConfig` struct for the runtime and serializes it into a JSON
/// blob. It returns a `Vec<u8>` containing the JSON representation of the default `GenesisConfig`.
fn get_default_as_json() -> sp_std::vec::Vec<u8>;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
fn get_default_as_json() -> sp_std::vec::Vec<u8>;
fn create_default_config() -> sp_std::vec::Vec<u8>;

/// `GenesisConfig`. This is helpful for changing enum variant value.
///
/// Please note that the patch may contain full `GenesisConfig`.
fn build_config(patch: sp_std::vec::Vec<u8>) -> Result;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
fn build_config(patch: sp_std::vec::Vec<u8>) -> Result;
fn patch_default_config(patch: sp_std::vec::Vec<u8>) -> sp_std::result::Result<sp_std::vec::Vec<u8>, RuntimeString>;

/// It is recommended to log any errors encountered during the process.
///
/// Please note that provided json blob must contain all `GenesisConfig` fields, no defaults will be used.
fn build_config_no_defaults(json: sp_std::vec::Vec<u8>) -> Result;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
fn build_config_no_defaults(json: sp_std::vec::Vec<u8>) -> Result;
fn build_config(json: sp_std::vec::Vec<u8>) -> Result;

@michalkucharczyk
Copy link
Contributor Author

I think the current function naming is confusing.

If we think about what we need, then we need to get the default config as json. We need to be able to patch the default config to create configs for dev networks etc. Lastly, we need to be able to build the config. Maybe using here build is a too confusing term, not sure, but given the internals it makes sense.

With these three functions we should be able to implement all possibles functionality as it exists right now.

(docs then also require updating)

Please note that we don’t need runtime call to merge jsons. Can be done on client side.

build_config(patch) was meant to be convenient function which patches and builds in single call.

If you want stand alone patching then I propose to remove it from runtime and left only build_config (w/o patching).

@bkchr wdyt?

@bkchr
Copy link
Member

bkchr commented Jun 21, 2023

Please note that we don’t need runtime call to merge jsons. Can be done on client side.

Very good point! :D

If you want stand alone patching then I propose to remove it from runtime and left only build_config (w/o patching).

Sounds good.

@michalkucharczyk
Copy link
Contributor Author

bot merge

@paritytech-processbot paritytech-processbot bot merged commit 0bf1433 into master Jun 26, 2023
@paritytech-processbot paritytech-processbot bot deleted the mku-genesis-builder-api-proposal branch June 26, 2023 11:56
nathanwhit pushed a commit to nathanwhit/substrate that referenced this pull request Jul 19, 2023
* GenesisConfigBuilder: preliminary API proposal

* fmt

* comment removed

* build_default_config removed

* Update client/genesis-builder/src/lib.rs

* config -> gensis_config

* GenesisConfigBuilder: helper added

* moved to primitives

* licesne changed to apache-2.0

* Cargo.toml: name/path to genesis-builder updated

* helper removed

* sp-sd version bumped

* sp-std bump

* naming + new function

* fix

* build_from_patch_json -> build_with_patch

* fix

* Cargo.lock updated

* readme: license updated

* Update primitives/genesis-builder/src/lib.rs

Co-authored-by: Davide Galassi <davxy@datawok.net>

* Update primitives/genesis-builder/src/lib.rs

Co-authored-by: Davide Galassi <davxy@datawok.net>

* Update primitives/genesis-builder/Cargo.toml

Co-authored-by: Davide Galassi <davxy@datawok.net>

* Cargo.lock updated

* removed redundant function

* GenesisConfigBuilder API: no_defaults function added

* Cargo.lock updated

* GenesisConfigBuilder API: patching fn removed

* trigger CI job

---------

Co-authored-by: parity-processbot <>
Co-authored-by: Davide Galassi <davxy@datawok.net>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. B1-note_worthy Changes should be noted in the release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit T1-runtime This PR/Issue is related to the topic “runtime”.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants