fix: gas used discrepancy in estimate gas vs actual execution gas

evm-e2e/.gitignore

@@ -0,0 +1,4 @@
+types
+artifacts
+cache
+.env

x/evm/keeper/call_contract.go

@@ -71,11 +71,7 @@ func (k Keeper) CallContractWithInput(
 ) (evmResp *evm.MsgEthereumTxResponse, evmObj *vm.EVM, 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 func() {
-		if err != nil {
-			err = fmt.Errorf("CallContractError: %w", err)
-		}
-	}()
+	defer HandleOutOfGasPanic(&err, "CallContractError")
 	nonce := k.GetAccNonce(ctx, fromAcc)
 
 	unusedBigInt := big.NewInt(0)
@@ -108,11 +104,8 @@ func (k Keeper) CallContractWithInput(
 	// sent by a user
 	txConfig := k.TxConfig(ctx, gethcommon.BigToHash(big.NewInt(0)))
 
-	// Using tmp context to not modify the state in case of evm revert
-	tmpCtx, commitCtx := ctx.CacheContext()
-
 	evmResp, evmObj, err = k.ApplyEvmMsg(
-		tmpCtx, evmMsg, evm.NewNoOpTracer(), commit, evmCfg, txConfig, true,
+		ctx, evmMsg, evm.NewNoOpTracer(), commit, evmCfg, txConfig, true,
 	)
 	if err != nil {
 		// We don't know the actual gas used, so consuming the gas limit
@@ -135,20 +128,21 @@ func (k Keeper) CallContractWithInput(
 	} else {
 		// Success, committing the state to ctx
 		if commit {
-			commitCtx()
-			totalGasUsed, err := k.AddToBlockGasUsed(ctx, evmResp.GasUsed)
+			blockGasUsed, err := k.AddToBlockGasUsed(ctx, evmResp.GasUsed)
 			if err != nil {
 				k.ResetGasMeterAndConsumeGas(ctx, ctx.GasMeter().Limit())
 				return nil, nil, errors.Wrap(err, "error adding transient gas used to block")
 			}
-			k.ResetGasMeterAndConsumeGas(ctx, totalGasUsed)
+			k.ResetGasMeterAndConsumeGas(ctx, blockGasUsed)
 			k.updateBlockBloom(ctx, evmResp, uint64(txConfig.LogIndex))
-			err = k.EmitEthereumTxEvents(ctx, contract, gethcore.LegacyTxType, evmMsg, evmResp)
-			if err != nil {
-				return nil, nil, errors.Wrap(err, "error emitting ethereum tx events")
-			}
-			blockTxIdx := uint64(txConfig.TxIndex) + 1
-			k.EvmState.BlockTxIndex.Set(ctx, blockTxIdx)
+			// TODO: remove after migrating logs
+			//err = k.EmitLogEvents(ctx, evmResp)
+			//if err != nil {
+			//	return nil, nil, errors.Wrap(err, "error emitting tx logs")
+			//}
+
+			//blockTxIdx := uint64(txConfig.TxIndex) + 1
+			//k.EvmState.BlockTxIndex.Set(ctx, blockTxIdx)
 		}
 		return evmResp, evmObj, nil
 	}

x/evm/keeper/erc20.go

@@ -81,37 +81,37 @@ Transfer implements "ERC20.transfer"
 func (e erc20Calls) Transfer(
 	contract, from, to gethcommon.Address, amount *big.Int,
 	ctx sdk.Context,
-) (balanceIncrease *big.Int, err error) {
+) (balanceIncrease *big.Int, resp *evm.MsgEthereumTxResponse, err error) {
 	recipientBalanceBefore, err := e.BalanceOf(contract, to, ctx)
 	if err != nil {
-		return balanceIncrease, errors.Wrap(err, "failed to retrieve recipient balance")
+		return balanceIncrease, nil, errors.Wrap(err, "failed to retrieve recipient balance")
 	}
 
 	input, err := e.ABI.Pack("transfer", to, amount)
 	if err != nil {
-		return balanceIncrease, fmt.Errorf("failed to pack ABI args: %w", err)
+		return balanceIncrease, nil, fmt.Errorf("failed to pack ABI args: %w", err)
 	}
 
-	resp, _, err := e.CallContractWithInput(ctx, from, &contract, true, input, Erc20GasLimitExecute)
+	resp, _, err = e.CallContractWithInput(ctx, from, &contract, true, input, Erc20GasLimitExecute)
 	if err != nil {
-		return balanceIncrease, err
+		return balanceIncrease, nil, err
 	}
 
 	var erc20Bool ERC20Bool
 	err = e.ABI.UnpackIntoInterface(&erc20Bool, "transfer", resp.Ret)
 	if err != nil {
-		return balanceIncrease, err
+		return balanceIncrease, nil, err
 	}
 
 	// Handle the case of success=false: https://github.com/NibiruChain/nibiru/issues/2080
 	success := erc20Bool.Value
 	if !success {
-		return balanceIncrease, fmt.Errorf("transfer executed but returned success=false")
+		return balanceIncrease, nil, fmt.Errorf("transfer executed but returned success=false")
 	}
 
 	recipientBalanceAfter, err := e.BalanceOf(contract, to, ctx)
 	if err != nil {
-		return balanceIncrease, errors.Wrap(err, "failed to retrieve recipient balance")
+		return balanceIncrease, nil, errors.Wrap(err, "failed to retrieve recipient balance")
 	}
 
 	balanceIncrease = new(big.Int).Sub(recipientBalanceAfter, recipientBalanceBefore)
@@ -121,13 +121,13 @@ func (e erc20Calls) Transfer(
 	// the call "amount". Instead, verify that the recipient got tokens and
 	// return the amount.
 	if balanceIncrease.Sign() <= 0 {
-		return balanceIncrease, fmt.Errorf(
+		return balanceIncrease, nil, fmt.Errorf(
 			"amount of ERC20 tokens received MUST be positive: the balance of recipient %s would've changed by %v for token %s",
 			to.Hex(), balanceIncrease.String(), contract.Hex(),
 		)
 	}
 
-	return balanceIncrease, err
+	return balanceIncrease, resp, err
 }
 
 // BalanceOf retrieves the balance of an ERC20 token for a specific account.

x/evm/keeper/keeper.go

@@ -2,6 +2,7 @@
 package keeper
 
 import (
+	"fmt"
 	"math/big"
 
 	"cosmossdk.io/math"
@@ -101,13 +102,19 @@ func (k Keeper) EthChainID(ctx sdk.Context) *big.Int {
 // block tx.
 func (k *Keeper) AddToBlockGasUsed(
 	ctx sdk.Context, gasUsed uint64,
-) (uint64, error) {
-	result := k.EvmState.BlockGasUsed.GetOr(ctx, 0) + gasUsed
-	if result < gasUsed {
+) (blockGasUsed uint64, err error) {
+
+	// Either k.EvmState.BlockGasUsed.GetOr() or k.EvmState.BlockGasUsed.Set()
+	// also consume gas and could panic.
+	defer HandleOutOfGasPanic(&err, "")
+
+	blockGasUsed = k.EvmState.BlockGasUsed.GetOr(ctx, 0) + gasUsed
+	if blockGasUsed < gasUsed {
 		return 0, sdkerrors.Wrap(evm.ErrGasOverflow, "transient gas used")
 	}
-	k.EvmState.BlockGasUsed.Set(ctx, result)
-	return result, nil
+	k.EvmState.BlockGasUsed.Set(ctx, blockGasUsed)
+
+	return blockGasUsed, nil
 }
 
 // GetMinGasUsedMultiplier - value from 0 to 1
@@ -143,3 +150,20 @@ func (k Keeper) Tracer(
 ) vm.EVMLogger {
 	return evm.NewTracer(k.tracer, msg, ethCfg, ctx.BlockHeight())
 }
+
+// HandleOutOfGasPanic gracefully captures "out of gas" panic and just sets the value to err
+func HandleOutOfGasPanic(err *error, format string) func() {
+	return func() {
+		if r := recover(); r != nil {
+			switch r.(type) {
+			case sdk.ErrorOutOfGas:
+				*err = vm.ErrOutOfGas
+			default:
+				panic(r)
+			}
+		}
+		if err != nil && format != "" {
+			*err = fmt.Errorf("%s: %w", format, *err)
+		}
+	}
+}

x/evm/keeper/msg_server.go

@@ -64,6 +64,7 @@ func (k *Keeper) EthereumTx(
 
 	// pass true to commit the StateDB
 	evmResp, _, err = k.ApplyEvmMsg(tmpCtx, evmMsg, nil, true, evmConfig, txConfig, false)
+
 	if err != nil {
 		// when a transaction contains multiple msg, as long as one of the msg fails
 		// all gas will be deducted. so is not msg.Gas()
@@ -74,32 +75,35 @@ func (k *Keeper) EthereumTx(
 	if !evmResp.Failed() {
 		commitCtx()
 	}
+	k.updateBlockBloom(ctx, evmResp, uint64(txConfig.LogIndex))
+
+	blockGasUsed, err := k.AddToBlockGasUsed(ctx, evmResp.GasUsed)
+	if err != nil {
+		return nil, errors.Wrap(err, "error adding transient gas used to block")
+	}
 
 	// refund gas in order to match the Ethereum gas consumption instead of the
 	// default SDK one.
 	refundGas := uint64(0)
-	if evmMsg.Gas() > evmResp.GasUsed {
-		refundGas = evmMsg.Gas() - evmResp.GasUsed
+	if evmMsg.Gas() > blockGasUsed {
+		refundGas = evmMsg.Gas() - blockGasUsed
 	}
 	weiPerGas := txMsg.EffectiveGasPriceWeiPerGas(evmConfig.BaseFeeWei)
 	if err = k.RefundGas(ctx, evmMsg.From(), refundGas, weiPerGas); err != nil {
 		return nil, errors.Wrapf(err, "error refunding leftover gas to sender %s", evmMsg.From())
 	}
 
-	k.updateBlockBloom(ctx, evmResp, uint64(txConfig.LogIndex))
-
-	totalGasUsed, err := k.AddToBlockGasUsed(ctx, evmResp.GasUsed)
-	if err != nil {
-		return nil, errors.Wrap(err, "error adding transient gas used to block")
-	}
-
 	// reset the gas meter for current TxMsg (EthereumTx)
-	k.ResetGasMeterAndConsumeGas(ctx, totalGasUsed)
+	k.ResetGasMeterAndConsumeGas(ctx, blockGasUsed)
 
 	err = k.EmitEthereumTxEvents(ctx, tx.To(), tx.Type(), evmMsg, evmResp)
 	if err != nil {
 		return nil, errors.Wrap(err, "error emitting ethereum tx events")
 	}
+	err = k.EmitLogEvents(ctx, evmResp)
+	if err != nil {
+		return nil, errors.Wrap(err, "error emitting tx logs")
+	}
 
 	blockTxIdx := uint64(txConfig.TxIndex) + 1
 	k.EvmState.BlockTxIndex.Set(ctx, blockTxIdx)
@@ -584,7 +588,7 @@ func (k Keeper) convertCoinToEvmBornERC20(
 	// converted to its Bank Coin representation, a balance of the ERC20 is left
 	// inside the EVM module account in order to convert the coins back to
 	// ERC20s.
-	actualSentAmount, err := k.ERC20().Transfer(
+	actualSentAmount, _, err := k.ERC20().Transfer(
 		erc20Addr,
 		evm.EVM_MODULE_ADDRESS,
 		recipient,
@@ -644,17 +648,6 @@ func (k *Keeper) EmitEthereumTxEvents(
 		return errors.Wrap(err, "failed to emit event ethereum tx")
 	}
 
-	// Typed event: eth.evm.v1.EventTxLog
-	txLogs := make([]string, len(evmResp.Logs))
-	for i, log := range evmResp.Logs {
-		value, err := json.Marshal(log)
-		if err != nil {
-			return errors.Wrap(err, "failed to encode log")
-		}
-		txLogs[i] = string(value)
-	}
-	_ = ctx.EventManager().EmitTypedEvent(&evm.EventTxLog{TxLogs: txLogs})
-
 	// Untyped event: "message", used for tendermint subscription
 	ctx.EventManager().EmitEvent(
 		sdk.NewEvent(
@@ -690,6 +683,25 @@ func (k *Keeper) EmitEthereumTxEvents(
 	return nil
 }
 
+// EmitLogEvents emits all types of EVM events applicable to a particular execution case
+func (k *Keeper) EmitLogEvents(
+	ctx sdk.Context,
+	evmResp *evm.MsgEthereumTxResponse,
+) error {
+	// Typed event: eth.evm.v1.EventTxLog
+	txLogs := make([]string, len(evmResp.Logs))
+	for i, log := range evmResp.Logs {
+		value, err := json.Marshal(log)
+		if err != nil {
+			return errors.Wrap(err, "failed to encode log")
+		}
+		txLogs[i] = string(value)
+	}
+	_ = ctx.EventManager().EmitTypedEvent(&evm.EventTxLog{TxLogs: txLogs})
+
+	return nil
+}
+
 // updateBlockBloom updates transient block bloom filter
 func (k *Keeper) updateBlockBloom(
 	ctx sdk.Context,

x/evm/precompile/funtoken.go

@@ -109,11 +109,11 @@ type precompileFunToken struct {
 //	function bankSend(address erc20, uint256 amount, string memory to) external;
 //	```
 func (p precompileFunToken) bankSend(
-	start OnRunStartResult,
+	startResult OnRunStartResult,
 	caller gethcommon.Address,
 	readOnly bool,
 ) (bz []byte, err error) {
-	ctx, method, args := start.CacheCtx, start.Method, start.Args
+	ctx, method, args := startResult.CacheCtx, startResult.Method, startResult.Args
 	if err := assertNotReadonlyTx(readOnly, method); err != nil {
 		return nil, err
 	}
@@ -123,6 +123,8 @@ func (p precompileFunToken) bankSend(
 		return
 	}
 
+	var evmResponses []*evm.MsgEthereumTxResponse
+
 	// ERC20 must have FunToken mapping
 	funtokens := p.evmKeeper.FunTokens.Collect(
 		ctx, p.evmKeeper.FunTokens.Indexes.ERC20Addr.ExactMatch(ctx, erc20),
@@ -146,10 +148,11 @@ func (p precompileFunToken) bankSend(
 
 	// Caller transfers ERC20 to the EVM account
 	transferTo := evm.EVM_MODULE_ADDRESS
-	gotAmount, err := p.evmKeeper.ERC20().Transfer(erc20, caller, transferTo, amount, ctx)
+	gotAmount, transferResp, err := p.evmKeeper.ERC20().Transfer(erc20, caller, transferTo, amount, ctx)
 	if err != nil {
 		return nil, fmt.Errorf("error in ERC20.transfer from caller to EVM account: %w", err)
 	}
+	evmResponses = append(evmResponses, transferResp)
 
 	// EVM account mints FunToken.BankDenom to module account
 	coinToSend := sdk.NewCoin(funtoken.BankDenom, math.NewIntFromBigInt(gotAmount))
@@ -158,17 +161,18 @@ func (p precompileFunToken) bankSend(
 		// owns the ERC20 contract and was the original minter of the ERC20 tokens.
 		// Since we're sending them away and want accurate total supply tracking, the
 		// tokens need to be burned.
-		_, err = p.evmKeeper.ERC20().Burn(erc20, evm.EVM_MODULE_ADDRESS, gotAmount, ctx)
-		if err != nil {
-			err = fmt.Errorf("ERC20.Burn: %w", err)
+		burnResp, e := p.evmKeeper.ERC20().Burn(erc20, evm.EVM_MODULE_ADDRESS, gotAmount, ctx)
+		if e != nil {
+			err = fmt.Errorf("ERC20.Burn: %w", e)
 			return
 		}
+		evmResponses = append(evmResponses, burnResp)
 	} else {
 		// NOTE: The NibiruBankKeeper needs to reference the current [vm.StateDB] before
 		// any operation that has the potential to use Bank send methods. This will
 		// guarantee that [evmkeeper.Keeper.SetAccBalance] journal changes are
 		// recorded if wei (NIBI) is transferred.
-		p.evmKeeper.Bank.StateDB = start.StateDB
+		p.evmKeeper.Bank.StateDB = startResult.StateDB
 		err = p.evmKeeper.Bank.MintCoins(ctx, evm.ModuleName, sdk.NewCoins(coinToSend))
 		if err != nil {
 			return nil, fmt.Errorf("mint failed for module \"%s\" (%s): contract caller %s: %w",
@@ -183,7 +187,7 @@ func (p precompileFunToken) bankSend(
 	// any operation that has the potential to use Bank send methods. This will
 	// guarantee that [evmkeeper.Keeper.SetAccBalance] journal changes are
 	// recorded if wei (NIBI) is transferred.
-	p.evmKeeper.Bank.StateDB = start.StateDB
+	p.evmKeeper.Bank.StateDB = startResult.StateDB
 	err = p.evmKeeper.Bank.SendCoinsFromModuleToAccount(
 		ctx,
 		evm.ModuleName,
@@ -195,6 +199,11 @@ func (p precompileFunToken) bankSend(
 			evm.ModuleName, evm.EVM_MODULE_ADDRESS.Hex(), caller.Hex(), err,
 		)
 	}
+	for _, resp := range evmResponses {
+		for _, log := range resp.Logs {
+			startResult.StateDB.AddLog(log.ToEthereum())
+		}
+	}
 
 	// TODO: UD-DEBUG: feat: Emit EVM events