Add Block Height to EpochInfo

[jeebster]

Closes: #452

Description

NOTES:

• start_height and current_epoch_start_height fields have been added following the timing scheme. If some of this information is unnecessary, please advise and I will deprecate accordingly
• protobuf compilation via make proto-all appears to have bumped the cosmos-sdk version dependency
• Please review logic refactoring in epoch module abci and genesis implementation and test files; I'm not too familiar with the protocol yet so I made some assumptions relative to the issue description

---

For contributor use:

• Targeted PR against correct branch (see CONTRIBUTING.md)
• Updated relevant documentation (docs/) or specification (x/<module>/spec/)
• Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
• Re-reviewed Files changed in the Github PR explorer

[ValarDragon]

Should just be one field correct? (I think just current_epoch_start_height

[jeebster]

addressed in 535e15c

[codecov-commenter]

Codecov Report

Merging #562 (a821daa) into main (0a105c9) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main     #562   +/-   ##
=======================================
  Coverage   20.31%   20.32%           
=======================================
  Files         164      164           
  Lines       23262    23264    +2     
=======================================
+ Hits         4726     4728    +2     
  Misses      17762    17762           
  Partials      774      774           

Impacted Files	Coverage Δ
x/gamm/types/balancerPool.pb.go	0.57% <ø> (ø)
x/epochs/abci.go	100.00% <100.00%> (ø)
x/epochs/genesis.go	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0a105c9...a821daa. Read the comment docs.

[ValarDragon]

Perfect, the change LGTM!

It just occurred to me, would it be easy to make a test where you manually protobuf serialized an epoch info prior to this change, and ensured that it can still be decoded in the new logic?

We basically need to double check that in order to not need any state migrations before it gets on mainnet. Sorry for not giving that feedback earlier.

[jeebster]

Perfect, the change LGTM!

It just occurred to me, would it be easy to make a test where you manually protobuf serialized an epoch info prior to this change, and ensured that it can still be decoded in the new logic?

We basically need to double check that in order to not need any state migrations before it gets on mainnet. Sorry for not giving that feedback earlier.

@ValarDragon I'll take a look into what this may entail - assuming mocking generated protobuf definitions and running backwards-compatible tests. I don't think this would be ideal but a temporary solution could be to check for the existence of the new proto property definition and removing the check after migration. Also, let me know if the base branch should be changed to a different branch if this shouldn't be targeting main

[ValarDragon]

The test shouldn't be too complex, protobuf should still allow parsing old serialized data with the new proto format, as long as no new required fields were added. (As is the case in this PR)

So it would protobuf serializing in old code, copying those bytes out, and then making a new test in this branch that tried proto deserializing those bytes and got no error

[jeebster]

@ValarDragon fbc9a1d should address legacy EpochInfo message compatibility

[ValarDragon]

Perfect, thanks!