evm: GenesisAccount validation (#317)

* evm: GenesisAccount validation

* changelog

* fix tests

* typo

CHANGELOG.md

@@ -55,6 +55,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
   * (x/evm) Moved `BeginBlock` and `EndBlock` to `x/evm/abci.go`
 * (`x/evm`) [\#255](https://github.com/ChainSafe/ethermint/pull/255) Add missing `GenesisState` fields and support `ExportGenesis` functionality.
 * [\#272](https://github.com/ChainSafe/ethermint/pull/272) Add `Logger` for evm module.
+* [\#317](https://github.com/ChainSafe/ethermint/pull/317) `GenesisAccount` validation.
 
 ### Features
 

x/evm/genesis.go

@@ -14,12 +14,14 @@ import (
 func InitGenesis(ctx sdk.Context, k Keeper, data GenesisState) []abci.ValidatorUpdate {
 	for _, account := range data.Accounts {
 		csdb := k.CommitStateDB.WithContext(ctx)
+		// FIXME: this will override bank InitGenesis balance!
 		csdb.SetBalance(account.Address, account.Balance)
 		csdb.SetCode(account.Address, account.Code)
 		for _, storage := range account.Storage {
 			csdb.SetState(account.Address, storage.Key, storage.Value)
 		}
 	}
+	// TODO: Commit?
 	return []abci.ValidatorUpdate{}
 }
 

x/evm/types/genesis.go

@@ -3,13 +3,12 @@ package types
 import (
 	"bytes"
 	"errors"
+	"fmt"
 	"math/big"
 
 	ethcmn "github.com/ethereum/go-ethereum/common"
 )
 
-var zeroAddrBytes = []byte{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}
-
 type (
 	// GenesisState defines the application's genesis state. It contains all the
 	// information required and accounts to initialize the blockchain.
@@ -35,6 +34,35 @@ type (
 	}
 )
 
+// Validate performs a basic validation of a GenesisAccount fields.
+func (ga GenesisAccount) Validate() error {
+	if bytes.Equal(ga.Address.Bytes(), ethcmn.Address{}.Bytes()) {
+		return fmt.Errorf("address cannot be the zero address %s", ga.Address.String())
+	}
+	if ga.Balance == nil {
+		return errors.New("balance cannot be nil")
+	}
+	if ga.Balance.Sign() == -1 {
+		return errors.New("balance cannot be negative")
+	}
+	if ga.Code != nil && len(ga.Code) == 0 {
+		return errors.New("code bytes cannot be empty")
+	}
+
+	seenStorage := make(map[string]bool)
+	for i, state := range ga.Storage {
+		if seenStorage[state.Key.String()] {
+			return fmt.Errorf("duplicate state key %d", i)
+		}
+		if bytes.Equal(state.Key.Bytes(), ethcmn.Hash{}.Bytes()) {
+			return fmt.Errorf("state %d key hash cannot be empty", i)
+		}
+		// NOTE: state value can be empty
+		seenStorage[state.Key.String()] = true
+	}
+	return nil
+}
+
 // NewGenesisStorage creates a new GenesisStorage instance
 func NewGenesisStorage(key, value ethcmn.Hash) GenesisStorage {
 	return GenesisStorage{
@@ -53,13 +81,15 @@ func DefaultGenesisState() GenesisState {
 // Validate performs basic genesis state validation returning an error upon any
 // failure.
 func (gs GenesisState) Validate() error {
+	seenAccounts := make(map[string]bool)
 	for _, acc := range gs.Accounts {
-		if bytes.Equal(acc.Address.Bytes(), zeroAddrBytes) {
+		if seenAccounts[acc.Address.String()] {
-			return errors.New("invalid GenesisAccount: address cannot be empty")
+			return fmt.Errorf("duplicated genesis account %s", acc.Address.String())
 		}
-		if acc.Balance == nil {
+		if err := acc.Validate(); err != nil {
-			return errors.New("invalid GenesisAccount: balance cannot be empty")
+			return fmt.Errorf("invalid genesis account %s: %w", acc.Address.String(), err)
 		}
+		seenAccounts[acc.Address.String()] = true
 	}
 	return nil
 }

x/evm/types/genesis_test.go

@@ -9,6 +9,95 @@ import (
 	ethcmn "github.com/ethereum/go-ethereum/common"
 )
 
+func TestValidateGenesisAccount(t *testing.T) {
+	testCases := []struct {
+		name           string
+		genesisAccount GenesisAccount
+		expPass        bool
+	}{
+		{
+			"valid genesis account",
+			GenesisAccount{
+				Address: ethcmn.BytesToAddress([]byte{1, 2, 3, 4, 5}),
+				Balance: big.NewInt(1),
+				Code:    []byte{1, 2, 3},
+				Storage: []GenesisStorage{
+					NewGenesisStorage(ethcmn.BytesToHash([]byte{1, 2, 3}), ethcmn.BytesToHash([]byte{1, 2, 3})),
+				},
+			},
+			true,
+		},
+		{
+			"empty account address bytes",
+			GenesisAccount{
+				Address: ethcmn.Address{},
+				Balance: big.NewInt(1),
+			},
+			false,
+		},
+		{
+			"nil account balance",
+			GenesisAccount{
+				Address: ethcmn.BytesToAddress([]byte{1, 2, 3, 4, 5}),
+				Balance: nil,
+			},
+			false,
+		},
+		{
+			"nil account balance",
+			GenesisAccount{
+				Address: ethcmn.BytesToAddress([]byte{1, 2, 3, 4, 5}),
+				Balance: big.NewInt(-1),
+			},
+			false,
+		},
+		{
+			"empty code bytes",
+			GenesisAccount{
+				Address: ethcmn.BytesToAddress([]byte{1, 2, 3, 4, 5}),
+				Balance: big.NewInt(1),
+				Code:    []byte{},
+			},
+			false,
+		},
+		{
+			"empty storage key bytes",
+			GenesisAccount{
+				Address: ethcmn.BytesToAddress([]byte{1, 2, 3, 4, 5}),
+				Balance: big.NewInt(1),
+				Code:    []byte{1, 2, 3},
+				Storage: []GenesisStorage{
+					{Key: ethcmn.Hash{}},
+				},
+			},
+			false,
+		},
+		{
+			"duplicated storage key",
+			GenesisAccount{
+				Address: ethcmn.BytesToAddress([]byte{1, 2, 3, 4, 5}),
+				Balance: big.NewInt(1),
+				Code:    []byte{1, 2, 3},
+				Storage: []GenesisStorage{
+					{Key: ethcmn.BytesToHash([]byte{1, 2, 3})},
+					{Key: ethcmn.BytesToHash([]byte{1, 2, 3})},
+				},
+			},
+			false,
+		},
+	}
+
+	for _, tc := range testCases {
+		tc := tc
+		err := tc.genesisAccount.Validate()
+		if tc.expPass {
+			require.NoError(t, err, tc.name)
+		} else {
+			require.Error(t, err, tc.name)
+		}
+	}
+}
+
 func TestValidateGenesis(t *testing.T) {
 
 	testCases := []struct {
@@ -22,24 +111,51 @@ func TestValidateGenesis(t *testing.T) {
 			expPass:  true,
 		},
 		{
-			name: "empty account address bytes",
+			name: "valid genesis",
+			genState: GenesisState{
+				Accounts: []GenesisAccount{
+					{
+						Address: ethcmn.BytesToAddress([]byte{1, 2, 3, 4, 5}),
+						Balance: big.NewInt(1),
+						Code:    []byte{1, 2, 3},
+						Storage: []GenesisStorage{
+							{Key: ethcmn.BytesToHash([]byte{1, 2, 3})},
+						},
+					},
+				},
+			},
+			expPass: true,
+		},
+		{
+			name: "invalid genesis",
 			genState: GenesisState{
 				Accounts: []GenesisAccount{
 					{
 						Address: ethcmn.Address{},
-						Balance: big.NewInt(1),
 					},
 				},
 			},
 			expPass: false,
 		},
 		{
-			name: "nil account balance",
+			name: "duplicated genesis account",
 			genState: GenesisState{
 				Accounts: []GenesisAccount{
 					{
 						Address: ethcmn.BytesToAddress([]byte{1, 2, 3, 4, 5}),
-						Balance: nil,
+						Balance: big.NewInt(1),
+						Code:    []byte{1, 2, 3},
+						Storage: []GenesisStorage{
+							NewGenesisStorage(ethcmn.BytesToHash([]byte{1, 2, 3}), ethcmn.BytesToHash([]byte{1, 2, 3})),
+						},
+					},
+					{
+						Address: ethcmn.BytesToAddress([]byte{1, 2, 3, 4, 5}),
+						Balance: big.NewInt(1),
+						Code:    []byte{1, 2, 3},
+						Storage: []GenesisStorage{
+							NewGenesisStorage(ethcmn.BytesToHash([]byte{1, 2, 3}), ethcmn.BytesToHash([]byte{1, 2, 3})),
+						},
 					},
 				},
 			},