From cd02af6027d32e263de6cd2595f6d9809e883105 Mon Sep 17 00:00:00 2001 From: Nick DeLuca Date: Thu, 1 Aug 2024 15:26:49 +0000 Subject: [PATCH] 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 | 18 ++++ x/evm/genesis_test.go | 173 ++++++++++++++++++++++++++++++++++++ x/evm/types/genesis.go | 6 ++ x/evm/types/genesis_test.go | 12 +++ 4 files changed, 209 insertions(+) diff --git a/x/evm/genesis.go b/x/evm/genesis.go index bcf1af61a9..a2ad2ff215 100644 --- a/x/evm/genesis.go +++ b/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,9 +100,22 @@ 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) + 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", diff --git a/x/evm/genesis_test.go b/x/evm/genesis_test.go index b82994c98f..58a986e348 100644 --- a/x/evm/genesis_test.go +++ b/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" @@ -259,6 +262,7 @@ 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" @@ -293,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" @@ -327,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() { @@ -381,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() { @@ -482,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, @@ -499,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 { diff --git a/x/evm/types/genesis.go b/x/evm/types/genesis.go index 35f7bb3a66..3906cf0139 100644 --- a/x/evm/types/genesis.go +++ b/x/evm/types/genesis.go @@ -76,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 } diff --git a/x/evm/types/genesis_test.go b/x/evm/types/genesis_test.go index 0aa4618a99..9b1f466d74 100644 --- a/x/evm/types/genesis_test.go +++ b/x/evm/types/genesis_test.go @@ -309,6 +309,18 @@ func TestGenesisStateValidate(t *testing.T) { }, expectedErr: "invalid hex address", }, + { + name: "genesis is invalid if enabled precompile does not have a matching genesis account", + getState: func() *types.GenesisState { + state := types.DefaultGenesisState() + + account := defaultGenesisAccount() + state.Params.EnabledPrecompiles = append(state.Params.EnabledPrecompiles, account.Address) + + return state + }, + expectedErr: "must have a matching genesis account", + }, } for _, tc := range testCases {