Add 30M gas limit to sudo helper (#7527)

* add 30M gas limit to sudo helper

* add changelog

* ensure existing lower limit is not overridden

* using min, which is allowed now that we support go 1.21

* start implementing tests

* catch panics and add tests

* clean up test cases

* change error return to generic default and clean up tests

---------

Co-authored-by: Nicolas Lara <nicolaslara@gmail.com>

Author: AlpinYukseloglu

CHANGELOG.md

@@ -75,6 +75,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 * [#7768](https://github.com/osmosis-labs/osmosis/pull/7768) Allow governance module account to transfer any CL position
 * [#7746](https://github.com/osmosis-labs/osmosis/pull/7746) Make forfeited incentives redeposit into the pool instead of sending to community pool
 * [#7785](https://github.com/osmosis-labs/osmosis/pull/7785) Remove reward claiming during position transfers
+* [#7527](https://github.com/osmosis-labs/osmosis/pull/7527) Add 30M gas limit to CW pool contract calls
 
 ### SDK
 

osmoutils/cosmwasm/helpers.go

@@ -2,11 +2,14 @@ package cosmwasm
 
 import (
 	"encoding/json"
+	"fmt"
 
 	storetypes "github.com/cosmos/cosmos-sdk/store/types"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 )
 
+const DefaultContractCallGasLimit = 30_000_000
+
 // ContracKeeper defines the interface needed to be fulfilled for
 // the ContractKeeper.
 type ContractKeeper interface {
@@ -118,11 +121,26 @@ func Sudo[T any, K any](ctx sdk.Context, contractKeeper ContractKeeper, contract
 		return response, err
 	}
 
-	responseBz, err := contractKeeper.Sudo(ctx, sdk.MustAccAddressFromBech32(contractAddress), bz)
+	// Defer to catch panics in case the sudo call runs out of gas.
+	defer func() {
+		if r := recover(); r != nil {
+			var emptyResponse K
+			response = emptyResponse
+			err = fmt.Errorf("contract call ran out of gas")
+		}
+	}()
+
+	// Make contract call with a gas limit of 30M to ensure contracts cannot run unboundedly
+	gasLimit := min(ctx.GasMeter().Limit(), DefaultContractCallGasLimit)
+	childCtx := ctx.WithGasMeter(sdk.NewGasMeter(gasLimit))
+	responseBz, err := contractKeeper.Sudo(childCtx, sdk.MustAccAddressFromBech32(contractAddress), bz)
 	if err != nil {
 		return response, err
 	}
 
+	// Consume gas used for calling contract to the parent ctx
+	ctx.GasMeter().ConsumeGas(childCtx.GasMeter().GasConsumed(), "Track contract call gas")
+
 	// valid empty response
 	if len(responseBz) == 0 {
 		return response, nil

osmoutils/cosmwasm/helpers_test.go

@@ -0,0 +1,108 @@
+package cosmwasm_test
+
+import (
+	"fmt"
+	"os"
+	"testing"
+
+	wasmkeeper "github.com/CosmWasm/wasmd/x/wasm/keeper"
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	"github.com/stretchr/testify/suite"
+
+	"github.com/osmosis-labs/osmosis/osmoutils/cosmwasm"
+	"github.com/osmosis-labs/osmosis/v23/app/apptesting"
+)
+
+type KeeperTestSuite struct {
+	apptesting.KeeperTestHelper
+}
+
+func TestKeeperTestSuite(t *testing.T) {
+	suite.Run(t, new(KeeperTestSuite))
+}
+
+func (s *KeeperTestSuite) TestSudoGasLimit() {
+	// Skip test if there is system-side incompatibility
+	s.SkipIfWSL()
+
+	// We use contracts already defined in existing modules to avoid duplicate test contract code.
+	// This is a simple counter contract that counts `Amount` times and does a state write on each iteration.
+	// Source code can be found in x/concentrated-liquidity/testcontracts/contract-sources
+	counterContractPath := "../../x/concentrated-liquidity/testcontracts/compiled-wasm/counter.wasm"
+
+	// Message structs for the test CW contract
+	type CountMsg struct {
+		Amount int64 `json:"amount"`
+	}
+	type CountMsgResponse struct {
+	}
+	type CountSudoMsg struct {
+		Count CountMsg `json:"count"`
+	}
+
+	tests := map[string]struct {
+		wasmFile      string
+		msg           CountSudoMsg
+		noContractSet bool
+
+		expectedError error
+	}{
+		"contract consumes less than limit": {
+			wasmFile: counterContractPath,
+			msg: CountSudoMsg{
+				Count: CountMsg{
+					// Consumes roughly 100k gas, which should be comfortably under the limit.
+					Amount: 10,
+				},
+			},
+		},
+		"contract that consumes more than limit": {
+			wasmFile: counterContractPath,
+			msg: CountSudoMsg{
+				Count: CountMsg{
+					// Consumes roughly 1B gas, which is well above the 30M limit.
+					Amount: 100000,
+				},
+			},
+			expectedError: fmt.Errorf("contract call ran out of gas"),
+		},
+	}
+	for name, tc := range tests {
+		s.Run(name, func() {
+			s.Setup()
+
+			// We use a gov permissioned contract keeper to avoid having to manually set permissions
+			contractKeeper := wasmkeeper.NewGovPermissionKeeper(s.App.WasmKeeper)
+
+			// Upload and instantiate wasm code
+			_, cosmwasmAddressBech32 := s.uploadAndInstantiateContract(contractKeeper, tc.wasmFile)
+
+			// System under test
+			response, err := cosmwasm.Sudo[CountSudoMsg, CountMsgResponse](s.Ctx, contractKeeper, cosmwasmAddressBech32, tc.msg)
+
+			if tc.expectedError != nil {
+				s.Require().ErrorContains(err, tc.expectedError.Error())
+				return
+			}
+
+			s.Require().NoError(err)
+			s.Require().Equal(CountMsgResponse{}, response)
+		})
+	}
+}
+
+// uploadAndInstantiateContract is a helper function to upload and instantiate a contract from a given file path.
+// It calls an empty Instantiate message on the created contract and returns the bech32 address after instantiation.
+func (s *KeeperTestSuite) uploadAndInstantiateContract(contractKeeper *wasmkeeper.PermissionedKeeper, filePath string) (rawCWAddr sdk.AccAddress, bech32CWAddr string) {
+	// Upload and instantiate wasm code
+	wasmCode, err := os.ReadFile(filePath)
+	s.Require().NoError(err)
+	codeID, _, err := contractKeeper.Create(s.Ctx, s.TestAccs[0], wasmCode, nil)
+	s.Require().NoError(err)
+	rawCWAddr, _, err = contractKeeper.Instantiate(s.Ctx, codeID, s.TestAccs[0], s.TestAccs[0], []byte("{}"), "", sdk.NewCoins())
+	s.Require().NoError(err)
+	bech32CWAddr, err = sdk.Bech32ifyAddressBytes("osmo", rawCWAddr)
+	s.Require().NoError(err)
+
+	return rawCWAddr, bech32CWAddr
+}