Merge pull request #5430 from ethereum-optimism/revert-5426-michael/support-unbatched-calls

Revert "[op-node] Add support for non-batched RPC calls when batchSize == 1"

op-node/sources/batching.go

@@ -24,7 +24,6 @@ type IterativeBatchCall[K any, V any] struct {
 
 	makeRequest func(K) (V, rpc.BatchElem)
 	getBatch    BatchCallContextFn
-	getSingle   CallContextFn
 
 	requestsValues []V
 	scheduled      chan rpc.BatchElem
@@ -36,7 +35,6 @@ func NewIterativeBatchCall[K any, V any](
 	requestsKeys []K,
 	makeRequest func(K) (V, rpc.BatchElem),
 	getBatch BatchCallContextFn,
-	getSingle CallContextFn,
 	batchSize int) *IterativeBatchCall[K, V] {
 
 	if len(requestsKeys) < batchSize {
@@ -49,7 +47,6 @@ func NewIterativeBatchCall[K any, V any](
 	out := &IterativeBatchCall[K, V]{
 		completed:    0,
 		getBatch:     getBatch,
-		getSingle:    getSingle,
 		requestsKeys: requestsKeys,
 		batchSize:    batchSize,
 		makeRequest:  makeRequest,
@@ -122,23 +119,11 @@ func (ibc *IterativeBatchCall[K, V]) Fetch(ctx context.Context) error {
 		break
 	}
 
-	if len(batch) == 0 {
-		return nil
-	}
-
-	if ibc.batchSize == 1 {
-		first := batch[0]
-		if err := ibc.getSingle(ctx, &first.Result, first.Method, first.Args...); err != nil {
-			ibc.scheduled <- first
-			return err
-		}
-	} else {
-		if err := ibc.getBatch(ctx, batch); err != nil {
-			for _, r := range batch {
-				ibc.scheduled <- r
-			}
-			return fmt.Errorf("failed batch-retrieval: %w", err)
+	if err := ibc.getBatch(ctx, batch); err != nil {
+		for _, r := range batch {
+			ibc.scheduled <- r
 		}
+		return fmt.Errorf("failed batch-retrieval: %w", err)
 	}
 	var result error
 	for _, elem := range batch {

op-node/sources/batching_test.go

@@ -34,8 +34,7 @@ type batchTestCase struct {
 
 	batchSize int
 
-	batchCalls  []batchCall
-	singleCalls []elemCall
+	batchCalls []batchCall
 
 	mock.Mock
 }
@@ -54,14 +53,7 @@ func (tc *batchTestCase) GetBatch(ctx context.Context, b []rpc.BatchElem) error
 	if ctx.Err() != nil {
 		return ctx.Err()
 	}
-	return tc.Mock.MethodCalled("getBatch", b).Get(0).([]error)[0]
-}
-
-func (tc *batchTestCase) GetSingle(ctx context.Context, result any, method string, args ...any) error {
-	if ctx.Err() != nil {
-		return ctx.Err()
-	}
-	return tc.Mock.MethodCalled("getSingle", (*(result.(*interface{}))).(*string), method, args[0]).Get(0).([]error)[0]
+	return tc.Mock.MethodCalled("get", b).Get(0).([]error)[0]
 }
 
 var mockErr = errors.New("mockErr")
@@ -72,7 +64,7 @@ func (tc *batchTestCase) Run(t *testing.T) {
 		keys[i] = i
 	}
 
-	makeBatchMock := func(bci int, bc batchCall) func(args mock.Arguments) {
+	makeMock := func(bci int, bc batchCall) func(args mock.Arguments) {
 		return func(args mock.Arguments) {
 			batch := args[0].([]rpc.BatchElem)
 			for i, elem := range batch {
@@ -102,30 +94,10 @@ func (tc *batchTestCase) Run(t *testing.T) {
 			})
 		}
 		if len(bc.elems) > 0 {
-			tc.On("getBatch", batch).Once().Run(makeBatchMock(bci, bc)).Return([]error{bc.rpcErr}) // wrap to preserve nil as type of error
-		}
-	}
-	makeSingleMock := func(eci int, ec elemCall) func(args mock.Arguments) {
-		return func(args mock.Arguments) {
-			result := args[0].(*string)
-			id := args[2].(int)
-			require.Equal(t, ec.id, id, "element should match expected element")
-			if ec.err {
-				*result = ""
-			} else {
-				*result = fmt.Sprintf("mock result id %d", id)
-			}
+			tc.On("get", batch).Once().Run(makeMock(bci, bc)).Return([]error{bc.rpcErr}) // wrap to preserve nil as type of error
 		}
 	}
-	// mock the results of unbatched calls
-	for eci, ec := range tc.singleCalls {
-		var ret error
-		if ec.err {
-			ret = mockErr
-		}
-		tc.On("getSingle", new(string), "testing_foobar", ec.id).Once().Run(makeSingleMock(eci, ec)).Return([]error{ret})
-	}
-	iter := NewIterativeBatchCall[int, *string](keys, makeTestRequest, tc.GetBatch, tc.GetSingle, tc.batchSize)
+	iter := NewIterativeBatchCall[int, *string](keys, makeTestRequest, tc.GetBatch, tc.batchSize)
 	for i, bc := range tc.batchCalls {
 		ctx := context.Background()
 		if bc.makeCtx != nil {
@@ -144,20 +116,6 @@ func (tc *batchTestCase) Run(t *testing.T) {
 			}
 		}
 	}
-	for i, ec := range tc.singleCalls {
-		ctx := context.Background()
-		err := iter.Fetch(ctx)
-		if err == io.EOF {
-			require.Equal(t, i, len(tc.singleCalls)-1, "EOF only on last call")
-		} else {
-			require.False(t, iter.Complete())
-			if ec.err {
-				require.Error(t, err)
-			} else {
-				require.NoError(t, err)
-			}
-		}
-	}
 	require.True(t, iter.Complete(), "batch iter should be complete after the expected calls")
 	out, err := iter.Result()
 	require.NoError(t, err)
@@ -196,37 +154,6 @@ func TestFetchBatched(t *testing.T) {
 				},
 			},
 		},
-		{
-			name:      "single element",
-			items:     1,
-			batchSize: 4,
-			singleCalls: []elemCall{
-				{id: 0, err: false},
-			},
-		},
-		{
-			name:      "unbatched",
-			items:     4,
-			batchSize: 1,
-			singleCalls: []elemCall{
-				{id: 0, err: false},
-				{id: 1, err: false},
-				{id: 2, err: false},
-				{id: 3, err: false},
-			},
-		},
-		{
-			name:      "unbatched with retry",
-			items:     4,
-			batchSize: 1,
-			singleCalls: []elemCall{
-				{id: 0, err: false},
-				{id: 1, err: true},
-				{id: 2, err: false},
-				{id: 3, err: false},
-				{id: 1, err: false},
-			},
-		},
 		{
 			name:      "split",
 			items:     5,
@@ -313,7 +240,7 @@ func TestFetchBatched(t *testing.T) {
 		},
 		{
 			name:      "context timeout",
-			items:     2,
+			items:     1,
 			batchSize: 3,
 			batchCalls: []batchCall{
 				{
@@ -328,7 +255,6 @@ func TestFetchBatched(t *testing.T) {
 				{
 					elems: []elemCall{
 						{id: 0, err: false},
-						{id: 1, err: false},
 					},
 					err: "",
 				},

op-node/sources/receipts.go

@@ -373,7 +373,6 @@ func (job *receiptsFetchingJob) runFetcher(ctx context.Context) error {
 			job.txHashes,
 			makeReceiptRequest,
 			job.client.BatchCallContext,
-			job.client.CallContext,
 			job.maxBatchSize,
 		)
 	}