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,28 @@ 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]
+
+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).
+- [#2084](https://github.com/NibiruChain/nibiru/pull/2084) - feat(evm-forge): foundry support and template for Nibiru EVM develoment
+- [#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.
+- [#2088](https://github.com/NibiruChain/nibiru/pull/2088) - refactor(evm): remove outdated comment and improper error message text
+- [#2089](https://github.com/NibiruChain/nibiru/pull/2089) - better handling of gas consumption within erc20 contract execution
 
-- [#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
 
-#### 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 +81,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,15 +139,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).
-- [#2084](https://github.com/NibiruChain/nibiru/pull/2084) - feat(evm-forge): foundry support and template for Nibiru EVM develoment
-- [#2088](https://github.com/NibiruChain/nibiru/pull/2088) - refactor(evm): remove outdated comment and improper error message text
-- [#2089](https://github.com/NibiruChain/nibiru/pull/2089) - better handling of gas consumption within erc20 contract execution
 
+### 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/errors.go

@@ -74,9 +74,17 @@ var (
 func NewExecErrorWithReason(revertReason []byte) *RevertError {
 	result := common.CopyBytes(revertReason)
 	reason, errUnpack := abi.UnpackRevert(result)
-	err := errors.New("execution reverted")
+
+	var err error
+	errPrefix := "execution reverted"
 	if errUnpack == nil {
-		err = fmt.Errorf("execution reverted: %v", reason)
+		reasonStr := reason
+		err = fmt.Errorf("%s with reason \"%v\"", errPrefix, reasonStr)
+	} else if string(result) != "" {
+		reasonStr := string(result)
+		err = fmt.Errorf("%s with reason \"%v\"", errPrefix, reasonStr)
+	} else {
+		err = errors.New(errPrefix)
 	}
 	return &RevertError{
 		error:  err,

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
@@ -99,3 +99,9 @@ func AssertBankBalanceEqual(
 	actualBalance := deps.App.BankKeeper.GetBalance(deps.Ctx, bech32Addr, denom).Amount.BigInt()
 	assert.Zero(t, expectedBalance.Cmp(actualBalance), "expected %s, got %s", expectedBalance, actualBalance)
 }
+
+// BigPow multiplies "amount" by 10 to the "pow10Exp".
+func BigPow(amount *big.Int, pow10Exp uint8) (powAmount *big.Int) {
+	pow10 := new(big.Int).Exp(big.NewInt(10), big.NewInt(int64(pow10Exp)), nil)
+	return new(big.Int).Mul(amount, pow10)
+}

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) StateDB() *statedb.StateDB {
 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,6 +20,8 @@ import (
 
 	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"
 )
@@ -123,7 +125,9 @@ func ExecuteNibiTransfer(deps *TestDeps, t *testing.T) *evm.MsgEthereumTx {
 		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 +157,20 @@ func DeployContract(
 	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 +180,7 @@ func DeployContract(
 
 	return &DeployContractResult{
 		TxResp:       resp,
-		EthTxMsg:     msgEthTx,
+		EthTxMsg:     ethTxMsg,
 		ContractData: contract,
 		Nonce:        nonce,
 		ContractAddr: crypto.CreateAddress(deps.Sender.EthAddr, nonce),
@@ -184,15 +190,19 @@ func DeployContract(
 // DeployAndExecuteERC20Transfer deploys contract, executes transfer and returns tx hash
 func DeployAndExecuteERC20Transfer(
 	deps *TestDeps, t *testing.T,
-) (erc20Transfer *evm.MsgEthereumTx, predecessors []*evm.MsgEthereumTx) {
+) (
+	erc20Transfer *evm.MsgEthereumTx,
+	predecessors []*evm.MsgEthereumTx,
+	contractAddr gethcommon.Address,
+) {
 	// TX 1: Deploy ERC-20 contract
 	deployResp, err := DeployContract(deps, embeds.SmartContract_TestERC20)
 	require.NoError(t, err)
 	contractData := deployResp.ContractData
 	nonce := deployResp.Nonce
 
 	// Contract address is deterministic
-	contractAddress := crypto.CreateAddress(deps.Sender.EthAddr, nonce)
+	contractAddr = crypto.CreateAddress(deps.Sender.EthAddr, nonce)
 	deps.App.Commit()
 	predecessors = []*evm.MsgEthereumTx{
 		deployResp.EthTxMsg,
@@ -206,27 +216,67 @@ func DeployAndExecuteERC20Transfer(
 	nonce = deps.StateDB().GetNonce(deps.Sender.EthAddr)
 	txArgs := evm.JsonTxArgs{
 		From:  &deps.Sender.EthAddr,
-		To:    &contractAddress,
+		To:    &contractAddr,
 		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
+	return erc20Transfer, predecessors, contractAddr
 }
 
-// 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
+	}
+
+	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 +288,13 @@ func GenerateAndSignEthTxMsg(
 		},
 	)
 	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/evmtest/tx_test.go

@@ -0,0 +1,90 @@
+// Copyright (c) 2023-2024 Nibi, Inc.
+package evmtest_test
+
+import (
+	"math/big"
+
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	"github.com/ethereum/go-ethereum/crypto"
+
+	"github.com/NibiruChain/nibiru/v2/x/common/testutil/testapp"
+	"github.com/NibiruChain/nibiru/v2/x/evm"
+	"github.com/NibiruChain/nibiru/v2/x/evm/embeds"
+	"github.com/NibiruChain/nibiru/v2/x/evm/evmtest"
+)
+
+func (s *Suite) TestCallContractTx() {
+	deps := evmtest.NewTestDeps()
+
+	s.T().Log("Deploy some ERC20")
+	deployArgs := []any{"name", "SYMBOL", uint8(18)}
+	deployResp, err := evmtest.DeployContract(
+		&deps,
+		embeds.SmartContract_ERC20Minter,
+		deployArgs...,
+	)
+	s.Require().NoError(err, deployResp)
+	contractAddr := crypto.CreateAddress(deps.Sender.EthAddr, deployResp.Nonce)
+	gotContractAddr := deployResp.ContractAddr
+	s.Require().Equal(contractAddr, gotContractAddr)
+
+	s.T().Log("expect zero balance")
+	{
+		wantBal := big.NewInt(0)
+		evmtest.AssertERC20BalanceEqual(
+			s.T(), deps, contractAddr, deps.Sender.EthAddr, wantBal,
+		)
+	}
+
+	abi := deployResp.ContractData.ABI
+	s.T().Log("mint some tokens")
+	{
+		amount := big.NewInt(69_420)
+		to := deps.Sender.EthAddr
+		callArgs := []any{to, amount}
+		input, err := abi.Pack(
+			"mint", callArgs...,
+		)
+		s.Require().NoError(err)
+		_, resp, err := evmtest.CallContractTx(
+			&deps,
+			contractAddr,
+			input,
+			deps.Sender,
+		)
+		s.Require().NoError(err)
+		s.Require().Empty(resp.VmError)
+	}
+
+	s.T().Log("expect nonzero balance")
+	{
+		wantBal := big.NewInt(69_420)
+		evmtest.AssertERC20BalanceEqual(
+			s.T(), deps, contractAddr, deps.Sender.EthAddr, wantBal,
+		)
+	}
+}
+
+func (s *Suite) TestTransferWei() {
+	deps := evmtest.NewTestDeps()
+
+	s.Require().NoError(testapp.FundAccount(
+		deps.App.BankKeeper,
+		deps.Ctx,
+		deps.Sender.NibiruAddr,
+		sdk.NewCoins(sdk.NewCoin(evm.EVMBankDenom, sdk.NewInt(69_420))),
+	))
+
+	randomAcc := evmtest.NewEthPrivAcc()
+	to := randomAcc.EthAddr
+	err := evmtest.TransferWei(&deps, to, evm.NativeToWei(big.NewInt(420)))
+	s.Require().NoError(err)
+
+	evmtest.AssertBankBalanceEqual(
+		s.T(), deps, evm.EVMBankDenom, deps.Sender.EthAddr, big.NewInt(69_000),
+	)
+
+	s.Run("DeployAndExecuteERC20Transfer", func() {
+		evmtest.DeployAndExecuteERC20Transfer(&deps, s.T())
+	})
+}