From 33e6489a982c06f2efeb5ba48b8b7ac7acfde61e Mon Sep 17 00:00:00 2001 From: Federico Kunze Date: Wed, 27 May 2020 23:17:46 -0400 Subject: [PATCH 1/4] evm: GenesisAccount validation --- x/evm/types/genesis.go | 44 +++++++++++++++++++---- x/evm/types/genesis_test.go | 69 +++++++++++++++++++++++++++++++++++++ 2 files changed, 106 insertions(+), 7 deletions(-) diff --git a/x/evm/types/genesis.go b/x/evm/types/genesis.go index 066aab6b7..6ef7fda37 100644 --- a/x/evm/types/genesis.go +++ b/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 errors.New("address cannot be the zero address") + } + 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, storage := range ga.Storage { + if seenStorage[storage.Key.String()] { + return fmt.Errorf("duplicate storage key %d", i) + } + if bytes.Equal(storage.Key.Bytes(), ethcmn.Hash{}.Bytes()) { + return fmt.Errorf("storage %d key hash cannot be empty", i) + } + // NOTE: storage value can be empty + seenStorage[storage.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 { - for _, acc := range gs.Accounts { - if bytes.Equal(acc.Address.Bytes(), zeroAddrBytes) { - return errors.New("invalid GenesisAccount: address cannot be empty") + seenAccounts := make(map[string]bool) + for i, acc := range gs.Accounts { + if seenAccounts[acc.Address.String()] { + return fmt.Errorf("duplicated genesis account %s", acc.Address) } - if acc.Balance == nil { - return errors.New("invalid GenesisAccount: balance cannot be empty") + if err := acc.Validate(); err != nil { + return fmt.Errorf("invalid genesis account %s: %w", acc.Address, err) } + seenAccounts[acc.Address.String()] = true } return nil } diff --git a/x/evm/types/genesis_test.go b/x/evm/types/genesis_test.go index e1dfa0a3c..b92d40a53 100644 --- a/x/evm/types/genesis_test.go +++ b/x/evm/types/genesis_test.go @@ -9,6 +9,75 @@ import ( ethcmn "github.com/ethereum/go-ethereum/common" ) +func TestValidateGenesisAccount(t *testing.T) { + testCases := []struct { + name string + genesisAccount GenesisAccount + expPass bool + }{ + { + "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, + }, + { + "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.Hash{1, 2}}, + {Key: ethcmn.Hash{1, 2}}, + }, + }, + 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 { From c7eb12f10442fa167610a11e12c5633eb62ffa16 Mon Sep 17 00:00:00 2001 From: Federico Kunze Date: Wed, 27 May 2020 23:20:18 -0400 Subject: [PATCH 2/4] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ffad07ea1..a3e9e5f88 100644 --- a/CHANGELOG.md +++ b/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 From 8dc9cc73a2e047d733ce46d233810414803c2eea Mon Sep 17 00:00:00 2001 From: Federico Kunze Date: Thu, 28 May 2020 09:07:21 -0400 Subject: [PATCH 3/4] fix tests --- x/evm/genesis.go | 2 ++ x/evm/types/genesis.go | 22 +++++++------- x/evm/types/genesis_test.go | 59 +++++++++++++++++++++++++++++++++---- 3 files changed, 66 insertions(+), 17 deletions(-) diff --git a/x/evm/genesis.go b/x/evm/genesis.go index 9adb18253..47f4867bc 100644 --- a/x/evm/genesis.go +++ b/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{} } diff --git a/x/evm/types/genesis.go b/x/evm/types/genesis.go index 6ef7fda37..305350aba 100644 --- a/x/evm/types/genesis.go +++ b/x/evm/types/genesis.go @@ -37,7 +37,7 @@ 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 errors.New("address cannot be the zero address") + return fmt.Errorf("address cannot be the zero address %s", ga.Address.String()) } if ga.Balance == nil { return errors.New("balance cannot be nil") @@ -50,15 +50,15 @@ func (ga GenesisAccount) Validate() error { } seenStorage := make(map[string]bool) - for i, storage := range ga.Storage { - if seenStorage[storage.Key.String()] { - return fmt.Errorf("duplicate storage key %d", i) + for i, state := range ga.Storage { + if seenStorage[state.Key.String()] { + return fmt.Errorf("duplicate state key %d", i) } - if bytes.Equal(storage.Key.Bytes(), ethcmn.Hash{}.Bytes()) { - return fmt.Errorf("storage %d key hash cannot be empty", i) + if bytes.Equal(state.Key.Bytes(), ethcmn.Hash{}.Bytes()) { + return fmt.Errorf("state %d key hash cannot be empty", i) } - // NOTE: storage value can be empty - seenStorage[storage.Key.String()] = true + // NOTE: state value can be empty + seenStorage[state.Key.String()] = true } return nil } @@ -82,12 +82,12 @@ func DefaultGenesisState() GenesisState { // failure. func (gs GenesisState) Validate() error { seenAccounts := make(map[string]bool) - for i, acc := range gs.Accounts { + for _, acc := range gs.Accounts { if seenAccounts[acc.Address.String()] { - return fmt.Errorf("duplicated genesis account %s", acc.Address) + return fmt.Errorf("duplicated genesis account %s", acc.Address.String()) } if err := acc.Validate(); err != nil { - return fmt.Errorf("invalid genesis account %s: %w", acc.Address, err) + return fmt.Errorf("invalid genesis account %s: %w", acc.Address.String(), err) } seenAccounts[acc.Address.String()] = true } diff --git a/x/evm/types/genesis_test.go b/x/evm/types/genesis_test.go index b92d40a53..11b853494 100644 --- a/x/evm/types/genesis_test.go +++ b/x/evm/types/genesis_test.go @@ -15,6 +15,18 @@ func TestValidateGenesisAccount(t *testing.T) { 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{ @@ -31,6 +43,14 @@ func TestValidateGenesisAccount(t *testing.T) { }, false, }, + { + "nil account balance", + GenesisAccount{ + Address: ethcmn.BytesToAddress([]byte{1, 2, 3, 4, 5}), + Balance: big.NewInt(-1), + }, + false, + }, { "empty code bytes", GenesisAccount{ @@ -59,8 +79,8 @@ func TestValidateGenesisAccount(t *testing.T) { Balance: big.NewInt(1), Code: []byte{1, 2, 3}, Storage: []GenesisStorage{ - {Key: ethcmn.Hash{1, 2}}, - {Key: ethcmn.Hash{1, 2}}, + {Key: ethcmn.BytesToHash([]byte{1, 2, 3})}, + {Key: ethcmn.BytesToHash([]byte{1, 2, 3})}, }, }, false, @@ -91,24 +111,51 @@ func TestValidateGenesis(t *testing.T) { expPass: true, }, { - name: "empty account address bytes", + name: "valid genesis", genState: GenesisState{ Accounts: []GenesisAccount{ { - Address: ethcmn.Address{}, + 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{}, }, }, }, 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})), + }, }, }, }, From 978b6abbdefca6c1f20a8fbc9ebd5b5e05028243 Mon Sep 17 00:00:00 2001 From: Federico Kunze Date: Thu, 28 May 2020 09:08:55 -0400 Subject: [PATCH 4/4] typo --- x/evm/types/genesis.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/evm/types/genesis.go b/x/evm/types/genesis.go index 305350aba..a1e2648e8 100644 --- a/x/evm/types/genesis.go +++ b/x/evm/types/genesis.go @@ -34,7 +34,7 @@ type ( } ) -// Validate performs a basic validation of a GenesisAccount fields/ +// 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())