Skip to content

Commit

Permalink
Only allow supported pubKey types (#2949)
Browse files Browse the repository at this point in the history
* Only allow supported pubKey types
* Add type and supported types to error message
* Add default value for ConsensusParams
  • Loading branch information
hendrikhofstadt authored and jackzampolin committed Nov 29, 2018
1 parent b2b026b commit db77117
Show file tree
Hide file tree
Showing 8 changed files with 65 additions and 11 deletions.
1 change: 1 addition & 0 deletions PENDING.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ BREAKING CHANGES
* [\#2801](https://github.com/cosmos/cosmos-sdk/pull/2801) Remove AppInit structure.
* [\#2798](https://github.com/cosmos/cosmos-sdk/issues/2798) Governance API has miss-spelled English word in JSON response ('depositer' -> 'depositor')
* [\#2943](https://github.com/cosmos/cosmos-sdk/pull/2943) Transaction action tags equal the message type. Stake EndBlocker tags are included.
* [\#2945](https://github.com/cosmos/cosmos-sdk/issues/2945) CreateValidator pubKey type is now validated against `ConsensusParams`

* Tendermint
- Update to Tendermint 0.27.0
Expand Down
3 changes: 2 additions & 1 deletion baseapp/baseapp.go
Original file line number Diff line number Diff line change
Expand Up @@ -587,7 +587,8 @@ func validateBasicTxMsgs(msgs []sdk.Msg) sdk.Error {
func (app *BaseApp) getContextForTx(mode runTxMode, txBytes []byte) (ctx sdk.Context) {
ctx = app.getState(mode).ctx.
WithTxBytes(txBytes).
WithVoteInfos(app.voteInfos)
WithVoteInfos(app.voteInfos).
WithConsensusParams(app.consensusParams)
if mode == runTxModeSimulate {
ctx, _ = ctx.CacheContext()
}
Expand Down
10 changes: 10 additions & 0 deletions types/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ func NewContext(ms MultiStore, header abci.Header, isCheckTx bool, logger log.Lo
c = c.WithVoteInfos(nil)
c = c.WithGasMeter(NewInfiniteGasMeter())
c = c.WithMinimumFees(Coins{})
c = c.WithConsensusParams(nil)
return c
}

Expand Down Expand Up @@ -141,6 +142,7 @@ const (
contextKeyGasMeter
contextKeyBlockGasMeter
contextKeyMinimumFees
contextKeyConsensusParams
)

func (c Context) MultiStore() MultiStore {
Expand Down Expand Up @@ -169,6 +171,10 @@ func (c Context) IsCheckTx() bool { return c.Value(contextKeyIsCheckTx).(bool) }

func (c Context) MinimumFees() Coins { return c.Value(contextKeyMinimumFees).(Coins) }

func (c Context) ConsensusParams() *abci.ConsensusParams {
return c.Value(contextKeyConsensusParams).(*abci.ConsensusParams)
}

func (c Context) WithMultiStore(ms MultiStore) Context {
return c.withValue(contextKeyMultiStore, ms)
}
Expand Down Expand Up @@ -220,6 +226,10 @@ func (c Context) WithMinimumFees(minFees Coins) Context {
return c.withValue(contextKeyMinimumFees, minFees)
}

func (c Context) WithConsensusParams(params *abci.ConsensusParams) Context {
return c.withValue(contextKeyConsensusParams, params)
}

// Cache the multistore and return a new cached context. The cached context is
// written to the context when writeCache is called.
func (c Context) CacheContext() (cc Context, writeCache func()) {
Expand Down
12 changes: 11 additions & 1 deletion x/stake/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,14 @@ package stake
import (
"bytes"

abci "github.com/tendermint/tendermint/abci/types"
"github.com/tendermint/tendermint/libs/common"
tmtypes "github.com/tendermint/tendermint/types"

sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/x/stake/keeper"
"github.com/cosmos/cosmos-sdk/x/stake/tags"
"github.com/cosmos/cosmos-sdk/x/stake/types"
abci "github.com/tendermint/tendermint/abci/types"
)

func NewHandler(k keeper.Keeper) sdk.Handler {
Expand Down Expand Up @@ -101,6 +104,13 @@ func handleMsgCreateValidator(ctx sdk.Context, msg types.MsgCreateValidator, k k
return ErrBadDenom(k.Codespace()).Result()
}

if ctx.ConsensusParams() != nil {
tmPubKey := tmtypes.TM2PB.PubKey(msg.PubKey)
if !common.StringInSlice(tmPubKey.Type, ctx.ConsensusParams().Validator.PubKeyTypes) {
return ErrValidatorPubKeyTypeUnsupported(k.Codespace(), tmPubKey.Type, ctx.ConsensusParams().Validator.PubKeyTypes).Result()
}
}

validator := NewValidator(msg.ValidatorAddr, msg.PubKey, msg.Description)
commission := NewCommissionWithTime(
msg.Commission.Rate, msg.Commission.MaxRate,
Expand Down
23 changes: 23 additions & 0 deletions x/stake/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

abci "github.com/tendermint/tendermint/abci/types"
"github.com/tendermint/tendermint/crypto/secp256k1"
tmtypes "github.com/tendermint/tendermint/types"

sdk "github.com/cosmos/cosmos-sdk/types"
keep "github.com/cosmos/cosmos-sdk/x/stake/keeper"
"github.com/cosmos/cosmos-sdk/x/stake/types"
Expand Down Expand Up @@ -157,6 +161,25 @@ func TestDuplicatesMsgCreateValidator(t *testing.T) {
assert.Equal(t, Description{}, validator.Description)
}

func TestInvalidPubKeyTypeMsgCreateValidator(t *testing.T) {
ctx, _, keeper := keep.CreateTestInput(t, false, 1000)

addr := sdk.ValAddress(keep.Addrs[0])
invalidPk := secp256k1.GenPrivKey().PubKey()

// invalid pukKey type should not be allowed
msgCreateValidator := NewTestMsgCreateValidator(addr, invalidPk, 10)
got := handleMsgCreateValidator(ctx, msgCreateValidator, keeper)
require.False(t, got.IsOK(), "%v", got)

ctx = ctx.WithConsensusParams(&abci.ConsensusParams{
Validator: &abci.ValidatorParams{PubKeyTypes: []string{tmtypes.ABCIPubKeyTypeSecp256k1}},
})

got = handleMsgCreateValidator(ctx, msgCreateValidator, keeper)
require.True(t, got.IsOK(), "%v", got)
}

func TestDuplicatesMsgCreateValidatorOnBehalfOf(t *testing.T) {
ctx, _, keeper := keep.CreateTestInput(t, false, 1000)

Expand Down
2 changes: 2 additions & 0 deletions x/stake/keeper/test_common.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"github.com/tendermint/tendermint/crypto/ed25519"
dbm "github.com/tendermint/tendermint/libs/db"
"github.com/tendermint/tendermint/libs/log"
tmtypes "github.com/tendermint/tendermint/types"

"github.com/cosmos/cosmos-sdk/codec"
"github.com/cosmos/cosmos-sdk/store"
Expand Down Expand Up @@ -93,6 +94,7 @@ func CreateTestInput(t *testing.T, isCheckTx bool, initCoins int64) (sdk.Context
require.Nil(t, err)

ctx := sdk.NewContext(ms, abci.Header{ChainID: "foochainid"}, isCheckTx, log.NewNopLogger())
ctx = ctx.WithConsensusParams(&abci.ConsensusParams{Validator: &abci.ValidatorParams{PubKeyTypes: []string{tmtypes.ABCIPubKeyTypeEd25519}}})
cdc := MakeTestCodec()
accountKeeper := auth.NewAccountKeeper(
cdc, // amino codec
Expand Down
19 changes: 10 additions & 9 deletions x/stake/stake.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,15 +118,16 @@ const (
)

var (
ErrNilValidatorAddr = types.ErrNilValidatorAddr
ErrNoValidatorFound = types.ErrNoValidatorFound
ErrValidatorOwnerExists = types.ErrValidatorOwnerExists
ErrValidatorPubKeyExists = types.ErrValidatorPubKeyExists
ErrValidatorJailed = types.ErrValidatorJailed
ErrBadRemoveValidator = types.ErrBadRemoveValidator
ErrDescriptionLength = types.ErrDescriptionLength
ErrCommissionNegative = types.ErrCommissionNegative
ErrCommissionHuge = types.ErrCommissionHuge
ErrNilValidatorAddr = types.ErrNilValidatorAddr
ErrNoValidatorFound = types.ErrNoValidatorFound
ErrValidatorOwnerExists = types.ErrValidatorOwnerExists
ErrValidatorPubKeyExists = types.ErrValidatorPubKeyExists
ErrValidatorPubKeyTypeUnsupported = types.ErrValidatorPubKeyTypeNotSupported
ErrValidatorJailed = types.ErrValidatorJailed
ErrBadRemoveValidator = types.ErrBadRemoveValidator
ErrDescriptionLength = types.ErrDescriptionLength
ErrCommissionNegative = types.ErrCommissionNegative
ErrCommissionHuge = types.ErrCommissionHuge

ErrNilDelegatorAddr = types.ErrNilDelegatorAddr
ErrBadDenom = types.ErrBadDenom
Expand Down
6 changes: 6 additions & 0 deletions x/stake/types/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package types

import (
"fmt"
"strings"
"time"

sdk "github.com/cosmos/cosmos-sdk/types"
Expand Down Expand Up @@ -44,6 +45,11 @@ func ErrValidatorPubKeyExists(codespace sdk.CodespaceType) sdk.Error {
return sdk.NewError(codespace, CodeInvalidValidator, "validator already exist for this pubkey, must use new validator pubkey")
}

func ErrValidatorPubKeyTypeNotSupported(codespace sdk.CodespaceType, keyType string, supportedTypes []string) sdk.Error {
msg := fmt.Sprintf("validator pubkey type %s is not supported, must use %s", keyType, strings.Join(supportedTypes, ","))
return sdk.NewError(codespace, CodeInvalidValidator, msg)
}

func ErrValidatorJailed(codespace sdk.CodespaceType) sdk.Error {
return sdk.NewError(codespace, CodeInvalidValidator, "validator for this address is currently jailed")
}
Expand Down

0 comments on commit db77117

Please sign in to comment.