Merge pull request #4737 from ethereum-optimism/attributes-builder

op-node: attributes builder interface

op-e2e/actions/l2_sequencer.go

@@ -26,9 +26,10 @@ type L2Sequencer struct {
 
 func NewL2Sequencer(t Testing, log log.Logger, l1 derive.L1Fetcher, eng L2API, cfg *rollup.Config, seqConfDepth uint64) *L2Sequencer {
 	ver := NewL2Verifier(t, log, l1, eng, cfg)
+	attrBuilder := derive.NewFetchingAttributesBuilder(cfg, l1, eng)
 	return &L2Sequencer{
 		L2Verifier:              *ver,
-		sequencer:               driver.NewSequencer(log, cfg, l1, eng, ver.derivation, metrics.NoopMetrics),
+		sequencer:               driver.NewSequencer(log, cfg, eng, ver.derivation, attrBuilder, metrics.NoopMetrics),
 		l1OriginSelector:        driver.NewL1OriginSelector(log, cfg, l1, seqConfDepth),
 		seqOldOrigin:            false,
 		failL2GossipUnsafeBlock: nil,

op-node/rollup/derive/attributes.go

@@ -23,17 +23,32 @@ type SystemConfigL2Fetcher interface {
 	SystemConfigByL2Hash(ctx context.Context, hash common.Hash) (eth.SystemConfig, error)
 }
 
+// FetchingAttributesBuilder fetches inputs for the building of L2 payload attributes on the fly.
+type FetchingAttributesBuilder struct {
+	cfg *rollup.Config
+	l1  L1ReceiptsFetcher
+	l2  SystemConfigL2Fetcher
+}
+
+func NewFetchingAttributesBuilder(cfg *rollup.Config, l1 L1ReceiptsFetcher, l2 SystemConfigL2Fetcher) *FetchingAttributesBuilder {
+	return &FetchingAttributesBuilder{
+		cfg: cfg,
+		l1:  l1,
+		l2:  l2,
+	}
+}
+
 // PreparePayloadAttributes prepares a PayloadAttributes template that is ready to build a L2 block with deposits only, on top of the given l2Parent, with the given epoch as L1 origin.
 // The template defaults to NoTxPool=true, and no sequencer transactions: the caller has to modify the template to add transactions,
 // by setting NoTxPool=false as sequencer, or by appending batch transactions as verifier.
 // The severity of the error is returned; a crit=false error means there was a temporary issue, like a failed RPC or time-out.
 // A crit=true error means the input arguments are inconsistent or invalid.
-func PreparePayloadAttributes(ctx context.Context, cfg *rollup.Config, dl L1ReceiptsFetcher, l2 SystemConfigL2Fetcher, l2Parent eth.L2BlockRef, timestamp uint64, epoch eth.BlockID) (attrs *eth.PayloadAttributes, err error) {
+func (ba *FetchingAttributesBuilder) PreparePayloadAttributes(ctx context.Context, l2Parent eth.L2BlockRef, epoch eth.BlockID) (attrs *eth.PayloadAttributes, err error) {
 	var l1Info eth.BlockInfo
 	var depositTxs []hexutil.Bytes
 	var seqNumber uint64
 
-	sysConfig, err := l2.SystemConfigByL2Hash(ctx, l2Parent.Hash)
+	sysConfig, err := ba.l2.SystemConfigByL2Hash(ctx, l2Parent.Hash)
 	if err != nil {
 		return nil, NewTemporaryError(fmt.Errorf("failed to retrieve L2 parent block: %w", err))
 	}
@@ -42,7 +57,7 @@ func PreparePayloadAttributes(ctx context.Context, cfg *rollup.Config, dl L1Rece
 	// case we need to fetch all transaction receipts from the L1 origin block so we can scan for
 	// user deposits.
 	if l2Parent.L1Origin.Number != epoch.Number {
-		info, receipts, err := dl.FetchReceipts(ctx, epoch.Hash)
+		info, receipts, err := ba.l1.FetchReceipts(ctx, epoch.Hash)
 		if err != nil {
 			return nil, NewTemporaryError(fmt.Errorf("failed to fetch L1 block info and receipts: %w", err))
 		}
@@ -52,13 +67,13 @@ func PreparePayloadAttributes(ctx context.Context, cfg *rollup.Config, dl L1Rece
 					epoch, info.ParentHash(), l2Parent.L1Origin))
 		}
 
-		deposits, err := DeriveDeposits(receipts, cfg.DepositContractAddress)
+		deposits, err := DeriveDeposits(receipts, ba.cfg.DepositContractAddress)
 		if err != nil {
 			// deposits may never be ignored. Failing to process them is a critical error.
 			return nil, NewCriticalError(fmt.Errorf("failed to derive some deposits: %w", err))
 		}
 		// apply sysCfg changes
-		if err := UpdateSystemConfigWithL1Receipts(&sysConfig, receipts, cfg); err != nil {
+		if err := UpdateSystemConfigWithL1Receipts(&sysConfig, receipts, ba.cfg); err != nil {
 			return nil, NewCriticalError(fmt.Errorf("failed to apply derived L1 sysCfg updates: %w", err))
 		}
 
@@ -69,7 +84,7 @@ func PreparePayloadAttributes(ctx context.Context, cfg *rollup.Config, dl L1Rece
 		if l2Parent.L1Origin.Hash != epoch.Hash {
 			return nil, NewResetError(fmt.Errorf("cannot create new block with L1 origin %s in conflict with L1 origin %s", epoch, l2Parent.L1Origin))
 		}
-		info, err := dl.InfoByHash(ctx, epoch.Hash)
+		info, err := ba.l1.InfoByHash(ctx, epoch.Hash)
 		if err != nil {
 			return nil, NewTemporaryError(fmt.Errorf("failed to fetch L1 block info: %w", err))
 		}
@@ -78,6 +93,13 @@ func PreparePayloadAttributes(ctx context.Context, cfg *rollup.Config, dl L1Rece
 		seqNumber = l2Parent.SequenceNumber + 1
 	}
 
+	// Sanity check the L1 origin was correctly selected to maintain the time invariant between L1 and L2
+	nextL2Time := l2Parent.Time + ba.cfg.BlockTime
+	if nextL2Time < l1Info.Time() {
+		return nil, NewResetError(fmt.Errorf("cannot build L2 block on top %s for time %d before L1 origin %s at time %d",
+			l2Parent, nextL2Time, eth.ToBlockID(l1Info), l1Info.Time()))
+	}
+
 	l1InfoTx, err := L1InfoDepositBytes(seqNumber, l1Info, sysConfig)
 	if err != nil {
 		return nil, NewCriticalError(fmt.Errorf("failed to create l1InfoTx: %w", err))
@@ -88,7 +110,7 @@ func PreparePayloadAttributes(ctx context.Context, cfg *rollup.Config, dl L1Rece
 	txs = append(txs, depositTxs...)
 
 	return &eth.PayloadAttributes{
-		Timestamp:             hexutil.Uint64(timestamp),
+		Timestamp:             hexutil.Uint64(nextL2Time),
 		PrevRandao:            eth.Bytes32(l1Info.MixDigest()),
 		SuggestedFeeRecipient: predeploys.SequencerFeeVaultAddr,
 		Transactions:          txs,

op-node/rollup/derive/attributes_queue.go

@@ -23,21 +23,23 @@ import (
 // This stage can be reset by clearing it's batch buffer.
 // This stage does not need to retain any references to L1 blocks.
 
+type AttributesBuilder interface {
+	PreparePayloadAttributes(ctx context.Context, l2Parent eth.L2BlockRef, epoch eth.BlockID) (attrs *eth.PayloadAttributes, err error)
+}
+
 type AttributesQueue struct {
 	log     log.Logger
 	config  *rollup.Config
-	dl     L1ReceiptsFetcher
-	eng    SystemConfigL2Fetcher
+	builder AttributesBuilder
 	prev    *BatchQueue
 	batch   *BatchData
 }
 
-func NewAttributesQueue(log log.Logger, cfg *rollup.Config, l1Fetcher L1ReceiptsFetcher, eng SystemConfigL2Fetcher, prev *BatchQueue) *AttributesQueue {
+func NewAttributesQueue(log log.Logger, cfg *rollup.Config, builder AttributesBuilder, prev *BatchQueue) *AttributesQueue {
 	return &AttributesQueue{
 		log:     log,
 		config:  cfg,
-		dl:     l1Fetcher,
-		eng:    eng,
+		builder: builder,
 		prev:    prev,
 	}
 }
@@ -74,9 +76,13 @@ func (aq *AttributesQueue) createNextAttributes(ctx context.Context, batch *Batc
 	if batch.ParentHash != l2SafeHead.Hash {
 		return nil, NewResetError(fmt.Errorf("valid batch has bad parent hash %s, expected %s", batch.ParentHash, l2SafeHead.Hash))
 	}
+	// sanity check timestamp
+	if expected := l2SafeHead.Time + aq.config.BlockTime; expected != batch.Timestamp {
+		return nil, NewResetError(fmt.Errorf("valid batch has bad timestamp %d, expected %d", batch.Timestamp, expected))
+	}
 	fetchCtx, cancel := context.WithTimeout(ctx, 20*time.Second)
 	defer cancel()
-	attrs, err := PreparePayloadAttributes(fetchCtx, aq.config, aq.dl, aq.eng, l2SafeHead, batch.Timestamp, batch.Epoch())
+	attrs, err := aq.builder.PreparePayloadAttributes(fetchCtx, l2SafeHead, batch.Epoch())
 	if err != nil {
 		return nil, err
 	}

op-node/rollup/derive/attributes_queue_test.go

@@ -40,6 +40,7 @@ func TestAttributesQueue(t *testing.T) {
 
 	safeHead := testutils.RandomL2BlockRef(rng)
 	safeHead.L1Origin = l1Info.ID()
+	safeHead.Time = l1Info.InfoTime
 
 	batch := &BatchData{BatchV1{
 		ParentHash:   safeHead.Hash,
@@ -75,11 +76,12 @@ func TestAttributesQueue(t *testing.T) {
 		NoTxPool:              true,
 		GasLimit:              (*eth.Uint64Quantity)(&expectedL1Cfg.GasLimit),
 	}
+	attrBuilder := NewFetchingAttributesBuilder(cfg, l1Fetcher, l2Fetcher)
 
-	aq := NewAttributesQueue(testlog.Logger(t, log.LvlError), cfg, l1Fetcher, l2Fetcher, nil)
+	aq := NewAttributesQueue(testlog.Logger(t, log.LvlError), cfg, attrBuilder, nil)
 
 	actual, err := aq.createNextAttributes(context.Background(), batch, safeHead)
 
-	require.Nil(t, err)
+	require.NoError(t, err)
 	require.Equal(t, attrs, *actual)
 }

op-node/rollup/derive/attributes_test.go

@@ -43,12 +43,12 @@ func TestPreparePayloadAttributes(t *testing.T) {
 		l1CfgFetcher := &testutils.MockL2Client{}
 		l1CfgFetcher.ExpectSystemConfigByL2Hash(l2Parent.Hash, testSysCfg, nil)
 		defer l1CfgFetcher.AssertExpectations(t)
-		l2Time := l2Parent.Time + cfg.BlockTime
 		l1Info := testutils.RandomBlockInfo(rng)
 		l1Info.InfoNum = l2Parent.L1Origin.Number + 1
 		epoch := l1Info.ID()
 		l1Fetcher.ExpectFetchReceipts(epoch.Hash, l1Info, nil, nil)
-		_, err := PreparePayloadAttributes(context.Background(), cfg, l1Fetcher, l1CfgFetcher, l2Parent, l2Time, epoch)
+		attrBuilder := NewFetchingAttributesBuilder(cfg, l1Fetcher, l1CfgFetcher)
+		_, err := attrBuilder.PreparePayloadAttributes(context.Background(), l2Parent, epoch)
 		require.NotNil(t, err, "inconsistent L1 origin error expected")
 		require.ErrorIs(t, err, ErrReset, "inconsistent L1 origin transition must be handled like a critical error with reorg")
 	})
@@ -60,11 +60,11 @@ func TestPreparePayloadAttributes(t *testing.T) {
 		l1CfgFetcher := &testutils.MockL2Client{}
 		l1CfgFetcher.ExpectSystemConfigByL2Hash(l2Parent.Hash, testSysCfg, nil)
 		defer l1CfgFetcher.AssertExpectations(t)
-		l2Time := l2Parent.Time + cfg.BlockTime
 		l1Info := testutils.RandomBlockInfo(rng)
 		l1Info.InfoNum = l2Parent.L1Origin.Number
 		epoch := l1Info.ID()
-		_, err := PreparePayloadAttributes(context.Background(), cfg, l1Fetcher, l1CfgFetcher, l2Parent, l2Time, epoch)
+		attrBuilder := NewFetchingAttributesBuilder(cfg, l1Fetcher, l1CfgFetcher)
+		_, err := attrBuilder.PreparePayloadAttributes(context.Background(), l2Parent, epoch)
 		require.NotNil(t, err, "inconsistent L1 origin error expected")
 		require.ErrorIs(t, err, ErrReset, "inconsistent L1 origin transition must be handled like a critical error with reorg")
 	})
@@ -76,12 +76,12 @@ func TestPreparePayloadAttributes(t *testing.T) {
 		l1CfgFetcher := &testutils.MockL2Client{}
 		l1CfgFetcher.ExpectSystemConfigByL2Hash(l2Parent.Hash, testSysCfg, nil)
 		defer l1CfgFetcher.AssertExpectations(t)
-		l2Time := l2Parent.Time + cfg.BlockTime
 		epoch := l2Parent.L1Origin
 		epoch.Number += 1
 		mockRPCErr := errors.New("mock rpc error")
 		l1Fetcher.ExpectFetchReceipts(epoch.Hash, nil, nil, mockRPCErr)
-		_, err := PreparePayloadAttributes(context.Background(), cfg, l1Fetcher, l1CfgFetcher, l2Parent, l2Time, epoch)
+		attrBuilder := NewFetchingAttributesBuilder(cfg, l1Fetcher, l1CfgFetcher)
+		_, err := attrBuilder.PreparePayloadAttributes(context.Background(), l2Parent, epoch)
 		require.ErrorIs(t, err, mockRPCErr, "mock rpc error expected")
 		require.ErrorIs(t, err, ErrTemporary, "rpc errors should not be critical, it is not necessary to reorg")
 	})
@@ -93,11 +93,11 @@ func TestPreparePayloadAttributes(t *testing.T) {
 		l1CfgFetcher := &testutils.MockL2Client{}
 		l1CfgFetcher.ExpectSystemConfigByL2Hash(l2Parent.Hash, testSysCfg, nil)
 		defer l1CfgFetcher.AssertExpectations(t)
-		l2Time := l2Parent.Time + cfg.BlockTime
 		epoch := l2Parent.L1Origin
 		mockRPCErr := errors.New("mock rpc error")
 		l1Fetcher.ExpectInfoByHash(epoch.Hash, nil, mockRPCErr)
-		_, err := PreparePayloadAttributes(context.Background(), cfg, l1Fetcher, l1CfgFetcher, l2Parent, l2Time, epoch)
+		attrBuilder := NewFetchingAttributesBuilder(cfg, l1Fetcher, l1CfgFetcher)
+		_, err := attrBuilder.PreparePayloadAttributes(context.Background(), l2Parent, epoch)
 		require.ErrorIs(t, err, mockRPCErr, "mock rpc error expected")
 		require.ErrorIs(t, err, ErrTemporary, "rpc errors should not be critical, it is not necessary to reorg")
 	})
@@ -109,7 +109,6 @@ func TestPreparePayloadAttributes(t *testing.T) {
 		l1CfgFetcher := &testutils.MockL2Client{}
 		l1CfgFetcher.ExpectSystemConfigByL2Hash(l2Parent.Hash, testSysCfg, nil)
 		defer l1CfgFetcher.AssertExpectations(t)
-		l2Time := l2Parent.Time + cfg.BlockTime
 		l1Info := testutils.RandomBlockInfo(rng)
 		l1Info.InfoParentHash = l2Parent.L1Origin.Hash
 		l1Info.InfoNum = l2Parent.L1Origin.Number + 1
@@ -117,7 +116,8 @@ func TestPreparePayloadAttributes(t *testing.T) {
 		l1InfoTx, err := L1InfoDepositBytes(0, l1Info, testSysCfg)
 		require.NoError(t, err)
 		l1Fetcher.ExpectFetchReceipts(epoch.Hash, l1Info, nil, nil)
-		attrs, err := PreparePayloadAttributes(context.Background(), cfg, l1Fetcher, l1CfgFetcher, l2Parent, l2Time, epoch)
+		attrBuilder := NewFetchingAttributesBuilder(cfg, l1Fetcher, l1CfgFetcher)
+		attrs, err := attrBuilder.PreparePayloadAttributes(context.Background(), l2Parent, epoch)
 		require.NoError(t, err)
 		require.NotNil(t, attrs)
 		require.Equal(t, l2Parent.Time+cfg.BlockTime, uint64(attrs.Timestamp))
@@ -135,7 +135,6 @@ func TestPreparePayloadAttributes(t *testing.T) {
 		l1CfgFetcher := &testutils.MockL2Client{}
 		l1CfgFetcher.ExpectSystemConfigByL2Hash(l2Parent.Hash, testSysCfg, nil)
 		defer l1CfgFetcher.AssertExpectations(t)
-		l2Time := l2Parent.Time + cfg.BlockTime
 		l1Info := testutils.RandomBlockInfo(rng)
 		l1Info.InfoParentHash = l2Parent.L1Origin.Hash
 		l1Info.InfoNum = l2Parent.L1Origin.Number + 1
@@ -157,7 +156,8 @@ func TestPreparePayloadAttributes(t *testing.T) {
 		l2Txs := append(append(make([]eth.Data, 0), l1InfoTx), usedDepositTxs...)
 
 		l1Fetcher.ExpectFetchReceipts(epoch.Hash, l1Info, receipts, nil)
-		attrs, err := PreparePayloadAttributes(context.Background(), cfg, l1Fetcher, l1CfgFetcher, l2Parent, l2Time, epoch)
+		attrBuilder := NewFetchingAttributesBuilder(cfg, l1Fetcher, l1CfgFetcher)
+		attrs, err := attrBuilder.PreparePayloadAttributes(context.Background(), l2Parent, epoch)
 		require.NoError(t, err)
 		require.NotNil(t, attrs)
 		require.Equal(t, l2Parent.Time+cfg.BlockTime, uint64(attrs.Timestamp))
@@ -175,7 +175,6 @@ func TestPreparePayloadAttributes(t *testing.T) {
 		l1CfgFetcher := &testutils.MockL2Client{}
 		l1CfgFetcher.ExpectSystemConfigByL2Hash(l2Parent.Hash, testSysCfg, nil)
 		defer l1CfgFetcher.AssertExpectations(t)
-		l2Time := l2Parent.Time + cfg.BlockTime
 		l1Info := testutils.RandomBlockInfo(rng)
 		l1Info.InfoHash = l2Parent.L1Origin.Hash
 		l1Info.InfoNum = l2Parent.L1Origin.Number
@@ -185,7 +184,8 @@ func TestPreparePayloadAttributes(t *testing.T) {
 		require.NoError(t, err)
 
 		l1Fetcher.ExpectInfoByHash(epoch.Hash, l1Info, nil)
-		attrs, err := PreparePayloadAttributes(context.Background(), cfg, l1Fetcher, l1CfgFetcher, l2Parent, l2Time, epoch)
+		attrBuilder := NewFetchingAttributesBuilder(cfg, l1Fetcher, l1CfgFetcher)
+		attrs, err := attrBuilder.PreparePayloadAttributes(context.Background(), l2Parent, epoch)
 		require.NoError(t, err)
 		require.NotNil(t, attrs)
 		require.Equal(t, l2Parent.Time+cfg.BlockTime, uint64(attrs.Timestamp))

op-node/rollup/derive/pipeline.go

@@ -74,7 +74,8 @@ func NewDerivationPipeline(log log.Logger, cfg *rollup.Config, l1Fetcher L1Fetch
 	bank := NewChannelBank(log, cfg, frameQueue, l1Fetcher)
 	chInReader := NewChannelInReader(log, bank)
 	batchQueue := NewBatchQueue(log, cfg, chInReader)
-	attributesQueue := NewAttributesQueue(log, cfg, l1Fetcher, engine, batchQueue)
+	attrBuilder := NewFetchingAttributesBuilder(cfg, l1Fetcher, engine)
+	attributesQueue := NewAttributesQueue(log, cfg, attrBuilder, batchQueue)
 
 	// Step stages
 	eng := NewEngineQueue(log, cfg, engine, metrics, attributesQueue, l1Fetcher)

op-node/rollup/driver/driver.go

@@ -89,8 +89,8 @@ func NewDriver(driverCfg *Config, cfg *rollup.Config, l2 L2Chain, l1 L1Chain, ne
 	findL1Origin := NewL1OriginSelector(log, cfg, l1, driverCfg.SequencerConfDepth)
 	verifConfDepth := NewConfDepth(driverCfg.VerifierConfDepth, l1State.L1Head, l1)
 	derivationPipeline := derive.NewDerivationPipeline(log, cfg, verifConfDepth, l2, metrics)
-	sequencer := NewSequencer(log, cfg, l1, l2, derivationPipeline, metrics)
-
+	attrBuilder := derive.NewFetchingAttributesBuilder(cfg, l1, l2)
+	sequencer := NewSequencer(log, cfg, l2, derivationPipeline, attrBuilder, metrics)
 	return &Driver{
 		l1State:          l1State,
 		derivation:       derivationPipeline,

op-node/rollup/driver/sequencer.go

@@ -37,10 +37,10 @@ type Sequencer struct {
 	log    log.Logger
 	config *rollup.Config
 
-	l1          Downloader
 	l2          derive.Engine
 	engineState EngineState
 
+	attrBuilder       derive.AttributesBuilder
 	buildingOnto      eth.L2BlockRef
 	buildingID        eth.PayloadID
 	buildingStartTime time.Time
@@ -48,14 +48,14 @@ type Sequencer struct {
 	metrics SequencerMetrics
 }
 
-func NewSequencer(log log.Logger, cfg *rollup.Config, l1 Downloader, l2 derive.Engine, engineState EngineState, metrics SequencerMetrics) *Sequencer {
+func NewSequencer(log log.Logger, cfg *rollup.Config, l2 derive.Engine, engineState EngineState, attributesBuilder derive.AttributesBuilder, metrics SequencerMetrics) *Sequencer {
 	return &Sequencer{
 		log:         log,
 		config:      cfg,
-		l1:          l1,
 		l2:          l2,
 		metrics:     metrics,
 		engineState: engineState,
+		attrBuilder: attributesBuilder,
 	}
 }
 
@@ -75,7 +75,7 @@ func (d *Sequencer) StartBuildingBlock(ctx context.Context, l1Origin eth.L1Block
 	fetchCtx, cancel := context.WithTimeout(ctx, time.Second*20)
 	defer cancel()
 
-	attrs, err := derive.PreparePayloadAttributes(fetchCtx, d.config, d.l1, d.l2, l2Head, l2Head.Time+d.config.BlockTime, l1Origin.ID())
+	attrs, err := d.attrBuilder.PreparePayloadAttributes(fetchCtx, l2Head, l1Origin.ID())
 	if err != nil {
 		return err
 	}