Skip to content

op-node: Properly set timestamp in PreparePayloadAttributes #3151

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mergify merged 2 commits into from
Aug 1, 2022
Merged

op-node: Properly set timestamp in PreparePayloadAttributes #3151

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bb35cfe
op-node: Properly set timestamp in PreparePayloadAttributes
trianglesphere Aug 1, 2022
0721118
Merge branch 'develop' into jg/fix_attributes_timestamp_selection
mergify[bot] Aug 1, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions op-node/rollup/derive/attributes.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion op-node/rollup/derive/attributes_queue.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion op-node/rollup/derive/attributes_queue_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")},
}}

Expand Down
21 changes: 14 additions & 7 deletions op-node/rollup/derive/attributes_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")
})
Expand All @@ -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")
})
Expand All @@ -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")
})
Expand All @@ -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")
})
Expand All @@ -81,14 +85,15 @@ 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
epoch := l1Info.ID()
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)
Expand All @@ -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
Expand All @@ -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)
Expand All @@ -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
Expand All @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion op-node/rollup/driver/step.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down