Skip to content

Commit 7d85ee0

Browse files
feat: Allow multiple data submission per PFB (#2334)
<!-- 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 - Allows multiple signed data to be submitted as blobs in one PFB - Gets rid of batchSubmissionChan in favor of a pending headers style struct called pending data which allows persistence of unsubmitted data to DA between restarts as well - Refactors pending data and pending headers around a pending base generic struct - Refactors DA submission logic for headers and data to improve code maintainability and reduce duplication - Removes dataCommitmentToHeight struct and simplifies syncing logic - Fixes a bug in makeEmptyHeaderHash along with adding a corresponding test for it - Modify MaxPendingHeaders to MaxPendingHeadersAndData and modified block production to halt if we reach MaxPendingHeadersAndData number of header or data not submitted to DA <!-- Please provide an explanation of the PR, including the appropriate context, background, goal, and rationale. If there is an issue with this information, please provide a tl;dr and link the issue. Ex: Closes #<issue number> --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Introduced unified logic to manage both pending headers and data, improving block production control and reliability. - Added persistent, ordered tracking for data awaiting submission, ensuring atomic and consistent data availability submissions. - Added a dedicated data submission loop alongside the header submission loop for data availability. - **Bug Fixes** - Improved handling of empty data blocks and header/data arrival order during synchronization, reducing risk of missed or duplicated processing. - **Refactor** - Consolidated and generalized data and header submission logic, reducing code duplication and improving maintainability. - Simplified sequencer and cache implementations by removing unused fields and methods. - Refactored pending headers to reuse generic base logic, enhancing modularity. - Removed batch submission channels from sequencers and dummy sequencers, streamlining batch handling. - **Tests** - Expanded and refactored test suites for pending data/headers and submission logic, enhancing coverage and robustness. - Consolidated sync tests to cover multiple headers arriving before data in realistic scenarios. - Introduced generic, reusable test cases for submission success, failure, and retry scenarios. - **Chores** - Updated configuration and command-line flags to reflect the new combined pending headers and data limit. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Marko <marko@baricevic.me>
1 parent e4e4881 commit 7d85ee0

28 files changed

Lines changed: 1384 additions & 1156 deletions

block/da_includer_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -418,8 +418,8 @@ func TestIncrementDAIncludedHeight_WithMetricsRecorder(t *testing.T) {
418418
lastSubmittedBytes := make([]byte, 8)
419419
binary.LittleEndian.PutUint64(lastSubmittedBytes, startDAIncludedHeight)
420420

421-
store.On("GetMetadata", mock.Anything, LastSubmittedHeightKey).Return(lastSubmittedBytes, nil).Maybe() // For pendingHeaders init
422-
store.On("Height", mock.Anything).Return(uint64(7), nil).Maybe() // 7 - 4 = 3 pending headers
421+
store.On("GetMetadata", mock.Anything, LastSubmittedHeaderHeightKey).Return(lastSubmittedBytes, nil).Maybe() // For pendingHeaders init
422+
store.On("Height", mock.Anything).Return(uint64(7), nil).Maybe() // 7 - 4 = 3 pending headers
423423

424424
// Initialize pendingHeaders properly
425425
pendingHeaders, err := NewPendingHeaders(store, logger)

block/manager.go

Lines changed: 10 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,7 @@ type Manager struct {
147147
txsAvailable bool
148148

149149
pendingHeaders *PendingHeaders
150+
pendingData *PendingData
150151

151152
// for reporting metrics
152153
metrics *Metrics
@@ -170,13 +171,6 @@ type Manager struct {
170171
// txNotifyCh is used to signal when new transactions are available
171172
txNotifyCh chan struct{}
172173

173-
// batchSubmissionChan is used to submit batches to the sequencer
174-
batchSubmissionChan chan coresequencer.Batch
175-
176-
// dataCommitmentToHeight tracks the height a data commitment (data hash) has been seen on.
177-
// Key: data commitment (string), Value: uint64 (height)
178-
dataCommitmentToHeight sync.Map
179-
180174
// signaturePayloadProvider is used to provide a signature payload for the header.
181175
// It is used to sign the header with the provided signer.
182176
signaturePayloadProvider types.SignaturePayloadProvider
@@ -344,6 +338,11 @@ func NewManager(
344338
return nil, err
345339
}
346340

341+
pendingData, err := NewPendingData(store, logger)
342+
if err != nil {
343+
return nil, err
344+
}
345+
347346
// If lastBatchHash is not set, retrieve the last batch hash from store
348347
lastBatchDataBytes, err := store.GetMetadata(ctx, LastBatchDataKey)
349348
if err != nil && s.LastBlockHeight > 0 {
@@ -383,14 +382,14 @@ func NewManager(
383382
logger: logger,
384383
txsAvailable: false,
385384
pendingHeaders: pendingHeaders,
385+
pendingData: pendingData,
386386
metrics: seqMetrics,
387387
sequencer: sequencer,
388388
exec: exec,
389389
da: da,
390390
gasPrice: gasPrice,
391391
gasMultiplier: gasMultiplier,
392392
txNotifyCh: make(chan struct{}, 1), // Non-blocking channel
393-
batchSubmissionChan: make(chan coresequencer.Batch, eventInChLength),
394393
signaturePayloadProvider: signaturePayloadProvider,
395394
}
396395

@@ -401,11 +400,6 @@ func NewManager(
401400

402401
// Set the default publishBlock implementation
403402
m.publishBlock = m.publishBlockInternal
404-
if s, ok := m.sequencer.(interface {
405-
SetBatchSubmissionChan(chan coresequencer.Batch)
406-
}); ok {
407-
s.SetBatchSubmissionChan(m.batchSubmissionChan)
408-
}
409403

410404
// fetch caches from disks
411405
if err := m.LoadCache(); err != nil {
@@ -552,8 +546,8 @@ func (m *Manager) publishBlockInternal(ctx context.Context) error {
552546
default:
553547
}
554548

555-
if m.config.Node.MaxPendingHeaders != 0 && m.pendingHeaders.numPendingHeaders() >= m.config.Node.MaxPendingHeaders {
556-
m.logger.Warn(fmt.Sprintf("refusing to create block: pending blocks [%d] reached limit [%d]", m.pendingHeaders.numPendingHeaders(), m.config.Node.MaxPendingHeaders))
549+
if m.config.Node.MaxPendingHeadersAndData != 0 && (m.pendingHeaders.numPendingHeaders() >= m.config.Node.MaxPendingHeadersAndData || m.pendingData.numPendingData() >= m.config.Node.MaxPendingHeadersAndData) {
550+
m.logger.Warn(fmt.Sprintf("refusing to create block: pending headers [%d] or data [%d] reached limit [%d]", m.pendingHeaders.numPendingHeaders(), m.pendingData.numPendingData(), m.config.Node.MaxPendingHeadersAndData))
557551
return nil
558552
}
559553

@@ -961,6 +955,7 @@ func (m *Manager) getDataSignature(data *types.Data) (types.Signature, error) {
961955
if err != nil {
962956
return nil, err
963957
}
958+
964959
if m.signer == nil {
965960
return nil, fmt.Errorf("signer is nil; cannot sign data")
966961
}

block/manager_test.go

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,7 @@ func Test_submitBlocksToDA_BlockMarshalErrorCase1(t *testing.T) {
218218

219219
store := mocks.NewStore(t)
220220
invalidateBlockHeader(header1)
221-
store.On("GetMetadata", mock.Anything, LastSubmittedHeightKey).Return(nil, ds.ErrNotFound)
221+
store.On("GetMetadata", mock.Anything, LastSubmittedHeaderHeightKey).Return(nil, ds.ErrNotFound)
222222
store.On("GetBlockData", mock.Anything, uint64(1)).Return(header1, data1, nil)
223223
store.On("GetBlockData", mock.Anything, uint64(2)).Return(header2, data2, nil)
224224
store.On("GetBlockData", mock.Anything, uint64(3)).Return(header3, data3, nil)
@@ -230,7 +230,9 @@ func Test_submitBlocksToDA_BlockMarshalErrorCase1(t *testing.T) {
230230
m.pendingHeaders, err = NewPendingHeaders(store, m.logger)
231231
require.NoError(err)
232232

233-
err = m.submitHeadersToDA(ctx)
233+
headers, err := m.pendingHeaders.getPendingHeaders(ctx)
234+
require.NoError(err)
235+
err = m.submitHeadersToDA(ctx, headers)
234236
assert.ErrorContains(err, "failed to transform header to proto")
235237
blocks, err := m.pendingHeaders.getPendingHeaders(ctx)
236238
assert.NoError(err)
@@ -254,7 +256,7 @@ func Test_submitBlocksToDA_BlockMarshalErrorCase2(t *testing.T) {
254256

255257
store := mocks.NewStore(t)
256258
invalidateBlockHeader(header3)
257-
store.On("GetMetadata", mock.Anything, LastSubmittedHeightKey).Return(nil, ds.ErrNotFound)
259+
store.On("GetMetadata", mock.Anything, LastSubmittedHeaderHeightKey).Return(nil, ds.ErrNotFound)
258260
store.On("GetBlockData", mock.Anything, uint64(1)).Return(header1, data1, nil)
259261
store.On("GetBlockData", mock.Anything, uint64(2)).Return(header2, data2, nil)
260262
store.On("GetBlockData", mock.Anything, uint64(3)).Return(header3, data3, nil)
@@ -265,7 +267,10 @@ func Test_submitBlocksToDA_BlockMarshalErrorCase2(t *testing.T) {
265267
var err error
266268
m.pendingHeaders, err = NewPendingHeaders(store, m.logger)
267269
require.NoError(err)
268-
err = m.submitHeadersToDA(ctx)
270+
271+
headers, err := m.pendingHeaders.getPendingHeaders(ctx)
272+
require.NoError(err)
273+
err = m.submitHeadersToDA(ctx, headers)
269274
assert.ErrorContains(err, "failed to transform header to proto")
270275
blocks, err := m.pendingHeaders.getPendingHeaders(ctx)
271276
assert.NoError(err)

block/pending_base.go

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
package block
2+
3+
import (
4+
"context"
5+
"encoding/binary"
6+
"errors"
7+
"fmt"
8+
"sync/atomic"
9+
10+
"cosmossdk.io/log"
11+
ds "github.com/ipfs/go-datastore"
12+
13+
"github.com/rollkit/rollkit/pkg/store"
14+
)
15+
16+
// pendingBase is a generic struct for tracking items (headers, data, etc.)
17+
// that need to be published to the DA layer in order. It handles persistence
18+
// of the last submitted height and provides methods for retrieving pending items.
19+
type pendingBase[T any] struct {
20+
logger log.Logger
21+
store store.Store
22+
metaKey string
23+
fetch func(ctx context.Context, store store.Store, height uint64) (T, error)
24+
lastHeight atomic.Uint64
25+
}
26+
27+
// newPendingBase constructs a new pendingBase for a given type.
28+
func newPendingBase[T any](store store.Store, logger log.Logger, metaKey string, fetch func(ctx context.Context, store store.Store, height uint64) (T, error)) (*pendingBase[T], error) {
29+
pb := &pendingBase[T]{
30+
store: store,
31+
logger: logger,
32+
metaKey: metaKey,
33+
fetch: fetch,
34+
}
35+
if err := pb.init(); err != nil {
36+
return nil, err
37+
}
38+
return pb, nil
39+
}
40+
41+
// getPending returns a sorted slice of pending items of type T.
42+
func (pb *pendingBase[T]) getPending(ctx context.Context) ([]T, error) {
43+
lastSubmitted := pb.lastHeight.Load()
44+
height, err := pb.store.Height(ctx)
45+
if err != nil {
46+
return nil, err
47+
}
48+
if lastSubmitted == height {
49+
return nil, nil
50+
}
51+
if lastSubmitted > height {
52+
return nil, fmt.Errorf("height of last submitted item (%d) is greater than height of last item (%d)", lastSubmitted, height)
53+
}
54+
pending := make([]T, 0, height-lastSubmitted)
55+
for i := lastSubmitted + 1; i <= height; i++ {
56+
item, err := pb.fetch(ctx, pb.store, i)
57+
if err != nil {
58+
return pending, err
59+
}
60+
pending = append(pending, item)
61+
}
62+
return pending, nil
63+
}
64+
65+
func (pb *pendingBase[T]) isEmpty() bool {
66+
height, err := pb.store.Height(context.Background())
67+
if err != nil {
68+
pb.logger.Error("failed to get height in isEmpty", "err", err)
69+
return false
70+
}
71+
return height == pb.lastHeight.Load()
72+
}
73+
74+
func (pb *pendingBase[T]) numPending() uint64 {
75+
height, err := pb.store.Height(context.Background())
76+
if err != nil {
77+
pb.logger.Error("failed to get height in numPending", "err", err)
78+
return 0
79+
}
80+
return height - pb.lastHeight.Load()
81+
}
82+
83+
func (pb *pendingBase[T]) setLastSubmittedHeight(ctx context.Context, newLastSubmittedHeight uint64) {
84+
lsh := pb.lastHeight.Load()
85+
if newLastSubmittedHeight > lsh && pb.lastHeight.CompareAndSwap(lsh, newLastSubmittedHeight) {
86+
bz := make([]byte, 8)
87+
binary.LittleEndian.PutUint64(bz, newLastSubmittedHeight)
88+
err := pb.store.SetMetadata(ctx, pb.metaKey, bz)
89+
if err != nil {
90+
pb.logger.Error("failed to store height of latest item submitted to DA", "err", err)
91+
}
92+
}
93+
}
94+
95+
func (pb *pendingBase[T]) init() error {
96+
raw, err := pb.store.GetMetadata(context.Background(), pb.metaKey)
97+
if errors.Is(err, ds.ErrNotFound) {
98+
return nil
99+
}
100+
if err != nil {
101+
return err
102+
}
103+
if len(raw) != 8 {
104+
return fmt.Errorf("invalid length of last submitted height: %d, expected 8", len(raw))
105+
}
106+
lsh := binary.LittleEndian.Uint64(raw)
107+
if lsh == 0 {
108+
return nil
109+
}
110+
pb.lastHeight.CompareAndSwap(0, lsh)
111+
return nil
112+
}

0 commit comments

Comments
 (0)