InitGenesis in migrations when fromVersion==0 #9007

amaury1093 · 2021-03-26T12:26:07Z

Description

closes: #8989

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 · 2021-03-26T12:38:07Z

Codecov Report

Merging #9007 (7dff858) into master (e3a0148) will decrease coverage by 0.01%.
The diff coverage is 22.22%.

@@            Coverage Diff             @@
##           master    #9007      +/-   ##
==========================================
- Coverage   58.95%   58.93%   -0.02%     
==========================================
  Files         575      575              
  Lines       32191    32198       +7     
==========================================
  Hits        18977    18977              
- Misses      10999    11006       +7     
  Partials     2215     2215

Impacted Files	Coverage Δ
types/module/module.go	`68.36% <0.00%> (-5.26%)`	⬇️
simapp/app.go	`82.75% <100.00%> (-0.09%)`	⬇️
types/module/configurator.go	`18.51% <100.00%> (+3.13%)`	⬆️

types/module/configurator.go

orijbot · 2021-03-26T12:57:26Z

Benchmark finished. See the result: https://github.orijtech.com/benchmark/result?id=8c43a38225f743778d7d098bea3ed744

orijbot · 2021-03-26T12:57:34Z

Benchmark beginning. Status page: https://github.orijtech.com/benchmark/status?commit=8dab2b75f6a7228593d137a419f1da2a5008ddbe

orijbot · 2021-03-26T13:20:41Z

Benchmark finished. See the result: https://github.orijtech.com/benchmark/result?id=066f7c42d7aa49a5a45224066c28ca05

orijbot · 2021-03-26T13:20:50Z

Benchmark beginning. Status page: https://github.orijtech.com/benchmark/status?commit=86e02a4c72701f435fc46ccec458739221a26408

types/module/configurator.go

aaronc

This needs documentation at the function level. All the doc comments are inline, but user facing documentation needs to explain clearly all of the behavior here.

types/module/configurator.go

amaury1093 · 2021-04-01T09:38:53Z

simapp/app.go

