Merge PR #5359: Params Validation

types/coin.go

@@ -52,7 +52,7 @@ func (coin Coin) String() string {
 // validate returns an error if the Coin has a negative amount or if
 // the denom is invalid.
 func validate(denom string, amount Int) error {
-	if err := validateDenom(denom); err != nil {
+	if err := ValidateDenom(denom); err != nil {
 		return err
 	}
 
@@ -203,7 +203,7 @@ func (coins Coins) IsValid() bool {
 	case 0:
 		return true
 	case 1:
-		if err := validateDenom(coins[0].Denom); err != nil {
+		if err := ValidateDenom(coins[0].Denom); err != nil {
 			return false
 		}
 		return coins[0].IsPositive()
@@ -599,7 +599,9 @@ var (
 	reDecCoin   = regexp.MustCompile(fmt.Sprintf(`^(%s)%s(%s)$`, reDecAmt, reSpc, reDnmString))
 )
 
-func validateDenom(denom string) error {
+// ValidateDenom validates a denomination string returning an error if it is
+// invalid.
+func ValidateDenom(denom string) error {
 	if !reDnm.MatchString(denom) {
 		return fmt.Errorf("invalid denom: %s", denom)
 	}
@@ -607,7 +609,7 @@ func validateDenom(denom string) error {
 }
 
 func mustValidateDenom(denom string) {
-	if err := validateDenom(denom); err != nil {
+	if err := ValidateDenom(denom); err != nil {
 		panic(err)
 	}
 }
@@ -629,7 +631,7 @@ func ParseCoin(coinStr string) (coin Coin, err error) {
 		return Coin{}, fmt.Errorf("failed to parse coin amount: %s", amountStr)
 	}
 
-	if err := validateDenom(denomStr); err != nil {
+	if err := ValidateDenom(denomStr); err != nil {
 		return Coin{}, fmt.Errorf("invalid denom cannot contain upper case characters or spaces: %s", err)
 	}
 

types/dec_coin.go

@@ -470,7 +470,7 @@ func (coins DecCoins) IsValid() bool {
 		return true
 
 	case 1:
-		if err := validateDenom(coins[0].Denom); err != nil {
+		if err := ValidateDenom(coins[0].Denom); err != nil {
 			return false
 		}
 		return coins[0].IsPositive()
@@ -570,7 +570,7 @@ func ParseDecCoin(coinStr string) (coin DecCoin, err error) {
 		return DecCoin{}, errors.Wrap(err, fmt.Sprintf("failed to parse decimal coin amount: %s", amountStr))
 	}
 
-	if err := validateDenom(denomStr); err != nil {
+	if err := ValidateDenom(denomStr); err != nil {
 		return DecCoin{}, fmt.Errorf("invalid denom cannot contain upper case characters or spaces: %s", err)
 	}
 

types/denom.go

@@ -11,7 +11,7 @@ var denomUnits = map[string]Dec{}
 // RegisterDenom registers a denomination with a corresponding unit. If the
 // denomination is already registered, an error will be returned.
 func RegisterDenom(denom string, unit Dec) error {
-	if err := validateDenom(denom); err != nil {
+	if err := ValidateDenom(denom); err != nil {
 		return err
 	}
 
@@ -26,7 +26,7 @@ func RegisterDenom(denom string, unit Dec) error {
 // GetDenomUnit returns a unit for a given denomination if it exists. A boolean
 // is returned if the denomination is registered.
 func GetDenomUnit(denom string) (Dec, bool) {
-	if err := validateDenom(denom); err != nil {
+	if err := ValidateDenom(denom); err != nil {
 		return ZeroDec(), false
 	}
 
@@ -42,7 +42,7 @@ func GetDenomUnit(denom string) (Dec, bool) {
 // denomination is invalid or if neither denomination is registered, an error
 // is returned.
 func ConvertCoin(coin Coin, denom string) (Coin, error) {
-	if err := validateDenom(denom); err != nil {
+	if err := ValidateDenom(denom); err != nil {
 		return Coin{}, err
 	}
 

x/auth/ante/ante_test.go

@@ -781,8 +781,7 @@ func TestAnteHandlerReCheck(t *testing.T) {
 		name   string
 		params types.Params
 	}{
-		{"memo size check", types.NewParams(0, types.DefaultTxSigLimit, types.DefaultTxSizeCostPerByte, types.DefaultSigVerifyCostED25519, types.DefaultSigVerifyCostSecp256k1)},
+		{"memo size check", types.NewParams(1, types.DefaultTxSigLimit, types.DefaultTxSizeCostPerByte, types.DefaultSigVerifyCostED25519, types.DefaultSigVerifyCostSecp256k1)},
-		{"tx sig limit check", types.NewParams(types.DefaultMaxMemoCharacters, 0, types.DefaultTxSizeCostPerByte, types.DefaultSigVerifyCostED25519, types.DefaultSigVerifyCostSecp256k1)},
 		{"txsize check", types.NewParams(types.DefaultMaxMemoCharacters, types.DefaultTxSigLimit, 10000000, types.DefaultSigVerifyCostED25519, types.DefaultSigVerifyCostSecp256k1)},
 		{"sig verify cost check", types.NewParams(types.DefaultMaxMemoCharacters, types.DefaultTxSigLimit, types.DefaultTxSizeCostPerByte, types.DefaultSigVerifyCostED25519, 100000000)},
 	}

x/auth/simulation/genesis.go

@@ -81,7 +81,7 @@ func RandomizedGenState(simState *module.SimulationState) {
 	var sigVerifyCostSECP256K1 uint64
 	simState.AppParams.GetOrGenerate(
 		simState.Cdc, SigVerifyCostSECP256K1, &sigVerifyCostSECP256K1, simState.Rand,
-		func(r *rand.Rand) { sigVerifyCostED25519 = GenSigVerifyCostSECP256K1(r) },
+		func(r *rand.Rand) { sigVerifyCostSECP256K1 = GenSigVerifyCostSECP256K1(r) },
 	)
 
 	params := types.NewParams(maxMemoChars, txSigLimit, txSizeCostPerByte,

x/auth/simulation/params.go

@@ -20,17 +20,17 @@ const (
 // on the simulation
 func ParamChanges(r *rand.Rand) []simulation.ParamChange {
 	return []simulation.ParamChange{
-		simulation.NewSimParamChange(types.ModuleName, keyMaxMemoCharacters, "",
+		simulation.NewSimParamChange(types.ModuleName, keyMaxMemoCharacters,
 			func(r *rand.Rand) string {
 				return fmt.Sprintf("\"%d\"", GenMaxMemoChars(r))
 			},
 		),
-		simulation.NewSimParamChange(types.ModuleName, keyTxSigLimit, "",
+		simulation.NewSimParamChange(types.ModuleName, keyTxSigLimit,
 			func(r *rand.Rand) string {
 				return fmt.Sprintf("\"%d\"", GenTxSigLimit(r))
 			},
 		),
-		simulation.NewSimParamChange(types.ModuleName, keyTxSizeCostPerByte, "",
+		simulation.NewSimParamChange(types.ModuleName, keyTxSizeCostPerByte,
 			func(r *rand.Rand) string {
 				return fmt.Sprintf("\"%d\"", GenTxSizeCostPerByte(r))
 			},

x/auth/types/params.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"strings"
 
+	"github.com/cosmos/cosmos-sdk/x/params"
 	"github.com/cosmos/cosmos-sdk/x/params/subspace"
 )
 
@@ -63,11 +64,11 @@ func ParamKeyTable() subspace.KeyTable {
 // nolint
 func (p *Params) ParamSetPairs() subspace.ParamSetPairs {
 	return subspace.ParamSetPairs{
-		{KeyMaxMemoCharacters, &p.MaxMemoCharacters},
+		params.NewParamSetPair(KeyMaxMemoCharacters, &p.MaxMemoCharacters, validateMaxMemoCharacters),
-		{KeyTxSigLimit, &p.TxSigLimit},
+		params.NewParamSetPair(KeyTxSigLimit, &p.TxSigLimit, validateTxSigLimit),
-		{KeyTxSizeCostPerByte, &p.TxSizeCostPerByte},
+		params.NewParamSetPair(KeyTxSizeCostPerByte, &p.TxSizeCostPerByte, validateTxSizeCostPerByte),
-		{KeySigVerifyCostED25519, &p.SigVerifyCostED25519},
+		params.NewParamSetPair(KeySigVerifyCostED25519, &p.SigVerifyCostED25519, validateSigVerifyCostED25519),
-		{KeySigVerifyCostSecp256k1, &p.SigVerifyCostSecp256k1},
+		params.NewParamSetPair(KeySigVerifyCostSecp256k1, &p.SigVerifyCostSecp256k1, validateSigVerifyCostSecp256k1),
 	}
 }
 
@@ -101,22 +102,88 @@ func (p Params) String() string {
 	return sb.String()
 }
 
-// Validate checks that the parameters have valid values.
+func validateTxSigLimit(i interface{}) error {
-func (p Params) Validate() error {
+	v, ok := i.(uint64)
-	if p.TxSigLimit == 0 {
+	if !ok {
-		return fmt.Errorf("invalid tx signature limit: %d", p.TxSigLimit)
+		return fmt.Errorf("invalid parameter type: %T", i)
 	}
-	if p.SigVerifyCostED25519 == 0 {
+
-		return fmt.Errorf("invalid ED25519 signature verification cost: %d", p.SigVerifyCostED25519)
+	if v == 0 {
-	}
+		return fmt.Errorf("invalid tx signature limit: %d", v)
-	if p.SigVerifyCostSecp256k1 == 0 {
-		return fmt.Errorf("invalid SECK256k1 signature verification cost: %d", p.SigVerifyCostSecp256k1)
-	}
-	if p.MaxMemoCharacters == 0 {
-		return fmt.Errorf("invalid max memo characters: %d", p.MaxMemoCharacters)
-	}
-	if p.TxSizeCostPerByte == 0 {
-		return fmt.Errorf("invalid tx size cost per byte: %d", p.TxSizeCostPerByte)
 	}
+
+	return nil
+}
+
+func validateSigVerifyCostED25519(i interface{}) error {
+	v, ok := i.(uint64)
+	if !ok {
+		return fmt.Errorf("invalid parameter type: %T", i)
+	}
+
+	if v == 0 {
+		return fmt.Errorf("invalid ED25519 signature verification cost: %d", v)
+	}
+
+	return nil
+}
+
+func validateSigVerifyCostSecp256k1(i interface{}) error {
+	v, ok := i.(uint64)
+	if !ok {
+		return fmt.Errorf("invalid parameter type: %T", i)
+	}
+
+	if v == 0 {
+		return fmt.Errorf("invalid SECK256k1 signature verification cost: %d", v)
+	}
+
+	return nil
+}
+
+func validateMaxMemoCharacters(i interface{}) error {
+	v, ok := i.(uint64)
+	if !ok {
+		return fmt.Errorf("invalid parameter type: %T", i)
+	}
+
+	if v == 0 {
+		return fmt.Errorf("invalid max memo characters: %d", v)
+	}
+
+	return nil
+}
+
+func validateTxSizeCostPerByte(i interface{}) error {
+	v, ok := i.(uint64)
+	if !ok {
+		return fmt.Errorf("invalid parameter type: %T", i)
+	}
+
+	if v == 0 {
+		return fmt.Errorf("invalid tx size cost per byte: %d", v)
+	}
+
+	return nil
+}
+
+// Validate checks that the parameters have valid values.
+func (p Params) Validate() error {
+	if err := validateTxSigLimit(p.TxSigLimit); err != nil {
+		return err
+	}
+	if err := validateSigVerifyCostED25519(p.SigVerifyCostED25519); err != nil {
+		return err
+	}
+	if err := validateSigVerifyCostSecp256k1(p.SigVerifyCostSecp256k1); err != nil {
+		return err
+	}
+	if err := validateSigVerifyCostSecp256k1(p.MaxMemoCharacters); err != nil {
+		return err
+	}
+	if err := validateTxSizeCostPerByte(p.TxSizeCostPerByte); err != nil {
+		return err
+	}
+
 	return nil
 }

x/bank/internal/types/params.go

@@ -1,6 +1,8 @@
 package types
 
 import (
+	"fmt"
+
 	"github.com/cosmos/cosmos-sdk/x/params"
 )
 
@@ -17,6 +19,15 @@ var ParamStoreKeySendEnabled = []byte("sendenabled")
 // ParamKeyTable type declaration for parameters
 func ParamKeyTable() params.KeyTable {
 	return params.NewKeyTable(
-		ParamStoreKeySendEnabled, false,
+		params.NewParamSetPair(ParamStoreKeySendEnabled, false, validateSendEnabled),
 	)
 }
+
+func validateSendEnabled(i interface{}) error {
+	_, ok := i.(bool)
+	if !ok {
+		return fmt.Errorf("invalid parameter type: %T", i)
+	}
+
+	return nil
+}

x/bank/simulation/params.go

@@ -16,7 +16,7 @@ const keySendEnabled = "sendenabled"
 // on the simulation
 func ParamChanges(r *rand.Rand) []simulation.ParamChange {
 	return []simulation.ParamChange{
-		simulation.NewSimParamChange(types.ModuleName, keySendEnabled, "",
+		simulation.NewSimParamChange(types.ModuleName, keySendEnabled,
 			func(r *rand.Rand) string {
 				return fmt.Sprintf("%v", GenSendEnabled(r))
 			},

x/crisis/internal/types/params.go

@@ -1,6 +1,8 @@
 package types
 
 import (
+	"fmt"
+
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/cosmos/cosmos-sdk/x/params"
 )
@@ -18,6 +20,19 @@ var (
 // type declaration for parameters
 func ParamKeyTable() params.KeyTable {
 	return params.NewKeyTable(
-		ParamStoreKeyConstantFee, sdk.Coin{},
+		params.NewParamSetPair(ParamStoreKeyConstantFee, sdk.Coin{}, validateConstantFee),
 	)
 }
+
+func validateConstantFee(i interface{}) error {
+	v, ok := i.(sdk.Coin)
+	if !ok {
+		return fmt.Errorf("invalid parameter type: %T", i)
+	}
+
+	if !v.IsValid() {
+		return fmt.Errorf("invalid constant fee: %s", v)
+	}
+
+	return nil
+}

x/distribution/keeper/params.go

@@ -1,6 +1,8 @@
 package keeper
 
 import (
+	"fmt"
+
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/cosmos/cosmos-sdk/x/params"
 )
@@ -8,13 +10,70 @@ import (
 // type declaration for parameters
 func ParamKeyTable() params.KeyTable {
 	return params.NewKeyTable(
-		ParamStoreKeyCommunityTax, sdk.Dec{},
+		params.NewParamSetPair(ParamStoreKeyCommunityTax, sdk.Dec{}, validateCommunityTax),
-		ParamStoreKeyBaseProposerReward, sdk.Dec{},
+		params.NewParamSetPair(ParamStoreKeyBaseProposerReward, sdk.Dec{}, validateBaseProposerReward),
-		ParamStoreKeyBonusProposerReward, sdk.Dec{},
+		params.NewParamSetPair(ParamStoreKeyBonusProposerReward, sdk.Dec{}, validateBonusProposerReward),
-		ParamStoreKeyWithdrawAddrEnabled, false,
+		params.NewParamSetPair(ParamStoreKeyWithdrawAddrEnabled, false, validateWithdrawAddrEnabled),
 	)
 }
 
+func validateCommunityTax(i interface{}) error {
+	v, ok := i.(sdk.Dec)
+	if !ok {
+		return fmt.Errorf("invalid parameter type: %T", i)
+	}
+
+	if v.IsNegative() {
+		return fmt.Errorf("community tax must be positive: %s", v)
+	}
+	if v.GT(sdk.OneDec()) {
+		return fmt.Errorf("community tax too large: %s", v)
+	}
+
+	return nil
+}
+
+func validateBaseProposerReward(i interface{}) error {
+	v, ok := i.(sdk.Dec)
+	if !ok {
+		return fmt.Errorf("invalid parameter type: %T", i)
+	}
+
+	if v.IsNegative() {
+		return fmt.Errorf("base proposer reward must be positive: %s", v)
+	}
+	if v.GT(sdk.OneDec()) {
+		return fmt.Errorf("base proposer reward too large: %s", v)
+	}
+
+	return nil
+}
+
+func validateBonusProposerReward(i interface{}) error {
+	v, ok := i.(sdk.Dec)
+	if !ok {
+		return fmt.Errorf("invalid parameter type: %T", i)
+	}
+
+	if v.IsNegative() {
+		return fmt.Errorf("bonus proposer reward must be positive: %s", v)
+	}
+	if v.GT(sdk.OneDec()) {
+		return fmt.Errorf("bonus proposer reward too large: %s", v)
+	}
+
+	return nil
+}
+
+func validateWithdrawAddrEnabled(i interface{}) error {
+	_, ok := i.(bool)
+	if !ok {
+		return fmt.Errorf("invalid parameter type: %T", i)
+	}
+
+	return nil
+}
+
 // returns the current CommunityTax rate from the global param store
 // nolint: errcheck
 func (k Keeper) GetCommunityTax(ctx sdk.Context) sdk.Dec {

x/distribution/simulation/params.go

@@ -20,17 +20,17 @@ const (
 // on the simulation
 func ParamChanges(r *rand.Rand) []simulation.ParamChange {
 	return []simulation.ParamChange{
-		simulation.NewSimParamChange(types.ModuleName, keyCommunityTax, "",
+		simulation.NewSimParamChange(types.ModuleName, keyCommunityTax,
 			func(r *rand.Rand) string {
 				return fmt.Sprintf("\"%s\"", GenCommunityTax(r))
 			},
 		),
-		simulation.NewSimParamChange(types.ModuleName, keyBaseProposerReward, "",
+		simulation.NewSimParamChange(types.ModuleName, keyBaseProposerReward,
 			func(r *rand.Rand) string {
 				return fmt.Sprintf("\"%s\"", GenBaseProposerReward(r))
 			},
 		),
-		simulation.NewSimParamChange(types.ModuleName, keyBonusProposerReward, "",
+		simulation.NewSimParamChange(types.ModuleName, keyBonusProposerReward,
 			func(r *rand.Rand) string {
 				return fmt.Sprintf("\"%s\"", GenBonusProposerReward(r))
 			},

x/evidence/internal/types/params.go

@@ -1,6 +1,7 @@
 package types
 
 import (
+	"fmt"
 	"time"
 
 	"github.com/cosmos/cosmos-sdk/x/params"
@@ -48,7 +49,7 @@ func (p Params) String() string {
 // ParamSetPairs returns the parameter set pairs.
 func (p *Params) ParamSetPairs() params.ParamSetPairs {
 	return params.ParamSetPairs{
-		params.NewParamSetPair(KeyMaxEvidenceAge, &p.MaxEvidenceAge),
+		params.NewParamSetPair(KeyMaxEvidenceAge, &p.MaxEvidenceAge, validateMaxEvidenceAge),
 	}
 }
 
@@ -58,3 +59,16 @@ func DefaultParams() Params {
 		MaxEvidenceAge: DefaultMaxEvidenceAge,
 	}
 }
+
+func validateMaxEvidenceAge(i interface{}) error {
+	v, ok := i.(time.Duration)
+	if !ok {
+		return fmt.Errorf("invalid parameter type: %T", i)
+	}
+
+	if v <= 0 {
+		return fmt.Errorf("max evidence age must be positive: %s", v)
+	}
+
+	return nil
+}

x/gov/simulation/params.go

@@ -25,17 +25,17 @@ const (
 // on the simulation
 func ParamChanges(r *rand.Rand) []simulation.ParamChange {
 	return []simulation.ParamChange{
-		simulation.NewSimParamChange(types.ModuleName, keyVotingParams, "",
+		simulation.NewSimParamChange(types.ModuleName, keyVotingParams,
 			func(r *rand.Rand) string {
 				return fmt.Sprintf(`{"voting_period": "%d"}`, GenVotingParamsVotingPeriod(r))
 			},
 		),
-		simulation.NewSimParamChange(types.ModuleName, keyDepositParams, "",
+		simulation.NewSimParamChange(types.ModuleName, keyDepositParams,
 			func(r *rand.Rand) string {
 				return fmt.Sprintf(`{"max_deposit_period": "%d"}`, GenDepositParamsDepositPeriod(r))
 			},
 		),
-		simulation.NewSimParamChange(types.ModuleName, keyTallyParams, "",
+		simulation.NewSimParamChange(types.ModuleName, keyTallyParams,
 			func(r *rand.Rand) string {
 				changes := []struct {
 					key   string

x/gov/types/params.go

@@ -31,9 +31,9 @@ var (
 // ParamKeyTable - Key declaration for parameters
 func ParamKeyTable() params.KeyTable {
 	return params.NewKeyTable(
-		ParamStoreKeyDepositParams, DepositParams{},
+		params.NewParamSetPair(ParamStoreKeyDepositParams, DepositParams{}, validateDepositParams),
-		ParamStoreKeyVotingParams, VotingParams{},
+		params.NewParamSetPair(ParamStoreKeyVotingParams, VotingParams{}, validateVotingParams),
-		ParamStoreKeyTallyParams, TallyParams{},
+		params.NewParamSetPair(ParamStoreKeyTallyParams, TallyParams{}, validateTallyParams),
 	)
 }
 
@@ -71,6 +71,22 @@ func (dp DepositParams) Equal(dp2 DepositParams) bool {
 	return dp.MinDeposit.IsEqual(dp2.MinDeposit) && dp.MaxDepositPeriod == dp2.MaxDepositPeriod
 }
 
+func validateDepositParams(i interface{}) error {
+	v, ok := i.(DepositParams)
+	if !ok {
+		return fmt.Errorf("invalid parameter type: %T", i)
+	}
+
+	if !v.MinDeposit.IsValid() {
+		return fmt.Errorf("invalid minimum deposit: %s", v.MinDeposit)
+	}
+	if v.MaxDepositPeriod <= 0 {
+		return fmt.Errorf("maximum deposit period must be positive: %d", v.MaxDepositPeriod)
+	}
+
+	return nil
+}
+
 // TallyParams defines the params around Tallying votes in governance
 type TallyParams struct {
 	Quorum    sdk.Dec `json:"quorum,omitempty" yaml:"quorum,omitempty"`       //  Minimum percentage of total stake needed to vote for a result to be considered valid
@@ -101,6 +117,34 @@ func (tp TallyParams) String() string {
 		tp.Quorum, tp.Threshold, tp.Veto)
 }
 
+func validateTallyParams(i interface{}) error {
+	v, ok := i.(TallyParams)
+	if !ok {
+		return fmt.Errorf("invalid parameter type: %T", i)
+	}
+
+	if v.Quorum.IsNegative() {
+		return fmt.Errorf("quorom cannot be negative: %s", v.Quorum)
+	}
+	if v.Quorum.GT(sdk.OneDec()) {
+		return fmt.Errorf("quorom too large: %s", v)
+	}
+	if !v.Threshold.IsPositive() {
+		return fmt.Errorf("vote threshold must be positive: %s", v.Threshold)
+	}
+	if v.Threshold.GT(sdk.OneDec()) {
+		return fmt.Errorf("vote threshold too large: %s", v)
+	}
+	if !v.Veto.IsPositive() {
+		return fmt.Errorf("veto threshold must be positive: %s", v.Threshold)
+	}
+	if v.Veto.GT(sdk.OneDec()) {
+		return fmt.Errorf("veto threshold too large: %s", v)
+	}
+
+	return nil
+}
+
 // VotingParams defines the params around Voting in governance
 type VotingParams struct {
 	VotingPeriod time.Duration `json:"voting_period,omitempty" yaml:"voting_period,omitempty"` //  Length of the voting period.
@@ -124,6 +168,19 @@ func (vp VotingParams) String() string {
   Voting Period:      %s`, vp.VotingPeriod)
 }
 
+func validateVotingParams(i interface{}) error {
+	v, ok := i.(VotingParams)
+	if !ok {
+		return fmt.Errorf("invalid parameter type: %T", i)
+	}
+
+	if v.VotingPeriod <= 0 {
+		return fmt.Errorf("voting period must be positive: %s", v.VotingPeriod)
+	}
+
+	return nil
+}
+
 // Params returns all of the governance params
 type Params struct {
 	VotingParams  VotingParams  `json:"voting_params" yaml:"voting_params"`

x/mint/alias.go

@@ -34,7 +34,6 @@ var (
 	ParamKeyTable        = types.ParamKeyTable
 	NewParams            = types.NewParams
 	DefaultParams        = types.DefaultParams
-	ValidateParams       = types.ValidateParams
 
 	// variable aliases
 	ModuleCdc              = types.ModuleCdc

x/mint/internal/types/genesis.go

@@ -25,8 +25,7 @@ func DefaultGenesisState() GenesisState {
 // ValidateGenesis validates the provided genesis state to ensure the
 // expected invariants holds.
 func ValidateGenesis(data GenesisState) error {
-	err := ValidateParams(data.Params)
+	if err := data.Params.Validate(); err != nil {
-	if err != nil {
 		return err
 	}
 

x/mint/internal/types/params.go

@@ -1,7 +1,9 @@
 package types
 
 import (
+	"errors"
 	"fmt"
+	"strings"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/cosmos/cosmos-sdk/x/params"
@@ -32,8 +34,9 @@ func ParamKeyTable() params.KeyTable {
 	return params.NewKeyTable().RegisterParamSet(&Params{})
 }
 
-func NewParams(mintDenom string, inflationRateChange, inflationMax,
+func NewParams(
-	inflationMin, goalBonded sdk.Dec, blocksPerYear uint64) Params {
+	mintDenom string, inflationRateChange, inflationMax, inflationMin, goalBonded sdk.Dec, blocksPerYear uint64,
+) Params {
 
 	return Params{
 		MintDenom:           mintDenom,
@@ -58,20 +61,34 @@ func DefaultParams() Params {
 }
 
 // validate params
-func ValidateParams(params Params) error {
+func (p Params) Validate() error {
-	if params.GoalBonded.IsNegative() {
+	if err := validateMintDenom(p.MintDenom); err != nil {
-		return fmt.Errorf("mint parameter GoalBonded should be positive, is %s ", params.GoalBonded.String())
+		return err
 	}
-	if params.GoalBonded.GT(sdk.OneDec()) {
+	if err := validateInflationRateChange(p.InflationRateChange); err != nil {
-		return fmt.Errorf("mint parameter GoalBonded must be <= 1, is %s", params.GoalBonded.String())
+		return err
 	}
-	if params.InflationMax.LT(params.InflationMin) {
+	if err := validateInflationMax(p.InflationMax); err != nil {
-		return fmt.Errorf("mint parameter Max inflation must be greater than or equal to min inflation")
+		return err
 	}
-	if params.MintDenom == "" {
+	if err := validateInflationMin(p.InflationMin); err != nil {
-		return fmt.Errorf("mint parameter MintDenom can't be an empty string")
+		return err
 	}
+	if err := validateGoalBonded(p.GoalBonded); err != nil {
+		return err
+	}
+	if err := validateBlocksPerYear(p.BlocksPerYear); err != nil {
+		return err
+	}
+	if p.InflationMax.LT(p.InflationMin) {
+		return fmt.Errorf(
+			"max inflation (%s) must be greater than or equal to min inflation (%s)",
+			p.InflationMax, p.InflationMin,
+		)
+	}
+
 	return nil
+
 }
 
 func (p Params) String() string {
@@ -91,11 +108,104 @@ func (p Params) String() string {
 // Implements params.ParamSet
 func (p *Params) ParamSetPairs() params.ParamSetPairs {
 	return params.ParamSetPairs{
-		{Key: KeyMintDenom, Value: &p.MintDenom},
+		params.NewParamSetPair(KeyMintDenom, &p.MintDenom, validateMintDenom),
-		{Key: KeyInflationRateChange, Value: &p.InflationRateChange},
+		params.NewParamSetPair(KeyInflationRateChange, &p.InflationRateChange, validateInflationRateChange),
-		{Key: KeyInflationMax, Value: &p.InflationMax},
+		params.NewParamSetPair(KeyInflationMax, &p.InflationMax, validateInflationMax),
-		{Key: KeyInflationMin, Value: &p.InflationMin},
+		params.NewParamSetPair(KeyInflationMin, &p.InflationMin, validateInflationMin),
-		{Key: KeyGoalBonded, Value: &p.GoalBonded},
+		params.NewParamSetPair(KeyGoalBonded, &p.GoalBonded, validateGoalBonded),
-		{Key: KeyBlocksPerYear, Value: &p.BlocksPerYear},
+		params.NewParamSetPair(KeyBlocksPerYear, &p.BlocksPerYear, validateBlocksPerYear),
 	}
 }
+
+func validateMintDenom(i interface{}) error {
+	v, ok := i.(string)
+	if !ok {
+		return fmt.Errorf("invalid parameter type: %T", i)
+	}
+
+	if strings.TrimSpace(v) == "" {
+		return errors.New("mint denom cannot be blank")
+	}
+	if err := sdk.ValidateDenom(v); err != nil {
+		return err
+	}
+
+	return nil
+}
+
+func validateInflationRateChange(i interface{}) error {
+	v, ok := i.(sdk.Dec)
+	if !ok {
+		return fmt.Errorf("invalid parameter type: %T", i)
+	}
+
+	if v.IsNegative() {
+		return fmt.Errorf("inflation rate change cannot be negative: %s", v)
+	}
+	if v.GT(sdk.OneDec()) {
+		return fmt.Errorf("inflation rate change too large: %s", v)
+	}
+
+	return nil
+}
+
+func validateInflationMax(i interface{}) error {
+	v, ok := i.(sdk.Dec)
+	if !ok {
+		return fmt.Errorf("invalid parameter type: %T", i)
+	}
+
+	if v.IsNegative() {
+		return fmt.Errorf("max inflation cannot be negative: %s", v)
+	}
+	if v.GT(sdk.OneDec()) {
+		return fmt.Errorf("max inflation too large: %s", v)
+	}
+
+	return nil
+}
+
+func validateInflationMin(i interface{}) error {
+	v, ok := i.(sdk.Dec)
+	if !ok {
+		return fmt.Errorf("invalid parameter type: %T", i)
+	}
+
+	if v.IsNegative() {
+		return fmt.Errorf("min inflation cannot be negative: %s", v)
+	}
+	if v.GT(sdk.OneDec()) {
+		return fmt.Errorf("min inflation too large: %s", v)
+	}
+
+	return nil
+}
+
+func validateGoalBonded(i interface{}) error {
+	v, ok := i.(sdk.Dec)
+	if !ok {
+		return fmt.Errorf("invalid parameter type: %T", i)
+	}
+
+	if v.IsNegative() {
+		return fmt.Errorf("goal bonded cannot be negative: %s", v)
+	}
+	if v.GT(sdk.OneDec()) {
+		return fmt.Errorf("goal bonded too large: %s", v)
+	}
+
+	return nil
+}
+
+func validateBlocksPerYear(i interface{}) error {
+	v, ok := i.(uint64)
+	if !ok {
+		return fmt.Errorf("invalid parameter type: %T", i)
+	}
+
+	if v == 0 {
+		return fmt.Errorf("blocks per year must be positive: %d", v)
+	}
+
+	return nil
+}

x/mint/simulation/params.go

@@ -21,22 +21,22 @@ const (
 // on the simulation
 func ParamChanges(r *rand.Rand) []simulation.ParamChange {
 	return []simulation.ParamChange{
-		simulation.NewSimParamChange(types.ModuleName, keyInflationRateChange, "",
+		simulation.NewSimParamChange(types.ModuleName, keyInflationRateChange,
 			func(r *rand.Rand) string {
 				return fmt.Sprintf("\"%s\"", GenInflationRateChange(r))
 			},
 		),
-		simulation.NewSimParamChange(types.ModuleName, keyInflationMax, "",
+		simulation.NewSimParamChange(types.ModuleName, keyInflationMax,
 			func(r *rand.Rand) string {
 				return fmt.Sprintf("\"%s\"", GenInflationMax(r))
 			},
 		),
-		simulation.NewSimParamChange(types.ModuleName, keyInflationMin, "",
+		simulation.NewSimParamChange(types.ModuleName, keyInflationMin,
 			func(r *rand.Rand) string {
 				return fmt.Sprintf("\"%s\"", GenInflationMin(r))
 			},
 		),
-		simulation.NewSimParamChange(types.ModuleName, keyGoalBonded, "",
+		simulation.NewSimParamChange(types.ModuleName, keyGoalBonded,
 			func(r *rand.Rand) string {
 				return fmt.Sprintf("\"%s\"", GenGoalBonded(r))
 			},

x/params/alias.go

@@ -1,9 +1,10 @@
+package params
+
 // nolint
 // autogenerated code using github.com/rigelrozanski/multitool
 // aliases generated for the following subdirectories:
 // ALIASGEN: github.com/cosmos/cosmos-sdk/x/params/subspace
 // ALIASGEN: github.com/cosmos/cosmos-sdk/x/params/types
-package params
 
 import (
 	"github.com/cosmos/cosmos-sdk/x/params/subspace"
@@ -13,7 +14,6 @@ import (
 const (
 	StoreKey             = subspace.StoreKey
 	TStoreKey            = subspace.TStoreKey
-	TestParamStore       = subspace.TestParamStore
 	DefaultCodespace     = types.DefaultCodespace
 	CodeUnknownSubspace  = types.CodeUnknownSubspace
 	CodeSettingParameter = types.CodeSettingParameter
@@ -28,7 +28,6 @@ var (
 	NewParamSetPair            = subspace.NewParamSetPair
 	NewSubspace                = subspace.NewSubspace
 	NewKeyTable                = subspace.NewKeyTable
-	DefaultTestComponents      = subspace.DefaultTestComponents
 	RegisterCodec              = types.RegisterCodec
 	ErrUnknownSubspace         = types.ErrUnknownSubspace
 	ErrSettingParameter        = types.ErrSettingParameter
@@ -38,7 +37,6 @@ var (
 	ErrEmptyValue              = types.ErrEmptyValue
 	NewParameterChangeProposal = types.NewParameterChangeProposal
 	NewParamChange             = types.NewParamChange
-	NewParamChangeWithSubkey   = types.NewParamChangeWithSubkey
 	ValidateChanges            = types.ValidateChanges
 
 	// variable aliases

x/params/client/utils/utils.go

@@ -20,7 +20,6 @@ type (
 	ParamChangeJSON struct {
 		Subspace string          `json:"subspace" yaml:"subspace"`
 		Key      string          `json:"key" yaml:"key"`
-		Subkey   string          `json:"subkey,omitempty" yaml:"subkey,omitempty"`
 		Value    json.RawMessage `json:"value" yaml:"value"`
 	}
 
@@ -45,13 +44,13 @@ type (
 	}
 )
 
-func NewParamChangeJSON(subspace, key, subkey string, value json.RawMessage) ParamChangeJSON {
+func NewParamChangeJSON(subspace, key string, value json.RawMessage) ParamChangeJSON {
-	return ParamChangeJSON{subspace, key, subkey, value}
+	return ParamChangeJSON{subspace, key, value}
 }
 
 // ToParamChange converts a ParamChangeJSON object to ParamChange.
 func (pcj ParamChangeJSON) ToParamChange() params.ParamChange {
-	return params.NewParamChangeWithSubkey(pcj.Subspace, pcj.Key, pcj.Subkey, string(pcj.Value))
+	return params.NewParamChange(pcj.Subspace, pcj.Key, string(pcj.Value))
 }
 
 // ToParamChanges converts a slice of ParamChangeJSON objects to a slice of

x/params/client/utils/utils_test.go

@@ -8,16 +8,15 @@ import (
 )
 
 func TestNewParamChangeJSON(t *testing.T) {
-	pcj := NewParamChangeJSON("subspace", "key", "subkey", json.RawMessage(`{}`))
+	pcj := NewParamChangeJSON("subspace", "key", json.RawMessage(`{}`))
 	require.Equal(t, "subspace", pcj.Subspace)
 	require.Equal(t, "key", pcj.Key)
-	require.Equal(t, "subkey", pcj.Subkey)
 	require.Equal(t, json.RawMessage(`{}`), pcj.Value)
 }
 
 func TestToParamChanges(t *testing.T) {
-	pcj1 := NewParamChangeJSON("subspace", "key1", "", json.RawMessage(`{}`))
+	pcj1 := NewParamChangeJSON("subspace", "key1", json.RawMessage(`{}`))
-	pcj2 := NewParamChangeJSON("subspace", "key2", "", json.RawMessage(`{}`))
+	pcj2 := NewParamChangeJSON("subspace", "key2", json.RawMessage(`{}`))
 	pcjs := ParamChangesJSON{pcj1, pcj2}
 
 	paramChanges := pcjs.ToParamChanges()
@@ -25,11 +24,9 @@ func TestToParamChanges(t *testing.T) {
 
 	require.Equal(t, paramChanges[0].Subspace, pcj1.Subspace)
 	require.Equal(t, paramChanges[0].Key, pcj1.Key)
-	require.Equal(t, paramChanges[0].Subkey, pcj1.Subkey)
 	require.Equal(t, paramChanges[0].Value, string(pcj1.Value))
 
 	require.Equal(t, paramChanges[1].Subspace, pcj2.Subspace)
 	require.Equal(t, paramChanges[1].Key, pcj2.Key)
-	require.Equal(t, paramChanges[1].Subkey, pcj2.Subkey)
 	require.Equal(t, paramChanges[1].Value, string(pcj2.Value))
 }

x/params/doc.go

@@ -1,11 +1,10 @@
-package params
-
 /*
-Package params provides a globally available parameter store.
+Package params provides a namespaced module parameter store.
 
-There are two main types, Keeper and Subspace. Subspace is an isolated namespace for a
+There are two core components, Keeper and Subspace. Subspace is an isolated
-paramstore, where keys are prefixed by preconfigured spacename. Keeper has a
+namespace for a parameter store, where keys are prefixed by pre-configured
-permission to access all existing spaces.
+subspace names which modules provide. The Keeper has a permission to access all
+existing subspaces.
 
 Subspace can be used by the individual keepers, who needs a private parameter store
 that the other keeper cannot modify. Keeper can be used by the Governance keeper,
@@ -13,12 +12,10 @@ who need to modify any parameter in case of the proposal passes.
 
 Basic Usage:
 
-First, declare parameter space and parameter keys for the module. Then include
+1. Declare constant module parameter keys and the globally unique Subspace name:
-params.Subspace in the keeper. Since we prefix the keys with the spacename, it is
-recommended to use the same name with the module's.
 
 	const (
-		DefaultParamspace = "mymodule"
+		ModuleSubspace = "mymodule"
 	)
 
 	const (
@@ -26,93 +23,89 @@ recommended to use the same name with the module's.
 		KeyParameter2 = "myparameter2"
 	)
 
-	type Keeper struct {
+2. Create a concrete parameter struct and define the validation functions:
-		cdc *codec.Codec
-		key sdk.StoreKey
-
-		ps params.Subspace
-	}
-
-	func ParamKeyTable() params.KeyTable {
-		return params.NewKeyTable(
-			KeyParameter1, MyStruct{},
-			KeyParameter2, MyStruct{},
-		)
-	}
-
-	func NewKeeper(cdc *codec.Codec, key sdk.StoreKey, ps params.Subspace) Keeper {
-		return Keeper {
-			cdc: cdc,
-			key: key,
-			ps: ps.WithKeyTable(ParamKeyTable()),
-		}
-	}
-
-Pass a params.Subspace to NewKeeper with DefaultParamspace (or another)
-
-	app.myKeeper = mymodule.NewKeeper(app.paramStore.SubStore(mymodule.DefaultParamspace))
-
-Now we can access to the paramstore using Paramstore Keys
-
-	var param MyStruct
-	k.ps.Get(ctx, KeyParameter1, &param)
-	k.ps.Set(ctx, KeyParameter2, param)
-
-If you want to store an unknown number of parameters, or want to store a mapping,
-you can use subkeys. Subkeys can be used with a main key, where the subkeys are
-inheriting the key properties.
-
-	func ParamKeyTable() params.KeyTable {
-		return params.NewKeyTable(
-			KeyParamMain, MyStruct{},
-		)
-	}
-
-
-	func (k Keeper) GetDynamicParameter(ctx sdk.Context, subkey []byte) (res MyStruct) {
-		k.ps.GetWithSubkey(ctx, KeyParamMain, subkey, &res)
-	}
-
-Genesis Usage:
-
-Declare a struct for parameters and make it implement params.ParamSet. It will then
-be able to be passed to SetParamSet.
 
 	type MyParams struct {
-		Parameter1 uint64
+		MyParam1 int64 `json:"my_param1" yaml:"my_param1"`
-		Parameter2 string
+		MyParam2 bool `json:"my_param2" yaml:"my_param2"`
 	}
 
-	// Implements params.ParamSet
+	func validateMyParam1(i interface{}) error {
-	// KeyValuePairs must return the list of (ParamKey, PointerToTheField)
+		_, ok := i.(int64)
-	func (p *MyParams) KeyValuePairs() params.KeyValuePairs {
-		return params.KeyFieldPairs {
-			{KeyParameter1, &p.Parameter1},
-			{KeyParameter2, &p.Parameter2},
-		}
-	}
-
-	func InitGenesis(ctx sdk.Context, k Keeper, data GenesisState) {
-		k.ps.SetParamSet(ctx, &data.params)
-	}
-
-The method is pointer receiver because there could be a case that we read from
-the store and set the result to the struct.
-
-Master Keeper Usage:
-
-Keepers that require master permission to the paramstore, such as gov, can take
-params.Keeper itself to access all subspace(using GetSubspace)
-
-	type MasterKeeper struct {
-		pk params.Keeper
-	}
-
-	func (k MasterKeeper) SetParam(ctx sdk.Context, space string, key string, param interface{}) {
-		space, ok := k.pk.GetSubspace(space)
 		if !ok {
-			return
+			return fmt.Errorf("invalid parameter type: %T", i)
 		}
-		space.Set(ctx, key, param)
+
+		// validate (if necessary)...
+
+		return nil
 	}
+
+	func validateMyParam2(i interface{}) error {
+		_, ok := i.(bool)
+		if !ok {
+			return fmt.Errorf("invalid parameter type: %T", i)
+		}
+
+		// validate (if necessary)...
+
+		return nil
+	}
+
+3. Implement the params.ParamSet interface:
+
+	func (p *MyParams) ParamSetPairs() params.ParamSetPairs {
+		return params.ParamSetPairs{
+			{KeyParameter1, &p.MyParam1, validateMyParam1},
+			{KeyParameter2, &p.MyParam2, validateMyParam2},
+		}
+	}
+
+	func paramKeyTable() params.KeyTable {
+		return params.NewKeyTable().RegisterParamSet(&MyParams{})
+	}
+
+4. Have the module accept a Subspace in the constructor and set the KeyTable (if necessary):
+
+	func NewKeeper(..., paramSpace params.Subspace, ...) Keeper {
+		// set KeyTable if it has not already been set
+		if !paramSpace.HasKeyTable() {
+			paramSpace = paramSpace.WithKeyTable(paramKeyTable())
+		}
+
+		return Keeper {
+			// ...
+			paramSpace: paramSpace,
+		}
+	}
+
+Now we have access to the module's parameters that are namespaced using the keys defined:
+
+	func InitGenesis(ctx sdk.Context, k Keeper, gs GenesisState) {
+		// ...
+		k.SetParams(ctx, gs.Params)
+	}
+
+	func (k Keeper) SetParams(ctx sdk.Context, params Params) {
+		k.paramSpace.SetParamSet(ctx, &params)
+	}
+
+	func (k Keeper) GetParams(ctx sdk.Context) (params Params) {
+		k.paramSpace.GetParamSet(ctx, &params)
+		return params
+	}
+
+	func (k Keeper) MyParam1(ctx sdk.Context) (res int64) {
+		k.paramSpace.Get(ctx, KeyParameter1, &res)
+		return res
+	}
+
+	func (k Keeper) MyParam2(ctx sdk.Context) (res bool) {
+		k.paramSpace.Get(ctx, KeyParameter2, &res)
+		return res
+	}
+
+NOTE: Any call to SetParamSet will panic or any call to Update will error if any
+given parameter value is invalid based on the registered value validation function.
 */
+package params

x/params/keeper.go

@@ -21,16 +21,14 @@ type Keeper struct {
 }
 
 // NewKeeper constructs a params keeper
-func NewKeeper(cdc *codec.Codec, key, tkey sdk.StoreKey, codespace sdk.CodespaceType) (k Keeper) {
+func NewKeeper(cdc *codec.Codec, key, tkey sdk.StoreKey, codespace sdk.CodespaceType) Keeper {
-	k = Keeper{
+	return Keeper{
 		cdc:       cdc,
 		key:       key,
 		tkey:      tkey,
 		codespace: codespace,
 		spaces:    make(map[string]*Subspace),
 	}
-
-	return k
 }
 
 // Logger returns a module-specific logger.

x/params/keeper_test.go

@@ -10,6 +10,8 @@ import (
 	sdk "github.com/cosmos/cosmos-sdk/types"
 )
 
+func validateNoOp(_ interface{}) error { return nil }
+
 func TestKeeper(t *testing.T) {
 	kvs := []struct {
 		key   string
@@ -25,15 +27,15 @@ func TestKeeper(t *testing.T) {
 	}
 
 	table := NewKeyTable(
-		[]byte("key1"), int64(0),
+		NewParamSetPair([]byte("key1"), int64(0), validateNoOp),
-		[]byte("key2"), int64(0),
+		NewParamSetPair([]byte("key2"), int64(0), validateNoOp),
-		[]byte("key3"), int64(0),
+		NewParamSetPair([]byte("key3"), int64(0), validateNoOp),
-		[]byte("key4"), int64(0),
+		NewParamSetPair([]byte("key4"), int64(0), validateNoOp),
-		[]byte("key5"), int64(0),
+		NewParamSetPair([]byte("key5"), int64(0), validateNoOp),
-		[]byte("key6"), int64(0),
+		NewParamSetPair([]byte("key6"), int64(0), validateNoOp),
-		[]byte("key7"), int64(0),
+		NewParamSetPair([]byte("key7"), int64(0), validateNoOp),
-		[]byte("extra1"), bool(false),
+		NewParamSetPair([]byte("extra1"), bool(false), validateNoOp),
-		[]byte("extra2"), string(""),
+		NewParamSetPair([]byte("extra2"), string(""), validateNoOp),
 	)
 
 	cdc, ctx, skey, _, keeper := testComponents()
@@ -135,18 +137,18 @@ func TestSubspace(t *testing.T) {
 	}
 
 	table := NewKeyTable(
-		[]byte("string"), string(""),
+		NewParamSetPair([]byte("string"), string(""), validateNoOp),
-		[]byte("bool"), bool(false),
+		NewParamSetPair([]byte("bool"), bool(false), validateNoOp),
-		[]byte("int16"), int16(0),
+		NewParamSetPair([]byte("int16"), int16(0), validateNoOp),
-		[]byte("int32"), int32(0),
+		NewParamSetPair([]byte("int32"), int32(0), validateNoOp),
-		[]byte("int64"), int64(0),
+		NewParamSetPair([]byte("int64"), int64(0), validateNoOp),
-		[]byte("uint16"), uint16(0),
+		NewParamSetPair([]byte("uint16"), uint16(0), validateNoOp),
-		[]byte("uint32"), uint32(0),
+		NewParamSetPair([]byte("uint32"), uint32(0), validateNoOp),
-		[]byte("uint64"), uint64(0),
+		NewParamSetPair([]byte("uint64"), uint64(0), validateNoOp),
-		[]byte("int"), sdk.Int{},
+		NewParamSetPair([]byte("int"), sdk.Int{}, validateNoOp),
-		[]byte("uint"), sdk.Uint{},
+		NewParamSetPair([]byte("uint"), sdk.Uint{}, validateNoOp),
-		[]byte("dec"), sdk.Dec{},
+		NewParamSetPair([]byte("dec"), sdk.Dec{}, validateNoOp),
-		[]byte("struct"), s{},
+		NewParamSetPair([]byte("struct"), s{}, validateNoOp),
 	)
 
 	store := prefix.NewStore(ctx.KVStore(key), []byte("test/"))
@@ -200,7 +202,7 @@ func TestJSONUpdate(t *testing.T) {
 
 	key := []byte("key")
 
-	space := keeper.Subspace("test").WithKeyTable(NewKeyTable(key, paramJSON{}))
+	space := keeper.Subspace("test").WithKeyTable(NewKeyTable(NewParamSetPair(key, paramJSON{}, validateNoOp)))
 
 	var param paramJSON
 

x/params/proposal_handler.go

@@ -28,22 +28,12 @@ func handleParameterChangeProposal(ctx sdk.Context, k Keeper, p ParameterChangeP
 			return ErrUnknownSubspace(k.codespace, c.Subspace)
 		}
 
-		var err error
+		k.Logger(ctx).Info(
-		if len(c.Subkey) == 0 {
+			fmt.Sprintf("attempt to set new parameter value; key: %s, value: %s", c.Key, c.Value),
-			k.Logger(ctx).Info(
+		)
-				fmt.Sprintf("setting new parameter; key: %s, value: %s", c.Key, c.Value),
-			)
 
-			err = ss.Update(ctx, []byte(c.Key), []byte(c.Value))
+		if err := ss.Update(ctx, []byte(c.Key), []byte(c.Value)); err != nil {
-		} else {
+			return ErrSettingParameter(k.codespace, c.Key, c.Value, err.Error())
-			k.Logger(ctx).Info(
-				fmt.Sprintf("setting new parameter; key: %s, subkey: %s, value: %s", c.Key, c.Subspace, c.Value),
-			)
-			err = ss.UpdateWithSubkey(ctx, []byte(c.Key), []byte(c.Subkey), []byte(c.Value))
-		}
-
-		if err != nil {
-			return ErrSettingParameter(k.codespace, c.Key, c.Subkey, c.Value, err.Error())
 		}
 	}
 

x/params/proposal_handler_test.go

@@ -17,6 +17,8 @@ import (
 	"github.com/cosmos/cosmos-sdk/x/params/types"
 )
 
+func validateNoOp(_ interface{}) error { return nil }
+
 type testInput struct {
 	ctx    sdk.Context
 	cdc    *codec.Codec
@@ -43,8 +45,8 @@ type testParams struct {
 
 func (tp *testParams) ParamSetPairs() subspace.ParamSetPairs {
 	return subspace.ParamSetPairs{
-		{Key: []byte(keyMaxValidators), Value: &tp.MaxValidators},
+		params.NewParamSetPair([]byte(keyMaxValidators), &tp.MaxValidators, validateNoOp),
-		{Key: []byte(keySlashingRate), Value: &tp.SlashingRate},
+		params.NewParamSetPair([]byte(keySlashingRate), &tp.SlashingRate, validateNoOp),
 	}
 }
 

x/params/simulation/operations.go

@@ -40,7 +40,7 @@ func SimulateParamChangeProposalContent(paramChangePool []simulation.ParamChange
 			// add a new distinct parameter to the set of changes and register the key
 			// to avoid further duplicates
 			paramChangesKeys[spc.ComposedKey()] = struct{}{}
-			paramChanges[i] = types.NewParamChangeWithSubkey(spc.Subspace, spc.Key, spc.Subkey, spc.SimValue(r))
+			paramChanges[i] = types.NewParamChange(spc.Subspace, spc.Key, spc.SimValue(r))
 		}
 
 		return types.NewParameterChangeProposal(

x/params/subspace/common_test.go

@@ -0,0 +1,72 @@
+package subspace_test
+
+import (
+	"errors"
+	"fmt"
+	"time"
+
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	"github.com/cosmos/cosmos-sdk/x/params/subspace"
+)
+
+var (
+	keyUnbondingTime = []byte("UnbondingTime")
+	keyMaxValidators = []byte("MaxValidators")
+	keyBondDenom     = []byte("BondDenom")
+
+	key  = sdk.NewKVStoreKey("storekey")
+	tkey = sdk.NewTransientStoreKey("transientstorekey")
+)
+
+type params struct {
+	UnbondingTime time.Duration `json:"unbonding_time" yaml:"unbonding_time"`
+	MaxValidators uint16        `json:"max_validators" yaml:"max_validators"`
+	BondDenom     string        `json:"bond_denom" yaml:"bond_denom"`
+}
+
+func validateUnbondingTime(i interface{}) error {
+	v, ok := i.(time.Duration)
+	if !ok {
+		return fmt.Errorf("invalid parameter type: %T", i)
+	}
+
+	if v < (24 * time.Hour) {
+		return fmt.Errorf("unbonding time must be at least one day")
+	}
+
+	return nil
+}
+
+func validateMaxValidators(i interface{}) error {
+	_, ok := i.(uint16)
+	if !ok {
+		return fmt.Errorf("invalid parameter type: %T", i)
+	}
+
+	return nil
+}
+
+func validateBondDenom(i interface{}) error {
+	v, ok := i.(string)
+	if !ok {
+		return fmt.Errorf("invalid parameter type: %T", i)
+	}
+
+	if len(v) == 0 {
+		return errors.New("denom cannot be empty")
+	}
+
+	return nil
+}
+
+func (p *params) ParamSetPairs() subspace.ParamSetPairs {
+	return subspace.ParamSetPairs{
+		{keyUnbondingTime, &p.UnbondingTime, validateUnbondingTime},
+		{keyMaxValidators, &p.MaxValidators, validateMaxValidators},
+		{keyBondDenom, &p.BondDenom, validateBondDenom},
+	}
+}
+
+func paramKeyTable() subspace.KeyTable {
+	return subspace.NewKeyTable().RegisterParamSet(&params{})
+}

x/params/subspace/doc.go

@@ -1,14 +1,13 @@
-package subspace
-
 /*
-To prevent namespace collision between consumer modules, we define type
+To prevent namespace collision between consumer modules, we define a type
-"space". A Space can only be generated by the keeper, and the keeper checks
+Subspace. A Subspace can only be generated by the keeper, and the keeper checks
-the existence of the space having the same name before generating the
+the existence of the Subspace having the same name before generating the
-space.
+Subspace.
 
-Consumer modules must take a space (via Keeper.Subspace), not the keeper
+Consumer modules must take a Subspace (via Keeper.Subspace), not the keeper
-itself. This isolates each modules from the others and make them modify the
+itself. This isolates each modules from the others and make them modify their
-parameters safely. Keeper can be treated as master permission for all
+respective parameters safely. Keeper can be treated as master permission for all
-subspaces (via Keeper.GetSubspace), so should be passed to proper modules
+Subspaces (via Keeper.GetSubspace), so should be passed to proper modules
-(ex. gov)
+(ex. x/governance).
 */
+package subspace

x/params/subspace/paramset.go

@@ -1,14 +1,20 @@
 package subspace
 
-// ParamSetPair is used for associating paramsubspace key and field of param structs
+type (
-type ParamSetPair struct {
+	ValueValidatorFn func(value interface{}) error
-	Key   []byte
-	Value interface{}
-}
 
-// NewParamSetPair creates a new ParamSetPair instance
+	// ParamSetPair is used for associating paramsubspace key and field of param
-func NewParamSetPair(key []byte, value interface{}) ParamSetPair {
+	// structs.
-	return ParamSetPair{key, value}
+	ParamSetPair struct {
+		Key         []byte
+		Value       interface{}
+		ValidatorFn ValueValidatorFn
+	}
+)
+
+// NewParamSetPair creates a new ParamSetPair instance.
+func NewParamSetPair(key []byte, value interface{}, vfn ValueValidatorFn) ParamSetPair {
+	return ParamSetPair{key, value, vfn}
 }
 
 // ParamSetPairs Slice of KeyFieldPair

x/params/subspace/subspace.go

@@ -22,28 +22,22 @@ const (
 // Transient store persists for a block, so we use it for
 // recording whether the parameter has been changed or not
 type Subspace struct {
-	cdc  *codec.Codec
+	cdc   *codec.Codec
-	key  sdk.StoreKey // []byte -> []byte, stores parameter
+	key   sdk.StoreKey // []byte -> []byte, stores parameter
-	tkey sdk.StoreKey // []byte -> bool, stores parameter change
+	tkey  sdk.StoreKey // []byte -> bool, stores parameter change
-
+	name  []byte
-	name []byte
-
 	table KeyTable
 }
 
 // NewSubspace constructs a store with namestore
-func NewSubspace(cdc *codec.Codec, key sdk.StoreKey, tkey sdk.StoreKey, name string) (res Subspace) {
+func NewSubspace(cdc *codec.Codec, key sdk.StoreKey, tkey sdk.StoreKey, name string) Subspace {
-	res = Subspace{
+	return Subspace{
-		cdc:  cdc,
+		cdc:   cdc,
-		key:  key,
+		key:   key,
-		tkey: tkey,
+		tkey:  tkey,
-		name: []byte(name),
+		name:  []byte(name),
-		table: KeyTable{
+		table: NewKeyTable(),
-			m: make(map[string]attribute),
-		},
 	}
-
-	return
 }
 
 // HasKeyTable returns if the Subspace has a KeyTable registered.
@@ -64,7 +58,7 @@ func (s Subspace) WithKeyTable(table KeyTable) Subspace {
 		s.table.m[k] = v
 	}
 
-	// Allocate additional capicity for Subspace.name
+	// Allocate additional capacity for Subspace.name
 	// So we don't have to allocate extra space each time appending to the key
 	name := s.name
 	s.name = make([]byte, len(name), len(name)+table.maxKeyLength())
@@ -87,105 +81,110 @@ func (s Subspace) transientStore(ctx sdk.Context) sdk.KVStore {
 	return prefix.NewStore(ctx.TransientStore(s.tkey), append(s.name, '/'))
 }
 
-func concatKeys(key, subkey []byte) (res []byte) {
+// Validate attempts to validate a parameter value by its key. If the key is not
-	res = make([]byte, len(key)+1+len(subkey))
+// registered or if the validation of the value fails, an error is returned.
-	copy(res, key)
+func (s Subspace) Validate(ctx sdk.Context, key []byte, value interface{}) error {
-	res[len(key)] = '/'
+	attr, ok := s.table.m[string(key)]
-	copy(res[len(key)+1:], subkey)
+	if !ok {
-	return
+		return fmt.Errorf("parameter %s not registered", string(key))
+	}
+
+	if err := attr.vfn(value); err != nil {
+		return fmt.Errorf("invalid parameter value: %s", err)
+	}
+
+	return nil
 }
 
-// Get parameter from store
+// Get queries for a parameter by key from the Subspace's KVStore and sets the
+// value to the provided pointer. If the value does not exist, it will panic.
 func (s Subspace) Get(ctx sdk.Context, key []byte, ptr interface{}) {
 	store := s.kvStore(ctx)
 	bz := store.Get(key)
-	err := s.cdc.UnmarshalJSON(bz, ptr)
+
-	if err != nil {
+	if err := s.cdc.UnmarshalJSON(bz, ptr); err != nil {
 		panic(err)
 	}
 }
 
-// GetIfExists do not modify ptr if the stored parameter is nil
+// GetIfExists queries for a parameter by key from the Subspace's KVStore and
+// sets the value to the provided pointer. If the value does not exist, it will
+// perform a no-op.
 func (s Subspace) GetIfExists(ctx sdk.Context, key []byte, ptr interface{}) {
 	store := s.kvStore(ctx)
 	bz := store.Get(key)
 	if bz == nil {
 		return
 	}
-	err := s.cdc.UnmarshalJSON(bz, ptr)
+
-	if err != nil {
+	if err := s.cdc.UnmarshalJSON(bz, ptr); err != nil {
 		panic(err)
 	}
 }
 
-// GetWithSubkey returns a parameter with a given key and a subkey.
+// GetRaw queries for the raw values bytes for a parameter by key.
-func (s Subspace) GetWithSubkey(ctx sdk.Context, key, subkey []byte, ptr interface{}) {
-	s.Get(ctx, concatKeys(key, subkey), ptr)
-}
-
-// GetWithSubkeyIfExists  returns a parameter with a given key and a subkey but does not
-// modify ptr if the stored parameter is nil.
-func (s Subspace) GetWithSubkeyIfExists(ctx sdk.Context, key, subkey []byte, ptr interface{}) {
-	s.GetIfExists(ctx, concatKeys(key, subkey), ptr)
-}
-
-// Get raw bytes of parameter from store
 func (s Subspace) GetRaw(ctx sdk.Context, key []byte) []byte {
 	store := s.kvStore(ctx)
 	return store.Get(key)
 }
 
-// Check if the parameter is set in the store
+// Has returns if a parameter key exists or not in the Subspace's KVStore.
 func (s Subspace) Has(ctx sdk.Context, key []byte) bool {
 	store := s.kvStore(ctx)
 	return store.Has(key)
 }
 
-// Returns true if the parameter is set in the block
+// Modified returns true if the parameter key is set in the Subspace's transient
+// KVStore.
 func (s Subspace) Modified(ctx sdk.Context, key []byte) bool {
 	tstore := s.transientStore(ctx)
 	return tstore.Has(key)
 }
 
-func (s Subspace) checkType(store sdk.KVStore, key []byte, param interface{}) {
+// checkType verifies that the provided key and value are comptable and registered.
+func (s Subspace) checkType(key []byte, value interface{}) {
 	attr, ok := s.table.m[string(key)]
 	if !ok {
 		panic(fmt.Sprintf("parameter %s not registered", string(key)))
 	}
 
 	ty := attr.ty
-	pty := reflect.TypeOf(param)
+	pty := reflect.TypeOf(value)
 	if pty.Kind() == reflect.Ptr {
 		pty = pty.Elem()
 	}
 
 	if pty != ty {
-		panic("Type mismatch with registered table")
+		panic("type mismatch with registered table")
 	}
 }
 
-// Set stores the parameter. It returns error if stored parameter has different type from input.
+// Set stores a value for given a parameter key assuming the parameter type has
-// It also set to the transient store to record change.
+// been registered. It will panic if the parameter type has not been registered
-func (s Subspace) Set(ctx sdk.Context, key []byte, param interface{}) {
+// or if the value cannot be encoded. A change record is also set in the Subspace's
+// transient KVStore to mark the parameter as modified.
+func (s Subspace) Set(ctx sdk.Context, key []byte, value interface{}) {
+	s.checkType(key, value)
 	store := s.kvStore(ctx)
 
-	s.checkType(store, key, param)
+	bz, err := s.cdc.MarshalJSON(value)
-
-	bz, err := s.cdc.MarshalJSON(param)
 	if err != nil {
 		panic(err)
 	}
+
 	store.Set(key, bz)
 
 	tstore := s.transientStore(ctx)
 	tstore.Set(key, []byte{})
-
 }
 
-// Update stores raw parameter bytes. It returns error if the stored parameter
+// Update stores an updated raw value for a given parameter key assuming the
-// has a different type from the input. It also sets to the transient store to
+// parameter type has been registered. It will panic if the parameter type has
-// record change.
+// not been registered or if the value cannot be encoded. An error is returned
-func (s Subspace) Update(ctx sdk.Context, key []byte, param []byte) error {
+// if the raw value is not compatible with the registered type for the parameter
+// key or if the new value is invalid as determined by the registered type's
+// validation function.
+func (s Subspace) Update(ctx sdk.Context, key, value []byte) error {
 	attr, ok := s.table.m[string(key)]
 	if !ok {
 		panic(fmt.Sprintf("parameter %s not registered", string(key)))
@@ -194,72 +193,33 @@ func (s Subspace) Update(ctx sdk.Context, key []byte, param []byte) error {
 	ty := attr.ty
 	dest := reflect.New(ty).Interface()
 	s.GetIfExists(ctx, key, dest)
-	err := s.cdc.UnmarshalJSON(param, dest)
+
-	if err != nil {
+	if err := s.cdc.UnmarshalJSON(value, dest); err != nil {
+		return err
+	}
+
+	// destValue contains the dereferenced value of dest so validation function do
+	// not have to operate on pointers.
+	destValue := reflect.Indirect(reflect.ValueOf(dest)).Interface()
+	if err := s.Validate(ctx, key, destValue); err != nil {
 		return err
 	}
 
 	s.Set(ctx, key, dest)
-
-	// TODO: Remove; seems redundant as Set already does this.
-	tStore := s.transientStore(ctx)
-	tStore.Set(key, []byte{})
-
 	return nil
 }
 
-// SetWithSubkey set a parameter with a key and subkey
+// GetParamSet iterates through each ParamSetPair where for each pair, it will
-// Checks parameter type only over the key
+// retrieve the value and set it to the corresponding value pointer provided
-func (s Subspace) SetWithSubkey(ctx sdk.Context, key []byte, subkey []byte, param interface{}) {
+// in the ParamSetPair by calling Subspace#Get.
-	store := s.kvStore(ctx)
-
-	s.checkType(store, key, param)
-
-	newkey := concatKeys(key, subkey)
-
-	bz, err := s.cdc.MarshalJSON(param)
-	if err != nil {
-		panic(err)
-	}
-	store.Set(newkey, bz)
-
-	tstore := s.transientStore(ctx)
-	tstore.Set(newkey, []byte{})
-}
-
-// UpdateWithSubkey stores raw parameter bytes  with a key and subkey. It checks
-// the parameter type only over the key.
-func (s Subspace) UpdateWithSubkey(ctx sdk.Context, key []byte, subkey []byte, param []byte) error {
-	concatkey := concatKeys(key, subkey)
-
-	attr, ok := s.table.m[string(concatkey)]
-	if !ok {
-		return fmt.Errorf("parameter %s not registered", string(key))
-	}
-
-	ty := attr.ty
-	dest := reflect.New(ty).Interface()
-	s.GetWithSubkeyIfExists(ctx, key, subkey, dest)
-	err := s.cdc.UnmarshalJSON(param, dest)
-	if err != nil {
-		return err
-	}
-
-	s.SetWithSubkey(ctx, key, subkey, dest)
-	tStore := s.transientStore(ctx)
-	tStore.Set(concatkey, []byte{})
-
-	return nil
-}
-
-// Get to ParamSet
 func (s Subspace) GetParamSet(ctx sdk.Context, ps ParamSet) {
 	for _, pair := range ps.ParamSetPairs() {
 		s.Get(ctx, pair.Key, pair.Value)
 	}
 }
 
-// Set from ParamSet
+// SetParamSet iterates through each ParamSetPair and sets the value with the
+// corresponding parameter key in the Subspace's KVStore.
 func (s Subspace) SetParamSet(ctx sdk.Context, ps ParamSet) {
 	for _, pair := range ps.ParamSetPairs() {
 		// pair.Field is a pointer to the field, so indirecting the ptr.
@@ -267,11 +227,16 @@ func (s Subspace) SetParamSet(ctx sdk.Context, ps ParamSet) {
 		// since SetStruct is meant to be used in InitGenesis
 		// so this method will not be called frequently
 		v := reflect.Indirect(reflect.ValueOf(pair.Value)).Interface()
+
+		if err := pair.ValidatorFn(v); err != nil {
+			panic(fmt.Sprintf("value from ParamSetPair is invalid: %s", err))
+		}
+
 		s.Set(ctx, pair.Key, v)
 	}
 }
 
-// Returns name of Subspace
+// Name returns the name of the Subspace.
 func (s Subspace) Name() string {
 	return string(s.name)
 }
@@ -281,27 +246,27 @@ type ReadOnlySubspace struct {
 	s Subspace
 }
 
-// Exposes Get
+// Get delegates a read-only Get call to the Subspace.
 func (ros ReadOnlySubspace) Get(ctx sdk.Context, key []byte, ptr interface{}) {
 	ros.s.Get(ctx, key, ptr)
 }
 
-// Exposes GetRaw
+// GetRaw delegates a read-only GetRaw call to the Subspace.
 func (ros ReadOnlySubspace) GetRaw(ctx sdk.Context, key []byte) []byte {
 	return ros.s.GetRaw(ctx, key)
 }
 
-// Exposes Has
+// Has delegates a read-only Has call to the Subspace.
 func (ros ReadOnlySubspace) Has(ctx sdk.Context, key []byte) bool {
 	return ros.s.Has(ctx, key)
 }
 
-// Exposes Modified
+// Modified delegates a read-only Modified call to the Subspace.
 func (ros ReadOnlySubspace) Modified(ctx sdk.Context, key []byte) bool {
 	return ros.s.Modified(ctx, key)
 }
 
-// Exposes Space
+// Name delegates a read-only Name call to the Subspace.
 func (ros ReadOnlySubspace) Name() string {
 	return ros.s.Name()
 }

x/params/subspace/subspace_test.go

@@ -0,0 +1,203 @@
+package subspace_test
+
+import (
+	"fmt"
+	"testing"
+	"time"
+
+	"github.com/cosmos/cosmos-sdk/codec"
+	"github.com/cosmos/cosmos-sdk/store"
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	"github.com/cosmos/cosmos-sdk/x/params/subspace"
+	"github.com/stretchr/testify/suite"
+	abci "github.com/tendermint/tendermint/abci/types"
+	"github.com/tendermint/tendermint/libs/log"
+	dbm "github.com/tendermint/tm-db"
+)
+
+type SubspaceTestSuite struct {
+	suite.Suite
+
+	cdc *codec.Codec
+	ctx sdk.Context
+	ss  subspace.Subspace
+}
+
+func (suite *SubspaceTestSuite) SetupTest() {
+	cdc := codec.New()
+	db := dbm.NewMemDB()
+
+	ms := store.NewCommitMultiStore(db)
+	ms.MountStoreWithDB(key, sdk.StoreTypeIAVL, db)
+	ms.MountStoreWithDB(tkey, sdk.StoreTypeTransient, db)
+	suite.NoError(ms.LoadLatestVersion())
+
+	ss := subspace.NewSubspace(cdc, key, tkey, "testsubspace")
+
+	suite.cdc = cdc
+	suite.ctx = sdk.NewContext(ms, abci.Header{}, false, log.NewNopLogger())
+	suite.ss = ss.WithKeyTable(paramKeyTable())
+}
+
+func (suite *SubspaceTestSuite) TestKeyTable() {
+	suite.Require().True(suite.ss.HasKeyTable())
+	suite.Require().Panics(func() {
+		suite.ss.WithKeyTable(paramKeyTable())
+	})
+	suite.Require().NotPanics(func() {
+		ss := subspace.NewSubspace(codec.New(), key, tkey, "testsubspace2")
+		ss = ss.WithKeyTable(paramKeyTable())
+	})
+}
+
+func (suite *SubspaceTestSuite) TestGetSet() {
+	var v time.Duration
+	t := time.Hour * 48
+
+	suite.Require().Panics(func() {
+		suite.ss.Get(suite.ctx, keyUnbondingTime, &v)
+	})
+	suite.Require().NotEqual(t, v)
+	suite.Require().NotPanics(func() {
+		suite.ss.Set(suite.ctx, keyUnbondingTime, t)
+	})
+	suite.Require().NotPanics(func() {
+		suite.ss.Get(suite.ctx, keyUnbondingTime, &v)
+	})
+	suite.Require().Equal(t, v)
+}
+
+func (suite *SubspaceTestSuite) TestGetIfExists() {
+	var v time.Duration
+
+	suite.Require().NotPanics(func() {
+		suite.ss.GetIfExists(suite.ctx, keyUnbondingTime, &v)
+	})
+	suite.Require().Equal(time.Duration(0), v)
+}
+
+func (suite *SubspaceTestSuite) TestGetRaw() {
+	t := time.Hour * 48
+
+	suite.Require().NotPanics(func() {
+		suite.ss.Set(suite.ctx, keyUnbondingTime, t)
+	})
+	suite.Require().NotPanics(func() {
+		res := suite.ss.GetRaw(suite.ctx, keyUnbondingTime)
+		suite.Require().Equal("2231373238303030303030303030303022", fmt.Sprintf("%X", res))
+	})
+}
+
+func (suite *SubspaceTestSuite) TestHas() {
+	t := time.Hour * 48
+
+	suite.Require().False(suite.ss.Has(suite.ctx, keyUnbondingTime))
+	suite.Require().NotPanics(func() {
+		suite.ss.Set(suite.ctx, keyUnbondingTime, t)
+	})
+	suite.Require().True(suite.ss.Has(suite.ctx, keyUnbondingTime))
+}
+
+func (suite *SubspaceTestSuite) TestModified() {
+	t := time.Hour * 48
+
+	suite.Require().False(suite.ss.Modified(suite.ctx, keyUnbondingTime))
+	suite.Require().NotPanics(func() {
+		suite.ss.Set(suite.ctx, keyUnbondingTime, t)
+	})
+	suite.Require().True(suite.ss.Modified(suite.ctx, keyUnbondingTime))
+}
+
+func (suite *SubspaceTestSuite) TestUpdate() {
+	suite.Require().Panics(func() {
+		suite.ss.Update(suite.ctx, []byte("invalid_key"), nil)
+	})
+
+	t := time.Hour * 48
+	suite.Require().NotPanics(func() {
+		suite.ss.Set(suite.ctx, keyUnbondingTime, t)
+	})
+
+	bad := time.Minute * 5
+
+	bz, err := suite.cdc.MarshalJSON(bad)
+	suite.Require().NoError(err)
+	suite.Require().Error(suite.ss.Update(suite.ctx, keyUnbondingTime, bz))
+
+	good := time.Hour * 360
+	bz, err = suite.cdc.MarshalJSON(good)
+	suite.Require().NoError(err)
+	suite.Require().NoError(suite.ss.Update(suite.ctx, keyUnbondingTime, bz))
+
+	var v time.Duration
+
+	suite.Require().NotPanics(func() {
+		suite.ss.Get(suite.ctx, keyUnbondingTime, &v)
+	})
+	suite.Require().Equal(good, v)
+}
+
+func (suite *SubspaceTestSuite) TestGetParamSet() {
+	a := params{
+		UnbondingTime: time.Hour * 48,
+		MaxValidators: 100,
+		BondDenom:     "stake",
+	}
+	suite.Require().NotPanics(func() {
+		suite.ss.Set(suite.ctx, keyUnbondingTime, a.UnbondingTime)
+		suite.ss.Set(suite.ctx, keyMaxValidators, a.MaxValidators)
+		suite.ss.Set(suite.ctx, keyBondDenom, a.BondDenom)
+	})
+
+	b := params{}
+	suite.Require().NotPanics(func() {
+		suite.ss.GetParamSet(suite.ctx, &b)
+	})
+	suite.Require().Equal(a.UnbondingTime, b.UnbondingTime)
+	suite.Require().Equal(a.MaxValidators, b.MaxValidators)
+	suite.Require().Equal(a.BondDenom, b.BondDenom)
+}
+
+func (suite *SubspaceTestSuite) TestSetParamSet() {
+	testCases := []struct {
+		name string
+		ps   subspace.ParamSet
+	}{
+		{"invalid unbonding time", &params{time.Hour * 1, 100, "stake"}},
+		{"invalid bond denom", &params{time.Hour * 48, 100, ""}},
+	}
+
+	for _, tc := range testCases {
+		tc := tc
+		suite.Run(tc.name, func() {
+			suite.Require().Panics(func() {
+				suite.ss.SetParamSet(suite.ctx, tc.ps)
+			})
+		})
+	}
+
+	a := params{
+		UnbondingTime: time.Hour * 48,
+		MaxValidators: 100,
+		BondDenom:     "stake",
+	}
+	suite.Require().NotPanics(func() {
+		suite.ss.SetParamSet(suite.ctx, &a)
+	})
+
+	b := params{}
+	suite.Require().NotPanics(func() {
+		suite.ss.GetParamSet(suite.ctx, &b)
+	})
+	suite.Require().Equal(a.UnbondingTime, b.UnbondingTime)
+	suite.Require().Equal(a.MaxValidators, b.MaxValidators)
+	suite.Require().Equal(a.BondDenom, b.BondDenom)
+}
+
+func (suite *SubspaceTestSuite) TestName() {
+	suite.Require().Equal("testsubspace", suite.ss.Name())
+}
+
+func TestKeeperTestSuite(t *testing.T) {
+	suite.Run(t, new(SubspaceTestSuite))
+}

x/params/subspace/table.go

@@ -2,10 +2,13 @@ package subspace
 
 import (
 	"reflect"
+
+	sdk "github.com/cosmos/cosmos-sdk/types"
 )
 
 type attribute struct {
-	ty reflect.Type
+	ty  reflect.Type
+	vfn ValueValidatorFn
 }
 
 // KeyTable subspaces appropriate type for each parameter key
@@ -13,65 +16,54 @@ type KeyTable struct {
 	m map[string]attribute
 }
 
-// Constructs new table
+func NewKeyTable(pairs ...ParamSetPair) KeyTable {
-func NewKeyTable(keytypes ...interface{}) (res KeyTable) {
+	keyTable := KeyTable{
-	if len(keytypes)%2 != 0 {
-		panic("odd number arguments in NewTypeKeyTable")
-	}
-
-	res = KeyTable{
 		m: make(map[string]attribute),
 	}
 
-	for i := 0; i < len(keytypes); i += 2 {
+	for _, psp := range pairs {
-		res = res.RegisterType(keytypes[i].([]byte), keytypes[i+1])
+		keyTable = keyTable.RegisterType(psp)
 	}
 
-	return
+	return keyTable
 }
 
-func isAlphaNumeric(key []byte) bool {
+// RegisterType registers a single ParamSetPair (key-type pair) in a KeyTable.
-	for _, b := range key {
+func (t KeyTable) RegisterType(psp ParamSetPair) KeyTable {
-		if !((48 <= b && b <= 57) || // numeric
+	if len(psp.Key) == 0 {
-			(65 <= b && b <= 90) || // upper case
+		panic("cannot register ParamSetPair with an parameter empty key")
-			(97 <= b && b <= 122)) { // lower case
+	}
-			return false
+	if !sdk.IsAlphaNumeric(string(psp.Key)) {
-		}
+		panic("cannot register ParamSetPair with a non-alphanumeric parameter key")
+	}
+	if psp.ValidatorFn == nil {
+		panic("cannot register ParamSetPair without a value validation function")
 	}
-	return true
-}
 
-// Register single key-type pair
+	keystr := string(psp.Key)
-func (t KeyTable) RegisterType(key []byte, ty interface{}) KeyTable {
-	if len(key) == 0 {
-		panic("cannot register empty key")
-	}
-	if !isAlphaNumeric(key) {
-		panic("non alphanumeric parameter key")
-	}
-	keystr := string(key)
 	if _, ok := t.m[keystr]; ok {
 		panic("duplicate parameter key")
 	}
 
-	rty := reflect.TypeOf(ty)
+	rty := reflect.TypeOf(psp.Value)
 
-	// Indirect rty if it is ptr
+	// indirect rty if it is a pointer
 	if rty.Kind() == reflect.Ptr {
 		rty = rty.Elem()
 	}
 
 	t.m[keystr] = attribute{
-		ty: rty,
+		vfn: psp.ValidatorFn,
+		ty:  rty,
 	}
 
 	return t
 }
 
-// Register multiple pairs from ParamSet
+// RegisterParamSet registers multiple ParamSetPairs from a ParamSet in a KeyTable.
 func (t KeyTable) RegisterParamSet(ps ParamSet) KeyTable {
-	for _, kvp := range ps.ParamSetPairs() {
+	for _, psp := range ps.ParamSetPairs() {
-		t = t.RegisterType(kvp.Key, kvp.Value)
+		t = t.RegisterType(psp)
 	}
 	return t
 }
@@ -83,5 +75,6 @@ func (t KeyTable) maxKeyLength() (res int) {
 			res = l
 		}
 	}
+
 	return
 }

x/params/subspace/table_test.go

@@ -1,35 +1,45 @@
-package subspace
+package subspace_test
 
 import (
 	"testing"
+	"time"
 
+	"github.com/cosmos/cosmos-sdk/x/params/subspace"
 	"github.com/stretchr/testify/require"
 )
 
-type testparams struct {
-	i int64
-	b bool
-}
-
-func (tp *testparams) ParamSetPairs() ParamSetPairs {
-	return ParamSetPairs{
-		{[]byte("i"), &tp.i},
-		{[]byte("b"), &tp.b},
-	}
-}
-
 func TestKeyTable(t *testing.T) {
-	table := NewKeyTable()
+	table := subspace.NewKeyTable()
 
-	require.Panics(t, func() { table.RegisterType([]byte(""), nil) })
+	require.Panics(t, func() { table.RegisterType(subspace.ParamSetPair{[]byte(""), nil, nil}) })
-	require.Panics(t, func() { table.RegisterType([]byte("!@#$%"), nil) })
+	require.Panics(t, func() { table.RegisterType(subspace.ParamSetPair{[]byte("!@#$%"), nil, nil}) })
-	require.Panics(t, func() { table.RegisterType([]byte("hello,"), nil) })
+	require.Panics(t, func() { table.RegisterType(subspace.ParamSetPair{[]byte("hello,"), nil, nil}) })
-	require.Panics(t, func() { table.RegisterType([]byte("hello"), nil) })
+	require.Panics(t, func() { table.RegisterType(subspace.ParamSetPair{[]byte("hello"), nil, nil}) })
 
-	require.NotPanics(t, func() { table.RegisterType([]byte("hello"), bool(false)) })
+	require.NotPanics(t, func() {
-	require.NotPanics(t, func() { table.RegisterType([]byte("world"), int64(0)) })
+		table.RegisterType(subspace.ParamSetPair{keyBondDenom, string("stake"), validateBondDenom})
-	require.Panics(t, func() { table.RegisterType([]byte("hello"), bool(false)) })
+	})
+	require.NotPanics(t, func() {
+		table.RegisterType(subspace.ParamSetPair{keyMaxValidators, uint16(100), validateMaxValidators})
+	})
+	require.Panics(t, func() {
+		table.RegisterType(subspace.ParamSetPair{keyUnbondingTime, time.Duration(1), nil})
+	})
+	require.NotPanics(t, func() {
+		table.RegisterType(subspace.ParamSetPair{keyUnbondingTime, time.Duration(1), validateMaxValidators})
+	})
+	require.NotPanics(t, func() {
+		newTable := subspace.NewKeyTable()
+		newTable.RegisterParamSet(&params{})
+	})
 
-	require.NotPanics(t, func() { table.RegisterParamSet(&testparams{}) })
+	require.Panics(t, func() { table.RegisterParamSet(&params{}) })
-	require.Panics(t, func() { table.RegisterParamSet(&testparams{}) })
+	require.Panics(t, func() { subspace.NewKeyTable(subspace.ParamSetPair{[]byte(""), nil, nil}) })
+
+	require.NotPanics(t, func() {
+		subspace.NewKeyTable(
+			subspace.ParamSetPair{[]byte("test"), string("stake"), validateBondDenom},
+			subspace.ParamSetPair{[]byte("test2"), uint16(100), validateMaxValidators},
+		)
+	})
 }

x/params/subspace/test_common.go

@@ -1,40 +0,0 @@
-package subspace
-
-import (
-	"os"
-	"testing"
-
-	"github.com/stretchr/testify/require"
-
-	abci "github.com/tendermint/tendermint/abci/types"
-	"github.com/tendermint/tendermint/libs/log"
-	dbm "github.com/tendermint/tm-db"
-
-	"github.com/cosmos/cosmos-sdk/codec"
-	"github.com/cosmos/cosmos-sdk/store"
-	sdk "github.com/cosmos/cosmos-sdk/types"
-)
-
-// Keys for parameter access
-const (
-	TestParamStore = "ParamsTest"
-)
-
-// Returns components for testing
-func DefaultTestComponents(t *testing.T) (sdk.Context, Subspace, func() sdk.CommitID) {
-	cdc := codec.New()
-	key := sdk.NewKVStoreKey(StoreKey)
-	tkey := sdk.NewTransientStoreKey(TStoreKey)
-	db := dbm.NewMemDB()
-	ms := store.NewCommitMultiStore(db)
-	ms.SetTracer(os.Stdout)
-	ms.SetTracingContext(sdk.TraceContext{})
-	ms.MountStoreWithDB(key, sdk.StoreTypeIAVL, db)
-	ms.MountStoreWithDB(tkey, sdk.StoreTypeTransient, db)
-	err := ms.LoadLatestVersion()
-	require.Nil(t, err)
-	ctx := sdk.NewContext(ms, abci.Header{}, false, log.NewTMLogger(os.Stdout))
-	subspace := NewSubspace(cdc, key, tkey, TestParamStore)
-
-	return ctx, subspace, ms.Commit
-}

x/params/types/errors.go

@@ -21,8 +21,8 @@ func ErrUnknownSubspace(codespace sdk.CodespaceType, space string) sdk.Error {
 }
 
 // ErrSettingParameter returns an error for failing to set a parameter.
-func ErrSettingParameter(codespace sdk.CodespaceType, key, subkey, value, msg string) sdk.Error {
+func ErrSettingParameter(codespace sdk.CodespaceType, key, value, msg string) sdk.Error {
-	return sdk.NewError(codespace, CodeSettingParameter, fmt.Sprintf("error setting parameter %s on %s (%s): %s", value, key, subkey, msg))
+	return sdk.NewError(codespace, CodeSettingParameter, fmt.Sprintf("error setting parameter %s on %s: %s", value, key, msg))
 }
 
 // ErrEmptyChanges returns an error for empty parameter changes.

x/params/types/proposal.go

@@ -69,9 +69,8 @@ func (pcp ParameterChangeProposal) String() string {
 		b.WriteString(fmt.Sprintf(`    Param Change:
       Subspace: %s
       Key:      %s
-      Subkey:   %X
       Value:    %X
-`, pc.Subspace, pc.Key, pc.Subkey, pc.Value))
+`, pc.Subspace, pc.Key, pc.Value))
 	}
 
 	return b.String()
@@ -81,16 +80,11 @@ func (pcp ParameterChangeProposal) String() string {
 type ParamChange struct {
 	Subspace string `json:"subspace" yaml:"subspace"`
 	Key      string `json:"key" yaml:"key"`
-	Subkey   string `json:"subkey,omitempty" yaml:"subkey,omitempty"`
 	Value    string `json:"value" yaml:"value"`
 }
 
 func NewParamChange(subspace, key, value string) ParamChange {
-	return ParamChange{subspace, key, "", value}
+	return ParamChange{subspace, key, value}
-}
-
-func NewParamChangeWithSubkey(subspace, key, subkey, value string) ParamChange {
-	return ParamChange{subspace, key, subkey, value}
 }
 
 // String implements the Stringer interface.
@@ -98,9 +92,8 @@ func (pc ParamChange) String() string {
 	return fmt.Sprintf(`Param Change:
   Subspace: %s
   Key:      %s
-  Subkey:   %X
   Value:    %X
-`, pc.Subspace, pc.Key, pc.Subkey, pc.Value)
+`, pc.Subspace, pc.Key, pc.Value)
 }
 
 // ValidateChanges performs basic validation checks over a set of ParamChange. It

x/params/types/proposal_test.go

@@ -8,7 +8,7 @@ import (
 
 func TestParameterChangeProposal(t *testing.T) {
 	pc1 := NewParamChange("sub", "foo", "baz")
-	pc2 := NewParamChangeWithSubkey("sub", "bar", "cat", "dog")
+	pc2 := NewParamChange("sub", "bar", "cat")
 	pcp := NewParameterChangeProposal("test title", "test description", []ParamChange{pc1, pc2})
 
 	require.Equal(t, "test title", pcp.GetTitle())
@@ -17,15 +17,11 @@ func TestParameterChangeProposal(t *testing.T) {
 	require.Equal(t, ProposalTypeChange, pcp.ProposalType())
 	require.Nil(t, pcp.ValidateBasic())
 
-	pc3 := NewParamChangeWithSubkey("", "bar", "cat", "dog")
+	pc3 := NewParamChange("", "bar", "cat")
 	pcp = NewParameterChangeProposal("test title", "test description", []ParamChange{pc3})
 	require.Error(t, pcp.ValidateBasic())
 
-	pc4 := NewParamChangeWithSubkey("sub", "", "cat", "dog")
+	pc4 := NewParamChange("sub", "", "cat")
 	pcp = NewParameterChangeProposal("test title", "test description", []ParamChange{pc4})
 	require.Error(t, pcp.ValidateBasic())
-
-	pc5 := NewParamChangeWithSubkey("sub", "foo", "cat", "")
-	pcp = NewParameterChangeProposal("test title", "test description", []ParamChange{pc5})
-	require.Error(t, pcp.ValidateBasic())
 }

x/simulation/params.go

@@ -94,23 +94,21 @@ type SimValFn func(r *rand.Rand) string
 type ParamChange struct {
 	Subspace string
 	Key      string
-	Subkey   string
 	SimValue SimValFn
 }
 
 // NewSimParamChange creates a new ParamChange instance
-func NewSimParamChange(subspace, key, subkey string, simVal SimValFn) ParamChange {
+func NewSimParamChange(subspace, key string, simVal SimValFn) ParamChange {
 	return ParamChange{
 		Subspace: subspace,
 		Key:      key,
-		Subkey:   subkey,
 		SimValue: simVal,
 	}
 }
 
 // ComposedKey creates a new composed key for the param change proposal
 func (spc ParamChange) ComposedKey() string {
-	return fmt.Sprintf("%s/%s/%s", spc.Subspace, spc.Key, spc.Subkey)
+	return fmt.Sprintf("%s/%s", spc.Subspace, spc.Key)
 }
 
 //-----------------------------------------------------------------------------

x/slashing/handler_test.go

@@ -72,7 +72,6 @@ func TestJailedValidatorDelegations(t *testing.T) {
 	ctx, _, stakingKeeper, _, slashingKeeper := slashingkeeper.CreateTestInput(t, DefaultParams())
 
 	stakingParams := stakingKeeper.GetParams(ctx)
-	stakingParams.UnbondingTime = 0
 	stakingKeeper.SetParams(ctx, stakingParams)
 
 	// create a validator

x/slashing/internal/types/params.go

@@ -75,11 +75,11 @@ func (p Params) String() string {
 // ParamSetPairs - Implements params.ParamSet
 func (p *Params) ParamSetPairs() params.ParamSetPairs {
 	return params.ParamSetPairs{
-		params.NewParamSetPair(KeySignedBlocksWindow, &p.SignedBlocksWindow),
+		params.NewParamSetPair(KeySignedBlocksWindow, &p.SignedBlocksWindow, validateSignedBlocksWindow),
-		params.NewParamSetPair(KeyMinSignedPerWindow, &p.MinSignedPerWindow),
+		params.NewParamSetPair(KeyMinSignedPerWindow, &p.MinSignedPerWindow, validateMinSignedPerWindow),
-		params.NewParamSetPair(KeyDowntimeJailDuration, &p.DowntimeJailDuration),
+		params.NewParamSetPair(KeyDowntimeJailDuration, &p.DowntimeJailDuration, validateDowntimeJailDuration),
-		params.NewParamSetPair(KeySlashFractionDoubleSign, &p.SlashFractionDoubleSign),
+		params.NewParamSetPair(KeySlashFractionDoubleSign, &p.SlashFractionDoubleSign, validateSlashFractionDoubleSign),
-		params.NewParamSetPair(KeySlashFractionDowntime, &p.SlashFractionDowntime),
+		params.NewParamSetPair(KeySlashFractionDowntime, &p.SlashFractionDowntime, validateSlashFractionDowntime),
 	}
 }
 
@@ -90,3 +90,77 @@ func DefaultParams() Params {
 		DefaultSlashFractionDoubleSign, DefaultSlashFractionDowntime,
 	)
 }
+
+func validateSignedBlocksWindow(i interface{}) error {
+	v, ok := i.(int64)
+	if !ok {
+		return fmt.Errorf("invalid parameter type: %T", i)
+	}
+
+	if v <= 0 {
+		return fmt.Errorf("signed blocks window must be positive: %d", v)
+	}
+
+	return nil
+}
+
+func validateMinSignedPerWindow(i interface{}) error {
+	v, ok := i.(sdk.Dec)
+	if !ok {
+		return fmt.Errorf("invalid parameter type: %T", i)
+	}
+
+	if v.IsNegative() {
+		return fmt.Errorf("min signed per window cannot be negative: %s", v)
+	}
+	if v.GT(sdk.OneDec()) {
+		return fmt.Errorf("min signed per window too large: %s", v)
+	}
+
+	return nil
+}
+
+func validateDowntimeJailDuration(i interface{}) error {
+	v, ok := i.(time.Duration)
+	if !ok {
+		return fmt.Errorf("invalid parameter type: %T", i)
+	}
+
+	if v <= 0 {
+		return fmt.Errorf("downtime jail duration must be positive: %s", v)
+	}
+
+	return nil
+}
+
+func validateSlashFractionDoubleSign(i interface{}) error {
+	v, ok := i.(sdk.Dec)
+	if !ok {
+		return fmt.Errorf("invalid parameter type: %T", i)
+	}
+
+	if v.IsNegative() {
+		return fmt.Errorf("double sign slash fraction cannot be negative: %s", v)
+	}
+	if v.GT(sdk.OneDec()) {
+		return fmt.Errorf("double sign slash fraction too large: %s", v)
+	}
+
+	return nil
+}
+
+func validateSlashFractionDowntime(i interface{}) error {
+	v, ok := i.(sdk.Dec)
+	if !ok {
+		return fmt.Errorf("invalid parameter type: %T", i)
+	}
+
+	if v.IsNegative() {
+		return fmt.Errorf("downtime slash fraction cannot be negative: %s", v)
+	}
+	if v.GT(sdk.OneDec()) {
+		return fmt.Errorf("downtime slash fraction too large: %s", v)
+	}
+
+	return nil
+}

x/slashing/simulation/params.go

@@ -20,17 +20,17 @@ const (
 // on the simulation
 func ParamChanges(r *rand.Rand) []simulation.ParamChange {
 	return []simulation.ParamChange{
-		simulation.NewSimParamChange(types.ModuleName, keySignedBlocksWindow, "",
+		simulation.NewSimParamChange(types.ModuleName, keySignedBlocksWindow,
 			func(r *rand.Rand) string {
 				return fmt.Sprintf("\"%d\"", GenSignedBlocksWindow(r))
 			},
 		),
-		simulation.NewSimParamChange(types.ModuleName, keyMinSignedPerWindow, "",
+		simulation.NewSimParamChange(types.ModuleName, keyMinSignedPerWindow,
 			func(r *rand.Rand) string {
 				return fmt.Sprintf("\"%s\"", GenMinSignedPerWindow(r))
 			},
 		),
-		simulation.NewSimParamChange(types.ModuleName, keySlashFractionDowntime, "",
+		simulation.NewSimParamChange(types.ModuleName, keySlashFractionDowntime,
 			func(r *rand.Rand) string {
 				return fmt.Sprintf("\"%s\"", GenSlashFractionDowntime(r))
 			},

x/staking/handler_test.go

@@ -17,25 +17,12 @@ import (
 	"github.com/cosmos/cosmos-sdk/x/staking/types"
 )
 
-//______________________________________________________________________
-
-// retrieve params which are instant
-func setInstantUnbondPeriod(keeper keep.Keeper, ctx sdk.Context) types.Params {
-	params := keeper.GetParams(ctx)
-	params.UnbondingTime = 0
-	keeper.SetParams(ctx, params)
-	return params
-}
-
-//______________________________________________________________________
-
 func TestValidatorByPowerIndex(t *testing.T) {
 	validatorAddr, validatorAddr3 := sdk.ValAddress(keep.Addrs[0]), sdk.ValAddress(keep.Addrs[1])
 
 	initPower := int64(1000000)
 	initBond := sdk.TokensFromConsensusPower(initPower)
 	ctx, _, keeper, _ := keep.CreateTestInput(t, false, initPower)
-	_ = setInstantUnbondPeriod(keeper, ctx)
 
 	// create validator
 	msgCreateValidator := NewTestMsgCreateValidator(validatorAddr, keep.PKs[0], initBond)
@@ -186,7 +173,6 @@ func TestInvalidPubKeyTypeMsgCreateValidator(t *testing.T) {
 
 func TestLegacyValidatorDelegations(t *testing.T) {
 	ctx, _, keeper, _ := keep.CreateTestInput(t, false, int64(1000))
-	setInstantUnbondPeriod(keeper, ctx)
 
 	bondAmount := sdk.TokensFromConsensusPower(10)
 	valAddr := sdk.ValAddress(keep.Addrs[0])
@@ -350,7 +336,6 @@ func TestEditValidatorDecreaseMinSelfDelegation(t *testing.T) {
 	initPower := int64(100)
 	initBond := sdk.TokensFromConsensusPower(100)
 	ctx, _, keeper, _ := keep.CreateTestInput(t, false, initPower)
-	_ = setInstantUnbondPeriod(keeper, ctx)
 
 	// create validator
 	msgCreateValidator := NewTestMsgCreateValidator(validatorAddr, keep.PKs[0], initBond)
@@ -382,7 +367,6 @@ func TestEditValidatorIncreaseMinSelfDelegationBeyondCurrentBond(t *testing.T) {
 	initPower := int64(100)
 	initBond := sdk.TokensFromConsensusPower(100)
 	ctx, _, keeper, _ := keep.CreateTestInput(t, false, initPower)
-	_ = setInstantUnbondPeriod(keeper, ctx)
 
 	// create validator
 	msgCreateValidator := NewTestMsgCreateValidator(validatorAddr, keep.PKs[0], initBond)
@@ -412,7 +396,8 @@ func TestIncrementsMsgUnbond(t *testing.T) {
 	initPower := int64(1000)
 	initBond := sdk.TokensFromConsensusPower(initPower)
 	ctx, accMapper, keeper, _ := keep.CreateTestInput(t, false, initPower)
-	params := setInstantUnbondPeriod(keeper, ctx)
+
+	params := keeper.GetParams(ctx)
 	denom := params.BondDenom
 
 	// create validator, delegate
@@ -510,7 +495,10 @@ func TestMultipleMsgCreateValidator(t *testing.T) {
 	initPower := int64(1000)
 	initTokens := sdk.TokensFromConsensusPower(initPower)
 	ctx, accMapper, keeper, _ := keep.CreateTestInput(t, false, initPower)
-	params := setInstantUnbondPeriod(keeper, ctx)
+
+	params := keeper.GetParams(ctx)
+	blockTime := time.Now().UTC()
+	ctx = ctx.WithBlockTime(blockTime)
 
 	validatorAddrs := []sdk.ValAddress{
 		sdk.ValAddress(keep.Addrs[0]),
@@ -544,6 +532,8 @@ func TestMultipleMsgCreateValidator(t *testing.T) {
 		require.Equal(t, balanceExpd, balanceGot, "expected account to have %d, got %d", balanceExpd, balanceGot)
 	}
 
+	EndBlocker(ctx, keeper)
+
 	// unbond them all by removing delegation
 	for i, validatorAddr := range validatorAddrs {
 		_, found := keeper.GetValidator(ctx, validatorAddr)
@@ -552,16 +542,17 @@ func TestMultipleMsgCreateValidator(t *testing.T) {
 		unbondAmt := sdk.NewCoin(sdk.DefaultBondDenom, sdk.TokensFromConsensusPower(10))
 		msgUndelegate := NewMsgUndelegate(delegatorAddrs[i], validatorAddr, unbondAmt) // remove delegation
 		got := handleMsgUndelegate(ctx, msgUndelegate, keeper)
-
 		require.True(t, got.IsOK(), "expected msg %d to be ok, got %v", i, got)
+
 		var finishTime time.Time
-
-		// Jump to finishTime for unbonding period and remove from unbonding queue
 		types.ModuleCdc.MustUnmarshalBinaryLengthPrefixed(got.Data, &finishTime)
-		ctx = ctx.WithBlockTime(finishTime)
 
+		// adds validator into unbonding queue
 		EndBlocker(ctx, keeper)
 
+		// removes validator from queue and set
+		EndBlocker(ctx.WithBlockTime(blockTime.Add(params.UnbondingTime)), keeper)
+
 		// Check that the validator is deleted from state
 		validators := keeper.GetValidators(ctx, 100)
 		require.Equal(t, len(validatorAddrs)-(i+1), len(validators),
@@ -578,7 +569,6 @@ func TestMultipleMsgCreateValidator(t *testing.T) {
 func TestMultipleMsgDelegate(t *testing.T) {
 	ctx, _, keeper, _ := keep.CreateTestInput(t, false, 1000)
 	validatorAddr, delegatorAddrs := sdk.ValAddress(keep.Addrs[0]), keep.Addrs[1:]
-	_ = setInstantUnbondPeriod(keeper, ctx)
 
 	// first make a validator
 	msgCreateValidator := NewTestMsgCreateValidator(validatorAddr, keep.PKs[0], sdk.NewInt(10))
@@ -620,7 +610,6 @@ func TestMultipleMsgDelegate(t *testing.T) {
 func TestJailValidator(t *testing.T) {
 	ctx, _, keeper, _ := keep.CreateTestInput(t, false, 1000)
 	validatorAddr, delegatorAddr := sdk.ValAddress(keep.Addrs[0]), keep.Addrs[1]
-	_ = setInstantUnbondPeriod(keeper, ctx)
 
 	// create the validator
 	msgCreateValidator := NewTestMsgCreateValidator(validatorAddr, keep.PKs[0], sdk.NewInt(10))
@@ -873,10 +862,8 @@ func TestTransitiveRedelegation(t *testing.T) {
 	validatorAddr2 := sdk.ValAddress(keep.Addrs[1])
 	validatorAddr3 := sdk.ValAddress(keep.Addrs[2])
 
-	// set the unbonding time
+	blockTime := time.Now().UTC()
-	params := keeper.GetParams(ctx)
+	ctx = ctx.WithBlockTime(blockTime)
-	params.UnbondingTime = 0
-	keeper.SetParams(ctx, params)
 
 	// create the validators
 	msgCreateValidator := NewTestMsgCreateValidator(validatorAddr, keep.PKs[0], sdk.NewInt(10))
@@ -902,12 +889,15 @@ func TestTransitiveRedelegation(t *testing.T) {
 	got = handleMsgBeginRedelegate(ctx, msgBeginRedelegate, keeper)
 	require.True(t, !got.IsOK(), "expected an error, msg: %v", msgBeginRedelegate)
 
+	params := keeper.GetParams(ctx)
+	ctx = ctx.WithBlockTime(blockTime.Add(params.UnbondingTime))
+
 	// complete first redelegation
 	EndBlocker(ctx, keeper)
 
 	// now should be able to redelegate from the second validator to the third
 	got = handleMsgBeginRedelegate(ctx, msgBeginRedelegate, keeper)
-	require.True(t, got.IsOK(), "expected no error")
+	require.True(t, got.IsOK(), "expected no error", got.Log)
 }
 
 func TestMultipleRedelegationAtSameTime(t *testing.T) {
@@ -1125,7 +1115,6 @@ func TestUnbondingWhenExcessValidators(t *testing.T) {
 
 	// set the unbonding time
 	params := keeper.GetParams(ctx)
-	params.UnbondingTime = 0
 	params.MaxValidators = 2
 	keeper.SetParams(ctx, params)
 

x/staking/simulation/params.go

@@ -19,12 +19,12 @@ const (
 // on the simulation
 func ParamChanges(r *rand.Rand) []simulation.ParamChange {
 	return []simulation.ParamChange{
-		simulation.NewSimParamChange(types.ModuleName, keyMaxValidators, "",
+		simulation.NewSimParamChange(types.ModuleName, keyMaxValidators,
 			func(r *rand.Rand) string {
 				return fmt.Sprintf("%d", GenMaxValidators(r))
 			},
 		),
-		simulation.NewSimParamChange(types.ModuleName, keyUnbondingTime, "",
+		simulation.NewSimParamChange(types.ModuleName, keyUnbondingTime,
 			func(r *rand.Rand) string {
 				return fmt.Sprintf("\"%d\"", GenUnbondingTime(r))
 			},

x/staking/types/params.go

@@ -2,7 +2,9 @@ package types
 
 import (
 	"bytes"
+	"errors"
 	"fmt"
+	"strings"
 	"time"
 
 	"github.com/cosmos/cosmos-sdk/codec"
@@ -39,8 +41,7 @@ type Params struct {
 	UnbondingTime time.Duration `json:"unbonding_time" yaml:"unbonding_time"` // time duration of unbonding
 	MaxValidators uint16        `json:"max_validators" yaml:"max_validators"` // maximum number of validators (max uint16 = 65535)
 	MaxEntries    uint16        `json:"max_entries" yaml:"max_entries"`       // max entries for either unbonding delegation or redelegation (per pair/trio)
-	// note: we need to be a bit careful about potential overflow here, since this is user-determined
+	BondDenom     string        `json:"bond_denom" yaml:"bond_denom"`         // bondable coin denomination
-	BondDenom string `json:"bond_denom" yaml:"bond_denom"` // bondable coin denomination
 }
 
 // NewParams creates a new Params instance
@@ -58,10 +59,10 @@ func NewParams(unbondingTime time.Duration, maxValidators, maxEntries uint16,
 // Implements params.ParamSet
 func (p *Params) ParamSetPairs() params.ParamSetPairs {
 	return params.ParamSetPairs{
-		{Key: KeyUnbondingTime, Value: &p.UnbondingTime},
+		params.NewParamSetPair(KeyUnbondingTime, &p.UnbondingTime, validateUnbondingTime),
-		{Key: KeyMaxValidators, Value: &p.MaxValidators},
+		params.NewParamSetPair(KeyMaxValidators, &p.MaxValidators, validateMaxValidators),
-		{Key: KeyMaxEntries, Value: &p.MaxEntries},
+		params.NewParamSetPair(KeyMaxEntries, &p.MaxEntries, validateMaxEntries),
-		{Key: KeyBondDenom, Value: &p.BondDenom},
+		params.NewParamSetPair(KeyBondDenom, &p.BondDenom, validateBondDenom),
 	}
 }
 
@@ -108,11 +109,73 @@ func UnmarshalParams(cdc *codec.Codec, value []byte) (params Params, err error)
 
 // validate a set of params
 func (p Params) Validate() error {
-	if p.BondDenom == "" {
+	if err := validateUnbondingTime(p.UnbondingTime); err != nil {
-		return fmt.Errorf("staking parameter BondDenom can't be an empty string")
+		return err
 	}
-	if p.MaxValidators == 0 {
+	if err := validateMaxValidators(p.MaxValidators); err != nil {
-		return fmt.Errorf("staking parameter MaxValidators must be a positive integer")
+		return err
 	}
+	if err := validateMaxEntries(p.MaxEntries); err != nil {
+		return err
+	}
+	if err := validateBondDenom(p.BondDenom); err != nil {
+		return err
+	}
+
+	return nil
+}
+
+func validateUnbondingTime(i interface{}) error {
+	v, ok := i.(time.Duration)
+	if !ok {
+		return fmt.Errorf("invalid parameter type: %T", i)
+	}
+
+	if v <= 0 {
+		return fmt.Errorf("unbonding time must be positive: %d", v)
+	}
+
+	return nil
+}
+
+func validateMaxValidators(i interface{}) error {
+	v, ok := i.(uint16)
+	if !ok {
+		return fmt.Errorf("invalid parameter type: %T", i)
+	}
+
+	if v == 0 {
+		return fmt.Errorf("max validators must be positive: %d", v)
+	}
+
+	return nil
+}
+
+func validateMaxEntries(i interface{}) error {
+	v, ok := i.(uint16)
+	if !ok {
+		return fmt.Errorf("invalid parameter type: %T", i)
+	}
+
+	if v == 0 {
+		return fmt.Errorf("max entries must be positive: %d", v)
+	}
+
+	return nil
+}
+
+func validateBondDenom(i interface{}) error {
+	v, ok := i.(string)
+	if !ok {
+		return fmt.Errorf("invalid parameter type: %T", i)
+	}
+
+	if strings.TrimSpace(v) == "" {
+		return errors.New("bond denom cannot be blank")
+	}
+	if err := sdk.ValidateDenom(v); err != nil {
+		return err
+	}
+
 	return nil
 }