From fb78f1d0af10b0559cdea831335a5ad4bb96a2f1 Mon Sep 17 00:00:00 2001 From: mossid Date: Sun, 14 Oct 2018 02:29:59 +0900 Subject: [PATCH 1/4] add docs, fix GetSubspace, address comments rm Subspace.ptr --- docs/spec/params/README.md | 20 +++++++ docs/spec/params/keeper.md | 17 ++++++ docs/spec/params/subspace.md | 76 +++++++++++++++++++++++++ store/prefixstore_test.go | 4 -- x/params/doc.go | 35 ++++++------ x/params/keeper_test.go | 103 +++++++++++++++++++++++++--------- x/params/subspace/subspace.go | 37 ++++++------ x/params/subspace/table.go | 22 ++++++-- 8 files changed, 244 insertions(+), 70 deletions(-) create mode 100644 docs/spec/params/README.md create mode 100644 docs/spec/params/keeper.md create mode 100644 docs/spec/params/subspace.md diff --git a/docs/spec/params/README.md b/docs/spec/params/README.md new file mode 100644 index 0000000000..d9c8b9221f --- /dev/null +++ b/docs/spec/params/README.md @@ -0,0 +1,20 @@ +# Params module specification + +## Abstract + +Package params provides a globally available 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. + +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, +who need to modify any parameter in case of the proposal passes. + +The following contents explains how to use params module for master and user modules. + +## Contents + +1. [Keeper](keeper.md) +1. [Subspace](subspace.md) diff --git a/docs/spec/params/keeper.md b/docs/spec/params/keeper.md new file mode 100644 index 0000000000..fe376911ed --- /dev/null +++ b/docs/spec/params/keeper.md @@ -0,0 +1,17 @@ +# Keeper + +In the app initialization stage, `Keeper.Subspace(Paramspace)` is passed to the user modules, and the subspaces are stored in `Keeper.spaces`. Later it can be retrieved with `Keeper.GetSubspace`, so the keepers holding `Keeper` can access to any subspace. For example, Gov module can take `Keeper` as its argument and modify parameter of any subspace when a `ParameterChangeProposal` is accepted. + +```go +type MasterKeeper struct { + pk params.Keeper +} + +func (k MasterKeeper) SetParam(ctx sdk.Context, space string, key string, param interface{}) { + space, ok := k.ps.GetSubspace(space) + if !ok { + return + } + space.Set(ctx, key, param) +} +``` diff --git a/docs/spec/params/subspace.md b/docs/spec/params/subspace.md new file mode 100644 index 0000000000..3781778982 --- /dev/null +++ b/docs/spec/params/subspace.md @@ -0,0 +1,76 @@ +# Subspace + +## 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. + +```go +const ( + DefaultParamspace = "mymodule" +) + +const ( + KeyParameter1 = "myparameter1" + KeyParameter2 = "myparameter2" +) + +type Keeper struct { + cdc *wire.Codec + key sdk.StoreKey + + ps params.Subspace +} +``` + +Pass a params.Subspace to NewKeeper with DefaultParamSubspace (or another) + +```go +app.myKeeper = mymodule.NewKeeper(cdc, key, app.paramStore.SubStore(mymodule.DefaultParamspace)) +``` + +`NewKeeper` should register a `TypeTable`, which defines a map from parameter keys from types. + +```go +func NewKeeper(cdc *codec.Codec, key sdk.StoreKey, space params.Subspace) Keeper { + return Keeper { + cdc: cdc, + key: key, + ps: space.WithTypeTable(ParamTypeTable()), + } +} +``` + +Now we can access to the paramstore using Paramstore Keys + +```go +var param MyStruct +k.ps.Get(KeyParameter1, ¶m) +k.ps.Set(KeyParameter2, param) +``` + +# Genesis Usage + +Declare a struct for parameters and make it implement params.ParamSet. It will then be able to be passed to SetParamSet. + +```go +type MyParams struct { + Parameter1 uint64 + Parameter2 string +} + +// 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}, + } +} + +func InitGenesis(ctx sdk.Context, k Keeper, data GenesisState) { + k.ps.SetParamSet(ctx, &data.params) +} +``` + +The method is pointer receiver because there could be a case that we read from the store and set the result to the struct. + diff --git a/store/prefixstore_test.go b/store/prefixstore_test.go index 8445991b6e..4e555292c3 100644 --- a/store/prefixstore_test.go +++ b/store/prefixstore_test.go @@ -119,10 +119,6 @@ func TestPrefixStoreIterate(t *testing.T) { } func incFirstByte(bz []byte) { - if bz[0] == byte(255) { - bz[0] = byte(0) - return - } bz[0]++ } diff --git a/x/params/doc.go b/x/params/doc.go index 06d6620b20..77b05273b3 100644 --- a/x/params/doc.go +++ b/x/params/doc.go @@ -3,18 +3,18 @@ package params /* Package params provides a globally available parameter store. -There are two main types, Keeper and Space. Space is an isolated namespace for a +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 and create new space. +permission to access all existing spaces. -Space can be used by the individual keepers, who needs a private parameter store -that the other keeper are not able to modify. Keeper can be used by the Governance -keeper, who need to modify any parameter in case of the proposal passes. +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, +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.Store in the keeper. Since we prefix the keys with the spacename, it is +params.Subspace in the keeper. Since we prefix the keys with the spacename, it is recommended to use the same name with the module's. const ( @@ -33,26 +33,29 @@ recommended to use the same name with the module's. ps params.Subspace } -Pass a params.Store to NewKeeper with DefaultParamSpace (or another) +Pass a params.Subspace to NewKeeper with DefaultParamSubspace (or another) app.myKeeper = mymodule.NewKeeper(app.paramStore.SubStore(mymodule.DefaultParamspace)) Now we can access to the paramstore using Paramstore Keys + var param MyStruct k.ps.Get(KeyParameter1, ¶m) k.ps.Set(KeyParameter2, param) Genesis Usage: -Declare a struct for parameters and make it implement ParamStruct. It will then -be able to be passed to SetFromParamStruct. +Declare a struct for parameters and make it implement params.ParamSet. It will then +be able to be passed to SetParamSet. type MyParams struct { Parameter1 uint64 Parameter2 string } - func (p *MyParams) KeyFieldPairs() params.KeyFieldPairs { + // 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}, @@ -60,26 +63,26 @@ be able to be passed to SetFromParamStruct. } func InitGenesis(ctx sdk.Context, k Keeper, data GenesisState) { - k.ps.SetFromParamStruct(ctx, &data.params) + k.ps.SetParamSet(ctx, &data.params) } The method is pointer receiver because there could be a case that we read from the store and set the result to the struct. -Master Permission Usage: +Master Keeper Usage: Keepers that require master permission to the paramstore, such as gov, can take -params.Keeper itself to access all substores(using GetSubstore) +params.Keeper itself to access all subspace(using GetSubspace) type MasterKeeper struct { - ps params.Store + pk params.Keeper } func (k MasterKeeper) SetParam(ctx sdk.Context, space string, key string, param interface{}) { - store, ok := k.ps.GetSubstore(space) + space, ok := k.ps.GetSubspace(space) if !ok { return } - store.Set(ctx, key, param) + space.Set(ctx, key, param) } */ diff --git a/x/params/keeper_test.go b/x/params/keeper_test.go index 640661a245..30584421f5 100644 --- a/x/params/keeper_test.go +++ b/x/params/keeper_test.go @@ -65,45 +65,82 @@ func TestKeeper(t *testing.T) { []byte("extra2"), string(""), ) + cdc := codec.New() skey := sdk.NewKVStoreKey("test") tkey := sdk.NewTransientStoreKey("transient_test") ctx := defaultContext(skey, tkey) - store := NewKeeper(codec.New(), skey, tkey).Subspace("test").WithTypeTable(table) + keeper := NewKeeper(cdc, skey, tkey) + space := keeper.Subspace("test").WithTypeTable(table) + store := ctx.KVStore(skey).Prefix([]byte("test/")) + // Set params for i, kv := range kvs { - require.NotPanics(t, func() { store.Set(ctx, []byte(kv.key), kv.param) }, "store.Set panics, tc #%d", i) + require.NotPanics(t, func() { space.Set(ctx, []byte(kv.key), kv.param) }, "space.Set panics, tc #%d", i) } + // Test space.Get for i, kv := range kvs { var param int64 - require.NotPanics(t, func() { store.Get(ctx, []byte(kv.key), ¶m) }, "store.Get panics, tc #%d", i) - require.Equal(t, kv.param, param, "stored param not equal, tc #%d", i) + require.NotPanics(t, func() { space.Get(ctx, []byte(kv.key), ¶m) }, "space.Get panics, tc #%d", i) + require.Equal(t, kv.param, param, "param not equal, tc #%d", i) } - cdc := codec.New() + // Test space.GetRaw for i, kv := range kvs { var param int64 - bz := store.GetRaw(ctx, []byte(kv.key)) + bz := space.GetRaw(ctx, []byte(kv.key)) err := cdc.UnmarshalJSON(bz, ¶m) require.Nil(t, err, "err is not nil, tc #%d", i) - require.Equal(t, kv.param, param, "stored param not equal, tc #%d", i) + require.Equal(t, kv.param, param, "param not equal, tc #%d", i) } + // Test store.Get equals space.Get + for i, kv := range kvs { + var param int64 + bz := store.Get([]byte(kv.key)) + require.NotNil(t, bz, "KVStore.Get returns nil, tc #%d", i) + err := cdc.UnmarshalJSON(bz, ¶m) + require.NoError(t, err, "UnmarshalJSON returns error, tc #%d", i) + require.Equal(t, kv.param, param, "param not equal, tc #%d", i) + } + + // Test invalid space.Get for i, kv := range kvs { var param bool - require.Panics(t, func() { store.Get(ctx, []byte(kv.key), ¶m) }, "invalid store.Get not panics, tc #%d", i) + require.Panics(t, func() { space.Get(ctx, []byte(kv.key), ¶m) }, "invalid space.Get not panics, tc #%d", i) } + // Test invalid space.Set for i, kv := range kvs { - require.Panics(t, func() { store.Set(ctx, []byte(kv.key), true) }, "invalid store.Set not panics, tc #%d", i) + require.Panics(t, func() { space.Set(ctx, []byte(kv.key), true) }, "invalid space.Set not panics, tc #%d", i) + } + + // Test GetSubspace + for i, kv := range kvs { + var gparam, param int64 + gspace, ok := keeper.GetSubspace("test") + require.True(t, ok, "cannot retrieve subspace, tc #%d", i) + + require.NotPanics(t, func() { gspace.Get(ctx, []byte(kv.key), &gparam) }) + require.NotPanics(t, func() { space.Get(ctx, []byte(kv.key), ¶m) }) + require.Equal(t, gparam, param, "GetSubspace().Get not equal with space.Get, tc #%d", i) + + require.NotPanics(t, func() { gspace.Set(ctx, []byte(kv.key), int64(i)) }) + require.NotPanics(t, func() { space.Get(ctx, []byte(kv.key), ¶m) }) + require.Equal(t, int64(i), param, "GetSubspace().Set not equal with space.Get, tc #%d", i) } } -func TestGet(t *testing.T) { +func indirect(ptr interface{}) interface{} { + return reflect.ValueOf(ptr).Elem().Interface() +} + +func TestSubspace(t *testing.T) { + cdc := createTestCodec() key := sdk.NewKVStoreKey("test") tkey := sdk.NewTransientStoreKey("transient_test") ctx := defaultContext(key, tkey) - keeper := NewKeeper(createTestCodec(), key, tkey) + keeper := NewKeeper(cdc, key, tkey) kvs := []struct { key string @@ -140,29 +177,41 @@ func TestGet(t *testing.T) { []byte("struct"), s{}, ) - store := keeper.Subspace("test").WithTypeTable(table) + store := ctx.KVStore(key).Prefix([]byte("test/")) + space := keeper.Subspace("test").WithTypeTable(table) + // Test space.Set, space.Modified for i, kv := range kvs { - require.False(t, store.Modified(ctx, []byte(kv.key)), "store.Modified returns true before setting, tc #%d", i) - require.NotPanics(t, func() { store.Set(ctx, []byte(kv.key), kv.param) }, "store.Set panics, tc #%d", i) - require.True(t, store.Modified(ctx, []byte(kv.key)), "store.Modified returns false after setting, tc #%d", i) + require.False(t, space.Modified(ctx, []byte(kv.key)), "space.Modified returns true before setting, tc #%d", i) + require.NotPanics(t, func() { space.Set(ctx, []byte(kv.key), kv.param) }, "space.Set panics, tc #%d", i) + require.True(t, space.Modified(ctx, []byte(kv.key)), "space.Modified returns false after setting, tc #%d", i) } + // Test space.Get, space.GetIfExists for i, kv := range kvs { - require.NotPanics(t, func() { store.GetIfExists(ctx, []byte("invalid"), kv.ptr) }, "store.GetIfExists panics when no value exists, tc #%d", i) - require.Equal(t, kv.zero, reflect.ValueOf(kv.ptr).Elem().Interface(), "store.GetIfExists unmarshalls when no value exists, tc #%d", i) - require.Panics(t, func() { store.Get(ctx, []byte("invalid"), kv.ptr) }, "invalid store.Get not panics when no value exists, tc #%d", i) - require.Equal(t, kv.zero, reflect.ValueOf(kv.ptr).Elem().Interface(), "invalid store.Get unmarshalls when no value exists, tc #%d", i) + require.NotPanics(t, func() { space.GetIfExists(ctx, []byte("invalid"), kv.ptr) }, "space.GetIfExists panics when no value exists, tc #%d", i) + require.Equal(t, kv.zero, indirect(kv.ptr), "space.GetIfExists unmarshalls when no value exists, tc #%d", i) + require.Panics(t, func() { space.Get(ctx, []byte("invalid"), kv.ptr) }, "invalid space.Get not panics when no value exists, tc #%d", i) + require.Equal(t, kv.zero, indirect(kv.ptr), "invalid space.Get unmarshalls when no value exists, tc #%d", i) - require.NotPanics(t, func() { store.GetIfExists(ctx, []byte(kv.key), kv.ptr) }, "store.GetIfExists panics, tc #%d", i) - require.Equal(t, kv.param, reflect.ValueOf(kv.ptr).Elem().Interface(), "stored param not equal, tc #%d", i) - require.NotPanics(t, func() { store.Get(ctx, []byte(kv.key), kv.ptr) }, "store.Get panics, tc #%d", i) - require.Equal(t, kv.param, reflect.ValueOf(kv.ptr).Elem().Interface(), "stored param not equal, tc #%d", i) + require.NotPanics(t, func() { space.GetIfExists(ctx, []byte(kv.key), kv.ptr) }, "space.GetIfExists panics, tc #%d", i) + require.Equal(t, kv.param, indirect(kv.ptr), "spaced param not equal, tc #%d", i) + require.NotPanics(t, func() { space.Get(ctx, []byte(kv.key), kv.ptr) }, "space.Get panics, tc #%d", i) + require.Equal(t, kv.param, indirect(kv.ptr), "spaced param not equal, tc #%d", i) - require.Panics(t, func() { store.Get(ctx, []byte("invalid"), kv.ptr) }, "invalid store.Get not panics when no value exists, tc #%d", i) - require.Equal(t, kv.param, reflect.ValueOf(kv.ptr).Elem().Interface(), "invalid store.Get unmarshalls when no value existt, tc #%d", i) + require.Panics(t, func() { space.Get(ctx, []byte("invalid"), kv.ptr) }, "invalid space.Get not panics when no value exists, tc #%d", i) + require.Equal(t, kv.param, indirect(kv.ptr), "invalid space.Get unmarshalls when no value existt, tc #%d", i) - require.Panics(t, func() { store.Get(ctx, []byte(kv.key), nil) }, "invalid store.Get not panics when the pointer is nil, tc #%d", i) - require.Panics(t, func() { store.Get(ctx, []byte(kv.key), new(invalid)) }, "invalid store.Get not panics when the pointer is different type, tc #%d", i) + require.Panics(t, func() { space.Get(ctx, []byte(kv.key), nil) }, "invalid space.Get not panics when the pointer is nil, tc #%d", i) + require.Panics(t, func() { space.Get(ctx, []byte(kv.key), new(invalid)) }, "invalid space.Get not panics when the pointer is different type, tc #%d", i) + } + + // Test store.Get equals space.Get + for i, kv := range kvs { + bz := store.Get([]byte(kv.key)) + require.NotNil(t, bz, "store.Get() returns nil, tc #%d", i) + err := cdc.UnmarshalJSON(bz, kv.ptr) + require.NoError(t, err, "cdc.UnmarshalJSON() returns error, tc #%d", i) + require.Equal(t, kv.param, indirect(kv.ptr), "param not equal, tc #%d", i) } } diff --git a/x/params/subspace/subspace.go b/x/params/subspace/subspace.go index 7c9ca5fdaa..fe5889a908 100644 --- a/x/params/subspace/subspace.go +++ b/x/params/subspace/subspace.go @@ -7,10 +7,6 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" ) -// Additional capicity to be allocated for Subspace.name -// So we don't have to allocate extra space each time appending to the key -const extraKeyCap = 20 - // Individual parameter store for each keeper // Transient store persists for a block, so we use it for // recording whether the parameter has been changed or not @@ -30,32 +26,35 @@ func NewSubspace(cdc *codec.Codec, key sdk.StoreKey, tkey sdk.StoreKey, name str cdc: cdc, key: key, tkey: tkey, + name: []byte(name), + table: TypeTable{ + m: make(map[string]reflect.Type), + }, } - namebz := []byte(name) - res.name = make([]byte, len(namebz), len(namebz)+extraKeyCap) - copy(res.name, namebz) return } // WithTypeTable initializes TypeTable and returns modified Subspace -func (s Subspace) WithTypeTable(table TypeTable) (res Subspace) { - if table == nil { +func (s Subspace) WithTypeTable(table TypeTable) Subspace { + if table.m == nil { panic("SetTypeTable() called with nil TypeTable") } - if s.table != nil { - panic("SetTypeTable() called on initialized Subspace") + if len(s.table.m) != 0 { + panic("SetTypeTable() called on already initialized Subspace") } - res = Subspace{ - cdc: s.cdc, - key: s.key, - tkey: s.tkey, - name: s.name, - table: table, + for k, v := range table.m { + s.table.m[k] = v } - return + // Allocate additional capicity 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()) + copy(s.name, name) + + return s } // Returns a KVStore identical with ctx.KVStore(s.key).Prefix() @@ -118,7 +117,7 @@ func (s Subspace) Modified(ctx sdk.Context, key []byte) bool { func (s Subspace) Set(ctx sdk.Context, key []byte, param interface{}) { store := s.kvStore(ctx) - ty, ok := s.table[string(key)] + ty, ok := s.table.m[string(key)] if !ok { panic("Parameter not registered") } diff --git a/x/params/subspace/table.go b/x/params/subspace/table.go index 363d0243d2..628ebef477 100644 --- a/x/params/subspace/table.go +++ b/x/params/subspace/table.go @@ -5,7 +5,9 @@ import ( ) // TypeTable subspaces appropriate type for each parameter key -type TypeTable map[string]reflect.Type +type TypeTable struct { + m map[string]reflect.Type +} // Constructs new table func NewTypeTable(keytypes ...interface{}) (res TypeTable) { @@ -13,7 +15,9 @@ func NewTypeTable(keytypes ...interface{}) (res TypeTable) { panic("odd number arguments in NewTypeTypeTable") } - res = make(map[string]reflect.Type) + res = TypeTable{ + m: make(map[string]reflect.Type), + } for i := 0; i < len(keytypes); i += 2 { res = res.RegisterType(keytypes[i].([]byte), keytypes[i+1]) @@ -25,7 +29,7 @@ func NewTypeTable(keytypes ...interface{}) (res TypeTable) { // Register single key-type pair func (t TypeTable) RegisterType(key []byte, ty interface{}) TypeTable { keystr := string(key) - if _, ok := t[keystr]; ok { + if _, ok := t.m[keystr]; ok { panic("duplicate parameter key") } @@ -36,7 +40,7 @@ func (t TypeTable) RegisterType(key []byte, ty interface{}) TypeTable { rty = rty.Elem() } - t[keystr] = rty + t.m[keystr] = rty return t } @@ -48,3 +52,13 @@ func (t TypeTable) RegisterParamSet(ps ParamSet) TypeTable { } return t } + +func (t TypeTable) maxKeyLength() (res int) { + for k := range t.m { + l := len(k) + if l > res { + res = l + } + } + return +} From 73d83b02cb678c51db3e282d92f32a41ebcdf1c5 Mon Sep 17 00:00:00 2001 From: mossid Date: Sun, 14 Oct 2018 02:50:32 +0900 Subject: [PATCH 2/4] typo --- x/params/doc.go | 4 ++-- x/params/keeper_test.go | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/x/params/doc.go b/x/params/doc.go index 77b05273b3..bee45e79ec 100644 --- a/x/params/doc.go +++ b/x/params/doc.go @@ -33,7 +33,7 @@ recommended to use the same name with the module's. ps params.Subspace } -Pass a params.Subspace to NewKeeper with DefaultParamSubspace (or another) +Pass a params.Subspace to NewKeeper with DefaultParamspace (or another) app.myKeeper = mymodule.NewKeeper(app.paramStore.SubStore(mymodule.DefaultParamspace)) @@ -79,7 +79,7 @@ params.Keeper itself to access all subspace(using GetSubspace) } func (k MasterKeeper) SetParam(ctx sdk.Context, space string, key string, param interface{}) { - space, ok := k.ps.GetSubspace(space) + space, ok := k.pk.GetSubspace(space) if !ok { return } diff --git a/x/params/keeper_test.go b/x/params/keeper_test.go index 30584421f5..585db3c410 100644 --- a/x/params/keeper_test.go +++ b/x/params/keeper_test.go @@ -82,7 +82,7 @@ func TestKeeper(t *testing.T) { for i, kv := range kvs { var param int64 require.NotPanics(t, func() { space.Get(ctx, []byte(kv.key), ¶m) }, "space.Get panics, tc #%d", i) - require.Equal(t, kv.param, param, "param not equal, tc #%d", i) + require.Equal(t, kv.param, param, "stored param not equal, tc #%d", i) } // Test space.GetRaw @@ -91,7 +91,7 @@ func TestKeeper(t *testing.T) { bz := space.GetRaw(ctx, []byte(kv.key)) err := cdc.UnmarshalJSON(bz, ¶m) require.Nil(t, err, "err is not nil, tc #%d", i) - require.Equal(t, kv.param, param, "param not equal, tc #%d", i) + require.Equal(t, kv.param, param, "stored param not equal, tc #%d", i) } // Test store.Get equals space.Get @@ -101,7 +101,7 @@ func TestKeeper(t *testing.T) { require.NotNil(t, bz, "KVStore.Get returns nil, tc #%d", i) err := cdc.UnmarshalJSON(bz, ¶m) require.NoError(t, err, "UnmarshalJSON returns error, tc #%d", i) - require.Equal(t, kv.param, param, "param not equal, tc #%d", i) + require.Equal(t, kv.param, param, "stored param not equal, tc #%d", i) } // Test invalid space.Get @@ -195,9 +195,9 @@ func TestSubspace(t *testing.T) { require.Equal(t, kv.zero, indirect(kv.ptr), "invalid space.Get unmarshalls when no value exists, tc #%d", i) require.NotPanics(t, func() { space.GetIfExists(ctx, []byte(kv.key), kv.ptr) }, "space.GetIfExists panics, tc #%d", i) - require.Equal(t, kv.param, indirect(kv.ptr), "spaced param not equal, tc #%d", i) + require.Equal(t, kv.param, indirect(kv.ptr), "stored param not equal, tc #%d", i) require.NotPanics(t, func() { space.Get(ctx, []byte(kv.key), kv.ptr) }, "space.Get panics, tc #%d", i) - require.Equal(t, kv.param, indirect(kv.ptr), "spaced param not equal, tc #%d", i) + require.Equal(t, kv.param, indirect(kv.ptr), "stored param not equal, tc #%d", i) require.Panics(t, func() { space.Get(ctx, []byte("invalid"), kv.ptr) }, "invalid space.Get not panics when no value exists, tc #%d", i) require.Equal(t, kv.param, indirect(kv.ptr), "invalid space.Get unmarshalls when no value existt, tc #%d", i) @@ -212,6 +212,6 @@ func TestSubspace(t *testing.T) { require.NotNil(t, bz, "store.Get() returns nil, tc #%d", i) err := cdc.UnmarshalJSON(bz, kv.ptr) require.NoError(t, err, "cdc.UnmarshalJSON() returns error, tc #%d", i) - require.Equal(t, kv.param, indirect(kv.ptr), "param not equal, tc #%d", i) + require.Equal(t, kv.param, indirect(kv.ptr), "stored param not equal, tc #%d", i) } } From 8c978d24769d29a369c0c8359febd8b66e03c66b Mon Sep 17 00:00:00 2001 From: mossid Date: Tue, 16 Oct 2018 03:02:48 +0900 Subject: [PATCH 3/4] add tests and restriction for TypeTable --- x/params/subspace/table.go | 17 ++++++++++++++++ x/params/subspace/table_test.go | 35 +++++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+) create mode 100644 x/params/subspace/table_test.go diff --git a/x/params/subspace/table.go b/x/params/subspace/table.go index 628ebef477..19e41da090 100644 --- a/x/params/subspace/table.go +++ b/x/params/subspace/table.go @@ -26,8 +26,25 @@ func NewTypeTable(keytypes ...interface{}) (res TypeTable) { return } +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 + } + } + return true +} + // Register single key-type pair func (t TypeTable) RegisterType(key []byte, ty interface{}) TypeTable { + if len(key) == 0 { + panic("cannot register empty key") + } + if !isAlphaNumeric(key) { + panic("non alphanumeric parameter key") + } keystr := string(key) if _, ok := t.m[keystr]; ok { panic("duplicate parameter key") diff --git a/x/params/subspace/table_test.go b/x/params/subspace/table_test.go new file mode 100644 index 0000000000..8f9142e1ec --- /dev/null +++ b/x/params/subspace/table_test.go @@ -0,0 +1,35 @@ +package subspace + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +type testparams struct { + i int64 + b bool +} + +func (tp *testparams) KeyValuePairs() KeyValuePairs { + return KeyValuePairs{ + {[]byte("i"), &tp.i}, + {[]byte("b"), &tp.b}, + } +} + +func TestTypeTable(t *testing.T) { + table := NewTypeTable() + + 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{}) }) +} From c903f39d1f548d820b240b8177ca51dc48a75f6c Mon Sep 17 00:00:00 2001 From: mossid Date: Tue, 16 Oct 2018 04:11:31 +0900 Subject: [PATCH 4/4] update PENDING --- PENDING.md | 1 + 1 file changed, 1 insertion(+) diff --git a/PENDING.md b/PENDING.md index 0b4e63ff37..416dc00d7d 100644 --- a/PENDING.md +++ b/PENDING.md @@ -69,6 +69,7 @@ BREAKING CHANGES * [x/staking] \#2244 staking now holds a consensus-address-index instead of a consensus-pubkey-index * [x/staking] \#2236 more distribution hooks for distribution * [x/stake] \#2394 Split up UpdateValidator into distinct state transitions applied only in EndBlock + * [x/stake] Global Paramstore refactored * Tendermint * Update tendermint version from v0.23.0 to v0.25.0, notable changes