[Bugfix] limit wasm max gas with param

x/wasm/keeper/api.go

@@ -59,9 +59,6 @@ func (k Keeper) getGasRemaining(ctx sdk.Context) uint64 {
 	}
 
 	remaining := (meter.Limit() - meter.GasConsumed())
-	if maxGas := k.MaxContractGas(ctx); remaining > maxGas {
-		remaining = maxGas
-	}
 	return remaining * types.GasMultiplier
 }
 

x/wasm/keeper/connector.go

@@ -105,8 +105,7 @@ func (k Keeper) dispatchMessages(ctx sdk.Context, contractAddr sdk.AccAddress, m
 
 // dispatchMessageWithGasLimit does not emit events to prevent duplicate emission
 func (k Keeper) dispatchMessageWithGasLimit(ctx sdk.Context, contractAddr sdk.AccAddress, msg wasmvmtypes.CosmosMsg, gasLimit uint64) (events sdk.Events, data []byte, err error) {
-	limitedMeter := sdk.NewGasMeter(gasLimit)
-	subCtx := ctx.WithGasMeter(limitedMeter)
+	subCtx := ctx.WithGasMeter(sdk.NewGasMeter(gasLimit))
 
 	// catch out of gas panic and just charge the entire gas limit
 	defer func() {

x/wasm/keeper/contract.go

@@ -155,8 +155,8 @@ func (k Keeper) InstantiateContract(
 		k.getGasRemaining(ctx),
 	)
 
-	// consume gas before raise error
-	k.consumeGas(ctx, gasUsed, "Contract init")
+	// add types.GasMultiplier to occur out of gas panic
+	k.consumeGas(ctx, gasUsed+types.GasMultiplier, "Contract init")
 	if err != nil {
 		return nil, nil, sdkerrors.Wrap(types.ErrInstantiateFailed, err.Error())
 	}
@@ -230,7 +230,8 @@ func (k Keeper) ExecuteContract(
 		k.getGasRemaining(ctx),
 	)
 
-	k.consumeGas(ctx, gasUsed, "Contract Execution")
+	// add types.GasMultiplier to occur out of gas panic
+	k.consumeGas(ctx, gasUsed+types.GasMultiplier, "Contract Execution")
 	if err != nil {
 		return nil, sdkerrors.Wrap(types.ErrExecuteFailed, err.Error())
 	}
@@ -306,7 +307,8 @@ func (k Keeper) MigrateContract(
 		k.getGasRemaining(ctx),
 	)
 
-	k.consumeGas(ctx, gasUsed, "Contract Migration")
+	// add types.GasMultiplier to occur out of gas panic
+	k.consumeGas(ctx, gasUsed+types.GasMultiplier, "Contract Migration")
 	if err != nil {
 		return nil, sdkerrors.Wrap(types.ErrMigrationFailed, err.Error())
 	}
@@ -367,9 +369,10 @@ func (k Keeper) reply(
 		k.getGasRemaining(ctx),
 	)
 
-	k.consumeGas(ctx, gasUsed, "Contract Reply")
+	// add types.GasMultiplier to occur out of gas panic
+	k.consumeGas(ctx, gasUsed+types.GasMultiplier, "Contract Reply")
 	if err != nil {
-		return sdkerrors.Wrap(types.ErrExecuteFailed, err.Error())
+		return sdkerrors.Wrap(types.ErrReplyFailed, err.Error())
 	}
 
 	// vaildate events is size and parse to sdk events
@@ -427,9 +430,10 @@ func (k Keeper) queryToContract(ctx sdk.Context, contractAddress sdk.AccAddress,
 		k.getGasRemaining(ctx),
 	)
 
-	k.consumeGas(ctx, gasUsed, "Contract Query")
+	// add types.GasMultiplier to occur out of gas panic
+	k.consumeGas(ctx, gasUsed+types.GasMultiplier, "Contract Query")
 	if err != nil {
-		err = sdkerrors.Wrap(types.ErrContractQueryFailed, err.Error())
+		return nil, sdkerrors.Wrap(types.ErrContractQueryFailed, err.Error())
 	}
 
 	return queryResult, err

x/wasm/keeper/msg_server.go

@@ -97,9 +97,15 @@ func (k msgServer) InstantiateContract(goCtx context.Context, msg *types.MsgInst
 		}
 	}
 
-	eventManager := sdk.NewEventManager()
+	maxGas := k.MaxContractGas(ctx)
+	remain := ctx.GasMeter().Limit() - ctx.GasMeter().GasConsumed()
+	if remain > maxGas {
+		remain = maxGas
+	}
+
+	subCtx := ctx.WithEventManager(sdk.NewEventManager()).WithGasMeter(sdk.NewGasMeter(remain))
 	contractAddr, data, err := k.Keeper.InstantiateContract(
-		ctx.WithEventManager(eventManager),
+		subCtx,
 		msg.CodeID,
 		senderAddr,
 		adminAddr,
@@ -110,6 +116,9 @@ func (k msgServer) InstantiateContract(goCtx context.Context, msg *types.MsgInst
 		return nil, err
 	}
 
+	// consume gas used from wasm execution
+	ctx.GasMeter().ConsumeGas(subCtx.GasMeter().GasConsumed(), "wasm vm execute")
+
 	// prepend the event to keep the events order
 	ctx.EventManager().EmitEvents(
 		sdk.Events{
@@ -125,7 +134,7 @@ func (k msgServer) InstantiateContract(goCtx context.Context, msg *types.MsgInst
 				sdk.NewAttribute(sdk.AttributeKeyModule, types.AttributeValueCategory),
 				sdk.NewAttribute(sdk.AttributeKeySender, msg.Sender),
 			),
-		}.AppendEvents(eventManager.Events()),
+		}.AppendEvents(subCtx.EventManager().Events()),
 	)
 
 	return &types.MsgInstantiateContractResponse{
@@ -147,9 +156,15 @@ func (k msgServer) ExecuteContract(goCtx context.Context, msg *types.MsgExecuteC
 		return nil, err
 	}
 
-	eventManager := sdk.NewEventManager()
+	maxGas := k.MaxContractGas(ctx)
+	remain := ctx.GasMeter().Limit() - ctx.GasMeter().GasConsumed()
+	if remain > maxGas {
+		remain = maxGas
+	}
+
+	subCtx := ctx.WithEventManager(sdk.NewEventManager()).WithGasMeter(sdk.NewGasMeter(remain))
 	data, err := k.Keeper.ExecuteContract(
-		ctx.WithEventManager(eventManager),
+		subCtx,
 		contractAddr,
 		senderAddr,
 		msg.ExecuteMsg,
@@ -159,6 +174,9 @@ func (k msgServer) ExecuteContract(goCtx context.Context, msg *types.MsgExecuteC
 		return nil, err
 	}
 
+	// consume gas used from wasm execution
+	ctx.GasMeter().ConsumeGas(subCtx.GasMeter().GasConsumed(), "wasm vm execute")
+
 	// prepend the event to keep the events order
 	ctx.EventManager().EmitEvents(
 		sdk.Events{
@@ -172,7 +190,7 @@ func (k msgServer) ExecuteContract(goCtx context.Context, msg *types.MsgExecuteC
 				sdk.NewAttribute(sdk.AttributeKeyModule, types.AttributeValueCategory),
 				sdk.NewAttribute(sdk.AttributeKeySender, msg.Sender),
 			),
-		}.AppendEvents(eventManager.Events()),
+		}.AppendEvents(subCtx.EventManager().Events()),
 	)
 
 	return &types.MsgExecuteContractResponse{
@@ -193,9 +211,15 @@ func (k msgServer) MigrateContract(goCtx context.Context, msg *types.MsgMigrateC
 		return nil, err
 	}
 
-	eventManager := sdk.NewEventManager()
+	maxGas := k.MaxContractGas(ctx)
+	remain := ctx.GasMeter().Limit() - ctx.GasMeter().GasConsumed()
+	if remain > maxGas {
+		remain = maxGas
+	}
+
+	subCtx := ctx.WithEventManager(sdk.NewEventManager()).WithGasMeter(sdk.NewGasMeter(remain))
 	data, err := k.Keeper.MigrateContract(
-		ctx.WithEventManager(eventManager),
+		subCtx,
 		contractAddr,
 		adminAddr,
 		msg.NewCodeID,
@@ -205,6 +229,9 @@ func (k msgServer) MigrateContract(goCtx context.Context, msg *types.MsgMigrateC
 		return nil, err
 	}
 
+	// consume gas used from wasm execution
+	ctx.GasMeter().ConsumeGas(subCtx.GasMeter().GasConsumed(), "wasm vm execute")
+
 	// prepend the event to keep the events order
 	ctx.EventManager().EmitEvents(
 		sdk.Events{
@@ -218,7 +245,7 @@ func (k msgServer) MigrateContract(goCtx context.Context, msg *types.MsgMigrateC
 				sdk.NewAttribute(sdk.AttributeKeyModule, types.AttributeValueCategory),
 				sdk.NewAttribute(sdk.AttributeKeySender, msg.Admin),
 			),
-		}.AppendEvents(eventManager.Events()),
+		}.AppendEvents(subCtx.EventManager().Events()),
 	)
 
 	return &types.MsgMigrateContractResponse{

x/wasm/keeper/msg_server_test.go

@@ -0,0 +1,116 @@
+package keeper
+
+import (
+	"encoding/json"
+	"io/ioutil"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	core "github.com/terra-money/core/types"
+	"github.com/terra-money/core/x/wasm/types"
+
+	sdk "github.com/cosmos/cosmos-sdk/types"
+)
+
+func TestInstantiateExceedMaxGas(t *testing.T) {
+	input := CreateTestInput(t)
+	ctx, accKeeper, bankKeeper, keeper := input.Ctx, input.AccKeeper, input.BankKeeper, input.WasmKeeper
+
+	deposit := sdk.NewCoins(sdk.NewInt64Coin(core.MicroLunaDenom, 100000))
+	creator := createFakeFundedAccount(ctx, accKeeper, bankKeeper, deposit)
+
+	wasmCode, err := ioutil.ReadFile("./testdata/hackatom.wasm")
+	require.NoError(t, err)
+
+	codeID, err := keeper.StoreCode(ctx, creator, wasmCode)
+	require.NoError(t, err)
+
+	_, _, bob := keyPubAddr()
+	_, _, fred := keyPubAddr()
+
+	initMsg := HackatomExampleInitMsg{
+		Verifier:    fred,
+		Beneficiary: bob,
+	}
+
+	initMsgBz, err := json.Marshal(initMsg)
+	require.NoError(t, err)
+
+	// must panic
+	require.Panics(t, func() {
+		params := keeper.GetParams(ctx)
+		params.MaxContractGas = types.InstanceCost + 1
+		keeper.SetParams(ctx, params)
+		NewMsgServerImpl(keeper).InstantiateContract(ctx.Context(), types.NewMsgInstantiateContract(creator, sdk.AccAddress{}, codeID, initMsgBz, nil))
+	})
+}
+
+func TestExecuteExceedMaxGas(t *testing.T) {
+	input := CreateTestInput(t)
+	ctx, accKeeper, bankKeeper, keeper := input.Ctx, input.AccKeeper, input.BankKeeper, input.WasmKeeper
+
+	deposit := sdk.NewCoins(sdk.NewInt64Coin(core.MicroLunaDenom, 100000))
+	creator := createFakeFundedAccount(ctx, accKeeper, bankKeeper, deposit)
+
+	wasmCode, err := ioutil.ReadFile("./testdata/hackatom.wasm")
+	require.NoError(t, err)
+
+	codeID, err := keeper.StoreCode(ctx, creator, wasmCode)
+	require.NoError(t, err)
+
+	_, _, bob := keyPubAddr()
+	_, _, fred := keyPubAddr()
+
+	initMsg := HackatomExampleInitMsg{
+		Verifier:    fred,
+		Beneficiary: bob,
+	}
+
+	initMsgBz, err := json.Marshal(initMsg)
+	require.NoError(t, err)
+
+	addr, _, err := keeper.InstantiateContract(ctx, codeID, creator, sdk.AccAddress{}, initMsgBz, nil)
+
+	// must panic
+	require.Panics(t, func() {
+		params := keeper.GetParams(ctx)
+		params.MaxContractGas = types.InstanceCost + 1
+		keeper.SetParams(ctx, params)
+		NewMsgServerImpl(keeper).ExecuteContract(ctx.Context(), types.NewMsgExecuteContract(creator, addr, []byte(`{"release":{}}`), nil))
+	})
+}
+
+func TestMigrateExceedMaxGas(t *testing.T) {
+	input := CreateTestInput(t)
+	ctx, accKeeper, bankKeeper, keeper := input.Ctx, input.AccKeeper, input.BankKeeper, input.WasmKeeper
+
+	deposit := sdk.NewCoins(sdk.NewInt64Coin(core.MicroLunaDenom, 100000))
+	creator := createFakeFundedAccount(ctx, accKeeper, bankKeeper, deposit)
+
+	wasmCode, err := ioutil.ReadFile("./testdata/hackatom.wasm")
+	require.NoError(t, err)
+
+	codeID, err := keeper.StoreCode(ctx, creator, wasmCode)
+	require.NoError(t, err)
+
+	_, _, bob := keyPubAddr()
+	_, _, fred := keyPubAddr()
+
+	initMsg := HackatomExampleInitMsg{
+		Verifier:    fred,
+		Beneficiary: bob,
+	}
+
+	initMsgBz, err := json.Marshal(initMsg)
+	require.NoError(t, err)
+
+	addr, _, err := keeper.InstantiateContract(ctx, codeID, creator, sdk.AccAddress{}, initMsgBz, nil)
+
+	// must panic
+	require.Panics(t, func() {
+		params := keeper.GetParams(ctx)
+		params.MaxContractGas = types.InstanceCost + 1
+		keeper.SetParams(ctx, params)
+		NewMsgServerImpl(keeper).MigrateContract(ctx.Context(), types.NewMsgMigrateContract(creator, addr, codeID, []byte(`{"release":{}}`)))
+	})
+}

x/wasm/keeper/recursive_test.go

@@ -95,7 +95,7 @@ func initRecurseContract(t *testing.T) (contract sdk.AccAddress, creator sdk.Acc
 }
 
 func TestGasCostOnQuery(t *testing.T) {
-	GasNoWork := types.InstanceCost + 4_372
+	GasNoWork := types.InstanceCost + 3_343
 	// Note: about 100 SDK gas (10k wasmVM gas) for each round of sha256
 	GasWork50 := GasNoWork + 5_692 // this is a little shy of 50k gas - to keep an eye on the limit
 
@@ -182,14 +182,14 @@ func TestGasCostOnQuery(t *testing.T) {
 }
 
 func TestGasOnExternalQuery(t *testing.T) {
-	GasNoWork := types.InstanceCost + 4_372
+	GasNoWork := types.InstanceCost + 3_343
 	// Note: about 100 SDK gas (10k wasmVM gas) for each round of sha256
 	GasWork50 := GasNoWork + 5_692 // this is a little shy of 50k gas - to keep an eye on the limit
 
 	cases := map[string]struct {
-		gasLimit    uint64
-		msg         Recurse
-		expectPanic bool
+		gasLimit       uint64
+		msg            Recurse
+		expectOutOfGas bool
 	}{
 		"no recursion, plenty gas": {
 			gasLimit: 400_000,
@@ -210,7 +210,7 @@ func TestGasOnExternalQuery(t *testing.T) {
 			msg: Recurse{
 				Work: 50,
 			},
-			expectPanic: true,
+			expectOutOfGas: true,
 		},
 		"recursion 4, external gas limit": {
 			// this uses 244708 gas but give less
@@ -219,7 +219,7 @@ func TestGasOnExternalQuery(t *testing.T) {
 				Depth: 4,
 				Work:  50,
 			},
-			expectPanic: true,
+			expectOutOfGas: true,
 		},
 	}
 
@@ -239,7 +239,7 @@ func TestGasOnExternalQuery(t *testing.T) {
 			bz, err := cdc.MarshalJSON(types.NewQueryContractParams(contractAddr, msg))
 			require.NoError(t, err)
 
-			if tc.expectPanic {
+			if tc.expectOutOfGas {
 				_, err = querier(ctx, []string{types.QueryContractStore}, abci.RequestQuery{Data: []byte(bz)})
 				require.Error(t, err)
 				require.Contains(t, err.Error(), sdkerror.ErrOutOfGas.Error())
@@ -260,7 +260,7 @@ func TestLimitRecursiveQueryGas(t *testing.T) {
 	// This attack would allow us to use far more than the provided gas before
 	// eventually hitting an OutOfGas panic.
 
-	GasNoWork := types.InstanceCost + 4_372
+	GasNoWork := types.InstanceCost + 3_343
 	GasWork2k := GasNoWork + 229_495
 
 	// This is overhead for calling into a sub-contract