fix(x/staking): Check existing pubkeys when creating a Validator + refactors (#20713)

Co-authored-by: Hieu Vu <72878483+hieuvubk@users.noreply.github.com>
Co-authored-by: Ezequiel Raynaudo <raynaudo.ee@gmail.com>
This commit is contained in:
Facundo Medica 2024-06-19 17:52:47 +02:00 committed by GitHub
parent 2d014b26c2
commit 3c13bc03cd
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
10 changed files with 112 additions and 60 deletions

View File

@ -264,7 +264,7 @@ func TestRotateConsPubKey(t *testing.T) {
validator: validators[0].GetOperator(),
newPubKey: validators[1].ConsensusPubkey.GetCachedValue().(cryptotypes.PubKey),
pass: false,
expErrMsg: "consensus pubkey is already used for a validator",
expErrMsg: "validator already exist for this pubkey; must use new validator pubkey",
},
{
name: "consensus pubkey rotation limit check",
@ -314,7 +314,6 @@ func TestRotateConsPubKey(t *testing.T) {
// this shouldn't remove the existing keys from waiting queue since unbonding time isn't reached
_, err = stakingKeeper.EndBlocker(ctx)
assert.NilError(t, err)
// stakingKeeper.UpdateAllMaturedConsKeyRotatedKeys(ctx, ctx.BlockHeader().Time)
msg, err = types.NewMsgRotateConsPubKey(
validators[3].GetOperator(),
@ -331,7 +330,6 @@ func TestRotateConsPubKey(t *testing.T) {
// this should remove keys from waiting queue since unbonding time is reached
_, err = stakingKeeper.EndBlocker(newCtx)
assert.NilError(t, err)
// stakingKeeper.UpdateAllMaturedConsKeyRotatedKeys(newCtx, newCtx.BlockHeader().Time)
return newCtx
},

View File

@ -278,9 +278,9 @@ ValidatorConsensusKeyRotationRecordQueueKey: `103 | format(time) -> ProtocolBuff
ValidatorConsensusKeyRotationRecordIndexKey:`104 | valAddr | format(time) -> ProtocolBuffer([]Byte{})`
OldToNewConsKeyMap:`105 | byte(oldConsKey) -> byte(newConsKey)`
OldToNewConsAddrMap:`105 | byte(oldConsAddr) -> byte(newConsAddr)`
ConsKeyToValidatorIdentifierMap:`106 | byte(newConsKey) -> byte(initialConsKey)`
ConsAddrToValidatorIdentifierMap:`106 | byte(newConsAddr) -> byte(initialConsAddr)`
`ConsPubKeyRotationHistory` is used for querying the rotations of a validator
@ -290,9 +290,9 @@ ConsKeyToValidatorIdentifierMap:`106 | byte(newConsKey) -> byte(initialConsKey)`
A `ConsPubKeyRotationHistory` object is created every time a consensus pubkey rotation occurs.
An entry is added in `OldToNewConsKeyMap` collection for every rotation (Note: this is to handle the evidences when submitted with old cons key).
An entry is added in `OldToNewConsAddrMap` collection for every rotation (Note: this is to handle the evidences when submitted with old cons key).
An entry is added in `ConsKeyToValidatorIdentifierMap` collection for every rotation, this entry is to block the rotation if the validator is rotating to the cons key which is involved in the history.
An entry is added in `ConsAddrToValidatorIdentifierMap` collection for every rotation, this entry is to block the rotation if the validator is rotating to the cons key which is involved in the history.
To prevent the spam:

View File

@ -42,7 +42,7 @@ func (k Keeper) setConsPubKeyRotationHistory(
}
for _, r := range allRotations {
if r.NewConsPubkey.Compare(newPubKey) == 0 {
return types.ErrConsensusPubKeyAlreadyUsedForValidator
return types.ErrValidatorPubKeyExists
}
}
@ -105,37 +105,36 @@ func (k Keeper) updateToNewPubkey(ctx context.Context, val types.Validator, oldP
}
// sets a map: oldConsKey -> newConsKey
if err := k.OldToNewConsKeyMap.Set(ctx, oldPk.Address(), newPk.Address()); err != nil {
if err := k.OldToNewConsAddrMap.Set(ctx, oldPk.Address(), newPk.Address()); err != nil {
return err
}
// sets a map: newConsKey -> initialConsKey
if err := k.setConsKeyToValidatorIdentifierMap(ctx, sdk.ConsAddress(oldPk.Address()), sdk.ConsAddress(newPk.Address())); err != nil {
if err := k.setConsAddrToValidatorIdentifierMap(ctx, sdk.ConsAddress(oldPk.Address()), sdk.ConsAddress(newPk.Address())); err != nil {
return err
}
return k.Hooks().AfterConsensusPubKeyUpdate(ctx, oldPk, newPk, fee)
}
// setConsKeyToValidatorIdentifierMap adds an entry in the state with the current consKey to the initial consKey of the validator.
// it tries to find the oldPk if there is a entry already present in the state
func (k Keeper) setConsKeyToValidatorIdentifierMap(ctx context.Context, oldPk, newPk sdk.ConsAddress) error {
pk, err := k.ConsKeyToValidatorIdentifierMap.Get(ctx, oldPk)
// setConsAddrToValidatorIdentifierMap adds an entry in the state with the current consAddr to the initial consAddr of the validator.
// It first tries to find the validatorIdentifier if there is a entry already present in the state.
func (k Keeper) setConsAddrToValidatorIdentifierMap(ctx context.Context, oldConsAddr, newConsAddr sdk.ConsAddress) error {
validatorIdentifier, err := k.ConsAddrToValidatorIdentifierMap.Get(ctx, oldConsAddr)
if err != nil && !errors.Is(err, collections.ErrNotFound) {
return err
}
if pk != nil {
oldPk = pk
if validatorIdentifier != nil {
oldConsAddr = validatorIdentifier
}
return k.ConsKeyToValidatorIdentifierMap.Set(ctx, newPk, oldPk)
return k.ConsAddrToValidatorIdentifierMap.Set(ctx, newConsAddr, oldConsAddr)
}
// ValidatorIdentifier maps the new cons key to previous cons key (which is the address before the rotation).
// (that is: newConsKey -> oldConsKey)
// (that is: newConsAddr -> oldConsAddr)
func (k Keeper) ValidatorIdentifier(ctx context.Context, newPk sdk.ConsAddress) (sdk.ConsAddress, error) {
pk, err := k.ConsKeyToValidatorIdentifierMap.Get(ctx, newPk)
pk, err := k.ConsAddrToValidatorIdentifierMap.Get(ctx, newPk)
if err != nil && !errors.Is(err, collections.ErrNotFound) {
return nil, err
}

View File

@ -122,10 +122,10 @@ type Keeper struct {
ValidatorConsensusKeyRotationRecordIndexKey collections.KeySet[collections.Pair[[]byte, time.Time]]
// ValidatorConsensusKeyRotationRecordQueue: this key is used to set the unbonding period time on each rotation
ValidatorConsensusKeyRotationRecordQueue collections.Map[time.Time, types.ValAddrsOfRotatedConsKeys]
// ConsKeyToValidatorIdentifierMap: maps the new cons key to the initial cons key
ConsKeyToValidatorIdentifierMap collections.Map[[]byte, []byte]
// OldToNewConsKeyMap: maps the old cons key to the new cons key
OldToNewConsKeyMap collections.Map[[]byte, []byte]
// ConsAddrToValidatorIdentifierMap: maps the new cons addr to the initial cons addr
ConsAddrToValidatorIdentifierMap collections.Map[[]byte, []byte]
// OldToNewConsAddrMap: maps the old cons addr to the new cons addr
OldToNewConsAddrMap collections.Map[[]byte, []byte]
// ValidatorConsPubKeyRotationHistory: consPubkey rotation history by validator
// A index is being added with key `BlockConsPubKeyRotationHistory`: consPubkey rotation history by height
RotationHistory *collections.IndexedMap[collections.Pair[[]byte, uint64], types.ConsPubKeyRotationHistory, rotationHistoryIndexes]
@ -280,16 +280,16 @@ func NewKeeper(
),
// key format is: 105 | consAddr
ConsKeyToValidatorIdentifierMap: collections.NewMap(
sb, types.ConsKeyToValidatorIdentifierMapPrefix,
"new_to_old_cons_key_map",
ConsAddrToValidatorIdentifierMap: collections.NewMap(
sb, types.ConsAddrToValidatorIdentifierMapPrefix,
"new_to_old_cons_addr_map",
collections.BytesKey,
collections.BytesValue,
),
// key format is: 106 | consAddr
OldToNewConsKeyMap: collections.NewMap(
sb, types.OldToNewConsKeyMap,
OldToNewConsAddrMap: collections.NewMap(
sb, types.OldToNewConsAddrMap,
"old_to_new_cons_key_map",
collections.BytesKey,
collections.BytesValue,

View File

@ -1,6 +1,7 @@
package keeper
import (
"bytes"
"context"
"errors"
"fmt"
@ -89,8 +90,9 @@ func (k msgServer) CreateValidator(ctx context.Context, msg *types.MsgCreateVali
}
}
if _, err := k.GetValidatorByConsAddr(ctx, sdk.GetConsAddress(pk)); err == nil {
return nil, types.ErrValidatorPubKeyExists
err = k.checkConsKeyAlreadyUsed(ctx, pk)
if err != nil {
return nil, err
}
bondDenom, err := k.BondDenom(ctx)
@ -649,40 +651,26 @@ func (k msgServer) RotateConsPubKey(ctx context.Context, msg *types.MsgRotateCon
return nil, errorsmod.Wrapf(sdkerrors.ErrInvalidType, "expecting cryptotypes.PubKey, got %T", cv)
}
// check cons key is already present in the key rotation history.
rotatedTo, err := k.ConsKeyToValidatorIdentifierMap.Get(ctx, pk.Address())
if err != nil && !errors.Is(err, collections.ErrNotFound) {
err = k.checkConsKeyAlreadyUsed(ctx, pk)
if err != nil {
return nil, err
}
if rotatedTo != nil {
return nil, errorsmod.Wrap(sdkerrors.ErrInvalidAddress,
"the new public key is already present in rotation history, please try with a different one")
}
newConsAddr := sdk.ConsAddress(pk.Address())
// checks if NewPubKey is not duplicated on ValidatorsByConsAddr
_, err = k.Keeper.ValidatorByConsAddr(ctx, newConsAddr)
if err == nil {
return nil, types.ErrConsensusPubKeyAlreadyUsedForValidator
}
valAddr, err := k.validatorAddressCodec.StringToBytes(msg.ValidatorAddress)
if err != nil {
return nil, err
}
validator2, err := k.Keeper.GetValidator(ctx, valAddr)
validator, err := k.Keeper.GetValidator(ctx, valAddr)
if err != nil {
return nil, err
}
if validator2.GetOperator() == "" {
if validator.GetOperator() == "" {
return nil, types.ErrNoValidatorFound
}
if status := validator2.GetStatus(); status != sdk.Bonded {
if status := validator.GetStatus(); status != sdk.Bonded {
return nil, errorsmod.Wrapf(sdkerrors.ErrInvalidType, "validator status is not bonded, got %x", status)
}
@ -709,7 +697,7 @@ func (k msgServer) RotateConsPubKey(ctx context.Context, msg *types.MsgRotateCon
err = k.setConsPubKeyRotationHistory(
ctx,
valAddr,
validator2.ConsensusPubkey,
validator.ConsensusPubkey,
msg.NewPubkey,
params.KeyRotationFee,
)
@ -719,3 +707,42 @@ func (k msgServer) RotateConsPubKey(ctx context.Context, msg *types.MsgRotateCon
return res, nil
}
// checkConsKeyAlreadyUsed returns an error if the consensus public key is already used,
// in ConsAddrToValidatorIdentifierMap, OldToNewConsAddrMap, or in the current block (RotationHistory).
func (k msgServer) checkConsKeyAlreadyUsed(ctx context.Context, newConsPubKey cryptotypes.PubKey) error {
newConsAddr := sdk.ConsAddress(newConsPubKey.Address())
rotatedTo, err := k.ConsAddrToValidatorIdentifierMap.Get(ctx, newConsAddr)
if err != nil && !errors.Is(err, collections.ErrNotFound) {
return err
}
if rotatedTo != nil {
return sdkerrors.ErrInvalidAddress.Wrap(
"public key was already used")
}
// check in the current block
rotationHistory, err := k.GetBlockConsPubKeyRotationHistory(ctx)
if err != nil {
return err
}
for _, rotation := range rotationHistory {
cachedValue := rotation.NewConsPubkey.GetCachedValue()
if cachedValue == nil {
return sdkerrors.ErrInvalidAddress.Wrap("new public key is nil")
}
if bytes.Equal(cachedValue.(cryptotypes.PubKey).Address(), newConsAddr) {
return sdkerrors.ErrInvalidAddress.Wrap("public key was already used")
}
}
// checks if NewPubKey is not duplicated on ValidatorsByConsAddr
_, err = k.Keeper.ValidatorByConsAddr(ctx, newConsAddr)
if err == nil {
return types.ErrValidatorPubKeyExists
}
return nil
}

View File

@ -1205,6 +1205,9 @@ func (s *KeeperTestSuite) TestConsKeyRotn() {
existingPubkey, ok := validators[1].ConsensusPubkey.GetCachedValue().(cryptotypes.PubKey)
s.Require().True(ok)
validator0PubKey, ok := validators[0].ConsensusPubkey.GetCachedValue().(cryptotypes.PubKey)
s.Require().True(ok)
bondedPool := authtypes.NewEmptyModuleAccount(types.BondedPoolName)
accountKeeper.EXPECT().GetModuleAccount(gomock.Any(), types.BondedPoolName).Return(bondedPool).AnyTimes()
bankKeeper.EXPECT().GetBalance(gomock.Any(), bondedPool.GetAddress(), sdk.DefaultBondDenom).Return(sdk.NewInt64Coin(sdk.DefaultBondDenom, 1000000)).AnyTimes()
@ -1235,7 +1238,7 @@ func (s *KeeperTestSuite) TestConsKeyRotn() {
name: "pubkey already associated with another validator",
malleate: func() sdk.Context { return ctx },
isErr: true,
errMsg: "consensus pubkey is already used for a validator",
errMsg: "validator already exist for this pubkey; must use new validator pubkey",
newPubKey: existingPubkey,
validator: validators[0].GetOperator(),
},
@ -1350,6 +1353,20 @@ func (s *KeeperTestSuite) TestConsKeyRotn() {
errMsg: "exceeding maximum consensus pubkey rotations within unbonding period",
validator: validators[5].GetOperator(),
},
{
name: "try using the old pubkey of another validator that rotated",
malleate: func() sdk.Context {
val, err := stakingKeeper.ValidatorAddressCodec().StringToBytes(validators[0].GetOperator())
s.Require().NoError(err)
bankKeeper.EXPECT().SendCoinsFromAccountToModule(gomock.Any(), sdk.AccAddress(val), gomock.Any(), gomock.Any()).Return(nil).AnyTimes()
return ctx
},
isErr: true,
errMsg: "validator already exist for this pubkey; must use new validator pubkey",
newPubKey: validator0PubKey,
validator: validators[2].GetOperator(),
},
}
for _, tc := range testCases {
@ -1403,5 +1420,5 @@ func (s *KeeperTestSuite) TestConsKeyRotationInSameBlock() {
s.Require().NoError(err)
_, err = msgServer.RotateConsPubKey(ctx, req)
s.Require().ErrorContains(err, "consensus pubkey is already used for a validator")
s.Require().ErrorContains(err, "public key was already used")
}

View File

@ -38,12 +38,12 @@ func (k Keeper) GetValidator(ctx context.Context, addr sdk.ValAddress) (validato
func (k Keeper) GetValidatorByConsAddr(ctx context.Context, consAddr sdk.ConsAddress) (validator types.Validator, err error) {
opAddr, err := k.ValidatorByConsensusAddress.Get(ctx, consAddr)
if err != nil {
// if the validator not found try to find it in the map of `OldToNewConsKeyMap` because validator may've rotated it's key.
// if the validator not found try to find it in the map of `OldToNewConsAddrMap` because validator may've rotated it's key.
if !errors.Is(err, collections.ErrNotFound) {
return types.Validator{}, err
}
newConsAddr, err := k.OldToNewConsKeyMap.Get(ctx, consAddr.Bytes())
newConsAddr, err := k.OldToNewConsAddrMap.Get(ctx, consAddr.Bytes())
if err != nil {
if errors.Is(err, collections.ErrNotFound) {
return types.Validator{}, types.ErrNoValidatorFound

View File

@ -192,11 +192,23 @@ func SimulateMsgCreateValidator(
if err != nil {
return simtypes.NoOpMsg(types.ModuleName, msgType, "unable to generate validator address"), nil, err
}
msg, err := types.NewMsgCreateValidator(addr, simAccount.ConsKey.PubKey(), selfDelegation, description, commission, math.OneInt())
if err != nil {
return simtypes.NoOpMsg(types.ModuleName, sdk.MsgTypeURL(msg), "unable to create CreateValidator message"), nil, err
}
// check if there's another key rotation for this same key in the same block
allRotations, err := k.GetBlockConsPubKeyRotationHistory(ctx)
if err != nil {
return simtypes.NoOpMsg(types.ModuleName, msgType, "cannot get block cons key rotation history"), nil, err
}
for _, r := range allRotations {
if r.NewConsPubkey.Compare(msg.Pubkey) == 0 {
return simtypes.NoOpMsg(types.ModuleName, msgType, "cons key already used in this block"), nil, nil
}
}
txCtx := simulation.OperationInput{
R: r,
App: app,

View File

@ -49,7 +49,6 @@ var (
ErrNoUnbondingType = errors.Register(ModuleName, 45, "unbonding type not found")
// consensus key errors
ErrConsensusPubKeyAlreadyUsedForValidator = errors.Register(ModuleName, 46, "consensus pubkey is already used for a validator")
ErrExceedingMaxConsPubKeyRotations = errors.Register(ModuleName, 47, "exceeding maximum consensus pubkey rotations within unbonding period")
ErrConsensusPubKeyLenInvalid = errors.Register(ModuleName, 48, "consensus pubkey len is invalid")
ErrExceedingMaxConsPubKeyRotations = errors.Register(ModuleName, 46, "exceeding maximum consensus pubkey rotations within unbonding period")
ErrConsensusPubKeyLenInvalid = errors.Register(ModuleName, 47, "consensus pubkey len is invalid")
)

View File

@ -66,8 +66,8 @@ var (
BlockConsPubKeyRotationHistoryKey = collections.NewPrefix(102) // prefix for consPubkey rotation history by height
ValidatorConsensusKeyRotationRecordQueueKey = collections.NewPrefix(103) // this key is used to set the unbonding period time on each rotation
ValidatorConsensusKeyRotationRecordIndexKey = collections.NewPrefix(104) // this key is used to restrict the validator next rotation within waiting (unbonding) period
ConsKeyToValidatorIdentifierMapPrefix = collections.NewPrefix(105) // prefix for rotated cons address to new cons address
OldToNewConsKeyMap = collections.NewPrefix(106) // prefix for rotated cons address to new cons address
ConsAddrToValidatorIdentifierMapPrefix = collections.NewPrefix(105) // prefix for rotated cons address to new cons address
OldToNewConsAddrMap = collections.NewPrefix(106) // prefix for rotated cons address to new cons address
)
// Reserved kvstore keys