diff --git a/simapp/app.go b/simapp/app.go index e13779f402..4dd78ed0e8 100644 --- a/simapp/app.go +++ b/simapp/app.go @@ -526,7 +526,7 @@ func (app *SimApp) GetMemKey(storeKey string) *sdk.MemoryStoreKey { // GetSubspace returns a param subspace for a given module name. // // NOTE: This is solely to be used for testing purposes. -func (app *SimApp) GetSubspace(moduleName string) paramstypes.Subspace { +func (app *SimApp) GetSubspace(moduleName string) *paramstypes.Subspace { subspace, _ := app.ParamsKeeper.GetSubspace(moduleName) return subspace } diff --git a/x/auth/keeper/keeper.go b/x/auth/keeper/keeper.go index 08c2c359ca..b01749405a 100644 --- a/x/auth/keeper/keeper.go +++ b/x/auth/keeper/keeper.go @@ -49,7 +49,7 @@ type AccountKeeperI interface { type AccountKeeper struct { key sdk.StoreKey cdc codec.BinaryMarshaler - paramSubspace paramtypes.Subspace + paramSubspace *paramtypes.Subspace permAddrs map[string]types.PermissionsForAddress // The prototypical AccountI constructor. @@ -61,7 +61,7 @@ var _ AccountKeeperI = &AccountKeeper{} // NewAccountKeeper returns a new AccountKeeperI that uses go-amino to // (binary) encode and decode concrete sdk.Accounts. func NewAccountKeeper( - cdc codec.BinaryMarshaler, key sdk.StoreKey, paramstore paramtypes.Subspace, proto func() types.AccountI, + cdc codec.BinaryMarshaler, key sdk.StoreKey, paramstore *paramtypes.Subspace, proto func() types.AccountI, maccPerms map[string][]string, ) AccountKeeper { diff --git a/x/bank/keeper/grpc_query_test.go b/x/bank/keeper/grpc_query_test.go index 878ec191fc..fe9947671d 100644 --- a/x/bank/keeper/grpc_query_test.go +++ b/x/bank/keeper/grpc_query_test.go @@ -118,7 +118,9 @@ func (suite *IntegrationTestSuite) TestQueryParams() { res, err := suite.queryClient.Params(gocontext.Background(), &types.QueryParamsRequest{}) suite.Require().NoError(err) suite.Require().NotNil(res) - suite.Require().Equal(suite.app.BankKeeper.GetParams(suite.ctx), res.GetParams()) + param := suite.app.BankKeeper.GetParams(suite.ctx) + suite.Require().Equal(len(param.SendEnabled), len(res.GetParams().SendEnabled)) + suite.Require().Equal(param.DefaultSendEnabled, res.GetParams().DefaultSendEnabled) } func (suite *IntegrationTestSuite) QueryDenomsMetadataRequest() { diff --git a/x/bank/keeper/keeper.go b/x/bank/keeper/keeper.go index 9190464efd..4641bf0a20 100644 --- a/x/bank/keeper/keeper.go +++ b/x/bank/keeper/keeper.go @@ -54,11 +54,11 @@ type BaseKeeper struct { ak types.AccountKeeper cdc codec.BinaryMarshaler storeKey sdk.StoreKey - paramSpace paramtypes.Subspace + paramSpace *paramtypes.Subspace } func NewBaseKeeper( - cdc codec.BinaryMarshaler, storeKey sdk.StoreKey, ak types.AccountKeeper, paramSpace paramtypes.Subspace, + cdc codec.BinaryMarshaler, storeKey sdk.StoreKey, ak types.AccountKeeper, paramSpace *paramtypes.Subspace, blockedAddrs map[string]bool, ) BaseKeeper { diff --git a/x/bank/keeper/send.go b/x/bank/keeper/send.go index ffe05dd5f3..dac031a14e 100644 --- a/x/bank/keeper/send.go +++ b/x/bank/keeper/send.go @@ -43,14 +43,14 @@ type BaseSendKeeper struct { cdc codec.BinaryMarshaler ak types.AccountKeeper storeKey sdk.StoreKey - paramSpace paramtypes.Subspace + paramSpace *paramtypes.Subspace // list of addresses that are restricted from receiving transactions blockedAddrs map[string]bool } func NewBaseSendKeeper( - cdc codec.BinaryMarshaler, storeKey sdk.StoreKey, ak types.AccountKeeper, paramSpace paramtypes.Subspace, blockedAddrs map[string]bool, + cdc codec.BinaryMarshaler, storeKey sdk.StoreKey, ak types.AccountKeeper, paramSpace *paramtypes.Subspace, blockedAddrs map[string]bool, ) BaseSendKeeper { return BaseSendKeeper{ diff --git a/x/crisis/keeper/keeper.go b/x/crisis/keeper/keeper.go index d87ec7cef4..65fbd78050 100644 --- a/x/crisis/keeper/keeper.go +++ b/x/crisis/keeper/keeper.go @@ -14,7 +14,7 @@ import ( // Keeper - crisis keeper type Keeper struct { routes []types.InvarRoute - paramSpace paramtypes.Subspace + paramSpace *paramtypes.Subspace invCheckPeriod uint supplyKeeper types.SupplyKeeper @@ -24,7 +24,7 @@ type Keeper struct { // NewKeeper creates a new Keeper object func NewKeeper( - paramSpace paramtypes.Subspace, invCheckPeriod uint, supplyKeeper types.SupplyKeeper, + paramSpace *paramtypes.Subspace, invCheckPeriod uint, supplyKeeper types.SupplyKeeper, feeCollectorName string, ) Keeper { diff --git a/x/distribution/keeper/keeper.go b/x/distribution/keeper/keeper.go index aade756d76..51baa9951d 100644 --- a/x/distribution/keeper/keeper.go +++ b/x/distribution/keeper/keeper.go @@ -16,7 +16,7 @@ import ( type Keeper struct { storeKey sdk.StoreKey cdc codec.BinaryMarshaler - paramSpace paramtypes.Subspace + paramSpace *paramtypes.Subspace authKeeper types.AccountKeeper bankKeeper types.BankKeeper stakingKeeper types.StakingKeeper @@ -28,7 +28,7 @@ type Keeper struct { // NewKeeper creates a new distribution Keeper instance func NewKeeper( - cdc codec.BinaryMarshaler, key sdk.StoreKey, paramSpace paramtypes.Subspace, + cdc codec.BinaryMarshaler, key sdk.StoreKey, paramSpace *paramtypes.Subspace, ak types.AccountKeeper, bk types.BankKeeper, sk types.StakingKeeper, feeCollectorName string, blockedAddrs map[string]bool, ) Keeper { diff --git a/x/ibc/applications/transfer/keeper/keeper.go b/x/ibc/applications/transfer/keeper/keeper.go index 7c0dcbbec5..d5f35168f7 100644 --- a/x/ibc/applications/transfer/keeper/keeper.go +++ b/x/ibc/applications/transfer/keeper/keeper.go @@ -21,7 +21,7 @@ import ( type Keeper struct { storeKey sdk.StoreKey cdc codec.BinaryMarshaler - paramSpace paramtypes.Subspace + paramSpace *paramtypes.Subspace channelKeeper types.ChannelKeeper portKeeper types.PortKeeper @@ -32,7 +32,7 @@ type Keeper struct { // NewKeeper creates a new IBC transfer Keeper instance func NewKeeper( - cdc codec.BinaryMarshaler, key sdk.StoreKey, paramSpace paramtypes.Subspace, + cdc codec.BinaryMarshaler, key sdk.StoreKey, paramSpace *paramtypes.Subspace, channelKeeper types.ChannelKeeper, portKeeper types.PortKeeper, authKeeper types.AccountKeeper, bankKeeper types.BankKeeper, scopedKeeper capabilitykeeper.ScopedKeeper, ) Keeper { diff --git a/x/ibc/core/02-client/keeper/keeper.go b/x/ibc/core/02-client/keeper/keeper.go index f8dd00d43d..07e030c32f 100644 --- a/x/ibc/core/02-client/keeper/keeper.go +++ b/x/ibc/core/02-client/keeper/keeper.go @@ -26,12 +26,12 @@ import ( type Keeper struct { storeKey sdk.StoreKey cdc codec.BinaryMarshaler - paramSpace paramtypes.Subspace + paramSpace *paramtypes.Subspace stakingKeeper types.StakingKeeper } // NewKeeper creates a new NewKeeper instance -func NewKeeper(cdc codec.BinaryMarshaler, key sdk.StoreKey, paramSpace paramtypes.Subspace, sk types.StakingKeeper) Keeper { +func NewKeeper(cdc codec.BinaryMarshaler, key sdk.StoreKey, paramSpace *paramtypes.Subspace, sk types.StakingKeeper) Keeper { // set KeyTable if it has not already been set if !paramSpace.HasKeyTable() { paramSpace = paramSpace.WithKeyTable(types.ParamKeyTable()) diff --git a/x/ibc/core/keeper/keeper.go b/x/ibc/core/keeper/keeper.go index 7d0d46f727..1b8719e1e2 100644 --- a/x/ibc/core/keeper/keeper.go +++ b/x/ibc/core/keeper/keeper.go @@ -32,7 +32,7 @@ type Keeper struct { // NewKeeper creates a new ibc Keeper func NewKeeper( - cdc codec.BinaryMarshaler, key sdk.StoreKey, paramSpace paramtypes.Subspace, + cdc codec.BinaryMarshaler, key sdk.StoreKey, paramSpace *paramtypes.Subspace, stakingKeeper clienttypes.StakingKeeper, scopedKeeper capabilitykeeper.ScopedKeeper, ) *Keeper { clientKeeper := clientkeeper.NewKeeper(cdc, key, paramSpace, stakingKeeper) diff --git a/x/mint/keeper/keeper.go b/x/mint/keeper/keeper.go index d432b5d7f7..0cfbeec508 100644 --- a/x/mint/keeper/keeper.go +++ b/x/mint/keeper/keeper.go @@ -13,7 +13,7 @@ import ( type Keeper struct { cdc codec.BinaryMarshaler storeKey sdk.StoreKey - paramSpace paramtypes.Subspace + paramSpace *paramtypes.Subspace stakingKeeper types.StakingKeeper bankKeeper types.BankKeeper feeCollectorName string @@ -21,7 +21,7 @@ type Keeper struct { // NewKeeper creates a new mint Keeper instance func NewKeeper( - cdc codec.BinaryMarshaler, key sdk.StoreKey, paramSpace paramtypes.Subspace, + cdc codec.BinaryMarshaler, key sdk.StoreKey, paramSpace *paramtypes.Subspace, sk types.StakingKeeper, ak types.AccountKeeper, bk types.BankKeeper, feeCollectorName string, ) Keeper { diff --git a/x/params/keeper/grpc_query_test.go b/x/params/keeper/grpc_query_test.go index 1267ed1b32..e451ee7504 100644 --- a/x/params/keeper/grpc_query_test.go +++ b/x/params/keeper/grpc_query_test.go @@ -12,7 +12,7 @@ func (suite *KeeperTestSuite) TestGRPCQueryParams() { var ( req *proposal.QueryParamsRequest expValue string - space types.Subspace + space *types.Subspace ) key := []byte("key") diff --git a/x/params/keeper/keeper.go b/x/params/keeper/keeper.go index 48d4f50cc6..33b7dae579 100644 --- a/x/params/keeper/keeper.go +++ b/x/params/keeper/keeper.go @@ -35,7 +35,7 @@ func (k Keeper) Logger(ctx sdk.Context) log.Logger { } // Allocate subspace used for keepers -func (k Keeper) Subspace(s string) types.Subspace { +func (k Keeper) Subspace(s string) *types.Subspace { _, ok := k.spaces[s] if ok { panic("subspace already occupied") @@ -46,16 +46,16 @@ func (k Keeper) Subspace(s string) types.Subspace { } space := types.NewSubspace(k.cdc, k.legacyAmino, k.key, k.tkey, s) - k.spaces[s] = &space + k.spaces[s] = space return space } // Get existing substore from keeper -func (k Keeper) GetSubspace(s string) (types.Subspace, bool) { +func (k Keeper) GetSubspace(s string) (*types.Subspace, bool) { space, ok := k.spaces[s] if !ok { - return types.Subspace{}, false + return &types.Subspace{}, false } - return *space, ok + return space, ok } diff --git a/x/params/types/subspace.go b/x/params/types/subspace.go index 273af1ae56..2f5943a627 100644 --- a/x/params/types/subspace.go +++ b/x/params/types/subspace.go @@ -3,6 +3,8 @@ package types import ( "fmt" "reflect" + "sync/atomic" + "unsafe" "github.com/line/lbm-sdk/v2/codec" "github.com/line/lbm-sdk/v2/store/prefix" @@ -30,8 +32,8 @@ type Subspace struct { } // NewSubspace constructs a store with namestore -func NewSubspace(cdc codec.BinaryMarshaler, legacyAmino *codec.LegacyAmino, key sdk.StoreKey, tkey sdk.StoreKey, name string) Subspace { - return Subspace{ +func NewSubspace(cdc codec.BinaryMarshaler, legacyAmino *codec.LegacyAmino, key sdk.StoreKey, tkey sdk.StoreKey, name string) *Subspace { + return &Subspace{ cdc: cdc, legacyAmino: legacyAmino, key: key, @@ -42,12 +44,12 @@ func NewSubspace(cdc codec.BinaryMarshaler, legacyAmino *codec.LegacyAmino, key } // HasKeyTable returns if the Subspace has a KeyTable registered. -func (s Subspace) HasKeyTable() bool { +func (s *Subspace) HasKeyTable() bool { return len(s.table.m) > 0 } // WithKeyTable initializes KeyTable and returns modified Subspace -func (s Subspace) WithKeyTable(table KeyTable) Subspace { +func (s *Subspace) WithKeyTable(table KeyTable) *Subspace { if table.m == nil { panic("SetKeyTable() called with nil KeyTable") } @@ -69,7 +71,7 @@ func (s Subspace) WithKeyTable(table KeyTable) Subspace { } // Returns a KVStore identical with ctx.KVStore(s.key).Prefix() -func (s Subspace) kvStore(ctx sdk.Context) sdk.KVStore { +func (s *Subspace) kvStore(ctx sdk.Context) sdk.KVStore { // this function can be called concurrently so we should not call append on s.name directly name := make([]byte, len(s.name)) copy(name, s.name) @@ -77,7 +79,7 @@ func (s Subspace) kvStore(ctx sdk.Context) sdk.KVStore { } // Returns a transient store for modification -func (s Subspace) transientStore(ctx sdk.Context) sdk.KVStore { +func (s *Subspace) transientStore(ctx sdk.Context) sdk.KVStore { // this function can be called concurrently so we should not call append on s.name directly name := make([]byte, len(s.name)) copy(name, s.name) @@ -86,7 +88,7 @@ func (s Subspace) transientStore(ctx sdk.Context) sdk.KVStore { // Validate attempts to validate a parameter value by its key. If the key is not // registered or if the validation of the value fails, an error is returned. -func (s Subspace) Validate(ctx sdk.Context, key []byte, value interface{}) error { +func (s *Subspace) Validate(ctx sdk.Context, key []byte, value interface{}) error { attr, ok := s.table.m[string(key)] if !ok { return fmt.Errorf("parameter %s not registered", string(key)) @@ -101,21 +103,30 @@ func (s Subspace) Validate(ctx sdk.Context, key []byte, value interface{}) error // Get queries for a parameter by key from the Subspace's KVStore and sets the // value to the provided pointer. If the value does not exist, it will panic. -func (s Subspace) Get(ctx sdk.Context, key []byte, ptr interface{}) { +func (s *Subspace) Get(ctx sdk.Context, key []byte, ptr interface{}) { s.checkType(key, ptr) + if s.loadFromCache(key, ptr) { + // cache hit + return + } store := s.kvStore(ctx) bz := store.Get(key) if err := s.legacyAmino.UnmarshalJSON(bz, ptr); err != nil { panic(err) } + s.cacheValue(key, ptr) } // GetIfExists queries for a parameter by key from the Subspace's KVStore and // sets the value to the provided pointer. If the value does not exist, it will // perform a no-op. -func (s Subspace) GetIfExists(ctx sdk.Context, key []byte, ptr interface{}) { +func (s *Subspace) GetIfExists(ctx sdk.Context, key []byte, ptr interface{}) { + if s.loadFromCache(key, ptr) { + // cache hit + return + } store := s.kvStore(ctx) bz := store.Get(key) if bz == nil { @@ -127,29 +138,33 @@ func (s Subspace) GetIfExists(ctx sdk.Context, key []byte, ptr interface{}) { if err := s.legacyAmino.UnmarshalJSON(bz, ptr); err != nil { panic(err) } + s.cacheValue(key, ptr) } // GetRaw queries for the raw values bytes for a parameter by key. -func (s Subspace) GetRaw(ctx sdk.Context, key []byte) []byte { +func (s *Subspace) GetRaw(ctx sdk.Context, key []byte) []byte { store := s.kvStore(ctx) return store.Get(key) } // Has returns if a parameter key exists or not in the Subspace's KVStore. -func (s Subspace) Has(ctx sdk.Context, key []byte) bool { +func (s *Subspace) Has(ctx sdk.Context, key []byte) bool { + if s.hasCache(key) { + return true + } store := s.kvStore(ctx) return store.Has(key) } // Modified returns true if the parameter key is set in the Subspace's transient // KVStore. -func (s Subspace) Modified(ctx sdk.Context, key []byte) bool { +func (s *Subspace) Modified(ctx sdk.Context, key []byte) bool { tstore := s.transientStore(ctx) return tstore.Has(key) } // checkType verifies that the provided key and value are comptable and registered. -func (s Subspace) checkType(key []byte, value interface{}) { +func (s *Subspace) checkType(key []byte, value interface{}) { attr, ok := s.table.m[string(key)] if !ok { panic(fmt.Sprintf("parameter %s not registered", string(key))) @@ -166,11 +181,74 @@ func (s Subspace) checkType(key []byte, value interface{}) { } } +// All the cache-related functions here are thread-safe. +// Currently, since `CheckTx` and `DeliverTx` can run without abci locking, +// these functions must be thread-safe as tx can run concurrently. +// The map data type is not thread-safe by itself, but concurrent access is +// possible with entry fixed. If we access the subspace with an unregistered key, +// it panics, ensuring that the entry of the map is not extended after the server runs. +// Value update and read operations for a single entry of a map can be performed concurrently by +// `atomic.StorePointer` and `atomic.LoadPointer`. +func (s *Subspace) cacheValue(key []byte, value interface{}) { + attr, ok := s.table.m[string(key)] + if !ok { + panic(fmt.Sprintf("parameter %s not registered", string(key))) + } + val := reflect.ValueOf(value) + if reflect.TypeOf(value).Kind() == reflect.Ptr { + val = val.Elem() + } + valueToBeCached := reflect.New(val.Type()) + valueToBeCached.Elem().Set(val) + atomic.StorePointer(&attr.cachedValue, unsafe.Pointer(&valueToBeCached)) +} + +func (s *Subspace) hasCache(key []byte) bool { + attr, ok := s.table.m[string(key)] + if !ok { + panic(fmt.Sprintf("parameter %s not registered", string(key))) + } + cachedValuePtr := (*reflect.Value)(atomic.LoadPointer(&attr.cachedValue)) + return cachedValuePtr != nil +} + +func (s *Subspace) loadFromCache(key []byte, value interface{}) bool { + attr, ok := s.table.m[string(key)] + if !ok { + return false + } + if reflect.TypeOf(value).Kind() != reflect.Ptr { + panic("value should be a Pointer") + } + + cachedValuePtr := (*reflect.Value)(atomic.LoadPointer(&attr.cachedValue)) + if cachedValuePtr == nil { + return false + } + reflect.ValueOf(value).Elem().Set((*cachedValuePtr).Elem()) + return true +} + +// Only for test +func (s *Subspace) GetCachedValueForTesting(key []byte, value interface{}) bool { + return s.loadFromCache(key, value) +} + +// Only for test +func (s *Subspace) HasCacheForTesting(key []byte) bool { + return s.hasCache(key) +} + +// Only for test +func (s *Subspace) SetCacheForTesting(key []byte, value interface{}) { + s.cacheValue(key, value) +} + // Set stores a value for given a parameter key assuming the parameter type has // been registered. It will panic if the parameter type has not been registered // or if the value cannot be encoded. A change record is also set in the Subspace's // transient KVStore to mark the parameter as modified. -func (s Subspace) Set(ctx sdk.Context, key []byte, value interface{}) { +func (s *Subspace) Set(ctx sdk.Context, key []byte, value interface{}) { s.checkType(key, value) store := s.kvStore(ctx) @@ -183,6 +261,8 @@ func (s Subspace) Set(ctx sdk.Context, key []byte, value interface{}) { tstore := s.transientStore(ctx) tstore.Set(key, []byte{}) + + s.cacheValue(key, value) } // Update stores an updated raw value for a given parameter key assuming the @@ -191,7 +271,7 @@ func (s Subspace) Set(ctx sdk.Context, key []byte, value interface{}) { // if the raw value is not compatible with the registered type for the parameter // key or if the new value is invalid as determined by the registered type's // validation function. -func (s Subspace) Update(ctx sdk.Context, key, value []byte) error { +func (s *Subspace) Update(ctx sdk.Context, key, value []byte) error { attr, ok := s.table.m[string(key)] if !ok { panic(fmt.Sprintf("parameter %s not registered", string(key))) @@ -219,7 +299,7 @@ func (s Subspace) Update(ctx sdk.Context, key, value []byte) error { // GetParamSet 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#Get. -func (s Subspace) GetParamSet(ctx sdk.Context, ps ParamSet) { +func (s *Subspace) GetParamSet(ctx sdk.Context, ps ParamSet) { for _, pair := range ps.ParamSetPairs() { s.Get(ctx, pair.Key, pair.Value) } @@ -227,7 +307,7 @@ func (s Subspace) GetParamSet(ctx sdk.Context, ps ParamSet) { // 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) { +func (s *Subspace) SetParamSet(ctx sdk.Context, ps ParamSet) { for _, pair := range ps.ParamSetPairs() { // pair.Field is a pointer to the field, so indirecting the ptr. // go-amino automatically handles it but just for sure, @@ -244,13 +324,13 @@ func (s Subspace) SetParamSet(ctx sdk.Context, ps ParamSet) { } // Name returns the name of the Subspace. -func (s Subspace) Name() string { +func (s *Subspace) Name() string { return string(s.name) } // Wrapper of Subspace, provides immutable functions only type ReadOnlySubspace struct { - s Subspace + s *Subspace } // Get delegates a read-only Get call to the Subspace. diff --git a/x/params/types/subspace_test.go b/x/params/types/subspace_test.go index 018c2d518a..82f59c1ff5 100644 --- a/x/params/types/subspace_test.go +++ b/x/params/types/subspace_test.go @@ -23,7 +23,7 @@ type SubspaceTestSuite struct { cdc codec.BinaryMarshaler amino *codec.LegacyAmino ctx sdk.Context - ss types.Subspace + ss *types.Subspace } func (suite *SubspaceTestSuite) SetupTest() { @@ -198,6 +198,55 @@ func (suite *SubspaceTestSuite) TestSetParamSet() { suite.Require().Equal(a.BondDenom, b.BondDenom) } +func (suite *SubspaceTestSuite) TestParamSetCache() { + a := params{ + UnbondingTime: time.Hour * 48, + MaxValidators: 100, + BondDenom: "stake", + } + b := params{} + // confirm cache is empty + suite.Require().False(suite.ss.GetCachedValueForTesting(keyUnbondingTime, &b.UnbondingTime)) + suite.Require().False(suite.ss.GetCachedValueForTesting(keyMaxValidators, &b.MaxValidators)) + suite.Require().False(suite.ss.GetCachedValueForTesting(keyBondDenom, &b.BondDenom)) + suite.Require().False(suite.ss.HasCacheForTesting(keyUnbondingTime)) + suite.Require().False(suite.ss.HasCacheForTesting(keyMaxValidators)) + suite.Require().False(suite.ss.HasCacheForTesting(keyBondDenom)) + suite.Require().NotEqual(a, b) + + suite.Require().NotPanics(func() { + suite.ss.SetParamSet(suite.ctx, &a) + }) + // confirm cached + suite.Require().True(suite.ss.GetCachedValueForTesting(keyUnbondingTime, &b.UnbondingTime)) + suite.Require().True(suite.ss.GetCachedValueForTesting(keyMaxValidators, &b.MaxValidators)) + suite.Require().True(suite.ss.GetCachedValueForTesting(keyBondDenom, &b.BondDenom)) + suite.Require().Equal(a, b) + + // update local variable + a.UnbondingTime = time.Hour * 24 + a.MaxValidators = 50 + a.BondDenom = "link" + + // confirm the cache is not updated + c := params{} + suite.Require().True(suite.ss.GetCachedValueForTesting(keyUnbondingTime, &c.UnbondingTime)) + suite.Require().True(suite.ss.GetCachedValueForTesting(keyMaxValidators, &c.MaxValidators)) + suite.Require().True(suite.ss.GetCachedValueForTesting(keyBondDenom, &c.BondDenom)) + suite.Require().Equal(b, c) // cached value is not updated + + // update only cache + suite.ss.SetCacheForTesting(keyUnbondingTime, &a.UnbondingTime) + suite.ss.SetCacheForTesting(keyMaxValidators, &a.MaxValidators) + suite.ss.SetCacheForTesting(keyBondDenom, &a.BondDenom) + + // ensure GetParamSet to get value not from db but from cache + suite.Require().NotPanics(func() { + suite.ss.GetParamSet(suite.ctx, &c) + }) + suite.Require().Equal(a, c) +} + func (suite *SubspaceTestSuite) TestName() { suite.Require().Equal("testsubspace", suite.ss.Name()) } diff --git a/x/params/types/table.go b/x/params/types/table.go index b4ddbef640..45d3269f82 100644 --- a/x/params/types/table.go +++ b/x/params/types/table.go @@ -2,23 +2,25 @@ package types import ( "reflect" + "unsafe" sdk "github.com/line/lbm-sdk/v2/types" ) type attribute struct { - ty reflect.Type - vfn ValueValidatorFn + ty reflect.Type + vfn ValueValidatorFn + cachedValue unsafe.Pointer } // KeyTable subspaces appropriate type for each parameter key type KeyTable struct { - m map[string]attribute + m map[string]*attribute } func NewKeyTable(pairs ...ParamSetPair) KeyTable { keyTable := KeyTable{ - m: make(map[string]attribute), + m: make(map[string]*attribute), } for _, psp := range pairs { @@ -52,7 +54,7 @@ func (t KeyTable) RegisterType(psp ParamSetPair) KeyTable { rty = rty.Elem() } - t.m[keystr] = attribute{ + t.m[keystr] = &attribute{ vfn: psp.ValidatorFn, ty: rty, } diff --git a/x/slashing/types/expected_keepers.go b/x/slashing/types/expected_keepers.go index 8b228f5a76..b7d6b6ca89 100644 --- a/x/slashing/types/expected_keepers.go +++ b/x/slashing/types/expected_keepers.go @@ -27,7 +27,7 @@ type BankKeeper interface { // ParamSubspace defines the expected Subspace interfacace type ParamSubspace interface { HasKeyTable() bool - WithKeyTable(table paramtypes.KeyTable) paramtypes.Subspace + WithKeyTable(table paramtypes.KeyTable) *paramtypes.Subspace Get(ctx sdk.Context, key []byte, ptr interface{}) GetParamSet(ctx sdk.Context, ps paramtypes.ParamSet) SetParamSet(ctx sdk.Context, ps paramtypes.ParamSet) diff --git a/x/staking/keeper/keeper.go b/x/staking/keeper/keeper.go index 5c8f98e90f..a17430b8c1 100644 --- a/x/staking/keeper/keeper.go +++ b/x/staking/keeper/keeper.go @@ -25,14 +25,14 @@ type Keeper struct { authKeeper types.AccountKeeper bankKeeper types.BankKeeper hooks types.StakingHooks - paramstore paramtypes.Subspace + paramstore *paramtypes.Subspace validatorCacheList *list.List } // NewKeeper creates a new staking Keeper instance func NewKeeper( cdc codec.BinaryMarshaler, key sdk.StoreKey, ak types.AccountKeeper, bk types.BankKeeper, - ps paramtypes.Subspace, + ps *paramtypes.Subspace, ) Keeper { // set KeyTable if it has not already been set if !ps.HasKeyTable() { diff --git a/x/wasm/internal/keeper/keeper.go b/x/wasm/internal/keeper/keeper.go index 14ab34fdc4..7855bfbe9a 100644 --- a/x/wasm/internal/keeper/keeper.go +++ b/x/wasm/internal/keeper/keeper.go @@ -59,7 +59,7 @@ type Keeper struct { // queryGasLimit is the max wasmvm gas that can be spent on executing a query with a contract queryGasLimit uint64 authZPolicy AuthorizationPolicy - paramSpace paramtypes.Subspace + paramSpace *paramtypes.Subspace } // NewKeeper creates a new contract Keeper instance @@ -67,7 +67,7 @@ type Keeper struct { func NewKeeper( cdc codec.Marshaler, storeKey sdk.StoreKey, - paramSpace paramtypes.Subspace, + paramSpace *paramtypes.Subspace, accountKeeper authkeeper.AccountKeeper, bankKeeper types.BankKeeper, stakingKeeper types.StakingKeeper, diff --git a/x/wasm/internal/keeper/keeper_test.go b/x/wasm/internal/keeper/keeper_test.go index 31b307f87e..86d20b27de 100644 --- a/x/wasm/internal/keeper/keeper_test.go +++ b/x/wasm/internal/keeper/keeper_test.go @@ -287,7 +287,7 @@ func TestInstantiate(t *testing.T) { gasAfter := ctx.GasMeter().GasConsumed() if types.EnableGasVerification { - require.Equal(t, uint64(0x13360), gasAfter-gasBefore) + require.Equal(t, uint64(0x11b90), gasAfter-gasBefore) } // ensure it is stored properly @@ -521,7 +521,7 @@ func TestExecute(t *testing.T) { // make sure gas is properly deducted from ctx gasAfter := ctx.GasMeter().GasConsumed() if types.EnableGasVerification { - require.Equal(t, uint64(0x14060), gasAfter-gasBefore) + require.Equal(t, uint64(0x1180c), gasAfter-gasBefore) } // ensure bob now exists and got both payments released bobAcct = accKeeper.GetAccount(ctx, bob) diff --git a/x/wasm/internal/keeper/recurse_test.go b/x/wasm/internal/keeper/recurse_test.go index 58c899d3fa..643ad6199d 100644 --- a/x/wasm/internal/keeper/recurse_test.go +++ b/x/wasm/internal/keeper/recurse_test.go @@ -58,9 +58,9 @@ func initRecurseContract(t *testing.T) (contract sdk.AccAddress, creator sdk.Acc func TestGasCostOnQuery(t *testing.T) { const ( - GasNoWork uint64 = 50_155 + GasNoWork uint64 = 44_059 // Note: about 100 SDK gas (10k wasmer gas) for each round of sha256 - GasWork50 uint64 = 55_824 // this is a little shy of 50k gas - to keep an eye on the limit + GasWork50 uint64 = 49_728 // this is a little shy of 50k gas - to keep an eye on the limit GasReturnUnhashed uint64 = 287 GasReturnHashed uint64 = 262 @@ -225,7 +225,7 @@ func TestLimitRecursiveQueryGas(t *testing.T) { const ( // Note: about 100 SDK gas (10k wasmer gas) for each round of sha256 - GasWork2k uint64 = 278_878 // = InstanceCost + x // we have 6x gas used in cpu than in the instance + GasWork2k uint64 = 272_782 // = InstanceCost + x // we have 6x gas used in cpu than in the instance // This is overhead for calling into a sub-contract GasReturnHashed uint64 = 265 ) diff --git a/x/wasm/internal/keeper/relay_test.go b/x/wasm/internal/keeper/relay_test.go index be5971366d..0d263ae2a8 100644 --- a/x/wasm/internal/keeper/relay_test.go +++ b/x/wasm/internal/keeper/relay_test.go @@ -67,7 +67,7 @@ func TestOnOpenChannel(t *testing.T) { } require.NoError(t, err) // verify gas consumed - const storageCosts = sdk.Gas(0x1a67) + const storageCosts = sdk.Gas(0xa8b) assert.Equal(t, spec.contractGas, ctx.GasMeter().GasConsumed()-before-storageCosts) }) } @@ -174,7 +174,7 @@ func TestOnConnectChannel(t *testing.T) { } require.NoError(t, err) // verify gas consumed - const storageCosts = sdk.Gas(0x1a67) + const storageCosts = sdk.Gas(0xa8b) assert.Equal(t, spec.contractGas, ctx.GasMeter().GasConsumed()-before-storageCosts) // verify msgs dispatched assert.Equal(t, spec.contractResp.Messages, *capturedMsgs) @@ -283,7 +283,7 @@ func TestOnCloseChannel(t *testing.T) { } require.NoError(t, err) // verify gas consumed - const storageCosts = sdk.Gas(0x1a67) + const storageCosts = sdk.Gas(0xa8b) assert.Equal(t, spec.contractGas, ctx.GasMeter().GasConsumed()-before-storageCosts) // verify msgs dispatched assert.Equal(t, spec.contractResp.Messages, *capturedMsgs) @@ -408,7 +408,7 @@ func TestOnRecvPacket(t *testing.T) { require.Equal(t, spec.contractResp.Acknowledgement, gotAck) // verify gas consumed - const storageCosts = sdk.Gas(0x1a67) + const storageCosts = sdk.Gas(0xa8b) assert.Equal(t, spec.contractGas, ctx.GasMeter().GasConsumed()-before-storageCosts) // verify msgs dispatched assert.Equal(t, spec.contractResp.Messages, *capturedMsgs) @@ -519,7 +519,7 @@ func TestOnAckPacket(t *testing.T) { } require.NoError(t, err) // verify gas consumed - const storageCosts = sdk.Gas(0x1a67) + const storageCosts = sdk.Gas(0xa8b) assert.Equal(t, spec.contractGas, ctx.GasMeter().GasConsumed()-before-storageCosts) // verify msgs dispatched assert.Equal(t, spec.contractResp.Messages, *capturedMsgs) @@ -629,7 +629,7 @@ func TestOnTimeoutPacket(t *testing.T) { } require.NoError(t, err) // verify gas consumed - const storageCosts = sdk.Gas(0x1a67) + const storageCosts = sdk.Gas(0xa8b) assert.Equal(t, spec.contractGas, ctx.GasMeter().GasConsumed()-before-storageCosts) // verify msgs dispatched assert.Equal(t, spec.contractResp.Messages, *capturedMsgs) diff --git a/x/wasm/internal/keeper/submsg_test.go b/x/wasm/internal/keeper/submsg_test.go index 120c312660..d8d7151191 100644 --- a/x/wasm/internal/keeper/submsg_test.go +++ b/x/wasm/internal/keeper/submsg_test.go @@ -262,14 +262,14 @@ func TestDispatchSubMsgErrorHandling(t *testing.T) { submsgID: 5, msg: validBankSend, // note we charge another 40k for the reply call - resultAssertions: []assertion{assertReturnedEvents(3), assertGasUsed(123000, 136000)}, + resultAssertions: []assertion{assertReturnedEvents(3), assertGasUsed(121000, 134000)}, }, "not enough tokens": { submsgID: 6, msg: invalidBankSend, subMsgError: true, // uses less gas than the send tokens (cost of bank transfer) - resultAssertions: []assertion{assertGasUsed(97000, 120000), assertErrorString("insufficient funds")}, + resultAssertions: []assertion{assertGasUsed(96000, 110000), assertErrorString("insufficient funds")}, }, "out of gas panic with no gas limit": { submsgID: 7, @@ -282,7 +282,7 @@ func TestDispatchSubMsgErrorHandling(t *testing.T) { msg: validBankSend, gasLimit: &subGasLimit, // uses same gas as call without limit - resultAssertions: []assertion{assertReturnedEvents(3), assertGasUsed(123000, 136000)}, + resultAssertions: []assertion{assertReturnedEvents(3), assertGasUsed(121000, 134000)}, }, "not enough tokens with limit": { submsgID: 16, @@ -290,7 +290,7 @@ func TestDispatchSubMsgErrorHandling(t *testing.T) { subMsgError: true, gasLimit: &subGasLimit, // uses same gas as call without limit - resultAssertions: []assertion{assertGasUsed(97000, 120000), assertErrorString("insufficient funds")}, + resultAssertions: []assertion{assertGasUsed(96000, 110000), assertErrorString("insufficient funds")}, }, "out of gas caught with gas limit": { submsgID: 17,