Changes based on PR comments

consensus/beacon/consensus.go

@@ -261,11 +261,7 @@ func (beacon *Beacon) verifyHeader(chain consensus.ChainHeaderReader, header, pa
 		return err
 	}
 	// Verify existence / non-existence of withdrawalsHash.
-	headerInfo, err := types.DeserializeHeaderExtraInformation(header)
-	if err != nil {
-		return err
-	}
-	shanghai := chain.Config().IsShanghai(header.Time, headerInfo.ArbOSFormatVersion)
+	shanghai := chain.Config().IsShanghai(header.Time, types.DeserializeHeaderExtraInformation(header).ArbOSFormatVersion)
 	if shanghai && header.WithdrawalsHash == nil {
 		return fmt.Errorf("missing withdrawalsHash")
 	}
@@ -358,11 +354,7 @@ func (beacon *Beacon) FinalizeAndAssemble(chain consensus.ChainHeaderReader, hea
 	if !beacon.IsPoSHeader(header) {
 		return beacon.ethone.FinalizeAndAssemble(chain, header, state, txs, uncles, receipts, nil)
 	}
-	headerInfo, err := types.DeserializeHeaderExtraInformation(header)
-	if err != nil {
-		return nil, err
-	}
-	shanghai := chain.Config().IsShanghai(header.Time, headerInfo.ArbOSFormatVersion)
+	shanghai := chain.Config().IsShanghai(header.Time, types.DeserializeHeaderExtraInformation(header).ArbOSFormatVersion)
 	if shanghai {
 		// All blocks after Shanghai must include a withdrawals root.
 		if withdrawals == nil {

consensus/clique/clique.go

@@ -298,11 +298,7 @@ func (c *Clique) verifyHeader(chain consensus.ChainHeaderReader, header *types.H
 	if header.GasLimit > params.MaxGasLimit {
 		return fmt.Errorf("invalid gasLimit: have %v, max %v", header.GasLimit, params.MaxGasLimit)
 	}
-	headerInfo, err := types.DeserializeHeaderExtraInformation(header)
-	if err != nil {
-		return err
-	}
-	if chain.Config().IsShanghai(header.Time, headerInfo.ArbOSFormatVersion) {
+	if chain.Config().IsShanghai(header.Time, types.DeserializeHeaderExtraInformation(header).ArbOSFormatVersion) {
 		return fmt.Errorf("clique does not support shanghai fork")
 	}
 	// If all checks passed, validate any special fields for hard forks

consensus/ethash/consensus.go

@@ -310,11 +310,7 @@ func (ethash *Ethash) verifyHeader(chain consensus.ChainHeaderReader, header, pa
 	if diff := new(big.Int).Sub(header.Number, parent.Number); diff.Cmp(big.NewInt(1)) != 0 {
 		return consensus.ErrInvalidNumber
 	}
-	headerInfo, err := types.DeserializeHeaderExtraInformation(header)
-	if err != nil {
-		return err
-	}
-	if chain.Config().IsShanghai(header.Time, headerInfo.ArbOSFormatVersion) {
+	if chain.Config().IsShanghai(header.Time, types.DeserializeHeaderExtraInformation(header).ArbOSFormatVersion) {
 		return fmt.Errorf("ethash does not support shanghai fork")
 	}
 	// Verify the engine specific seal securing the block

core/evm.go

@@ -55,7 +55,6 @@ func NewEVMBlockContext(header *types.Header, chain ChainContext, author *common
 	if header.Difficulty.Cmp(common.Big0) == 0 {
 		random = &header.MixDigest
 	}
-	headerInfo, _ := types.DeserializeHeaderExtraInformation(header)
 	return vm.BlockContext{
 		CanTransfer:  CanTransfer,
 		Transfer:     Transfer,
@@ -67,7 +66,7 @@ func NewEVMBlockContext(header *types.Header, chain ChainContext, author *common
 		BaseFee:      baseFee,
 		GasLimit:     header.GasLimit,
 		Random:       random,
-		ArbOSVersion: headerInfo.ArbOSFormatVersion,
+		ArbOSVersion: types.DeserializeHeaderExtraInformation(header).ArbOSFormatVersion,
 	}
 }
 

core/genesis.go

@@ -466,11 +466,7 @@ func (g *Genesis) ToBlock() *types.Block {
 		}
 	}
 	var withdrawals []*types.Withdrawal
-	headInfo, err := types.DeserializeHeaderExtraInformation(head)
-	if err != nil {
-		return nil
-	}
-	if g.Config != nil && g.Config.IsShanghai(g.Timestamp, headInfo.ArbOSFormatVersion) {
+	if g.Config != nil && g.Config.IsShanghai(g.Timestamp, types.DeserializeHeaderExtraInformation(head).ArbOSFormatVersion) {
 		head.WithdrawalsHash = &types.EmptyWithdrawalsHash
 		withdrawals = make([]*types.Withdrawal, 0)
 	}

core/state_processor.go

@@ -87,11 +87,7 @@ func (p *StateProcessor) Process(block *types.Block, statedb *state.StateDB, cfg
 	}
 	// Fail if Shanghai not enabled and len(withdrawals) is non-zero.
 	withdrawals := block.Withdrawals()
-	headerInfo, err := types.DeserializeHeaderExtraInformation(header)
-	if err != nil {
-		return nil, nil, 0, err
-	}
-	if len(withdrawals) > 0 && !p.config.IsShanghai(block.Time(), headerInfo.ArbOSFormatVersion) {
+	if len(withdrawals) > 0 && !p.config.IsShanghai(block.Time(), types.DeserializeHeaderExtraInformation(header).ArbOSFormatVersion) {
 		return nil, nil, 0, fmt.Errorf("withdrawals before shanghai")
 	}
 	// Finalize the block, applying any consensus engine specific extras (e.g. block rewards)

core/state_processor_test.go

@@ -403,11 +403,7 @@ func GenerateBadBlock(parent *types.Block, engine consensus.Engine, txs types.Tr
 	if config.IsLondon(header.Number) {
 		header.BaseFee = misc.CalcBaseFee(config, parent.Header())
 	}
-	headerInfo, err := types.DeserializeHeaderExtraInformation(header)
-	if err != nil {
-		return nil
-	}
-	if config.IsShanghai(header.Time, headerInfo.ArbOSFormatVersion) {
+	if config.IsShanghai(header.Time, types.DeserializeHeaderExtraInformation(header).ArbOSFormatVersion) {
 		header.WithdrawalsHash = &types.EmptyWithdrawalsHash
 	}
 	var receipts []*types.Receipt
@@ -427,11 +423,7 @@ func GenerateBadBlock(parent *types.Block, engine consensus.Engine, txs types.Tr
 	}
 	header.Root = common.BytesToHash(hasher.Sum(nil))
 	// Assemble and return the final block for sealing
-	headerInfo, err = types.DeserializeHeaderExtraInformation(header)
-	if err != nil {
-		return nil
-	}
-	if config.IsShanghai(header.Time, headerInfo.ArbOSFormatVersion) {
+	if config.IsShanghai(header.Time, types.DeserializeHeaderExtraInformation(header).ArbOSFormatVersion) {
 		return types.NewBlockWithWithdrawals(header, txs, nil, receipts, []*types.Withdrawal{}, trie.NewStackTrie(nil))
 	}
 	return types.NewBlock(header, txs, nil, receipts, trie.NewStackTrie(nil))

core/txpool/txpool.go

@@ -1312,12 +1312,7 @@ func (pool *TxPool) reset(oldHead, newHead *types.Header) {
 	pool.istanbul = pool.chainconfig.IsIstanbul(next)
 	pool.eip2718 = pool.chainconfig.IsBerlin(next)
 	pool.eip1559 = pool.chainconfig.IsLondon(next)
-	newHeadInfo, err := types.DeserializeHeaderExtraInformation(newHead)
-	if err != nil {
-		log.Error(err.Error())
-		return
-	}
-	pool.shanghai = pool.chainconfig.IsShanghai(uint64(time.Now().Unix()), newHeadInfo.ArbOSFormatVersion)
+	pool.shanghai = pool.chainconfig.IsShanghai(uint64(time.Now().Unix()), types.DeserializeHeaderExtraInformation(newHead).ArbOSFormatVersion)
 }
 
 // promoteExecutables moves transactions that have become processable from the

core/types/arb_types.go

@@ -3,7 +3,6 @@ package types
 import (
 	"context"
 	"encoding/binary"
-	"fmt"
 	"math/big"
 
 	"github.com/ethereum/go-ethereum/common/hexutil"
@@ -444,19 +443,16 @@ func (info HeaderInfo) UpdateHeaderWithInfo(header *Header) {
 	header.Extra = info.extra()
 }
 
-func DeserializeHeaderExtraInformation(header *Header) (HeaderInfo, error) {
-	if header.BaseFee == nil || header.BaseFee.Sign() == 0 || len(header.Extra) == 0 {
+func DeserializeHeaderExtraInformation(header *Header) HeaderInfo {
+	if header.BaseFee == nil || header.BaseFee.Sign() == 0 || len(header.Extra) != 32 || header.Difficulty.Cmp(common.Big1) != 0 {
 		// imported blocks have no base fee
 		// The genesis block doesn't have an ArbOS encoded extra field
-		return HeaderInfo{}, nil
-	}
-	if len(header.Extra) != 32 {
-		return HeaderInfo{}, fmt.Errorf("unexpected header extra field length %v", len(header.Extra))
+		return HeaderInfo{}
 	}
 	extra := HeaderInfo{}
 	copy(extra.SendRoot[:], header.Extra)
 	extra.SendCount = binary.BigEndian.Uint64(header.MixDigest[:8])
 	extra.L1BlockNumber = binary.BigEndian.Uint64(header.MixDigest[8:16])
 	extra.ArbOSFormatVersion = binary.BigEndian.Uint64(header.MixDigest[16:24])
-	return extra, nil
+	return extra
 }

eth/catalyst/api.go

@@ -173,11 +173,7 @@ func (api *ConsensusAPI) ForkchoiceUpdatedV1(update engine.ForkchoiceStateV1, pa
 		if payloadAttributes.Withdrawals != nil {
 			return engine.STATUS_INVALID, engine.InvalidParams.With(fmt.Errorf("withdrawals not supported in V1"))
 		}
-		headerInfo, err := types.DeserializeHeaderExtraInformation(api.eth.BlockChain().CurrentHeader())
-		if err != nil {
-			return engine.ForkChoiceResponse{}, err
-		}
-		if api.eth.BlockChain().Config().IsShanghai(payloadAttributes.Timestamp, headerInfo.ArbOSFormatVersion) {
+		if api.eth.BlockChain().Config().IsShanghai(payloadAttributes.Timestamp, types.DeserializeHeaderExtraInformation(api.eth.BlockChain().CurrentHeader()).ArbOSFormatVersion) {
 			return engine.STATUS_INVALID, engine.InvalidParams.With(fmt.Errorf("forkChoiceUpdateV1 called post-shanghai"))
 		}
 	}
@@ -195,11 +191,7 @@ func (api *ConsensusAPI) ForkchoiceUpdatedV2(update engine.ForkchoiceStateV1, pa
 }
 
 func (api *ConsensusAPI) verifyPayloadAttributes(attr *engine.PayloadAttributes) error {
-	headerInfo, err := types.DeserializeHeaderExtraInformation(api.eth.BlockChain().CurrentHeader())
-	if err != nil {
-		return err
-	}
-	if !api.eth.BlockChain().Config().IsShanghai(attr.Timestamp, headerInfo.ArbOSFormatVersion) {
+	if !api.eth.BlockChain().Config().IsShanghai(attr.Timestamp, types.DeserializeHeaderExtraInformation(api.eth.BlockChain().CurrentHeader()).ArbOSFormatVersion) {
 		// Reject payload attributes with withdrawals before shanghai
 		if attr.Withdrawals != nil {
 			return errors.New("withdrawals before shanghai")
@@ -425,11 +417,7 @@ func (api *ConsensusAPI) NewPayloadV1(params engine.ExecutableData) (engine.Payl
 
 // NewPayloadV2 creates an Eth1 block, inserts it in the chain, and returns the status of the chain.
 func (api *ConsensusAPI) NewPayloadV2(params engine.ExecutableData) (engine.PayloadStatusV1, error) {
-	headerInfo, err := types.DeserializeHeaderExtraInformation(api.eth.BlockChain().CurrentHeader())
-	if err != nil {
-		return engine.PayloadStatusV1{}, err
-	}
-	if api.eth.BlockChain().Config().IsShanghai(params.Timestamp, headerInfo.ArbOSFormatVersion) {
+	if api.eth.BlockChain().Config().IsShanghai(params.Timestamp, types.DeserializeHeaderExtraInformation(api.eth.BlockChain().CurrentHeader()).ArbOSFormatVersion) {
 		if params.Withdrawals == nil {
 			return engine.PayloadStatusV1{Status: engine.INVALID}, engine.InvalidParams.With(fmt.Errorf("nil withdrawals post-shanghai"))
 		}

internal/ethapi/api.go

@@ -1353,14 +1353,10 @@ func RPCMarshalBlock(block *types.Block, inclTx bool, fullTx bool, config *param
 }
 
 func fillArbitrumNitroHeaderInfo(header *types.Header, fields map[string]interface{}) {
-	info, err := types.DeserializeHeaderExtraInformation(header)
-	if err != nil {
-		log.Error("Expected header to contain arbitrum data", "blockHash", header.Hash())
-	} else {
-		fields["l1BlockNumber"] = hexutil.Uint64(info.L1BlockNumber)
-		fields["sendRoot"] = info.SendRoot
-		fields["sendCount"] = hexutil.Uint64(info.SendCount)
-	}
+	info := types.DeserializeHeaderExtraInformation(header)
+	fields["l1BlockNumber"] = hexutil.Uint64(info.L1BlockNumber)
+	fields["sendRoot"] = info.SendRoot
+	fields["sendCount"] = hexutil.Uint64(info.SendCount)
 }
 
 // rpcMarshalHeader uses the generalized output filler, then adds the total difficulty field, which requires
@@ -1640,12 +1636,8 @@ func AccessList(ctx context.Context, b Backend, blockNrOrHash rpc.BlockNumberOrH
 		to = crypto.CreateAddress(args.from(), uint64(*args.Nonce))
 	}
 	isPostMerge := header.Difficulty.Cmp(common.Big0) == 0
-	headerInfo, err := types.DeserializeHeaderExtraInformation(header)
-	if err != nil {
-		return nil, 0, nil, err
-	}
 	// Retrieve the precompiles since they don't need to be added to the access list
-	precompiles := vm.ActivePrecompiles(b.ChainConfig().Rules(header.Number, isPostMerge, header.Time, headerInfo.ArbOSFormatVersion))
+	precompiles := vm.ActivePrecompiles(b.ChainConfig().Rules(header.Number, isPostMerge, header.Time, types.DeserializeHeaderExtraInformation(header).ArbOSFormatVersion))
 
 	// Create an initial tracer
 	prevTracer := logger.NewAccessListTracer(nil, args.from(), to, precompiles)
@@ -1882,12 +1874,7 @@ func (s *TransactionAPI) GetTransactionReceipt(ctx context.Context, hash common.
 		}
 		if s.b.ChainConfig().IsArbitrumNitro(header.Number) {
 			fields["effectiveGasPrice"] = hexutil.Uint64(header.BaseFee.Uint64())
-			info, err := types.DeserializeHeaderExtraInformation(header)
-			if err != nil {
-				log.Error("Expected header to contain arbitrum data", "blockHash", blockHash)
-			} else {
-				fields["l1BlockNumber"] = hexutil.Uint64(info.L1BlockNumber)
-			}
+			fields["l1BlockNumber"] = hexutil.Uint64(types.DeserializeHeaderExtraInformation(header).L1BlockNumber)
 		} else {
 			inner := tx.GetInner()
 			arbTx, ok := inner.(*types.ArbitrumLegacyTxData)

light/txpool.go

@@ -318,12 +318,7 @@ func (pool *TxPool) setNewHead(head *types.Header) {
 	next := new(big.Int).Add(head.Number, big.NewInt(1))
 	pool.istanbul = pool.config.IsIstanbul(next)
 	pool.eip2718 = pool.config.IsBerlin(next)
-	headInfo, err := types.DeserializeHeaderExtraInformation(head)
-	if err != nil {
-		log.Error(err.Error())
-		return
-	}
-	pool.shanghai = pool.config.IsShanghai(uint64(time.Now().Unix()), headInfo.ArbOSFormatVersion)
+	pool.shanghai = pool.config.IsShanghai(uint64(time.Now().Unix()), types.DeserializeHeaderExtraInformation(head).ArbOSFormatVersion)
 }
 
 // Stop stops the light transaction pool