Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add GetParamSetIfExists to prevent panic on breaking param changes #12615

Merged
merged 9 commits into from
Aug 2, 2022
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ Ref: https://keepachangelog.com/en/1.0.0/

### Improvements

* (x/params) [#12615](https://github.com/cosmos/cosmos-sdk/pull/12615) Add `GetParamSetIfExists` function to params `Subspace` to prevent panics on breaking changes.
* [#12717](https://github.com/cosmos/cosmos-sdk/pull/12717) Use injected encoding params in simapp.
* (x/bank) [#12674](https://github.com/cosmos/cosmos-sdk/pull/12674) Add convenience function `CreatePrefixedAccountStoreKey()` to construct key to access account's balance for a given denom.
* [#12702](https://github.com/cosmos/cosmos-sdk/pull/12702) Linting and tidiness, fixed two minor security warnings.
Expand Down
32 changes: 29 additions & 3 deletions x/params/types/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,10 @@ import (
)

var (
keyUnbondingTime = []byte("UnbondingTime")
keyMaxValidators = []byte("MaxValidators")
keyBondDenom = []byte("BondDenom")
keyUnbondingTime = []byte("UnbondingTime")
keyMaxValidators = []byte("MaxValidators")
keyBondDenom = []byte("BondDenom")
keyMaxRedelegationEntries = []byte("MaxRedelegationEntries")

key = sdk.NewKVStoreKey("storekey")
tkey = sdk.NewTransientStoreKey("transientstorekey")
Expand All @@ -24,6 +25,13 @@ type params struct {
BondDenom string `json:"bond_denom" yaml:"bond_denom"`
}

type paramsV2 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"`
MaxRedelegationEntries uint32 `json:"max_redelegation_entries" yaml:"max_redelegation_entries"`
}

func validateUnbondingTime(i interface{}) error {
v, ok := i.(time.Duration)
if !ok {
Expand Down Expand Up @@ -59,6 +67,15 @@ func validateBondDenom(i interface{}) error {
return nil
}

func validateMaxRedelegationEntries(i interface{}) error {
_, ok := i.(uint32)
if !ok {
return fmt.Errorf("invalid parameter type: %T", i)
}

return nil
}

func (p *params) ParamSetPairs() types.ParamSetPairs {
return types.ParamSetPairs{
{keyUnbondingTime, &p.UnbondingTime, validateUnbondingTime},
Expand All @@ -67,6 +84,15 @@ func (p *params) ParamSetPairs() types.ParamSetPairs {
}
}

func (p *paramsV2) ParamSetPairs() types.ParamSetPairs {
return types.ParamSetPairs{
{keyUnbondingTime, &p.UnbondingTime, validateUnbondingTime},
{keyMaxValidators, &p.MaxValidators, validateMaxValidators},
{keyBondDenom, &p.BondDenom, validateBondDenom},
{keyMaxRedelegationEntries, &p.MaxRedelegationEntries, validateMaxRedelegationEntries},
}
}

func paramKeyTable() types.KeyTable {
return types.NewKeyTable().RegisterParamSet(&params{})
}
9 changes: 9 additions & 0 deletions x/params/types/subspace.go
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,15 @@ func (s Subspace) GetParamSet(ctx sdk.Context, ps ParamSet) {
}
}

// GetParamSetIfExists 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#GetIfExists.
func (s Subspace) GetParamSetIfExists(ctx sdk.Context, ps ParamSet) {
for _, pair := range ps.ParamSetPairs() {
s.GetIfExists(ctx, pair.Key, pair.Value)
}
}

// 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) {
Expand Down
23 changes: 23 additions & 0 deletions x/params/types/subspace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,29 @@ func (suite *SubspaceTestSuite) TestGetParamSet() {
suite.Require().Equal(a.BondDenom, b.BondDenom)
}

func (suite *SubspaceTestSuite) TestGetParamSetIfExists() {
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 := paramsV2{}
suite.Require().NotPanics(func() {
suite.ss.GetParamSetIfExists(suite.ctx, &b)
})
suite.Require().Equal(a.UnbondingTime, b.UnbondingTime)
suite.Require().Equal(a.MaxValidators, b.MaxValidators)
suite.Require().Equal(a.BondDenom, b.BondDenom)
suite.Require().Zero(b.MaxRedelegationEntries)
suite.Require().False(suite.ss.Has(suite.ctx, keyMaxRedelegationEntries), "key from the new param version should not yet exist")
}

func (suite *SubspaceTestSuite) TestSetParamSet() {
testCases := []struct {
name string
Expand Down