From 00ab22f7d54b6658d1c024440fb37e110dd5198d Mon Sep 17 00:00:00 2001 From: forcodedancing Date: Fri, 28 Jul 2023 18:41:15 +0800 Subject: [PATCH 1/4] feat: add flag to enable or disable heavy queries --- baseapp/abci.go | 3 ++- baseapp/baseapp.go | 5 ++++- baseapp/options.go | 5 +++++ server/config/config.go | 4 ++++ server/config/toml.go | 4 ++++ server/start.go | 3 +++ server/util.go | 1 + types/context.go | 11 +++++++++++ types/query/pagination.go | 9 +++++++++ x/auth/keeper/grpc_query.go | 6 ++++++ 10 files changed, 49 insertions(+), 2 deletions(-) diff --git a/baseapp/abci.go b/baseapp/abci.go index 2ae558f5ef..f99fc47fd7 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -954,7 +954,8 @@ func (app *BaseApp) CreateQueryContext(height int64, prove bool, path ...string) // branch the commit-multistore for safety ctx := sdk.NewContext(cacheMS, qs.ctx.BlockHeader(), true, app.upgradeChecker, app.logger). WithMinGasPrices(app.minGasPrices). - WithBlockHeight(height) + WithBlockHeight(height). + WithEnableUnsafeQuery(app.enableUnsafeQuery) if height != lastBlockHeight { rms, ok := app.cms.(*rootmulti.Store) diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 2a04d0bcb4..cc73a85000 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -163,8 +163,11 @@ type BaseApp struct { //nolint: maligned // upgradeChecker is a hook function from the upgrade module to check upgrade is executed or not. upgradeChecker func(ctx sdk.Context, name string) bool - // Signatures of recent blocks to speed up + // sigCache caches the signatures of recent blocks to speed up sigCache *lru.ARCCache + + // enableUnsafeQuery defines whether the unsafe queries will be enabled or not + enableUnsafeQuery bool } // NewBaseApp returns a reference to an initialized BaseApp. It accepts a diff --git a/baseapp/options.go b/baseapp/options.go index e57bee6a25..4fee55e7c6 100644 --- a/baseapp/options.go +++ b/baseapp/options.go @@ -102,6 +102,11 @@ func SetChainID(chainID string) func(*BaseApp) { return func(app *BaseApp) { app.chainID = chainID } } +// SetEnableUnsafeQuery sets the flag to enable unsafe query in BaseApp. +func SetEnableUnsafeQuery(enabled bool) func(*BaseApp) { + return func(app *BaseApp) { app.enableUnsafeQuery = enabled } +} + func (app *BaseApp) SetName(name string) { if app.sealed { panic("SetName() on sealed BaseApp") diff --git a/server/config/config.go b/server/config/config.go index 732d785ec0..27939deb7a 100644 --- a/server/config/config.go +++ b/server/config/config.go @@ -98,6 +98,9 @@ type BaseConfig struct { // AppDBBackend defines the type of Database to use for the application and snapshots databases. // An empty string indicates that the Tendermint config's DBBackend value should be used. AppDBBackend string `mapstructure:"app-db-backend"` + + // EnableUnsafeQuery enable/disable unsafe query apis. + EnableUnsafeQuery bool `mapstructure:"enable-unsafe-query"` } // APIConfig defines the API listener configuration. @@ -279,6 +282,7 @@ func DefaultConfig() *Config { IAVLDisableFastNode: false, IAVLLazyLoading: false, AppDBBackend: "", + EnableUnsafeQuery: false, }, Telemetry: telemetry.Config{ Enabled: false, diff --git a/server/config/toml.go b/server/config/toml.go index 7636434c0b..6c66a5a1d8 100644 --- a/server/config/toml.go +++ b/server/config/toml.go @@ -91,6 +91,10 @@ iavl-lazy-loading = {{ .BaseConfig.IAVLLazyLoading }} # Second fallback (if the types.DBBackend also isn't set), is the db-backend value set in Tendermint's config.toml. app-db-backend = "{{ .BaseConfig.AppDBBackend }}" +# EnableUnsafeQuery enables or disables the unsafe queries. +# Default is false. +enable-unsafe-query = "{{ .BaseConfig.EnableUnsafeQuery }}" + ############################################################################### ### Upgrade Configuration ### ############################################################################### diff --git a/server/start.go b/server/start.go index 2d387ba92e..535a004bdf 100644 --- a/server/start.go +++ b/server/start.go @@ -60,6 +60,8 @@ const ( FlagDisableIAVLFastNode = "iavl-disable-fastnode" FlagIAVLLazyLoading = "iavl-lazy-loading" + FlagEnableUnsafeQuery = "enable-unsafe-query" + // state sync-related flags FlagStateSyncSnapshotInterval = "state-sync.snapshot-interval" FlagStateSyncSnapshotKeepRecent = "state-sync.snapshot-keep-recent" @@ -186,6 +188,7 @@ is performed. Note, when enabled, gRPC will also be automatically enabled. cmd.Flags().Uint(FlagInvCheckPeriod, 0, "Assert registered invariants every N blocks") cmd.Flags().Uint64(FlagMinRetainBlocks, 0, "Minimum block height offset during ABCI commit to prune Tendermint blocks") cmd.Flags().String(FlagEventing, sdk.EventingOptionEverything, "Eventing strategy (everything|nothing)") + cmd.Flags().Bool(FlagEnableUnsafeQuery, true, "Define if unsafe query should be enabled (unsafe - use it at your own risk)") cmd.Flags().Bool(FlagAPIEnable, false, "Define if the API server should be enabled") cmd.Flags().Bool(FlagAPISwagger, false, "Define if swagger documentation should automatically be registered (Note: the API must also be enabled)") diff --git a/server/util.go b/server/util.go index ed37b5784e..d4798cbc78 100644 --- a/server/util.go +++ b/server/util.go @@ -498,6 +498,7 @@ func DefaultBaseappOptions(appOpts types.AppOptions) []func(*baseapp.BaseApp) { ), baseapp.SetIAVLLazyLoading(cast.ToBool(appOpts.Get(FlagIAVLLazyLoading))), baseapp.SetChainID(chainID), + baseapp.SetEnableUnsafeQuery(cast.ToBool(appOpts.Get(FlagEnableUnsafeQuery))), } } diff --git a/types/context.go b/types/context.go index 87c1ae5f6a..956765fc8d 100644 --- a/types/context.go +++ b/types/context.go @@ -44,6 +44,7 @@ type Context struct { upgradeChecker func(ctx Context, name string) bool txSize uint64 // The tx bytes length sigCache *lru.ARCCache + enableUnsafeQuery bool } // Proposed rename, not done to avoid API breakage @@ -76,6 +77,10 @@ func (c Context) IsUpgraded(name string) bool { return c.upgradeChecker(c, name) } +func (c Context) IsEnableUnsafeQuery() bool { + return c.enableUnsafeQuery +} + // clone the header before returning func (c Context) BlockHeader() tmproto.Header { msg := proto.Clone(&c.header).(*tmproto.Header) @@ -279,6 +284,12 @@ func (c Context) WithSigCache(cache *lru.ARCCache) Context { return c } +// WithEnableUnsafeQuery returns a Context with unsafe query enabled +func (c Context) WithEnableUnsafeQuery(enabled bool) Context { + c.enableUnsafeQuery = enabled + return c +} + // TODO: remove??? func (c Context) IsZero() bool { return c.ms == nil diff --git a/types/query/pagination.go b/types/query/pagination.go index 730741afc5..de984dd2f8 100644 --- a/types/query/pagination.go +++ b/types/query/pagination.go @@ -9,6 +9,7 @@ import ( "google.golang.org/grpc/status" "github.com/cosmos/cosmos-sdk/store/types" + sdk "github.com/cosmos/cosmos-sdk/types" ) // DefaultPage is the default `page` number for queries. @@ -23,6 +24,14 @@ const DefaultLimit = 100 // which equals the maximum value that can be stored in uint64 const MaxLimit = math.MaxUint64 +// CheckOffsetQueryNotAllowed is used for checking query which offset/count total parameter is not allowed +func CheckOffsetQueryNotAllowed(ctx sdk.Context, req *PageRequest) error { + if req != nil && (req.Offset > 0 || req.CountTotal) && !ctx.IsEnableUnsafeQuery() { + return status.Error(codes.InvalidArgument, "query with offset/count total is not allowed") + } + return nil +} + // ParsePagination validate PageRequest and returns page number & limit. func ParsePagination(pageReq *PageRequest) (page, limit int, err error) { offset := 0 diff --git a/x/auth/keeper/grpc_query.go b/x/auth/keeper/grpc_query.go index 1fb96654d5..debd049271 100644 --- a/x/auth/keeper/grpc_query.go +++ b/x/auth/keeper/grpc_query.go @@ -45,6 +45,12 @@ func (ak AccountKeeper) Accounts(c context.Context, req *types.QueryAccountsRequ } ctx := sdk.UnwrapSDKContext(c) + + // offset query is not allowed + if err := query.CheckOffsetQueryNotAllowed(ctx, req.Pagination); err != nil { + return nil, err + } + store := ctx.KVStore(ak.storeKey) accountsStore := prefix.NewStore(store, types.AddressStoreKeyPrefix) From d884cd26d32faa2acef6f0212a3bc4c340264cd6 Mon Sep 17 00:00:00 2001 From: forcodedancing Date: Mon, 31 Jul 2023 09:51:00 +0800 Subject: [PATCH 2/4] fix ut --- x/auth/keeper/deterministic_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/x/auth/keeper/deterministic_test.go b/x/auth/keeper/deterministic_test.go index 04d4599769..306e1cce2d 100644 --- a/x/auth/keeper/deterministic_test.go +++ b/x/auth/keeper/deterministic_test.go @@ -2,6 +2,7 @@ package keeper_test import ( "encoding/hex" + "github.com/cosmos/cosmos-sdk/types/query" "sort" "testing" @@ -129,7 +130,7 @@ func (suite *DeterministicTestSuite) TestGRPCQueryAccounts() { numAccs := rapid.IntRange(1, 10).Draw(t, "accounts") accs := suite.createAndSetAccounts(t, numAccs) - req := &types.QueryAccountsRequest{Pagination: testdata.PaginationGenerator(t, uint64(numAccs)).Draw(t, "accounts")} + req := &types.QueryAccountsRequest{Pagination: &query.PageRequest{}} testdata.DeterministicIterations(suite.ctx, suite.Require(), req, suite.queryClient.Accounts, 0, true) for i := 0; i < numAccs; i++ { From 0cf9b866f2a6198c78f6519131f86eb057962a67 Mon Sep 17 00:00:00 2001 From: forcodedancing Date: Tue, 1 Aug 2023 15:25:21 +0800 Subject: [PATCH 3/4] disable query evidences using offset --- x/evidence/keeper/grpc_query.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/x/evidence/keeper/grpc_query.go b/x/evidence/keeper/grpc_query.go index 369b38eab7..ed677d1756 100644 --- a/x/evidence/keeper/grpc_query.go +++ b/x/evidence/keeper/grpc_query.go @@ -62,7 +62,9 @@ func (k Keeper) AllEvidence(c context.Context, req *types.QueryAllEvidenceReques } ctx := sdk.UnwrapSDKContext(c) - k.GetAllEvidence(ctx) + if err := query.CheckOffsetQueryNotAllowed(ctx, req.Pagination); err != nil { + return nil, err + } var evidence []*codectypes.Any store := ctx.KVStore(k.storeKey) From d981d87cd8cf4600648babb6a3454d84925566c7 Mon Sep 17 00:00:00 2001 From: forcodedancing Date: Wed, 2 Aug 2023 18:22:48 +0800 Subject: [PATCH 4/4] update the default value of EnableUnsafeQuery --- server/start.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/start.go b/server/start.go index 535a004bdf..dee5d2527f 100644 --- a/server/start.go +++ b/server/start.go @@ -188,7 +188,7 @@ is performed. Note, when enabled, gRPC will also be automatically enabled. cmd.Flags().Uint(FlagInvCheckPeriod, 0, "Assert registered invariants every N blocks") cmd.Flags().Uint64(FlagMinRetainBlocks, 0, "Minimum block height offset during ABCI commit to prune Tendermint blocks") cmd.Flags().String(FlagEventing, sdk.EventingOptionEverything, "Eventing strategy (everything|nothing)") - cmd.Flags().Bool(FlagEnableUnsafeQuery, true, "Define if unsafe query should be enabled (unsafe - use it at your own risk)") + cmd.Flags().Bool(FlagEnableUnsafeQuery, false, "Define if unsafe query should be enabled (unsafe - use it at your own risk)") cmd.Flags().Bool(FlagAPIEnable, false, "Define if the API server should be enabled") cmd.Flags().Bool(FlagAPISwagger, false, "Define if swagger documentation should automatically be registered (Note: the API must also be enabled)")