From c0eec96941004c92310849463f5142dace7a60fe 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/4] 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 42afe6cc9b21..948d5f1c3227 100644 --- a/x/params/types/subspace.go +++ b/x/params/types/subspace.go @@ -223,6 +223,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 a347a5f5427e..c856f84d6c9e 100644 --- a/x/params/types/subspace_test.go +++ b/x/params/types/subspace_test.go @@ -162,6 +162,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 ec14bf97e7361a4b3c9e52ea9a40695d810ec131 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 2/4] 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 c856f84d6c9e..ddee81bc5bee 100644 --- a/x/params/types/subspace_test.go +++ b/x/params/types/subspace_test.go @@ -182,6 +182,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() { From 0d8d2fd825b94317e31d1155a4a181e74c7dbcd8 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 3/4] imp(params): Add GetParamSetIfExists to prevent panic on breaking param changes --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d34e76d6320e..81cbab660825 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,6 +43,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. * [#12668](https://github.com/cosmos/cosmos-sdk/pull/12668) Add `authz_msg_index` event attribute to message events emitted when executing via `MsgExec` through `x/authz`. * [#12697](https://github.com/cosmos/cosmos-sdk/pull/12697) Upgrade IAVL to v0.19.0 with fast index and error propagation. NOTE: first start will take a while to propagate into new model. From 0ebda845facc77a4ec6ff8008a836208a13f386c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Federico=20Kunze=20K=C3=BCllmer?= <31522760+fedekunze@users.noreply.github.com> Date: Tue, 26 Jul 2022 08:45:11 +0200 Subject: [PATCH 4/4] fix params --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 81cbab660825..5ebdf1237341 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,7 +43,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. +* (x/params) [#12724](https://github.com/cosmos/cosmos-sdk/pull/12724) Add `GetParamSetIfExists` function to params `Subspace` to prevent panics on breaking changes. * [#12668](https://github.com/cosmos/cosmos-sdk/pull/12668) Add `authz_msg_index` event attribute to message events emitted when executing via `MsgExec` through `x/authz`. * [#12697](https://github.com/cosmos/cosmos-sdk/pull/12697) Upgrade IAVL to v0.19.0 with fast index and error propagation. NOTE: first start will take a while to propagate into new model.