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

[Bug]: v0.50 init cmd uses wrong json marshaller #18477

Closed
1 task done
nhannamsiu opened this issue Nov 15, 2023 · 9 comments
Closed
1 task done

[Bug]: v0.50 init cmd uses wrong json marshaller #18477

nhannamsiu opened this issue Nov 15, 2023 · 9 comments
Assignees
Labels
S:needs more info This bug can't be addressed until more information is provided by the reporter. T:Bug

Comments

@nhannamsiu
Copy link

Is there an existing issue for this?

  • I have searched the existing issues

What happened?

This is how init cmd saves genesis file on v0.47. It uses correct cometbft json pkg to marshal.

func (genDoc *GenesisDoc) SaveAs(file string) error {
	genDocBytes, err := cmtjson.MarshalIndent(genDoc, "", "  ")
	if err != nil {
		return err
	}
	return cmtos.WriteFile(file, genDocBytes, 0644)
}

This is for v0.50.1. It uses default golang json pkg to marshal in x/genutil/types/genesis.go

func (ag *AppGenesis) SaveAs(file string) error {
	appGenesisBytes, err := json.MarshalIndent(ag, "", "  ")
	if err != nil {
		return err
	}

	return os.WriteFile(file, appGenesisBytes, 0o600)
}

This led to confusing error when we tried to run the node, especially after upgrading to v0.50.1, which breaks many stuff.

Cosmos SDK Version

v0.50.1

How to reproduce?

  1. Run init command on both v0.47.x and v0.50.x to generate genesis files.
  2. Compare 2 files and see that on v0.47.x it's "initial_height": **"1"** while on v0.50.x it's "initial_height": **1**
@julienrbrt
Copy link
Member

julienrbrt commented Nov 15, 2023

v0.47 uses a CometBFT genesis, while v0.50 uses an application genesis. The encoding is indeed different, but CometBFT genesis is still supported on v0.50. Take a look here: https://docs.cosmos.network/v0.50/build/modules/genutil

On the other hand, you should not be getting any errors. I am curious which errors have you got?

@nhannamsiu
Copy link
Author

nhannamsiu commented Nov 15, 2023

I get this after init and try to start the node, it's coming from initial_height int64.

panic: error reading GenesisDoc at /Users/nam/.fluxd/config/genesis.json: invalid 64-bit integer encoding "1", expected string

@julienrbrt
Copy link
Member

Are you starting the correct binary? It looks like you are starting a v0.47 chain with a v0.50 application genesis.

@nhannamsiu
Copy link
Author

I ported my chain node from 0.47.x to v0.50.1 and start seeing this error. On the main branch that uses 0.47 it's running fine. I have init script to craft node genesis every time I build so it's not from the old version.

@nhannamsiu
Copy link
Author

nhannamsiu commented Nov 15, 2023

Confirm it's fixed when I change default json to cmtjson. The sdk uses cmtjson to read from file, so it makes sense to marshal using the same package when we init and write genesis to file.

@julienrbrt
Copy link
Member

Okay, could you post the genesis, your repo link/snippet and the steps to reproduce?

Best would be that you try to reproduce with simapp (v0.50 and v0.47)

@julienrbrt julienrbrt added the S:needs more info This bug can't be addressed until more information is provided by the reporter. label Nov 15, 2023
@github-project-automation github-project-automation bot moved this to 👀 To Do in Cosmos-SDK Nov 16, 2023
@julienrbrt
Copy link
Member

Closing as stale.

