FIX: checkpointing/changed PoP by signing BLS public key #97
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
aakoshh
approved these changes
Aug 17, 2022
privval/types.go
Outdated
| // where valPrivKey is Ed25519_sk, accPubkey is Secp256k1_pk, blsPrivkey is BLS_sk | ||
| func BuildPop(valPrivKey tmcrypto.PrivKey, blsPrivkey bls12381.PrivateKey, accPubkey cryptotypes.PubKey) (*types.ProofOfPossession, error) { | ||
| data, err := valPrivKey.Sign(accPubkey.Bytes()) | ||
| // BuildPoP builds a proof-of-possession by PoP=encrypt(key = BLS_sk, data = encrypt(key = Ed25519_sk, data = BLS_pk)) |
There was a problem hiding this comment.
Should we say sign instead of encrypt?
x/checkpointing/spec/registration.md
Outdated
| 3. Otherwise, the corresponding BLS key registered before should be removed to ensure atomicity. | ||
| 4. The validator should register again. | ||
| 1. The `Checkpointing` module first processes `MsgWrappedCreateValidator` to register the validator's BLS key. If success, then | ||
| 2. extract `MsgCreateValidator` and deliver `MsgCreateValidator` to the epoching module's message queue, which will be processed until the end of this epoch. If success, the registration is succeeded. |
There was a problem hiding this comment.
Suggested change
| 2. extract `MsgCreateValidator` and deliver `MsgCreateValidator` to the epoching module's message queue, which will be processed until the end of this epoch. If success, the registration is succeeded. | |
| 2. extract `MsgCreateValidator` and deliver `MsgCreateValidator` to the epoching module's message queue, which will be processed at the end of this epoch. If success, the registration is succeeded. |
|
|
||
| Genesis validators are registered via the legacy `genutil` module from the Cosmos-SDK, which processes `MsgCreateValidator` messages contained in genesis transactions. | ||
| The BLS keys are registered as `GenesisState` in the checkpointing module. | ||
| The checkpointing module's `ValidateGenesis` should ensure that each genesis validator has both an Ed25519 key and BLS key which are bonded by PoP. |
There was a problem hiding this comment.
I don't think the ValidateGenesis method can do this because this seems to run on just the module specific part of the genesis.json part, so it won't be able to see both the genutil and the checkpointing parts, and because there's no execution, it also can't look up validator keys. That's why I suggested there to be a dedicated CLI command to do the validation, by accessing both parts. ValidateGenesis can validate the PoP, though.
There was a problem hiding this comment.
Thanks for the comment, I'll revise the doc in the following PR.
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.
This PR follows the discussions in a PR where we decided to change PoP by signing the BLS public key instead of the account's public key.
This change works because
MsgCreateValidatoris signed by both the delegator and the validator. Signing BLS public key will not break the binding between the delegator's key and the validator's key while maintaining the binding between the BLS key and the validator's key.This change is better to be done in a separate PR.