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

feat: add validate-genesis cmd #187

Merged
merged 3 commits into from
Nov 4, 2022
Merged

feat: add validate-genesis cmd #187

merged 3 commits into from
Nov 4, 2022

Conversation

gitferry
Copy link
Contributor

@gitferry gitferry commented Nov 3, 2022

This PR adds validate-genesis command. The implementation is adapted from the original validate-genesis command. The additional check is about the correspondence of genesis txs and genesis BLS keys. The example of using the command is as follows.

babylond validate-genesis genesis.json
File at genesis.json is a valid genesis file

Copy link
Member

@vitsalis vitsalis left a comment

Choose a reason for hiding this comment

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

Looks good! Some suggestions

// validation rule
// 2. each genesis BLS key or gentx should have a corresponding gentx or genesis
// BLS key
// modified based on "github.com/cosmos/cosmos-sdk/x/genutil/client/cli/validate_genesis.go"
Copy link
Member

Choose a reason for hiding this comment

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

This link does not lead to a file. Note to also use the commit in the link instead of a tag/branch

}
if len(addresses) != len(gks) {
return errors.New("duplicate genesis BLS keys")
}
Copy link
Member

Choose a reason for hiding this comment

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

Since you check that on the Validate function of the checkpointing module, you don't need to do it again here as the ValidateFunction already gets called.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is true, but I just thought CheckCorrespondence is rather independent. It does not hurt if we do the length checking here again. What if some day we accidentally move CheckCorrespondence before the module validation?

if len(addresses) != len(gks) {
return errors.New("duplicate genesis BLS keys")
}
if len(addresses) != len(genTxState.GenTxs) {
Copy link
Member

Choose a reason for hiding this comment

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

This check means that you don't have to do a reverse loop later to check whether all BLS sigs have a matching gentx, right? Maybe worth adding a comment about your checking algorithm.

}
msgCreateValidator := msgs[0].(*stakingtypes.MsgCreateValidator)
if _, exists := addresses[msgCreateValidator.ValidatorAddress]; !exists {
return errors.New("genesis txs and genesis BLS keys do not match")
Copy link
Member

Choose a reason for hiding this comment

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

Maybe give details about what is not matching? Also, maybe say that there is no BLS key for the gentx.

"testing"
)

var misMatchGenesis = `
Copy link
Member

Choose a reason for hiding this comment

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

Could those structures be created in a programmatic manner? Would make this a bit cleaner.

"github.com/babylonchain/babylon/x/checkpointing/types"
)

const chainUpgradeGuide = "https://github.com/cosmos/cosmos-sdk/blob/main/UPGRADING.md"
Copy link
Member

Choose a reason for hiding this comment

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

The main branch here is going to change over time. This should point to an unchangeable link to the version that we are using.

@gitferry
Copy link
Contributor Author

gitferry commented Nov 4, 2022

Thanks, @vitsalis for your suggestions. To generate genesis docs for testing in a programmatic manner, I exported TestnetCmd. Hope that would not cause any issues.

@gitferry gitferry merged commit 7c2f02b into main Nov 4, 2022
@gitferry gitferry deleted the genesis/validate-genesis branch November 4, 2022 05:13
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