Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Params, Clients] add generic ParamsQuerier #995

Open
wants to merge 9 commits into
base: issues/543/cache/memory
Choose a base branch
from
13 changes: 13 additions & 0 deletions pkg/client/interface.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
//go:generate mockgen -destination=../../testutil/mockclient/grpc_conn_mock.go -package=mockclient github.com/cosmos/gogoproto/grpc ClientConn
//go:generate mockgen -destination=../../testutil/mockclient/events_query_client_mock.go -package=mockclient . Dialer,Connection,EventsQueryClient
//go:generate mockgen -destination=../../testutil/mockclient/block_client_mock.go -package=mockclient . Block,BlockClient
//go:generate mockgen -destination=../../testutil/mockclient/delegation_client_mock.go -package=mockclient . DelegationClient
Expand Down Expand Up @@ -380,3 +381,15 @@ type HistoricalQueryCache[T any] interface {
// SetAtHeight adds or updates a value at a specific height
SetAtHeight(key string, value T, height int64) error
}

// ParamsQuerier represents a generic querier for module parameters.
// This interface should be implemented by any module-specific querier
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// This interface should be implemented by any module-specific querier
// This interface SHOULD be implemented by any module-specific querier

// that needs to access and cache on-chain parameters.
type ParamsQuerier[P cosmostypes.Msg] interface {
// GetParams queries the chain for the current module parameters, where
// P is the params type of a given module (e.g. sharedtypes.Params).
GetParams(ctx context.Context) (P, error)
// GetParamsAtHeight returns the parameters as they were at the specified
// height, where M is the params type of a given module (e.g. sharedtypes.Params).
bryanchriswhite marked this conversation as resolved.
Show resolved Hide resolved
GetParamsAtHeight(ctx context.Context, height int64) (P, error)
}
52 changes: 52 additions & 0 deletions pkg/client/query/options.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
package query

import (
sdkerrors "cosmossdk.io/errors"

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

const (
defaultPruneOlderThan = 100
defaultMaxKeys = 1000
)

// 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
}

// ParamsQuerierOptionFn is a function which receives a paramsQuerierConfig for configuration.
type ParamsQuerierOptionFn func(*paramsQuerierConfig)

// DefaultParamsQuerierConfig returns the default configuration for parameter queriers
func DefaultParamsQuerierConfig() *paramsQuerierConfig {
return &paramsQuerierConfig{
CacheOpts: []cache.QueryCacheOptionFn{
cache.WithHistoricalMode(defaultPruneOlderThan),
cache.WithMaxKeys(defaultMaxKeys),
cache.WithEvictionPolicy(cache.FirstInFirstOut),
},
}
}

