From 03ac44e641c67da7cbaf58e0dc571991bf0801fe Mon Sep 17 00:00:00 2001 From: Oleg Nikonychev Date: Wed, 25 Dec 2024 21:49:53 +0400 Subject: [PATCH 1/4] fix(evm): proper nonce management in statedb --- 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 ++-- 4 files changed, 162 insertions(+), 11 deletions(-) create mode 100644 eth/rpc/backend/nonce_test.go 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 1e946066f..9c784bed2 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 From 6f886422c051c85218e9df0de86e580e8b292c74 Mon Sep 17 00:00:00 2001 From: Oleg Nikonychev Date: Wed, 25 Dec 2024 21:51:15 +0400 Subject: [PATCH 2/4] chore: changelog update --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index efc5f45f4..e439262c8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -51,6 +51,7 @@ only on the "bankkeeper.BaseKeeper"'s gas consumption. Remove unnecessary argument in the `VerifyFee` function, which returns the token payment required based on the effective fee from the tx data. Improve documentation. +- [#2130](https://github.com/NibiruChain/nibiru/pull/2130) - fix(evm): proper nonce management in statedb #### Nibiru EVM | Before Audit 2 - 2024-12-06 From caddee01992862bae076f8574a9727d3dda1d5e3 Mon Sep 17 00:00:00 2001 From: Oleg Nikonychev Date: Tue, 7 Jan 2025 21:38:12 +0400 Subject: [PATCH 3/4] test: fixed statedb journal test --- x/evm/statedb/journal_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/x/evm/statedb/journal_test.go b/x/evm/statedb/journal_test.go index e1260b819..68d7912c6 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 changes") } s.T().Log("Expect no change since the StateDB has not been committed") From 14a520030546b8d4dd741bec09c508157f00619a Mon Sep 17 00:00:00 2001 From: Oleg Nikonychev Date: Tue, 7 Jan 2025 21:56:33 +0400 Subject: [PATCH 4/4] fix: text comment --- x/evm/statedb/journal_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/evm/statedb/journal_test.go b/x/evm/statedb/journal_test.go index 68d7912c6..058c09542 100644 --- a/x/evm/statedb/journal_test.go +++ b/x/evm/statedb/journal_test.go @@ -159,7 +159,7 @@ func (s *Suite) TestComplexJournalChanges() { s.T().Log("Expect exactly 1 dirty journal entry for the precompile snapshot") if stateDB.DebugDirtiesCount() != 1 { debugDirtiesCountMismatch(stateDB, s.T()) - s.FailNow("expected 1 dirty journal changes") + s.FailNow("expected 1 dirty journal change") } s.T().Log("Expect no change since the StateDB has not been committed")