From fbd59342476df899a97ee6224243e7eb75387457 Mon Sep 17 00:00:00 2001 From: Bryan White Date: Wed, 18 Dec 2024 17:48:56 +0100 Subject: [PATCH] chore: review feedback improvements --- api/poktroll/tokenomics/types.pulsar.go | 4 +- app/app_config.go | 2 +- pkg/client/query/options.go | 37 +++++++++++++------ pkg/client/query/paramsquerier.go | 16 ++++---- .../keeper/token_logic_modules_test.go | 4 +- 5 files changed, 38 insertions(+), 25 deletions(-) diff --git a/api/poktroll/tokenomics/types.pulsar.go b/api/poktroll/tokenomics/types.pulsar.go index e48600e95..b6cd8ff4d 100644 --- a/api/poktroll/tokenomics/types.pulsar.go +++ b/api/poktroll/tokenomics/types.pulsar.go @@ -3,11 +3,11 @@ package tokenomics import ( v1beta1 "cosmossdk.io/api/cosmos/base/v1beta1" - proof "github.com/pokt-network/poktroll/api/poktroll/proof" fmt "fmt" _ "github.com/cosmos/cosmos-proto" runtime "github.com/cosmos/cosmos-proto/runtime" _ "github.com/cosmos/gogoproto/gogoproto" + proof "github.com/pokt-network/poktroll/api/poktroll/proof" protoreflect "google.golang.org/protobuf/reflect/protoreflect" protoiface "google.golang.org/protobuf/runtime/protoiface" protoimpl "google.golang.org/protobuf/runtime/protoimpl" @@ -3067,7 +3067,7 @@ type ModToModTransfer struct { OpReason SettlementOpReason `protobuf:"varint,1,opt,name=op_reason,json=opReason,proto3,enum=poktroll.tokenomics.SettlementOpReason" json:"op_reason,omitempty"` SenderModule string `protobuf:"bytes,2,opt,name=SenderModule,proto3" json:"SenderModule,omitempty"` - RecipientModule string `protobuf:"bytes,3,opt,name=RecipientModule,proto3" json:"RecipientModule,omitempty"` // This the semantic module named that can be found by searching for "ModuleName =" in the codebase + RecipientModule string `protobuf:"bytes,3,opt,name=RecipientModule,proto3" json:"RecipientModule,omitempty"` // This the semantic module named that can be found by searching for "moduleName =" in the codebase Coin *v1beta1.Coin `protobuf:"bytes,4,opt,name=coin,proto3" json:"coin,omitempty"` } diff --git a/app/app_config.go b/app/app_config.go index 76cc7b5b2..e2e3096cf 100644 --- a/app/app_config.go +++ b/app/app_config.go @@ -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) diff --git a/pkg/client/query/options.go b/pkg/client/query/options.go index 4437fa0dc..a335a68a3 100644 --- a/pkg/client/query/options.go +++ b/pkg/client/query/options.go @@ -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 ( @@ -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. @@ -28,7 +32,7 @@ type ParamsQuerierOptionFn func(*paramsQuerierConfig) // DefaultParamsQuerierConfig returns the default configuration for parameter queriers func DefaultParamsQuerierConfig() *paramsQuerierConfig { return ¶msQuerierConfig{ - CacheOpts: []cache.QueryCacheOptionFn{ + cacheOpts: []cache.QueryCacheOptionFn{ cache.WithHistoricalMode(defaultPruneOlderThan), cache.WithMaxKeys(defaultMaxKeys), cache.WithEvictionPolicy(cache.FirstInFirstOut), @@ -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 } } diff --git a/pkg/client/query/paramsquerier.go b/pkg/client/query/paramsquerier.go index 1a5007757..08c1d1499 100644 --- a/pkg/client/query/paramsquerier.go +++ b/pkg/client/query/paramsquerier.go @@ -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 @@ -33,6 +32,7 @@ type paramsQuerierIface[P cosmostypes.Msg] interface { // concrete query client constructor and the configuration which results from // applying the given options. func NewCachedParamsQuerier[P cosmostypes.Msg, Q paramsQuerierIface[P]]( + ctx context.Context, deps depinject.Config, queryClientConstructor func(conn gogogrpc.ClientConn) Q, opts ...ParamsQuerierOptionFn, @@ -42,7 +42,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 } @@ -78,8 +78,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", ) @@ -99,8 +98,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 } @@ -121,14 +120,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") diff --git a/x/tokenomics/keeper/token_logic_modules_test.go b/x/tokenomics/keeper/token_logic_modules_test.go index b70d32608..8c312efc2 100644 --- a/x/tokenomics/keeper/token_logic_modules_test.go +++ b/x/tokenomics/keeper/token_logic_modules_test.go @@ -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) @@ -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)