From 58dc5239d17632fe4006d7a58344974bd59002e1 Mon Sep 17 00:00:00 2001 From: axelKingsley Date: Wed, 7 Feb 2024 11:23:34 -0600 Subject: [PATCH 1/5] Refactor Receipt Validation to ReceiptsProvider --- op-service/sources/eth_client.go | 7 +------ op-service/sources/receipts.go | 2 +- op-service/sources/receipts_basic.go | 6 +++++- op-service/sources/receipts_basic_test.go | 9 ++++++--- op-service/sources/receipts_caching.go | 5 +++-- op-service/sources/receipts_caching_test.go | 5 +++-- op-service/sources/receipts_rcp.go | 4 ++-- 7 files changed, 21 insertions(+), 17 deletions(-) diff --git a/op-service/sources/eth_client.go b/op-service/sources/eth_client.go index c2b7bf9687bb..46f58cadc71b 100644 --- a/op-service/sources/eth_client.go +++ b/op-service/sources/eth_client.go @@ -318,15 +318,10 @@ func (s *EthClient) FetchReceipts(ctx context.Context, blockHash common.Hash) (e } txHashes, block := eth.TransactionsToHashes(txs), eth.ToBlockID(info) - receipts, err := s.recProvider.FetchReceipts(ctx, block, txHashes) + receipts, err := s.recProvider.FetchReceipts(ctx, block, txHashes, info) 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/receipts.go b/op-service/sources/receipts.go index a7a2b67131ee..421e83040443 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, block eth.BlockID, txHashes []common.Hash, bInfo eth.BlockInfo) (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..ae9f6024065e 100644 --- a/op-service/sources/receipts_basic.go +++ b/op-service/sources/receipts_basic.go @@ -31,7 +31,7 @@ func NewBasicRPCReceiptsFetcher(client rpcClient, maxBatchSize int) *BasicRPCRec } } -func (f *BasicRPCReceiptsFetcher) FetchReceipts(ctx context.Context, block eth.BlockID, txHashes []common.Hash) (types.Receipts, error) { +func (f *BasicRPCReceiptsFetcher) FetchReceipts(ctx context.Context, block eth.BlockID, txHashes []common.Hash, bInfo eth.BlockInfo) (types.Receipts, error) { call := f.getOrCreateBatchCall(block.Hash, txHashes) // Fetch all receipts @@ -46,6 +46,10 @@ func (f *BasicRPCReceiptsFetcher) FetchReceipts(ctx context.Context, block eth.B if err != nil { return nil, err } + err = validateReceipts(block, bInfo.ReceiptHash(), txHashes, res) + if err != nil { + return nil, err + } // call successful, remove from cache f.deleteBatchCall(block.Hash) return res, nil diff --git a/op-service/sources/receipts_basic_test.go b/op-service/sources/receipts_basic_test.go index 2efeb04f3df3..5856ed064e5e 100644 --- a/op-service/sources/receipts_basic_test.go +++ b/op-service/sources/receipts_basic_test.go @@ -75,8 +75,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, blockid, txHashes, bInfo) require.Error(err) require.Nil(recs) require.EqualValues(2, numCalls.Load()) @@ -84,7 +86,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, blockid, txHashes, bInfo) require.NoError(err) require.NotNil(recs) for i, rec := range recs { @@ -141,12 +143,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, block.BlockID(), txHashes, bInfo) fetchResults <- fetchResult{rs: recs, err: err} }() } diff --git a/op-service/sources/receipts_caching.go b/op-service/sources/receipts_caching.go index bc02e39e8728..656e076309c1 100644 --- a/op-service/sources/receipts_caching.go +++ b/op-service/sources/receipts_caching.go @@ -51,7 +51,7 @@ 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) { +func (p *CachingReceiptsProvider) FetchReceipts(ctx context.Context, block eth.BlockID, txHashes []common.Hash, bInfo eth.BlockInfo) (types.Receipts, error) { if r, ok := p.cache.Get(block.Hash); ok { return r, nil } @@ -67,10 +67,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, block, txHashes, bInfo) 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..7a892e9e2839 100644 --- a/op-service/sources/receipts_caching_test.go +++ b/op-service/sources/receipts_caching_test.go @@ -17,7 +17,7 @@ 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, block eth.BlockID, txHashes []common.Hash, bInfo eth.BlockInfo) (types.Receipts, error) { args := m.Called(ctx, block, txHashes) return args.Get(0).(types.Receipts), args.Error(1) } @@ -35,8 +35,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, blockid, txHashes, bInfo) 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_rcp.go index bf6398b5a8f2..bb843bd3f42c 100644 --- a/op-service/sources/receipts_rcp.go +++ b/op-service/sources/receipts_rcp.go @@ -70,11 +70,11 @@ 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, block eth.BlockID, txHashes []common.Hash, bInfo eth.BlockInfo) (result types.Receipts, err error) { m := f.PickReceiptsMethod(len(txHashes)) switch m { case EthGetTransactionReceiptBatch: - result, err = f.basic.FetchReceipts(ctx, block, txHashes) + result, err = f.basic.FetchReceipts(ctx, block, txHashes, bInfo) case AlchemyGetTransactionReceipts: var tmp receiptsWrapper err = f.client.CallContext(ctx, &tmp, "alchemy_getTransactionReceipts", blockHashParameter{BlockHash: block.Hash}) From 9dbb7b016b2ac39a954242a5d79b2f9980ad6cfc Mon Sep 17 00:00:00 2001 From: axelKingsley Date: Wed, 7 Feb 2024 15:45:27 -0600 Subject: [PATCH 2/5] PR Comments --- op-service/sources/eth_client.go | 4 +- op-service/sources/eth_client_test.go | 41 +++++++++++++++++++ op-service/sources/receipts.go | 2 +- op-service/sources/receipts_basic.go | 8 ++-- op-service/sources/receipts_basic_test.go | 7 ++-- op-service/sources/receipts_caching.go | 5 ++- op-service/sources/receipts_caching_test.go | 5 ++- .../{receipts_rcp.go => receipts_rpc.go} | 9 +++- 8 files changed, 63 insertions(+), 18 deletions(-) rename op-service/sources/{receipts_rcp.go => receipts_rpc.go} (98%) diff --git a/op-service/sources/eth_client.go b/op-service/sources/eth_client.go index 46f58cadc71b..8f73b13b6110 100644 --- a/op-service/sources/eth_client.go +++ b/op-service/sources/eth_client.go @@ -317,8 +317,8 @@ 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, info) + txHashes, _ := eth.TransactionsToHashes(txs), eth.ToBlockID(info) + receipts, err := s.recProvider.FetchReceipts(ctx, info, txHashes) if err != nil { return nil, nil, err } diff --git a/op-service/sources/eth_client_test.go b/op-service/sources/eth_client_test.go index 3589da0dfafa..8531ae1bd056 100644 --- a/op-service/sources/eth_client_test.go +++ b/op-service/sources/eth_client_test.go @@ -3,6 +3,7 @@ package sources import ( "context" crand "crypto/rand" + "fmt" "math/big" "math/rand" "testing" @@ -272,3 +273,43 @@ func newEthClientWithCaches(metrics caching.Metrics, cacheSize int) *EthClient { payloadsCache: caching.NewLRUCache[common.Hash, *eth.ExecutionPayloadEnvelope](metrics, "payloads", cacheSize), } } + +// TestReceiptValidation tests that the receipt validation is performed by the underlying RPCReceiptsFetcher +func TestReceiptValidation(t *testing.T) { + require := require.New(t) + mrpc := new(mockRPC) + rp := NewRPCReceiptsFetcher(mrpc, nil, RPCReceiptsConfig{}) + const numTxs = 1 + block, _ := randomRpcBlockAndReceipts(rand.New(rand.NewSource(420)), numTxs) + //txHashes := receiptTxHashes(receipts) + ctx := context.Background() + + mrpc.On("CallContext", + ctx, + mock.Anything, + "eth_getTransactionReceipt", + mock.Anything). + Run(func(args mock.Arguments) { + fmt.Println("getTransactionReceipt called", args) + }). + 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}) + + ethcl := newEthClientWithCaches(nil, numTxs) + ethcl.client = mrpc + ethcl.recProvider = rp + ethcl.trustRPC = true + + _, _, 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 421e83040443..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, bInfo eth.BlockInfo) (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 ae9f6024065e..f88d09b1971b 100644 --- a/op-service/sources/receipts_basic.go +++ b/op-service/sources/receipts_basic.go @@ -31,7 +31,8 @@ func NewBasicRPCReceiptsFetcher(client rpcClient, maxBatchSize int) *BasicRPCRec } } -func (f *BasicRPCReceiptsFetcher) FetchReceipts(ctx context.Context, block eth.BlockID, txHashes []common.Hash, bInfo eth.BlockInfo) (types.Receipts, error) { +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 @@ -46,10 +47,7 @@ func (f *BasicRPCReceiptsFetcher) FetchReceipts(ctx context.Context, block eth.B if err != nil { return nil, err } - err = validateReceipts(block, bInfo.ReceiptHash(), txHashes, res) - if err != nil { - return nil, err - } + // call successful, remove from cache f.deleteBatchCall(block.Hash) return res, nil diff --git a/op-service/sources/receipts_basic_test.go b/op-service/sources/receipts_basic_test.go index 5856ed064e5e..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 { @@ -78,7 +77,7 @@ func TestBasicRPCReceiptsFetcher_Reuse(t *testing.T) { bInfo, _, _ := block.Info(true, true) // 1st fetching should result in errors - recs, err := rp.FetchReceipts(ctx, blockid, txHashes, bInfo) + recs, err := rp.FetchReceipts(ctx, bInfo, txHashes) require.Error(err) require.Nil(recs) require.EqualValues(2, numCalls.Load()) @@ -86,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, bInfo) + recs, err = rp.FetchReceipts(ctx, bInfo, txHashes) require.NoError(err) require.NotNil(recs) for i, rec := range recs { @@ -149,7 +148,7 @@ func runConcurrentFetchingTest(t *testing.T, rp ReceiptsProvider, numFetchers in for i := 0; i < numFetchers; i++ { go func() { <-barrier - recs, err := rp.FetchReceipts(ctx, block.BlockID(), txHashes, bInfo) + 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 656e076309c1..ac9f89ac4e4f 100644 --- a/op-service/sources/receipts_caching.go +++ b/op-service/sources/receipts_caching.go @@ -51,7 +51,8 @@ func (p *CachingReceiptsProvider) deleteFetchingLock(blockHash common.Hash) { delete(p.fetching, blockHash) } -func (p *CachingReceiptsProvider) FetchReceipts(ctx context.Context, block eth.BlockID, txHashes []common.Hash, bInfo eth.BlockInfo) (types.Receipts, error) { +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,7 +68,7 @@ func (p *CachingReceiptsProvider) FetchReceipts(ctx context.Context, block eth.B return r, nil } - r, err := p.inner.FetchReceipts(ctx, block, txHashes, bInfo) + r, err := p.inner.FetchReceipts(ctx, blockInfo, txHashes) if err != nil { return nil, err } diff --git a/op-service/sources/receipts_caching_test.go b/op-service/sources/receipts_caching_test.go index 7a892e9e2839..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, bInfo eth.BlockInfo) (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) } @@ -37,7 +38,7 @@ func TestCachingReceiptsProvider_Caching(t *testing.T) { bInfo, _, _ := block.Info(true, true) for i := 0; i < 4; i++ { - gotRecs, err := rp.FetchReceipts(ctx, blockid, txHashes, bInfo) + 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 bb843bd3f42c..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, bInfo eth.BlockInfo) (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, bInfo) + 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 } From ad1cb95e0216f79b235b338745f9ebf45350b7f3 Mon Sep 17 00:00:00 2001 From: axelKingsley Date: Wed, 7 Feb 2024 15:49:13 -0600 Subject: [PATCH 3/5] tweaks --- op-service/sources/eth_client_test.go | 2 -- op-service/sources/receipts_basic.go | 1 - 2 files changed, 3 deletions(-) diff --git a/op-service/sources/eth_client_test.go b/op-service/sources/eth_client_test.go index 8531ae1bd056..2ec33c0f1e96 100644 --- a/op-service/sources/eth_client_test.go +++ b/op-service/sources/eth_client_test.go @@ -3,7 +3,6 @@ package sources import ( "context" crand "crypto/rand" - "fmt" "math/big" "math/rand" "testing" @@ -290,7 +289,6 @@ func TestReceiptValidation(t *testing.T) { "eth_getTransactionReceipt", mock.Anything). Run(func(args mock.Arguments) { - fmt.Println("getTransactionReceipt called", args) }). Return([]error{nil}) diff --git a/op-service/sources/receipts_basic.go b/op-service/sources/receipts_basic.go index f88d09b1971b..550453ec1239 100644 --- a/op-service/sources/receipts_basic.go +++ b/op-service/sources/receipts_basic.go @@ -47,7 +47,6 @@ func (f *BasicRPCReceiptsFetcher) FetchReceipts(ctx context.Context, blockInfo e if err != nil { return nil, err } - // call successful, remove from cache f.deleteBatchCall(block.Hash) return res, nil From 1b8d07d69e0e3b513bd020c161aa3dddc088a5f2 Mon Sep 17 00:00:00 2001 From: axelKingsley Date: Wed, 7 Feb 2024 15:55:05 -0600 Subject: [PATCH 4/5] remove client validation test; client no longer owns validation --- op-service/sources/eth_client_test.go | 85 --------------------------- 1 file changed, 85 deletions(-) diff --git a/op-service/sources/eth_client_test.go b/op-service/sources/eth_client_test.go index 2ec33c0f1e96..fe2d9839f009 100644 --- a/op-service/sources/eth_client_test.go +++ b/op-service/sources/eth_client_test.go @@ -180,91 +180,6 @@ 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 testEthClient_validateReceipts(t *testing.T, test validateReceiptsTest) { - 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) - 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}). - 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() - - 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) -} - func newEthClientWithCaches(metrics caching.Metrics, cacheSize int) *EthClient { return &EthClient{ transactionsCache: caching.NewLRUCache[common.Hash, types.Transactions](metrics, "txs", cacheSize), From 771f9c2d84acf0313e1f3aaec392233cfe24ffd1 Mon Sep 17 00:00:00 2001 From: axelKingsley Date: Wed, 7 Feb 2024 17:00:16 -0600 Subject: [PATCH 5/5] add comments --- op-service/sources/receipts_basic.go | 2 ++ op-service/sources/receipts_caching.go | 2 ++ 2 files changed, 4 insertions(+) diff --git a/op-service/sources/receipts_basic.go b/op-service/sources/receipts_basic.go index 550453ec1239..ec63c0565bf2 100644 --- a/op-service/sources/receipts_basic.go +++ b/op-service/sources/receipts_basic.go @@ -31,6 +31,8 @@ func NewBasicRPCReceiptsFetcher(client rpcClient, maxBatchSize int) *BasicRPCRec } } +// 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) diff --git a/op-service/sources/receipts_caching.go b/op-service/sources/receipts_caching.go index ac9f89ac4e4f..4631e4e78b23 100644 --- a/op-service/sources/receipts_caching.go +++ b/op-service/sources/receipts_caching.go @@ -51,6 +51,8 @@ func (p *CachingReceiptsProvider) deleteFetchingLock(blockHash common.Hash) { delete(p.fetching, blockHash) } +// 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 {