From 2c4c224abc8139e654ccc3fbf6994a63b4df3cb2 Mon Sep 17 00:00:00 2001 From: Oleg Nikonychev Date: Wed, 8 Jan 2025 09:02:50 +0400 Subject: [PATCH] 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 <51418232+Unique-Divine@users.noreply.github.com> --- CHANGELOG.md | 3 +- eth/rpc/backend/backend_suite_test.go | 82 +++++++++++++++++++++++++-- eth/rpc/backend/call_tx.go | 5 ++ eth/rpc/backend/nonce_test.go | 75 ++++++++++++++++++++++++ x/evm/keeper/msg_server.go | 11 ++-- x/evm/statedb/journal_test.go | 6 +- 6 files changed, 167 insertions(+), 15 deletions(-) create mode 100644 eth/rpc/backend/nonce_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index f91fe5bce..52be83549 100644 --- 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 index 1ba9bd03e..80549a60a 100644 --- 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 index 66deeb8fb..19b9ee4fc 100644 --- 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 new file mode 100644 index 000000000..8f79fef1e --- /dev/null +++ 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 index 8b1f03c91..0b36e3a61 100644 --- 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 index e1260b819..058c09542 100644 --- 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")