@github-project-automation github-project-automation bot moved this from 👀 To Do to 🥳 Done in Cosmos-SDK Feb 19, 2024
@tac0turtle tac0turtle removed this from Cosmos-SDK Feb 29, 2024
yihuang added a commit to yihuang/cometbft that referenced this issue Apr 12, 2024
The cosmos-sdk 0.50 genesis doc marshalling not being compatible with the
cometbft default one (see cosmos/cosmos-sdk#18477),
so we need to support custom genesis doc provider in this public API as well,
it's called by the `bootstrap-state` command in cosmos-sdk.
@yihuang
Copy link
Collaborator

yihuang commented Apr 12, 2024

@julienrbrt we have indeed broken the genesis format in SDK 0.50, the json type of initial_height field is changed from string to number, it at least breaks the command bootstrap-state as far as I can tell.

because the genesis of a chain can be generated by any legacy version, we need to be aware of these format changes whenever we need to parse the genesis file, fortunately we have used a custom low level json parser for the chain-id parsing.

yihuang added a commit to yihuang/cometbft that referenced this issue Apr 12, 2024
The cosmos-sdk 0.50 genesis doc marshalling not being compatible with the
cometbft default one (see cosmos/cosmos-sdk#18477),
so we need to support custom genesis doc provider in this public API as well,
it's called by the `bootstrap-state` command in cosmos-sdk.
yihuang added a commit to yihuang/cometbft that referenced this issue Apr 12, 2024
…ft#2791)

The cosmos-sdk 0.50 genesis doc marshalling not being compatible with the
cometbft default one (see cosmos/cosmos-sdk#18477),
so we need to support custom genesis doc provider in this public API as well,
it's called by the `bootstrap-state` command in cosmos-sdk.
@julienrbrt
Copy link
Member

Right, this makes sense for functions in comet that were directly reading the genesis. Good one!
However, within the SDK we always support both.

github-merge-queue bot pushed a commit to cometbft/cometbft that referenced this issue Apr 12, 2024
The cosmos-sdk 0.50 genesis doc marshalling not being compatible with
the cometbft default one (see
cosmos/cosmos-sdk#18477), so we need to
support custom genesis doc provider in this public API as well, it's
called by the `bootstrap-state` command in cosmos-sdk.

Please backport to 0.38, the SDK 0.50 `bootstrap-state` command is
already broken.

<!--

Please add a reference to the issue that this PR addresses and indicate
which
files are most critical to review. If it fully addresses a particular
issue,
please include "Closes #XXX" (where "XXX" is the issue number).

If this PR is non-trivial/large/complex, please ensure that you have
either
created an issue that the team's had a chance to respond to, or had some
discussion with the team prior to submitting substantial pull requests.
The team
can be reached via GitHub Discussions or the Cosmos Network Discord
server in
the #cometbft channel. GitHub Discussions is preferred over Discord as
it
allows us to keep track of conversations topically.
https://github.com/cometbft/cometbft/discussions

If the work in this PR is not aligned with the team's current
priorities, please
be advised that it may take some time before it is merged - especially
if it has
not yet been discussed with the team.

See the project board for the team's current priorities:
https://github.com/orgs/cometbft/projects/1

-->

---

#### PR checklist

- [ ] Tests written/updated
- [ ] Changelog entry added in `.changelog` (we use
[unclog](https://github.com/informalsystems/unclog) to manage our
changelog)
- [ ] Updated relevant documentation (`docs/` or `spec/`) and code
comments
- [ ] Title follows the [Conventional
Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec
mergify bot pushed a commit to cometbft/cometbft that referenced this issue Apr 12, 2024
The cosmos-sdk 0.50 genesis doc marshalling not being compatible with
the cometbft default one (see
cosmos/cosmos-sdk#18477), so we need to
support custom genesis doc provider in this public API as well, it's
called by the `bootstrap-state` command in cosmos-sdk.

Please backport to 0.38, the SDK 0.50 `bootstrap-state` command is
already broken.

<!--

Please add a reference to the issue that this PR addresses and indicate
which
files are most critical to review. If it fully addresses a particular
issue,
please include "Closes #XXX" (where "XXX" is the issue number).

If this PR is non-trivial/large/complex, please ensure that you have
either
created an issue that the team's had a chance to respond to, or had some
discussion with the team prior to submitting substantial pull requests.
The team
can be reached via GitHub Discussions or the Cosmos Network Discord
server in
the #cometbft channel. GitHub Discussions is preferred over Discord as
it
allows us to keep track of conversations topically.
https://github.com/cometbft/cometbft/discussions

If the work in this PR is not aligned with the team's current
priorities, please
be advised that it may take some time before it is merged - especially
if it has
not yet been discussed with the team.

See the project board for the team's current priorities:
https://github.com/orgs/cometbft/projects/1

-->

---

#### PR checklist

- [ ] Tests written/updated
- [ ] Changelog entry added in `.changelog` (we use
[unclog](https://github.com/informalsystems/unclog) to manage our
changelog)
- [ ] Updated relevant documentation (`docs/` or `spec/`) and code
comments
- [ ] Title follows the [Conventional
Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec

(cherry picked from commit 7279fe8)
andynog added a commit to cometbft/cometbft that referenced this issue Apr 12, 2024
…2796)

The cosmos-sdk 0.50 genesis doc marshalling not being compatible with
the cometbft default one (see
cosmos/cosmos-sdk#18477), so we need to
support custom genesis doc provider in this public API as well, it's
called by the `bootstrap-state` command in cosmos-sdk.

Please backport to 0.38, the SDK 0.50 `bootstrap-state` command is
already broken.



---

#### PR checklist

- [ ] Tests written/updated
- [ ] Changelog entry added in `.changelog` (we use
[unclog](https://github.com/informalsystems/unclog) to manage our
changelog)
- [ ] Updated relevant documentation (`docs/` or `spec/`) and code
comments
- [ ] Title follows the [Conventional
Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec
<hr>This is an automatic backport of pull request #2791 done by
[Mergify](https://mergify.com).

Co-authored-by: yihuang <[email protected]>
Co-authored-by: Anton Kaliaev <[email protected]>
Co-authored-by: Andy Nogueira <[email protected]>
andynog pushed a commit to cometbft/cometbft that referenced this issue Apr 12, 2024
…#2793)

The cosmos-sdk 0.50 genesis doc marshalling not being compatible with
the cometbft default one (see
cosmos/cosmos-sdk#18477), so we need to
support custom genesis doc provider in this public API as well, it's
called by the `bootstrap-state` command in cosmos-sdk.

<!--

Please add a reference to the issue that this PR addresses and indicate
which
files are most critical to review. If it fully addresses a particular
issue,
please include "Closes #XXX" (where "XXX" is the issue number).

If this PR is non-trivial/large/complex, please ensure that you have
either
created an issue that the team's had a chance to respond to, or had some
discussion with the team prior to submitting substantial pull requests.
The team
can be reached via GitHub Discussions or the Cosmos Network Discord
server in
the #cometbft channel. GitHub Discussions is preferred over Discord as
it
allows us to keep track of conversations topically.
https://github.com/cometbft/cometbft/discussions

If the work in this PR is not aligned with the team's current
priorities, please
be advised that it may take some time before it is merged - especially
if it has
not yet been discussed with the team.

See the project board for the team's current priorities:
https://github.com/orgs/cometbft/projects/1

-->

---

#### PR checklist

- [ ] Tests written/updated
- [ ] Changelog entry added in `.changelog` (we use
[unclog](https://github.com/informalsystems/unclog) to manage our
changelog)
- [ ] Updated relevant documentation (`docs/` or `spec/`) and code
comments
- [ ] Title follows the [Conventional
Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec

Co-authored-by: Anton Kaliaev <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S:needs more info This bug can't be addressed until more information is provided by the reporter. T:Bug
Projects
None yet
Development

No branches or pull requests

3 participants