// WithModuleInfo sets the module name and param error for the querier.
func WithModuleInfo(moduleName string, moduleParamError *sdkerrors.Error) ParamsQuerierOptionFn {
return func(cfg *paramsQuerierConfig) {
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...)
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is clean!

148 changes: 148 additions & 0 deletions pkg/client/query/paramsquerier.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,148 @@
package query

import (
"context"
"errors"

"cosmossdk.io/depinject"
cosmostypes "github.com/cosmos/cosmos-sdk/types"
gogogrpc "github.com/cosmos/gogoproto/grpc"

"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
// compile-time interface compliance assertion that immediately follows.
type abstractParamsQuerier = cachedParamsQuerier[cosmostypes.Msg, paramsQuerierIface[cosmostypes.Msg]]

var _ client.ParamsQuerier[cosmostypes.Msg] = (*abstractParamsQuerier)(nil)

// paramsQuerierIface is an interface which generated query clients MUST implement
// to be compatible with the cachedParamsQuerier.
//
// DEV_NOTE: It is mainly required due to syntactic constraints imposed by the generics
// (i.e. otherwise, P here MUST be a value type, and there's no way to express that Q
// (below) SHOULD be in terms of the concrete type of P in NewCachedParamsQuerier).
type paramsQuerierIface[P cosmostypes.Msg] interface {
GetParams(context.Context) (P, error)
}

// NewCachedParamsQuerier creates a new, generic, params querier with the given
// concrete query client constructor and the configuration which results from
// applying the given options.
func NewCachedParamsQuerier[P cosmostypes.Msg, Q paramsQuerierIface[P]](
deps depinject.Config,
queryClientConstructor func(conn gogogrpc.ClientConn) Q,
opts ...ParamsQuerierOptionFn,
) (_ client.ParamsQuerier[P], err error) {
cfg := DefaultParamsQuerierConfig()
for _, opt := range opts {
opt(cfg)
}

paramsCache, err := cache.NewInMemoryCache[P](cfg.CacheOpts...)
if err != nil {
return nil, err
}

querier := &cachedParamsQuerier[P, Q]{
config: cfg,
paramsCache: paramsCache,
}

if err = depinject.Inject(
deps,
&querier.clientConn,
); err != nil {
return nil, err
}

querier.queryClient = queryClientConstructor(querier.clientConn)

return querier, nil
}

// cachedParamsQuerier provides a generic implementation of cached param querying.
// It handles parameter caching and chain querying in a generic way, where
// P is a pointer type of the parameters, and Q is the interface type of the
// corresponding query client.
type cachedParamsQuerier[P cosmostypes.Msg, Q paramsQuerierIface[P]] struct {
clientConn gogogrpc.ClientConn
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this needed at the cachedParamsQuerier level?

Copy link
Contributor Author

@bryanchriswhite bryanchriswhite Dec 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, indeed. The cachedParamsQuerier has to hold a reference to this because it's a dependency of all query clients, and it also has to know how to generically construct a given modules query client. See NewCachedParamsQuerier():

	if err = depinject.Inject(
		deps,
		&querier.clientConn,
	); err != nil {
		return nil, err
	}

	querier.queryClient = queryClientConstructor(querier.clientConn)

queryClient Q
paramsCache client.HistoricalQueryCache[P]
config *paramsQuerierConfig
}

// 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(
bryanchriswhite marked this conversation as resolved.
Show resolved Hide resolved
"module", bq.config.ModuleName,
"method", "GetParams",
)

// Check the cache first.
var paramsZero P
cached, err := bq.paramsCache.Get("params")
switch {
case err == nil:
logger.Debug().Msgf("params cache hit")
return cached, nil
case !errors.Is(err, cache.ErrCacheMiss):
return paramsZero, err
}

logger.Debug().Msgf("%s", err)

// 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())
}
return paramsZero, err
}

// Update the cache.
if err = bq.paramsCache.Set("params", params); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should consider the case where paramsCache is in historical mode, otherwise this call might fail.

Additionally, I think we should find a way to get the height/version of the retrieved params in order to set it using SetAsOfVersion.

Maybe add the height to the QueryParamsResponse?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed:

  1. cachedParamsQuerier's cache is ALWAYS in historical mode.
  2. This isn't possible anymore due to the decision we made to disable #Set() in historical mode.
  3. We're going to transition to event-driven cache warming.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're going to transition to event-driven cache warming.

Can you add a TODO for that somewhere?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cachedParamsQuerier's cache is ALWAYS in historical mode.

#PUC

return paramsZero, err
}

return params, nil
}

// GetParamsAtHeight returns parameters as they were as of the given height, **if
// that height is present in the cache**. Otherwise, it queries the current params
// and returns them.
//
// TODO_MAINNET(@bryanchriswhite): Once on-chain historical data is available,
// 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,
"method", "GetParamsAtHeight",
"height", height,
)

// Try to get from cache at specific height
cached, err := bq.paramsCache.GetAtHeight("params", height)
bryanchriswhite marked this conversation as resolved.
Show resolved Hide resolved
switch {
case err == nil:
logger.Debug().Msg("params cache hit")
return cached, nil
case !errors.Is(err, cache.ErrCacheMiss):
return cached, err
}

logger.Debug().Msgf("%s", err)

// TODO_MAINNET(@bryanchriswhite): Implement querying historical params from chain
err = cache.ErrCacheMiss.Wrapf("TODO: on-chain historical data not implemented")
logger.Error().Msgf("%s", err)

// Meanwhile, return current params as fallback. 😬
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rube_Goldberg's__Self-Operating_Napkin__(cropped)

return bq.GetParams(ctx)
}
Loading