Skip to content

Commit

Permalink
chore: review feedback improvements
Browse files Browse the repository at this point in the history
  • Loading branch information
bryanchriswhite committed Dec 18, 2024
1 parent afebf13 commit 3abc4b5
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 25 deletions.
4 changes: 2 additions & 2 deletions api/poktroll/tokenomics/types.pulsar.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion app/app_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ var (
stakingtypes.BondedPoolName,
stakingtypes.NotBondedPoolName,
// We allow the following module accounts to receive funds:
// govtypes.ModuleName
// govtypes.moduleName
}

// appConfig application configuration (used by depinject)
Expand Down
37 changes: 26 additions & 11 deletions pkg/client/query/options.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
package query

import (
"context"

sdkerrors "cosmossdk.io/errors"

"github.com/pokt-network/poktroll/pkg/client/query/cache"
"github.com/pokt-network/poktroll/pkg/polylog"
)

const (
Expand All @@ -14,12 +17,13 @@ const (
// paramsQuerierConfig is the configuration for parameter queriers. It is intended
// to be configured via ParamsQuerierOptionFn functions.
type paramsQuerierConfig struct {
// CacheOpts are the options passed to create the params cache
CacheOpts []cache.QueryCacheOptionFn
// ModuleName is used for logging and error context
ModuleName string
// ModuleParamError is the base error type for parameter query errors
ModuleParamError *sdkerrors.Error
logger polylog.Logger
// cacheOpts are the options passed to create the params cache
cacheOpts []cache.QueryCacheOptionFn
// moduleName is used for logging and error context
moduleName string
// moduleParamError is the base error type for parameter query errors
moduleParamError *sdkerrors.Error
}

// ParamsQuerierOptionFn is a function which receives a paramsQuerierConfig for configuration.
Expand All @@ -28,7 +32,7 @@ type ParamsQuerierOptionFn func(*paramsQuerierConfig)
// DefaultParamsQuerierConfig returns the default configuration for parameter queriers
func DefaultParamsQuerierConfig() *paramsQuerierConfig {
return &paramsQuerierConfig{
CacheOpts: []cache.QueryCacheOptionFn{
cacheOpts: []cache.QueryCacheOptionFn{
cache.WithHistoricalMode(defaultPruneOlderThan),
cache.WithMaxKeys(defaultMaxKeys),
cache.WithEvictionPolicy(cache.FirstInFirstOut),
Expand All @@ -37,16 +41,27 @@ func DefaultParamsQuerierConfig() *paramsQuerierConfig {
}

// WithModuleInfo sets the module name and param error for the querier.
func WithModuleInfo(moduleName string, moduleParamError *sdkerrors.Error) ParamsQuerierOptionFn {
func WithModuleInfo(ctx context.Context, moduleName string, moduleParamError *sdkerrors.Error) ParamsQuerierOptionFn {
logger := polylog.Ctx(ctx).With(
"module_params_querier", moduleName,
)
return func(cfg *paramsQuerierConfig) {
cfg.ModuleName = moduleName
cfg.ModuleParamError = moduleParamError
cfg.logger = logger
cfg.moduleName = moduleName
cfg.moduleParamError = moduleParamError
}
}

// WithQueryCacheOptions is used to configure the params HistoricalQueryCache.
func WithQueryCacheOptions(opts ...cache.QueryCacheOptionFn) ParamsQuerierOptionFn {
return func(cfg *paramsQuerierConfig) {
cfg.CacheOpts = append(cfg.CacheOpts, opts...)
cfg.cacheOpts = append(cfg.cacheOpts, opts...)
}
}

// WithLogger sets the logger for the querier.
func WithLogger(logger polylog.Logger) ParamsQuerierOptionFn {
return func(cfg *paramsQuerierConfig) {
cfg.logger = logger
}
}
15 changes: 6 additions & 9 deletions pkg/client/query/paramsquerier.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (

"github.com/pokt-network/poktroll/pkg/client"
"github.com/pokt-network/poktroll/pkg/client/query/cache"
"github.com/pokt-network/poktroll/pkg/polylog"
)

// abstractParamsQuerier is NOT intended to be used for anything except the
Expand Down Expand Up @@ -42,7 +41,7 @@ func NewCachedParamsQuerier[P cosmostypes.Msg, Q paramsQuerierIface[P]](
opt(cfg)
}

paramsCache, err := cache.NewInMemoryCache[P](cfg.CacheOpts...)
paramsCache, err := cache.NewInMemoryCache[P](cfg.cacheOpts...)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -78,8 +77,7 @@ type cachedParamsQuerier[P cosmostypes.Msg, Q paramsQuerierIface[P]] struct {
// GetParams returns the latest cached params, if any; otherwise, it queries the
// current on-chain params and caches them.
func (bq *cachedParamsQuerier[P, Q]) GetParams(ctx context.Context) (P, error) {
logger := polylog.Ctx(ctx).With(
"module", bq.config.ModuleName,
logger := bq.config.logger.With(
"method", "GetParams",
)

Expand All @@ -99,8 +97,8 @@ func (bq *cachedParamsQuerier[P, Q]) GetParams(ctx context.Context) (P, error) {
// Query on-chain on cache miss.
params, err := bq.queryClient.GetParams(ctx)
if err != nil {
if bq.config.ModuleParamError != nil {
return paramsZero, bq.config.ModuleParamError.Wrap(err.Error())
if bq.config.moduleParamError != nil {
return paramsZero, bq.config.moduleParamError.Wrap(err.Error())
}
return paramsZero, err
}
Expand All @@ -121,14 +119,13 @@ func (bq *cachedParamsQuerier[P, Q]) GetParams(ctx context.Context) (P, error) {
// update this to query for the historical params, rather than returning the
// current params, if the case of a cache miss.
func (bq *cachedParamsQuerier[P, Q]) GetParamsAtHeight(ctx context.Context, height int64) (P, error) {
logger := polylog.Ctx(ctx).With(
"module", bq.config.ModuleName,
logger := bq.config.logger.With(
"method", "GetParamsAtHeight",
"height", height,
)

// Try to get from cache at specific height
cached, err := bq.paramsCache.GetAtHeight("params", height)
cached, err := bq.paramsCache.GetAsOfVersion("params", height)
switch {
case err == nil:
logger.Debug().Msg("params cache hit")
Expand Down
4 changes: 2 additions & 2 deletions x/tokenomics/keeper/token_logic_modules_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ func TestProcessTokenLogicModules_TLMBurnEqualsMint_Valid(t *testing.T) {
require.True(t, supplierIsFound)
require.Equal(t, &supplierStake, supplier.GetStake())

// Assert that `suppliertypes.ModuleName` account module balance is *unchanged*
// Assert that `suppliertypes.moduleName` account module balance is *unchanged*
// NB: Supplier rewards are minted to the supplier module account but then immediately
// distributed to the supplier accounts which provided service in a given session.
supplierModuleEndBalance := getBalance(t, ctx, keepers, supplierModuleAddress)
Expand Down Expand Up @@ -305,7 +305,7 @@ func TestProcessTokenLogicModules_TLMBurnEqualsMint_Valid_SupplierExceedsMaxClai
require.True(t, supplierIsFound)
require.Equal(t, &supplierStake, supplier.GetStake())

// Assert that `suppliertypes.ModuleName` account module balance is *unchanged*
// Assert that `suppliertypes.moduleName` account module balance is *unchanged*
// NB: Supplier rewards are minted to the supplier module account but then immediately
// distributed to the supplier accounts which provided service in a given session.
supplierModuleEndBalance := getBalance(t, ctx, keepers, supplierModuleAddress)
Expand Down

0 comments on commit 3abc4b5

Please sign in to comment.