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

Handle panics in query contract smart #322

Merged
merged 1 commit into from
Nov 30, 2020
Merged
Show file tree
Hide file tree
Changes from all 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
7 changes: 7 additions & 0 deletions x/wasm/internal/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package keeper
import (
"bytes"
"encoding/binary"
"fmt"
"path/filepath"

"github.com/CosmWasm/wasmd/x/wasm/internal/types"
Expand All @@ -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
Expand Down Expand Up @@ -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))
}
29 changes: 25 additions & 4 deletions x/wasm/internal/keeper/querier.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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

}

Expand Down
44 changes: 43 additions & 1 deletion x/wasm/internal/keeper/querier_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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
}
Expand All @@ -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
Expand Down
8 changes: 3 additions & 5 deletions x/wasm/internal/keeper/recurse_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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
)
Expand Down