From 1d3bc0ee3052c7fdc91eac318354d7151ffa01d8 Mon Sep 17 00:00:00 2001 From: Alex Peters Date: Mon, 23 Nov 2020 12:50:31 +0100 Subject: [PATCH] Handle panics in query contract smart --- x/wasm/internal/keeper/keeper.go | 7 ++++ x/wasm/internal/keeper/querier.go | 29 ++++++++++++++--- x/wasm/internal/keeper/querier_test.go | 44 +++++++++++++++++++++++++- x/wasm/internal/keeper/recurse_test.go | 8 ++--- 4 files changed, 78 insertions(+), 10 deletions(-) diff --git a/x/wasm/internal/keeper/keeper.go b/x/wasm/internal/keeper/keeper.go index 72aae77896..aa7ce10758 100644 --- a/x/wasm/internal/keeper/keeper.go +++ b/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)) +} diff --git a/x/wasm/internal/keeper/querier.go b/x/wasm/internal/keeper/querier.go index efa1160afe..08c959a78b 100644 --- a/x/wasm/internal/keeper/querier.go +++ b/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 } diff --git a/x/wasm/internal/keeper/querier_test.go b/x/wasm/internal/keeper/querier_test.go index 11870da65a..3df85c2170 100644 --- a/x/wasm/internal/keeper/querier_test.go +++ b/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 diff --git a/x/wasm/internal/keeper/recurse_test.go b/x/wasm/internal/keeper/recurse_test.go index eddec5c1ce..4657938145 100644 --- a/x/wasm/internal/keeper/recurse_test.go +++ b/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 )