Merge pull request #5350 from ethereum-optimism/aj/extract-block-processor

op-program: Extract block processing logic from engine api

op-program/l2/engineapi/block_processor.go

+package engineapi
+
+import (
+	"errors"
+	"fmt"
+	"math/big"
+
+	"github.com/ethereum-optimism/optimism/op-node/eth"
+	"github.com/ethereum/go-ethereum/common"
+	"github.com/ethereum/go-ethereum/consensus"
+	"github.com/ethereum/go-ethereum/consensus/misc"
+	"github.com/ethereum/go-ethereum/core"
+	"github.com/ethereum/go-ethereum/core/state"
+	"github.com/ethereum/go-ethereum/core/types"
+	"github.com/ethereum/go-ethereum/core/vm"
+	"github.com/ethereum/go-ethereum/params"
+)
+
+var (
+	ErrExceedsGasLimit = errors.New("tx gas exceeds block gas limit")
+	ErrUsesTooMuchGas  = errors.New("action takes too much gas")
+)
+
+type BlockDataProvider interface {
+	StateAt(root common.Hash) (*state.StateDB, error)
+	GetHeader(common.Hash, uint64) *types.Header
+	Engine() consensus.Engine
+	GetVMConfig() *vm.Config
+	Config() *params.ChainConfig
+	consensus.ChainHeaderReader
+}
+
+type BlockProcessor struct {
+	header       *types.Header
+	state        *state.StateDB
+	receipts     types.Receipts
+	transactions types.Transactions
+	gasPool      *core.GasPool
+	dataProvider BlockDataProvider
+}
+
+func NewBlockProcessorFromPayloadAttributes(provider BlockDataProvider, parent common.Hash, params *eth.PayloadAttributes) (*BlockProcessor, error) {
+	header := &types.Header{
+		ParentHash: parent,
+		Coinbase:   params.SuggestedFeeRecipient,
+		Difficulty: common.Big0,
+		GasLimit:   uint64(*params.GasLimit),
+		Time:       uint64(params.Timestamp),
+		Extra:      nil,
+		MixDigest:  common.Hash(params.PrevRandao),
+		Nonce:      types.EncodeNonce(0),
+	}
+	return NewBlockProcessorFromHeader(provider, header)
+}
+
+func NewBlockProcessorFromHeader(provider BlockDataProvider, h *types.Header) (*BlockProcessor, error) {
+	header := *h // Copy to avoid mutating the original header
+
+	if header.GasLimit > params.MaxGasLimit {
+		return nil, fmt.Errorf("invalid gasLimit: have %v, max %v", header.GasLimit, params.MaxGasLimit)
+	}
+	parentHeader := provider.GetHeaderByHash(header.ParentHash)
+	if header.Time <= parentHeader.Time {
+		return nil, errors.New("invalid timestamp")
+	}
+	statedb, err := provider.StateAt(parentHeader.Root)
+	if err != nil {
+		return nil, fmt.Errorf("get parent state: %w", err)
+	}
+	header.Number = new(big.Int).Add(parentHeader.Number, common.Big1)
+	header.BaseFee = misc.CalcBaseFee(provider.Config(), parentHeader)
+	header.GasUsed = 0
+	gasPool := new(core.GasPool).AddGas(header.GasLimit)
+	return &BlockProcessor{
+		header:       &header,
+		state:        statedb,
+		gasPool:      gasPool,
+		dataProvider: provider,
+	}, nil
+}
+
+func (b *BlockProcessor) CheckTxWithinGasLimit(tx *types.Transaction) error {
+	if tx.Gas() > b.header.GasLimit {
+		return fmt.Errorf("%w tx gas: %d, block gas limit: %d", ErrExceedsGasLimit, tx.Gas(), b.header.GasLimit)
+	}
+	if tx.Gas() > b.gasPool.Gas() {
+		return fmt.Errorf("%w: %d, only have %d", ErrUsesTooMuchGas, tx.Gas(), b.gasPool.Gas())
+	}
+	return nil
+}
+
+func (b *BlockProcessor) AddTx(tx *types.Transaction) error {
+	txIndex := len(b.transactions)
+	b.state.SetTxContext(tx.Hash(), txIndex)
+	receipt, err := core.ApplyTransaction(b.dataProvider.Config(), b.dataProvider, &b.header.Coinbase,
+		b.gasPool, b.state, b.header, tx, &b.header.GasUsed, *b.dataProvider.GetVMConfig())
+	if err != nil {
+		return fmt.Errorf("failed to apply deposit transaction to L2 block (tx %d): %w", txIndex, err)
+	}
+	b.receipts = append(b.receipts, receipt)
+	b.transactions = append(b.transactions, tx)
+	return nil
+}
+
+func (b *BlockProcessor) Assemble() (*types.Block, error) {
+	return b.dataProvider.Engine().FinalizeAndAssemble(b.dataProvider, b.header, b.state, b.transactions, nil, b.receipts, nil)
+}
+
+func (b *BlockProcessor) Commit() error {
+	root, err := b.state.Commit(b.dataProvider.Config().IsEIP158(b.header.Number))
+	if err != nil {
+		return fmt.Errorf("state write error: %w", err)
+	}
+	if err := b.state.Database().TrieDB().Commit(root, false); err != nil {
+		return fmt.Errorf("trie write error: %w", err)
+	}
+	return nil
+}

