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: adds error handling for out-of-gas panics in grpc query handlers. #20945

Merged
merged 10 commits into from
Jul 16, 2024
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ Every module contains its own CHANGELOG.md. Please refer to the module you are i
* (baseapp) [#20208](https://github.com/cosmos/cosmos-sdk/pull/20208) Skip running validateBasic for rechecking txs.
* (baseapp) [#20380](https://github.com/cosmos/cosmos-sdk/pull/20380) Enhanced OfferSnapshot documentation.
* (client) [#20771](https://github.com/cosmos/cosmos-sdk/pull/20771) Remove `ReadDefaultValuesFromDefaultClientConfig` from `client` package. (It was introduced in `v0.50.6` as a quick fix).
* (grpcserver) [#20945](https://github.com/cosmos/cosmos-sdk/pull/20945) Adds error handling for out-of-gas panics in grpc query handlers.

### Bug Fixes

Expand Down
13 changes: 13 additions & 0 deletions baseapp/grpcserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"google.golang.org/grpc/status"

errorsmod "cosmossdk.io/errors"
storetypes "cosmossdk.io/store/types"

sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
Expand Down Expand Up @@ -67,6 +68,18 @@ func (app *BaseApp) RegisterGRPCServer(server gogogrpc.Server) {

app.logger.Debug("gRPC query received of type: " + fmt.Sprintf("%#v", req))

// Catch an OutOfGasPanic caused in the query handlers
defer func() {
if r := recover(); r != nil {
switch rType := r.(type) {
case storetypes.ErrorOutOfGas:
err = errorsmod.Wrapf(sdkerrors.ErrOutOfGas, "Query gas limit exceeded: %v, out of gas in location: %v", sdkCtx.GasMeter().Limit(), rType.Descriptor)
default:
panic(r)
}
}
}()

return handler(grpcCtx, req)
}

Expand Down
98 changes: 98 additions & 0 deletions tests/integration/server/grpc/out_of_gas_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
package grpc_test

import (
"context"
"fmt"
"testing"

"github.com/stretchr/testify/suite"
"google.golang.org/grpc"
"google.golang.org/grpc/metadata"

_ "cosmossdk.io/x/accounts"
_ "cosmossdk.io/x/auth"
_ "cosmossdk.io/x/auth/tx/config"
_ "cosmossdk.io/x/bank"
banktypes "cosmossdk.io/x/bank/types"
_ "cosmossdk.io/x/consensus"
_ "cosmossdk.io/x/staking"

"github.com/cosmos/cosmos-sdk/codec"
"github.com/cosmos/cosmos-sdk/testutil/configurator"
"github.com/cosmos/cosmos-sdk/testutil/network"
"github.com/cosmos/cosmos-sdk/testutil/testdata"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
)

type IntegrationTestOutOfGasSuite struct {
suite.Suite

cfg network.Config
network network.NetworkI
conn *grpc.ClientConn
}

func (s *IntegrationTestOutOfGasSuite) SetupSuite() {
var err error
s.T().Log("setting up integration test suite")

s.cfg, err = network.DefaultConfigWithAppConfigWithQueryGasLimit(configurator.NewAppConfig(
configurator.AccountsModule(),
configurator.AuthModule(),
configurator.BankModule(),
configurator.GenutilModule(),
configurator.StakingModule(),
configurator.ConsensusModule(),
configurator.TxModule(),
), 10)
s.NoError(err)
s.cfg.NumValidators = 1

s.network, err = network.New(s.T(), s.T().TempDir(), s.cfg)
s.Require().NoError(err)

_, err = s.network.WaitForHeight(2)
s.Require().NoError(err)

val0 := s.network.GetValidators()[0]
s.conn, err = grpc.NewClient(
val0.GetAppConfig().GRPC.Address,
grpc.WithInsecure(), //nolint:staticcheck // ignore SA1019, we don't need to use a secure connection for tests
grpc.WithDefaultCallOptions(grpc.ForceCodec(codec.NewProtoCodec(s.cfg.InterfaceRegistry).GRPCCodec())),
)
s.Require().NoError(err)
}

func (s *IntegrationTestOutOfGasSuite) TearDownSuite() {
s.T().Log("tearing down integration test suite")
s.conn.Close()
s.network.Cleanup()
}

func (s *IntegrationTestOutOfGasSuite) TestGRPCServer_TestService() {
// gRPC query to test service should work
testClient := testdata.NewQueryClient(s.conn)
testRes, err := testClient.Echo(context.Background(), &testdata.EchoRequest{Message: "hello"})
s.Require().NoError(err)
s.Require().Equal("hello", testRes.Message)
}

func (s *IntegrationTestOutOfGasSuite) TestGRPCServer_BankBalance_OutOfGas() {
val0 := s.network.GetValidators()[0]

// gRPC query to bank service should work
denom := fmt.Sprintf("%stoken", val0.GetMoniker())
bankClient := banktypes.NewQueryClient(s.conn)
var header metadata.MD
_, err := bankClient.Balance(
context.Background(),
&banktypes.QueryBalanceRequest{Address: val0.GetAddress().String(), Denom: denom},
grpc.Header(&header), // Also fetch grpc header
)

s.Require().ErrorContains(err, sdkerrors.ErrOutOfGas.Error())
}

func TestIntegrationTestOutOfGasSuite(t *testing.T) {
suite.Run(t, new(IntegrationTestOutOfGasSuite))
}
5 changes: 5 additions & 0 deletions testutil/network/network.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,10 @@ func DefaultConfig(factory TestFixtureFactory) Config {
}

func DefaultConfigWithAppConfig(appConfig depinject.Config) (Config, error) {
return DefaultConfigWithAppConfigWithQueryGasLimit(appConfig, 0)
}

func DefaultConfigWithAppConfigWithQueryGasLimit(appConfig depinject.Config, queryGasLimit uint64) (Config, error) {
var (
appBuilder *runtime.AppBuilder
txConfig client.TxConfig
Expand Down Expand Up @@ -221,6 +225,7 @@ func DefaultConfigWithAppConfig(appConfig depinject.Config) (Config, error) {
baseapp.SetPruning(pruningtypes.NewPruningOptionsFromString(val.GetAppConfig().Pruning)),
baseapp.SetMinGasPrices(val.GetAppConfig().MinGasPrices),
baseapp.SetChainID(cfg.ChainID),
baseapp.SetQueryGasLimit(queryGasLimit),
)

testdata.RegisterQueryServer(app.GRPCQueryRouter(), testdata.QueryImpl{})
Expand Down
Loading