fix(x/staking): Verify the pubkey type when rotating (#20714)

This commit is contained in:
Facundo Medica 2024-06-21 10:41:58 +02:00 committed by GitHub
parent a6407f411e
commit 207b30262f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 123 additions and 11 deletions

View File

@ -651,6 +651,28 @@ func (k msgServer) RotateConsPubKey(ctx context.Context, msg *types.MsgRotateCon
return nil, errorsmod.Wrapf(sdkerrors.ErrInvalidType, "expecting cryptotypes.PubKey, got %T", cv)
}
// check if the new public key type is valid
paramsRes := consensusv1.QueryParamsResponse{}
if err := k.QueryRouterService.InvokeTyped(ctx, &consensusv1.QueryParamsRequest{}, &paramsRes); err != nil {
return nil, errorsmod.Wrapf(sdkerrors.ErrInvalidRequest, "failed to query consensus params: %s", err)
}
if paramsRes.Params.Validator != nil {
pkType := pk.Type()
if !slices.Contains(paramsRes.Params.Validator.PubKeyTypes, pkType) {
return nil, errorsmod.Wrapf(
types.ErrValidatorPubKeyTypeNotSupported,
"got: %s, expected: %s", pk.Type(), paramsRes.Params.Validator.PubKeyTypes,
)
}
if pkType == sdk.PubKeyEd25519Type && len(pk.Bytes()) != ed25519.PubKeySize {
return nil, errorsmod.Wrapf(
types.ErrConsensusPubKeyLenInvalid,
"got: %d, expected: %d", len(pk.Bytes()), ed25519.PubKeySize,
)
}
}
err = k.checkConsKeyAlreadyUsed(ctx, pk)
if err != nil {
return nil, err

View File

@ -4,7 +4,6 @@ import (
"testing"
"time"
cmtproto "github.com/cometbft/cometbft/api/cometbft/types/v1"
"github.com/golang/mock/gomock"
"cosmossdk.io/collections"
@ -17,6 +16,8 @@ import (
"github.com/cosmos/cosmos-sdk/codec/address"
codectypes "github.com/cosmos/cosmos-sdk/codec/types"
"github.com/cosmos/cosmos-sdk/crypto/keys/ed25519"
"github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1"
"github.com/cosmos/cosmos-sdk/crypto/keys/secp256r1"
cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types"
simtestutil "github.com/cosmos/cosmos-sdk/testutil/sims"
sdk "github.com/cosmos/cosmos-sdk/types"
@ -48,17 +49,21 @@ func (s *KeeperTestSuite) TestMsgCreateValidator() {
pubkeyInvalidLen, err := codectypes.NewAnyWithValue(ed25519pk)
require.NoError(err)
ctx = ctx.WithConsensusParams(cmtproto.ConsensusParams{
Validator: &cmtproto.ValidatorParams{
PubKeyTypes: []string{sdk.PubKeyEd25519Type},
},
})
invalidPk, _ := secp256r1.GenPrivKey()
invalidPubkey, err := codectypes.NewAnyWithValue(invalidPk.PubKey())
require.NoError(err)
badKey := secp256k1.GenPrivKey()
badPubKey, err := codectypes.NewAnyWithValue(&secp256k1.PubKey{Key: badKey.PubKey().Bytes()[:len(badKey.PubKey().Bytes())-1]})
require.NoError(err)
testCases := []struct {
name string
input *types.MsgCreateValidator
expErr bool
expErrMsg string
name string
input *types.MsgCreateValidator
expErr bool
expErrMsg string
expPanic bool
expPanicMsg string
}{
{
name: "empty description",
@ -238,6 +243,52 @@ func (s *KeeperTestSuite) TestMsgCreateValidator() {
expErr: true,
expErrMsg: "validator's self delegation must be greater than their minimum self delegation",
},
{
name: "invalid pubkey type",
input: &types.MsgCreateValidator{
Description: types.Description{
Moniker: "NewValidator",
Identity: "xyz",
Website: "xyz.com",
SecurityContact: "xyz@gmail.com",
Details: "details",
},
Commission: types.CommissionRates{
Rate: math.LegacyNewDecWithPrec(5, 1),
MaxRate: math.LegacyNewDecWithPrec(5, 1),
MaxChangeRate: math.LegacyNewDec(0),
},
MinSelfDelegation: math.NewInt(1),
DelegatorAddress: s.addressToString(Addr),
ValidatorAddress: s.valAddressToString(ValAddr),
Pubkey: invalidPubkey,
Value: sdk.NewInt64Coin(sdk.DefaultBondDenom, 10000),
},
expErr: true,
expErrMsg: "got: secp256r1, expected: [ed25519 secp256k1]: validator pubkey type is not supported",
},
{
name: "invalid pubkey length",
input: &types.MsgCreateValidator{
Description: types.Description{
Moniker: "NewValidator",
Identity: "xyz",
Website: "xyz.com",
},
Commission: types.CommissionRates{
Rate: math.LegacyNewDecWithPrec(5, 1),
MaxRate: math.LegacyNewDecWithPrec(5, 1),
MaxChangeRate: math.LegacyNewDec(0),
},
MinSelfDelegation: math.NewInt(1),
DelegatorAddress: s.addressToString(Addr),
ValidatorAddress: s.valAddressToString(ValAddr),
Pubkey: badPubKey,
Value: sdk.NewInt64Coin(sdk.DefaultBondDenom, 10000),
},
expPanic: true,
expPanicMsg: "length of pubkey is incorrect",
},
{
name: "valid msg",
input: &types.MsgCreateValidator{
@ -265,6 +316,13 @@ func (s *KeeperTestSuite) TestMsgCreateValidator() {
for _, tc := range testCases {
tc := tc
s.T().Run(tc.name, func(t *testing.T) {
if tc.expPanic {
require.PanicsWithValue(tc.expPanicMsg, func() {
_, _ = msgServer.CreateValidator(ctx, tc.input)
})
return
}
_, err := msgServer.CreateValidator(ctx, tc.input)
if tc.expErr {
require.Error(err)
@ -1212,6 +1270,12 @@ func (s *KeeperTestSuite) TestConsKeyRotn() {
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()
invalidPK, _ := secp256r1.GenPrivKey()
invalidPubkey := invalidPK.PubKey()
badKey := secp256k1.GenPrivKey()
badPubKey := &secp256k1.PubKey{Key: badKey.PubKey().Bytes()[:len(badKey.PubKey().Bytes())-1]}
testCases := []struct {
name string
malleate func() sdk.Context
@ -1219,6 +1283,8 @@ func (s *KeeperTestSuite) TestConsKeyRotn() {
newPubKey cryptotypes.PubKey
isErr bool
errMsg string
isPanic bool
panicMsg string
}{
{
name: "1st iteration no error",
@ -1234,6 +1300,22 @@ func (s *KeeperTestSuite) TestConsKeyRotn() {
newPubKey: PKs[499],
validator: validators[0].GetOperator(),
},
{
name: "invalid pubkey type",
malleate: func() sdk.Context { return ctx },
isErr: true,
errMsg: "secp256r1, expected: [ed25519 secp256k1]: validator pubkey type is not supported",
newPubKey: invalidPubkey,
validator: validators[0].GetOperator(),
},
{
name: "invalid pubkey length",
malleate: func() sdk.Context { return ctx },
isPanic: true,
panicMsg: "length of pubkey is incorrect",
newPubKey: badPubKey,
validator: validators[0].GetOperator(),
},
{
name: "pubkey already associated with another validator",
malleate: func() sdk.Context { return ctx },
@ -1376,7 +1458,15 @@ func (s *KeeperTestSuite) TestConsKeyRotn() {
req, err := types.NewMsgRotateConsPubKey(tc.validator, tc.newPubKey)
s.Require().NoError(err)
_, err = msgServer.RotateConsPubKey(newCtx, req)
if tc.isPanic {
s.Require().PanicsWithValue(tc.panicMsg, func() {
_, err = msgServer.RotateConsPubKey(ctx, req)
}, tc.isPanic)
return
} else {
_, err = msgServer.RotateConsPubKey(newCtx, req)
}
if tc.isErr {
s.Require().Error(err)
s.Require().Contains(err.Error(), tc.errMsg)