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

R4R: Add a flag to export for zero-height start #2827

Merged
merged 73 commits into from
Nov 26, 2018

Conversation

cwgoes
Copy link
Contributor

@cwgoes cwgoes commented Nov 15, 2018

Closes #2812

Depends on #2807 and #2825, can be retargeted to develop once those PRs are merged.

This PR adds the flag --for-zero-height to gaiad export, which runs several alterations to the application state to prepare for restarting a new chain in a consistent fashion.

It also:

  • Moves Gaia's export code to cmd/gaia/app/export.go for cleaner separation.
  • Fixes an inconsistency where we treated the initChainer as happening at height -1 - it should now happen at height 0, since the first header sent by Tendermint has height 1.
  • Runs the runtime invariant checks on start (in initChainer)
  • Adds a few auxiliary functions to clear slashing periods
  • Removes the Height field from Delegation objects in x/stake, which was not used anywhere

Standard checklist:

  • Linked to github-issue with discussion and accepted design OR link to spec that describes this work.
  • Wrote tests
  • Updated relevant documentation (docs/)
  • Added entries in PENDING.md with issue #
  • 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)

@cwgoes cwgoes added wip tooling dev tooling within the sdk labels Nov 15, 2018
@codecov
Copy link

codecov bot commented Nov 15, 2018

Codecov Report

❗ No coverage uploaded for pull request base (develop@7cb1ba6). Click here to learn what that means.
The diff coverage is 34.78%.

@@            Coverage Diff             @@
##             develop    #2827   +/-   ##
==========================================
  Coverage           ?   56.73%           
==========================================
  Files              ?      120           
  Lines              ?     8333           
  Branches           ?        0           
==========================================
  Hits               ?     4728           
  Misses             ?     3286           
  Partials           ?      319

@cwgoes cwgoes changed the base branch from develop to cwgoes/runtime-assertable-invariants November 15, 2018 15:04
@@ -21,8 +21,8 @@ func InitGenesis(ctx sdk.Context, keeper Keeper, data types.GenesisState) (res [

// We need to pretend to be "n blocks before genesis", where "n" is the validator update delay,
// so that e.g. slashing periods are correctly initialized for the validator set
// e.g. with a one-block offset - the first TM block is at height 0, so state updates applied from genesis.json are in block -1.
ctx = ctx.WithBlockHeight(-types.ValidatorUpdateDelay)
// e.g. with a one-block offset - the first TM block is at height 1, so state updates applied from genesis.json are in block 0.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tendermint used to start at block height 0, now it starts at 1, so we needed to update this.

@cwgoes cwgoes changed the title WIP: Add a flag to export for zero-height start R4R: Add a flag to export for zero-height start Nov 15, 2018
Copy link
Contributor

@rigelrozanski rigelrozanski left a comment

Choose a reason for hiding this comment

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

woot! - few comments herein - Also did a push for some formatting and a missing Close()

cmd/gaia/app/export.go Outdated Show resolved Hide resolved
cmd/gaia/app/export.go Show resolved Hide resolved
cmd/gaia/app/export.go Outdated Show resolved Hide resolved
x/slashing/slashing_period.go Outdated Show resolved Hide resolved
cmd/gaia/app/sim_test.go Outdated Show resolved Hide resolved
@cwgoes
Copy link
Contributor Author

cwgoes commented Nov 22, 2018

Retargeted on #2825 for the new invariants.

@cwgoes cwgoes changed the base branch from develop to rigel/blockly-mint November 22, 2018 13:36
Copy link
Contributor

@rigelrozanski rigelrozanski left a comment

Choose a reason for hiding this comment

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

Thanks Chris!

@cwgoes cwgoes requested a review from sunnya97 November 23, 2018 14:19
@cwgoes
Copy link
Contributor Author

cwgoes commented Nov 23, 2018

OK, there's a latent distribution oddity somewhere. It isn't introduced by this PR, but it seems to have been discovered only now. I've added the invariant to the general simulation, which now fails on some seeds.

@rigelrozanski
Copy link
Contributor

@cwgoes you should test a merge on top of some of the bugfixes in #2825 - could easily be related as those bugfixes related to distribution

@jaekwon
Copy link
Contributor

jaekwon commented Nov 25, 2018

To debug this, I'm improving the simulator a bit...
#2897

@rigelrozanski I've verified on my branch above that the bugfixes do not address chris's new bug. NOTE: chris's new invariance has been added to the above PR. It changes usage of rand, so we'll need to find new seeds that cause the problem.

@jaekwon jaekwon changed the base branch from rigel/blockly-mint to develop November 26, 2018 11:56
@jaekwon jaekwon changed the base branch from develop to rigel/blockly-mint November 26, 2018 12:01
@jaekwon jaekwon changed the base branch from rigel/blockly-mint to develop November 26, 2018 12:16
@jaekwon jaekwon merged commit ad121f1 into develop Nov 26, 2018
@cwgoes cwgoes deleted the cwgoes/export-for-zero-height branch November 26, 2018 12:28
chillyvee pushed a commit to chillyvee/cosmos-sdk that referenced this pull request Mar 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tooling dev tooling within the sdk
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants