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

feat: add flag to enable or disable heavy queries #258

Merged
merged 4 commits into from
Aug 3, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion baseapp/abci.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
5 changes: 4 additions & 1 deletion baseapp/baseapp.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions baseapp/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
4 changes: 4 additions & 0 deletions server/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -279,6 +282,7 @@ func DefaultConfig() *Config {
IAVLDisableFastNode: false,
IAVLLazyLoading: false,
AppDBBackend: "",
EnableUnsafeQuery: false,
},
Telemetry: telemetry.Config{
Enabled: false,
Expand Down
4 changes: 4 additions & 0 deletions server/config/toml.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 ###
###############################################################################
Expand Down
3 changes: 3 additions & 0 deletions server/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)")
Copy link
Collaborator

Choose a reason for hiding this comment

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

The default value should be false, user need to set it to true only they know what exact it means.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed


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)")
Expand Down
1 change: 1 addition & 0 deletions server/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))),
}
}

Expand Down
11 changes: 11 additions & 0 deletions types/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down
9 changes: 9 additions & 0 deletions types/query/pagination.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand Down
3 changes: 2 additions & 1 deletion x/auth/keeper/deterministic_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package keeper_test

import (
"encoding/hex"
"github.com/cosmos/cosmos-sdk/types/query"
"sort"
"testing"

Expand Down Expand Up @@ -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++ {
Expand Down
6 changes: 6 additions & 0 deletions x/auth/keeper/grpc_query.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
4 changes: 3 additions & 1 deletion x/evidence/keeper/grpc_query.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down