op-program/l2/engineapi/l2_engine_api.go

@@ -6,33 +6,25 @@ import (
 	"encoding/binary"
 	"errors"
 	"fmt"
-	"math/big"
 	"time"
 
+	"github.com/ethereum-optimism/optimism/op-node/eth"
 	"github.com/ethereum/go-ethereum/beacon/engine"
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/consensus"
-	"github.com/ethereum/go-ethereum/consensus/misc"
-	"github.com/ethereum/go-ethereum/core"
 	"github.com/ethereum/go-ethereum/core/state"
 	"github.com/ethereum/go-ethereum/core/types"
 	"github.com/ethereum/go-ethereum/core/vm"
 	"github.com/ethereum/go-ethereum/log"
 	"github.com/ethereum/go-ethereum/params"
-	"github.com/ethereum/go-ethereum/trie"
-
-	"github.com/ethereum-optimism/optimism/op-node/eth"
 )
 
 type EngineBackend interface {
 	CurrentBlock() *types.Header
 	CurrentSafeBlock() *types.Header
 	CurrentFinalBlock() *types.Header
-	GetHeaderByHash(hash common.Hash) *types.Header
 	GetBlockByHash(hash common.Hash) *types.Block
 	GetBlock(hash common.Hash, number uint64) *types.Block
-	// GetHeader returns the header corresponding to the hash/number argument pair.
-	GetHeader(common.Hash, uint64) *types.Header
 	HasBlockAndState(hash common.Hash, number uint64) bool
 	GetCanonicalHash(n uint64) common.Hash
 
@@ -47,6 +39,8 @@ type EngineBackend interface {
 	SetCanonical(head *types.Block) (common.Hash, error)
 	SetFinalized(header *types.Header)
 	SetSafe(header *types.Header)
+
+	consensus.ChainHeaderReader
 }
 
 // L2EngineAPI wraps an engine actor, and implements the RPC backend required to serve the engine API.
@@ -57,14 +51,10 @@ type L2EngineAPI struct {
 	backend EngineBackend
 
 	// L2 block building data
-	l2BuildingHeader *types.Header             // block header that we add txs to for block building
-	l2BuildingState  *state.StateDB            // state used for block building
-	l2GasPool        *core.GasPool             // track gas used of ongoing building
-	pendingIndices   map[common.Address]uint64 // per account, how many txs from the pool were already included in the block, since the pool is lagging behind block mining.
-	l2Transactions   []*types.Transaction      // collects txs that were successfully included into current block build
-	l2Receipts       []*types.Receipt          // collect receipts of ongoing building
-	l2ForceEmpty     bool                      // when no additional txs may be processed (i.e. when sequencer drift runs out)
-	l2TxFailed       []*types.Transaction      // log of failed transactions which could not be included
+	blockProcessor *BlockProcessor
+	pendingIndices map[common.Address]uint64 // per account, how many txs from the pool were already included in the block, since the pool is lagging behind block mining.
+	l2ForceEmpty   bool                      // when no additional txs may be processed (i.e. when sequencer drift runs out)
+	l2TxFailed     []*types.Transaction      // log of failed transactions which could not be included
 
 	payloadID engine.PayloadID // ID of payload that is currently being built
 }
@@ -102,7 +92,10 @@ func computePayloadId(headBlockHash common.Hash, params *eth.PayloadAttributes)
 }
 
 func (ea *L2EngineAPI) RemainingBlockGas() uint64 {
-	return ea.l2GasPool.Gas()
+	if ea.blockProcessor == nil {
+		return 0
+	}
+	return ea.blockProcessor.gasPool.Gas()
 }
 
 func (ea *L2EngineAPI) ForcedEmpty() bool {
@@ -115,12 +108,10 @@ func (ea *L2EngineAPI) PendingIndices(from common.Address) uint64 {
 
 var (
 	ErrNotBuildingBlock = errors.New("not currently building a block, cannot include tx from queue")
-	ErrExceedsGasLimit  = errors.New("tx gas exceeds block gas limit")
-	ErrUsesTooMuchGas   = errors.New("action takes too much gas")
 )
 
 func (ea *L2EngineAPI) IncludeTx(tx *types.Transaction, from common.Address) error {
-	if ea.l2BuildingHeader == nil {
+	if ea.blockProcessor == nil {
 		return ErrNotBuildingBlock
 	}
 	if ea.l2ForceEmpty {
@@ -129,60 +120,32 @@ func (ea *L2EngineAPI) IncludeTx(tx *types.Transaction, from common.Address) err
 		return nil
 	}
 
-	if tx.Gas() > ea.l2BuildingHeader.GasLimit {
-		return fmt.Errorf("%w tx gas: %d, block gas limit: %d", ErrExceedsGasLimit, tx.Gas(), ea.l2BuildingHeader.GasLimit)
-	}
-	if tx.Gas() > uint64(*ea.l2GasPool) {
-		return fmt.Errorf("%w: %d, only have %d", ErrUsesTooMuchGas, tx.Gas(), uint64(*ea.l2GasPool))
+	err := ea.blockProcessor.CheckTxWithinGasLimit(tx)
+	if err != nil {
+		return err
 	}
 
 	ea.pendingIndices[from] = ea.pendingIndices[from] + 1 // won't retry the tx
-	ea.l2BuildingState.SetTxContext(tx.Hash(), len(ea.l2Transactions))
-	receipt, err := core.ApplyTransaction(ea.backend.Config(), ea.backend, &ea.l2BuildingHeader.Coinbase,
-		ea.l2GasPool, ea.l2BuildingState, ea.l2BuildingHeader, tx, &ea.l2BuildingHeader.GasUsed, *ea.backend.GetVMConfig())
+	err = ea.blockProcessor.AddTx(tx)
 	if err != nil {
 		ea.l2TxFailed = append(ea.l2TxFailed, tx)
-		return fmt.Errorf("invalid L2 block (tx %d): %w", len(ea.l2Transactions), err)
+		return fmt.Errorf("invalid L2 block (tx %d): %w", len(ea.blockProcessor.transactions), err)
 	}
-	ea.l2Receipts = append(ea.l2Receipts, receipt)
-	ea.l2Transactions = append(ea.l2Transactions, tx)
 	return nil
 }
 
 func (ea *L2EngineAPI) startBlock(parent common.Hash, params *eth.PayloadAttributes) error {
-	if ea.l2BuildingHeader != nil {
-		ea.log.Warn("started building new block without ending previous block", "previous", ea.l2BuildingHeader, "prev_payload_id", ea.payloadID)
+	if ea.blockProcessor != nil {
+		ea.log.Warn("started building new block without ending previous block", "previous", ea.blockProcessor.header, "prev_payload_id", ea.payloadID)
 	}
 
-	parentHeader := ea.backend.GetHeaderByHash(parent)
-	if parentHeader == nil {
-		return fmt.Errorf("uknown parent block: %s", parent)
-	}
-	statedb, err := ea.backend.StateAt(parentHeader.Root)
+	processor, err := NewBlockProcessorFromPayloadAttributes(ea.backend, parent, params)
 	if err != nil {
-		return fmt.Errorf("failed to init state db around block %s (state %s): %w", parent, parentHeader.Root, err)
-	}
-
-	header := &types.Header{
-		ParentHash: parent,
-		Coinbase:   params.SuggestedFeeRecipient,
-		Difficulty: common.Big0,
-		Number:     new(big.Int).Add(parentHeader.Number, common.Big1),
-		GasLimit:   uint64(*params.GasLimit),
-		Time:       uint64(params.Timestamp),
-		Extra:      nil,
-		MixDigest:  common.Hash(params.PrevRandao),
+		return err
 	}
-
-	header.BaseFee = misc.CalcBaseFee(ea.backend.Config(), parentHeader)
-
-	ea.l2BuildingHeader = header
-	ea.l2BuildingState = statedb
-	ea.l2Receipts = make([]*types.Receipt, 0)
-	ea.l2Transactions = make([]*types.Transaction, 0)
+	ea.blockProcessor = processor
 	ea.pendingIndices = make(map[common.Address]uint64)
 	ea.l2ForceEmpty = params.NoTxPool
-	ea.l2GasPool = new(core.GasPool).AddGas(header.GasLimit)
 	ea.payloadID = computePayloadId(parent, params)
 
 	// pre-process the deposits
@@ -191,29 +154,26 @@ func (ea *L2EngineAPI) startBlock(parent common.Hash, params *eth.PayloadAttribu
 		if err := tx.UnmarshalBinary(otx); err != nil {
 			return fmt.Errorf("transaction %d is not valid: %w", i, err)
 		}
-		ea.l2BuildingState.SetTxContext(tx.Hash(), i)
-		receipt, err := core.ApplyTransaction(ea.backend.Config(), ea.backend, &ea.l2BuildingHeader.Coinbase,
-			ea.l2GasPool, ea.l2BuildingState, ea.l2BuildingHeader, &tx, &ea.l2BuildingHeader.GasUsed, *ea.backend.GetVMConfig())
+		err := ea.blockProcessor.AddTx(&tx)
 		if err != nil {
 			ea.l2TxFailed = append(ea.l2TxFailed, &tx)
 			return fmt.Errorf("failed to apply deposit transaction to L2 block (tx %d): %w", i, err)
 		}
-		ea.l2Receipts = append(ea.l2Receipts, receipt)
-		ea.l2Transactions = append(ea.l2Transactions, &tx)
 	}
 	return nil
 }
 
 func (ea *L2EngineAPI) endBlock() (*types.Block, error) {
-	if ea.l2BuildingHeader == nil {
+	if ea.blockProcessor == nil {
 		return nil, fmt.Errorf("no block is being built currently (id %s)", ea.payloadID)
 	}
-	header := ea.l2BuildingHeader
-	ea.l2BuildingHeader = nil
+	processor := ea.blockProcessor
+	ea.blockProcessor = nil
 
-	header.GasUsed = header.GasLimit - uint64(*ea.l2GasPool)
-	header.Root = ea.l2BuildingState.IntermediateRoot(ea.backend.Config().IsEIP158(header.Number))
-	block := types.NewBlock(header, ea.l2Transactions, nil, ea.l2Receipts, trie.NewStackTrie(nil))
+	block, err := processor.Assemble()
+	if err != nil {
+		return nil, fmt.Errorf("assemble block: %w", err)
+	}
 	return block, nil
 }
 

op-program/l2/engineapi/test/l2_engine_api_tests.go

@@ -11,6 +11,7 @@ import (
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/core/types"
 	"github.com/ethereum/go-ethereum/log"
+	"github.com/ethereum/go-ethereum/params"
 	"github.com/stretchr/testify/require"
 )
 
@@ -36,6 +37,38 @@ func RunEngineAPITests(t *testing.T, createBackend func() engineapi.EngineBacken
 		api.assert.Equal(block.BlockHash, api.headHash(), "should create and import new block")
 		imported := api.backend.GetBlockByHash(block.BlockHash)
 		api.assert.Len(imported.Transactions(), 1, "should include transaction")
+
+		api.assert.NotEqual(genesis.Root, block.StateRoot)
+		newState, err := api.backend.StateAt(common.Hash(block.StateRoot))
+		require.NoError(t, err, "imported block state should be available")
+		require.NotNil(t, newState)
+	})
+
+	t.Run("RejectCreatingBlockWithInvalidRequiredTransaction", func(t *testing.T) {
+		api := newTestHelper(t, createBackend)
+		genesis := api.backend.CurrentBlock()
+
+		txData, err := derive.L1InfoDeposit(1, eth.HeaderBlockInfo(genesis), eth.SystemConfig{}, true)
+		api.assert.NoError(err)
+		txData.Gas = uint64(gasLimit + 1)
+		tx := types.NewTx(txData)
+		txRlp, err := tx.MarshalBinary()
+		api.assert.NoError(err)
+
+		result, err := api.engine.ForkchoiceUpdatedV1(api.ctx, &eth.ForkchoiceState{
+			HeadBlockHash:      genesis.Hash(),
+			SafeBlockHash:      genesis.Hash(),
+			FinalizedBlockHash: genesis.Hash(),
+		}, &eth.PayloadAttributes{
+			Timestamp:             eth.Uint64Quantity(genesis.Time + 1),
+			PrevRandao:            eth.Bytes32(genesis.MixDigest),
+			SuggestedFeeRecipient: feeRecipient,
+			Transactions:          []eth.Data{txRlp},
+			NoTxPool:              true,
+			GasLimit:              &gasLimit,
+		})
+		api.assert.Error(err)
+		api.assert.Equal(eth.ExecutionInvalid, result.PayloadStatus.Status)
 	})
 
 	t.Run("IgnoreUpdateHeadToOlderBlock", func(t *testing.T) {
@@ -98,9 +131,14 @@ func RunEngineAPITests(t *testing.T, createBackend func() engineapi.EngineBacken
 		api := newTestHelper(t, createBackend)
 		genesis := api.backend.CurrentBlock()
 
-		payloadID := api.startBlockBuilding(genesis, eth.Uint64Quantity(genesis.Time))
+		// Start with a valid time
+		payloadID := api.startBlockBuilding(genesis, eth.Uint64Quantity(genesis.Time+1))
 		newBlock := api.getPayload(payloadID)
 
+		// Then make it invalid to check NewPayload rejects it
+		newBlock.Timestamp = eth.Uint64Quantity(genesis.Time)
+		updateBlockHash(newBlock)
+
 		r, err := api.engine.NewPayloadV1(api.ctx, newBlock)
 		api.assert.NoError(err)
 		api.assert.Equal(eth.ExecutionInvalid, r.Status)
@@ -110,14 +148,81 @@ func RunEngineAPITests(t *testing.T, createBackend func() engineapi.EngineBacken
 		api := newTestHelper(t, createBackend)
 		genesis := api.backend.CurrentBlock()
 
-		payloadID := api.startBlockBuilding(genesis, eth.Uint64Quantity(genesis.Time-1))
+		// Start with a valid time
+		payloadID := api.startBlockBuilding(genesis, eth.Uint64Quantity(genesis.Time+1))
 		newBlock := api.getPayload(payloadID)
 
+		// Then make it invalid to check NewPayload rejects it
+		newBlock.Timestamp = eth.Uint64Quantity(genesis.Time - 1)
+		updateBlockHash(newBlock)
+
 		r, err := api.engine.NewPayloadV1(api.ctx, newBlock)
 		api.assert.NoError(err)
 		api.assert.Equal(eth.ExecutionInvalid, r.Status)
 	})
 
+	t.Run("RejectCreateBlockWithSameTimeAsParent", func(t *testing.T) {
+		api := newTestHelper(t, createBackend)
+		genesis := api.backend.CurrentBlock()
+
+		result, err := api.engine.ForkchoiceUpdatedV1(api.ctx, &eth.ForkchoiceState{
+			HeadBlockHash:      genesis.Hash(),
+			SafeBlockHash:      genesis.Hash(),
+			FinalizedBlockHash: genesis.Hash(),
+		}, &eth.PayloadAttributes{
+			Timestamp:             eth.Uint64Quantity(genesis.Time),
+			PrevRandao:            eth.Bytes32(genesis.MixDigest),
+			SuggestedFeeRecipient: feeRecipient,
+			Transactions:          nil,
+			NoTxPool:              true,
+			GasLimit:              &gasLimit,
+		})
+		api.assert.Error(err)
+		api.assert.Equal(eth.ExecutionInvalid, result.PayloadStatus.Status)
+	})
+
+	t.Run("RejectCreateBlockWithTimeBeforeParent", func(t *testing.T) {
+		api := newTestHelper(t, createBackend)
+		genesis := api.backend.CurrentBlock()
+
+		result, err := api.engine.ForkchoiceUpdatedV1(api.ctx, &eth.ForkchoiceState{
+			HeadBlockHash:      genesis.Hash(),
+			SafeBlockHash:      genesis.Hash(),
+			FinalizedBlockHash: genesis.Hash(),
+		}, &eth.PayloadAttributes{
+			Timestamp:             eth.Uint64Quantity(genesis.Time - 1),
+			PrevRandao:            eth.Bytes32(genesis.MixDigest),
+			SuggestedFeeRecipient: feeRecipient,
+			Transactions:          nil,
+			NoTxPool:              true,
+			GasLimit:              &gasLimit,
+		})
+		api.assert.Error(err)
+		api.assert.Equal(eth.ExecutionInvalid, result.PayloadStatus.Status)
+	})
+
+	t.Run("RejectCreateBlockWithGasLimitAboveMax", func(t *testing.T) {
+		api := newTestHelper(t, createBackend)
+		genesis := api.backend.CurrentBlock()
+
+		gasLimit := eth.Uint64Quantity(params.MaxGasLimit + 1)
+
+		result, err := api.engine.ForkchoiceUpdatedV1(api.ctx, &eth.ForkchoiceState{
+			HeadBlockHash:      genesis.Hash(),
+			SafeBlockHash:      genesis.Hash(),
+			FinalizedBlockHash: genesis.Hash(),
+		}, &eth.PayloadAttributes{
+			Timestamp:             eth.Uint64Quantity(genesis.Time + 1),
+			PrevRandao:            eth.Bytes32(genesis.MixDigest),
+			SuggestedFeeRecipient: feeRecipient,
+			Transactions:          nil,
+			NoTxPool:              true,
+			GasLimit:              &gasLimit,
+		})
+		api.assert.Error(err)
+		api.assert.Equal(eth.ExecutionInvalid, result.PayloadStatus.Status)
+	})
+
 	t.Run("UpdateSafeAndFinalizedHead", func(t *testing.T) {
 		api := newTestHelper(t, createBackend)