core: use storage root of MP only if isthmus

also removed usage of a 3rd party package and custom json tags in header.

core/genesis.go

@@ -130,7 +130,7 @@ func ReadGenesis(db ethdb.Database) (*Genesis, error) {
 // * computed state root according to the genesis specification.
 // * storage root of the L2ToL1MessagePasser contract.
 // * error if any, when committing the genesis state (if so, state root and storage root will be empty).
-func hashAlloc(ga *types.GenesisAlloc, isVerkle bool) (common.Hash, common.Hash, error) {
+func hashAlloc(ga *types.GenesisAlloc, isVerkle, isIsthmus bool) (common.Hash, common.Hash, error) {
 	// If a genesis-time verkle trie is requested, create a trie config
 	// with the verkle trie enabled so that the tree can be initialized
 	// as such.
@@ -164,15 +164,18 @@ func hashAlloc(ga *types.GenesisAlloc, isVerkle bool) (common.Hash, common.Hash,
 		return common.Hash{}, common.Hash{}, err
 	}
 	// get the storage root of the L2ToL1MessagePasser contract
-	storageRootMessagePasser := statedb.GetStorageRoot(params.OptimismL2ToL1MessagePasser)
+	var storageRootMessagePasser common.Hash
+	if isIsthmus {
+		storageRootMessagePasser = statedb.GetStorageRoot(params.OptimismL2ToL1MessagePasser)
+	}
 
 	return stateRoot, storageRootMessagePasser, nil
 }
 
 // flushAlloc is very similar with hash, but the main difference is all the
 // generated states will be persisted into the given database. Returns the
 // same values as hashAlloc.
-func flushAlloc(ga *types.GenesisAlloc, triedb *triedb.Database) (common.Hash, common.Hash, error) {
+func flushAlloc(ga *types.GenesisAlloc, triedb *triedb.Database, isIsthmus bool) (common.Hash, common.Hash, error) {
 	statedb, err := state.New(types.EmptyRootHash, state.NewDatabase(triedb, nil))
 	if err != nil {
 		return common.Hash{}, common.Hash{}, err
@@ -194,7 +197,10 @@ func flushAlloc(ga *types.GenesisAlloc, triedb *triedb.Database) (common.Hash, c
 		return common.Hash{}, common.Hash{}, err
 	}
 	// get the storage root of the L2ToL1MessagePasser contract
-	storageRootMessagePasser := statedb.GetStorageRoot(params.OptimismL2ToL1MessagePasser)
+	var storageRootMessagePasser common.Hash
+	if isIsthmus {
+		storageRootMessagePasser = statedb.GetStorageRoot(params.OptimismL2ToL1MessagePasser)
+	}
 	// Commit newly generated states into disk if it's not empty.
 	if stateRoot != types.EmptyRootHash {
 		if err := triedb.Commit(stateRoot, true); err != nil {
@@ -489,6 +495,11 @@ func (g *Genesis) IsVerkle() bool {
 	return g.Config.IsVerkle(new(big.Int).SetUint64(g.Number), g.Timestamp)
 }
 
+// IsIsthmus indicates whether Isthmus is active at genesis time.
+func (g *Genesis) IsIsthmus() bool {
+	return g.Config.IsIsthmus(g.Timestamp)
+}
+
 // ToBlock returns the genesis block according to genesis specification.
 func (g *Genesis) ToBlock() *types.Block {
 	var stateRoot, storageRootMessagePasser common.Hash
@@ -498,9 +509,13 @@ func (g *Genesis) ToBlock() *types.Block {
 			panic(fmt.Errorf("cannot both have genesis hash %s "+
 				"and non-empty state-allocation", *g.StateHash))
 		}
-		// TODO - need to get the storage root of the L2ToL1MessagePasser contract?
+		// stateHash is only relevant for pre-bedrock (and hence pre-isthmus) chains.
+		// we bail here since this is not a valid usage of StateHash
+		if g.IsIsthmus() {
+			panic(fmt.Errorf("stateHash usage disallowed in isthmus chain"))
+		}
 		stateRoot = *g.StateHash
-	} else if stateRoot, storageRootMessagePasser, err = hashAlloc(&g.Alloc, g.IsVerkle()); err != nil {
+	} else if stateRoot, storageRootMessagePasser, err = hashAlloc(&g.Alloc, g.IsVerkle(), g.IsIsthmus()); err != nil {
 		panic(err)
 	}
 	return g.toBlockWithRoot(stateRoot, storageRootMessagePasser)
@@ -565,8 +580,13 @@ func (g *Genesis) toBlockWithRoot(stateRoot, storageRootMessagePasser common.Has
 			requests = make(types.Requests, 0)
 		}
 		// If Isthmus is active at genesis, set the WithdrawalRoot to the storage root of the L2ToL1MessagePasser contract.
-		if conf.IsIsthmus(g.Timestamp) {
-			head.WithdrawalsHash = &storageRootMessagePasser
+		if g.IsIsthmus() {
+			if storageRootMessagePasser == (common.Hash{}) {
+				// if there was no MessagePasser contract storage, set the WithdrawalsHash to the empty hash
+				head.WithdrawalsHash = &types.EmptyWithdrawalsHash
+			} else {
+				head.WithdrawalsHash = &storageRootMessagePasser
+			}
 		}
 	}
 	return types.NewBlock(head, &types.Body{Withdrawals: withdrawals, Requests: requests}, nil, trie.NewStackTrie(nil), g.Config)
@@ -599,7 +619,7 @@ func (g *Genesis) Commit(db ethdb.Database, triedb *triedb.Database) (*types.Blo
 		}
 	} else {
 		// flush the data to disk and compute the state root
-		stateRoot, storageRootMessagePasser, err = flushAlloc(&g.Alloc, triedb)
+		stateRoot, storageRootMessagePasser, err = flushAlloc(&g.Alloc, triedb, g.IsIsthmus())
 		if err != nil {
 			return nil, err
 		}

core/genesis_test.go

@@ -223,9 +223,9 @@ func TestReadWriteGenesisAlloc(t *testing.T) {
 			{1}: {Balance: big.NewInt(1), Storage: map[common.Hash]common.Hash{{1}: {1}}},
 			{2}: {Balance: big.NewInt(2), Storage: map[common.Hash]common.Hash{{2}: {2}}},
 		}
-		stateRoot, storageRootMessagePasser, _ = hashAlloc(alloc, false)
+		stateRoot, storageRootMessagePasser, _ = hashAlloc(alloc, false, false)
 	)
-	if storageRootMessagePasser.Cmp(common.Hash{}) != 0 {
+	if storageRootMessagePasser != (common.Hash{}) {
 		t.Fatalf("unexpected storage root")
 	}
 	blob, _ := json.Marshal(alloc)

core/rlp_test.go

@@ -19,6 +19,7 @@ package core
 import (
 	"fmt"
 	"math/big"
+	"reflect"
 	"testing"
 
 	"github.com/ethereum/go-ethereum/common"
@@ -27,7 +28,6 @@ import (
 	"github.com/ethereum/go-ethereum/crypto"
 	"github.com/ethereum/go-ethereum/params"
 	"github.com/ethereum/go-ethereum/rlp"
-	"github.com/go-test/deep"
 	"github.com/stretchr/testify/assert"
 	"golang.org/x/crypto/sha3"
 )
@@ -219,16 +219,23 @@ func TestBlockRlpEncodeDecode(t *testing.T) {
 	assert.Nil(t, err)
 
 	check := func(f string, got, want interface{}) {
-		if diff := deep.Equal(got, want); diff != nil {
-			t.Errorf("%s mismatch: diff: %v", f, diff)
+		if equal := reflect.DeepEqual(got, want); equal != true {
+			t.Errorf("%s mismatch", f)
+			t.Errorf("Got: %+v", got)
+			t.Errorf("Want: %+v", want)
 		}
 	}
 
+	// There's an odd inconsistency in the way `ExtraData` field, when it is empty is returned after
+	// rlp encode-decode roundtrip. The input is an empty byte slice, but the output is a nil slice,
+	// due to which the reflect.DeepEqual fails. So, we compare a few fields in the header/block manually.
+	// for triaging this, "https://pkg.go.dev/github.com/go-test/deep" was useful since it spits out the
+	// exact field that is different.
 	check("Header WithdrawalsHash", decoded.Header().WithdrawalsHash, block.Header().WithdrawalsHash)
-	check("Header", *decoded.Header(), *block.Header())
+	check("Header Parent Hash", decoded.Header().ParentHash, block.Header().ParentHash)
 	check("Transactions", len(decoded.Transactions()), len(block.Transactions()))
-	check("Uncles[0]", *decoded.Uncles()[0], *block.Uncles()[0])
-	check("Uncles[1]", *decoded.Uncles()[1], *block.Uncles()[1])
+	check("Uncles[0]", (*decoded.Uncles()[0]).ParentHash, (*block.Uncles()[0]).ParentHash)
+	check("Uncles[1]", (*decoded.Uncles()[1]).ParentHash, (*block.Uncles()[1]).ParentHash)
 	check("Withdrawals", decoded.Withdrawals(), block.Withdrawals())
 	check("Requests", decoded.Requests(), block.Requests())
 }

core/types/block.go

@@ -86,7 +86,7 @@ type Header struct {
 	GasLimit    uint64         `json:"gasLimit"         gencodec:"required"`
 	GasUsed     uint64         `json:"gasUsed"          gencodec:"required"`
 	Time        uint64         `json:"timestamp"        gencodec:"required"`
-	Extra       []byte         `json:"extraData"        gencodec:"required" deep:"-"`
+	Extra       []byte         `json:"extraData"        gencodec:"required"`
 	MixDigest   common.Hash    `json:"mixHash"`
 	Nonce       BlockNonce     `json:"nonce"`
 
@@ -302,18 +302,16 @@ func NewBlock(header *Header, body *Body, receipts []*Receipt, hasher TrieHasher
 		}
 		b.header.WithdrawalsHash = header.WithdrawalsHash
 		b.withdrawals = slices.Clone(withdrawals)
-	} else {
+	} else if withdrawals == nil {
 		// pre-Canyon
-		if withdrawals == nil {
-			b.header.WithdrawalsHash = nil
-		} else if len(withdrawals) == 0 {
-			b.header.WithdrawalsHash = &EmptyWithdrawalsHash
-			b.withdrawals = Withdrawals{}
-		} else {
-			hash := DeriveSha(Withdrawals(withdrawals), hasher)
-			b.header.WithdrawalsHash = &hash
-			b.withdrawals = slices.Clone(withdrawals)
-		}
+		b.header.WithdrawalsHash = nil
+	} else if len(withdrawals) == 0 {
+		b.header.WithdrawalsHash = &EmptyWithdrawalsHash
+		b.withdrawals = Withdrawals{}
+	} else {
+		hash := DeriveSha(Withdrawals(withdrawals), hasher)
+		b.header.WithdrawalsHash = &hash
+		b.withdrawals = slices.Clone(withdrawals)
 	}
 
 	if requests == nil {

go.mod

@@ -29,7 +29,6 @@ require (
 	github.com/fjl/gencodec v0.0.0-20230517082657-f9840df7b83e
 	github.com/fsnotify/fsnotify v1.6.0
 	github.com/gballet/go-libpcsclite v0.0.0-20190607065134-2772fd86a8ff
-	github.com/go-test/deep v1.1.1
 	github.com/gofrs/flock v0.8.1
 	github.com/golang-jwt/jwt/v4 v4.5.0
 	github.com/golang/snappy v0.0.5-0.20220116011046-fa5810519dcb

go.sum

@@ -215,8 +215,6 @@ github.com/go-openapi/swag v0.19.5/go.mod h1:POnQmlKehdgb5mhVOsnJFsivZCEZ/vjK9gh
 github.com/go-sourcemap/sourcemap v2.1.3+incompatible h1:W1iEw64niKVGogNgBN3ePyLFfuisuzeidWPMPWmECqU=
 github.com/go-sourcemap/sourcemap v2.1.3+incompatible/go.mod h1:F8jJfvm2KbVjc5NqelyYJmf/v5J0dwNLS2mL4sNA1Jg=
 github.com/go-stack/stack v1.8.0/go.mod h1:v0f6uXyyMGvRgIKkXu+yp6POWl0qKG85gN/melR3HDY=
-github.com/go-test/deep v1.1.1 h1:0r/53hagsehfO4bzD2Pgr/+RgHqhmf+k1Bpse2cTu1U=
-github.com/go-test/deep v1.1.1/go.mod h1:5C2ZWiW0ErCdrYzpqxLbTX7MG14M9iiw8DgHncVwcsE=
 github.com/goccy/go-json v0.10.2 h1:CrxCmQqYDkv1z7lO7Wbh2HN93uovUHgrECaO5ZrCXAU=
 github.com/goccy/go-json v0.10.2/go.mod h1:6MelG93GURQebXPDq3khkgXZkazVtN9CRI+MGFi0w8I=
 github.com/gofrs/flock v0.8.1 h1:+gYjHKf32LDeiEEFhQaotPbLuUXjY5ZqxKgXy7n59aw=