refactor(evm): removed blockGasUsed transient variable (#2167)

* refactor(evm): removed blockGasUsed transient variable * chore: changelog update * fix: added testutil retry for backend test suite * fix: lint --------- Co-authored-by: Unique Divine <[email protected]>
NibiruChain · Jan 17, 2025 · f3ca671 · f3ca671
1 parent 04bd7c2
commit f3ca671
Show file tree

Hide file tree

Showing 19 changed files with 72 additions and 125 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -77,6 +77,7 @@ needed to include double quotes around the hexadecimal string.
 - [#2157](https://github.com/NibiruChain/nibiru/pull/2157) - fix(evm): Fix unit inconsistency related to AuthInfo.Fee and txData.Fee using effective fee
 - [#2160](https://github.com/NibiruChain/nibiru/pull/2160) - fix(evm-precompile): use bank.MsgServer Send in precompile IFunToken.bankMsgSend
 - [#2162](https://github.com/NibiruChain/nibiru/pull/2162) - test(testutil): try retrying for 'panic: pebbledb: closed'
+- [#2167](https://github.com/NibiruChain/nibiru/pull/2167) - refactor(evm): removed blockGasUsed transient variable
 - [#2168](https://github.com/NibiruChain/nibiru/pull/2168) - chore(evm-solidity): Move unrelated docs, gen-embeds, and add Solidity docs
 
 #### Nibiru EVM | Before Audit 2 - 2024-12-06

diff --git a/app/evmante/evmante_setup_ctx.go b/app/evmante/evmante_setup_ctx.go
@@ -41,8 +41,5 @@ func (esc EthSetupContextDecorator) AnteHandle(
 		WithKVGasConfig(storetypes.GasConfig{}).
 		WithTransientKVGasConfig(storetypes.GasConfig{})
 
-	// Reset transient gas used to prepare the execution of current cosmos tx.
-	// Transient gas-used is necessary to sum the gas-used of cosmos tx, when it contains multiple eth msgs.
-	esc.evmKeeper.ResetTransientGasUsed(ctx)
 	return next(newCtx, tx, simulate)
 }
diff --git a/app/evmante/evmante_setup_ctx_test.go b/app/evmante/evmante_setup_ctx_test.go
@@ -18,9 +18,6 @@ func (s *TestSuite) TestEthSetupContextDecorator() {
 	s.Require().NoError(stateDB.Commit())
 	tx := evmtest.HappyCreateContractTx(&deps)
 
-	// Set block gas used to non 0 to check that handler resets it
-	deps.App.EvmKeeper.EvmState.BlockGasUsed.Set(deps.Ctx, 1000)
-
 	// Ante handler returns new context
 	newCtx, err := anteDec.AnteHandle(
 		deps.Ctx, tx, false, evmtest.NextNoOpAnteHandler,
@@ -35,9 +32,4 @@ func (s *TestSuite) TestEthSetupContextDecorator() {
 	defaultGasConfig := storetypes.GasConfig{}
 	s.Require().Equal(defaultGasConfig, newCtx.KVGasConfig())
 	s.Require().Equal(defaultGasConfig, newCtx.TransientKVGasConfig())
-
-	// Check that block gas used is reset to 0
-	gas, err := deps.App.EvmKeeper.EvmState.BlockGasUsed.Get(newCtx)
-	s.Require().NoError(err)
-	s.Require().Equal(gas, uint64(0))
 }
diff --git a/eth/indexer/evm_tx_indexer_test.go b/eth/indexer/evm_tx_indexer_test.go
@@ -1,6 +1,7 @@
 package indexer_test
 
 import (
+	"fmt"
 	"math/big"
 	"testing"
 
@@ -109,6 +110,7 @@ func TestEVMTxIndexer(t *testing.T) {
 								{Key: "gas_used", Value: `"21000"`},
 								{Key: "index", Value: `"0"`},
 								{Key: "hash", Value: `"14A84ED06282645EFBF080E0B7ED80D8D8D6A36337668A12B5F229F81CDD3F57"`},
+								{Key: "eth_hash", Value: fmt.Sprintf(`"%s"`, txHash.Hex())},
 							},
 						},
 					},

diff --git a/eth/rpc/backend/backend_suite_test.go b/eth/rpc/backend/backend_suite_test.go
@@ -17,6 +17,8 @@ import (
 	"github.com/ethereum/go-ethereum/params"
 	"github.com/stretchr/testify/suite"
 
+	"github.com/NibiruChain/nibiru/v2/x/common/testutil"
+
 	"github.com/NibiruChain/nibiru/v2/x/evm/embeds"
 
 	"github.com/NibiruChain/nibiru/v2/x/evm/evmtest"
@@ -66,7 +68,9 @@ type BackendSuite struct {
 }
 
 func TestBackendSuite(t *testing.T) {
-	suite.Run(t, new(BackendSuite))
+	testutil.RetrySuiteRunIfDbClosed(t, func() {
+		suite.Run(t, new(BackendSuite))
+	}, 2)
 }
 
 func (s *BackendSuite) SetupSuite() {
@@ -223,15 +227,15 @@ func (s *BackendSuite) broadcastSDKTx(sdkTx sdk.Tx) *sdk.TxResponse {
 }
 
 // buildContractCreationTx builds a contract creation transaction
-func (s *BackendSuite) buildContractCreationTx(nonce uint64) gethcore.Transaction {
+func (s *BackendSuite) buildContractCreationTx(nonce uint64, gasLimit uint64) gethcore.Transaction {
 	packedArgs, err := embeds.SmartContract_TestERC20.ABI.Pack("")
 	s.Require().NoError(err)
 	bytecodeForCall := append(embeds.SmartContract_TestERC20.Bytecode, packedArgs...)
 
 	creationTx := &gethcore.LegacyTx{
 		Nonce:    nonce,
 		Data:     bytecodeForCall,
-		Gas:      1_500_000,
+		Gas:      gasLimit,
 		GasPrice: big.NewInt(1),
 	}
 
@@ -243,7 +247,11 @@ func (s *BackendSuite) buildContractCreationTx(nonce uint64) gethcore.Transactio
 }
 
 // buildContractCallTx builds a contract call transaction
-func (s *BackendSuite) buildContractCallTx(nonce uint64, contractAddr gethcommon.Address) gethcore.Transaction {
+func (s *BackendSuite) buildContractCallTx(
+	contractAddr gethcommon.Address,
+	nonce uint64,
+	gasLimit uint64,
+) gethcore.Transaction {
 	// recipient := crypto.CreateAddress(s.fundedAccEthAddr, 29381)
 	transferAmount := big.NewInt(100)
 
@@ -257,7 +265,7 @@ func (s *BackendSuite) buildContractCallTx(nonce uint64, contractAddr gethcommon
 	transferTx := &gethcore.LegacyTx{
 		Nonce:    nonce,
 		Data:     packedArgs,
-		Gas:      100_000,
+		Gas:      gasLimit,
 		GasPrice: big.NewInt(1),
 		To:       &contractAddr,
 	}

diff --git a/eth/rpc/backend/gas_used_test.go b/eth/rpc/backend/gas_used_test.go
@@ -186,3 +186,49 @@ func (s *BackendSuite) TestGasUsedFunTokens() {
 		balanceBefore.Uint64()-balanceAfter.Uint64(),
 	)
 }
+
+// TestMultipleMsgsTxGasUsage tests that the gas is correctly consumed per message in a single transaction.
+func (s *BackendSuite) TestMultipleMsgsTxGasUsage() {
+	// Test is broadcasting txs. Lock to avoid nonce conflicts.
+	testMutex.Lock()
+	defer testMutex.Unlock()
+
+	balanceBefore := s.getUnibiBalance(s.fundedAccEthAddr)
+
+	nonce := s.getCurrentNonce(s.fundedAccEthAddr)
+
+	contractCreationGasLimit := uint64(1_500_000)
+	contractCallGasLimit := uint64(100_000)
+
+	// Create series of 3 tx messages. Expecting nonce to be incremented by 3
+	creationTx := s.buildContractCreationTx(nonce, contractCreationGasLimit)
+	firstTransferTx := s.buildContractCallTx(testContractAddress, nonce+1, contractCallGasLimit)
+	secondTransferTx := s.buildContractCallTx(testContractAddress, nonce+2, contractCallGasLimit)
+
+	// Create and broadcast SDK transaction
+	sdkTx := s.buildSDKTxWithEVMMessages(
+		creationTx,
+		firstTransferTx,
+		secondTransferTx,
+	)
+	s.broadcastSDKTx(sdkTx)
+
+	_, _, receiptContractCreation := WaitForReceipt(s, creationTx.Hash())
+	_, _, receiptFirstTransfer := WaitForReceipt(s, firstTransferTx.Hash())
+	_, _, receiptSecondTransfer := WaitForReceipt(s, secondTransferTx.Hash())
+
+	s.Require().Greater(receiptContractCreation.GasUsed, uint64(0))
+	s.Require().LessOrEqual(receiptContractCreation.GasUsed, contractCreationGasLimit)
+
+	s.Require().Greater(receiptFirstTransfer.GasUsed, uint64(0))
+	s.Require().LessOrEqual(receiptFirstTransfer.GasUsed, contractCallGasLimit)
+
+	s.Require().Greater(receiptSecondTransfer.GasUsed, uint64(0))
+	s.Require().LessOrEqual(receiptSecondTransfer.GasUsed, contractCallGasLimit)
+
+	balanceAfter := s.getUnibiBalance(s.fundedAccEthAddr)
+	s.Require().Equal(
+		receiptContractCreation.GasUsed+receiptFirstTransfer.GasUsed+receiptSecondTransfer.GasUsed,
+		balanceBefore.Uint64()-balanceAfter.Uint64(),
+	)
+}
diff --git a/eth/rpc/backend/nonce_test.go b/eth/rpc/backend/nonce_test.go
@@ -20,9 +20,9 @@ func (s *BackendSuite) TestNonceIncrementWithMultipleMsgsTx() {
 	nonce := s.getCurrentNonce(s.fundedAccEthAddr)
 
 	// Create series of 3 tx messages. Expecting nonce to be incremented by 3
-	creationTx := s.buildContractCreationTx(nonce)
-	firstTransferTx := s.buildContractCallTx(nonce+1, testContractAddress)
-	secondTransferTx := s.buildContractCallTx(nonce+2, testContractAddress)
+	creationTx := s.buildContractCreationTx(nonce, 1_500_000)
+	firstTransferTx := s.buildContractCallTx(testContractAddress, nonce+1, 100_000)
+	secondTransferTx := s.buildContractCallTx(testContractAddress, nonce+2, 100_000)
 
 	// Create and broadcast SDK transaction
 	sdkTx := s.buildSDKTxWithEVMMessages(

diff --git a/eth/rpc/events_parser.go b/eth/rpc/events_parser.go
@@ -86,11 +86,11 @@ func ParseTxResult(result *abci.ResponseDeliverTx, tx sdk.Tx) (*ParsedTxs, error
 				GasUsed:    gasUsed,
 				Failed:     len(eventEthereumTx.EthTxFailed) > 0,
 			}
-			// replace pending tx with committed tx
-			if msgIndex >= 0 && len(parsedTxs.Txs) == msgIndex+1 {
-				parsedTxs.Txs[msgIndex] = committedTx
+			// find the pending tx to replace by tx hash
+			pendingMsgIndex, found := parsedTxs.TxHashes[committedTx.EthHash]
+			if found {
+				parsedTxs.Txs[pendingMsgIndex] = committedTx
 			} else {
-				// EventEthereumTx without EventPendingEthereumTx
 				return nil, errors.New("EventEthereumTx without pending_ethereum_tx event")
 			}
 		}

diff --git a/x/evm/evmtest/test_deps.go b/x/evm/evmtest/test_deps.go
@@ -61,8 +61,3 @@ func (deps *TestDeps) GethSigner() gethcore.Signer {
 func (deps TestDeps) GoCtx() context.Context {
 	return sdk.WrapSDKContext(deps.Ctx)
 }
-
-func (deps TestDeps) ResetGasMeter() {
-	deps.EvmKeeper.ResetTransientGasUsed(deps.Ctx)
-	deps.EvmKeeper.ResetGasMeterAndConsumeGas(deps.Ctx, 0)
-}
diff --git a/x/evm/keeper/evm_state.go b/x/evm/keeper/evm_state.go
@@ -39,8 +39,6 @@ type EvmState struct {
 		[]byte,
 	]
 
-	// BlockGasUsed: Gas used by Ethereum txs in the block (transient).
-	BlockGasUsed collections.ItemTransient[uint64]
 	// BlockLogSize: EVM tx log size for the block (transient).
 	BlockLogSize collections.ItemTransient[uint64]
 	// BlockTxIndex: EVM tx index for the block (transient).
@@ -71,11 +69,6 @@ func NewEvmState(
 			collections.PairKeyEncoder(eth.KeyEncoderEthAddr, eth.KeyEncoderEthHash),
 			eth.ValueEncoderBytes,
 		),
-		BlockGasUsed: collections.NewItemTransient(
-			storeKeyTransient,
-			evm.NamespaceBlockGasUsed,
-			collections.Uint64ValueEncoder,
-		),
 		BlockLogSize: collections.NewItemTransient(
 			storeKeyTransient,
 			evm.NamespaceBlockLogSize,
@@ -166,12 +159,6 @@ func (state EvmState) CalcBloomFromLogs(
 	return bloom
 }
 
-// ResetTransientGasUsed resets gas to prepare for the next block of execution.
-// Called in an ante handler.
-func (k *Keeper) ResetTransientGasUsed(ctx sdk.Context) {
-	k.EvmState.BlockGasUsed.Set(ctx, 0)
-}
-
 // GetAccNonce returns the sequence number of an account, returns 0 if not exists.
 func (k Keeper) GetAccNonce(ctx sdk.Context, addr gethcommon.Address) uint64 {
 	nibiruAddr := sdk.AccAddress(addr.Bytes())

diff --git a/x/evm/keeper/funtoken_from_coin.go b/x/evm/keeper/funtoken_from_coin.go
@@ -82,14 +82,9 @@ func (k *Keeper) deployERC20ForBankCoin(
 		ctx, evm.EVM_MODULE_ADDRESS, nil, true, bytecodeForCall, Erc20GasLimitDeploy,
 	)
 	if err != nil {
-		k.ResetGasMeterAndConsumeGas(ctx, ctx.GasMeter().Limit())
 		return gethcommon.Address{}, errors.Wrap(err, "failed to deploy ERC20 contract")
 	}
-	blockGasUsed, errBlockGasUsed := k.AddToBlockGasUsed(ctx, evmResp.GasUsed)
-	if errBlockGasUsed != nil {
-		return gethcommon.Address{}, errors.Wrap(errBlockGasUsed, "error adding transient gas used")
-	}
-	k.ResetGasMeterAndConsumeGas(ctx, blockGasUsed)
+	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
@@ -225,8 +225,6 @@ func (s *FunTokenFromCoinSuite) TestConvertCoinToEvmAndBack() {
 	)
 	s.Require().ErrorContains(err, "insufficient funds")
 
-	deps.ResetGasMeter()
-
 	s.T().Log("Convert erc-20 to back to bank coin")
 	_, err = deps.EvmKeeper.CallContract(
 		deps.Ctx,
@@ -255,8 +253,6 @@ func (s *FunTokenFromCoinSuite) TestConvertCoinToEvmAndBack() {
 	s.Require().NoError(err)
 	s.Require().Equal("0", balance.String())
 
-	deps.ResetGasMeter()
-
 	s.T().Log("sad: Convert more erc-20 to back to bank coin, insufficient funds")
 	_, err = deps.EvmKeeper.CallContract(
 		deps.Ctx,
@@ -358,7 +354,6 @@ func (s *FunTokenFromCoinSuite) TestNativeSendThenPrecompileSend() {
 	}.Assert(s.T(), deps)
 
 	s.T().Log("call test contract")
-	deps.ResetGasMeter()
 	newSendAmtSendToBank := new(big.Int).Quo(sendAmt, big.NewInt(2))
 	newSendAmtEvmTransfer := evm.NativeToWei(newSendAmtSendToBank)
 	evmResp, err := deps.EvmKeeper.CallContract(
@@ -402,7 +397,6 @@ func (s *FunTokenFromCoinSuite) TestNativeSendThenPrecompileSend() {
 		BalanceERC20: big.NewInt(0),
 	}.Assert(s.T(), deps)
 
-	deps.ResetGasMeter()
 	evmResp, err = deps.EvmKeeper.CallContract(
 		deps.Ctx,
 		embeds.SmartContract_TestNativeSendThenPrecompileSendJson.ABI,

diff --git a/x/evm/keeper/funtoken_from_erc20_test.go b/x/evm/keeper/funtoken_from_erc20_test.go
@@ -211,8 +211,6 @@ func (s *FunTokenFromErc20Suite) TestSendFromEvmToBank() {
 
 	randomAcc := testutil.AccAddress()
 
-	deps.ResetGasMeter()
-
 	s.T().Log("send erc20 tokens to Bank")
 	_, err = deps.EvmKeeper.CallContract(
 		deps.Ctx,
@@ -235,8 +233,6 @@ func (s *FunTokenFromErc20Suite) TestSendFromEvmToBank() {
 		deps.App.BankKeeper.GetBalance(deps.Ctx, randomAcc, bankDemon).Amount,
 	)
 
-	deps.ResetGasMeter()
-
 	s.T().Log("sad: send too many erc20 tokens to Bank")
 	evmResp, err := deps.EvmKeeper.CallContract(
 		deps.Ctx,
@@ -253,8 +249,6 @@ func (s *FunTokenFromErc20Suite) TestSendFromEvmToBank() {
 	s.T().Log("check balances")
 	s.Require().Error(err, evmResp.String())
 
-	deps.ResetGasMeter()
-
 	s.T().Log("send Bank tokens back to erc20")
 	_, err = deps.EvmKeeper.ConvertCoinToEvm(sdk.WrapSDKContext(deps.Ctx),
 		&evm.MsgConvertCoinToEvm{
@@ -367,8 +361,6 @@ func (s *FunTokenFromErc20Suite) TestFunTokenFromERC20MaliciousTransfer() {
 	s.Require().NoError(err)
 	randomAcc := testutil.AccAddress()
 
-	deps.ResetGasMeter()
-
 	s.T().Log("send erc20 tokens to cosmos")
 	_, err = deps.EvmKeeper.CallContract(
 		deps.Ctx,
@@ -423,8 +415,6 @@ func (s *FunTokenFromErc20Suite) TestFunTokenInfiniteRecursionERC20() {
 	)
 	s.Require().NoError(err)
 
-	deps.ResetGasMeter()
-
 	s.T().Log("happy: call attackBalance()")
 	res, err := deps.EvmKeeper.CallContract(
 		deps.Ctx,
@@ -490,8 +480,6 @@ func (s *FunTokenFromErc20Suite) TestSendERC20WithFee() {
 
 	randomAcc := testutil.AccAddress()
 
-	deps.ResetGasMeter()
-
 	s.T().Log("send erc20 tokens to Bank")
 	_, err = deps.EvmKeeper.CallContract(
 		deps.Ctx,
@@ -513,8 +501,6 @@ func (s *FunTokenFromErc20Suite) TestSendERC20WithFee() {
 	evmtest.AssertERC20BalanceEqual(s.T(), deps, deployResp.ContractAddr, evm.EVM_MODULE_ADDRESS, big.NewInt(90))
 	s.Require().Equal(sdk.NewInt(90), deps.App.BankKeeper.GetBalance(deps.Ctx, randomAcc, bankDemon).Amount)
 
-	deps.ResetGasMeter()
-
 	s.T().Log("send Bank tokens back to erc20")
 	_, err = deps.EvmKeeper.ConvertCoinToEvm(sdk.WrapSDKContext(deps.Ctx),
 		&evm.MsgConvertCoinToEvm{

diff --git a/x/evm/keeper/gas_fees.go b/x/evm/keeper/gas_fees.go
@@ -74,14 +74,6 @@ func (k *Keeper) RefundGas(
 	return nil
 }
 
-// ResetGasMeterAndConsumeGas reset first the gas meter consumed value to zero
-// and set it back to the new value 'gasUsed'.
-func (k *Keeper) ResetGasMeterAndConsumeGas(ctx sdk.Context, gasUsed uint64) {
-	// reset the gas count
-	ctx.GasMeter().RefundGas(ctx.GasMeter().GasConsumed(), "reset the gas count")
-	ctx.GasMeter().ConsumeGas(gasUsed, "apply evm transaction")
-}
-
 // GasToRefund calculates the amount of gas the state machine should refund to
 // the sender. It is capped by the refund quotient value. Note that passing a
 // jrefundQuotient of 0 will cause problems.

diff --git a/x/evm/keeper/keeper.go b/x/evm/keeper/keeper.go
@@ -9,7 +9,6 @@ import (
 	"github.com/ethereum/go-ethereum/core/vm"
 	gethparams "github.com/ethereum/go-ethereum/params"
 
-	"cosmossdk.io/errors"
 	"github.com/cometbft/cometbft/libs/log"
 
 	"github.com/cosmos/cosmos-sdk/codec"
@@ -97,24 +96,6 @@ func (k Keeper) EthChainID(ctx sdk.Context) *big.Int {
 	return appconst.GetEthChainID(ctx.ChainID())
 }
 
-// AddToBlockGasUsed accumulate gas used by each eth msgs included in current
-// block tx.
-func (k *Keeper) AddToBlockGasUsed(
-	ctx sdk.Context, gasUsed uint64,
-) (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, errors.Wrap(core.ErrGasUintOverflow, "transient gas used")
-	}
-	k.EvmState.BlockGasUsed.Set(ctx, blockGasUsed)
-
-	return blockGasUsed, nil
-}
-
 // BaseFeeMicronibiPerGas returns the gas base fee in units of the EVM denom. Note
 // that this function is currently constant/stateless.
 func (k Keeper) BaseFeeMicronibiPerGas(_ sdk.Context) *big.Int {