diff --git a/op-service/sources/eth_client.go b/op-service/sources/eth_client.go index c2b7bf9687bb..8f73b13b6110 100644 --- a/op-service/sources/eth_client.go +++ b/op-service/sources/eth_client.go @@ -317,16 +317,11 @@ func (s *EthClient) FetchReceipts(ctx context.Context, blockHash common.Hash) (e return nil, nil, fmt.Errorf("querying block: %w", err) } - txHashes, block := eth.TransactionsToHashes(txs), eth.ToBlockID(info) - receipts, err := s.recProvider.FetchReceipts(ctx, block, txHashes) + txHashes, _ := eth.TransactionsToHashes(txs), eth.ToBlockID(info) + receipts, err := s.recProvider.FetchReceipts(ctx, info, txHashes) if err != nil { return nil, nil, err } - - if err := validateReceipts(block, info.ReceiptHash(), txHashes, receipts); err != nil { - return info, nil, fmt.Errorf("invalid receipts: %w", err) - } - return info, receipts, nil } diff --git a/op-service/sources/eth_client_test.go b/op-service/sources/eth_client_test.go index 3589da0dfafa..fe2d9839f009 100644 --- a/op-service/sources/eth_client_test.go +++ b/op-service/sources/eth_client_test.go @@ -180,95 +180,49 @@ func TestEthClient_WrongInfoByHash(t *testing.T) { m.Mock.AssertExpectations(t) } -type validateReceiptsTest struct { - desc string - trustRPC bool - mutReceipts func(types.Receipts) types.Receipts - expError string -} - -func TestEthClient_validateReceipts(t *testing.T) { - mutBloom := func(rs types.Receipts) types.Receipts { - rs[2].Bloom[0] = 1 - return rs - } - mutTruncate := func(rs types.Receipts) types.Receipts { - return rs[:len(rs)-1] - } - - for _, tt := range []validateReceiptsTest{ - { - desc: "valid", - }, - { - desc: "invalid-mut-bloom", - mutReceipts: mutBloom, - expError: "invalid receipts: failed to fetch list of receipts: expected receipt root", - }, - { - desc: "invalid-truncated", - mutReceipts: mutTruncate, - expError: "invalid receipts: got 3 receipts but expected 4", - }, - } { - t.Run("no-trust-"+tt.desc, func(t *testing.T) { - testEthClient_validateReceipts(t, tt) - }) - // trusting the rpc should still lead to failed validation - tt.trustRPC = true - t.Run("trust-"+tt.desc, func(t *testing.T) { - testEthClient_validateReceipts(t, tt) - }) +func newEthClientWithCaches(metrics caching.Metrics, cacheSize int) *EthClient { + return &EthClient{ + transactionsCache: caching.NewLRUCache[common.Hash, types.Transactions](metrics, "txs", cacheSize), + headersCache: caching.NewLRUCache[common.Hash, eth.BlockInfo](metrics, "headers", cacheSize), + payloadsCache: caching.NewLRUCache[common.Hash, *eth.ExecutionPayloadEnvelope](metrics, "payloads", cacheSize), } } -func testEthClient_validateReceipts(t *testing.T, test validateReceiptsTest) { +// TestReceiptValidation tests that the receipt validation is performed by the underlying RPCReceiptsFetcher +func TestReceiptValidation(t *testing.T) { require := require.New(t) mrpc := new(mockRPC) - mrp := new(mockReceiptsProvider) - const numTxs = 4 - block, receipts := randomRpcBlockAndReceipts(rand.New(rand.NewSource(420)), numTxs) - txHashes := receiptTxHashes(receipts) + rp := NewRPCReceiptsFetcher(mrpc, nil, RPCReceiptsConfig{}) + const numTxs = 1 + block, _ := randomRpcBlockAndReceipts(rand.New(rand.NewSource(420)), numTxs) + //txHashes := receiptTxHashes(receipts) ctx := context.Background() - if mut := test.mutReceipts; mut != nil { - receipts = mut(receipts) - } - - mrpc.On("CallContext", ctx, mock.AnythingOfType("**sources.RPCBlock"), - "eth_getBlockByHash", []any{block.Hash, true}). + mrpc.On("CallContext", + ctx, + mock.Anything, + "eth_getTransactionReceipt", + mock.Anything). + Run(func(args mock.Arguments) { + }). + Return([]error{nil}) + + // when the block is requested, the block is returned + mrpc.On("CallContext", + ctx, + mock.Anything, + "eth_getBlockByHash", + mock.Anything). Run(func(args mock.Arguments) { *(args[1].(**RPCBlock)) = block }). - Return([]error{nil}).Once() - - mrp.On("FetchReceipts", ctx, block.BlockID(), txHashes). - Return(types.Receipts(receipts), error(nil)).Once() + Return([]error{nil}) ethcl := newEthClientWithCaches(nil, numTxs) ethcl.client = mrpc - ethcl.recProvider = mrp - ethcl.trustRPC = test.trustRPC - - info, recs, err := ethcl.FetchReceipts(ctx, block.Hash) - if test.expError != "" { - require.ErrorContains(err, test.expError) - } else { - require.NoError(err) - expInfo, _, err := block.Info(true, false) - require.NoError(err) - require.Equal(expInfo, info) - require.Equal(types.Receipts(receipts), recs) - } - - mrpc.AssertExpectations(t) - mrp.AssertExpectations(t) -} + ethcl.recProvider = rp + ethcl.trustRPC = true -func newEthClientWithCaches(metrics caching.Metrics, cacheSize int) *EthClient { - return &EthClient{ - transactionsCache: caching.NewLRUCache[common.Hash, types.Transactions](metrics, "txs", cacheSize), - headersCache: caching.NewLRUCache[common.Hash, eth.BlockInfo](metrics, "headers", cacheSize), - payloadsCache: caching.NewLRUCache[common.Hash, *eth.ExecutionPayloadEnvelope](metrics, "payloads", cacheSize), - } + _, _, err := ethcl.FetchReceipts(ctx, block.Hash) + require.ErrorContains(err, "unexpected nil block number") } diff --git a/op-service/sources/receipts.go b/op-service/sources/receipts.go index a7a2b67131ee..79929e4f7318 100644 --- a/op-service/sources/receipts.go +++ b/op-service/sources/receipts.go @@ -14,7 +14,7 @@ type ReceiptsProvider interface { // 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. - FetchReceipts(ctx context.Context, block eth.BlockID, txHashes []common.Hash) (types.Receipts, error) + FetchReceipts(ctx context.Context, blockInfo eth.BlockInfo, txHashes []common.Hash) (types.Receipts, error) } // validateReceipts validates that the receipt contents are valid. diff --git a/op-service/sources/receipts_basic.go b/op-service/sources/receipts_basic.go index 85d5cea679ba..ec63c0565bf2 100644 --- a/op-service/sources/receipts_basic.go +++ b/op-service/sources/receipts_basic.go @@ -31,7 +31,10 @@ func NewBasicRPCReceiptsFetcher(client rpcClient, maxBatchSize int) *BasicRPCRec } } -func (f *BasicRPCReceiptsFetcher) FetchReceipts(ctx context.Context, block eth.BlockID, txHashes []common.Hash) (types.Receipts, error) { +// FetchReceipts fetches receipts for the given block and transaction hashes +// it does not validate receipts, and expects the caller to do so +func (f *BasicRPCReceiptsFetcher) FetchReceipts(ctx context.Context, blockInfo eth.BlockInfo, txHashes []common.Hash) (types.Receipts, error) { + block := eth.ToBlockID(blockInfo) call := f.getOrCreateBatchCall(block.Hash, txHashes) // Fetch all receipts diff --git a/op-service/sources/receipts_basic_test.go b/op-service/sources/receipts_basic_test.go index 2efeb04f3df3..3a0bbdf2219d 100644 --- a/op-service/sources/receipts_basic_test.go +++ b/op-service/sources/receipts_basic_test.go @@ -35,7 +35,6 @@ func TestBasicRPCReceiptsFetcher_Reuse(t *testing.T) { require := require.New(t) batchSize, txCount := 2, uint64(4) block, receipts := randomRpcBlockAndReceipts(rand.New(rand.NewSource(123)), txCount) - blockid := block.BlockID() txHashes := make([]common.Hash, 0, len(receipts)) recMap := make(map[common.Hash]*types.Receipt, len(receipts)) for _, rec := range receipts { @@ -75,8 +74,10 @@ func TestBasicRPCReceiptsFetcher_Reuse(t *testing.T) { return err } + bInfo, _, _ := block.Info(true, true) + // 1st fetching should result in errors - recs, err := rp.FetchReceipts(ctx, blockid, txHashes) + recs, err := rp.FetchReceipts(ctx, bInfo, txHashes) require.Error(err) require.Nil(recs) require.EqualValues(2, numCalls.Load()) @@ -84,7 +85,7 @@ func TestBasicRPCReceiptsFetcher_Reuse(t *testing.T) { // prepare 2nd fetching - all should succeed now response[txHashes[2]] = true response[txHashes[3]] = true - recs, err = rp.FetchReceipts(ctx, blockid, txHashes) + recs, err = rp.FetchReceipts(ctx, bInfo, txHashes) require.NoError(err) require.NotNil(recs) for i, rec := range recs { @@ -141,12 +142,13 @@ func runConcurrentFetchingTest(t *testing.T, rp ReceiptsProvider, numFetchers in } fetchResults := make(chan fetchResult, numFetchers) barrier := make(chan struct{}) + bInfo, _, _ := block.Info(true, true) ctx, done := context.WithTimeout(context.Background(), 10*time.Second) defer done() for i := 0; i < numFetchers; i++ { go func() { <-barrier - recs, err := rp.FetchReceipts(ctx, block.BlockID(), txHashes) + recs, err := rp.FetchReceipts(ctx, bInfo, txHashes) fetchResults <- fetchResult{rs: recs, err: err} }() } diff --git a/op-service/sources/receipts_caching.go b/op-service/sources/receipts_caching.go index bc02e39e8728..4631e4e78b23 100644 --- a/op-service/sources/receipts_caching.go +++ b/op-service/sources/receipts_caching.go @@ -51,7 +51,10 @@ func (p *CachingReceiptsProvider) deleteFetchingLock(blockHash common.Hash) { delete(p.fetching, blockHash) } -func (p *CachingReceiptsProvider) FetchReceipts(ctx context.Context, block eth.BlockID, txHashes []common.Hash) (types.Receipts, error) { +// FetchReceipts fetches receipts for the given block and transaction hashes +// it expects that the inner FetchReceipts implementation handles validation +func (p *CachingReceiptsProvider) FetchReceipts(ctx context.Context, blockInfo eth.BlockInfo, txHashes []common.Hash) (types.Receipts, error) { + block := eth.ToBlockID(blockInfo) if r, ok := p.cache.Get(block.Hash); ok { return r, nil } @@ -67,10 +70,11 @@ func (p *CachingReceiptsProvider) FetchReceipts(ctx context.Context, block eth.B return r, nil } - r, err := p.inner.FetchReceipts(ctx, block, txHashes) + r, err := p.inner.FetchReceipts(ctx, blockInfo, txHashes) if err != nil { return nil, err } + p.cache.Add(block.Hash, r) // result now in cache, can delete fetching lock p.deleteFetchingLock(block.Hash) diff --git a/op-service/sources/receipts_caching_test.go b/op-service/sources/receipts_caching_test.go index a4eacf518fda..e7891b9e34fd 100644 --- a/op-service/sources/receipts_caching_test.go +++ b/op-service/sources/receipts_caching_test.go @@ -17,7 +17,8 @@ type mockReceiptsProvider struct { mock.Mock } -func (m *mockReceiptsProvider) FetchReceipts(ctx context.Context, block eth.BlockID, txHashes []common.Hash) (types.Receipts, error) { +func (m *mockReceiptsProvider) FetchReceipts(ctx context.Context, blockInfo eth.BlockInfo, txHashes []common.Hash) (types.Receipts, error) { + block := eth.ToBlockID(blockInfo) args := m.Called(ctx, block, txHashes) return args.Get(0).(types.Receipts), args.Error(1) } @@ -35,8 +36,9 @@ func TestCachingReceiptsProvider_Caching(t *testing.T) { Return(types.Receipts(receipts), error(nil)). Once() // receipts should be cached after first fetch + bInfo, _, _ := block.Info(true, true) for i := 0; i < 4; i++ { - gotRecs, err := rp.FetchReceipts(ctx, blockid, txHashes) + gotRecs, err := rp.FetchReceipts(ctx, bInfo, txHashes) require.NoError(t, err) for i, gotRec := range gotRecs { requireEqualReceipt(t, receipts[i], gotRec) diff --git a/op-service/sources/receipts_rcp.go b/op-service/sources/receipts_rpc.go similarity index 98% rename from op-service/sources/receipts_rcp.go rename to op-service/sources/receipts_rpc.go index bf6398b5a8f2..ecf2c8582210 100644 --- a/op-service/sources/receipts_rcp.go +++ b/op-service/sources/receipts_rpc.go @@ -70,11 +70,12 @@ func NewRPCReceiptsFetcher(client rpcClient, log log.Logger, config RPCReceiptsC } } -func (f *RPCReceiptsFetcher) FetchReceipts(ctx context.Context, block eth.BlockID, txHashes []common.Hash) (result types.Receipts, err error) { +func (f *RPCReceiptsFetcher) FetchReceipts(ctx context.Context, blockInfo eth.BlockInfo, txHashes []common.Hash) (result types.Receipts, err error) { m := f.PickReceiptsMethod(len(txHashes)) + block := eth.ToBlockID(blockInfo) switch m { case EthGetTransactionReceiptBatch: - result, err = f.basic.FetchReceipts(ctx, block, txHashes) + result, err = f.basic.FetchReceipts(ctx, blockInfo, txHashes) case AlchemyGetTransactionReceipts: var tmp receiptsWrapper err = f.client.CallContext(ctx, &tmp, "alchemy_getTransactionReceipts", blockHashParameter{BlockHash: block.Hash}) @@ -104,6 +105,10 @@ func (f *RPCReceiptsFetcher) FetchReceipts(ctx context.Context, block eth.BlockI return nil, err } + if err = validateReceipts(block, blockInfo.ReceiptHash(), txHashes, result); err != nil { + return nil, err + } + return }