imp(evm): restrict VFBC deployment on top of Module Account or EOA

CHANGELOG.md

@@ -42,6 +42,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 - (evm) [#11](https://github.com/dymensionxyz/ethermint/pull/11) Disabled contract creation for Dymension chains only (adjust the absolute logic in [#3](https://github.com/dymensionxyz/ethermint/pull/3))
 - (evm) [#11](https://github.com/dymensionxyz/ethermint/pull/11) Virtual Frontier Contract
 - (evm) [#15](https://github.com/dymensionxyz/ethermint/pull/15) VFBC deployment no longer revert changes on error while contract deployment
+- (evm) [#16](https://github.com/dymensionxyz/ethermint/pull/16) Restrict VFBC deployment on top of Module Account or EOA
 
 ### Bug Fixes
 - (rpc) [#6](https://github.com/dymensionxyz/ethermint/pull/6) Zero gas config for tracing methods

x/evm/keeper/virtual_frontier_contract.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
+	authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
 	banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
 	"github.com/ethereum/go-ethereum/common"
 	ethtypes "github.com/ethereum/go-ethereum/core/types"
@@ -322,17 +323,34 @@ func (k Keeper) DeployNewVirtualFrontierContract(ctx sdk.Context, vfContract *ty
 
 	nonce := deployerModuleAccount.GetSequence()
 	contractAddress = crypto.CreateAddress(types.VirtualFrontierContractDeployerAddress, nonce)
-	contractAccount := k.GetAccount(ctx, contractAddress)
-	if contractAccount != nil && contractAccount.IsContract() {
-		err = sdkerrors.ErrInvalidRequest.Wrapf("contract address already exists at %s", contractAddress)
-		return
-	}
 
 	if k.IsVirtualFrontierContract(ctx, contractAddress) {
-		err = sdkerrors.ErrInvalidRequest.Wrapf("virtual frontier contract %s already exists", contractAddress)
+		err = sdkerrors.ErrConflict.Wrapf("virtual frontier contract %s already exists", contractAddress)
 		return
 	}
 
+	existingAccount := k.GetAccountWithoutBalance(ctx, contractAddress)
+	if existingAccount != nil {
+		if existingAccount.IsContract() {
+			err = sdkerrors.ErrConflict.Wrapf("contract address already exists at %s", contractAddress)
+			return
+		}
+
+		if existingAccount.Nonce > 0 {
+			// sign of EOA, user used their private key to sent at least one transaction
+			err = sdkerrors.ErrConflict.Wrapf("can not deploy on top of external owned account %s", contractAddress)
+			return
+		}
+	}
+
+	existingCosmosAccount := k.accountKeeper.GetAccount(ctx, contractAddress.Bytes())
+	if existingCosmosAccount != nil {
+		if _, isModuleAccount := existingCosmosAccount.(authtypes.ModuleAccountI); isModuleAccount {
+			err = sdkerrors.ErrConflict.Wrapf("can not deploy on top of module account %s", contractAddress)
+			return
+		}
+	}
+
 	// deploy pseudo bytecode for virtual frontier contract.
 	//
 	// The VF contract is not accessible from the EVM,
@@ -403,7 +421,7 @@ func (k Keeper) DeployNewVirtualFrontierContract(ctx sdk.Context, vfContract *ty
 			return
 		}
 
-		contractAccount = k.GetAccountWithoutBalance(ctx, contractAddress)
+		contractAccount := k.GetAccountWithoutBalance(ctx, contractAddress)
 		if contractAccount == nil {
 			err = errorsmod.Wrap(types.ErrVMExecution, "contract account not found")
 			return

x/evm/keeper/virtual_frontier_contract_test.go

@@ -2,7 +2,9 @@ package keeper_test
 
 import (
 	"github.com/cosmos/cosmos-sdk/codec"
+	sdk "github.com/cosmos/cosmos-sdk/types"
 	authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
+	vestingtypes "github.com/cosmos/cosmos-sdk/x/auth/vesting/types"
 	banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/crypto"
@@ -431,6 +433,185 @@ func (suite *KeeperTestSuite) TestDeployNewVirtualFrontierBankContract() {
 			suite.Contains(err.Error(), "decimals does not fit uint8")
 		}
 	})
+
+	suite.Run("accounts which is neither contract account nor module account will not prevent deployment", func() {
+		defer func() {
+			suite.Commit()
+		}()
+
+		meta3 := testutil.NewBankDenomMetadata("ibc/denom3", 6)
+		suite.app.BankKeeper.SetDenomMetaData(suite.ctx, meta3)
+		vfbcMeta3, _ := types.CollectMetadataForVirtualFrontierBankContract(meta3)
+
+		deployerModuleAccount := suite.app.AccountKeeper.GetModuleAccount(suite.ctx, types.ModuleVirtualFrontierContractDeployerName)
+		suite.Require().NotNil(deployerModuleAccount)
+
+		expectedContractAddrMeta3 := crypto.CreateAddress(types.VirtualFrontierContractDeployerAddress, deployerModuleAccount.GetSequence()+0)
+
+		newAccount := suite.app.AccountKeeper.NewAccountWithAddress(suite.ctx, expectedContractAddrMeta3.Bytes())
+		newBaseAccount := authtypes.NewBaseAccount(
+			newAccount.GetAddress(),
+			newAccount.GetPubKey(),
+			newAccount.GetAccountNumber(),
+			newAccount.GetSequence(),
+		)
+		suite.app.AccountKeeper.SetAccount(
+			suite.ctx,
+			vestingtypes.NewContinuousVestingAccount(newBaseAccount, sdk.NewCoins(sdk.NewCoin(suite.denom, sdk.NewInt(1))), 0, 0),
+		)
+		recheckAuthAcc := suite.app.AccountKeeper.GetAccount(suite.ctx, expectedContractAddrMeta3.Bytes())
+		suite.Require().NotNil(recheckAuthAcc)
+		recheckEvmAcc := suite.app.EvmKeeper.GetAccount(suite.ctx, expectedContractAddrMeta3)
+		suite.Require().NotNil(recheckEvmAcc)
+		suite.Require().False(recheckEvmAcc.IsContract(), "can not yet be a contract account")
+
+		suite.Require().False(suite.app.EvmKeeper.IsVirtualFrontierContract(suite.ctx, expectedContractAddrMeta3))
+
+		addrMeta3, err := suite.app.EvmKeeper.DeployNewVirtualFrontierBankContract(
+			suite.ctx,
+			&types.VirtualFrontierContract{
+				Active: true,
+			}, &types.VFBankContractMetadata{
+				MinDenom: meta3.Base,
+			},
+			&vfbcMeta3,
+		)
+		suite.Require().NoError(err)
+
+		suite.Require().Equal(expectedContractAddrMeta3, addrMeta3)
+
+		suite.Require().True(
+			suite.app.EvmKeeper.IsVirtualFrontierContract(suite.ctx, addrMeta3),
+			"account must be converted into VFBC",
+		)
+
+		recheckAuthAcc = suite.app.AccountKeeper.GetAccount(suite.ctx, addrMeta3.Bytes())
+		suite.Require().NotNil(recheckAuthAcc)
+		_, isEthAccount := recheckAuthAcc.(*ethermint.EthAccount)
+		suite.Require().False(isEthAccount, "account type must be kept as is")
+
+		recheckEvmAcc = suite.app.EvmKeeper.GetAccount(suite.ctx, addrMeta3)
+		suite.Require().NotNil(recheckEvmAcc)
+		suite.True(recheckEvmAcc.IsContract(), "must be contract account")
+		suite.Equal(uint64(1), recheckEvmAcc.Nonce)
+	})
+
+	suite.Run("accounts is module account will prevent deployment", func() {
+		defer func() {
+			suite.Commit()
+		}()
+
+		meta4 := testutil.NewBankDenomMetadata("ibc/denom3", 6)
+		suite.app.BankKeeper.SetDenomMetaData(suite.ctx, meta4)
+		vfbcMeta4, _ := types.CollectMetadataForVirtualFrontierBankContract(meta4)
+
+		deployerModuleAccount := suite.app.AccountKeeper.GetModuleAccount(suite.ctx, types.ModuleVirtualFrontierContractDeployerName)
+		suite.Require().NotNil(deployerModuleAccount)
+
+		expectedContractAddrMeta4 := crypto.CreateAddress(types.VirtualFrontierContractDeployerAddress, deployerModuleAccount.GetSequence()+0)
+
+		newAccount := suite.app.AccountKeeper.NewAccountWithAddress(suite.ctx, expectedContractAddrMeta4.Bytes())
+		newBaseAccount := authtypes.NewBaseAccount(
+			newAccount.GetAddress(),
+			newAccount.GetPubKey(),
+			newAccount.GetAccountNumber(),
+			newAccount.GetSequence(),
+		)
+		newModuleAccount := authtypes.NewModuleAccount(newBaseAccount, "test", authtypes.Burner, authtypes.Minter)
+		suite.app.AccountKeeper.SetAccount(
+			suite.ctx,
+			newModuleAccount,
+		)
+		recheckAuthAcc := suite.app.AccountKeeper.GetAccount(suite.ctx, expectedContractAddrMeta4.Bytes())
+		suite.Require().NotNil(recheckAuthAcc)
+		recheckEvmAcc := suite.app.EvmKeeper.GetAccount(suite.ctx, expectedContractAddrMeta4)
+		suite.Require().NotNil(recheckEvmAcc)
+		suite.Require().False(recheckEvmAcc.IsContract(), "can not yet be a contract account")
+
+		suite.Require().False(suite.app.EvmKeeper.IsVirtualFrontierContract(suite.ctx, expectedContractAddrMeta4))
+
+		suite.Commit()
+
+		_, err := suite.app.EvmKeeper.DeployNewVirtualFrontierBankContract(
+			suite.ctx,
+			&types.VirtualFrontierContract{
+				Active: true,
+			}, &types.VFBankContractMetadata{
+				MinDenom: meta4.Base,
+			},
+			&vfbcMeta4,
+		)
+		suite.Require().Error(err)
+
+		suite.Require().False(suite.app.EvmKeeper.IsVirtualFrontierContract(suite.ctx, expectedContractAddrMeta4))
+
+		recheckAuthAcc = suite.app.AccountKeeper.GetAccount(suite.ctx, expectedContractAddrMeta4.Bytes())
+		suite.Require().NotNil(recheckAuthAcc)
+		_, isEthAccount := recheckAuthAcc.(*ethermint.EthAccount)
+		suite.Require().False(isEthAccount, "account type must be kept as is")
+
+		recheckEvmAcc = suite.app.EvmKeeper.GetAccount(suite.ctx, expectedContractAddrMeta4)
+		suite.Require().NotNil(recheckEvmAcc)
+		suite.False(recheckEvmAcc.IsContract(), "must not be contract account")
+	})
+
+	suite.Run("base account is ok but if having non-zero sequence number, will prevent deployment", func() {
+		defer func() {
+			suite.Commit()
+		}()
+
+		meta5 := testutil.NewBankDenomMetadata("ibc/denom5", 6)
+		suite.app.BankKeeper.SetDenomMetaData(suite.ctx, meta5)
+		vfbcMeta5, _ := types.CollectMetadataForVirtualFrontierBankContract(meta5)
+
+		deployerModuleAccount := suite.app.AccountKeeper.GetModuleAccount(suite.ctx, types.ModuleVirtualFrontierContractDeployerName)
+		suite.Require().NotNil(deployerModuleAccount)
+
+		expectedContractAddrMeta5 := crypto.CreateAddress(types.VirtualFrontierContractDeployerAddress, deployerModuleAccount.GetSequence()+0)
+
+		newAccount := suite.app.AccountKeeper.NewAccountWithAddress(suite.ctx, expectedContractAddrMeta5.Bytes())
+		newBaseAccount := authtypes.NewBaseAccount(
+			newAccount.GetAddress(),
+			newAccount.GetPubKey(),
+			newAccount.GetAccountNumber(),
+			1, // sequence number is non-zero
+		)
+		suite.app.AccountKeeper.SetAccount(
+			suite.ctx,
+			vestingtypes.NewContinuousVestingAccount(newBaseAccount, sdk.NewCoins(sdk.NewCoin(suite.denom, sdk.NewInt(1))), 0, 0),
+		)
+		recheckAuthAcc := suite.app.AccountKeeper.GetAccount(suite.ctx, expectedContractAddrMeta5.Bytes())
+		suite.Require().NotNil(recheckAuthAcc)
+		recheckEvmAcc := suite.app.EvmKeeper.GetAccount(suite.ctx, expectedContractAddrMeta5)
+		suite.Require().NotNil(recheckEvmAcc)
+		suite.Require().False(recheckEvmAcc.IsContract(), "can not yet be a contract account")
+
+		suite.Require().False(suite.app.EvmKeeper.IsVirtualFrontierContract(suite.ctx, expectedContractAddrMeta5))
+
+		suite.Commit()
+
+		_, err := suite.app.EvmKeeper.DeployNewVirtualFrontierBankContract(
+			suite.ctx,
+			&types.VirtualFrontierContract{
+				Active: true,
+			}, &types.VFBankContractMetadata{
+				MinDenom: meta5.Base,
+			},
+			&vfbcMeta5,
+		)
+		suite.Require().Error(err)
+
+		suite.Require().False(suite.app.EvmKeeper.IsVirtualFrontierContract(suite.ctx, expectedContractAddrMeta5))
+
+		recheckAuthAcc = suite.app.AccountKeeper.GetAccount(suite.ctx, expectedContractAddrMeta5.Bytes())
+		suite.Require().NotNil(recheckAuthAcc)
+		_, isEthAccount := recheckAuthAcc.(*ethermint.EthAccount)
+		suite.Require().False(isEthAccount, "account type must be kept as is")
+
+		recheckEvmAcc = suite.app.EvmKeeper.GetAccount(suite.ctx, expectedContractAddrMeta5)
+		suite.Require().NotNil(recheckEvmAcc)
+		suite.False(recheckEvmAcc.IsContract(), "must not be contract account")
+	})
 }
 
 func (suite *KeeperTestSuite) TestDeployedVirtualFrontierBankContracts() {