Merge pull request cosmos#322 from CosmWasm/handle_oog_321

Handle panics in query contract smart

Author: alpe

x/wasm/internal/keeper/keeper.go

@@ -3,6 +3,7 @@ package keeper
 import (
 	"bytes"
 	"encoding/binary"
+	"fmt"
 	"path/filepath"
 
 	"github.com/CosmWasm/wasmd/x/wasm/internal/types"
@@ -18,6 +19,7 @@ import (
 	paramtypes "github.com/cosmos/cosmos-sdk/x/params/types"
 	stakingkeeper "github.com/cosmos/cosmos-sdk/x/staking/keeper"
 	"github.com/tendermint/tendermint/crypto"
+	"github.com/tendermint/tendermint/libs/log"
 )
 
 // GasMultiplier is how many cosmwasm gas points = 1 sdk gas point
@@ -710,3 +712,8 @@ func gasMeter(ctx sdk.Context) MultipiedGasMeter {
 		originalMeter: ctx.GasMeter(),
 	}
 }
+
+// Logger returns a module-specific logger.
+func (k Keeper) Logger(ctx sdk.Context) log.Logger {
+	return ctx.Logger().With("module", fmt.Sprintf("x/%s", types.ModuleName))
+}

x/wasm/internal/keeper/querier.go

