From eb05526e1ff7eb436fb9780887515dc3eefcbe9b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Federico=20Kunze=20K=C3=BCllmer?= <31522760+fedekunze@users.noreply.github.com> Date: Mon, 18 Jul 2022 16:51:01 +0200 Subject: [PATCH 1/3] imp(params): Add GetParamSetIfExists to prevent panic on breaking param changes --- x/params/types/common_test.go | 32 +++++++++++++++++++++++++++++--- x/params/types/subspace.go | 9 +++++++++ x/params/types/subspace_test.go | 22 ++++++++++++++++++++++ 3 files changed, 60 insertions(+), 3 deletions(-) diff --git a/x/params/types/common_test.go b/x/params/types/common_test.go index c7cb067c62c4..1228aeb8c015 100644 --- a/x/params/types/common_test.go +++ b/x/params/types/common_test.go @@ -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") @@ -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 { @@ -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}, @@ -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(¶ms{}) } diff --git a/x/params/types/subspace.go b/x/params/types/subspace.go index e528a185eb9e..f90914f3a62a 100644 --- a/x/params/types/subspace.go +++ b/x/params/types/subspace.go @@ -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) { diff --git a/x/params/types/subspace_test.go b/x/params/types/subspace_test.go index 63874489dc6e..e14d32a694ef 100644 --- a/x/params/types/subspace_test.go +++ b/x/params/types/subspace_test.go @@ -202,6 +202,28 @@ 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) +} + func (suite *SubspaceTestSuite) TestSetParamSet() { testCases := []struct { name string From a7fb476e129f9747275c14e32851e883fbc5556f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Federico=20Kunze=20K=C3=BCllmer?= <31522760+fedekunze@users.noreply.github.com> Date: Mon, 18 Jul 2022 16:54:49 +0200 Subject: [PATCH 2/3] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 45e38b93aa18..decc67c6e5f6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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. * [#12596](https://github.com/cosmos/cosmos-sdk/pull/12596) Remove all imports of the non-existent gogo/protobuf v1.3.3 to ease downstream use and go workspaces. * [#12589](https://github.com/cosmos/cosmos-sdk/pull/12589) Allow zero gas in simulation mode. * [#12576](https://github.com/cosmos/cosmos-sdk/pull/12576) Remove dependency on cosmos/keyring and upgrade to 99designs/keyring v1.2.1 From f7c78b2035fdb42f2435f2f0c01c5907de53bbf6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Federico=20Kunze=20K=C3=BCllmer?= <31522760+fedekunze@users.noreply.github.com> Date: Mon, 18 Jul 2022 16:56:57 +0200 Subject: [PATCH 3/3] test --- x/params/types/subspace_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/x/params/types/subspace_test.go b/x/params/types/subspace_test.go index e14d32a694ef..1c4ac855c688 100644 --- a/x/params/types/subspace_test.go +++ b/x/params/types/subspace_test.go @@ -222,6 +222,7 @@ func (suite *SubspaceTestSuite) TestGetParamSetIfExists() { 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() {