From e8bac71f1e66212ad6157fae1a89be4d89df03fd Mon Sep 17 00:00:00 2001 From: VictorTrustyDev Date: Thu, 28 Mar 2024 22:44:57 +0700 Subject: [PATCH 1/2] restrict VFBC deployment on top of module accounts or EOA which has nonce > 0 --- x/evm/keeper/virtual_frontier_contract.go | 32 +++- .../keeper/virtual_frontier_contract_test.go | 181 ++++++++++++++++++ 2 files changed, 206 insertions(+), 7 deletions(-) diff --git a/x/evm/keeper/virtual_frontier_contract.go b/x/evm/keeper/virtual_frontier_contract.go index 41ccdfc5ae..60d1fda98e 100644 --- a/x/evm/keeper/virtual_frontier_contract.go +++ b/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 diff --git a/x/evm/keeper/virtual_frontier_contract_test.go b/x/evm/keeper/virtual_frontier_contract_test.go index f20ddf66c8..226a8e1ef6 100644 --- a/x/evm/keeper/virtual_frontier_contract_test.go +++ b/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() { From 23262203118033c366fc1a37c96c30444b121918 Mon Sep 17 00:00:00 2001 From: VictorTrustyDev Date: Thu, 28 Mar 2024 22:45:08 +0700 Subject: [PATCH 2/2] update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 34ce95e2d7..2cbdc3923d 100644 --- a/CHANGELOG.md +++ b/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