fix(evm): disallow permissionless creation of FunToken mappings when tokens do not already have metadata. (#2379)

* wip!: impl checkpoint with passing tests before change to use of commit constants

* wip!: refactor: working checkpoint where CallContractWithInput only ever accepts commit=false

* wip!: checkpoint with consistent use of commit=false

* finish impl

* wip!: save for now to implement this feature after
[feat(evm): Implement a state (ctx) serializable EVM StateDB to make asynchronous access more safe](#2378)

* revert precompile edits to do them in a future upgrade

* wip!: all passing tests. want to add more

* finish impl

* docs: fix ultra small typo

* fix v2_5_0_test to start from 0 decimals

* fix(evm): robustness improvments on metadata constructor

* --amend

* fix(funtoken_from_erc20): add robustness checks against ERC20.symbols
with deterministic display denom during bank.Metadata generation

* fix(funtoken_from_erc20): pass through ERC20 name and symbol in raw form to allow bank.Metadata.Validate to protect against empty values

Author: Unique-Divine

CHANGELOG.md

@@ -56,6 +56,7 @@ handling for NIBI via WNIBI.
 - [#2371](https://github.com/NibiruChain/nibiru/pull/2371) - feat(evm): fix UnmarshalJSON to accept ASCII hex strings
 - [#2372](https://github.com/NibiruChain/nibiru/pull/2372) - feat(tokenfactory-cli): add CLI commands for set denom functions
 - [#2375](https://github.com/NibiruChain/nibiru/pull/2375) - feat(evm): Inject WNIBI.sol for non-mainnet networks in the v2.7.0 upgrade handler
+- [#2379](https://github.com/NibiruChain/nibiru/pull/2379) - fix(evm): disallow permissionless creation of FunToken mappings when tokens do not already have metadata.
 
 ### Dependencies
 - Bump `base-x` from 3.0.10 to 3.0.11 ([#2355](https://github.com/NibiruChain/nibiru/pull/2355))

api/eth/evm/v1/tx.pulsar.go

@@ -156,14 +156,18 @@ func UpgradeStNibiContractOnMainnet(
 	}()
 	evmObj := keepers.EvmKeeper.NewEVM(ctx, evmMsg, keepers.EvmKeeper.GetEVMConfig(ctx), nil, stateDB)
 
-	evmResp, err := keepers.EvmKeeper.CallContractWithInput(
-		ctx, evmObj, evmMsg.From, nil, true /*commit*/, contractInput,
-		evmkeeper.Erc20GasLimitDeploy, nil,
+	evmResp, err := keepers.EvmKeeper.CallContract(
+		ctx, evmObj, evmMsg.From, nil, contractInput,
+		evmkeeper.Erc20GasLimitDeploy,
+		evm.COMMIT_ETH_TX, /*commit*/
+		nil,
 	)
 	if err != nil {
 		return fmt.Errorf("failed to deploy ERC20 contract: %w", err)
 	} else if len(evmResp.VmError) > 0 {
 		return fmt.Errorf("VM Error in deploy ERC20: %s", evmResp.VmError)
+	} else if err := stateDB.Commit(); err != nil {
+		return fmt.Errorf("%s: %w", evm.ErrStateDBCommit, err)
 	}
 
 	evmLogs = append(evmLogs, evmResp.Logs...)

app/upgrades/v2_5_0/v2_5_0.go

@@ -8,6 +8,7 @@ import (
 
 	sdkmath "cosmossdk.io/math"
 	sdk "github.com/cosmos/cosmos-sdk/types"
+	bank "github.com/cosmos/cosmos-sdk/x/bank/types"
 	gogoproto "github.com/cosmos/gogoproto/proto"
 	gethcommon "github.com/ethereum/go-ethereum/common"
 	"github.com/stretchr/testify/suite"
@@ -39,9 +40,7 @@ func (s *Suite) TestUpgrade() {
 		}.DefaultBankMetadata()
 
 		// FunToken mapping for stNIBI
-		funtoken = evmtest.CreateFunTokenForBankCoin(
-			deps, originalbankMetadata.Base, &s.Suite,
-		)
+		funtoken evm.FunToken
 
 		// ten holders for testing
 		holders = []gethcommon.Address{
@@ -58,6 +57,65 @@ func (s *Suite) TestUpgrade() {
 		}
 	)
 
+	{
+		bankDenom := originalbankMetadata.Base
+
+		if deps.App.BankKeeper.HasDenomMetaData(deps.Ctx, bankDenom) {
+			s.Failf("setting bank.DenomMetadata would overwrite existing denom \"%s\"", bankDenom)
+		}
+
+		s.T().Log("Setup: Create a coin in the bank state")
+		bankMetadata := bank.Metadata{
+			DenomUnits: []*bank.DenomUnit{
+				{
+					Denom:    bankDenom,
+					Exponent: 0,
+				},
+			},
+			Base:    bankDenom,
+			Display: bankDenom,
+			Name:    bankDenom,
+			Symbol:  bankDenom,
+		}
+
+		deps.App.BankKeeper.SetDenomMetaData(deps.Ctx, bankMetadata)
+
+		// Give the sender funds for the fee
+		s.Require().NoError(testapp.FundAccount(
+			deps.App.BankKeeper,
+			deps.Ctx,
+			deps.Sender.NibiruAddr,
+			deps.EvmKeeper.FeeForCreateFunToken(deps.Ctx),
+		))
+
+		s.T().Log("happy: CreateFunToken for the bank coin")
+		createFuntokenResp, err := deps.EvmKeeper.CreateFunToken(
+			sdk.WrapSDKContext(deps.Ctx),
+			&evm.MsgCreateFunToken{
+				FromBankDenom:     bankDenom,
+				Sender:            deps.Sender.NibiruAddr.String(),
+				AllowZeroDecimals: true,
+			},
+		)
+		s.Require().NoError(err, "bankDenom %s", bankDenom)
+
+		erc20 := createFuntokenResp.FuntokenMapping.Erc20Addr
+		funtoken = evm.FunToken{
+			Erc20Addr:      erc20,
+			BankDenom:      bankDenom,
+			IsMadeFromCoin: true,
+		}
+		s.Equal(funtoken, createFuntokenResp.FuntokenMapping)
+
+		s.T().Log("Expect ERC20 to be deployed")
+		_, err = deps.EvmKeeper.Code(deps.Ctx,
+			&evm.QueryCodeRequest{
+				Address: erc20.String(),
+			},
+		)
+		s.NoError(err)
+	}
+
 	s.T().Log("Adds some tokens in circulation. We'll use these later to create ERC20 holders")
 	s.NoError(
 		testapp.FundAccount(
@@ -171,14 +229,14 @@ func (s *Suite) TestUpgrade() {
 		input, err := compiledContract.ABI.Pack("owner")
 		s.Require().NoError(err)
 		evmObj, _ := deps.NewEVMLessVerboseLogger()
-		evmResp, err := deps.EvmKeeper.CallContractWithInput(
+		evmResp, err := deps.EvmKeeper.CallContract(
 			deps.Ctx,
 			evmObj,
 			deps.Sender.EthAddr,
 			&funtoken.Erc20Addr.Address,
-			false, // commit
 			input,
 			evmkeeper.Erc20GasLimitQuery,
+			evm.COMMIT_READONLY, /*commit*/
 			nil,
 		)
 		s.Require().NoError(err)

app/upgrades/v2_5_0/v2_5_0_test.go

@@ -2,11 +2,10 @@
 package pubsub
 
 import (
+	"fmt"
 	"sync"
 	"sync/atomic"
 
-	pkgerrors "github.com/pkg/errors"
-
 	coretypes "github.com/cometbft/cometbft/rpc/core/types"
 )
 
@@ -72,7 +71,7 @@ func (m *memEventBus) AddTopic(name string, src <-chan coretypes.ResultEvent) er
 	m.topicsMux.RUnlock()
 
 	if ok {
-		return pkgerrors.New("topic already registered")
+		return fmt.Errorf("topic already registered")
 	}
 
 	m.topicsMux.Lock()
@@ -101,7 +100,7 @@ func (m *memEventBus) Subscribe(name string) (<-chan coretypes.ResultEvent, Unsu
 	m.topicsMux.RUnlock()
 
 	if !ok {
-		return nil, nil, pkgerrors.Errorf("topic not found: %s", name)
+		return nil, nil, fmt.Errorf("topic not found: %s", name)
 	}
 
 	ch := make(chan coretypes.ResultEvent)

eth/rpc/pubsub/pubsub.go

@@ -27,12 +27,6 @@ import (
 // ignored in JSON-RPC API.
 const ErrExceedBlockGasLimit = "out of gas in location: block gas meter; gasWanted:"
 
-// ErrStateDBCommit defines the error message when commit after executing EVM
-// transaction, for example transfer native token to a distribution module
-// account using an evm transaction. Note, the transfer amount cannot be set to
-// 0, otherwise this problem will not be triggered.
-const ErrStateDBCommit = "failed to commit stateDB"
-
 // RawTxToEthTx returns a evm MsgEthereum transaction from raw tx bytes.
 func RawTxToEthTx(clientCtx client.Context, txBz cmttypes.Tx) ([]*evm.MsgEthereumTx, error) {
 	tx, err := clientCtx.TxConfig.TxDecoder()(txBz)
@@ -237,7 +231,7 @@ func TxIsValidEnough(res *abci.ResponseDeliverTx) (condition bool, reason string
 		return true, "tx succeeded"
 	} else if strings.Contains(res.Log, ErrExceedBlockGasLimit) {
 		return true, "tx exceeded block gas limit"
-	} else if strings.Contains(res.Log, ErrStateDBCommit) {
+	} else if strings.Contains(res.Log, evm.ErrStateDBCommit) {
 		return true, "tx state db commit error"
 	}
 	return false, "unexpected failure"

eth/rpc/rpc.go

@@ -37,7 +37,7 @@ func GetGRPCConnection(
 	conn, err := grpc.DialContext(ctx, grpcUrl, options...)
 	if err != nil {
 		return nil, fmt.Errorf(
-			"%w: Cannot connect to gRPC endpoint %s", err, grpcUrl)
+			"cannot connect to gRPC endpoint %s: %w", grpcUrl, err)
 	}
 
 	return conn, nil