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

Genesis port script v0.34.0 #4023

Merged
merged 29 commits into from
Apr 8, 2019

Conversation

fedekunze
Copy link
Collaborator

@fedekunze fedekunze commented Apr 2, 2019

Closes #4018

Run using

# if using a 0.33.0 chain
git checkout v0.33.2 && make install

# using the 0.33.1 or 0.33.2 branch
gaiad export > [exported_genesis.json]

# delete old data
gaiad unsafe-reset-all

# checkout the fedekunze/4018-genesis-scritp and build
git checkout fedekunze/4018-genesis-scritp && make install

# modify the genesis to be compatible with 0.34
python3 contrib/export/v0.33.x-to-v0.34.0.py --chain-id=<chain_id> \
    --start-time=<genesis-start-time-rfc3339> \
    <path_to_old_genesis.json> > [path_to_new_genesis.json]

# update to 0.34
git checkout release/v0.34.0 && make install

# restart chain
  • 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.

  • Wrote tests

  • Updated relevant documentation (docs/)

  • Added a relevant changelog entry: sdkch add [section] [stanza] [message]

  • rereviewed Files changed in the github PR explorer


For Admin Use:

  • Added appropriate labels to PR (ex. wip, ready-for-review, docs)
  • Reviewers Assigned
  • Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

@alexanderbez alexanderbez added WIP C:genesis relating to chain genesis labels Apr 2, 2019
@codecov
Copy link

codecov bot commented Apr 2, 2019

Codecov Report

Merging #4023 into release/v0.34.0 will increase coverage by 0.01%.
The diff coverage is n/a.

@@                 Coverage Diff                 @@
##           release/v0.34.0    #4023      +/-   ##
===================================================
+ Coverage            59.99%   60.01%   +0.01%     
===================================================
  Files                  212      212              
  Lines                15107    15107              
===================================================
+ Hits                  9064     9066       +2     
+ Misses                5422     5420       -2     
  Partials               621      621

cmd/gaia/app/genesis_port.go Outdated Show resolved Hide resolved
server/util.go Outdated Show resolved Hide resolved
@fedekunze fedekunze changed the base branch from develop to release/v0.34.0 April 3, 2019 16:58
@fedekunze fedekunze marked this pull request as ready for review April 3, 2019 17:32
@alexanderbez alexanderbez mentioned this pull request Apr 3, 2019
10 tasks
@alexanderbez
Copy link
Contributor

alexanderbez commented Apr 3, 2019

I will test this on my full node shortly. Also, here is a WIP gist I have of the full upgrade instructions.

Copy link
Contributor

@alexanderbez alexanderbez left a comment

Choose a reason for hiding this comment

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

Good work @fedekunze! Left some comments. Will test on my full node shortly.

scripts/export/export.go Outdated Show resolved Hide resolved
scripts/export/export.go Outdated Show resolved Hide resolved
scripts/export/export.go Outdated Show resolved Hide resolved
scripts/export/v0_33_0/main.go Outdated Show resolved Hide resolved
scripts/export/v0_33_0/main.go Outdated Show resolved Hide resolved
@fedekunze fedekunze dismissed alexanderbez’s stale review April 3, 2019 22:11

addressed comments

Copy link
Contributor

@cwgoes cwgoes left a comment

Choose a reason for hiding this comment

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

According to @rigelrozanski this needs to set a high constant fee for the new crisis module's invariant check transaction.

Copy link
Contributor

@alexanderbez alexanderbez left a comment

Choose a reason for hiding this comment

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

Yes, this needs the invariant fee param (from what @cwgoes suggested).

@fedekunze
Copy link
Collaborator Author

According to @rigelrozanski this needs to set a high constant fee for the new crisis module's invariant check transaction.

What's the value for it ?

@rigelrozanski
Copy link
Contributor

$5000 USD was suggested by jae online

@fedekunze
Copy link
Collaborator Author

updated, please test again

@gamarin2
Copy link
Contributor

gamarin2 commented Apr 7, 2019

I think it'd be great to add the procedure in /docs/cosmos-hub, something like upgrade-procedure.md

Copy link
Contributor

@alessio alessio left a comment

Choose a reason for hiding this comment

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

@fedekunze could we just rename this to scripts/export/v0_33_0.py?

- script moved from scripts/ to contrib/export
- Main and aux functions refactored and generalised
  for increased reusability in future scripts.
- .gitignore updated
@alexanderbez
Copy link
Contributor

@gamarin2 I have a personal gist that I can port to here.

contrib/export/v0.33.2-to-v0.34.0.py Outdated Show resolved Hide resolved
contrib/export/v0.33.2-to-v0.34.0.py Outdated Show resolved Hide resolved
contrib/export/lib.py Outdated Show resolved Hide resolved
cwgoes
cwgoes previously requested changes Apr 8, 2019
Copy link
Contributor

@cwgoes cwgoes left a comment

Choose a reason for hiding this comment

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

There's a typo I think - need to be careful since if we don't overwrite the old field it will remain (and the new field with the wrong name will be ignored).

contrib/export/v0.33.x-to-v0.34.0.py Outdated Show resolved Hide resolved
Co-Authored-By: fedekunze <31522760+fedekunze@users.noreply.github.com>
@fedekunze fedekunze dismissed cwgoes’s stale review April 8, 2019 15:33

good catch, updated

Copy link
Contributor

@cwgoes cwgoes left a comment

Choose a reason for hiding this comment

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

ACK - we should test this again with the exact exported genesis from mainnet once we have it.

@alexanderbez
Copy link
Contributor

Tested an export on my mainnet full node w/o any hiccups.

@alexanderbez alexanderbez merged commit bec4689 into release/v0.34.0 Apr 8, 2019
@alexanderbez alexanderbez deleted the fedekunze/4018-genesis-scritp branch April 8, 2019 16:49
@fedekunze fedekunze mentioned this pull request May 24, 2019
6 tasks
@fedekunze fedekunze mentioned this pull request Jun 4, 2019
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C:genesis relating to chain genesis
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants