op-node: Properly set timestamp in PreparePayloadAttributes (#3151)

This now uses a passed in timestamp for PreparePayloadAttributes.
This is important because when generating a payload attributes from
a batch it should be using the timestamp of the batch, not the
L2 parent + block time. When we are creating payloads based on the
L2 parent time, that is easy to set there instead of pulling it into
this function.

This enabled/exacerbated a divergence bug where the timestamp of the
batch was not the timestamp that was being set inside
PreparePayloadAttributes.
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

op-node/rollup/derive/attributes.go

@@ -22,7 +22,7 @@ type L1ReceiptsFetcher interface {
 // 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, l2Parent eth.L2BlockRef, epoch eth.BlockID) (attrs *eth.PayloadAttributes, crit bool, err error) {
+func PreparePayloadAttributes(ctx context.Context, cfg *rollup.Config, dl L1ReceiptsFetcher, l2Parent eth.L2BlockRef, timestamp uint64, epoch eth.BlockID) (attrs *eth.PayloadAttributes, crit bool, err error) {
 	var l1Info eth.L1Info
 	var depositTxs []hexutil.Bytes
 	var seqNumber uint64
@@ -68,7 +68,7 @@ func PreparePayloadAttributes(ctx context.Context, cfg *rollup.Config, dl L1Rece
 	txs = append(txs, depositTxs...)
 
 	return &eth.PayloadAttributes{
-		Timestamp:             hexutil.Uint64(l2Parent.Time + cfg.BlockTime),
+		Timestamp:             hexutil.Uint64(timestamp),
 		PrevRandao:            eth.Bytes32(l1Info.MixDigest()),
 		SuggestedFeeRecipient: cfg.FeeRecipientAddress,
 		Transactions:          txs,

op-node/rollup/derive/attributes_queue.go

@@ -55,7 +55,7 @@ func (aq *AttributesQueue) Step(ctx context.Context, outer Progress) error {
 
 	fetchCtx, cancel := context.WithTimeout(ctx, 20*time.Second)
 	defer cancel()
-	attrs, crit, err := PreparePayloadAttributes(fetchCtx, aq.config, aq.dl, aq.next.SafeL2Head(), batch.Epoch())
+	attrs, crit, err := PreparePayloadAttributes(fetchCtx, aq.config, aq.dl, aq.next.SafeL2Head(), batch.Timestamp, batch.Epoch())
 	if err != nil {
 		if crit {
 			return fmt.Errorf("failed to prepare payload attributes for batch: %v", err)

op-node/rollup/derive/attributes_queue_test.go

@@ -70,7 +70,7 @@ func TestAttributesQueue_Step(t *testing.T) {
 	batch := &BatchData{BatchV1{
 		EpochNum:     rollup.Epoch(l1Info.InfoNum),
 		EpochHash:    l1Info.InfoHash,
-		Timestamp:    12345,
+		Timestamp:    safeHead.Time + cfg.BlockTime,
 		Transactions: []eth.Data{eth.Data("foobar"), eth.Data("example")},
 	}}
 

op-node/rollup/derive/attributes_test.go

@@ -31,11 +31,12 @@ func TestPreparePayloadAttributes(t *testing.T) {
 		l1Fetcher := &testutils.MockL1Source{}
 		defer l1Fetcher.AssertExpectations(t)
 		l2Parent := testutils.RandomL2BlockRef(rng)
+		l2Time := l2Parent.Time + cfg.BlockTime
 		l1Info := testutils.RandomL1Info(rng)
 		l1Info.InfoNum = l2Parent.L1Origin.Number + 1
 		epoch := l1Info.ID()
 		l1Fetcher.ExpectFetch(epoch.Hash, l1Info, nil, nil, nil)
-		_, crit, err := PreparePayloadAttributes(context.Background(), cfg, l1Fetcher, l2Parent, epoch)
+		_, crit, err := PreparePayloadAttributes(context.Background(), cfg, l1Fetcher, l2Parent, l2Time, epoch)
 		require.NotNil(t, err, "inconsistent L1 origin error expected")
 		require.True(t, crit, "inconsistent L1 origin transition must be handled like a critical error with reorg")
 	})
@@ -44,10 +45,11 @@ func TestPreparePayloadAttributes(t *testing.T) {
 		l1Fetcher := &testutils.MockL1Source{}
 		defer l1Fetcher.AssertExpectations(t)
 		l2Parent := testutils.RandomL2BlockRef(rng)
+		l2Time := l2Parent.Time + cfg.BlockTime
 		l1Info := testutils.RandomL1Info(rng)
 		l1Info.InfoNum = l2Parent.L1Origin.Number
 		epoch := l1Info.ID()
-		_, crit, err := PreparePayloadAttributes(context.Background(), cfg, l1Fetcher, l2Parent, epoch)
+		_, crit, err := PreparePayloadAttributes(context.Background(), cfg, l1Fetcher, l2Parent, l2Time, epoch)
 		require.NotNil(t, err, "inconsistent L1 origin error expected")
 		require.True(t, crit, "inconsistent L1 origin transition must be handled like a critical error with reorg")
 	})
@@ -56,11 +58,12 @@ func TestPreparePayloadAttributes(t *testing.T) {
 		l1Fetcher := &testutils.MockL1Source{}
 		defer l1Fetcher.AssertExpectations(t)
 		l2Parent := testutils.RandomL2BlockRef(rng)
+		l2Time := l2Parent.Time + cfg.BlockTime
 		epoch := l2Parent.L1Origin
 		epoch.Number += 1
 		mockRPCErr := errors.New("mock rpc error")
 		l1Fetcher.ExpectFetch(epoch.Hash, nil, nil, nil, mockRPCErr)
-		_, crit, err := PreparePayloadAttributes(context.Background(), cfg, l1Fetcher, l2Parent, epoch)
+		_, crit, err := PreparePayloadAttributes(context.Background(), cfg, l1Fetcher, l2Parent, l2Time, epoch)
 		require.ErrorIs(t, err, mockRPCErr, "mock rpc error expected")
 		require.False(t, crit, "rpc errors should not be critical, it is not necessary to reorg")
 	})
@@ -69,10 +72,11 @@ func TestPreparePayloadAttributes(t *testing.T) {
 		l1Fetcher := &testutils.MockL1Source{}
 		defer l1Fetcher.AssertExpectations(t)
 		l2Parent := testutils.RandomL2BlockRef(rng)
+		l2Time := l2Parent.Time + cfg.BlockTime
 		epoch := l2Parent.L1Origin
 		mockRPCErr := errors.New("mock rpc error")
 		l1Fetcher.ExpectInfoByHash(epoch.Hash, nil, mockRPCErr)
-		_, crit, err := PreparePayloadAttributes(context.Background(), cfg, l1Fetcher, l2Parent, epoch)
+		_, crit, err := PreparePayloadAttributes(context.Background(), cfg, l1Fetcher, l2Parent, l2Time, epoch)
 		require.ErrorIs(t, err, mockRPCErr, "mock rpc error expected")
 		require.False(t, crit, "rpc errors should not be critical, it is not necessary to reorg")
 	})
@@ -81,6 +85,7 @@ func TestPreparePayloadAttributes(t *testing.T) {
 		l1Fetcher := &testutils.MockL1Source{}
 		defer l1Fetcher.AssertExpectations(t)
 		l2Parent := testutils.RandomL2BlockRef(rng)
+		l2Time := l2Parent.Time + cfg.BlockTime
 		l1Info := testutils.RandomL1Info(rng)
 		l1Info.InfoParentHash = l2Parent.L1Origin.Hash
 		l1Info.InfoNum = l2Parent.L1Origin.Number + 1
@@ -88,7 +93,7 @@ func TestPreparePayloadAttributes(t *testing.T) {
 		l1InfoTx, err := L1InfoDepositBytes(0, l1Info)
 		require.NoError(t, err)
 		l1Fetcher.ExpectFetch(epoch.Hash, l1Info, nil, nil, nil)
-		attrs, crit, err := PreparePayloadAttributes(context.Background(), cfg, l1Fetcher, l2Parent, epoch)
+		attrs, crit, err := PreparePayloadAttributes(context.Background(), cfg, l1Fetcher, l2Parent, l2Time, epoch)
 		require.NoError(t, err)
 		require.False(t, crit)
 		require.NotNil(t, attrs)
@@ -104,6 +109,7 @@ func TestPreparePayloadAttributes(t *testing.T) {
 		l1Fetcher := &testutils.MockL1Source{}
 		defer l1Fetcher.AssertExpectations(t)
 		l2Parent := testutils.RandomL2BlockRef(rng)
+		l2Time := l2Parent.Time + cfg.BlockTime
 		l1Info := testutils.RandomL1Info(rng)
 		l1Info.InfoParentHash = l2Parent.L1Origin.Hash
 		l1Info.InfoNum = l2Parent.L1Origin.Number + 1
@@ -126,7 +132,7 @@ func TestPreparePayloadAttributes(t *testing.T) {
 		// txs are ignored, API is a bit bloated to previous approach. Only l1Info and receipts matter.
 		l1Txs := make(types.Transactions, len(receipts))
 		l1Fetcher.ExpectFetch(epoch.Hash, l1Info, l1Txs, receipts, nil)
-		attrs, crit, err := PreparePayloadAttributes(context.Background(), cfg, l1Fetcher, l2Parent, epoch)
+		attrs, crit, err := PreparePayloadAttributes(context.Background(), cfg, l1Fetcher, l2Parent, l2Time, epoch)
 		require.NoError(t, err)
 		require.False(t, crit)
 		require.NotNil(t, attrs)
@@ -142,6 +148,7 @@ func TestPreparePayloadAttributes(t *testing.T) {
 		l1Fetcher := &testutils.MockL1Source{}
 		defer l1Fetcher.AssertExpectations(t)
 		l2Parent := testutils.RandomL2BlockRef(rng)
+		l2Time := l2Parent.Time + cfg.BlockTime
 		l1Info := testutils.RandomL1Info(rng)
 		l1Info.InfoHash = l2Parent.L1Origin.Hash
 		l1Info.InfoNum = l2Parent.L1Origin.Number
@@ -151,7 +158,7 @@ func TestPreparePayloadAttributes(t *testing.T) {
 		require.NoError(t, err)
 
 		l1Fetcher.ExpectInfoByHash(epoch.Hash, l1Info, nil)
-		attrs, crit, err := PreparePayloadAttributes(context.Background(), cfg, l1Fetcher, l2Parent, epoch)
+		attrs, crit, err := PreparePayloadAttributes(context.Background(), cfg, l1Fetcher, l2Parent, l2Time, epoch)
 		require.NoError(t, err)
 		require.False(t, crit)
 		require.NotNil(t, attrs)

op-node/rollup/driver/step.go

@@ -25,7 +25,7 @@ func (d *outputImpl) createNewBlock(ctx context.Context, l2Head eth.L2BlockRef,
 	fetchCtx, cancel := context.WithTimeout(ctx, time.Second*20)
 	defer cancel()
 
-	attrs, _, err := derive.PreparePayloadAttributes(fetchCtx, d.Config, d.dl, l2Head, l1Origin.ID())
+	attrs, _, err := derive.PreparePayloadAttributes(fetchCtx, d.Config, d.dl, l2Head, l2Head.Time+d.Config.BlockTime, l1Origin.ID())
 	if err != nil {
 		return l2Head, nil, err
 	}