@@ -339,7 +339,7 @@ func NewSimApp(

 	app.mm.RegisterInvariants(&app.CrisisKeeper)
 	app.mm.RegisterRoutes(app.Router(), app.QueryRouter(), encodingConfig.Amino)
-	app.configurator = module.NewConfigurator(app.MsgServiceRouter(), app.GRPCQueryRouter())
+	app.configurator = module.NewConfigurator(app.appCodec, app.MsgServiceRouter(), app.GRPCQueryRouter())


After the Configurator/module.Manager refactor, this would be the only place that might need to be updated by app developers (or maybe even not). The Configurator interface itself is not modified for now, to allow more flexibility on our side.

amaury1093 · 2021-04-01T09:53:10Z

this is r4r again

aaronc

Would like some doc updates but otherwise LGTM. In general the x/upgrade module docs need to be very clear in warning people to read everything very carefully.

types/module/module.go

x/upgrade/types/handler.go

amaury1093 · 2021-04-02T15:08:38Z

@technicallyty Would you be able to give this PR a review? and also double-check that your docs PR #8940 matches the same meaning as the functions comments from this PR?

technicallyty

LGTM, just some small nits leftover

types/module/module.go

Co-authored-by: technicallyty <48813565+technicallyty@users.noreply.github.com>

-include PR #9007 information

* upgrade draft docs * upgrade draft docs2 * change to generic id * Update docs/building-modules/upgrade.md Co-authored-by: Amaury <1293565+amaurym@users.noreply.github.com> * Update docs/building-modules/README.md Co-authored-by: Amaury <1293565+amaurym@users.noreply.github.com> * Update docs/building-modules/upgrade.md Co-authored-by: Amaury <1293565+amaurym@users.noreply.github.com> * Update docs/building-modules/upgrade.md Co-authored-by: Amaury <1293565+amaurym@users.noreply.github.com> * Update docs/building-modules/upgrade.md Co-authored-by: Amaury <1293565+amaurym@users.noreply.github.com> * Update docs/building-modules/upgrade.md Co-authored-by: Amaury <1293565+amaurym@users.noreply.github.com> * Update docs/building-modules/upgrade.md Co-authored-by: Amaury <1293565+amaurym@users.noreply.github.com> * Update docs/building-modules/upgrade.md Co-authored-by: Amaury <1293565+amaurym@users.noreply.github.com> * Update docs/core/README.md Co-authored-by: Amaury <1293565+amaurym@users.noreply.github.com> * Update docs/core/upgrade.md Co-authored-by: Amaury <1293565+amaurym@users.noreply.github.com> * Update docs/core/upgrade.md Co-authored-by: Amaury <1293565+amaurym@users.noreply.github.com> * added lines for version map and consensus versions * fix headers, add synopsis tag * docs for new modules * fix * remove line * Update docs/core/upgrade.md Co-authored-by: Amaury <1293565+amaurym@users.noreply.github.com> * Update docs/building-modules/upgrade.md Co-authored-by: Amaury <1293565+amaurym@users.noreply.github.com> * SetUpgradeHandler example snippet * -general clean up of docs -include PR #9007 information * Update docs/core/upgrade.md Co-authored-by: Amaury <1293565+amaurym@users.noreply.github.com> * Update docs/core/upgrade.md Co-authored-by: Amaury <1293565+amaurym@users.noreply.github.com> * Update docs/core/upgrade.md Co-authored-by: Amaury <1293565+amaurym@users.noreply.github.com> * Update docs/core/upgrade.md Co-authored-by: Amaury <1293565+amaurym@users.noreply.github.com> * Update docs/core/upgrade.md Co-authored-by: Amaury <1293565+amaurym@users.noreply.github.com> * Update docs/core/upgrade.md Co-authored-by: Amaury <1293565+amaurym@users.noreply.github.com> * Update docs/core/upgrade.md Co-authored-by: Amaury <1293565+amaurym@users.noreply.github.com> * Update docs/core/upgrade.md Co-authored-by: Amaury <1293565+amaurym@users.noreply.github.com> * Update docs/core/upgrade.md Co-authored-by: Amaury <1293565+amaurym@users.noreply.github.com> * Update docs/core/upgrade.md Co-authored-by: Amaury <1293565+amaurym@users.noreply.github.com> * Update docs/core/upgrade.md Co-authored-by: Amaury <1293565+amaurym@users.noreply.github.com> * Update docs/core/upgrade.md Co-authored-by: Amaury <1293565+amaurym@users.noreply.github.com> * Apply suggestions from code review Co-authored-by: Barrie Byron <barrie.byron@tendermint.com> * fix self-reference * clearer definitions for overwriting genesis functions * add sync section * forgot to save this * update description of initgenesis modules * specify upgrade method * Update docs/core/upgrade.md Co-authored-by: Barrie Byron <barrie.byron@tendermint.com> Co-authored-by: technicallyty <48813565+tytech3@users.noreply.github.com> Co-authored-by: Amaury <1293565+amaurym@users.noreply.github.com> Co-authored-by: Barrie Byron <barrie.byron@tendermint.com> Co-authored-by: Alessio Treglia <alessio@tendermint.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

* InitGenesis in migrations when fromVersion==0 * Add test * Fix test * Fix comment * cdc=>codec * Don't break Configurator * Remove method * Add comments * Add more comments * Update types/module/module.go Co-authored-by: technicallyty <48813565+technicallyty@users.noreply.github.com> * Update types/module/module.go Co-authored-by: technicallyty <48813565+technicallyty@users.noreply.github.com> * Update types/module/module.go Co-authored-by: technicallyty <48813565+technicallyty@users.noreply.github.com> Co-authored-by: technicallyty <48813565+technicallyty@users.noreply.github.com>

InitGenesis in migrations when fromVersion==0

6699ce5

amaury1093 added the T: Migration label Mar 26, 2021

amaury1093 commented Mar 26, 2021

View reviewed changes

types/module/configurator.go Outdated Show resolved Hide resolved

Add test

8dab2b7

amaury1093 added 2 commits March 26, 2021 14:19

Fix test

5850cae

Fix comment

86e02a4

amaury1093 marked this pull request as ready for review March 26, 2021 13:25

amaury1093 requested review from aaronc, alessio and alexanderbez as code owners March 26, 2021 13:25

amaury1093 requested review from technicallyty and robert-zaremba March 26, 2021 13:25

aaronc reviewed Mar 26, 2021

View reviewed changes

types/module/configurator.go Outdated Show resolved Hide resolved

cdc=>codec

a0e1bc8

aaronc requested changes Mar 29, 2021

View reviewed changes

types/module/configurator.go Outdated Show resolved Hide resolved

amaury1093 marked this pull request as draft March 30, 2021 14:20

amaury1093 added 4 commits March 31, 2021 19:34

Don't break Configurator

3fd6130

Remove method

735e553

Add comments

c33f820

Merge branch 'master' into am/8989-initgenesis-migration

8036dfa

amaury1093 commented Apr 1, 2021

View reviewed changes

amaury1093 marked this pull request as ready for review April 1, 2021 09:39

clevinson requested a review from aaronc April 2, 2021 14:24

aaronc approved these changes Apr 2, 2021

View reviewed changes

types/module/module.go Show resolved Hide resolved

x/upgrade/types/handler.go Outdated Show resolved Hide resolved

Add more comments

1999591

technicallyty approved these changes Apr 2, 2021

View reviewed changes

types/module/module.go Outdated Show resolved Hide resolved

types/module/module.go Outdated Show resolved Hide resolved

types/module/module.go Outdated Show resolved Hide resolved

amaury1093 and others added 4 commits April 2, 2021 17:23

Update types/module/module.go

16232c1

Co-authored-by: technicallyty <48813565+technicallyty@users.noreply.github.com>

Update types/module/module.go

c1e6b74

Co-authored-by: technicallyty <48813565+technicallyty@users.noreply.github.com>

Update types/module/module.go

625b892

Co-authored-by: technicallyty <48813565+technicallyty@users.noreply.github.com>

Merge branch 'master' into am/8989-initgenesis-migration

7dff858

amaury1093 added the A:automerge Automatically merge PR once all prerequisites pass. label Apr 2, 2021

mergify bot merged commit 5d13b1f into master Apr 2, 2021

mergify bot deleted the am/8989-initgenesis-migration branch April 2, 2021 15:41

technicallyty added a commit that referenced this pull request Apr 2, 2021

-general clean up of docs

5bc3bf6

-include PR #9007 information

amaury1093 mentioned this pull request Apr 6, 2021

documentation for in-place migrations with x/upgrade module #8967

Merged

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

InitGenesis in migrations when fromVersion==0 #9007

InitGenesis in migrations when fromVersion==0 #9007

amaury1093 commented Mar 26, 2021

codecov bot commented Mar 26, 2021 •

edited

Loading

orijbot commented Mar 26, 2021

orijbot commented Mar 26, 2021

orijbot commented Mar 26, 2021

orijbot commented Mar 26, 2021

aaronc left a comment

amaury1093 Apr 1, 2021

amaury1093 commented Apr 1, 2021

aaronc left a comment

amaury1093 commented Apr 2, 2021

technicallyty left a comment

InitGenesis in migrations when fromVersion==0 #9007

InitGenesis in migrations when fromVersion==0 #9007

Conversation

amaury1093 commented Mar 26, 2021

Description

codecov bot commented Mar 26, 2021 • edited Loading

Codecov Report

orijbot commented Mar 26, 2021

orijbot commented Mar 26, 2021

orijbot commented Mar 26, 2021

orijbot commented Mar 26, 2021

aaronc left a comment

Choose a reason for hiding this comment

amaury1093 Apr 1, 2021

Choose a reason for hiding this comment

amaury1093 commented Apr 1, 2021

aaronc left a comment

Choose a reason for hiding this comment

amaury1093 commented Apr 2, 2021

technicallyty left a comment

Choose a reason for hiding this comment

codecov bot commented Mar 26, 2021 •

edited

Loading