CLI `migrate` command follow-up: decode & re-encode #7464

amaury1093 · 2020-10-06T17:02:50Z

Description

Follow-up of #6839. In that PR, I only migrated the modules with breaking changes.

In this PR, all modules are migrated. For those without any breaking change, "migrating" just means decoding using amino, and re-encoding using JSONMarshaler.

Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

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
Review Codecov Report in the comment section below once CI passes

codecov · 2020-10-06T17:07:47Z

Codecov Report

Merging #7464 into master will increase coverage by 0.01%.
The diff coverage is 64.44%.

@@            Coverage Diff             @@
##           master    #7464      +/-   ##
==========================================
+ Coverage   56.02%   56.03%   +0.01%     
==========================================
  Files         591      592       +1     
  Lines       37177    37253      +76     
==========================================
+ Hits        20827    20875      +48     
- Misses      14232    14261      +29     
+ Partials     2118     2117       -1

amaury1093 · 2020-10-06T17:04:44Z

x/staking/legacy/v040/migrate.go

@@ -0,0 +1,116 @@
+package v040


I'll pay a beer to anyone who tells me if there's a way to make this boilerplate code shorter (esp the 5 for loops and some inner loops...)

You may be able to just amino JSON encode and decode from one version to the next. If there were no other changes, the amino JSON should be the same.

amaury1093 · 2020-10-06T17:06:07Z

x/staking/legacy/v038/types.go

