From cc75924182c8aeca434fcfb15cc9448f41d59c5e Mon Sep 17 00:00:00 2001 From: mossid Date: Wed, 19 Sep 2018 02:16:51 +0900 Subject: [PATCH] last rereview --- store/gaskvstore_test.go | 22 +++++++-------- x/gov/keeper.go | 24 ++++++++--------- x/params/store/store.go | 58 +++++++++++++++++++++++++--------------- x/stake/app_test.go | 2 +- x/stake/keeper/keeper.go | 12 ++++----- 5 files changed, 66 insertions(+), 52 deletions(-) diff --git a/store/gaskvstore_test.go b/store/gaskvstore_test.go index 9cdbd6495a..c664535a02 100644 --- a/store/gaskvstore_test.go +++ b/store/gaskvstore_test.go @@ -16,7 +16,7 @@ func newGasKVStore() KVStore { return NewGasKVStore(meter, sdk.DefaultKVGasConfig(), mem) } -func TestKVGasKVStoreBasic(t *testing.T) { +func TestGasKVStoreBasic(t *testing.T) { mem := dbStoreAdapter{dbm.NewMemDB()} meter := sdk.NewGasMeter(1000) st := NewGasKVStore(meter, sdk.DefaultKVGasConfig(), mem) @@ -28,7 +28,7 @@ func TestKVGasKVStoreBasic(t *testing.T) { require.Equal(t, meter.GasConsumed(), sdk.Gas(193)) } -func TestKVGasKVStoreIterator(t *testing.T) { +func TestGasKVStoreIterator(t *testing.T) { mem := dbStoreAdapter{dbm.NewMemDB()} meter := sdk.NewGasMeter(1000) st := NewGasKVStore(meter, sdk.DefaultKVGasConfig(), mem) @@ -52,14 +52,14 @@ func TestKVGasKVStoreIterator(t *testing.T) { require.Equal(t, meter.GasConsumed(), sdk.Gas(384)) } -func TestKVGasKVStoreOutOfGasSet(t *testing.T) { +func TestGasKVStoreOutOfGasSet(t *testing.T) { mem := dbStoreAdapter{dbm.NewMemDB()} meter := sdk.NewGasMeter(0) st := NewGasKVStore(meter, sdk.DefaultKVGasConfig(), mem) require.Panics(t, func() { st.Set(keyFmt(1), valFmt(1)) }, "Expected out-of-gas") } -func TestKVGasKVStoreOutOfGasIterator(t *testing.T) { +func TestGasKVStoreOutOfGasIterator(t *testing.T) { mem := dbStoreAdapter{dbm.NewMemDB()} meter := sdk.NewGasMeter(200) st := NewGasKVStore(meter, sdk.DefaultKVGasConfig(), mem) @@ -69,7 +69,7 @@ func TestKVGasKVStoreOutOfGasIterator(t *testing.T) { require.Panics(t, func() { iterator.Value() }, "Expected out-of-gas") } -func testKVGasKVStoreWrap(t *testing.T, store KVStore) { +func testGasKVStoreWrap(t *testing.T, store KVStore) { meter := sdk.NewGasMeter(10000) store = store.Gas(meter, sdk.GasConfig{HasCost: 10}) @@ -84,22 +84,22 @@ func testKVGasKVStoreWrap(t *testing.T, store KVStore) { require.Equal(t, int64(40), meter.GasConsumed()) } -func TestKVGasKVStoreWrap(t *testing.T) { +func TestGasKVStoreWrap(t *testing.T) { db := dbm.NewMemDB() tree, _ := newTree(t, db) iavl := newIAVLStore(tree, numRecent, storeEvery) - testKVGasKVStoreWrap(t, iavl) + testGasKVStoreWrap(t, iavl) st := NewCacheKVStore(iavl) - testKVGasKVStoreWrap(t, st) + testGasKVStoreWrap(t, st) pref := st.Prefix([]byte("prefix")) - testKVGasKVStoreWrap(t, pref) + testGasKVStoreWrap(t, pref) dsa := dbStoreAdapter{dbm.NewMemDB()} - testKVGasKVStoreWrap(t, dsa) + testGasKVStoreWrap(t, dsa) ts := newTransientStore() - testKVGasKVStoreWrap(t, ts) + testGasKVStoreWrap(t, ts) } diff --git a/x/gov/keeper.go b/x/gov/keeper.go index 0a31d5bf2c..01029bada8 100644 --- a/x/gov/keeper.go +++ b/x/gov/keeper.go @@ -42,8 +42,8 @@ type Keeper struct { // The codec codec for binary encoding/decoding. cdc *codec.Codec - // Reserved codestore - codestore sdk.CodespaceType + // Reserved codespace + codespace sdk.CodespaceType } // NewKeeper returns a governance keeper. It handles: @@ -51,7 +51,7 @@ type Keeper struct { // - depositing funds into proposals, and activating upon sufficient funds being deposited // - users voting on proposals, with weight proportional to stake in the system // - and tallying the result of the vote. -func NewKeeper(cdc *codec.Codec, key sdk.StoreKey, pk params.Keeper, ps params.Store, ck bank.Keeper, ds sdk.DelegationSet, codestore sdk.CodespaceType) Keeper { +func NewKeeper(cdc *codec.Codec, key sdk.StoreKey, pk params.Keeper, ps params.Store, ck bank.Keeper, ds sdk.DelegationSet, codespace sdk.CodespaceType) Keeper { return Keeper{ storeKey: key, pk: pk, @@ -60,7 +60,7 @@ func NewKeeper(cdc *codec.Codec, key sdk.StoreKey, pk params.Keeper, ps params.S ds: ds, vs: ds.GetValidatorSet(), cdc: cdc, - codestore: codestore, + codespace: codespace, } } @@ -164,7 +164,7 @@ func (keeper Keeper) setInitialProposalID(ctx sdk.Context, proposalID int64) sdk store := ctx.KVStore(keeper.storeKey) bz := store.Get(KeyNextProposalID) if bz != nil { - return ErrInvalidGenesis(keeper.codestore, "Initial ProposalID already set") + return ErrInvalidGenesis(keeper.codespace, "Initial ProposalID already set") } bz = keeper.cdc.MustMarshalBinary(proposalID) store.Set(KeyNextProposalID, bz) @@ -186,7 +186,7 @@ func (keeper Keeper) getNewProposalID(ctx sdk.Context) (proposalID int64, err sd store := ctx.KVStore(keeper.storeKey) bz := store.Get(KeyNextProposalID) if bz == nil { - return -1, ErrInvalidGenesis(keeper.codestore, "InitialProposalID never set") + return -1, ErrInvalidGenesis(keeper.codespace, "InitialProposalID never set") } keeper.cdc.MustUnmarshalBinary(bz, &proposalID) bz = keeper.cdc.MustMarshalBinary(proposalID + 1) @@ -199,7 +199,7 @@ func (keeper Keeper) peekCurrentProposalID(ctx sdk.Context) (proposalID int64, e store := ctx.KVStore(keeper.storeKey) bz := store.Get(KeyNextProposalID) if bz == nil { - return -1, ErrInvalidGenesis(keeper.codestore, "InitialProposalID never set") + return -1, ErrInvalidGenesis(keeper.codespace, "InitialProposalID never set") } keeper.cdc.MustUnmarshalBinary(bz, &proposalID) return proposalID, nil @@ -261,14 +261,14 @@ func (keeper Keeper) setTallyingProcedure(ctx sdk.Context, tallyingProcedure Tal func (keeper Keeper) AddVote(ctx sdk.Context, proposalID int64, voterAddr sdk.AccAddress, option VoteOption) sdk.Error { proposal := keeper.GetProposal(ctx, proposalID) if proposal == nil { - return ErrUnknownProposal(keeper.codestore, proposalID) + return ErrUnknownProposal(keeper.codespace, proposalID) } if proposal.GetStatus() != StatusVotingPeriod { - return ErrInactiveProposal(keeper.codestore, proposalID) + return ErrInactiveProposal(keeper.codespace, proposalID) } if !validVoteOption(option) { - return ErrInvalidVote(keeper.codestore, option) + return ErrInvalidVote(keeper.codespace, option) } vote := Vote{ @@ -337,12 +337,12 @@ func (keeper Keeper) AddDeposit(ctx sdk.Context, proposalID int64, depositerAddr // Checks to see if proposal exists proposal := keeper.GetProposal(ctx, proposalID) if proposal == nil { - return ErrUnknownProposal(keeper.codestore, proposalID), false + return ErrUnknownProposal(keeper.codespace, proposalID), false } // Check if proposal is still depositable if (proposal.GetStatus() != StatusDepositPeriod) && (proposal.GetStatus() != StatusVotingPeriod) { - return ErrAlreadyFinishedProposal(keeper.codestore, proposalID), false + return ErrAlreadyFinishedProposal(keeper.codespace, proposalID), false } // Subtract coins from depositer's account diff --git a/x/params/store/store.go b/x/params/store/store.go index a595729a3c..366fd8fc18 100644 --- a/x/params/store/store.go +++ b/x/params/store/store.go @@ -4,8 +4,6 @@ import ( "fmt" "reflect" - tmlibs "github.com/tendermint/tendermint/libs/common" - "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" ) @@ -16,27 +14,33 @@ type Store struct { key sdk.StoreKey tkey sdk.StoreKey - store []byte + space []byte } // NewStore constructs a store with namestore -func NewStore(cdc *codec.Codec, key sdk.StoreKey, tkey sdk.StoreKey, store string) Store { - if !tmlibs.IsASCIIText(store) { - panic("paramstore store expressions can only contain alphanumeric characters") - } - +func NewStore(cdc *codec.Codec, key sdk.StoreKey, tkey sdk.StoreKey, space string) Store { return Store{ cdc: cdc, key: key, tkey: tkey, - store: append([]byte(store), '/'), + space: []byte(space), } } +// Returns a KVStore identical with ctx,TransientStore(s.key).Prefix() +func (s Store) kvStore(ctx sdk.Context) sdk.KVStore { + return ctx.KVStore(s.key).Prefix(append([]byte(s.space), '/')) +} + +// Returns a KVStore identical with ctx.TransientStore(s.tkey).Prefix() +func (s Store) transientStore(ctx sdk.Context) sdk.KVStore { + return ctx.TransientStore(s.tkey).Prefix(append([]byte(s.space), '/')) +} + // Get parameter from store func (s Store) Get(ctx sdk.Context, key string, ptr interface{}) { - store := ctx.KVStore(s.key).Prefix(s.store) + store := s.kvStore(ctx) bz := store.Get([]byte(key)) err := s.cdc.UnmarshalJSON(bz, ptr) if err != nil { @@ -46,7 +50,7 @@ func (s Store) Get(ctx sdk.Context, key string, ptr interface{}) { // GetIfExists do not modify ptr if the stored parameter is nil func (s Store) GetIfExists(ctx sdk.Context, key string, ptr interface{}) { - store := ctx.KVStore(s.key).Prefix(s.store) + store := s.kvStore(ctx) bz := store.Get([]byte(key)) if bz == nil { return @@ -59,26 +63,26 @@ func (s Store) GetIfExists(ctx sdk.Context, key string, ptr interface{}) { // Get raw bytes of parameter from store func (s Store) GetRaw(ctx sdk.Context, key string) []byte { - store := ctx.KVStore(s.key).Prefix(s.store) + store := s.kvStore(ctx) res := store.Get([]byte(key)) return res } // Check if the parameter is set in the store func (s Store) Has(ctx sdk.Context, key string) bool { - store := ctx.KVStore(s.key).Prefix(s.store) + store := s.kvStore(ctx) return store.Has([]byte(key)) } // Returns true if the parameter is set in the block func (s Store) Modified(ctx sdk.Context, key string) bool { - tstore := ctx.KVStore(s.tkey).Prefix(s.store) + tstore := s.transientStore(ctx) return tstore.Has([]byte(key)) } // Set parameter, return error if stored parameter has different type from input func (s Store) Set(ctx sdk.Context, key string, param interface{}) { - store := ctx.KVStore(s.key).Prefix(s.store) + store := s.kvStore(ctx) keybz := []byte(key) bz := store.Get(keybz) @@ -97,7 +101,7 @@ func (s Store) Set(ctx sdk.Context, key string, param interface{}) { } store.Set(keybz, bz) - tstore := ctx.KVStore(s.tkey).Prefix(s.store) + tstore := s.transientStore(ctx) tstore.Set(keybz, []byte{}) } @@ -105,10 +109,10 @@ func (s Store) Set(ctx sdk.Context, key string, param interface{}) { func (s Store) SetRaw(ctx sdk.Context, key string, param []byte) { keybz := []byte(key) - store := ctx.KVStore(s.key).Prefix(s.store) + store := s.kvStore(ctx) store.Set(keybz, param) - tstore := ctx.KVStore(s.tkey).Prefix(s.store) + tstore := s.transientStore(ctx) tstore.Set(keybz, []byte{}) } @@ -122,13 +126,18 @@ func (s Store) GetStruct(ctx sdk.Context, ps ParamStruct) { // Set from ParamStruct func (s Store) SetStruct(ctx sdk.Context, ps ParamStruct) { for _, pair := range ps.KeyFieldPairs() { - s.Set(ctx, pair.Key, pair.Field) + // pair.Field is a pointer to the field, so indirecting the ptr. + // go-amino automatically handles it but just for sure, + // since SetStruct is meant to be used in InitGenesis + // so this method will not be called frequently + v := reflect.Indirect(reflect.ValueOf(pair.Field)).Interface() + s.Set(ctx, pair.Key, v) } } -// Returns a KVStore identical with the paramstore -func (s Store) KVStore(ctx sdk.Context) sdk.KVStore { - return ctx.KVStore(s.key).Prefix(s.store) +// Returns internal namespace +func (s Store) Space() string { + return string(s.space) } // Wrapper of Store, provides immutable functions only @@ -155,3 +164,8 @@ func (ros ReadOnlyStore) Has(ctx sdk.Context, key string) bool { func (ros ReadOnlyStore) Modified(ctx sdk.Context, key string) bool { return ros.s.Modified(ctx, key) } + +// Exposes Space +func (ros ReadOnlyStore) Space() string { + return ros.s.Space() +} diff --git a/x/stake/app_test.go b/x/stake/app_test.go index 0e09dfaa80..3367ec6057 100644 --- a/x/stake/app_test.go +++ b/x/stake/app_test.go @@ -45,7 +45,7 @@ func getMockApp(t *testing.T) (*mock.App, Keeper) { bankKeeper := bank.NewBaseKeeper(mApp.AccountMapper) pk := params.NewKeeper(mApp.Cdc, keyParams, tkeyParams) - keeper := NewKeeper(mApp.Cdc, keyStake, tkeyStake, bankKeeper, pk.Substore("stake"), mApp.RegisterCodespace(DefaultCodespace)) + keeper := NewKeeper(mApp.Cdc, keyStake, tkeyStake, bankKeeper, pk.Substore(DefaultParamspace), mApp.RegisterCodespace(DefaultCodespace)) mApp.Router().AddRoute("stake", NewHandler(keeper)) mApp.SetEndBlocker(getEndBlocker(keeper)) diff --git a/x/stake/keeper/keeper.go b/x/stake/keeper/keeper.go index a349797c6b..c45baaccb3 100644 --- a/x/stake/keeper/keeper.go +++ b/x/stake/keeper/keeper.go @@ -18,11 +18,11 @@ type Keeper struct { hooks sdk.StakingHooks paramstore params.Store - // codestore - codestore sdk.CodespaceType + // codespace + codespace sdk.CodespaceType } -func NewKeeper(cdc *codec.Codec, key, tkey sdk.StoreKey, ck bank.Keeper, paramstore params.Store, codestore sdk.CodespaceType) Keeper { +func NewKeeper(cdc *codec.Codec, key, tkey sdk.StoreKey, ck bank.Keeper, paramstore params.Store, codespace sdk.CodespaceType) Keeper { keeper := Keeper{ storeKey: key, storeTKey: tkey, @@ -30,7 +30,7 @@ func NewKeeper(cdc *codec.Codec, key, tkey sdk.StoreKey, ck bank.Keeper, paramst bankKeeper: ck, paramstore: paramstore, hooks: nil, - codestore: codestore, + codespace: codespace, } return keeper } @@ -46,9 +46,9 @@ func (k Keeper) WithHooks(sh sdk.StakingHooks) Keeper { //_________________________________________________________________________ -// return the codestore +// return the codespace func (k Keeper) Codespace() sdk.CodespaceType { - return k.codestore + return k.codespace } //_______________________________________________________________________