Revert "Isthmus: withdrawals root in block header" (ethereum#449)

beacon/engine/types.go

@@ -94,11 +94,6 @@ type ExecutableData struct {
 	ExcessBlobGas    *uint64                 `json:"excessBlobGas"`
 	Deposits         types.Deposits          `json:"depositRequests"`
 	ExecutionWitness *types.ExecutionWitness `json:"executionWitness,omitempty"`
-
-	// OP-Stack Isthmus specific field:
-	// instead of computing the root from a withdrawals list, set it directly.
-	// The "withdrawals" list attribute must be non-nil but empty.
-	WithdrawalsRoot *common.Hash `json:"withdrawalsRoot,omitempty"`
 }
 
 // JSON type overrides for executableData.
@@ -230,8 +225,8 @@ func decodeTransactions(enc [][]byte) ([]*types.Transaction, error) {
 // and that the blockhash of the constructed block matches the parameters. Nil
 // Withdrawals value will propagate through the returned block. Empty
 // Withdrawals value must be passed via non-nil, length 0 value in data.
-func ExecutableDataToBlock(data ExecutableData, versionedHashes []common.Hash, beaconRoot *common.Hash, config *params.ChainConfig) (*types.Block, error) {
-	block, err := ExecutableDataToBlockNoHash(data, versionedHashes, beaconRoot, config)
+func ExecutableDataToBlock(data ExecutableData, versionedHashes []common.Hash, beaconRoot *common.Hash) (*types.Block, error) {
+	block, err := ExecutableDataToBlockNoHash(data, versionedHashes, beaconRoot)
 	if err != nil {
 		return nil, err
 	}
@@ -244,7 +239,7 @@ func ExecutableDataToBlock(data ExecutableData, versionedHashes []common.Hash, b
 // ExecutableDataToBlockNoHash is analogous to ExecutableDataToBlock, but is used
 // for stateless execution, so it skips checking if the executable data hashes to
 // the requested hash (stateless has to *compute* the root hash, it's not given).
-func ExecutableDataToBlockNoHash(data ExecutableData, versionedHashes []common.Hash, beaconRoot *common.Hash, config *params.ChainConfig) (*types.Block, error) {
+func ExecutableDataToBlockNoHash(data ExecutableData, versionedHashes []common.Hash, beaconRoot *common.Hash) (*types.Block, error) {
 	txs, err := decodeTransactions(data.Transactions)
 	if err != nil {
 		return nil, err
@@ -275,18 +270,7 @@ func ExecutableDataToBlockNoHash(data ExecutableData, versionedHashes []common.H
 	// ExecutableData before withdrawals are enabled by marshaling
 	// Withdrawals as the json null value.
 	var withdrawalsRoot *common.Hash
-	if config.IsOptimismIsthmus(data.Timestamp) {
-		if data.WithdrawalsRoot == nil {
-			return nil, fmt.Errorf("attribute WithdrawalsRoot is required for Isthmus blocks")
-		}
-		if data.Withdrawals == nil || len(data.Withdrawals) > 0 {
-			return nil, fmt.Errorf("expected non-nil empty withdrawals operation list in Isthmus, but got: %v", data.Withdrawals)
-		}
-	}
-	if data.WithdrawalsRoot != nil {
-		h := *data.WithdrawalsRoot // copy, avoid any sharing of memory
-		withdrawalsRoot = &h
-	} else if data.Withdrawals != nil {
+	if data.Withdrawals != nil {
 		h := types.DeriveSha(types.Withdrawals(data.Withdrawals), trie.NewStackTrie(nil))
 		withdrawalsRoot = &h
 	}
@@ -353,8 +337,6 @@ func BlockToExecutableData(block *types.Block, fees *big.Int, sidecars []*types.
 		BlobGasUsed:      block.BlobGasUsed(),
 		ExcessBlobGas:    block.ExcessBlobGas(),
 		ExecutionWitness: block.ExecutionWitness(),
-		// OP-Stack addition: withdrawals list alone does not express the withdrawals storage-root.
-		WithdrawalsRoot: block.WithdrawalsRoot(),
 	}
 	bundle := BlobsBundleV1{
 		Commitments: make([]hexutil.Bytes, 0),

consensus/beacon/consensus.go

@@ -403,18 +403,8 @@ func (beacon *Beacon) FinalizeAndAssemble(chain consensus.ChainHeaderReader, hea
 	// Assign the final state root to header.
 	header.Root = state.IntermediateRoot(true)
 
-	if chain.Config().IsOptimismIsthmus(header.Time) {
-		if body.Withdrawals == nil || len(body.Withdrawals) > 0 { // We verify nil/empty withdrawals in the CL pre-Isthmus
-			return nil, fmt.Errorf("expected non-nil empty withdrawals operation list in Isthmus, but got: %v", body.Withdrawals)
-		}
-		// State-root has just been computed, we can get an accurate storage-root now.
-		h := state.GetStorageRoot(params.OptimismL2ToL1MessagePasser)
-		header.WithdrawalsHash = &h
-		state.AccessEvents().AddAccount(params.OptimismL2ToL1MessagePasser, false) // include in execution witness
-	}
-
 	// Assemble the final block.
-	block := types.NewBlock(header, body, receipts, trie.NewStackTrie(nil), chain.Config())
+	block := types.NewBlock(header, body, receipts, trie.NewStackTrie(nil))
 
 	// Create the block witness and attach to block.
 	// This step needs to happen as late as possible to catch all access events.

consensus/clique/clique.go

@@ -304,7 +304,7 @@ func (c *Clique) verifyHeader(chain consensus.ChainHeaderReader, header *types.H
 	}
 	// Verify the non-existence of withdrawalsHash.
 	if header.WithdrawalsHash != nil {
-		return fmt.Errorf("invalid withdrawalsHash: have %s, expected nil", header.WithdrawalsHash)
+		return fmt.Errorf("invalid withdrawalsHash: have %x, expected nil", header.WithdrawalsHash)
 	}
 	if chain.Config().IsCancun(header.Number, header.Time) {
 		return errors.New("clique does not support cancun fork")
@@ -597,7 +597,7 @@ func (c *Clique) FinalizeAndAssemble(chain consensus.ChainHeaderReader, header *
 	header.Root = state.IntermediateRoot(chain.Config().IsEIP158(header.Number))
 
 	// Assemble and return the final block for sealing.
-	return types.NewBlock(header, &types.Body{Transactions: body.Transactions}, receipts, trie.NewStackTrie(nil), chain.Config()), nil
+	return types.NewBlock(header, &types.Body{Transactions: body.Transactions}, receipts, trie.NewStackTrie(nil)), nil
 }
 
 // Authorize injects a private key into the consensus engine to mint new blocks

consensus/ethash/consensus.go

@@ -270,7 +270,7 @@ func (ethash *Ethash) verifyHeader(chain consensus.ChainHeaderReader, header, pa
 	}
 	// Verify the non-existence of withdrawalsHash.
 	if header.WithdrawalsHash != nil {
-		return fmt.Errorf("invalid withdrawalsHash: have %s, expected nil", header.WithdrawalsHash)
+		return fmt.Errorf("invalid withdrawalsHash: have %x, expected nil", header.WithdrawalsHash)
 	}
 	if chain.Config().IsCancun(header.Number, header.Time) {
 		return errors.New("ethash does not support cancun fork")
@@ -520,7 +520,7 @@ func (ethash *Ethash) FinalizeAndAssemble(chain consensus.ChainHeaderReader, hea
 	header.Root = state.IntermediateRoot(chain.Config().IsEIP158(header.Number))
 
 	// Header seems complete, assemble into a block and return
-	return types.NewBlock(header, &types.Body{Transactions: body.Transactions, Uncles: body.Uncles, Withdrawals: body.Withdrawals}, receipts, trie.NewStackTrie(nil), chain.Config()), nil
+	return types.NewBlock(header, &types.Body{Transactions: body.Transactions, Uncles: body.Uncles}, receipts, trie.NewStackTrie(nil)), nil
 }
 
 // SealHash returns the hash of a block prior to it being sealed.

core/block_validator.go

@@ -73,13 +73,8 @@ func (v *BlockValidator) ValidateBody(block *types.Block) error {
 		if block.Withdrawals() == nil {
 			return errors.New("missing withdrawals in block body")
 		}
-		if v.config.IsOptimismIsthmus(header.Time) {
-			if len(block.Withdrawals()) > 0 {
-				return errors.New("no withdrawal block-operations allowed, withdrawalsRoot is set to storage root")
-			}
-			// The withdrawalsHash is verified in ValidateState, like the state root, as verification requires state merkleization.
-		} else if hash := types.DeriveSha(block.Withdrawals(), trie.NewStackTrie(nil)); hash != *header.WithdrawalsHash {
-			return fmt.Errorf("withdrawals root hash mismatch (header value %s, calculated %s)", *header.WithdrawalsHash, hash)
+		if hash := types.DeriveSha(block.Withdrawals(), trie.NewStackTrie(nil)); hash != *header.WithdrawalsHash {
+			return fmt.Errorf("withdrawals root hash mismatch (header value %x, calculated %x)", *header.WithdrawalsHash, hash)
 		}
 	} else if block.Withdrawals() != nil {
 		// Withdrawals are not allowed prior to Shanghai fork
@@ -160,15 +155,6 @@ func (v *BlockValidator) ValidateState(block *types.Block, statedb *state.StateD
 	if root := statedb.IntermediateRoot(v.config.IsEIP158(header.Number)); header.Root != root {
 		return fmt.Errorf("invalid merkle root (remote: %x local: %x) dberr: %w", header.Root, root, statedb.Error())
 	}
-	if v.config.IsOptimismIsthmus(block.Time()) {
-		if header.WithdrawalsHash == nil {
-			return errors.New("expected withdrawals root in OP-Stack post-Isthmus block header")
-		}
-		// Validate the withdrawals root against the L2 withdrawals storage, similar to how the StateRoot is verified.
-		if root := statedb.GetStorageRoot(params.OptimismL2ToL1MessagePasser); *header.WithdrawalsHash != root {
-			return fmt.Errorf("invalid withdrawals hash (remote: %s local: %s) dberr: %w", *header.WithdrawalsHash, root, statedb.Error())
-		}
-	}
 	return nil
 }
 

core/chain_makers.go

@@ -329,11 +329,6 @@ func GenerateChain(config *params.ChainConfig, parent *types.Block, engine conse
 				b.header.Difficulty = big.NewInt(0)
 			}
 		}
-		if config.IsOptimismIsthmus(b.header.Time) {
-			b.withdrawals = make([]*types.Withdrawal, 0)
-			h := types.EmptyWithdrawalsHash
-			b.header.WithdrawalsHash = &h
-		}
 		// Mutate the state and block according to any hard-fork specs
 		if daoBlock := config.DAOForkBlock; daoBlock != nil {
 			limit := new(big.Int).Add(daoBlock, params.DAOForkExtraRange)

core/genesis.go

@@ -126,11 +126,8 @@ func ReadGenesis(db ethdb.Database) (*Genesis, error) {
 	return &genesis, nil
 }
 
-// hashAlloc returns the following:
-// * 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, isIsthmus bool) (common.Hash, common.Hash, error) {
+// hashAlloc computes the state root according to the genesis specification.
+func hashAlloc(ga *types.GenesisAlloc, isVerkle bool) (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.
@@ -146,7 +143,7 @@ func hashAlloc(ga *types.GenesisAlloc, isVerkle, isIsthmus bool) (common.Hash, c
 	db := rawdb.NewMemoryDatabase()
 	statedb, err := state.New(types.EmptyRootHash, state.NewDatabase(triedb.NewDatabase(db, config), nil))
 	if err != nil {
-		return common.Hash{}, common.Hash{}, err
+		return common.Hash{}, err
 	}
 	for addr, account := range *ga {
 		if account.Balance != nil {
@@ -158,27 +155,15 @@ func hashAlloc(ga *types.GenesisAlloc, isVerkle, isIsthmus bool) (common.Hash, c
 			statedb.SetState(addr, key, value)
 		}
 	}
-
-	stateRoot, err := statedb.Commit(0, false)
-	if err != nil {
-		return common.Hash{}, common.Hash{}, err
-	}
-	// get the storage root of the L2ToL1MessagePasser contract
-	var storageRootMessagePasser common.Hash
-	if isIsthmus {
-		storageRootMessagePasser = statedb.GetStorageRoot(params.OptimismL2ToL1MessagePasser)
-	}
-
-	return stateRoot, storageRootMessagePasser, nil
+	return statedb.Commit(0, false)
 }
 
 // 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, isIsthmus bool) (common.Hash, common.Hash, error) {
+// generated states will be persisted into the given database.
+func flushAlloc(ga *types.GenesisAlloc, triedb *triedb.Database) (common.Hash, error) {
 	statedb, err := state.New(types.EmptyRootHash, state.NewDatabase(triedb, nil))
 	if err != nil {
-		return common.Hash{}, common.Hash{}, err
+		return common.Hash{}, err
 	}
 	for addr, account := range *ga {
 		if account.Balance != nil {
@@ -192,22 +177,17 @@ func flushAlloc(ga *types.GenesisAlloc, triedb *triedb.Database, isIsthmus bool)
 			statedb.SetState(addr, key, value)
 		}
 	}
-	stateRoot, err := statedb.Commit(0, false)
+	root, err := statedb.Commit(0, false)
 	if err != nil {
-		return common.Hash{}, common.Hash{}, err
-	}
-	// get the storage root of the L2ToL1MessagePasser contract
-	var storageRootMessagePasser common.Hash
-	if isIsthmus {
-		storageRootMessagePasser = statedb.GetStorageRoot(params.OptimismL2ToL1MessagePasser)
+		return common.Hash{}, err
 	}
 	// Commit newly generated states into disk if it's not empty.
-	if stateRoot != types.EmptyRootHash {
-		if err := triedb.Commit(stateRoot, true); err != nil {
-			return common.Hash{}, common.Hash{}, err
+	if root != types.EmptyRootHash {
+		if err := triedb.Commit(root, true); err != nil {
+			return common.Hash{}, err
 		}
 	}
-	return stateRoot, storageRootMessagePasser, nil
+	return root, nil
 }
 
 func getGenesisState(db ethdb.Database, blockhash common.Hash) (alloc types.GenesisAlloc, err error) {
@@ -497,27 +477,22 @@ func (g *Genesis) IsVerkle() bool {
 
 // ToBlock returns the genesis block according to genesis specification.
 func (g *Genesis) ToBlock() *types.Block {
-	var stateRoot, storageRootMessagePasser common.Hash
+	var root common.Hash
 	var err error
 	if g.StateHash != nil {
 		if len(g.Alloc) > 0 {
 			panic(fmt.Errorf("cannot both have genesis hash %s "+
 				"and non-empty state-allocation", *g.StateHash))
 		}
-		// g.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.Config.IsOptimismIsthmus(g.Timestamp) {
-			panic(fmt.Errorf("stateHash usage disallowed in chain with isthmus active at genesis"))
-		}
-		stateRoot = *g.StateHash
-	} else if stateRoot, storageRootMessagePasser, err = hashAlloc(&g.Alloc, g.IsVerkle(), g.Config.IsOptimismIsthmus(g.Timestamp)); err != nil {
+		root = *g.StateHash
+	} else if root, err = hashAlloc(&g.Alloc, g.IsVerkle()); err != nil {
 		panic(err)
 	}
-	return g.toBlockWithRoot(stateRoot, storageRootMessagePasser)
+	return g.toBlockWithRoot(root)
 }
 
 // toBlockWithRoot constructs the genesis block with the given genesis state root.
-func (g *Genesis) toBlockWithRoot(stateRoot, storageRootMessagePasser common.Hash) *types.Block {
+func (g *Genesis) toBlockWithRoot(root common.Hash) *types.Block {
 	head := &types.Header{
 		Number:     new(big.Int).SetUint64(g.Number),
 		Nonce:      types.EncodeNonce(g.Nonce),
@@ -530,7 +505,7 @@ func (g *Genesis) toBlockWithRoot(stateRoot, storageRootMessagePasser common.Has
 		Difficulty: g.Difficulty,
 		MixDigest:  g.Mixhash,
 		Coinbase:   g.Coinbase,
-		Root:       stateRoot,
+		Root:       root,
 	}
 	if g.GasLimit == 0 {
 		head.GasLimit = params.GenesisGasLimit
@@ -574,16 +549,8 @@ func (g *Genesis) toBlockWithRoot(stateRoot, storageRootMessagePasser common.Has
 			head.RequestsHash = &types.EmptyRequestsHash
 			requests = make(types.Requests, 0)
 		}
-		// If Isthmus is active at genesis, set the WithdrawalRoot to the storage root of the L2ToL1MessagePasser contract.
-		if g.Config.IsOptimismIsthmus(g.Timestamp) {
-			if storageRootMessagePasser == (common.Hash{}) {
-				// if there was no MessagePasser contract storage, set the WithdrawalsHash to the empty hash
-				storageRootMessagePasser = types.EmptyWithdrawalsHash
-			}
-			head.WithdrawalsHash = &storageRootMessagePasser
-		}
 	}
-	return types.NewBlock(head, &types.Body{Withdrawals: withdrawals, Requests: requests}, nil, trie.NewStackTrie(nil), g.Config)
+	return types.NewBlock(head, &types.Body{Withdrawals: withdrawals, Requests: requests}, nil, trie.NewStackTrie(nil))
 }
 
 // Commit writes the block and state of a genesis specification to the database.
@@ -602,23 +569,23 @@ func (g *Genesis) Commit(db ethdb.Database, triedb *triedb.Database) (*types.Blo
 	if config.Clique != nil && len(g.ExtraData) < 32+crypto.SignatureLength {
 		return nil, errors.New("can't start clique chain without signers")
 	}
-	var stateRoot, storageRootMessagePasser common.Hash
-	var err error
+	var stateHash common.Hash
 	if len(g.Alloc) == 0 {
 		if g.StateHash == nil {
 			log.Warn("Empty genesis alloc, and no 'stateHash' override was set")
-			stateRoot = types.EmptyRootHash // default to the hash of the empty state. Some unit-tests rely on this.
+			stateHash = types.EmptyRootHash // default to the hash of the empty state. Some unit-tests rely on this.
 		} else {
-			stateRoot = *g.StateHash
+			stateHash = *g.StateHash
 		}
 	} else {
 		// flush the data to disk and compute the state root
-		stateRoot, storageRootMessagePasser, err = flushAlloc(&g.Alloc, triedb, g.Config.IsIsthmus(g.Timestamp))
+		root, err := flushAlloc(&g.Alloc, triedb)
 		if err != nil {
 			return nil, err
 		}
+		stateHash = root
 	}
-	block := g.toBlockWithRoot(stateRoot, storageRootMessagePasser)
+	block := g.toBlockWithRoot(stateHash)
 
 	// Marshal the genesis state specification and persist.
 	blob, err := json.Marshal(g.Alloc)

core/genesis_test.go

@@ -223,16 +223,13 @@ 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, false)
+		hash, _ = hashAlloc(alloc, false)
 	)
-	if storageRootMessagePasser != (common.Hash{}) {
-		t.Fatalf("unexpected storage root")
-	}
 	blob, _ := json.Marshal(alloc)
-	rawdb.WriteGenesisStateSpec(db, stateRoot, blob)
+	rawdb.WriteGenesisStateSpec(db, hash, blob)
 
 	var reload types.GenesisAlloc
-	err := reload.UnmarshalJSON(rawdb.ReadGenesisStateSpec(db, stateRoot))
+	err := reload.UnmarshalJSON(rawdb.ReadGenesisStateSpec(db, hash))
 	if err != nil {
 		t.Fatalf("Failed to load genesis state %v", err)
 	}

core/rawdb/accessors_indexes_test.go

@@ -76,7 +76,7 @@ func TestLookupStorage(t *testing.T) {
 			tx3 := types.NewTransaction(3, common.BytesToAddress([]byte{0x33}), big.NewInt(333), 3333, big.NewInt(33333), []byte{0x33, 0x33, 0x33})
 			txs := []*types.Transaction{tx1, tx2, tx3}
 
-			block := types.NewBlock(&types.Header{Number: big.NewInt(314)}, &types.Body{Transactions: txs}, nil, newTestHasher(), params.TestChainConfig)
+			block := types.NewBlock(&types.Header{Number: big.NewInt(314)}, &types.Body{Transactions: txs}, nil, newTestHasher())
 
 			// Check that no transactions entries are in a pristine database
 			for i, tx := range txs {