test(evm): more bank extension tests and EVM ABCI integration tests to prevent regressions

CHANGELOG.md

@@ -46,6 +46,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 that gas consumed during any send operation of the "NibiruBankKeeper" depends
 only on the "bankkeeper.BaseKeeper"'s gas consumption.
 - [#2120](https://github.com/NibiruChain/nibiru/pull/2120) - fix: Use canonical hexadecimal strings for Eip155 address encoding 
+- [#2122](https://github.com/NibiruChain/nibiru/pull/2122) - test(evm): more bank extension tests
 
 #### Nibiru EVM | Before Audit 2 - 2024-12-06
 

eth/rpc/backend/account_info.go

@@ -83,7 +83,11 @@ func (b *Backend) GetProof(
 
 	for i, key := range storageKeys {
 		hexKey := gethcommon.HexToHash(key)
-		valueBz, proof, err := b.queryClient.GetProof(clientCtx, evm.StoreKey, evm.StateKey(address, hexKey.Bytes()))
+		valueBz, proof, err := b.queryClient.GetProof(
+			clientCtx,
+			evm.StoreKey,
+			evm.StateKey(address, hexKey.Bytes()),
+		)
 		if err != nil {
 			return nil, err
 		}

go.mod

@@ -61,12 +61,12 @@ require (
 	github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc
 	github.com/gorilla/websocket v1.5.0
 	github.com/rs/cors v1.8.3
+	github.com/rs/zerolog v1.32.0
 	github.com/status-im/keycard-go v0.0.0-20190316090335-8537d3370df4
 	golang.org/x/crypto v0.21.0
 	golang.org/x/exp v0.0.0-20231006140011-7918f672742d
 	golang.org/x/net v0.23.0
 	golang.org/x/text v0.14.0
-	github.com/rs/zerolog v1.32.0
 )
 
 require (

x/common/testutil/testapp/testapp.go

@@ -12,6 +12,7 @@ import (
 	"github.com/cosmos/cosmos-sdk/baseapp"
 	"github.com/cosmos/cosmos-sdk/testutil/sims"
 	sdk "github.com/cosmos/cosmos-sdk/types"
+	auth "github.com/cosmos/cosmos-sdk/x/auth/types"
 	bankkeeper "github.com/cosmos/cosmos-sdk/x/bank/keeper"
 
 	"github.com/NibiruChain/nibiru/v2/app"
@@ -183,6 +184,19 @@ func FundModuleAccount(
 	return bankKeeper.SendCoinsFromModuleToModule(ctx, inflationtypes.ModuleName, recipientMod, amounts)
 }
 
+// FundFeeCollector funds the module account that collects gas fees with some
+// amount of "unibi", the gas token.
+func FundFeeCollector(
+	bk bankkeeper.Keeper, ctx sdk.Context, amount math.Int,
+) error {
+	return FundModuleAccount(
+		bk,
+		ctx,
+		auth.FeeCollectorName,
+		sdk.NewCoins(sdk.NewCoin(appconst.BondDenom, amount)),
+	)
+}
+
 // EnsureNibiruPrefix sets the account address prefix to Nibiru's rather than
 // the default from the Cosmos-SDK, guaranteeing that tests will work with nibi
 // addresses rather than cosmos ones (for Gaia).

x/evm/evmtest/tx.go

@@ -209,11 +209,20 @@
 	return evmTxMsg, gethSigner, sender.KeyringSigner, nil
 }
 
-func TransferWei(
-	deps *TestDeps,
-	to gethcommon.Address,
-	amountWei *big.Int,
-) error {
+type TxTransferWei struct {
+	Deps      *TestDeps
+	To        gethcommon.Address
+	AmountWei *big.Int
+	GasLimit  uint64
+}
+
+func (tx TxTransferWei) Run() (evmResp *evm.MsgEthereumTxResponse, err error) {
+	gasLimit := tx.GasLimit
+	if tx.GasLimit == 0 {
+		gasLimit = gethparams.TxGas
+	}
+	deps, to, amountWei := tx.Deps, tx.To, tx.AmountWei
+
 	ethAcc := deps.Sender
 	var innerTxData []byte = nil
 	var accessList gethcore.AccessList = nil
@@ -224,18 +233,18 @@
 		deps.NewStateDB().GetNonce(ethAcc.EthAddr),
 		&to,
 		amountWei,
-		gethparams.TxGas,
+		gasLimit,
 		accessList,
 	)
 	if err != nil {
-		return fmt.Errorf("error while transferring wei: %w", err)
+		return evmResp, fmt.Errorf("error while transferring wei: %w", err)
 	}
 
-	_, err = deps.App.EvmKeeper.EthereumTx(sdk.WrapSDKContext(deps.Ctx), ethTxMsg)
+	evmResp, err = deps.App.EvmKeeper.EthereumTx(sdk.WrapSDKContext(deps.Ctx), ethTxMsg)
 	if err != nil {
-		return fmt.Errorf("error while transferring wei: %w", err)
+		err = fmt.Errorf("error while transferring wei: %w", err)
 	}
-	return err
+	return evmResp, err
 }
 
 // --------------------------------------------------

x/evm/evmtest/tx_test.go

@@ -77,8 +77,13 @@ func (s *Suite) TestTransferWei() {
 
 	randomAcc := evmtest.NewEthPrivAcc()
 	to := randomAcc.EthAddr
-	err := evmtest.TransferWei(&deps, to, evm.NativeToWei(big.NewInt(420)))
-	s.Require().NoError(err)
+	evmResp, err := evmtest.TxTransferWei{
+		Deps:      &deps,
+		To:        to,
+		AmountWei: evm.NativeToWei(big.NewInt(420)),
+	}.Run()
+	s.Require().NoErrorf(err, "%#v", evmResp)
+	s.False(evmResp.Failed(), "%#v", evmResp)
 
 	evmtest.AssertBankBalanceEqual(
 		s.T(), deps, evm.EVMBankDenom, deps.Sender.EthAddr, big.NewInt(69_000),

x/evm/keeper/bank_extension.go

@@ -90,21 +90,28 @@ func (bk NibiruBankKeeper) ForceGasInvariant(
 	BaseOp func(ctx sdk.Context) error,
 	AfterOp func(ctx sdk.Context),
 ) error {
-	gasMeterBefore := ctx.GasMeter()
+	// Assign vars for the tx gas meter
+	gasMeterBefore := ctx.GasMeter() // Tx gas meter MUST be defined
 	gasConsumedBefore := gasMeterBefore.GasConsumed()
+	// Don't modify the "ctx.BlockGasMeter()" directly because this is
+	// handled in "BaseApp.runTx"
+
 	// Start baseGasConsumed at 0 in case we panic before BaseOp completes and
 	// baseGasConsumed gets a value assignment
 	baseOpGasConsumed := uint64(0)
+
 	defer func() {
 		gasMeterBefore.RefundGas(gasMeterBefore.GasConsumed(), "")
 		gasMeterBefore.ConsumeGas(gasConsumedBefore+baseOpGasConsumed, "NibiruBankKeeper invariant")
 	}()
+
 	// Note that because the ctx gas meter uses private variables to track gas,
 	// we have to branch off with a new gas meter instance to avoid mutating the
 	// "true" gas meter (called GasMeterBefore here).
 	ctx = ctx.
 		WithGasMeter(sdk.NewGasMeter(gasMeterBefore.Limit())).
-		WithKVGasConfig(store.GasConfig{})
+		WithKVGasConfig(zeroCostGasConfig).
+		WithTransientKVGasConfig(zeroCostGasConfig)
 
 	err := BaseOp(ctx)
 	baseOpGasConsumed = ctx.GasMeter().GasConsumed()
@@ -116,6 +123,16 @@ func (bk NibiruBankKeeper) ForceGasInvariant(
 	return nil
 }
 
+var zeroCostGasConfig store.GasConfig = store.GasConfig{
+	HasCost:          0,
+	DeleteCost:       0,
+	ReadCostFlat:     0,
+	ReadCostPerByte:  0,
+	WriteCostFlat:    0,
+	WriteCostPerByte: 0,
+	IterNextCostFlat: 0,
+}
+
 func (bk NibiruBankKeeper) SendCoins(
 	ctx sdk.Context,
 	fromAddr sdk.AccAddress,

x/evm/keeper/bank_extension_test.go

@@ -1,14 +1,21 @@
 package keeper_test
 
 import (
+	"encoding/json"
 	"fmt"
 
+	"cosmossdk.io/math"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	staking "github.com/cosmos/cosmos-sdk/x/staking/types"
+	"github.com/ethereum/go-ethereum/common/hexutil"
+	gethparams "github.com/ethereum/go-ethereum/params"
+	"github.com/rs/zerolog/log"
 
 	"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"
+	"github.com/NibiruChain/nibiru/v2/x/evm/statedb"
 )
 
 // TestGasConsumedInvariantSend: The "NibiruBankKeeper" is defined such that
@@ -301,3 +308,136 @@ func (s *Suite) TestGasConsumedInvariantOther() {
 		}.Run(s)
 	})
 }
+
+// TestStateDBReadonlyInvariant: The EVM Keeper's "ApplyEvmMsg" function is used
+// in both queries and transaction messages. Queries such as "eth_call",
+// "eth_estimateGas", "debug_traceTransaction", "debugTraceCall", and
+// "debug_traceBlock" interact with the EVM StateDB inside of "ApplyEvmMsg".
+//
+// Queries MUST NOT result in lingering effects on the blockchain multistore or
+// the keepers, as doing so would result in potential inconsistencies between
+// nodes and consensus failures. This test adds cases to make sure that invariant
+// is held.
+func (s *Suite) TestStateDBReadonlyInvariant() {
+	deps := evmtest.NewTestDeps()
+	_, _, erc20Contract := evmtest.DeployAndExecuteERC20Transfer(&deps, s.T())
+	to := evmtest.NewEthPrivAcc()
+
+	type StateDBWithExplanation struct {
+		StateDB     *statedb.StateDB
+		Explanation string
+		ExpectEqual bool
+	}
+
+	var stateDBs []StateDBWithExplanation
+	stateDBs = append(stateDBs, StateDBWithExplanation{
+		StateDB:     deps.App.EvmKeeper.Bank.StateDB,
+		Explanation: "initial DB after some EthereumTx",
+		ExpectEqual: true,
+	})
+	resetBank := func(deps *evmtest.TestDeps) {
+		deps.App.EvmKeeper.Bank.StateDB = stateDBs[0].StateDB
+	}
+
+	s.T().Log("eth_call")
+	{
+		fungibleTokenContract := embeds.SmartContract_ERC20Minter
+		jsonTxArgs, err := json.Marshal(&evm.JsonTxArgs{
+			From: &deps.Sender.EthAddr,
+			Data: (*hexutil.Bytes)(&fungibleTokenContract.Bytecode),
+		})
+		s.Require().NoError(err)
+		req := &evm.EthCallRequest{Args: jsonTxArgs}
+		_, err = deps.EvmKeeper.EthCall(deps.GoCtx(), req)
+		s.Require().NoError(err)
+		stateDBs = append(stateDBs, StateDBWithExplanation{
+			StateDB:     deps.App.EvmKeeper.Bank.StateDB,
+			Explanation: "DB after eth_call query",
+			ExpectEqual: true,
+		})
+	}
+
+	s.T().Log(`EthereumTx success, err == nil, vmError="insufficient balance for transfer"`)
+	{
+		balOfSender := deps.App.BankKeeper.GetBalance(
+			deps.Ctx, deps.Sender.NibiruAddr, evm.EVMBankDenom)
+		tooManyTokensWei := evm.NativeToWei(balOfSender.Amount.AddRaw(420).BigInt())
+		evmResp, err := evmtest.TxTransferWei{
+			Deps:      &deps,
+			To:        to.EthAddr,
+			AmountWei: tooManyTokensWei,
+		}.Run()
+		s.Require().NoErrorf(err, "%#v", evmResp)
+		s.Require().Contains(evmResp.VmError, "insufficient balance for transfer")
+		stateDBs = append(stateDBs, StateDBWithExplanation{
+			StateDB:     deps.App.EvmKeeper.Bank.StateDB,
+			Explanation: "DB after EthereumTx with vmError",
+			ExpectEqual: false,
+		})
+	}
+
+	resetBank(&deps)
+	stateDBs = append(stateDBs, StateDBWithExplanation{
+		StateDB:     deps.App.EvmKeeper.Bank.StateDB,
+		Explanation: "sanity check with original ctx",
+		ExpectEqual: true,
+	})
+
+	s.T().Log(`EthereumTx success, err == nil, no vmError"`)
+	{
+		sendCoins := sdk.NewCoins(sdk.NewInt64Coin(evm.EVMBankDenom, 420))
+		s.NoError(
+			testapp.FundAccount(deps.App.BankKeeper, deps.Ctx, deps.Sender.NibiruAddr, sendCoins),
+		)
+
+		ctx := deps.Ctx
+		log.Log().Msgf("ctx.GasMeter().GasConsumed() %d", ctx.GasMeter().GasConsumed())
+		log.Log().Msgf("ctx.GasMeter().Limit() %d", ctx.GasMeter().Limit())
+
+		wei := evm.NativeToWei(sendCoins[0].Amount.BigInt())
+		evmResp, err := evmtest.TxTransferWei{
+			Deps:      &deps,
+			To:        to.EthAddr,
+			AmountWei: wei,
+		}.Run()
+
+		s.Require().NoErrorf(err, "%#v", evmResp)
+		s.Require().Falsef(evmResp.Failed(), "%#v", evmResp)
+		stateDBs = append(stateDBs, StateDBWithExplanation{
+			StateDB:     deps.App.EvmKeeper.Bank.StateDB,
+			Explanation: "DB after EthereumTx success",
+			ExpectEqual: false,
+		})
+
+		for _, err := range []error{
+			testapp.FundAccount(deps.App.BankKeeper, deps.Ctx, deps.Sender.NibiruAddr, sendCoins),
+			testapp.FundFeeCollector(deps.App.BankKeeper, deps.Ctx,
+				math.NewIntFromUint64(gethparams.TxGas),
+			),
+		} {
+			s.NoError(err)
+		}
+		evmResp, err = evmtest.TxTransferWei{
+			Deps:      &deps,
+			To:        erc20Contract,
+			AmountWei: wei,
+			GasLimit:  gethparams.TxGas * 2,
+		}.Run()
+		s.Require().NoErrorf(err, "%#v", evmResp)
+		s.Require().Contains(evmResp.VmError, "execution reverted")
+	}
+
+	s.T().Log("Verify that the NibiruBankKeeper.StateDB is unaffected")
+	var first *statedb.StateDB
+	for idx, db := range stateDBs {
+		if idx == 0 {
+			first = db.StateDB
+			continue
+		}
+		if db.ExpectEqual {
+			s.True(first == db.StateDB, db.Explanation)
+			continue
+		}
+		s.False(first == db.StateDB, db.Explanation)
+	}
+}

x/evm/keeper/grpc_query_test.go

@@ -489,10 +489,9 @@ func (s *Suite) TestQueryEthCall() {
 			wantErr: "InvalidArgument",
 		},
 		{
-			name: "happy: eth call for erc20 token transfer",
+			name: "happy: eth call to deploy erc20 contract",
 			scenario: func(deps *evmtest.TestDeps) (req In, wantResp Out) {
 				fungibleTokenContract := embeds.SmartContract_ERC20Minter
-
 				jsonTxArgs, err := json.Marshal(&evm.JsonTxArgs{
 					From: &deps.Sender.EthAddr,
 					Data: (*hexutil.Bytes)(&fungibleTokenContract.Bytecode),

x/evm/keeper/keeper_test.go

@@ -3,6 +3,7 @@ package keeper_test
 import (
 	"testing"
 
+	"github.com/rs/zerolog/log"
 	"github.com/stretchr/testify/suite"
 )
 
@@ -14,3 +15,9 @@ type Suite struct {
 func TestSuite(t *testing.T) {
 	suite.Run(t, new(Suite))
 }
+
+var _ suite.SetupTestSuite = (*Suite)(nil)
+
+func (s *Suite) SetupTest() {
+	log.Log().Msgf("SetupTest %v", s.T().Name())
+}

x/evm/keeper/msg_ethereum_tx_test.go

@@ -38,7 +38,7 @@ func (s *Suite) TestMsgEthereumTx_CreateContract() {
 					deps.App.BankKeeper,
 					deps.Ctx,
 					authtypes.FeeCollectorName,
-					sdk.NewCoins(sdk.NewCoin("unibi", math.NewInt(1000_000))),
+					sdk.NewCoins(sdk.NewCoin("unibi", math.NewInt(1_000_000))),
 				)
 				s.Require().NoError(err)
 				s.T().Log("create eth tx msg, increase gas limit")