Skip to content

Commit 4ee6b80

Browse files
tac0turtletac0turtlegraphite-app[bot]
authored
chore: (core/da) cleanup interface (#2278)
<!-- Please read and fill out this form before submitting your PR. Please make sure you have reviewed our contributors guide before submitting your first PR. NOTE: PR titles should follow semantic commits: https://www.conventionalcommits.org/en/v1.0.0/ --> ## Overview This pr removes unused namespace from functions and the submit function from the da interface <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Refactor** - Simplified the data availability (DA) interface and related APIs by removing the namespace parameter from most methods, resulting in cleaner and more consistent method signatures. - Unified submission methods, consolidating options handling and ensuring consistent blob size checks. - Updated all internal usage, mocks, and tests to match the revised interface, removing unnecessary placeholder arguments. - Refactored JSON-RPC server and proxy client implementations for clearer method dispatch and improved modularity. - **Bug Fixes** - Improved error handling in tests to prevent nil pointer dereferences and better tolerate expected errors. - **Chores** - Removed obsolete mock generation commands and cleaned up unused code. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: tac0turtle <you@example.com> Co-authored-by: graphite-app[bot] <96075541+graphite-app[bot]@users.noreply.github.com>
1 parent 44fee3d commit 4ee6b80

22 files changed

Lines changed: 384 additions & 484 deletions

File tree

block/da_speed_test.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -47,16 +47,16 @@ func TestDASpeed(t *testing.T) {
4747
manager, mockDAClient := setupManagerForTest(t, daHeight)
4848

4949
var receivedBlockCount atomic.Uint64
50-
ids, namespace := []coreda.ID{[]byte("dummy-id")}, []byte("placeholder")
50+
ids := []coreda.ID{[]byte("dummy-id")}
5151
mockDAClient.
52-
On("GetIDs", mock.Anything, mock.Anything, namespace).
53-
Return(func(ctx context.Context, height uint64, namespace []byte) (*coreda.GetIDsResult, error) {
52+
On("GetIDs", mock.Anything, mock.Anything).
53+
Return(func(ctx context.Context, height uint64) (*coreda.GetIDsResult, error) {
5454
return &coreda.GetIDsResult{IDs: ids, Timestamp: time.Now()}, nil
5555
})
5656

5757
mockDAClient.
58-
On("Get", mock.Anything, ids, namespace).
59-
Return(func(ctx context.Context, ids []coreda.ID, namespace []byte) ([]coreda.Blob, error) {
58+
On("Get", mock.Anything, ids).
59+
Return(func(ctx context.Context, ids []coreda.ID) ([]coreda.Blob, error) {
6060
time.Sleep(spec.daDelay)
6161
// unique headers for cache misses
6262
n := receivedBlockCount.Add(1)

block/retriever_test.go

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -135,11 +135,11 @@ func TestProcessNextDAHeader_Success_SingleHeaderAndData(t *testing.T) {
135135
blockDataBytes, err := signedData.MarshalBinary()
136136
require.NoError(t, err)
137137
// -----------------------------------------------------------
138-
mockDAClient.On("GetIDs", mock.Anything, daHeight, []byte("placeholder")).Return(&coreda.GetIDsResult{
138+
mockDAClient.On("GetIDs", mock.Anything, daHeight).Return(&coreda.GetIDsResult{
139139
IDs: []coreda.ID{[]byte("dummy-id")},
140140
Timestamp: time.Now(),
141141
}, nil).Once()
142-
mockDAClient.On("Get", mock.Anything, []coreda.ID{[]byte("dummy-id")}, []byte("placeholder")).Return(
142+
mockDAClient.On("Get", mock.Anything, []coreda.ID{[]byte("dummy-id")}).Return(
143143
[]coreda.Blob{headerBytes, blockDataBytes}, nil,
144144
).Once()
145145

@@ -247,11 +247,11 @@ func TestProcessNextDAHeader_MultipleHeadersAndData(t *testing.T) {
247247
// Add a few more invalid blobs at the end
248248
blobs = append(blobs, invalidBlob, []byte{})
249249

250-
mockDAClient.On("GetIDs", mock.Anything, daHeight, []byte("placeholder")).Return(&coreda.GetIDsResult{
250+
mockDAClient.On("GetIDs", mock.Anything, daHeight).Return(&coreda.GetIDsResult{
251251
IDs: []coreda.ID{[]byte("dummy-id")},
252252
Timestamp: time.Now(),
253253
}, nil).Once()
254-
mockDAClient.On("Get", mock.Anything, []coreda.ID{[]byte("dummy-id")}, []byte("placeholder")).Return(
254+
mockDAClient.On("Get", mock.Anything, []coreda.ID{[]byte("dummy-id")}).Return(
255255
blobs, nil,
256256
).Once()
257257

@@ -311,11 +311,11 @@ func TestProcessNextDAHeaderAndData_NotFound(t *testing.T) {
311311
defer cancel()
312312

313313
// Mock GetIDs to return empty IDs to simulate "not found" scenario
314-
mockDAClient.On("GetIDs", mock.Anything, daHeight, []byte("placeholder")).Return(&coreda.GetIDsResult{
315-
IDs: []coreda.ID{}, // Empty IDs array
314+
// Example updates needed for one instance:
315+
mockDAClient.On("GetIDs", mock.Anything, daHeight).Return(&coreda.GetIDsResult{
316+
IDs: []coreda.ID{},
316317
Timestamp: time.Now(),
317318
}, coreda.ErrBlobNotFound).Once()
318-
319319
ctx := context.Background()
320320
err := manager.processNextDAHeaderAndData(ctx)
321321
require.NoError(t, err)
@@ -345,13 +345,13 @@ func TestProcessNextDAHeaderAndData_UnmarshalHeaderError(t *testing.T) {
345345
invalidBytes := []byte("this is not a valid protobuf message")
346346

347347
// Mock GetIDs to return success with dummy ID
348-
mockDAClient.On("GetIDs", mock.Anything, daHeight, []byte("placeholder")).Return(&coreda.GetIDsResult{
348+
mockDAClient.On("GetIDs", mock.Anything, daHeight).Return(&coreda.GetIDsResult{
349349
IDs: []coreda.ID{[]byte("dummy-id")},
350350
Timestamp: time.Now(),
351351
}, nil).Once()
352352

353353
// Mock Get to return invalid bytes
354-
mockDAClient.On("Get", mock.Anything, []coreda.ID{[]byte("dummy-id")}, []byte("placeholder")).Return(
354+
mockDAClient.On("Get", mock.Anything, []coreda.ID{[]byte("dummy-id")}).Return(
355355
[]coreda.Blob{invalidBytes}, nil,
356356
).Once()
357357

@@ -404,13 +404,13 @@ func TestProcessNextDAHeader_UnexpectedSequencer(t *testing.T) {
404404
require.NoError(t, err)
405405

406406
// Mock GetIDs to return success with dummy ID
407-
mockDAClient.On("GetIDs", mock.Anything, daHeight, []byte("placeholder")).Return(&coreda.GetIDsResult{
407+
mockDAClient.On("GetIDs", mock.Anything, daHeight).Return(&coreda.GetIDsResult{
408408
IDs: []coreda.ID{[]byte("dummy-id")},
409409
Timestamp: time.Now(),
410410
}, nil).Once()
411411

412412
// Mock Get to return header bytes
413-
mockDAClient.On("Get", mock.Anything, []coreda.ID{[]byte("dummy-id")}, []byte("placeholder")).Return(
413+
mockDAClient.On("Get", mock.Anything, []coreda.ID{[]byte("dummy-id")}).Return(
414414
[]coreda.Blob{headerBytes}, nil,
415415
).Once()
416416

@@ -448,7 +448,7 @@ func TestProcessNextDAHeader_FetchError_RetryFailure(t *testing.T) {
448448
fetchErr := errors.New("persistent DA connection error")
449449

450450
// Mock GetIDs to return error for all retries
451-
mockDAClient.On("GetIDs", mock.Anything, daHeight, []byte("placeholder")).Return(
451+
mockDAClient.On("GetIDs", mock.Anything, daHeight).Return(
452452
nil, fetchErr,
453453
).Times(dAFetcherRetries)
454454

@@ -538,7 +538,7 @@ func TestProcessNextDAHeader_HeaderAndDataAlreadySeen(t *testing.T) {
538538
Timestamp: time.Now(),
539539
}, nil).Once()
540540

541-
mockDAClient.On("Get", mock.Anything, []coreda.ID{[]byte("dummy-id")}, mock.Anything).Return(
541+
mockDAClient.On("Get", mock.Anything, []coreda.ID{[]byte("dummy-id")}).Return(
542542
[]coreda.Blob{headerBytes, blockDataBytes}, nil,
543543
).Once()
544544

@@ -576,12 +576,12 @@ func TestRetrieveLoop_ProcessError_HeightFromFuture(t *testing.T) {
576576
futureErr := fmt.Errorf("some error wrapping: %w", ErrHeightFromFutureStr)
577577

578578
// Mock GetIDs to return future error for all retries
579-
mockDAClient.On("GetIDs", mock.Anything, startDAHeight, []byte("placeholder")).Return(
579+
mockDAClient.On("GetIDs", mock.Anything, startDAHeight).Return(
580580
nil, futureErr,
581581
).Once()
582582

583583
// Optional: Mock for the next height if needed
584-
mockDAClient.On("GetIDs", mock.Anything, startDAHeight+1, []byte("placeholder")).Return(
584+
mockDAClient.On("GetIDs", mock.Anything, startDAHeight+1).Return(
585585
&coreda.GetIDsResult{IDs: []coreda.ID{}}, coreda.ErrBlobNotFound,
586586
).Maybe()
587587

@@ -624,7 +624,7 @@ func TestRetrieveLoop_ProcessError_Other(t *testing.T) {
624624
otherErr := errors.New("some other DA error")
625625

626626
// Mock GetIDs to return error for all retries
627-
mockDAClient.On("GetIDs", mock.Anything, startDAHeight, []byte("placeholder")).Return(
627+
mockDAClient.On("GetIDs", mock.Anything, startDAHeight).Return(
628628
nil, otherErr,
629629
).Times(dAFetcherRetries)
630630

@@ -708,11 +708,11 @@ func TestProcessNextDAHeader_WithNoTxs(t *testing.T) {
708708
emptyDataBytes, err := emptySignedData.MarshalBinary()
709709
require.NoError(t, err)
710710

711-
mockDAClient.On("GetIDs", mock.Anything, daHeight, []byte("placeholder")).Return(&coreda.GetIDsResult{
711+
mockDAClient.On("GetIDs", mock.Anything, daHeight).Return(&coreda.GetIDsResult{
712712
IDs: []coreda.ID{[]byte("dummy-id")},
713713
Timestamp: time.Now(),
714714
}, nil).Once()
715-
mockDAClient.On("Get", mock.Anything, []coreda.ID{[]byte("dummy-id")}, []byte("placeholder")).Return(
715+
mockDAClient.On("Get", mock.Anything, []coreda.ID{[]byte("dummy-id")}).Return(
716716
[]coreda.Blob{headerBytes, emptyDataBytes}, nil,
717717
).Once()
718718

@@ -761,23 +761,23 @@ func TestRetrieveLoop_DAHeightIncrementsOnlyOnSuccess(t *testing.T) {
761761
require.NoError(t, err)
762762

763763
// 1. First call: success (header)
764-
mockDAClient.On("GetIDs", mock.Anything, startDAHeight, []byte("placeholder")).Return(&coreda.GetIDsResult{
764+
mockDAClient.On("GetIDs", mock.Anything, startDAHeight).Return(&coreda.GetIDsResult{
765765
IDs: []coreda.ID{[]byte("dummy-id")},
766766
Timestamp: time.Now(),
767767
}, nil).Once()
768-
mockDAClient.On("Get", mock.Anything, []coreda.ID{[]byte("dummy-id")}, []byte("placeholder")).Return(
768+
mockDAClient.On("Get", mock.Anything, []coreda.ID{[]byte("dummy-id")}).Return(
769769
[]coreda.Blob{headerBytes}, nil,
770770
).Once()
771771

772772
// 2. Second call: NotFound
773-
mockDAClient.On("GetIDs", mock.Anything, startDAHeight+1, []byte("placeholder")).Return(&coreda.GetIDsResult{
773+
mockDAClient.On("GetIDs", mock.Anything, startDAHeight+1).Return(&coreda.GetIDsResult{
774774
IDs: nil,
775775
Timestamp: time.Now(),
776776
}, nil).Once()
777777

778778
// 3. Third call: Error
779779
errDA := errors.New("some DA error")
780-
mockDAClient.On("GetIDs", mock.Anything, startDAHeight+2, []byte("placeholder")).Return(
780+
mockDAClient.On("GetIDs", mock.Anything, startDAHeight+2).Return(
781781
&coreda.GetIDsResult{
782782
IDs: nil,
783783
Timestamp: time.Now(),

block/submitter_test.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ func TestSubmitDataToDA_Success(t *testing.T) {
5050

5151
// Simulate DA success
5252
da.On("GasMultiplier", mock.Anything).Return(2.0, nil)
53-
da.On("SubmitWithOptions", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).
53+
da.On("Submit", mock.Anything, mock.Anything, mock.Anything, mock.Anything).
5454
Return([]coreda.ID{[]byte("id")}, nil)
5555

5656
pubKey, err := m.signer.GetPublic()
@@ -102,7 +102,7 @@ func TestSubmitDataToDA_Failure(t *testing.T) {
102102
var gasPriceHistory []float64
103103
da.ExpectedCalls = nil
104104
da.On("GasMultiplier", mock.Anything).Return(2.0, nil)
105-
da.On("SubmitWithOptions", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).
105+
da.On("Submit", mock.Anything, mock.Anything, mock.Anything, mock.Anything).
106106
Run(func(args mock.Arguments) { gasPriceHistory = append(gasPriceHistory, args.Get(2).(float64)) }). //save the gas price to verify it later
107107
Return(nil, tc.daError)
108108

@@ -157,7 +157,7 @@ func TestSubmitHeadersToDA_Success(t *testing.T) {
157157
fillWithBlockData(context.Background(), t, m.pendingHeaders, "Test Submitting Headers")
158158

159159
// Simulate DA layer successfully accepting the header submission
160-
da.On("SubmitWithOptions", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).
160+
da.On("Submit", mock.Anything, mock.Anything, mock.Anything, mock.Anything).
161161
Return([]coreda.ID{[]byte("id")}, nil)
162162

163163
// Call submitHeadersToDA and expect no error
@@ -189,7 +189,7 @@ func TestSubmitHeadersToDA_Failure(t *testing.T) {
189189
da.ExpectedCalls = nil
190190
// Simulate DA layer returning a specific error
191191
var gasPriceHistory []float64
192-
da.On("SubmitWithOptions", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).
192+
da.On("Submit", mock.Anything, mock.Anything, mock.Anything, mock.Anything).
193193
Run(func(args mock.Arguments) { gasPriceHistory = append(gasPriceHistory, args.Get(2).(float64)) }). //save the gas price to verify it later
194194
Return(nil, tc.daError)
195195

@@ -226,7 +226,7 @@ func TestSubmitHeadersToDA_WithMetricsRecorder(t *testing.T) {
226226
fillWithBlockData(context.Background(), t, m.pendingHeaders, "Test Submitting Headers")
227227

228228
// Simulate DA layer successfully accepting the header submission
229-
da.On("SubmitWithOptions", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).
229+
da.On("Submit", mock.Anything, mock.Anything, mock.Anything, mock.Anything).
230230
Return([]coreda.ID{[]byte("id")}, nil)
231231

232232
// Expect RecordMetrics to be called with the correct parameters
@@ -262,7 +262,7 @@ func TestSubmitDataToDA_WithMetricsRecorder(t *testing.T) {
262262

263263
// Simulate DA success
264264
da.On("GasMultiplier", mock.Anything).Return(2.0, nil)
265-
da.On("SubmitWithOptions", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).
265+
da.On("Submit", mock.Anything, mock.Anything, mock.Anything, mock.Anything).
266266
Return([]coreda.ID{[]byte("id")}, nil)
267267

268268
// Expect RecordMetrics to be called with the correct parameters

core/da/da.go

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -13,28 +13,25 @@ type DA interface {
1313
//
1414
// Error should be returned if ID is not formatted properly, there is no Blob for given ID or any other client-level
1515
// error occurred (dropped connection, timeout, etc).
16-
Get(ctx context.Context, ids []ID, namespace []byte) ([]Blob, error)
16+
Get(ctx context.Context, ids []ID) ([]Blob, error)
1717

1818
// GetIDs returns IDs of all Blobs located in DA at given height.
19-
GetIDs(ctx context.Context, height uint64, namespace []byte) (*GetIDsResult, error)
19+
GetIDs(ctx context.Context, height uint64) (*GetIDsResult, error)
2020

2121
// GetProofs returns inclusion Proofs for Blobs specified by their IDs.
22-
GetProofs(ctx context.Context, ids []ID, namespace []byte) ([]Proof, error)
22+
GetProofs(ctx context.Context, ids []ID) ([]Proof, error)
2323

2424
// Commit creates a Commitment for each given Blob.
25-
Commit(ctx context.Context, blobs []Blob, namespace []byte) ([]Commitment, error)
25+
Commit(ctx context.Context, blobs []Blob) ([]Commitment, error)
2626

27-
// Submit submits the Blobs to Data Availability layer.
27+
// Submit submits the Blobs to Data Availability layer with additional options.
2828
//
2929
// This method is synchronous. Upon successful submission to Data Availability layer, it returns the IDs identifying blobs
3030
// in DA.
31-
Submit(ctx context.Context, blobs []Blob, gasPrice float64, namespace []byte) ([]ID, error)
32-
33-
// SubmitWithOptions submits the Blobs to Data Availability layer with additional options.
34-
SubmitWithOptions(ctx context.Context, blobs []Blob, gasPrice float64, namespace []byte, options []byte) ([]ID, error)
31+
Submit(ctx context.Context, blobs []Blob, gasPrice float64, options []byte) ([]ID, error)
3532

3633
// Validate validates Commitments against the corresponding Proofs. This should be possible without retrieving the Blobs.
37-
Validate(ctx context.Context, ids []ID, proofs []Proof, namespace []byte) ([]bool, error)
34+
Validate(ctx context.Context, ids []ID, proofs []Proof) ([]bool, error)
3835

3936
// GasPrice returns the gas price for the DA layer.
4037
GasPrice(ctx context.Context) (float64, error)

0 commit comments

Comments
 (0)