diff --git a/x/staking/keeper/msg_server.go b/x/staking/keeper/msg_server.go index d53cd07b55..166f5a1ef6 100644 --- a/x/staking/keeper/msg_server.go +++ b/x/staking/keeper/msg_server.go @@ -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{}, ¶msRes); 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 diff --git a/x/staking/keeper/msg_server_test.go b/x/staking/keeper/msg_server_test.go index 1f67108a6a..d7060abda3 100644 --- a/x/staking/keeper/msg_server_test.go +++ b/x/staking/keeper/msg_server_test.go @@ -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)