Skip to content

fix(kvstore): abci Info() returns invalid height at genesis#474

Merged
lklimek merged 4 commits intov0.10-devfrom
fix-kvstore-initial-height
Oct 21, 2022
Merged

fix(kvstore): abci Info() returns invalid height at genesis#474
lklimek merged 4 commits intov0.10-devfrom
fix-kvstore-initial-height

Conversation

@lklimek
Copy link
Collaborator

@lklimek lklimek commented Oct 7, 2022

Issue being fixed or feature implemented

For initial (genesis) height 1000, Example kvstore (used in tests) returns 999 in response to Info() call.

What was done?

Info() will return 0 for initial height.

Internally, kvstore.State stores height 0 if we are at genesis. It also stores InitialHeight and uses it to increment from 0 directly to InitialHeight(), as needed.

How Has This Been Tested?

  • unit tests - expanded one unit test

Breaking Changes

None

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have made corresponding changes to the documentation

For repository code-owners and collaborators only

  • I have assigned this pull request to a milestone

@lklimek lklimek added this to the v0.10.0 milestone Oct 7, 2022
Copy link
Collaborator

@shotonoff shotonoff left a comment

Choose a reason for hiding this comment

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

LGTM

lklimek and others added 2 commits October 21, 2022 09:24
// GetHeight returns height of the state
GetHeight() int64
// IncrementHeight increments height by 1
// IncrementHeight increments height by 1, but not lower than initial height.
Copy link
Collaborator

Choose a reason for hiding this comment

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

if I understand correctly, we never can be lower because we only grow

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If height == 0 and initialHeight == 1000, then IncrementHeight() sets height to 1000

@lklimek lklimek merged commit 3e4ea85 into v0.10-dev Oct 21, 2022
@lklimek lklimek deleted the fix-kvstore-initial-height branch October 21, 2022 08:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants