op-node: Simplify Receipts Fetching API

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.

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)

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.

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) {

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) {