@@ -56,8 +56,16 @@ type (

 	Validators []Validator

+	Params struct {


Fixing a 0.36->0.38 migration here

…igrate-followup

amaury1093 · 2020-10-07T15:44:30Z

I would need some help here, maybe @alexanderbez or @alessio ? We ran a small devnet inside Regen: using gaia, with 4 validators, around 100k blocks.

Here's the exported genesis after 100k blocks:
https://gist.github.com/amaurymartiny/10521ee802ab72ac7c5c384cc6c9e615

After running the CLI migrate command from this branch, here's the 0.40 genesis state:
https://gist.github.com/amaurymartiny/8dfb3ca35afa77f002193ca00b63429f

Repro

You can download the 0.40 genesis json, and run:

simd init --chain-id devnet-40 <your_name>
rm ~/.simapp/config/genesis.json
cp v040_genesis.json ~/.simapp/config/genesis.json
simd start

Expected

no error

Actual

$ ./build/simd start
I[2020-10-07|17:43:52.905] starting ABCI with Tendermint                module=main 
panic: invariant broken: staking: bonded and not bonded module account coins invariant
	Pool's bonded tokens: 101970100000stake
	sum of bonded tokens: 0
not bonded token invariance:
	Pool's not bonded tokens: 990000000stake
	sum of not bonded tokens: 102960100000
module accounts total (bonded + not bonded):
	Module Accounts' tokens: 102960100000stake
	sum tokens:              102960100000


	CRITICAL please submit the following transaction:
		 tx crisis invariant-broken staking module-accounts

alexanderbez · 2020-10-07T15:47:39Z

what version of Gaia did you export from? Did you go from 0.37 -> 0.39 -> 0.40? Also, did you export for zero-height?

amaury1093 · 2020-10-07T15:50:07Z

$ gaiacli version
2.0.13

Did you go from 0.37 -> 0.39 -> 0.40?

No, we didn't have a gaia updated to latest 0.40 yet. So we actually went from gaia (0.37) -> simapp (0.40), the error is coming from simapp.

Also, did you export for zero-height?

yes

alexanderbez · 2020-10-07T21:10:12Z

So we actually went from gaia (0.37) -> simapp (0.40)

How did you do this? Maybe there is a user error somewhere?

amaury1093 · 2020-10-07T21:29:44Z

How did you do this? Maybe there is a user error somewhere?

run gaia (0.37), export state with --for-zero-height. See the 1st gist for output.
checkout this branch of the SDK, run simd migrate 0.38, simd migrate 0.39, simd migrate 0.40 one after the other. The output is in the 2nd gist.
Assuming the final migrated genesis.json is called v040_genesis.json, run the repro I pasted above:

simd init --chain-id devnet-40 <your_name>
rm ~/.simapp/config/genesis.json
cp v040_genesis.json ~/.simapp/config/genesis.json
simd start

lmk if it's a user error (might be possible since we're running simapp on a gaia-exported genesis), not 100% sure, might also be an error in the CLI migrate command somewhere in the middle.

alexanderbez · 2020-10-07T21:35:51Z

Hmmm I think it'd be best to run the entire thing either against Gaia or Simapp and not switch tbh. That being said, it might be an error in migration. The bonded and non-bonded pools are wayyy off.

amaury1093 · 2020-10-08T18:30:11Z

Putting R4R, it'll be good to get a 1st round of review. We'll be testing internally at regen a bit more to find out why it panics (which might or might not be related to the migrate command).

zmanian · 2020-10-08T19:36:09Z

Just an FYI, we will be attempt non-zero height snapshot off the hub for a Stargate testnet next week. Would love to know more about the panic

…igrate-followup

…sdk into am-migrate-followup

types/staking.go

amaury1093 · 2020-10-09T10:06:48Z

The panic is fixed in 5d608ac, this PR is R4R now.

blushi

Pre-approving, this needs clarification #7464 (comment)

types/staking.go

x/genutil/legacy/v040/migrate.go

Co-authored-by: Marie Gauthier <[email protected]>

…sdk into am-migrate-followup

clevinson · 2020-10-09T17:00:47Z

Approving, but i'd like to see the int32 > enum piece still happen within v0.40.0 if possible. Should be a pretty small lift?

aaronc

ACK 🎉

aaronc · 2020-10-09T18:43:28Z

x/staking/legacy/v040/migrate.go

@@ -0,0 +1,116 @@
+package v040


You may be able to just amino JSON encode and decode from one version to the next. If there were no other changes, the amino JSON should be the same.

amaury1093 · 2020-10-09T19:08:16Z

@clevinson sounds good! I started #7499, can finish it Monday, and we can release an RC after.

Migrate staking module

267ef39

amaury1093 added C:CLI C:genesis relating to chain genesis C:x/genutil genutil module issues labels Oct 6, 2020

amaury1093 commented Oct 6, 2020

View reviewed changes

amaury1093 added 9 commits October 6, 2020 19:08

Merge branch 'master' into am-migrate-followup

0d50d31

Merge branch 'master' of ssh://github.com/cosmos/cosmos-sdk into am-m…

586ea7a

…igrate-followup

Add gov legacy

075a4ef

Add comments

e1521d5

Add x/distrib

ef05dde

x/crisis

e8488ae

x/mint

ab59c83

Merge branch 'master' into am-migrate-followup

a735812

Fix test

7770196

amaury1093 marked this pull request as ready for review October 8, 2020 18:29

amaury1093 requested review from aaronc, alessio and alexanderbez as code owners October 8, 2020 18:29

Merge branch 'master' into am-migrate-followup

0f8cf26

amaury1093 added 2 commits October 9, 2020 11:17

migrate x/genutil

f084fe1

Merge branch 'master' of ssh://github.com/cosmos/cosmos-sdk into am-m…

a97ca37

…igrate-followup

amaury1093 added 3 commits October 9, 2020 11:21

Merge branch 'am-migrate-followup' of ssh://github.com/cosmos/cosmos-…

846f63d

…sdk into am-migrate-followup

Fix lint

eb0a7ab

Fix staking constants

5d608ac

amaury1093 commented Oct 9, 2020

View reviewed changes

types/staking.go Outdated Show resolved Hide resolved

Fix test

06cb049

blushi approved these changes Oct 9, 2020

View reviewed changes

types/staking.go Outdated Show resolved Hide resolved

x/genutil/legacy/v040/migrate.go Outdated Show resolved Hide resolved

amaury1093 and others added 5 commits October 9, 2020 13:02

Update x/genutil/legacy/v040/migrate.go

b4d4289

Co-authored-by: Marie Gauthier <[email protected]>

Add migrate script instead of change BondStatus constants

b63535d

Merge branch 'am-migrate-followup' of ssh://github.com/cosmos/cosmos-…

9ca4e39

…sdk into am-migrate-followup

Fix test

6fcc163

Fix another test

2bc7a87

anilcse approved these changes Oct 9, 2020

View reviewed changes

amaury1093 mentioned this pull request Oct 9, 2020

Use enum instead of int32 for BondStatus #7499

Merged

10 tasks

aaronc approved these changes Oct 9, 2020

View reviewed changes

clevinson added the A:automerge Automatically merge PR once all prerequisites pass. label Oct 9, 2020

Merge branch 'master' into am-migrate-followup

e7c8a5a

mergify bot merged commit c14a3a7 into master Oct 9, 2020

mergify bot deleted the am-migrate-followup branch October 9, 2020 19:09

amaury1093 mentioned this pull request Oct 12, 2020

backport: Use enum instead of int32 for BondStatus (#7499) #7516

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CLI `migrate` command follow-up: decode & re-encode #7464

CLI `migrate` command follow-up: decode & re-encode #7464

amaury1093 commented Oct 6, 2020

codecov bot commented Oct 6, 2020 •

edited

Loading

amaury1093 Oct 6, 2020

aaronc Oct 9, 2020

amaury1093 Oct 6, 2020

amaury1093 commented Oct 7, 2020

alexanderbez commented Oct 7, 2020

amaury1093 commented Oct 7, 2020

alexanderbez commented Oct 7, 2020

amaury1093 commented Oct 7, 2020

alexanderbez commented Oct 7, 2020

amaury1093 commented Oct 8, 2020 •

edited

Loading

zmanian commented Oct 8, 2020

amaury1093 commented Oct 9, 2020

blushi left a comment •

edited

Loading

clevinson commented Oct 9, 2020

aaronc left a comment

aaronc Oct 9, 2020

amaury1093 commented Oct 9, 2020

		@@ -56,8 +56,16 @@ type (

		Validators []Validator

		Params struct {

CLI migrate command follow-up: decode & re-encode #7464

CLI migrate command follow-up: decode & re-encode #7464

Conversation

amaury1093 commented Oct 6, 2020

Description

codecov bot commented Oct 6, 2020 • edited Loading

Codecov Report

amaury1093 Oct 6, 2020

Choose a reason for hiding this comment

aaronc Oct 9, 2020

Choose a reason for hiding this comment

amaury1093 Oct 6, 2020

Choose a reason for hiding this comment

amaury1093 commented Oct 7, 2020

Repro

Expected

Actual

alexanderbez commented Oct 7, 2020

amaury1093 commented Oct 7, 2020

alexanderbez commented Oct 7, 2020

amaury1093 commented Oct 7, 2020

alexanderbez commented Oct 7, 2020

amaury1093 commented Oct 8, 2020 • edited Loading

zmanian commented Oct 8, 2020

amaury1093 commented Oct 9, 2020

blushi left a comment • edited Loading

Choose a reason for hiding this comment

clevinson commented Oct 9, 2020

aaronc left a comment

Choose a reason for hiding this comment

aaronc Oct 9, 2020

Choose a reason for hiding this comment

amaury1093 commented Oct 9, 2020

CLI `migrate` command follow-up: decode & re-encode #7464

CLI `migrate` command follow-up: decode & re-encode #7464

codecov bot commented Oct 6, 2020 •

edited

Loading

amaury1093 commented Oct 8, 2020 •

edited

Loading

blushi left a comment •

edited

Loading