epoching: fix genesis, param and bootstrapping #41
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thanks for the fix @SebastianElvis ! Looking at the build it seems that it is failing due to the |
Yeah somehow I broke some other stuff in the module... Will let you know when I fix it. |
|
@vitsalis After the last commit my local computer can run the CI pipelines without error. However the CI seems to still fail. Could you please try it locally and see if you can replicate the error? Thanks! |
|
For me, both the replicated CI commands and |
|
It's weird. The problem can also be triggered by The stacktrace basically says that some genesis is empty and cannot be deserialised. It's quite weird that the error comes from other modules, e.g., |
|
My last commit fixes all errors in my local machine and Docker. It turns out that to reproduce this error, one needs to remove the entire database ( The root cause is still the AnteHandler: initialising AnteHandlers happens earlier than exporting genesis states. My initial AnteHandler implementation needs to retrieve the epoch number and thus needs to query the DB, which at that moment is still not initilaised yet. A straightforward fix is to use block height in However, the CI still fails with the output Log in CI: |
Confirmed, |
|
Indeed, the issue is with the build right now. Reverted the commit until a more stable fix for the build is found. |
|
After merging the current |
aakoshh
reviewed
Jul 1, 2022
| return nil | ||
| } | ||
|
|
||
| func validateEpochInterval(i interface{}) error { |
There was a problem hiding this comment.
I don't understand why this method takes an interface{}. It's defined as uint64.
There was a problem hiding this comment.
This seems to be a convention in Cosmos SDK (e.g., https://github.com/cosmos/cosmos-sdk/blob/v0.45.5/x/staking/types/params.go). I guess this extra type check aims to rule out the issues during serialisation/deserialisation.
There was a problem hiding this comment.
How odd. You would think the reason to have Protobuf have a schema is so we don't have to worry about stuff like this.
There was a problem hiding this comment.
It's definitely an uin64 in https://github.com/babylonchain/babylon/blob/main/x/epoching/types/params.pb.go#L29
Probably the only use case for this test is to prevent changing the type in the proto.
Hopefully there don't need to be such tests for each field of each type 😖
|
Thanks for the entire process Vitalis and Akosh! Learned a lot. |
KonradStaniec
pushed a commit
that referenced
this pull request
Oct 25, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #36 and https://babylon-chain.atlassian.net/browse/BM-48
This PR implements the bootstrapping process for the epoching module such as genesis, parameters, and fixes a bootstrapping issue caused by
NewDropValidatorMsgDecorator.Root cause of the issue: Genesis block includes a set of txs for boostrapping the initial validator. These txs include msg types that are related to the validator set and thus are rejected by
NewDropValidatorMsgDecorator. To fix this, we will have to allow such validator-related msgs in the genesis. Other fixes might require touching the genesis state generation in Cosmos SDK, and thus are not desired for us.