@@ -3,6 +3,7 @@ package keeper
 import (
 	"context"
 	"encoding/binary"
+	"runtime/debug"
 
 	"github.com/CosmWasm/wasmd/x/wasm/internal/types"
 	"github.com/cosmos/cosmos-sdk/store/prefix"
@@ -146,21 +147,41 @@ func (q grpcQuerier) RawContractState(c context.Context, req *types.QueryRawCont
 	return &types.QueryRawContractStateResponse{Data: rsp}, nil
 }
 
-func (q grpcQuerier) SmartContractState(c context.Context, req *types.QuerySmartContractStateRequest) (*types.QuerySmartContractStateResponse, error) {
+func (q grpcQuerier) SmartContractState(c context.Context, req *types.QuerySmartContractStateRequest) (rsp *types.QuerySmartContractStateResponse, err error) {
 	contractAddr, err := sdk.AccAddressFromBech32(req.Address)
 	if err != nil {
 		return nil, err
 	}
 	ctx := sdk.UnwrapSDKContext(c).WithGasMeter(sdk.NewGasMeter(q.keeper.queryGasLimit))
+	// recover from out-of-gas panic
+	defer func() {
+		if r := recover(); r != nil {
+			switch rType := r.(type) {
+			case sdk.ErrorOutOfGas:
+				err = sdkerrors.Wrapf(sdkerrors.ErrOutOfGas,
+					"out of gas in location: %v; gasWanted: %d, gasUsed: %d",
+					rType.Descriptor, ctx.GasMeter().Limit(), ctx.GasMeter().GasConsumed(),
+				)
+			default:
+				err = sdkerrors.ErrPanic
+			}
+			rsp = nil
+			q.keeper.Logger(ctx).
+				Debug("smart query contract",
+					"error", "recovering panic",
+					"contract-address", req.Address,
+					"stacktrace", string(debug.Stack()))
+		}
+	}()
 
-	rsp, err := q.keeper.QuerySmart(ctx, contractAddr, req.QueryData)
+	bz, err := q.keeper.QuerySmart(ctx, contractAddr, req.QueryData)
 	switch {
 	case err != nil:
 		return nil, err
-	case rsp == nil:
+	case bz == nil:
 		return nil, types.ErrNotFound
 	}
-	return &types.QuerySmartContractStateResponse{Data: rsp}, nil
+	return &types.QuerySmartContractStateResponse{Data: bz}, nil
 
 }
 

x/wasm/internal/keeper/querier_test.go

@@ -8,11 +8,14 @@ import (
 	"testing"
 
 	"github.com/CosmWasm/wasmd/x/wasm/internal/types"
+	"github.com/CosmWasm/wasmvm"
+	wasmvmtypes "github.com/CosmWasm/wasmvm/types"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	sdkErrors "github.com/cosmos/cosmos-sdk/types/errors"
 	"github.com/cosmos/cosmos-sdk/types/query"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"github.com/tendermint/tendermint/libs/log"
 )
 
 func TestQueryAllContractState(t *testing.T) {
@@ -136,7 +139,7 @@ func TestQuerySmartContractState(t *testing.T) {
 	for msg, spec := range specs {
 		t.Run(msg, func(t *testing.T) {
 			got, err := q.SmartContractState(sdk.WrapSDKContext(ctx), spec.srcQuery)
-			require.True(t, spec.expErr.Is(err), err)
+			require.True(t, spec.expErr.Is(err), "but got %+v", err)
 			if spec.expErr != nil {
 				return
 			}
@@ -145,6 +148,45 @@ func TestQuerySmartContractState(t *testing.T) {
 	}
 }
 
+func TestQuerySmartContractPanics(t *testing.T) {
+	ctx, keepers := CreateTestInput(t, false, SupportedFeatures, nil, nil)
+	exampleContract := InstantiateHackatomExampleContract(t, ctx, keepers)
+	ctx = ctx.WithGasMeter(sdk.NewGasMeter(InstanceCost)).WithLogger(log.TestingLogger())
+
+	specs := map[string]struct {
+		doInContract func()
+		expErr       *sdkErrors.Error
+	}{
+		"out of gas": {
+			doInContract: func() {
+				ctx.GasMeter().ConsumeGas(ctx.GasMeter().Limit()+1, "test - consume more than limit")
+			},
+			expErr: sdkErrors.ErrOutOfGas,
+		},
+		"other panic": {
+			doInContract: func() {
+				panic("my panic")
+			},
+			expErr: sdkErrors.ErrPanic,
+		},
+	}
+	for msg, spec := range specs {
+		t.Run(msg, func(t *testing.T) {
+			keepers.WasmKeeper.wasmer = &MockWasmer{QueryFn: func(code cosmwasm.CodeID, env wasmvmtypes.Env, queryMsg []byte, store cosmwasm.KVStore, goapi cosmwasm.GoAPI, querier cosmwasm.Querier, gasMeter cosmwasm.GasMeter, gasLimit uint64) ([]byte, uint64, error) {
+				spec.doInContract()
+				return nil, 0, nil
+			}}
+			// when
+			q := NewQuerier(keepers.WasmKeeper)
+			got, err := q.SmartContractState(sdk.WrapSDKContext(ctx), &types.QuerySmartContractStateRequest{
+				Address: exampleContract.Contract.String(),
+			})
+			require.True(t, spec.expErr.Is(err), "got error: %+v", err)
+			assert.Nil(t, got)
+		})
+	}
+}
+
 func TestQueryRawContractState(t *testing.T) {
 	ctx, keepers := CreateTestInput(t, false, SupportedFeatures, nil, nil)
 	keeper := keepers.WasmKeeper

x/wasm/internal/keeper/recurse_test.go

@@ -55,11 +55,10 @@ func initRecurseContract(t *testing.T) (contract sdk.AccAddress, creator sdk.Acc
 }
 
 func TestGasCostOnQuery(t *testing.T) {
-	t.Skip("Alex: enable later when the model + gas costs become clear")
 	const (
-		GasNoWork uint64 = InstanceCost + 2_938
+		GasNoWork uint64 = 43229
 		// Note: about 100 SDK gas (10k wasmer gas) for each round of sha256
-		GasWork50 uint64 = 48646 // this is a little shy of 50k gas - to keep an eye on the limit
+		GasWork50 uint64 = 48937 // this is a little shy of 50k gas - to keep an eye on the limit
 
 		GasReturnUnhashed uint64 = 393
 		GasReturnHashed   uint64 = 342
@@ -214,7 +213,6 @@ func TestGasOnExternalQuery(t *testing.T) {
 }
 
 func TestLimitRecursiveQueryGas(t *testing.T) {
-	t.Skip("Alex: enable later when the model + gas costs become clear")
 	// The point of this test from https://github.com/CosmWasm/cosmwasm/issues/456
 	// Basically, if I burn 90% of gas in CPU loop, then query out (to my self)
 	// the sub-query will have all the original gas (minus the 40k instance charge)
@@ -224,7 +222,7 @@ func TestLimitRecursiveQueryGas(t *testing.T) {
 
 	const (
 		// Note: about 100 SDK gas (10k wasmer gas) for each round of sha256
-		GasWork2k uint64 = 273_560 // = InstanceCost + x // we have 6x gas used in cpu than in the instance
+		GasWork2k uint64 = 273_851 // = InstanceCost + x // we have 6x gas used in cpu than in the instance
 		// This is overhead for calling into a sub-contract
 		GasReturnHashed uint64 = 349
 	)