fix(evm): proper nonce management in statedb (#2130)

* fix(evm): proper nonce management in statedb * chore: changelog update * test: fixed statedb journal test * fix: text comment --------- Co-authored-by: Unique Divine <[email protected]>
NibiruChain · Jan 8, 2025 · 2c4c224 · 2c4c224
1 parent 20531e7
commit 2c4c224
Show file tree

Hide file tree

Showing 6 changed files with 167 additions and 15 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -51,9 +51,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   Remove unnecessary argument in the `VerifyFee` function, which returns the token
   payment required based on the effective fee from the tx data. Improve
   documentation.
-- [#2127](https://github.com/NibiruChain/nibiru/pull/2127) - fix(vesting): disabled built in auth/vesting module functionality
 - [#2125](https://github.com/NibiruChain/nibiru/pull/2125) - feat(evm-precompile):Emit EVM events created to reflect the ABCI events that occur outside the EVM to make sure that block explorers and indexers can find indexed ABCI event information.
+- [#2127](https://github.com/NibiruChain/nibiru/pull/2127) - fix(vesting): disabled built in auth/vesting module functionality
 - [#2129](https://github.com/NibiruChain/nibiru/pull/2129) - fix(evm): issue with infinite recursion in erc20 funtoken contracts
+- [#2130](https://github.com/NibiruChain/nibiru/pull/2130) - fix(evm): proper nonce management in statedb
 - [#2134](https://github.com/NibiruChain/nibiru/pull/2134) - fix(evm): query of NIBI should use bank state, not the StateDB
 - [#2139](https://github.com/NibiruChain/nibiru/pull/2139) - fix(evm): erc20 born funtoken: properly burn bank coins after converting coin back to erc20 
 - [#2140](https://github.com/NibiruChain/nibiru/pull/2140) - fix(bank): bank keeper extension now charges gas for the bank operations

diff --git a/eth/rpc/backend/backend_suite_test.go b/eth/rpc/backend/backend_suite_test.go
@@ -8,6 +8,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/cosmos/cosmos-sdk/client/flags"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	gethcommon "github.com/ethereum/go-ethereum/common"
 	gethcore "github.com/ethereum/go-ethereum/core/types"
@@ -115,13 +116,12 @@ func (s *BackendSuite) SendNibiViaEthTransfer(
 	amount *big.Int,
 	waitForNextBlock bool,
 ) gethcommon.Hash {
-	nonce, err := s.backend.GetTransactionCount(s.fundedAccEthAddr, rpc.EthPendingBlockNumber)
-	s.Require().NoError(err)
+	nonce := s.getCurrentNonce(s.fundedAccEthAddr)
 	return SendTransaction(
 		s,
 		&gethcore.LegacyTx{
 			To:       &to,
-			Nonce:    uint64(*nonce),
+			Nonce:    uint64(nonce),
 			Value:    amount,
 			Gas:      params.TxGas,
 			GasPrice: big.NewInt(1),
@@ -135,20 +135,20 @@ func (s *BackendSuite) DeployTestContract(waitForNextBlock bool) (gethcommon.Has
 	packedArgs, err := embeds.SmartContract_TestERC20.ABI.Pack("")
 	s.Require().NoError(err)
 	bytecodeForCall := append(embeds.SmartContract_TestERC20.Bytecode, packedArgs...)
-	nonce, err := s.backend.GetTransactionCount(s.fundedAccEthAddr, rpc.EthPendingBlockNumber)
+	nonce := s.getCurrentNonce(s.fundedAccEthAddr)
 	s.Require().NoError(err)
 
 	txHash := SendTransaction(
 		s,
 		&gethcore.LegacyTx{
-			Nonce:    uint64(*nonce),
+			Nonce:    uint64(nonce),
 			Data:     bytecodeForCall,
 			Gas:      1500_000,
 			GasPrice: big.NewInt(1),
 		},
 		waitForNextBlock,
 	)
-	contractAddr := crypto.CreateAddress(s.fundedAccEthAddr, (uint64)(*nonce))
+	contractAddr := crypto.CreateAddress(s.fundedAccEthAddr, nonce)
 	return txHash, contractAddr
 }
 
@@ -188,3 +188,73 @@ func WaitForReceipt(s *BackendSuite, txHash gethcommon.Hash) (*big.Int, *gethcom
 		}
 	}
 }
+
+// getCurrentNonce returns the current nonce of the funded account
+func (s *BackendSuite) getCurrentNonce(address gethcommon.Address) uint64 {
+	bn, err := s.backend.BlockNumber()
+	s.Require().NoError(err)
+	currentHeight := rpc.BlockNumber(bn)
+
+	nonce, err := s.backend.GetTransactionCount(address, currentHeight)
+	s.Require().NoError(err)
+
+	return uint64(*nonce)
+}
+
+// broadcastSDKTx broadcasts the given SDK transaction and returns the response
+func (s *BackendSuite) broadcastSDKTx(sdkTx sdk.Tx) *sdk.TxResponse {
+	txBytes, err := s.backend.ClientCtx().TxConfig.TxEncoder()(sdkTx)
+	s.Require().NoError(err)
+
+	syncCtx := s.backend.ClientCtx().WithBroadcastMode(flags.BroadcastSync)
+	rsp, err := syncCtx.BroadcastTx(txBytes)
+	s.Require().NoError(err)
+	return rsp
+}
+
+// buildContractCreationTx builds a contract creation transaction
+func (s *BackendSuite) buildContractCreationTx(nonce 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,
+		GasPrice: big.NewInt(1),
+	}
+
+	signer := gethcore.LatestSignerForChainID(s.ethChainID)
+	signedTx, err := gethcore.SignNewTx(s.fundedAccPrivateKey, signer, creationTx)
+	s.Require().NoError(err)
+
+	return *signedTx
+}
+
+// buildContractCallTx builds a contract call transaction
+func (s *BackendSuite) buildContractCallTx(nonce uint64, contractAddr gethcommon.Address) gethcore.Transaction {
+	//recipient := crypto.CreateAddress(s.fundedAccEthAddr, 29381)
+	transferAmount := big.NewInt(100)
+
+	packedArgs, err := embeds.SmartContract_TestERC20.ABI.Pack(
+		"transfer",
+		recipient,
+		transferAmount,
+	)
+	s.Require().NoError(err)
+
+	transferTx := &gethcore.LegacyTx{
+		Nonce:    nonce,
+		Data:     packedArgs,
+		Gas:      100_000,
+		GasPrice: big.NewInt(1),
+		To:       &contractAddr,
+	}
+
+	signer := gethcore.LatestSignerForChainID(s.ethChainID)
+	signedTx, err := gethcore.SignNewTx(s.fundedAccPrivateKey, signer, transferTx)
+	s.Require().NoError(err)
+
+	return *signedTx
+}
diff --git a/eth/rpc/backend/call_tx.go b/eth/rpc/backend/call_tx.go
@@ -9,6 +9,7 @@ import (
 	"math/big"
 
 	errorsmod "cosmossdk.io/errors"
+	"github.com/cosmos/cosmos-sdk/client"
 	"github.com/cosmos/cosmos-sdk/client/flags"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/ethereum/go-ethereum/common"
@@ -338,3 +339,7 @@ func (b *Backend) GasPrice() (*hexutil.Big, error) {
 
 	return (*hexutil.Big)(result), nil
 }
+
+func (b *Backend) ClientCtx() client.Context {
+	return b.clientCtx
+}
diff --git a/eth/rpc/backend/nonce_test.go b/eth/rpc/backend/nonce_test.go
@@ -0,0 +1,75 @@
+package backend_test
+
+import (
+	sdkmath "cosmossdk.io/math"
+	codectypes "github.com/cosmos/cosmos-sdk/codec/types"
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	authtx "github.com/cosmos/cosmos-sdk/x/auth/tx"
+	gethcore "github.com/ethereum/go-ethereum/core/types"
+
+	"github.com/NibiruChain/nibiru/v2/x/evm"
+)
+
+// TestNonceIncrementWithMultipleMsgsTx tests that the nonce is incremented correctly
+// when multiple messages are included in a single transaction.
+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)
+
+	// Create and broadcast SDK transaction
+	sdkTx := s.buildSDKTxWithEVMMessages(
+		creationTx,
+		firstTransferTx,
+		secondTransferTx,
+	)
+
+	// Broadcast transaction
+	rsp := s.broadcastSDKTx(sdkTx)
+	s.Assert().NotEqual(rsp.Code, 0)
+	s.Require().NoError(s.network.WaitForNextBlock())
+
+	// Expected nonce should be incremented by 3
+	currentNonce := s.getCurrentNonce(s.fundedAccEthAddr)
+	s.Assert().Equal(nonce+3, currentNonce)
+
+	// Assert all transactions included in block
+	for _, tx := range []gethcore.Transaction{creationTx, firstTransferTx, secondTransferTx} {
+		blockNum, blockHash := WaitForReceipt(s, tx.Hash())
+		s.Require().NotNil(blockNum)
+		s.Require().NotNil(blockHash)
+	}
+}
+
+// buildSDKTxWithEVMMessages creates an SDK transaction with EVM messages
+func (s *BackendSuite) buildSDKTxWithEVMMessages(txs ...gethcore.Transaction) sdk.Tx {
+	msgs := make([]sdk.Msg, len(txs))
+	for i, tx := range txs {
+		msg := &evm.MsgEthereumTx{}
+		err := msg.FromEthereumTx(&tx)
+		s.Require().NoError(err)
+		msgs[i] = msg
+	}
+
+	option, err := codectypes.NewAnyWithValue(&evm.ExtensionOptionsEthereumTx{})
+	s.Require().NoError(err)
+
+	txBuilder, _ := s.backend.ClientCtx().TxConfig.NewTxBuilder().(authtx.ExtensionOptionsTxBuilder)
+	txBuilder.SetExtensionOptions(option)
+	err = txBuilder.SetMsgs(msgs...)
+	s.Require().NoError(err)
+
+	// Set fees for all messages
+	totalGas := uint64(0)
+	for _, tx := range txs {
+		totalGas += tx.Gas()
+	}
+	fees := sdk.NewCoins(sdk.NewCoin("unibi", sdkmath.NewIntFromUint64(totalGas)))
+	txBuilder.SetFeeAmount(fees)
+	txBuilder.SetGasLimit(totalGas)
+
+	return txBuilder.GetTx()
+}
diff --git a/x/evm/keeper/msg_server.go b/x/evm/keeper/msg_server.go
@@ -335,18 +335,17 @@ func (k *Keeper) ApplyEvmMsg(ctx sdk.Context,
 		return nil, evmObj, errors.Wrapf(err, "ApplyEvmMsg: invalid wei amount %s", msg.Value())
 	}
 
+	// take over the nonce management from evm:
+	// - reset sender's nonce to msg.Nonce() before calling evm.
+	// - increase sender's nonce by one no matter the result.
+	stateDB.SetNonce(sender.Address(), msg.Nonce())
 	if contractCreation {
-		// take over the nonce management from evm:
-		// - reset sender's nonce to msg.Nonce() before calling evm.
-		// - increase sender's nonce by one no matter the result.
-		stateDB.SetNonce(sender.Address(), msg.Nonce())
 		ret, _, leftoverGas, vmErr = evmObj.Create(
 			sender,
 			msg.Data(),
 			leftoverGas,
 			msgWei,
 		)
-		stateDB.SetNonce(sender.Address(), msg.Nonce()+1)
 	} else {
 		ret, leftoverGas, vmErr = evmObj.Call(
 			sender,
@@ -356,6 +355,8 @@ func (k *Keeper) ApplyEvmMsg(ctx sdk.Context,
 			msgWei,
 		)
 	}
+	// Increment nonce after processing the message
+	stateDB.SetNonce(sender.Address(), msg.Nonce()+1)
 
 	// EVM execution error needs to be available for the JSON-RPC client
 	var vmError string

diff --git a/x/evm/statedb/journal_test.go b/x/evm/statedb/journal_test.go
@@ -156,10 +156,10 @@ func (s *Suite) TestComplexJournalChanges() {
 		stateDB, ok = evmObj.StateDB.(*statedb.StateDB)
 		s.Require().True(ok, "error retrieving StateDB from the EVM")
 
-		s.T().Log("Expect exactly 0 dirty journal entry for the precompile snapshot")
-		if stateDB.DebugDirtiesCount() != 0 {
+		s.T().Log("Expect exactly 1 dirty journal entry for the precompile snapshot")
+		if stateDB.DebugDirtiesCount() != 1 {
 			debugDirtiesCountMismatch(stateDB, s.T())
-			s.FailNow("expected 0 dirty journal changes")
+			s.FailNow("expected 1 dirty journal change")
 		}
 
 		s.T().Log("Expect no change since the StateDB has not been committed")