diff --git a/op-node/rollup/derive/attributes.go b/op-node/rollup/derive/attributes.go index 0a5b8504544a3..8551fa02ed6f2 100644 --- a/op-node/rollup/derive/attributes.go +++ b/op-node/rollup/derive/attributes.go @@ -3,7 +3,6 @@ package derive import ( "context" "fmt" - "io" "github.com/ethereum-optimism/optimism/op-node/eth" "github.com/ethereum-optimism/optimism/op-node/rollup" @@ -15,7 +14,7 @@ import ( // L1ReceiptsFetcher fetches L1 header info and receipts for the payload attributes derivation (the info tx and deposits) type L1ReceiptsFetcher interface { InfoByHash(ctx context.Context, hash common.Hash) (eth.BlockInfo, error) - Fetch(ctx context.Context, blockHash common.Hash) (eth.BlockInfo, types.Transactions, eth.ReceiptsFetcher, error) + FetchReceipts(ctx context.Context, blockHash common.Hash) (eth.BlockInfo, types.Receipts, error) } // 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. @@ -32,7 +31,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, _, receiptsFetcher, err := dl.Fetch(ctx, epoch.Hash) + info, receipts, err := dl.FetchReceipts(ctx, epoch.Hash) if err != nil { return nil, NewTemporaryError(fmt.Errorf("failed to fetch L1 block info and receipts: %w", err)) } @@ -41,17 +40,7 @@ func PreparePayloadAttributes(ctx context.Context, cfg *rollup.Config, dl L1Rece fmt.Errorf("cannot create new block with L1 origin %s (parent %s) on top of L1 origin %s", epoch, info.ParentHash(), l2Parent.L1Origin)) } - for { - if err := receiptsFetcher.Fetch(ctx); err == io.EOF { - break - } else if err != nil { - return nil, NewTemporaryError(fmt.Errorf("failed to fetch more receipts: %w", err)) - } - } - receipts, err := receiptsFetcher.Result() - if err != nil { - return nil, NewResetError(fmt.Errorf("fetched bad receipt data: %w", err)) - } + deposits, err := DeriveDeposits(receipts, cfg.DepositContractAddress) if err != nil { // deposits may never be ignored. Failing to process them is a critical error. diff --git a/op-node/rollup/derive/attributes_test.go b/op-node/rollup/derive/attributes_test.go index 58d5662331807..e334fe8931039 100644 --- a/op-node/rollup/derive/attributes_test.go +++ b/op-node/rollup/derive/attributes_test.go @@ -36,7 +36,7 @@ func TestPreparePayloadAttributes(t *testing.T) { l1Info := testutils.RandomBlockInfo(rng) l1Info.InfoNum = l2Parent.L1Origin.Number + 1 epoch := l1Info.ID() - l1Fetcher.ExpectFetch(epoch.Hash, l1Info, nil, nil, nil) + l1Fetcher.ExpectFetchReceipts(epoch.Hash, l1Info, nil, nil) _, err := PreparePayloadAttributes(context.Background(), cfg, l1Fetcher, l2Parent, l2Time, 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") @@ -63,7 +63,7 @@ func TestPreparePayloadAttributes(t *testing.T) { epoch := l2Parent.L1Origin epoch.Number += 1 mockRPCErr := errors.New("mock rpc error") - l1Fetcher.ExpectFetch(epoch.Hash, nil, nil, nil, mockRPCErr) + l1Fetcher.ExpectFetchReceipts(epoch.Hash, nil, nil, mockRPCErr) _, err := PreparePayloadAttributes(context.Background(), cfg, l1Fetcher, l2Parent, l2Time, 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,7 +93,7 @@ func TestPreparePayloadAttributes(t *testing.T) { epoch := l1Info.ID() l1InfoTx, err := L1InfoDepositBytes(0, l1Info) require.NoError(t, err) - l1Fetcher.ExpectFetch(epoch.Hash, l1Info, nil, nil, nil) + l1Fetcher.ExpectFetchReceipts(epoch.Hash, l1Info, nil, nil) attrs, err := PreparePayloadAttributes(context.Background(), cfg, l1Fetcher, l2Parent, l2Time, epoch) require.NoError(t, err) require.NotNil(t, attrs) @@ -129,9 +129,7 @@ func TestPreparePayloadAttributes(t *testing.T) { l2Txs := append(append(make([]eth.Data, 0), l1InfoTx), usedDepositTxs...) - // 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) + l1Fetcher.ExpectFetchReceipts(epoch.Hash, l1Info, receipts, nil) attrs, err := PreparePayloadAttributes(context.Background(), cfg, l1Fetcher, l2Parent, l2Time, epoch) require.NoError(t, err) require.NotNil(t, attrs) diff --git a/op-node/rollup/driver/sequencer.go b/op-node/rollup/driver/sequencer.go index 4705cf6524998..2c45b7e396692 100644 --- a/op-node/rollup/driver/sequencer.go +++ b/op-node/rollup/driver/sequencer.go @@ -16,7 +16,7 @@ import ( type Downloader interface { InfoByHash(ctx context.Context, hash common.Hash) (eth.BlockInfo, error) - Fetch(ctx context.Context, blockHash common.Hash) (eth.BlockInfo, types.Transactions, eth.ReceiptsFetcher, error) + FetchReceipts(ctx context.Context, blockHash common.Hash) (eth.BlockInfo, types.Receipts, error) } // Sequencer implements the sequencing interface of the driver: it starts and completes block building jobs. diff --git a/op-node/sources/eth_client.go b/op-node/sources/eth_client.go index 005d2daf358b0..97f663383717e 100644 --- a/op-node/sources/eth_client.go +++ b/op-node/sources/eth_client.go @@ -3,6 +3,7 @@ package sources import ( "context" "fmt" + "io" "github.com/ethereum-optimism/optimism/op-node/client" "github.com/ethereum-optimism/optimism/op-node/eth" @@ -80,7 +81,8 @@ type EthClient struct { log log.Logger // cache receipts in bundles per block hash - // common.Hash -> types.Receipts + // We cache the receipts fetcher to not lose progress when we have to retry the `Fetch` call + // common.Hash -> eth.ReceiptsFetcher receiptsCache *caching.LRUCache // cache transactions in bundles per block hash @@ -230,21 +232,42 @@ func (s *EthClient) PayloadByLabel(ctx context.Context, label eth.BlockLabel) (* return s.payloadCall(ctx, "eth_getBlockByNumber", string(label)) } -func (s *EthClient) Fetch(ctx context.Context, blockHash common.Hash) (eth.BlockInfo, types.Transactions, eth.ReceiptsFetcher, error) { +// FetchReceipts returns a block info and all of the receipts associated with transactions in the block. +// It verifies the receipt hash in the block header against the receipt hash of the fetched receipts +// to ensure that the execution engine did not fail to return any receipts. +func (s *EthClient) FetchReceipts(ctx context.Context, blockHash common.Hash) (eth.BlockInfo, types.Receipts, error) { info, txs, err := s.InfoAndTxsByHash(ctx, blockHash) if err != nil { - return nil, nil, nil, err + return nil, nil, err } + // Try to reuse the receipts fetcher because is caches the results of intermediate calls. This means + // that if just one of many calls fail, we only retry the failed call rather than all of the calls. + // The underlying fetcher uses the receipts hash to verify receipt integrity. + var fetcher eth.ReceiptsFetcher if v, ok := s.receiptsCache.Get(blockHash); ok { - return info, txs, v.(eth.ReceiptsFetcher), nil + fetcher = v.(eth.ReceiptsFetcher) + } else { + txHashes := make([]common.Hash, len(txs)) + for i := 0; i < len(txs); i++ { + txHashes[i] = txs[i].Hash() + } + fetcher = NewReceiptsFetcher(info.ID(), info.ReceiptHash(), txHashes, s.client.BatchCallContext, s.maxBatchSize) + s.receiptsCache.Add(blockHash, fetcher) } - txHashes := make([]common.Hash, len(txs)) - for i := 0; i < len(txs); i++ { - txHashes[i] = txs[i].Hash() + // Fetch all receipts + for { + if err := fetcher.Fetch(ctx); err == io.EOF { + break + } else if err != nil { + return nil, nil, err + } } - r := NewReceiptsFetcher(info.ID(), info.ReceiptHash(), txHashes, s.client.BatchCallContext, s.maxBatchSize) - s.receiptsCache.Add(blockHash, r) - return info, txs, r, nil + receipts, err := fetcher.Result() + if err != nil { + return nil, nil, err + } + + return info, receipts, nil } func (s *EthClient) GetProof(ctx context.Context, address common.Address, blockTag string) (*eth.AccountResult, error) { diff --git a/op-node/testutils/mock_eth_client.go b/op-node/testutils/mock_eth_client.go index 31dfe04bc1a87..2c377c4871336 100644 --- a/op-node/testutils/mock_eth_client.go +++ b/op-node/testutils/mock_eth_client.go @@ -105,13 +105,13 @@ func (m *MockEthClient) ExpectPayloadByLabel(label eth.BlockLabel, payload *eth. m.Mock.On("PayloadByLabel", label).Once().Return(payload, &err) } -func (m *MockEthClient) Fetch(ctx context.Context, blockHash common.Hash) (eth.BlockInfo, types.Transactions, eth.ReceiptsFetcher, error) { - out := m.Mock.MethodCalled("Fetch", blockHash) - return *out[0].(*eth.BlockInfo), out[1].(types.Transactions), out[2].(eth.ReceiptsFetcher), *out[3].(*error) +func (m *MockEthClient) FetchReceipts(ctx context.Context, blockHash common.Hash) (eth.BlockInfo, types.Receipts, error) { + out := m.Mock.MethodCalled("FetchReceipts", blockHash) + return *out[0].(*eth.BlockInfo), out[1].(types.Receipts), *out[2].(*error) } -func (m *MockEthClient) ExpectFetch(hash common.Hash, info eth.BlockInfo, transactions types.Transactions, receipts types.Receipts, err error) { - m.Mock.On("Fetch", hash).Once().Return(&info, transactions, eth.FetchedReceipts(receipts), &err) +func (m *MockEthClient) ExpectFetchReceipts(hash common.Hash, info eth.BlockInfo, receipts types.Receipts, err error) { + m.Mock.On("FetchReceipts", hash).Once().Return(&info, receipts, &err) } func (m *MockEthClient) GetProof(ctx context.Context, address common.Address, blockTag string) (*eth.AccountResult, error) {