fix(evm): commit temporary state on precompile start, avoiding bug stemming from uncommitted, dirty journal entries in the EVM StateDB

CHANGELOG.md

@@ -40,17 +40,24 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
-### State Machine Breaking
+### Nibiru EVM
 
-#### For next mainnet version
+#### Nibiru EVM | Before Audit 2 [Nov, 2024]
 
-- [#1766](https://github.com/NibiruChain/nibiru/pull/1766) - refactor(app-wasmext)!: remove wasmbinding `CosmosMsg::Custom` bindings.
-- [#1776](https://github.com/NibiruChain/nibiru/pull/1776) - feat(inflation): make inflation params a collection and add commands to update them
-- [#1872](https://github.com/NibiruChain/nibiru/pull/1872) - chore(math): use cosmossdk.io/math to replace sdk types
-- [#1874](https://github.com/NibiruChain/nibiru/pull/1874) - chore(proto): remove the proto stringer as per Cosmos SDK migration guidelines
-- [#1932](https://github.com/NibiruChain/nibiru/pull/1932) - fix(gosdk): fix keyring import functions
+The codebase went through a third-party [Code4rena
+Zenith](https://code4rena.com/zenith) Audit, running from 2024-10-07 until
+2024-11-01 and including both a primary review period and mitigation/remission
+period. This section describes code changes that occured after that audit in
+preparation for a second audit starting in November 2024.
+
+- [#2074](https://github.com/NibiruChain/nibiru/pull/2074) - fix(evm-keeper): better utilize ERC20 metadata during FunToken creation. The bank metadata for a new FunToken mapping ties a connection between the Bank Coin's `DenomUnit` and the ERC20 contract metadata like the name, decimals, and symbol.  This change brings parity between EVM wallets, such as MetaMask, and Interchain wallets like Keplr and Leap.
+- [#2076](https://github.com/NibiruChain/nibiru/pull/2076) - fix(evm-gas-fees):
+Use effective gas price in RefundGas and make sure that units are properly
+reflected on all occurences of "base fee" in the codebase. This fixes [#2059](https://github.com/NibiruChain/nibiru/issues/2059)
+and the [related comments from @Unique-Divine and @berndartmueller](https://github.com/NibiruChain/nibiru/issues/2059#issuecomment-2408625724).
+- [#2086](https://github.com/NibiruChain/nibiru/pull/2086) - fix(evm-precompiles): It is possible for the `Run` function of a custom precompile to retrieve stale state because EVM state changes can occur before the precompile is called that are recorded as entries in the StateDB journal for the transaction without being reflected in the `sdk.Context`. This pull request makes sure that state is committed as expected.
 
-#### Nibiru EVM
+#### Nibiru EVM | Before Audit 1 - 2024-10-18
 
 - [#1837](https://github.com/NibiruChain/nibiru/pull/1837) - feat(eth): protos, eth types, and evm module types
 - [#1838](https://github.com/NibiruChain/nibiru/pull/1838) - feat(eth): Go-ethereum, crypto, encoding, and unit tests for evm/types
@@ -70,7 +77,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - [#1909](https://github.com/NibiruChain/nibiru/pull/1909) - chore(evm): set is_london true by default and removed from config
 - [#1911](https://github.com/NibiruChain/nibiru/pull/1911) - chore(evm): simplified config by removing old eth forks
 - [#1912](https://github.com/NibiruChain/nibiru/pull/1912) - test(evm): unit tests for evm_ante
-- [#1914](https://github.com/NibiruChain/nibiru/pull/1914) - refactor(evm): Remove dead code and document non-EVM ante handler- [#1917](https://github.com/NibiruChain/nibiru/pull/1917) - test(e2e-evm): TypeScript support. Type generation from compiled contracts. Formatter for TS code.
+- [#1914](https://github.com/NibiruChain/nibiru/pull/1914) - refactor(evm): Remove dead code and document non-EVM ante handler
 - [#1917](https://github.com/NibiruChain/nibiru/pull/1917) - test(e2e-evm): TypeScript support. Type generation from compiled contracts. Formatter for TS code.
 - [#1922](https://github.com/NibiruChain/nibiru/pull/1922) - feat(evm): tracer option is read from the config.
 - [#1936](https://github.com/NibiruChain/nibiru/pull/1936) - feat(evm): EVM fungible token protobufs and encoding tests
@@ -128,12 +135,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - [#2060](https://github.com/NibiruChain/nibiru/pull/2060) - fix(evm-precompiles): add assertNumArgs validation
 - [#2056](https://github.com/NibiruChain/nibiru/pull/2056) - feat(evm): add oracle precompile
 - [#2065](https://github.com/NibiruChain/nibiru/pull/2065) - refactor(evm)!: Refactor out dead code from the evm.Params
-- [#2073](https://github.com/NibiruChain/nibiru/pull/2073) - fix(evm-keeper): better utilize ERC20 metadata during FunToken creation
-- [#2076](https://github.com/NibiruChain/nibiru/pull/2076) - fix(evm-gas-fees):
-Use effective gas price in RefundGas and make sure that units are properly
-reflected on all occurences of "base fee" in the codebase. This fixes [#2059](https://github.com/NibiruChain/nibiru/issues/2059)
-and the [related comments from @Unique-Divine and @berndartmueller](https://github.com/NibiruChain/nibiru/issues/2059#issuecomment-2408625724).
 
+### State Machine Breaking (Other)
+
+#### For next mainnet version
+
+- [#1766](https://github.com/NibiruChain/nibiru/pull/1766) - refactor(app-wasmext)!: remove wasmbinding `CosmosMsg::Custom` bindings.
+- [#1776](https://github.com/NibiruChain/nibiru/pull/1776) - feat(inflation): make inflation params a collection and add commands to update them
+- [#1872](https://github.com/NibiruChain/nibiru/pull/1872) - chore(math): use cosmossdk.io/math to replace sdk types
+- [#1874](https://github.com/NibiruChain/nibiru/pull/1874) - chore(proto): remove the proto stringer as per Cosmos SDK migration guidelines
+- [#1932](https://github.com/NibiruChain/nibiru/pull/1932) - fix(gosdk): fix keyring import functions
 
 #### Dapp modules: perp, spot, oracle, etc
 

x/evm/evmtest/erc20.go

@@ -23,7 +23,7 @@ func AssertERC20BalanceEqual(
 ) {
 	actualBalance, err := deps.EvmKeeper.ERC20().BalanceOf(erc20, account, deps.Ctx)
 	assert.NoError(t, err)
-	assert.Zero(t, expectedBalance.Cmp(actualBalance), "expected %s, got %s", expectedBalance, actualBalance)
+	assert.Equal(t, expectedBalance.String(), actualBalance.String(), "expected %s, got %s", expectedBalance, actualBalance)
 }
 
 // CreateFunTokenForBankCoin: Uses the "TestDeps.Sender" account to create a

x/evm/evmtest/test_deps.go

@@ -1,6 +1,8 @@
 package evmtest
 
 import (
+	"context"
+
 	sdk "github.com/cosmos/cosmos-sdk/types"
 
 	gethcommon "github.com/ethereum/go-ethereum/common"
@@ -54,3 +56,7 @@
 func (deps *TestDeps) GethSigner() gethcore.Signer {
 	return gethcore.LatestSignerForChainID(deps.App.EvmKeeper.EthChainID(deps.Ctx))
 }
+
+func (deps TestDeps) GoCtx() context.Context {
+	return sdk.WrapSDKContext(deps.Ctx)
+}

x/evm/evmtest/tx.go

@@ -20,8 +20,11 @@
 
 	srvconfig "github.com/NibiruChain/nibiru/v2/app/server/config"
 
+	"github.com/cosmos/cosmos-sdk/crypto/keyring"
+
 	"github.com/NibiruChain/nibiru/v2/x/evm"
 	"github.com/NibiruChain/nibiru/v2/x/evm/embeds"
+	"github.com/NibiruChain/nibiru/v2/x/evm/statedb"
 )
 
 type GethTxType = uint8
@@ -123,7 +126,9 @@
 		To:    &recipient,
 		Nonce: (*hexutil.Uint64)(&nonce),
 	}
-	ethTxMsg, err := GenerateAndSignEthTxMsg(txArgs, deps)
+	ethTxMsg, gethSigner, krSigner, err := GenerateEthTxMsgAndSigner(txArgs, deps, deps.Sender)
+	require.NoError(t, err)
+	err = ethTxMsg.Sign(gethSigner, krSigner)
 	require.NoError(t, err)
 
 	resp, err := deps.App.EvmKeeper.EthereumTx(sdk.WrapSDKContext(deps.Ctx), ethTxMsg)
@@ -153,18 +158,20 @@
 	bytecodeForCall := append(contract.Bytecode, packedArgs...)
 
 	nonce := deps.StateDB().GetNonce(deps.Sender.EthAddr)
-	msgEthTx, err := GenerateAndSignEthTxMsg(
+	ethTxMsg, gethSigner, krSigner, err := GenerateEthTxMsgAndSigner(
 		evm.JsonTxArgs{
 			Nonce: (*hexutil.Uint64)(&nonce),
 			Input: (*hexutil.Bytes)(&bytecodeForCall),
 			From:  &deps.Sender.EthAddr,
-		}, deps,
+		}, deps, deps.Sender,
 	)
 	if err != nil {
 		return nil, errors.Wrap(err, "failed to generate and sign eth tx msg")
+	} else if err := ethTxMsg.Sign(gethSigner, krSigner); err != nil {
+		return nil, errors.Wrap(err, "failed to generate and sign eth tx msg")
 	}
 
-	resp, err := deps.App.EvmKeeper.EthereumTx(sdk.WrapSDKContext(deps.Ctx), msgEthTx)
+	resp, err := deps.App.EvmKeeper.EthereumTx(sdk.WrapSDKContext(deps.Ctx), ethTxMsg)
 	if err != nil {
 		return nil, errors.Wrap(err, "failed to execute ethereum tx")
 	}
@@ -174,7 +181,7 @@
 
 	return &DeployContractResult{
 		TxResp:       resp,
-		EthTxMsg:     msgEthTx,
+		EthTxMsg:     ethTxMsg,
 		ContractData: contract,
 		Nonce:        nonce,
 		ContractAddr: crypto.CreateAddress(deps.Sender.EthAddr, nonce),
@@ -210,23 +217,70 @@
 		Nonce: (*hexutil.Uint64)(&nonce),
 		Data:  (*hexutil.Bytes)(&input),
 	}
-	erc20Transfer, err = GenerateAndSignEthTxMsg(txArgs, deps)
+	erc20Transfer, gethSigner, krSigner, err := GenerateEthTxMsgAndSigner(txArgs, deps, deps.Sender)
+	require.NoError(t, err)
+	err = erc20Transfer.Sign(gethSigner, krSigner)
 	require.NoError(t, err)
 
-	resp, err := deps.App.EvmKeeper.EthereumTx(sdk.WrapSDKContext(deps.Ctx), erc20Transfer)
+	resp, err := deps.App.EvmKeeper.EthereumTx(deps.GoCtx(), erc20Transfer)
 	require.NoError(t, err)
 	require.Empty(t, resp.VmError)
 
 	return erc20Transfer, predecessors
 }
 
-// GenerateAndSignEthTxMsg estimates gas, sets gas limit and sings the tx
-func GenerateAndSignEthTxMsg(
-	jsonTxArgs evm.JsonTxArgs, deps *TestDeps,
-) (*evm.MsgEthereumTx, error) {
+func CallContractTx(
+	deps *TestDeps,
+	contractAddr gethcommon.Address,
+	input []byte,
+	sender EthPrivKeyAcc,
+) (ethTxMsg *evm.MsgEthereumTx, resp *evm.MsgEthereumTxResponse, err error) {
+	nonce := deps.StateDB().GetNonce(sender.EthAddr)
+	ethTxMsg, gethSigner, krSigner, err := GenerateEthTxMsgAndSigner(evm.JsonTxArgs{
+		From:  &sender.EthAddr,
+		To:    &contractAddr,
+		Nonce: (*hexutil.Uint64)(&nonce),
+		Data:  (*hexutil.Bytes)(&input),
+	}, deps, sender)
+	if err != nil {
+		err = fmt.Errorf("CallContract error during tx generation: %w", err)
+		return
+	}
+
+	txConfig := deps.EvmKeeper.TxConfig(deps.Ctx, gethcommon.HexToHash(ethTxMsg.Hash))
+	stateDB := statedb.New(deps.Ctx, &deps.EvmKeeper, txConfig)
+	err = stateDB.Commit()
+	if err != nil {
+		return
+	}
+
+	err = ethTxMsg.Sign(gethSigner, krSigner)
+	if err != nil {
+		err = fmt.Errorf("CallContract error during signature: %w", err)
+		return
+	}
+
+	resp, err = deps.EvmKeeper.EthereumTx(deps.GoCtx(), ethTxMsg)
+	return ethTxMsg, resp, err
+}
+
+// GenerateEthTxMsgAndSigner estimates gas, sets gas limit and returns signer for
+// the tx.
+//
+// Usage:
+//
+//	```go
+//	evmTxMsg, gethSigner, krSigner, _ := GenerateEthTxMsgAndSigner(
+//	    jsonTxArgs, &deps, sender,
+//	)
+//	err := evmTxMsg.Sign(gethSigner, sender.KeyringSigner)
+//	```
+func GenerateEthTxMsgAndSigner(
+	jsonTxArgs evm.JsonTxArgs, deps *TestDeps, sender EthPrivKeyAcc,
+) (evmTxMsg *evm.MsgEthereumTx, gethSigner gethcore.Signer, krSigner keyring.Signer, err error) {
 	estimateArgs, err := json.Marshal(&jsonTxArgs)
 	if err != nil {
-		return nil, err
+		return
 	}
 	res, err := deps.App.EvmKeeper.EstimateGas(
 		sdk.WrapSDKContext(deps.Ctx),
@@ -238,13 +292,13 @@
 		},
 	)
 	if err != nil {
-		return nil, err
+		return
 	}
 	jsonTxArgs.Gas = (*hexutil.Uint64)(&res.Gas)
 
-	msgEthTx := jsonTxArgs.ToMsgEthTx()
-	gethSigner := gethcore.LatestSignerForChainID(deps.App.EvmKeeper.EthChainID(deps.Ctx))
-	return msgEthTx, msgEthTx.Sign(gethSigner, deps.Sender.KeyringSigner)
+	evmTxMsg = jsonTxArgs.ToMsgEthTx()
+	gethSigner = gethcore.LatestSignerForChainID(deps.App.EvmKeeper.EthChainID(deps.Ctx))
+	return evmTxMsg, gethSigner, sender.KeyringSigner, nil
 }
 
 func TransferWei(

x/evm/keeper/erc20.go

@@ -213,7 +213,11 @@ func (k Keeper) CallContractWithInput(
 		return nil, errors.Wrapf(err, "failed to load evm config")
 	}
 
-	txConfig := statedb.NewEmptyTxConfig(gethcommon.BytesToHash(ctx.HeaderHash()))
+	blockHash := gethcommon.BytesToHash(ctx.HeaderHash())
+	txConfig := statedb.NewEmptyTxConfig(blockHash)
+	txConfig.TxIndex = uint(k.EvmState.BlockLogSize.GetOr(ctx, 0))
+	txConfig.LogIndex = uint(k.EvmState.BlockLogSize.GetOr(ctx, 0))
+
 	evmResp, err = k.ApplyEvmMsg(
 		ctx, evmMsg, evm.NewNoOpTracer(), commit, evmCfg, txConfig,
 	)

x/evm/keeper/funtoken_from_coin_test.go

@@ -282,7 +282,7 @@ func (s *FunTokenFromCoinSuite) TestConvertCoinToEvmAndBack() {
 	// Check 3: erc-20 balance
 	balance, err = deps.EvmKeeper.ERC20().BalanceOf(funTokenErc20Addr.Address, alice.EthAddr, deps.Ctx)
 	s.Require().NoError(err)
-	s.Require().Zero(balance.Cmp(big.NewInt(0)))
+	s.Require().Equal("0", balance.String())
 
 	s.T().Log("sad: Convert more erc-20 to back to bank coin, insufficient funds")
 	_, err = deps.EvmKeeper.CallContract(

x/evm/keeper/funtoken_from_erc20_test.go

@@ -160,7 +160,7 @@ func (s *FunTokenFromErc20Suite) TestCreateFunTokenFromERC20() {
 	s.ErrorContains(err, "either the \"from_erc20\" or \"from_bank_denom\" must be set")
 }
 
-func (s *FunTokenFromErc20Suite) TestSendFromEvmToCosmos() {
+func (s *FunTokenFromErc20Suite) TestSendFromEvmToBank() {
 	deps := evmtest.NewTestDeps()
 
 	s.T().Log("Deploy ERC20")
@@ -210,7 +210,7 @@ func (s *FunTokenFromErc20Suite) TestSendFromEvmToCosmos() {
 
 	randomAcc := testutil.AccAddress()
 
-	s.T().Log("send erc20 tokens to cosmos")
+	s.T().Log("send erc20 tokens to Bank")
 	_, err = deps.EvmKeeper.CallContract(
 		deps.Ctx,
 		embeds.SmartContract_FunToken.ABI,
@@ -231,8 +231,8 @@ func (s *FunTokenFromErc20Suite) TestSendFromEvmToCosmos() {
 		deps.App.BankKeeper.GetBalance(deps.Ctx, randomAcc, bankDemon).Amount,
 	)
 
-	s.T().Log("sad: send too many erc20 tokens to cosmos")
-	_, err = deps.EvmKeeper.CallContract(
+	s.T().Log("sad: send too many erc20 tokens to Bank")
+	evmResp, err := deps.EvmKeeper.CallContract(
 		deps.Ctx,
 		embeds.SmartContract_FunToken.ABI,
 		deps.Sender.EthAddr,
@@ -243,9 +243,10 @@ func (s *FunTokenFromErc20Suite) TestSendFromEvmToCosmos() {
 		big.NewInt(70_000),
 		randomAcc.String(),
 	)
-	s.Require().Error(err)
+	s.T().Log("check balances")
+	s.Require().Error(err, evmResp.String())
 
-	s.T().Log("send cosmos tokens back to erc20")
+	s.T().Log("send Bank tokens back to erc20")
 	_, err = deps.EvmKeeper.ConvertCoinToEvm(sdk.WrapSDKContext(deps.Ctx),
 		&evm.MsgConvertCoinToEvm{
 			ToEthAddr: eth.EIP55Addr{
@@ -264,7 +265,7 @@ func (s *FunTokenFromErc20Suite) TestSendFromEvmToCosmos() {
 		deps.App.BankKeeper.GetBalance(deps.Ctx, randomAcc, bankDemon).Amount.Equal(sdk.NewInt(0)),
 	)
 
-	s.T().Log("sad: send too many cosmos tokens back to erc20")
+	s.T().Log("sad: send too many Bank tokens back to erc20")
 	_, err = deps.EvmKeeper.ConvertCoinToEvm(sdk.WrapSDKContext(deps.Ctx),
 		&evm.MsgConvertCoinToEvm{
 			ToEthAddr: eth.EIP55Addr{

x/evm/logs.go

@@ -11,14 +11,6 @@ import (
 	"github.com/NibiruChain/nibiru/v2/eth"
 )
 
-// NewTransactionLogs creates a new NewTransactionLogs instance.
-func NewTransactionLogs(hash gethcommon.Hash, logs []*Log) TransactionLogs {
-	return TransactionLogs{
-		Hash: hash.String(),
-		Logs: logs,
-	}
-}
-
 // NewTransactionLogsFromEth creates a new NewTransactionLogs instance using []*ethtypes.Log.
 func NewTransactionLogsFromEth(hash gethcommon.Hash, ethlogs []*gethcore.Log) TransactionLogs {
 	return TransactionLogs{

x/evm/logs_test.go

@@ -193,7 +193,10 @@ func TestConversionFunctions(t *testing.T) {
 	conversionErr := conversionLogs.Validate()
 
 	// create new transaction logs as copy of old valid one (and validate)
-	copyLogs := evm.NewTransactionLogs(common.BytesToHash([]byte("tx_hash")), txLogs.Logs)
+	copyLogs := evm.TransactionLogs{
+		Hash: common.BytesToHash([]byte("tx_hash")).Hex(),
+		Logs: txLogs.Logs,
+	}
 	copyErr := copyLogs.Validate()
 
 	require.Nil(t, conversionErr)