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

Add a trie_root_hash variant for chain specs genesis #10140

Merged
merged 7 commits into from
Nov 3, 2021

Conversation

tomaka
Copy link
Contributor

@tomaka tomaka commented Nov 1, 2021

Right now, chain specs can look like this:

"genesis": {
  "runtime": { }
}

Or like this:

"genesis": {
  "raw": { }
}

This PR adds a third variant:

"genesis": {
  "trie_root_hash": "0x0000000000000000000000000000000000000000000000000000000000000000"
}

Substrate isn't capable of loading chain specs with this trie_root_hash variant and will probably not be capable in the near future, but I'm adding this variant anyway because the chain_spec.rs file of Substrate is kind of the reference of the format of the chain specs.
I would like to add support for this variant in smoldot (paritytech/smoldot#1566), and this PR will provide a nice error message for people trying to load "smoldot chain specs" into Substrate.

EDIT: changed trie_root_hash to state_root_hash after review

@tomaka tomaka added A0-please_review Pull request needs code review. B5-clientnoteworthy C1-low PR touches the given topic and has a low impact on builders. labels Nov 1, 2021
@tomaka tomaka requested a review from bkchr November 1, 2021 11:57
@tomaka
Copy link
Contributor Author

tomaka commented Nov 1, 2021

One open question is whether the name trie_root_hash is explicit enough.
The trie can, in theory, use multiple different formats and hash functions depending on the chain. I think it's fine to leave the format of the trie implicit in this context.

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.

Besides bike-shedding it looks good to me :P state_root is the more common term that is used in the context of Substrate.

Regarding support these kind of chain specs, I think we should create an issue? As Substrate now also supports fast sync/warp sync. I assume that we could also make it work with only the hash for genesis? CC @arkpar

client/chain-spec/src/chain_spec.rs Outdated Show resolved Hide resolved
client/chain-spec/src/chain_spec.rs Outdated Show resolved Hide resolved
@bkchr bkchr requested a review from arkpar November 1, 2021 12:09
@bkchr bkchr merged commit 85a2a7a into paritytech:master Nov 3, 2021
@tomaka tomaka deleted the hash-chain-spec branch November 3, 2021 10:21
grishasobol pushed a commit to gear-tech/substrate that referenced this pull request Mar 28, 2022
)

* Add a `hash` variant for chain specs

* Add doc

* Rename to TrieRootHash

* Apply suggestions from code review

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

* Rustfmt

* More cargo fmt I guess

* Ok I have to use nightly cargo fmt

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>
ark0f pushed a commit to gear-tech/substrate that referenced this pull request Feb 27, 2023
)

* Add a `hash` variant for chain specs

* Add doc

* Rename to TrieRootHash

* Apply suggestions from code review

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

* Rustfmt

* More cargo fmt I guess

* Ok I have to use nightly cargo fmt

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>
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. C1-low PR touches the given topic and has a low impact on builders.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants