feat!: Enforce stricter validations for genesis accounts and enabled …

…precompiles (#74)

* feat!: Genesis Accounts can not contain empty code

Genesis accounts currently serve no purpose for non-contract accounts.
Currently, they can exist but code and storage will be empty and
therefore has no effect besids ensuring the matching account is subject
to similar validations as a contract.

This change restrics valid genesis accounts to those with Code only.
Storage can still be empty for contracts, as there is no requirement for
values to be set in state for all contracts.

* refactor: Capitalize test case names

Using capital letters for test case names improved readability when
reading and parsing.

* fix!: Add back code hash check for empty code

On some chains running ethermint, a self destruct bug that caused the
code for other contract accounts to be deleted would result in exported
genesis accounts with empty code.  A patch was then added to genesis
that allowed the hash check to be skipped in order for these genesis
files to be re-imported successfully.

This does not affect us contracts on Kava, so this check is being
removed.

* feat!: Add strict contract account checks

Since Genesis Accounts may only be contracts, we add the assertion that
they must have a positive sequence (eip-155) and that they do not have a
public key set.  A contract can not be associated with a public key and
must have a default nonce set.

In addition, since enabled precompiles are contract accounts, we enforce
that these have matching genesis accounts to share these validations and
in addition, enforce a fixed code.

x/evm/genesis.go

@@ -77,6 +77,11 @@ func InitGenesis(
 		panic("the EVM module account has not been set")
 	}
 
+	isEnabledPrecompile := make(map[string]struct{})
+	for _, ep := range data.Params.EnabledPrecompiles {
+		isEnabledPrecompile[ep] = struct{}{}
+	}
+
 	for _, account := range data.Accounts {
 		address := common.HexToAddress(account.Address)
 		accAddress := sdk.AccAddress(address.Bytes())
@@ -95,11 +100,23 @@ func InitGenesis(
 				),
 			)
 		}
+
+		if ethAcct.GetSequence() == 0 {
+			panic(fmt.Errorf("account %s must have a positive nonce", account.Address))
+		}
+
+		if ethAcct.GetPubKey() != nil {
+			panic(fmt.Errorf("account %s must not have a public key set", account.Address))
+		}
+
 		code := common.Hex2Bytes(account.Code)
 		codeHash := crypto.Keccak256Hash(code)
 
-		// we ignore the empty Code hash checking, see ethermint PR#1234
-		if len(account.Code) != 0 && !bytes.Equal(ethAcct.GetCodeHash().Bytes(), codeHash.Bytes()) {
+		if _, ok := isEnabledPrecompile[account.Address]; ok && !bytes.Equal(code, []byte{0x01}) {
+			panic(fmt.Errorf("enabled precompile %s must have code set to 0x01, got 0x%s", account.Address, account.Code))
+		}
+
+		if !bytes.Equal(ethAcct.GetCodeHash().Bytes(), codeHash.Bytes()) {
 			s := "the evm state code doesn't match with the codehash\n"
 			panic(fmt.Sprintf("%s account: %s , evm state codehash: %v, ethAccount codehash: %v, evm state code: %s\n",
 				s, account.Address, codeHash, ethAcct.GetCodeHash(), account.Code))

x/evm/genesis_test.go

@@ -8,6 +8,7 @@ import (
 
 	"cosmossdk.io/simapp"
 	tmproto "github.com/cometbft/cometbft/proto/tendermint/types"
+	codectypes "github.com/cosmos/cosmos-sdk/codec/types"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
 	"github.com/ethereum/go-ethereum/common"
@@ -194,6 +195,7 @@ func TestInitGenesis(t *testing.T) {
 				accAddr := sdk.AccAddress(common.HexToAddress(address).Bytes())
 
 				acc := authtypes.NewBaseAccountWithAddress(accAddr)
+				acc.Sequence = uint64(1)
 				tApp.AccountKeeper.SetAccount(ctx, acc)
 
 				return testFixture{
@@ -227,6 +229,7 @@ func TestInitGenesis(t *testing.T) {
 					BaseAccount: authtypes.NewBaseAccountWithAddress(accAddr),
 					CodeHash:    incorrectCodeHash.String(),
 				}
+				acc.Sequence = uint64(1)
 				tApp.AccountKeeper.SetAccount(ctx, &acc)
 
 				s := "the evm state code doesn't match with the codehash\n"
@@ -242,7 +245,7 @@ func TestInitGenesis(t *testing.T) {
 			},
 		},
 		{
-			name: "Does not panic when there is a code hash mismatch and matching genesis account contains no code",
+			name: "Panics when there is a code hash mismatch and matching genesis account contains no code",
 			genFixture: func(t *testing.T, ctx sdk.Context, tApp *app.EthermintApp) testFixture {
 				address := generateRandomAddress(t)
 
@@ -259,14 +262,18 @@ func TestInitGenesis(t *testing.T) {
 					BaseAccount: authtypes.NewBaseAccountWithAddress(accAddr),
 					CodeHash:    someCodeHash.String(),
 				}
+				acc.Sequence = uint64(1)
 				tApp.AccountKeeper.SetAccount(ctx, &acc)
 
+				s := "the evm state code doesn't match with the codehash\n"
+				expectedPanic := fmt.Sprintf("%s account: %s , evm state codehash: %v, ethAccount codehash: %v, evm state code: %s\n", s, address, common.BytesToHash(types.EmptyCodeHash), acc.GetCodeHash(), "")
+
 				return testFixture{
 					ctx:         ctx,
 					state:       state,
 					precompiles: nil,
 					expectFunc:  nil,
-					expectPanic: nil,
+					expectPanic: expectedPanic,
 				}
 			},
 		},
@@ -290,6 +297,7 @@ func TestInitGenesis(t *testing.T) {
 					BaseAccount: authtypes.NewBaseAccountWithAddress(accAddr),
 					CodeHash:    "", // we do not allow empty code hash when code is set
 				}
+				acc.Sequence = uint64(1)
 				tApp.AccountKeeper.SetAccount(ctx, &acc)
 
 				s := "the evm state code doesn't match with the codehash\n"
@@ -324,6 +332,7 @@ func TestInitGenesis(t *testing.T) {
 					BaseAccount: authtypes.NewBaseAccountWithAddress(accAddr),
 					CodeHash:    codeHash.String(),
 				}
+				acc.Sequence = uint64(1)
 				tApp.AccountKeeper.SetAccount(ctx, &acc)
 
 				expectFunc := func() {
@@ -378,6 +387,7 @@ func TestInitGenesis(t *testing.T) {
 					BaseAccount: authtypes.NewBaseAccountWithAddress(accAddr),
 					CodeHash:    codeHash.String(),
 				}
+				acc.Sequence = uint64(1)
 				tApp.AccountKeeper.SetAccount(ctx, &acc)
 
 				expectFunc := func() {
@@ -479,6 +489,36 @@ func TestInitGenesis(t *testing.T) {
 					{Address: addr2},
 				}
 
+				code := []byte{0x01}
+				codeHash := crypto.Keccak256Hash(code)
+				codeHex := common.Bytes2Hex(code)
+				state.Accounts = append(state.Accounts,
+					types.GenesisAccount{
+						Address: addr1.String(),
+						Code:    codeHex,
+					},
+					types.GenesisAccount{
+						Address: addr2.String(),
+						Code:    codeHex,
+					},
+				)
+
+				accAddr1 := sdk.AccAddress(addr1.Bytes())
+				acc1 := ethermint.EthAccount{
+					BaseAccount: authtypes.NewBaseAccountWithAddress(accAddr1),
+					CodeHash:    codeHash.String(),
+				}
+				acc1.Sequence = uint64(1)
+				tApp.AccountKeeper.SetAccount(ctx, &acc1)
+
+				accAddr2 := sdk.AccAddress(addr2.Bytes())
+				acc2 := ethermint.EthAccount{
+					BaseAccount: authtypes.NewBaseAccountWithAddress(accAddr2),
+					CodeHash:    codeHash.String(),
+				}
+				acc2.Sequence = uint64(1)
+				tApp.AccountKeeper.SetAccount(ctx, &acc2)
+
 				expectFunc := func() {
 					assert.Equal(t,
 						state.Params.EnabledPrecompiles,
@@ -496,6 +536,142 @@ func TestInitGenesis(t *testing.T) {
 				}
 			},
 		},
+		{
+			name: "Panics when genesis account has a nonce equal to zero",
+			genFixture: func(t *testing.T, ctx sdk.Context, tApp *app.EthermintApp) testFixture {
+				address := generateRandomAddress(t)
+
+				code := []byte{0x01, 0x02, 0x03}
+				codeHash := crypto.Keccak256Hash(code)
+				codeHex := common.Bytes2Hex(code)
+
+				state := types.DefaultGenesisState()
+				state.Accounts = append(state.Accounts, types.GenesisAccount{
+					Address: address,
+					Code:    codeHex,
+				})
+
+				accAddr := sdk.AccAddress(common.HexToAddress(address).Bytes())
+				acc := ethermint.EthAccount{
+					BaseAccount: authtypes.NewBaseAccountWithAddress(accAddr),
+					CodeHash:    codeHash.String(),
+				}
+				acc.Sequence = uint64(0) // Not allowed for contracts
+				tApp.AccountKeeper.SetAccount(ctx, &acc)
+
+				return testFixture{
+					ctx:         ctx,
+					state:       state,
+					precompiles: nil,
+					expectFunc:  nil,
+					expectPanic: fmt.Errorf("account %s must have a positive nonce", address),
+				}
+			},
+		},
+		{
+			name: "Allows a genesis account with a nonce greater than 1",
+			genFixture: func(t *testing.T, ctx sdk.Context, tApp *app.EthermintApp) testFixture {
+				address := generateRandomAddress(t)
+
+				code := []byte{0x01, 0x02, 0x03}
+				codeHash := crypto.Keccak256Hash(code)
+				codeHex := common.Bytes2Hex(code)
+
+				state := types.DefaultGenesisState()
+				state.Accounts = append(state.Accounts, types.GenesisAccount{
+					Address: address,
+					Code:    codeHex,
+				})
+
+				accAddr := sdk.AccAddress(common.HexToAddress(address).Bytes())
+				acc := ethermint.EthAccount{
+					BaseAccount: authtypes.NewBaseAccountWithAddress(accAddr),
+					CodeHash:    codeHash.String(),
+				}
+				acc.Sequence = uint64(1000)
+				tApp.AccountKeeper.SetAccount(ctx, &acc)
+
+				return testFixture{
+					ctx:         ctx,
+					state:       state,
+					precompiles: nil,
+					expectFunc:  nil,
+					expectPanic: nil,
+				}
+			},
+		},
+		{
+			name: "Panics when genesis account has a public key set",
+			genFixture: func(t *testing.T, ctx sdk.Context, tApp *app.EthermintApp) testFixture {
+				privkey, err := ethsecp256k1.GenerateKey()
+				require.NoError(t, err)
+				address := common.BytesToAddress(privkey.PubKey().Address()).String()
+
+				code := []byte{0x01, 0x02, 0x03}
+				codeHash := crypto.Keccak256Hash(code)
+				codeHex := common.Bytes2Hex(code)
+
+				state := types.DefaultGenesisState()
+				state.Accounts = append(state.Accounts, types.GenesisAccount{
+					Address: address,
+					Code:    codeHex,
+				})
+
+				accAddr := sdk.AccAddress(common.HexToAddress(address).Bytes())
+				acc := ethermint.EthAccount{
+					BaseAccount: authtypes.NewBaseAccountWithAddress(accAddr),
+					CodeHash:    codeHash.String(),
+				}
+				acc.Sequence = uint64(1)
+				pubkey, err := codectypes.NewAnyWithValue(privkey.PubKey())
+				require.NoError(t, err)
+				acc.PubKey = pubkey
+				tApp.AccountKeeper.SetAccount(ctx, &acc)
+
+				return testFixture{
+					ctx:         ctx,
+					state:       state,
+					precompiles: nil,
+					expectFunc:  nil,
+					expectPanic: fmt.Errorf("account %s must not have a public key set", address),
+				}
+			},
+		},
+		{
+			name: "Panics when enabled precompile does not have a genesis account with code 0x01",
+			genFixture: func(t *testing.T, ctx sdk.Context, tApp *app.EthermintApp) testFixture {
+				address := generateRandomAddress(t)
+
+				code := []byte{0x02}
+				codeHash := crypto.Keccak256Hash(code)
+				codeHex := common.Bytes2Hex(code)
+
+				state := types.DefaultGenesisState()
+				state.Params.EnabledPrecompiles = []string{address}
+				state.Accounts = append(state.Accounts, types.GenesisAccount{
+					Address: address,
+					Code:    codeHex,
+				})
+
+				accAddr := sdk.AccAddress(common.HexToAddress(address).Bytes())
+				acc := ethermint.EthAccount{
+					BaseAccount: authtypes.NewBaseAccountWithAddress(accAddr),
+					CodeHash:    codeHash.String(),
+				}
+				acc.Sequence = uint64(1)
+				tApp.AccountKeeper.SetAccount(ctx, &acc)
+
+				registeredPrecompiles := []precompile_modules.Module{{Address: common.HexToAddress(address)}}
+
+				return testFixture{
+					ctx:         ctx,
+					state:       state,
+					precompiles: registeredPrecompiles,
+					expectFunc:  nil,
+					expectPanic: fmt.Errorf("enabled precompile %s must have code set to 0x01, got 0x%s", address, codeHex),
+				}
+			},
+		},
 	}
 
 	for _, tc := range testCases {

x/evm/types/genesis.go

@@ -21,16 +21,17 @@ import (
 	ethermint "github.com/evmos/ethermint/types"
 )
 
-// NewGenesisState creates a new genesis state.
+// NewGenesisState creates a new genesis state with EVM configuration parameters
+// and initial contract accounts.
 func NewGenesisState(params Params, accounts []GenesisAccount) *GenesisState {
 	return &GenesisState{
 		Accounts: accounts,
 		Params:   params,
 	}
 }
 
-// DefaultGenesisState sets default evm genesis state with empty accounts and default params and
-// chain config values.
+// DefaultGenesisState sets default evm genesis state with default parameters and
+// no initial genesis accounts.
 func DefaultGenesisState() *GenesisState {
 	return &GenesisState{
 		Accounts: []GenesisAccount{},
@@ -39,15 +40,23 @@ func DefaultGenesisState() *GenesisState {
 }
 
 // Validate performs a basic validation of a GenesisAccount fields.
+// A valid genesis account has a valid address, non-empty code, and
+// valid storage.
 func (ga GenesisAccount) Validate() error {
 	if err := ethermint.ValidateAddress(ga.Address); err != nil {
 		return err
 	}
+
+	if ga.Code == "" {
+		return fmt.Errorf("code can not be empty")
+	}
+
 	return ga.Storage.Validate()
 }
 
 // Validate performs basic genesis state validation returning an error upon any
-// failure.
+// failure. It ensures the params are valid and every genesis account is unique
+// and valid.
 func (gs GenesisState) Validate() error {
 	seenAccounts := make(map[string]struct{})
 
@@ -67,5 +76,11 @@ func (gs GenesisState) Validate() error {
 		return fmt.Errorf("invalid params: %w", err)
 	}
 
+	for _, ep := range gs.Params.EnabledPrecompiles {
+		if _, ok := seenAccounts[ep]; !ok {
+			return fmt.Errorf("enabled precompile %s must have a matching genesis account", ep)
+		}
+	}
+
 	return nil
 }