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.

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

Add app version to genesis.json #14954

Closed
ValarDragon opened this issue Feb 8, 2023 · 7 comments · Fixed by #15031
Closed

Add app version to genesis.json #14954

ValarDragon opened this issue Feb 8, 2023 · 7 comments · Fixed by #15031
Assignees
Labels
T: Dev UX UX for SDK developers (i.e. how to call our code)

Comments

@ValarDragon
Copy link
Contributor

Summary

A frequent bug report we get in Osmosis, is "tried downloading binary + genesis.json, running start, got {very unfriendly error}".

We should add a parameter to genesis.json, indicating the App version its for. Then we can easily give a human friendly error, if it cannot be parsed correctly, and you need to sync / startup via another mechanism.

Problem Definition

Helps mitigate a frequent node operator onboarding problem.

Proposal

Add an AppVersion field in the genesis from export Genesis. Then apps can handle this better in InitGenesis.

@github-actions github-actions bot added the needs-triage Issue that needs to be triaged label Feb 8, 2023
@tac0turtle tac0turtle added T: Dev UX UX for SDK developers (i.e. how to call our code) T:Sprint and removed needs-triage Issue that needs to be triaged labels Feb 8, 2023
@github-project-automation github-project-automation bot moved this to 📝 Todo in Cosmos-SDK Feb 8, 2023
@julienrbrt julienrbrt self-assigned this Feb 9, 2023
@julienrbrt julienrbrt moved this from 📝 Todo to 💪 In Progress in Cosmos-SDK Feb 9, 2023
@julienrbrt
Copy link
Member

I think we should create an AppGenesis and use that through the app.

type AppGenesis struct {
	AppVersion string `json:"app_version"`
	cmttypes.GenesisDoc
}

However, it may be a tiny bit overkilled.

@ValarDragon
Copy link
Contributor Author

Out of curiosity, whats the thinking wrt not editing the genesis doc?
(I don't actually have an opinion here)

@julienrbrt
Copy link
Member

julienrbrt commented Feb 13, 2023

Out of curiosity, whats the thinking wrt not editing the genesis doc?
(I don't actually have an opinion here)

That works too, but we need CometBFT to update the GenesisDoc struct to add that field.
I was taking this as an opportunity to begin with abstracting from CometBFT genesis.

@julienrbrt julienrbrt moved this from 💪 In Progress to 📝 Todo in Cosmos-SDK Feb 13, 2023
@julienrbrt julienrbrt moved this from 📝 Todo to 💪 In Progress in Cosmos-SDK Feb 13, 2023
@ValarDragon
Copy link
Contributor Author

Oh that makes sense, in support of getting an application maintained genesis format, rather than the Comet one!

@tac0turtle
Copy link
Member

This seems required to me to decouple ourselves from comet. It's a larger refactor but needed

@julienrbrt
Copy link
Member

julienrbrt commented Feb 16, 2023

@aaronc, in our standup, had a good point: we could store this information in a module too, such a genutil, if the application genesis does not have a big use case.

So, does someone else sees a value in an application genesis, and if so, what extra could be stored in an application genesis?

Personally, I went this way only to abstract from CometBFT, however, if there is no any use case for the abstraction, going the module way will be simpler.

@ValarDragon
Copy link
Contributor Author

I think its a significant value add to get this abstracted away from CometBFT. It lets the SDK specify genesis in a number of different ways, unconstrained by CometBFT's single genesis.json & unmarshalling paradigm.

On some potentially simple human parseability improvements that are possible:

  • Its far easier to parse app version if its top-level, and at the very top.
  • In the future, we can have the SDK be able to parse several of the TM fields, to make them not top-level, but instead SDK provided / parsed from relevant app state. Namely:
    • Genesis Validators
    • Consensus Params
  • We can potentially avoid the layer of JSON indirection with these files.
  • We could bundle in "insert-order metadata" for ability to reconstruct the exact same app-hash that the chain was on in export.

But I think more importantly, diverging the format, lets us get more creative with how we want users to be able to specify their chain genesis, in the most natural way, or to make more "intermediary representations". For instance, we can make the chain import & export cycles work off something "minified" (e.g. removing all the string literals in every JSON key), but have easy conversion to/fro the more readable JSON.

@julienrbrt julienrbrt moved this from 💪 In Progress to 👀 Needs Review in Cosmos-SDK Feb 21, 2023
@github-project-automation github-project-automation bot moved this from 👀 Needs Review to 👏 Done in Cosmos-SDK Feb 22, 2023
@tac0turtle tac0turtle removed this from Cosmos-SDK May 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T: Dev UX UX for SDK developers (i.e. how to call our code)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants