fix(evm): consume gas in CallContractWithInput (#2180)

* fix(evm): consume gas in CallContractWithInput * Update CHANGELOG.md * fix: gas consumption in ApplyEvmMsg * fix: move commit after gas refund calculation so that we can return a valid evmResp * refactor: GasToRefund * fix: proper checking of err pointer nil Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * fix: remove fullRefundLeftoverGas * test: add gas assertions * Update gas_fees.go --------- Co-authored-by: Oleg Nikonychev <[email protected]> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
NibiruChain · Jan 28, 2025 · 1ddbc2a · 1ddbc2a
1 parent bc29b88
commit 1ddbc2a
Show file tree

Hide file tree

Showing 12 changed files with 231 additions and 188 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -88,6 +88,7 @@ needed to include double quotes around the hexadecimal string.
 - [#2173](https://github.com/NibiruChain/nibiru/pull/2173) - fix(evm): clear `StateDB` between calls
 - [#2177](https://github.com/NibiruChain/nibiru/pull/2177) - fix(cmd): Continue from #2127 and unwire vesting flags and logic from genaccounts.go
 - [#2176](https://github.com/NibiruChain/nibiru/pull/2176) - tests(evm): add dirty state tests from code4rena audit
+- [#2180](https://github.com/NibiruChain/nibiru/pull/2180) - fix(evm): apply gas consumption across the entire EVM codebase at `CallContractWithInput`
 
 #### Nibiru EVM | Before Audit 2 - 2024-12-06
 

diff --git a/x/evm/keeper/call_contract.go b/x/evm/keeper/call_contract.go
@@ -39,7 +39,7 @@ func (k Keeper) CallContractWithInput(
 ) (evmResp *evm.MsgEthereumTxResponse, err error) {
 	// This is a `defer` pattern to add behavior that runs in the case that the
 	// error is non-nil, creating a concise way to add extra information.
-	defer HandleOutOfGasPanic(&err, "CallContractError")
+	defer HandleOutOfGasPanic(&err, "CallContractError")()
 	nonce := k.GetAccNonce(ctx, fromAcc)
 
 	unusedBigInt := big.NewInt(0)
@@ -61,11 +61,13 @@ func (k Keeper) CallContractWithInput(
 	// sent by a user
 	txConfig := k.TxConfig(ctx, gethcommon.BigToHash(big.NewInt(0)))
 	evmResp, err = k.ApplyEvmMsg(
-		ctx, evmMsg, evmObj, evm.NewNoOpTracer(), commit, txConfig.TxHash, true,
+		ctx, evmMsg, evmObj, evm.NewNoOpTracer(), commit, txConfig.TxHash,
 	)
+	if evmResp != nil {
+		ctx.GasMeter().ConsumeGas(evmResp.GasUsed, "CallContractWithInput")
+	}
 	if err != nil {
-		err = errors.Wrap(err, "failed to apply ethereum core message")
-		return
+		return nil, errors.Wrap(err, "failed to apply ethereum core message")
 	}
 
 	if evmResp.Failed() {

diff --git a/x/evm/keeper/erc20.go b/x/evm/keeper/erc20.go
@@ -196,7 +196,7 @@ func (e erc20Calls) loadERC20String(
 	if err != nil {
 		return out, err
 	}
-	res, err := e.Keeper.CallContractWithInput(
+	evmResp, err := e.Keeper.CallContractWithInput(
 		ctx,
 		evmObj,
 		evm.EVM_MODULE_ADDRESS,
@@ -211,13 +211,13 @@ func (e erc20Calls) loadERC20String(
 
 	erc20Val := new(ERC20String)
 	if err := erc20Abi.UnpackIntoInterface(
-		erc20Val, methodName, res.Ret,
+		erc20Val, methodName, evmResp.Ret,
 	); err == nil {
 		return erc20Val.Value, err
 	}
 
 	erc20Bytes32Val := new(ERC20Bytes32)
-	if err := erc20Abi.UnpackIntoInterface(erc20Bytes32Val, methodName, res.Ret); err == nil {
+	if err := erc20Abi.UnpackIntoInterface(erc20Bytes32Val, methodName, evmResp.Ret); err == nil {
 		return bytes32ToString(erc20Bytes32Val.Value), nil
 	}
 
@@ -239,7 +239,7 @@ func (e erc20Calls) loadERC20Uint8(
 	if err != nil {
 		return out, err
 	}
-	res, err := e.Keeper.CallContractWithInput(
+	evmResp, err := e.Keeper.CallContractWithInput(
 		ctx,
 		evmObj,
 		evm.EVM_MODULE_ADDRESS,
@@ -254,14 +254,14 @@ func (e erc20Calls) loadERC20Uint8(
 
 	erc20Val := new(ERC20Uint8)
 	if err := erc20Abi.UnpackIntoInterface(
-		erc20Val, methodName, res.Ret,
+		erc20Val, methodName, evmResp.Ret,
 	); err == nil {
 		return erc20Val.Value, err
 	}
 
 	erc20Uint256Val := new(ERC20BigInt)
 	if err := erc20Abi.UnpackIntoInterface(
-		erc20Uint256Val, methodName, res.Ret,
+		erc20Uint256Val, methodName, evmResp.Ret,
 	); err == nil {
 		// We can safely cast to uint8 because it's nonsense for decimals to be larger than 255
 		return uint8(erc20Uint256Val.Value.Uint64()), err

diff --git a/x/evm/keeper/funtoken_from_coin.go b/x/evm/keeper/funtoken_from_coin.go
@@ -102,7 +102,7 @@ func (k *Keeper) deployERC20ForBankCoin(
 		k.Bank.StateDB = nil
 	}()
 	evmObj := k.NewEVM(ctx, evmMsg, evmCfg, nil /*tracer*/, stateDB)
-	evmResp, err := k.CallContractWithInput(
+	_, err = k.CallContractWithInput(
 		ctx, evmObj, evm.EVM_MODULE_ADDRESS, nil, true /*commit*/, input, Erc20GasLimitDeploy,
 	)
 	if err != nil {
@@ -114,7 +114,5 @@ func (k *Keeper) deployERC20ForBankCoin(
 		return gethcommon.Address{}, errors.Wrap(err, "failed to commit stateDB")
 	}
 
-	ctx.GasMeter().ConsumeGas(evmResp.GasUsed, "deploy erc20 funtoken contract")
-
 	return erc20Addr, nil
 }
diff --git a/x/evm/keeper/funtoken_from_coin_test.go b/x/evm/keeper/funtoken_from_coin_test.go
@@ -82,6 +82,7 @@ func (s *FunTokenFromCoinSuite) TestCreateFunTokenFromCoin() {
 	})
 
 	s.Run("happy: CreateFunToken for the bank coin", func() {
+		deps.Ctx = deps.Ctx.WithGasMeter(sdk.NewInfiniteGasMeter())
 		s.Require().NoError(testapp.FundAccount(
 			deps.App.BankKeeper,
 			deps.Ctx,
@@ -97,6 +98,7 @@ func (s *FunTokenFromCoinSuite) TestCreateFunTokenFromCoin() {
 			},
 		)
 		s.Require().NoError(err)
+		s.Require().NotZero(deps.Ctx.GasMeter().GasConsumed())
 
 		s.Equal(
 			createFuntokenResp.FuntokenMapping,
@@ -167,6 +169,7 @@ func (s *FunTokenFromCoinSuite) TestConvertCoinToEvmAndBack() {
 	funToken := s.fundAndCreateFunToken(deps, 100)
 
 	s.T().Log("Convert bank coin to erc-20")
+	deps.Ctx = deps.Ctx.WithGasMeter(sdk.NewInfiniteGasMeter())
 	_, err := deps.EvmKeeper.ConvertCoinToEvm(
 		sdk.WrapSDKContext(deps.Ctx),
 		&evm.MsgConvertCoinToEvm{
@@ -178,6 +181,7 @@ func (s *FunTokenFromCoinSuite) TestConvertCoinToEvmAndBack() {
 		},
 	)
 	s.Require().NoError(err)
+	s.Require().NotZero(deps.Ctx.GasMeter().GasConsumed())
 
 	s.T().Log("Check typed event")
 	testutil.RequireContainsTypedEvent(
@@ -226,6 +230,7 @@ func (s *FunTokenFromCoinSuite) TestConvertCoinToEvmAndBack() {
 		deps.Sender.NibiruAddr.String(),
 	)
 	s.Require().NoError(err)
+	deps.Ctx = deps.Ctx.WithGasMeter(sdk.NewInfiniteGasMeter())
 	evmObj, _ = deps.NewEVM()
 	_, err = deps.EvmKeeper.CallContractWithInput(
 		deps.Ctx,
@@ -237,6 +242,7 @@ func (s *FunTokenFromCoinSuite) TestConvertCoinToEvmAndBack() {
 		evmtest.FunTokenGasLimitSendToEvm,
 	)
 	s.Require().NoError(err)
+	s.Require().NotZero(deps.Ctx.GasMeter().GasConsumed())
 
 	// Check 1: module balance
 	moduleBalance = deps.App.BankKeeper.GetBalance(deps.Ctx, authtypes.NewModuleAddress(evm.ModuleName), evm.EVMBankDenom)
@@ -252,6 +258,7 @@ func (s *FunTokenFromCoinSuite) TestConvertCoinToEvmAndBack() {
 	s.Require().Equal("0", balance.String())
 
 	s.T().Log("sad: Convert more erc-20 to back to bank coin, insufficient funds")
+	deps.Ctx = deps.Ctx.WithGasMeter(sdk.NewInfiniteGasMeter())
 	evmObj, _ = deps.NewEVM()
 	_, err = deps.EvmKeeper.CallContractWithInput(
 		deps.Ctx,
@@ -263,6 +270,7 @@ func (s *FunTokenFromCoinSuite) TestConvertCoinToEvmAndBack() {
 		evmtest.FunTokenGasLimitSendToEvm,
 	)
 	s.Require().ErrorContains(err, "transfer amount exceeds balance")
+	s.Require().NotZero(deps.Ctx.GasMeter().GasConsumed())
 }
 
 // TestNativeSendThenPrecompileSend tests a race condition where the state DB
@@ -362,6 +370,7 @@ func (s *FunTokenFromCoinSuite) TestNativeSendThenPrecompileSend() {
 		newSendAmtSendToBank,      /*amount*/
 	)
 	s.Require().NoError(err)
+	deps.Ctx = deps.Ctx.WithGasMeter(sdk.NewInfiniteGasMeter())
 	evmObj, _ = deps.NewEVM()
 	evmResp, err := deps.EvmKeeper.CallContractWithInput(
 		deps.Ctx,
@@ -373,6 +382,9 @@ func (s *FunTokenFromCoinSuite) TestNativeSendThenPrecompileSend() {
 		evmtest.FunTokenGasLimitSendToEvm,
 	)
 	s.Require().NoError(err)
+	s.Require().NotZero(deps.Ctx.GasMeter().GasConsumed())
+	s.Require().NotZero(evmResp.GasUsed)
+	s.Require().Greaterf(deps.Ctx.GasMeter().GasConsumed(), evmResp.GasUsed, "total gas consumed on cosmos context should be greater than gas used by EVM")
 	s.Empty(evmResp.VmError)
 	gasUsedFor2Ops := evmResp.GasUsed
 
@@ -404,6 +416,7 @@ func (s *FunTokenFromCoinSuite) TestNativeSendThenPrecompileSend() {
 		newSendAmtSendToBank,      /*amount*/
 	)
 	s.Require().NoError(err)
+	deps.Ctx = deps.Ctx.WithGasMeter(sdk.NewInfiniteGasMeter())
 	evmObj, _ = deps.NewEVM()
 	evmResp, err = deps.EvmKeeper.CallContractWithInput(
 		deps.Ctx,
@@ -415,6 +428,9 @@ func (s *FunTokenFromCoinSuite) TestNativeSendThenPrecompileSend() {
 		evmtest.DefaultEthCallGasLimit,
 	)
 	s.Require().NoError(err)
+	s.Require().NotZero(deps.Ctx.GasMeter().GasConsumed())
+	s.Require().NotZero(evmResp.GasUsed)
+	s.Require().Greaterf(deps.Ctx.GasMeter().GasConsumed(), evmResp.GasUsed, "total gas consumed on cosmos context should be greater than gas used by EVM")
 	s.Empty(evmResp.VmError)
 	gasUsedFor1Op := evmResp.GasUsed
 
@@ -517,8 +533,9 @@ func (s *FunTokenFromCoinSuite) TestERC20TransferThenPrecompileSend() {
 		big.NewInt(9e6),           /*amount*/
 	)
 	s.Require().NoError(err)
+	deps.Ctx = deps.Ctx.WithGasMeter(sdk.NewInfiniteGasMeter())
 	evmObj, _ = deps.NewEVM()
-	_, err = deps.EvmKeeper.CallContractWithInput(
+	evmResp, err := deps.EvmKeeper.CallContractWithInput(
 		deps.Ctx,
 		evmObj,
 		deps.Sender.EthAddr, // from
@@ -528,6 +545,9 @@ func (s *FunTokenFromCoinSuite) TestERC20TransferThenPrecompileSend() {
 		10_000_000, // gas limit
 	)
 	s.Require().NoError(err)
+	s.Require().NotZero(deps.Ctx.GasMeter().GasConsumed())
+	s.Require().NotZero(evmResp.GasUsed)
+	s.Require().Greaterf(deps.Ctx.GasMeter().GasConsumed(), evmResp.GasUsed, "total gas consumed on cosmos context should be greater than gas used by EVM")
 
 	evmtest.FunTokenBalanceAssert{
 		FunToken:     funToken,
@@ -620,6 +640,7 @@ func (s *FunTokenFromCoinSuite) TestPrecompileSelfCallRevert() {
 	charles := evmtest.NewEthPrivAcc()
 
 	s.T().Log("call test contract")
+	deps.Ctx = deps.Ctx.WithGasMeter(sdk.NewInfiniteGasMeter())
 	evmObj, _ = deps.NewEVM()
 	contractInput, err := embeds.SmartContract_TestPrecompileSelfCallRevert.ABI.Pack(
 		"selfCallTransferFunds",
@@ -629,7 +650,7 @@ func (s *FunTokenFromCoinSuite) TestPrecompileSelfCallRevert() {
 		big.NewInt(9e6),
 	)
 	s.Require().NoError(err)
-	_, err = deps.EvmKeeper.CallContractWithInput(
+	evpResp, err := deps.EvmKeeper.CallContractWithInput(
 		deps.Ctx,
 		evmObj,
 		deps.Sender.EthAddr,
@@ -639,6 +660,9 @@ func (s *FunTokenFromCoinSuite) TestPrecompileSelfCallRevert() {
 		evmtest.FunTokenGasLimitSendToEvm,
 	)
 	s.Require().NoError(err)
+	s.Require().NotZero(deps.Ctx.GasMeter().GasConsumed())
+	s.Require().NotZero(evpResp.GasUsed)
+	s.Require().Greaterf(deps.Ctx.GasMeter().GasConsumed(), evpResp.GasUsed, "total gas consumed on cosmos context should be greater than gas used by EVM")
 
 	evmtest.FunTokenBalanceAssert{
 		FunToken:     funToken,
@@ -726,8 +750,9 @@ func (s *FunTokenFromCoinSuite) TestPrecompileSendToBankThenErc20Transfer() {
 		"attack",
 	)
 	s.Require().NoError(err)
+	deps.Ctx = deps.Ctx.WithGasMeter(sdk.NewInfiniteGasMeter())
 	evmObj, _ := deps.NewEVM()
-	_, err = deps.EvmKeeper.CallContractWithInput(
+	evpResp, err := deps.EvmKeeper.CallContractWithInput(
 		deps.Ctx,
 		evmObj,
 		deps.Sender.EthAddr,
@@ -737,6 +762,9 @@ func (s *FunTokenFromCoinSuite) TestPrecompileSendToBankThenErc20Transfer() {
 		evmtest.FunTokenGasLimitSendToEvm,
 	)
 	s.Require().ErrorContains(err, "execution reverted")
+	s.Require().NotZero(deps.Ctx.GasMeter().GasConsumed())
+	s.Require().NotZero(evpResp.GasUsed)
+	s.Require().Greaterf(deps.Ctx.GasMeter().GasConsumed(), evpResp.GasUsed, "total gas consumed on cosmos context should be greater than gas used by EVM")
 
 	evmtest.FunTokenBalanceAssert{
 		FunToken:     funToken,