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

evm: add missing genesis fields and export genesis state logic #255

Merged
merged 20 commits into from
May 18, 2020

Conversation

fedekunze
Copy link
Contributor

@fedekunze fedekunze commented Apr 16, 2020

Closes: #XXX

Description

  • Add missing evm GenesisState fields
  • Export the EVM state for upgrades and hardforks.

For contributor use:

  • Targeted PR against correct branch (see CONTRIBUTING.md)
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Code follows the module structure standards.
  • Wrote unit and integration tests
  • Updated relevant documentation (docs/) or specification (x/<module>/spec/)
  • Added relevant godoc comments.
  • Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
  • Re-reviewed Files changed in the Github PR explorer

For admin use:

  • Added appropriate labels to PR (ex. WIP, R4R, docs, etc)
  • Reviewers assigned
  • Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

@fedekunze fedekunze self-assigned this Apr 16, 2020
x/evm/genesis.go Outdated Show resolved Hide resolved
@fedekunze fedekunze marked this pull request as ready for review April 20, 2020 21:15
Copy link
Contributor

@noot noot 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, could you add instructions for testing the changes?

@fedekunze
Copy link
Contributor Author

It's the same instructions but with emintd

https://hub.cosmos.network/master/gaia-tutorials/upgrade-node.html#exporting-state-to-a-new-genesis-locally

@noot
Copy link
Contributor

noot commented Apr 22, 2020

@fedekunze I tried running the instructions and got an error:

$ emintd export --for-zero-height --height=1 > new_genesis.json
panic: app.baseKey expected to be nil; duplicate init?

goroutine 1 [running]:
github.com/cosmos/cosmos-sdk/baseapp.(*BaseApp).initFromMainStore(0xc000e49900, 0xc00012dfd0, 0x0, 0x0)
	/home/elizabeth/go/pkg/mod/github.com/cosmos/[email protected]/baseapp/baseapp.go:313 +0x2a0
github.com/cosmos/cosmos-sdk/baseapp.(*BaseApp).LoadVersion(0xc000e49900, 0x1, 0xc00012dfd0, 0x4, 0xc0003862f8)
	/home/elizabeth/go/pkg/mod/github.com/cosmos/[email protected]/baseapp/baseapp.go:291 +0x80
github.com/cosmos/ethermint/app.(*EthermintApp).LoadHeight(...)
	/home/elizabeth/go/src/github.com/ChainSafe/ethermint/app/ethermint.go:309
main.exportAppStateAndTMValidators(0x180fd40, 0xc000f80a00, 0x1826280, 0xc0005001f8, 0x0, 0x0, 0x1, 0xc000f80901, 0x23bbaf8, 0x0, ...)
	/home/elizabeth/go/src/github.com/ChainSafe/ethermint/cmd/emintd/main.go:105 +0x100
github.com/cosmos/cosmos-sdk/server.ExportCmd.func1(0xc000e7bb80, 0xc000e4db60, 0x0, 0x2, 0x0, 0x0)
	/home/elizabeth/go/pkg/mod/github.com/cosmos/[email protected]/server/export.go:65 +0x53d
github.com/spf13/cobra.(*Command).execute(0xc000e7bb80, 0xc000e4db20, 0x2, 0x2, 0xc000e7bb80, 0xc000e4db20)
	/home/elizabeth/go/pkg/mod/github.com/spf13/[email protected]/command.go:838 +0x460
github.com/spf13/cobra.(*Command).ExecuteC(0xc0000e8840, 0x13ee756, 0xc000ec5e48, 0x5893e2)
	/home/elizabeth/go/pkg/mod/github.com/spf13/[email protected]/command.go:943 +0x317
github.com/spf13/cobra.(*Command).Execute(...)
	/home/elizabeth/go/pkg/mod/github.com/spf13/[email protected]/command.go:883
github.com/tendermint/tendermint/libs/cli.Executor.Execute(0xc0000e8840, 0x15cdbb8, 0x13d0971, 0x10)
	/home/elizabeth/go/pkg/mod/github.com/tendermint/[email protected]/libs/cli/setup.go:89 +0x3c
main.main()
	/home/elizabeth/go/src/github.com/ChainSafe/ethermint/cmd/emintd/main.go:88 +0x760

@thomasmodeneis
Copy link

It seems like a test for running the cli should be added to the integration tests.

Copy link

@thomasmodeneis thomasmodeneis 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, since @noot reported the error, I think it make sense to add a test case for running the cmd: emintd export --for-zero-height --height=1 > new_genesis.json as part of the CI #268 in order to prevent it to break in the future.

@fedekunze fedekunze marked this pull request as draft May 1, 2020 15:33
@fedekunze fedekunze requested a review from noot May 18, 2020 15:46
@fedekunze fedekunze marked this pull request as ready for review May 18, 2020 15:47
@fedekunze
Copy link
Contributor Author

I think it make sense to add a test case for running the cmd: emintd export --for-zero-height --height=1 > new_genesis.json as part of the CI #268 in order to prevent it to break in the future.

Simulations handle that test case. The current export error seems to be unrelated to these changes (i.e it's from the SDK). I'd say we merge this PR as is and fix the export logic afterwards since this PR also includes the missing genesis fields for the evm module.

@fedekunze fedekunze changed the title evm: export genesis state evm: add missing genesis field and export genesis state logic May 18, 2020
@fedekunze fedekunze changed the title evm: add missing genesis field and export genesis state logic evm: add missing genesis fields and export genesis state logic May 18, 2020
Copy link
Contributor

@noot noot left a comment

Choose a reason for hiding this comment

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

I'm still seeing the same error I mentioned before but let's look into that later, everything else looks good

@fedekunze fedekunze merged commit 16df772 into development May 18, 2020
@fedekunze fedekunze deleted the evm-export branch May 18, 2020 19:21
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants