diff --git a/types/coin.go b/types/coin.go index c64ba0dca500..94b9dc901c77 100644 --- a/types/coin.go +++ b/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) } diff --git a/types/dec_coin.go b/types/dec_coin.go index ca50468383c0..0c11d3135341 100644 --- a/types/dec_coin.go +++ b/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) } diff --git a/types/denom.go b/types/denom.go index 73d396644523..f79bef8b9459 100644 --- a/types/denom.go +++ b/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 } diff --git a/x/auth/ante/ante_test.go b/x/auth/ante/ante_test.go index b6803c174189..5ac42a6dc855 100644 --- a/x/auth/ante/ante_test.go +++ b/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)}, - {"tx sig limit check", types.NewParams(types.DefaultMaxMemoCharacters, 0, types.DefaultTxSizeCostPerByte, types.DefaultSigVerifyCostED25519, types.DefaultSigVerifyCostSecp256k1)}, + {"memo size check", types.NewParams(1, types.DefaultTxSigLimit, 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)}, } diff --git a/x/auth/simulation/genesis.go b/x/auth/simulation/genesis.go index 61b248472312..808d81ac296f 100644 --- a/x/auth/simulation/genesis.go +++ b/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, diff --git a/x/auth/simulation/params.go b/x/auth/simulation/params.go index 045124a4a4a3..afca0348d3a5 100644 --- a/x/auth/simulation/params.go +++ b/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)) }, diff --git a/x/auth/types/params.go b/x/auth/types/params.go index 288ed334341e..2f994ef7eecf 100644 --- a/x/auth/types/params.go +++ b/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}, - {KeyTxSigLimit, &p.TxSigLimit}, - {KeyTxSizeCostPerByte, &p.TxSizeCostPerByte}, - {KeySigVerifyCostED25519, &p.SigVerifyCostED25519}, - {KeySigVerifyCostSecp256k1, &p.SigVerifyCostSecp256k1}, + params.NewParamSetPair(KeyMaxMemoCharacters, &p.MaxMemoCharacters, validateMaxMemoCharacters), + params.NewParamSetPair(KeyTxSigLimit, &p.TxSigLimit, validateTxSigLimit), + params.NewParamSetPair(KeyTxSizeCostPerByte, &p.TxSizeCostPerByte, validateTxSizeCostPerByte), + params.NewParamSetPair(KeySigVerifyCostED25519, &p.SigVerifyCostED25519, validateSigVerifyCostED25519), + params.NewParamSetPair(KeySigVerifyCostSecp256k1, &p.SigVerifyCostSecp256k1, validateSigVerifyCostSecp256k1), } } @@ -101,22 +102,88 @@ func (p Params) String() string { return sb.String() } +func validateTxSigLimit(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 signature limit: %d", v) + } + + 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 p.TxSigLimit == 0 { - return fmt.Errorf("invalid tx signature limit: %d", p.TxSigLimit) + if err := validateTxSigLimit(p.TxSigLimit); err != nil { + return err } - if p.SigVerifyCostED25519 == 0 { - return fmt.Errorf("invalid ED25519 signature verification cost: %d", p.SigVerifyCostED25519) + if err := validateSigVerifyCostED25519(p.SigVerifyCostED25519); err != nil { + return err } - if p.SigVerifyCostSecp256k1 == 0 { - return fmt.Errorf("invalid SECK256k1 signature verification cost: %d", p.SigVerifyCostSecp256k1) + if err := validateSigVerifyCostSecp256k1(p.SigVerifyCostSecp256k1); err != nil { + return err } - if p.MaxMemoCharacters == 0 { - return fmt.Errorf("invalid max memo characters: %d", p.MaxMemoCharacters) + if err := validateSigVerifyCostSecp256k1(p.MaxMemoCharacters); err != nil { + return err } - if p.TxSizeCostPerByte == 0 { - return fmt.Errorf("invalid tx size cost per byte: %d", p.TxSizeCostPerByte) + if err := validateTxSizeCostPerByte(p.TxSizeCostPerByte); err != nil { + return err } + return nil } diff --git a/x/bank/internal/types/params.go b/x/bank/internal/types/params.go index d232a975f0cb..cdd6aff84f81 100644 --- a/x/bank/internal/types/params.go +++ b/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 +} diff --git a/x/bank/simulation/params.go b/x/bank/simulation/params.go index f0dfec04b0b2..f5aa25bf6a0c 100644 --- a/x/bank/simulation/params.go +++ b/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)) }, diff --git a/x/crisis/internal/types/params.go b/x/crisis/internal/types/params.go index a0dbdf726783..5749c9192e34 100644 --- a/x/crisis/internal/types/params.go +++ b/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 +} diff --git a/x/distribution/keeper/params.go b/x/distribution/keeper/params.go index b95a6efb27c9..764dfdc21731 100644 --- a/x/distribution/keeper/params.go +++ b/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{}, - ParamStoreKeyBaseProposerReward, sdk.Dec{}, - ParamStoreKeyBonusProposerReward, sdk.Dec{}, - ParamStoreKeyWithdrawAddrEnabled, false, + params.NewParamSetPair(ParamStoreKeyCommunityTax, sdk.Dec{}, validateCommunityTax), + params.NewParamSetPair(ParamStoreKeyBaseProposerReward, sdk.Dec{}, validateBaseProposerReward), + params.NewParamSetPair(ParamStoreKeyBonusProposerReward, sdk.Dec{}, validateBonusProposerReward), + 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 { diff --git a/x/distribution/simulation/params.go b/x/distribution/simulation/params.go index 710243e6d033..60c591a6dc3b 100644 --- a/x/distribution/simulation/params.go +++ b/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)) }, diff --git a/x/evidence/internal/types/params.go b/x/evidence/internal/types/params.go index 5684786b73f7..6865eb2825ab 100644 --- a/x/evidence/internal/types/params.go +++ b/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 +} diff --git a/x/gov/simulation/params.go b/x/gov/simulation/params.go index c43f3952e15f..420b5a8eec78 100644 --- a/x/gov/simulation/params.go +++ b/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 diff --git a/x/gov/types/params.go b/x/gov/types/params.go index a7fe1da7b5b3..ff671d493169 100644 --- a/x/gov/types/params.go +++ b/x/gov/types/params.go @@ -31,9 +31,9 @@ var ( // ParamKeyTable - Key declaration for parameters func ParamKeyTable() params.KeyTable { return params.NewKeyTable( - ParamStoreKeyDepositParams, DepositParams{}, - ParamStoreKeyVotingParams, VotingParams{}, - ParamStoreKeyTallyParams, TallyParams{}, + params.NewParamSetPair(ParamStoreKeyDepositParams, DepositParams{}, validateDepositParams), + params.NewParamSetPair(ParamStoreKeyVotingParams, VotingParams{}, validateVotingParams), + 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"` diff --git a/x/mint/alias.go b/x/mint/alias.go index 4d31e826bcf0..9572d077b458 100644 --- a/x/mint/alias.go +++ b/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 diff --git a/x/mint/internal/types/genesis.go b/x/mint/internal/types/genesis.go index 43ee15f1e0d6..acb3a8efe80f 100644 --- a/x/mint/internal/types/genesis.go +++ b/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 != nil { + if err := data.Params.Validate(); err != nil { return err } diff --git a/x/mint/internal/types/params.go b/x/mint/internal/types/params.go index 122f88bfef01..082c9ff22599 100644 --- a/x/mint/internal/types/params.go +++ b/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, - inflationMin, goalBonded sdk.Dec, blocksPerYear uint64) Params { +func NewParams( + 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 { - if params.GoalBonded.IsNegative() { - return fmt.Errorf("mint parameter GoalBonded should be positive, is %s ", params.GoalBonded.String()) +func (p Params) Validate() error { + if err := validateMintDenom(p.MintDenom); err != nil { + return err } - if params.GoalBonded.GT(sdk.OneDec()) { - return fmt.Errorf("mint parameter GoalBonded must be <= 1, is %s", params.GoalBonded.String()) + if err := validateInflationRateChange(p.InflationRateChange); err != nil { + return err } - if params.InflationMax.LT(params.InflationMin) { - return fmt.Errorf("mint parameter Max inflation must be greater than or equal to min inflation") + if err := validateInflationMax(p.InflationMax); err != nil { + return err } - if params.MintDenom == "" { - return fmt.Errorf("mint parameter MintDenom can't be an empty string") + if err := validateInflationMin(p.InflationMin); err != nil { + 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}, - {Key: KeyInflationRateChange, Value: &p.InflationRateChange}, - {Key: KeyInflationMax, Value: &p.InflationMax}, - {Key: KeyInflationMin, Value: &p.InflationMin}, - {Key: KeyGoalBonded, Value: &p.GoalBonded}, - {Key: KeyBlocksPerYear, Value: &p.BlocksPerYear}, + params.NewParamSetPair(KeyMintDenom, &p.MintDenom, validateMintDenom), + params.NewParamSetPair(KeyInflationRateChange, &p.InflationRateChange, validateInflationRateChange), + params.NewParamSetPair(KeyInflationMax, &p.InflationMax, validateInflationMax), + params.NewParamSetPair(KeyInflationMin, &p.InflationMin, validateInflationMin), + params.NewParamSetPair(KeyGoalBonded, &p.GoalBonded, validateGoalBonded), + 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 } diff --git a/x/mint/simulation/params.go b/x/mint/simulation/params.go index 506b3bf45dc5..4f73183f058a 100644 --- a/x/mint/simulation/params.go +++ b/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)) }, diff --git a/x/params/alias.go b/x/params/alias.go index 47b89296de93..ace601c219dc 100644 --- a/x/params/alias.go +++ b/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 diff --git a/x/params/client/utils/utils.go b/x/params/client/utils/utils.go index 6bcbeb43a520..748d98502dc3 100644 --- a/x/params/client/utils/utils.go +++ b/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 { - return ParamChangeJSON{subspace, key, subkey, value} +func NewParamChangeJSON(subspace, key string, value json.RawMessage) ParamChangeJSON { + 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 diff --git a/x/params/client/utils/utils_test.go b/x/params/client/utils/utils_test.go index 1953d11ffe74..c19857339b42 100644 --- a/x/params/client/utils/utils_test.go +++ b/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(`{}`)) - pcj2 := NewParamChangeJSON("subspace", "key2", "", json.RawMessage(`{}`)) + pcj1 := NewParamChangeJSON("subspace", "key1", 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)) } diff --git a/x/params/doc.go b/x/params/doc.go index 1591507b465c..68ab4dee0e1f 100644 --- a/x/params/doc.go +++ b/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 -paramstore, where keys are prefixed by preconfigured spacename. Keeper has a -permission to access all existing spaces. +There are two core components, Keeper and Subspace. Subspace is an isolated +namespace for a parameter store, where keys are prefixed by pre-configured +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 -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. +1. Declare constant module parameter keys and the globally unique Subspace name: 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 { - cdc *codec.Codec - key sdk.StoreKey - - ps params.Subspace - } +2. Create a concrete parameter struct and define the validation functions: - func ParamKeyTable() params.KeyTable { - return params.NewKeyTable( - KeyParameter1, MyStruct{}, - KeyParameter2, MyStruct{}, - ) + type MyParams struct { + MyParam1 int64 `json:"my_param1" yaml:"my_param1"` + MyParam2 bool `json:"my_param2" yaml:"my_param2"` } - func NewKeeper(cdc *codec.Codec, key sdk.StoreKey, ps params.Subspace) Keeper { - return Keeper { - cdc: cdc, - key: key, - ps: ps.WithKeyTable(ParamKeyTable()), + func validateMyParam1(i interface{}) error { + _, ok := i.(int64) + if !ok { + return fmt.Errorf("invalid parameter type: %T", i) } - } - -Pass a params.Subspace to NewKeeper with DefaultParamspace (or another) - app.myKeeper = mymodule.NewKeeper(app.paramStore.SubStore(mymodule.DefaultParamspace)) + // validate (if necessary)... -Now we can access to the paramstore using Paramstore Keys + return nil + } - var param MyStruct - k.ps.Get(ctx, KeyParameter1, ¶m) - k.ps.Set(ctx, KeyParameter2, param) + func validateMyParam2(i interface{}) error { + _, ok := i.(bool) + if !ok { + return fmt.Errorf("invalid parameter type: %T", i) + } -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. + // validate (if necessary)... - func ParamKeyTable() params.KeyTable { - return params.NewKeyTable( - KeyParamMain, MyStruct{}, - ) + return nil } +3. Implement the params.ParamSet interface: - func (k Keeper) GetDynamicParameter(ctx sdk.Context, subkey []byte) (res MyStruct) { - k.ps.GetWithSubkey(ctx, KeyParamMain, subkey, &res) + func (p *MyParams) ParamSetPairs() params.ParamSetPairs { + return params.ParamSetPairs{ + {KeyParameter1, &p.MyParam1, validateMyParam1}, + {KeyParameter2, &p.MyParam2, validateMyParam2}, + } } -Genesis Usage: + func paramKeyTable() params.KeyTable { + return params.NewKeyTable().RegisterParamSet(&MyParams{}) + } -Declare a struct for parameters and make it implement params.ParamSet. It will then -be able to be passed to SetParamSet. +4. Have the module accept a Subspace in the constructor and set the KeyTable (if necessary): - type MyParams struct { - Parameter1 uint64 - Parameter2 string - } + func NewKeeper(..., paramSpace params.Subspace, ...) Keeper { + // set KeyTable if it has not already been set + if !paramSpace.HasKeyTable() { + paramSpace = paramSpace.WithKeyTable(paramKeyTable()) + } - // Implements params.ParamSet - // KeyValuePairs must return the list of (ParamKey, PointerToTheField) - func (p *MyParams) KeyValuePairs() params.KeyValuePairs { - return params.KeyFieldPairs { - {KeyParameter1, &p.Parameter1}, - {KeyParameter2, &p.Parameter2}, + return Keeper { + // ... + paramSpace: paramSpace, } } - func InitGenesis(ctx sdk.Context, k Keeper, data GenesisState) { - k.ps.SetParamSet(ctx, &data.params) - } +Now we have access to the module's parameters that are namespaced using the keys defined: -The method is pointer receiver because there could be a case that we read from -the store and set the result to the struct. + func InitGenesis(ctx sdk.Context, k Keeper, gs GenesisState) { + // ... + k.SetParams(ctx, gs.Params) + } -Master Keeper Usage: + func (k Keeper) SetParams(ctx sdk.Context, params Params) { + k.paramSpace.SetParamSet(ctx, ¶ms) + } -Keepers that require master permission to the paramstore, such as gov, can take -params.Keeper itself to access all subspace(using GetSubspace) + func (k Keeper) GetParams(ctx sdk.Context) (params Params) { + k.paramSpace.GetParamSet(ctx, ¶ms) + return params + } - type MasterKeeper struct { - pk params.Keeper + func (k Keeper) MyParam1(ctx sdk.Context) (res int64) { + k.paramSpace.Get(ctx, KeyParameter1, &res) + return res } - func (k MasterKeeper) SetParam(ctx sdk.Context, space string, key string, param interface{}) { - space, ok := k.pk.GetSubspace(space) - if !ok { - return - } - space.Set(ctx, key, param) + 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 diff --git a/x/params/keeper.go b/x/params/keeper.go index d5fc3ea539bb..9e91259df656 100644 --- a/x/params/keeper.go +++ b/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) { - k = Keeper{ +func NewKeeper(cdc *codec.Codec, key, tkey sdk.StoreKey, codespace sdk.CodespaceType) Keeper { + return Keeper{ cdc: cdc, key: key, tkey: tkey, codespace: codespace, spaces: make(map[string]*Subspace), } - - return k } // Logger returns a module-specific logger. diff --git a/x/params/keeper_test.go b/x/params/keeper_test.go index 7b94204a6207..3f2f1a4b2bf4 100644 --- a/x/params/keeper_test.go +++ b/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), - []byte("key2"), int64(0), - []byte("key3"), int64(0), - []byte("key4"), int64(0), - []byte("key5"), int64(0), - []byte("key6"), int64(0), - []byte("key7"), int64(0), - []byte("extra1"), bool(false), - []byte("extra2"), string(""), + NewParamSetPair([]byte("key1"), int64(0), validateNoOp), + NewParamSetPair([]byte("key2"), int64(0), validateNoOp), + NewParamSetPair([]byte("key3"), int64(0), validateNoOp), + NewParamSetPair([]byte("key4"), int64(0), validateNoOp), + NewParamSetPair([]byte("key5"), int64(0), validateNoOp), + NewParamSetPair([]byte("key6"), int64(0), validateNoOp), + NewParamSetPair([]byte("key7"), int64(0), validateNoOp), + NewParamSetPair([]byte("extra1"), bool(false), validateNoOp), + NewParamSetPair([]byte("extra2"), string(""), validateNoOp), ) cdc, ctx, skey, _, keeper := testComponents() @@ -135,18 +137,18 @@ func TestSubspace(t *testing.T) { } table := NewKeyTable( - []byte("string"), string(""), - []byte("bool"), bool(false), - []byte("int16"), int16(0), - []byte("int32"), int32(0), - []byte("int64"), int64(0), - []byte("uint16"), uint16(0), - []byte("uint32"), uint32(0), - []byte("uint64"), uint64(0), - []byte("int"), sdk.Int{}, - []byte("uint"), sdk.Uint{}, - []byte("dec"), sdk.Dec{}, - []byte("struct"), s{}, + NewParamSetPair([]byte("string"), string(""), validateNoOp), + NewParamSetPair([]byte("bool"), bool(false), validateNoOp), + NewParamSetPair([]byte("int16"), int16(0), validateNoOp), + NewParamSetPair([]byte("int32"), int32(0), validateNoOp), + NewParamSetPair([]byte("int64"), int64(0), validateNoOp), + NewParamSetPair([]byte("uint16"), uint16(0), validateNoOp), + NewParamSetPair([]byte("uint32"), uint32(0), validateNoOp), + NewParamSetPair([]byte("uint64"), uint64(0), validateNoOp), + NewParamSetPair([]byte("int"), sdk.Int{}, validateNoOp), + NewParamSetPair([]byte("uint"), sdk.Uint{}, validateNoOp), + NewParamSetPair([]byte("dec"), sdk.Dec{}, validateNoOp), + 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 diff --git a/x/params/proposal_handler.go b/x/params/proposal_handler.go index 97efd3e91226..e2e976454275 100644 --- a/x/params/proposal_handler.go +++ b/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 - if len(c.Subkey) == 0 { - 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)) - } else { - 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)) - } + k.Logger(ctx).Info( + fmt.Sprintf("attempt to set new parameter value; key: %s, value: %s", c.Key, c.Value), + ) - if err != nil { - return ErrSettingParameter(k.codespace, c.Key, c.Subkey, c.Value, err.Error()) + if err := ss.Update(ctx, []byte(c.Key), []byte(c.Value)); err != nil { + return ErrSettingParameter(k.codespace, c.Key, c.Value, err.Error()) } } diff --git a/x/params/proposal_handler_test.go b/x/params/proposal_handler_test.go index 934211356d14..841f832160c5 100644 --- a/x/params/proposal_handler_test.go +++ b/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}, - {Key: []byte(keySlashingRate), Value: &tp.SlashingRate}, + params.NewParamSetPair([]byte(keyMaxValidators), &tp.MaxValidators, validateNoOp), + params.NewParamSetPair([]byte(keySlashingRate), &tp.SlashingRate, validateNoOp), } } diff --git a/x/params/simulation/operations.go b/x/params/simulation/operations.go index ab514e06fec0..c9415a68e1df 100644 --- a/x/params/simulation/operations.go +++ b/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( diff --git a/x/params/subspace/common_test.go b/x/params/subspace/common_test.go new file mode 100644 index 000000000000..69c41478dbcc --- /dev/null +++ b/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(¶ms{}) +} diff --git a/x/params/subspace/doc.go b/x/params/subspace/doc.go index 1ab87ac01958..0bde2ebe62d4 100644 --- a/x/params/subspace/doc.go +++ b/x/params/subspace/doc.go @@ -1,14 +1,13 @@ -package subspace - /* -To prevent namespace collision between consumer modules, we define type -"space". A Space can only be generated by the keeper, and the keeper checks -the existence of the space having the same name before generating the -space. +To prevent namespace collision between consumer modules, we define a type +Subspace. A Subspace can only be generated by the keeper, and the keeper checks +the existence of the Subspace having the same name before generating the +Subspace. -Consumer modules must take a space (via Keeper.Subspace), not the keeper -itself. This isolates each modules from the others and make them modify the -parameters safely. Keeper can be treated as master permission for all -subspaces (via Keeper.GetSubspace), so should be passed to proper modules -(ex. gov) +Consumer modules must take a Subspace (via Keeper.Subspace), not the keeper +itself. This isolates each modules from the others and make them modify their +respective parameters safely. Keeper can be treated as master permission for all +Subspaces (via Keeper.GetSubspace), so should be passed to proper modules +(ex. x/governance). */ +package subspace diff --git a/x/params/subspace/paramset.go b/x/params/subspace/paramset.go index 8f2206217b8e..195944d8c9c4 100644 --- a/x/params/subspace/paramset.go +++ b/x/params/subspace/paramset.go @@ -1,14 +1,20 @@ package subspace -// ParamSetPair is used for associating paramsubspace key and field of param structs -type ParamSetPair struct { - Key []byte - Value interface{} -} +type ( + ValueValidatorFn func(value interface{}) error + + // ParamSetPair is used for associating paramsubspace key and field of param + // structs. + ParamSetPair struct { + Key []byte + Value interface{} + ValidatorFn ValueValidatorFn + } +) -// NewParamSetPair creates a new ParamSetPair instance -func NewParamSetPair(key []byte, value interface{}) ParamSetPair { - return ParamSetPair{key, value} +// NewParamSetPair creates a new ParamSetPair instance. +func NewParamSetPair(key []byte, value interface{}, vfn ValueValidatorFn) ParamSetPair { + return ParamSetPair{key, value, vfn} } // ParamSetPairs Slice of KeyFieldPair diff --git a/x/params/subspace/subspace.go b/x/params/subspace/subspace.go index aee65842035b..94533f9451f1 100644 --- a/x/params/subspace/subspace.go +++ b/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 - key sdk.StoreKey // []byte -> []byte, stores parameter - tkey sdk.StoreKey // []byte -> bool, stores parameter change - - name []byte - + cdc *codec.Codec + key sdk.StoreKey // []byte -> []byte, stores parameter + tkey sdk.StoreKey // []byte -> bool, stores parameter change + 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) { - res = Subspace{ - cdc: cdc, - key: key, - tkey: tkey, - name: []byte(name), - table: KeyTable{ - m: make(map[string]attribute), - }, +func NewSubspace(cdc *codec.Codec, key sdk.StoreKey, tkey sdk.StoreKey, name string) Subspace { + return Subspace{ + cdc: cdc, + key: key, + tkey: tkey, + name: []byte(name), + table: NewKeyTable(), } - - 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) { - res = make([]byte, len(key)+1+len(subkey)) - copy(res, key) - res[len(key)] = '/' - copy(res[len(key)+1:], subkey) - return +// Validate attempts to validate a parameter value by its key. If the key is not +// registered or if the validation of the value fails, an error is returned. +func (s Subspace) Validate(ctx sdk.Context, key []byte, value interface{}) error { + attr, ok := s.table.m[string(key)] + if !ok { + 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. -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 +// GetRaw queries for the raw values bytes for a parameter by key. 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. -// It also set to the transient store to record change. -func (s Subspace) Set(ctx sdk.Context, key []byte, param interface{}) { +// Set stores a value for given a parameter key assuming the parameter type has +// been registered. It will panic if the parameter type has not been registered +// 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(param) + bz, err := s.cdc.MarshalJSON(value) 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 -// has a different type from the input. It also sets to the transient store to -// record change. -func (s Subspace) Update(ctx sdk.Context, key []byte, param []byte) error { +// Update stores an updated raw value for a given parameter key assuming the +// parameter type has been registered. It will panic if the parameter type has +// not been registered or if the value cannot be encoded. An error is returned +// 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 { - 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 -// Checks parameter type only over the key -func (s Subspace) SetWithSubkey(ctx sdk.Context, key []byte, subkey []byte, param interface{}) { - 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)) + if err := s.cdc.UnmarshalJSON(value, dest); err != nil { + return err } - ty := attr.ty - dest := reflect.New(ty).Interface() - s.GetWithSubkeyIfExists(ctx, key, subkey, dest) - err := s.cdc.UnmarshalJSON(param, dest) - if err != nil { + // 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.SetWithSubkey(ctx, key, subkey, dest) - tStore := s.transientStore(ctx) - tStore.Set(concatkey, []byte{}) - + s.Set(ctx, key, dest) return nil } -// Get to ParamSet +// GetParamSet iterates through each ParamSetPair where for each pair, it will +// retrieve the value and set it to the corresponding value pointer provided +// in the ParamSetPair by calling Subspace#Get. 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() } diff --git a/x/params/subspace/subspace_test.go b/x/params/subspace/subspace_test.go new file mode 100644 index 000000000000..31a5d7635126 --- /dev/null +++ b/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", ¶ms{time.Hour * 1, 100, "stake"}}, + {"invalid bond denom", ¶ms{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)) +} diff --git a/x/params/subspace/table.go b/x/params/subspace/table.go index 75269cc06ef8..92e72242b967 100644 --- a/x/params/subspace/table.go +++ b/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(keytypes ...interface{}) (res KeyTable) { - if len(keytypes)%2 != 0 { - panic("odd number arguments in NewTypeKeyTable") - } - - res = KeyTable{ +func NewKeyTable(pairs ...ParamSetPair) KeyTable { + keyTable := KeyTable{ m: make(map[string]attribute), } - for i := 0; i < len(keytypes); i += 2 { - res = res.RegisterType(keytypes[i].([]byte), keytypes[i+1]) + for _, psp := range pairs { + keyTable = keyTable.RegisterType(psp) } - return + return keyTable } -func isAlphaNumeric(key []byte) bool { - for _, b := range key { - if !((48 <= b && b <= 57) || // numeric - (65 <= b && b <= 90) || // upper case - (97 <= b && b <= 122)) { // lower case - return false - } +// RegisterType registers a single ParamSetPair (key-type pair) in a KeyTable. +func (t KeyTable) RegisterType(psp ParamSetPair) KeyTable { + if len(psp.Key) == 0 { + panic("cannot register ParamSetPair with an parameter empty key") } - return true -} - -// Register single key-type pair -func (t KeyTable) RegisterType(key []byte, ty interface{}) KeyTable { - if len(key) == 0 { - panic("cannot register empty key") + if !sdk.IsAlphaNumeric(string(psp.Key)) { + panic("cannot register ParamSetPair with a non-alphanumeric parameter key") } - if !isAlphaNumeric(key) { - panic("non alphanumeric parameter key") + if psp.ValidatorFn == nil { + panic("cannot register ParamSetPair without a value validation function") } - keystr := string(key) + + keystr := string(psp.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() { - t = t.RegisterType(kvp.Key, kvp.Value) + for _, psp := range ps.ParamSetPairs() { + t = t.RegisterType(psp) } return t } @@ -83,5 +75,6 @@ func (t KeyTable) maxKeyLength() (res int) { res = l } } + return } diff --git a/x/params/subspace/table_test.go b/x/params/subspace/table_test.go index f1485b6e2653..fc47481a3bea 100644 --- a/x/params/subspace/table_test.go +++ b/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() - - require.Panics(t, func() { table.RegisterType([]byte(""), nil) }) - require.Panics(t, func() { table.RegisterType([]byte("!@#$%"), nil) }) - require.Panics(t, func() { table.RegisterType([]byte("hello,"), nil) }) - require.Panics(t, func() { table.RegisterType([]byte("hello"), nil) }) - - require.NotPanics(t, func() { table.RegisterType([]byte("hello"), bool(false)) }) - require.NotPanics(t, func() { table.RegisterType([]byte("world"), int64(0)) }) - require.Panics(t, func() { table.RegisterType([]byte("hello"), bool(false)) }) - - require.NotPanics(t, func() { table.RegisterParamSet(&testparams{}) }) - require.Panics(t, func() { table.RegisterParamSet(&testparams{}) }) + table := subspace.NewKeyTable() + + require.Panics(t, func() { table.RegisterType(subspace.ParamSetPair{[]byte(""), nil, nil}) }) + require.Panics(t, func() { table.RegisterType(subspace.ParamSetPair{[]byte("!@#$%"), nil, nil}) }) + require.Panics(t, func() { table.RegisterType(subspace.ParamSetPair{[]byte("hello,"), nil, nil}) }) + require.Panics(t, func() { table.RegisterType(subspace.ParamSetPair{[]byte("hello"), nil, nil}) }) + + require.NotPanics(t, func() { + table.RegisterType(subspace.ParamSetPair{keyBondDenom, string("stake"), validateBondDenom}) + }) + 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(¶ms{}) + }) + + require.Panics(t, func() { table.RegisterParamSet(¶ms{}) }) + 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}, + ) + }) } diff --git a/x/params/subspace/test_common.go b/x/params/subspace/test_common.go deleted file mode 100644 index 2b1d817beaa9..000000000000 --- a/x/params/subspace/test_common.go +++ /dev/null @@ -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 -} diff --git a/x/params/types/errors.go b/x/params/types/errors.go index bbf8f5a2b4d6..12726b0322f5 100644 --- a/x/params/types/errors.go +++ b/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 { - return sdk.NewError(codespace, CodeSettingParameter, fmt.Sprintf("error setting parameter %s on %s (%s): %s", value, key, subkey, msg)) +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", value, key, msg)) } // ErrEmptyChanges returns an error for empty parameter changes. diff --git a/x/params/types/proposal.go b/x/params/types/proposal.go index 58d559615723..a4bcefab2df6 100644 --- a/x/params/types/proposal.go +++ b/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} -} - -func NewParamChangeWithSubkey(subspace, key, subkey, value string) ParamChange { - return ParamChange{subspace, key, subkey, value} + return ParamChange{subspace, key, 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 diff --git a/x/params/types/proposal_test.go b/x/params/types/proposal_test.go index e4865aad7ad8..aa891c82031d 100644 --- a/x/params/types/proposal_test.go +++ b/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()) } diff --git a/x/simulation/params.go b/x/simulation/params.go index d3e2af44f15c..2673822d8f43 100644 --- a/x/simulation/params.go +++ b/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) } //----------------------------------------------------------------------------- diff --git a/x/slashing/handler_test.go b/x/slashing/handler_test.go index 077ea009f3a9..e0d5228326da 100644 --- a/x/slashing/handler_test.go +++ b/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 diff --git a/x/slashing/internal/types/params.go b/x/slashing/internal/types/params.go index 4817ac5ee3ef..15dd6cca6238 100644 --- a/x/slashing/internal/types/params.go +++ b/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(KeyMinSignedPerWindow, &p.MinSignedPerWindow), - params.NewParamSetPair(KeyDowntimeJailDuration, &p.DowntimeJailDuration), - params.NewParamSetPair(KeySlashFractionDoubleSign, &p.SlashFractionDoubleSign), - params.NewParamSetPair(KeySlashFractionDowntime, &p.SlashFractionDowntime), + params.NewParamSetPair(KeySignedBlocksWindow, &p.SignedBlocksWindow, validateSignedBlocksWindow), + params.NewParamSetPair(KeyMinSignedPerWindow, &p.MinSignedPerWindow, validateMinSignedPerWindow), + params.NewParamSetPair(KeyDowntimeJailDuration, &p.DowntimeJailDuration, validateDowntimeJailDuration), + params.NewParamSetPair(KeySlashFractionDoubleSign, &p.SlashFractionDoubleSign, validateSlashFractionDoubleSign), + 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 +} diff --git a/x/slashing/simulation/params.go b/x/slashing/simulation/params.go index 447073912c1a..6679928d8fc4 100644 --- a/x/slashing/simulation/params.go +++ b/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)) }, diff --git a/x/staking/handler_test.go b/x/staking/handler_test.go index 6a822603a88e..0e999ba9b43b 100644 --- a/x/staking/handler_test.go +++ b/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 + var finishTime time.Time 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 - params := keeper.GetParams(ctx) - params.UnbondingTime = 0 - keeper.SetParams(ctx, params) + blockTime := time.Now().UTC() + ctx = ctx.WithBlockTime(blockTime) // 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) diff --git a/x/staking/simulation/params.go b/x/staking/simulation/params.go index f4c292cae674..97938fb91e2d 100644 --- a/x/staking/simulation/params.go +++ b/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)) }, diff --git a/x/staking/types/params.go b/x/staking/types/params.go index 8be8cff4f24b..00d7aed07fb7 100644 --- a/x/staking/types/params.go +++ b/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}, - {Key: KeyMaxValidators, Value: &p.MaxValidators}, - {Key: KeyMaxEntries, Value: &p.MaxEntries}, - {Key: KeyBondDenom, Value: &p.BondDenom}, + params.NewParamSetPair(KeyUnbondingTime, &p.UnbondingTime, validateUnbondingTime), + params.NewParamSetPair(KeyMaxValidators, &p.MaxValidators, validateMaxValidators), + params.NewParamSetPair(KeyMaxEntries, &p.MaxEntries, validateMaxEntries), + 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 == "" { - return fmt.Errorf("staking parameter BondDenom can't be an empty string") + if err := validateUnbondingTime(p.UnbondingTime); err != nil { + return err } - if p.MaxValidators == 0 { - return fmt.Errorf("staking parameter MaxValidators must be a positive integer") + if err := validateMaxValidators(p.MaxValidators); err != nil { + 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 }