Skip to content

Commit 007ac1c

Browse files
julienrbrtjulienrbrt
committed
feedback + fix in syncer
1 parent f2e2817 commit 007ac1c

9 files changed

Lines changed: 148 additions & 58 deletions

File tree

‎block/internal/cache/generic_cache.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -126,8 +126,8 @@ func (c *Cache[T]) setSeen(hash string, height uint64) {
126126
}
127127

128128
// getDAIncluded returns the DA height if the hash has been DA-included.
129-
func (c *Cache[T]) getDAIncluded(hash string) (uint64, bool) {
130-
return c.daIncluded.Get(hash)
129+
func (c *Cache[T]) getDAIncluded(daCommitmentHash string) (uint64, bool) {
130+
return c.daIncluded.Get(daCommitmentHash)
131131
}
132132

133133
// getDAIncludedByHeight resolves DA height via the height→hash index.

‎block/internal/cache/manager.go‎

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -34,20 +34,17 @@ type CacheManager interface {
3434
// Header operations
3535
IsHeaderSeen(hash string) bool
3636
SetHeaderSeen(hash string, blockHeight uint64)
37-
GetHeaderDAIncluded(hash string) (uint64, bool)
38-
// GetHeaderDAIncludedByHeight looks up DA height via the height→hash index.
39-
// Works for both real hashes (steady state) and snapshot placeholders
40-
// (post-restart, before the DA retriever re-fires the real hash).
37+
GetHeaderDAIncludedByHash(hash string) (uint64, bool)
4138
GetHeaderDAIncludedByHeight(blockHeight uint64) (uint64, bool)
4239
SetHeaderDAIncluded(hash string, daHeight uint64, blockHeight uint64)
4340
RemoveHeaderDAIncluded(hash string)
4441

4542
// Data operations
4643
IsDataSeen(hash string) bool
4744
SetDataSeen(hash string, blockHeight uint64)
48-
GetDataDAIncluded(hash string) (uint64, bool)
45+
GetDataDAIncludedByHash(daCommitmentHash string) (uint64, bool)
4946
GetDataDAIncludedByHeight(blockHeight uint64) (uint64, bool)
50-
SetDataDAIncluded(hash string, daHeight uint64, blockHeight uint64)
47+
SetDataDAIncluded(daCommitmentHash string, daHeight uint64, blockHeight uint64)
5148
RemoveDataDAIncluded(hash string)
5249

5350
// Transaction operations
@@ -153,7 +150,7 @@ func (m *implementation) SetHeaderSeen(hash string, blockHeight uint64) {
153150
m.headerCache.setSeen(hash, blockHeight)
154151
}
155152

156-
func (m *implementation) GetHeaderDAIncluded(hash string) (uint64, bool) {
153+
func (m *implementation) GetHeaderDAIncludedByHash(hash string) (uint64, bool) {
157154
return m.headerCache.getDAIncluded(hash)
158155
}
159156

@@ -182,16 +179,16 @@ func (m *implementation) SetDataSeen(hash string, blockHeight uint64) {
182179
m.dataCache.setSeen(hash, blockHeight)
183180
}
184181

185-
func (m *implementation) GetDataDAIncluded(hash string) (uint64, bool) {
186-
return m.dataCache.getDAIncluded(hash)
182+
func (m *implementation) GetDataDAIncludedByHash(daCommitmentHash string) (uint64, bool) {
183+
return m.dataCache.getDAIncluded(daCommitmentHash)
187184
}
188185

189186
func (m *implementation) GetDataDAIncludedByHeight(blockHeight uint64) (uint64, bool) {
190187
return m.dataCache.getDAIncludedByHeight(blockHeight)
191188
}
192189

193-
func (m *implementation) SetDataDAIncluded(hash string, daHeight uint64, blockHeight uint64) {
194-
m.dataCache.setDAIncluded(hash, daHeight, blockHeight)
190+
func (m *implementation) SetDataDAIncluded(daCommitmentHash string, daHeight uint64, blockHeight uint64) {
191+
m.dataCache.setDAIncluded(daCommitmentHash, daHeight, blockHeight)
195192
}
196193

197194
func (m *implementation) RemoveDataDAIncluded(hash string) {

‎block/internal/cache/manager_test.go‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -46,9 +46,9 @@ func TestManager_HeaderDataOperations(t *testing.T) {
4646

4747
m.SetHeaderDAIncluded("h1", 10, 2)
4848
m.SetDataDAIncluded("d1", 11, 2)
49-
_, ok := m.GetHeaderDAIncluded("h1")
49+
_, ok := m.GetHeaderDAIncludedByHeight(2)
5050
assert.True(t, ok)
51-
_, ok = m.GetDataDAIncluded("d1")
51+
_, ok = m.GetDataDAIncludedByHeight(2)
5252
assert.True(t, ok)
5353
}
5454

@@ -143,7 +143,7 @@ func TestManager_SaveAndRestoreFromStore(t *testing.T) {
143143
require.NoError(t, err)
144144

145145
// Height 1 is finalized (DAIncludedHeight = 1): IsHeightDAIncluded returns true
146-
// via the height comparison, so GetHeaderDAIncluded is never consulted for it.
146+
// via the height comparison, so GetHeaderDAIncludedByHash is never consulted for it.
147147
// The cache entry is not restored — this is correct and intentional.
148148

149149
// Height 2 is in-flight: the window restore loads a placeholder entry keyed by
@@ -157,11 +157,11 @@ func TestManager_SaveAndRestoreFromStore(t *testing.T) {
157157
m2.SetHeaderDAIncluded(h2.Hash().String(), 101, 2)
158158
m2.SetDataDAIncluded(d2.DACommitment().String(), 101, 2)
159159

160-
daHeight, ok := m2.GetHeaderDAIncluded(h2.Hash().String())
160+
daHeight, ok := m2.GetHeaderDAIncludedByHeight(2)
161161
assert.True(t, ok)
162162
assert.Equal(t, uint64(101), daHeight)
163163

164-
daHeight, ok = m2.GetDataDAIncluded(d2.DACommitment().String())
164+
daHeight, ok = m2.GetDataDAIncludedByHeight(2)
165165
assert.True(t, ok)
166166
assert.Equal(t, uint64(101), daHeight)
167167
}
@@ -471,11 +471,11 @@ func TestManager_DAInclusionPersistence(t *testing.T) {
471471
m2.SetHeaderDAIncluded(headerHash, 100, 1)
472472
m2.SetDataDAIncluded(dataHash, 101, 1)
473473

474-
daHeight, ok := m2.GetHeaderDAIncluded(headerHash)
474+
daHeight, ok := m2.GetHeaderDAIncludedByHeight(1)
475475
assert.True(t, ok)
476476
assert.Equal(t, uint64(100), daHeight)
477477

478-
daHeight, ok = m2.GetDataDAIncluded(dataHash)
478+
daHeight, ok = m2.GetDataDAIncludedByHeight(1)
479479
assert.True(t, ok)
480480
assert.Equal(t, uint64(101), daHeight)
481481
}

‎block/internal/submitting/da_submitter_integration_test.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -108,13 +108,13 @@ func TestDASubmitter_SubmitHeadersAndData_MarksInclusionAndUpdatesLastSubmitted(
108108
require.NoError(t, daSubmitter.SubmitData(context.Background(), dataList, marshalledData, cm, n, gen))
109109

110110
// After submission, inclusion markers should be set
111-
_, ok := cm.GetHeaderDAIncluded(hdr1.Hash().String())
111+
_, ok := cm.GetHeaderDAIncludedByHeight(1)
112112
assert.True(t, ok)
113-
_, ok = cm.GetHeaderDAIncluded(hdr2.Hash().String())
113+
_, ok = cm.GetHeaderDAIncludedByHeight(2)
114114
assert.True(t, ok)
115-
_, ok = cm.GetDataDAIncluded(data1.DACommitment().String())
115+
_, ok = cm.GetDataDAIncludedByHeight(1)
116116
assert.True(t, ok)
117-
_, ok = cm.GetDataDAIncluded(data2.DACommitment().String())
117+
_, ok = cm.GetDataDAIncludedByHeight(2)
118118
assert.True(t, ok)
119119

120120
}

‎block/internal/submitting/da_submitter_test.go‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -222,11 +222,9 @@ func TestDASubmitter_SubmitHeaders_Success(t *testing.T) {
222222
require.NoError(t, err)
223223

224224
// Verify headers are marked as DA included
225-
hash1 := header1.Hash().String()
226-
hash2 := header2.Hash().String()
227-
_, ok1 := cm.GetHeaderDAIncluded(hash1)
225+
_, ok1 := cm.GetHeaderDAIncludedByHeight(1)
228226
assert.True(t, ok1)
229-
_, ok2 := cm.GetHeaderDAIncluded(hash2)
227+
_, ok2 := cm.GetHeaderDAIncludedByHeight(2)
230228
assert.True(t, ok2)
231229
}
232230

@@ -339,9 +337,9 @@ func TestDASubmitter_SubmitData_Success(t *testing.T) {
339337
require.NoError(t, err)
340338

341339
// Verify data is marked as DA included
342-
_, ok := cm.GetDataDAIncluded(data1.DACommitment().String())
340+
_, ok := cm.GetDataDAIncludedByHeight(1)
343341
assert.True(t, ok)
344-
_, ok = cm.GetDataDAIncluded(data2.DACommitment().String())
342+
_, ok = cm.GetDataDAIncludedByHeight(2)
345343
assert.True(t, ok)
346344
}
347345

@@ -394,7 +392,7 @@ func TestDASubmitter_SubmitData_SkipsEmptyData(t *testing.T) {
394392
mockDA.AssertNotCalled(t, "Submit", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything)
395393

396394
// Empty data should not be marked as DA included (it's implicitly included)
397-
_, ok := cm.GetDataDAIncluded(emptyData.DACommitment().String())
395+
_, ok := cm.GetDataDAIncludedByHash(emptyData.DACommitment().String())
398396
assert.False(t, ok)
399397
}
400398

‎block/internal/submitting/submitter_test.go‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,7 @@ func TestSubmitter_setNodeHeightToDAHeight_Errors(t *testing.T) {
211211
h, d := newHeaderAndData("chain", 1, true)
212212

213213
// No cache entries -> expect error on missing header
214-
_, ok := cm.GetHeaderDAIncluded(h.Hash().String())
214+
_, ok := cm.GetHeaderDAIncludedByHash(h.Hash().String())
215215
assert.False(t, ok)
216216
assert.Error(t, s.setNodeHeightToDAHeight(ctx, 1, d, false))
217217

@@ -537,10 +537,10 @@ func TestSubmitter_CacheClearedOnHeightInclusion(t *testing.T) {
537537
assert.False(t, cm.IsDataSeen(d2.DACommitment().String()), "height 2 data should be cleared from cache")
538538

539539
// Verify DA inclusion status is removed for processed heights (cleaned up after finalization)
540-
_, h1DAIncluded := cm.GetHeaderDAIncluded(h1.Hash().String())
541-
_, d1DAIncluded := cm.GetDataDAIncluded(d1.DACommitment().String())
542-
_, h2DAIncluded := cm.GetHeaderDAIncluded(h2.Hash().String())
543-
_, d2DAIncluded := cm.GetDataDAIncluded(d2.DACommitment().String())
540+
_, h1DAIncluded := cm.GetHeaderDAIncludedByHash(h1.Hash().String())
541+
_, d1DAIncluded := cm.GetDataDAIncludedByHash(d1.DACommitment().String())
542+
_, h2DAIncluded := cm.GetHeaderDAIncludedByHash(h2.Hash().String())
543+
_, d2DAIncluded := cm.GetDataDAIncludedByHash(d2.DACommitment().String())
544544
assert.False(t, h1DAIncluded, "height 1 header DA inclusion status should be removed after finalization")
545545
assert.False(t, d1DAIncluded, "height 1 data DA inclusion status should be removed after finalization")
546546
assert.False(t, h2DAIncluded, "height 2 header DA inclusion status should be removed after finalization")
@@ -551,8 +551,8 @@ func TestSubmitter_CacheClearedOnHeightInclusion(t *testing.T) {
551551
assert.True(t, cm.IsDataSeen(d3.DACommitment().String()), "height 3 data should remain in cache")
552552

553553
// Verify height 3 has no DA inclusion status since it wasn't processed
554-
_, h3DAIncluded := cm.GetHeaderDAIncluded(h3.Hash().String())
555-
_, d3DAIncluded := cm.GetDataDAIncluded(d3.DACommitment().String())
554+
_, h3DAIncluded := cm.GetHeaderDAIncludedByHash(h3.Hash().String())
555+
_, d3DAIncluded := cm.GetDataDAIncludedByHash(d3.DACommitment().String())
556556
assert.False(t, h3DAIncluded, "height 3 header should not have DA inclusion status")
557557
assert.False(t, d3DAIncluded, "height 3 data should not have DA inclusion status")
558558
}
@@ -645,7 +645,7 @@ func TestSubmitter_IsHeightDAIncluded_AfterRestart(t *testing.T) {
645645
// ── Step 3: check IsHeightDAIncluded BEFORE DA retriever re-fires ─────────
646646
// Height 3 is in-flight: above daIncludedHeight (2) so we can't short-circuit.
647647
// The real hashes are NOT in cm2 yet — only the snapshot placeholders are.
648-
_, realHeaderFound := cm2.GetHeaderDAIncluded(h3.Hash().String())
648+
_, realHeaderFound := cm2.GetHeaderDAIncludedByHash(h3.Hash().String())
649649
assert.False(t, realHeaderFound, "real hash must not be present before DA retriever re-fires")
650650

651651
included, err := s.IsHeightDAIncluded(3, d3)
@@ -658,7 +658,7 @@ func TestSubmitter_IsHeightDAIncluded_AfterRestart(t *testing.T) {
658658
cm2.SetHeaderDAIncluded(h3.Hash().String(), 12, 3)
659659
cm2.SetDataDAIncluded(d3.DACommitment().String(), 12, 3)
660660

661-
_, realHeaderFound = cm2.GetHeaderDAIncluded(h3.Hash().String())
661+
_, realHeaderFound = cm2.GetHeaderDAIncludedByHash(h3.Hash().String())
662662
assert.True(t, realHeaderFound, "real hash must be present after DA retriever re-fires")
663663

664664
included, err = s.IsHeightDAIncluded(3, d3)
@@ -715,8 +715,8 @@ func TestSubmitter_setNodeHeightToDAHeight_AfterRestart(t *testing.T) {
715715
require.NoError(t, err)
716716

717717
// Confirm the real content hashes are NOT present after restore.
718-
_, realHdrFound := cm2.GetHeaderDAIncluded(h3.Hash().String())
719-
_, realDataFound := cm2.GetDataDAIncluded(d3.DACommitment().String())
718+
_, realHdrFound := cm2.GetHeaderDAIncludedByHash(h3.Hash().String())
719+
_, realDataFound := cm2.GetDataDAIncludedByHash(d3.DACommitment().String())
720720
require.False(t, realHdrFound, "real header hash must not be in cache before DA retriever re-fires")
721721
require.False(t, realDataFound, "real data hash must not be in cache before DA retriever re-fires")
722722

@@ -732,7 +732,7 @@ func TestSubmitter_setNodeHeightToDAHeight_AfterRestart(t *testing.T) {
732732
}
733733

734734
// ── Step 3: call setNodeHeightToDAHeight — must succeed via fallback ──────
735-
// Before this fix, GetHeaderDAIncluded(realHash) would miss and the function
735+
// Before this fix, GetHeaderDAIncludedByHash(realHash) would miss and the function
736736
// would return an error, stalling processDAInclusionLoop.
737737
err = s.setNodeHeightToDAHeight(ctx, 3, d3, false)
738738
require.NoError(t, err,

‎block/internal/syncing/da_retriever_test.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -161,11 +161,11 @@ func TestDARetriever_ProcessBlobs_HeaderAndData_Success(t *testing.T) {
161161
assert.Equal(t, uint64(2), events[0].Data.Height())
162162
assert.Equal(t, uint64(77), events[0].DaHeight)
163163

164-
hHeight, ok := r.cache.GetHeaderDAIncluded(events[0].Header.Hash().String())
164+
hHeight, ok := r.cache.GetHeaderDAIncludedByHash(events[0].Header.Hash().String())
165165
assert.True(t, ok)
166166
assert.Equal(t, uint64(77), hHeight)
167167

168-
dHeight, ok := r.cache.GetDataDAIncluded(events[0].Data.DACommitment().String())
168+
dHeight, ok := r.cache.GetDataDAIncludedByHash(events[0].Data.DACommitment().String())
169169
assert.True(t, ok)
170170
assert.Equal(t, uint64(77), dHeight)
171171
}
@@ -185,12 +185,12 @@ func TestDARetriever_ProcessBlobs_HeaderOnly_EmptyDataExpected(t *testing.T) {
185185
assert.NotNil(t, events[0].Data)
186186
assert.Equal(t, uint64(88), events[0].DaHeight)
187187

188-
hHeight, ok := r.cache.GetHeaderDAIncluded(events[0].Header.Hash().String())
188+
hHeight, ok := r.cache.GetHeaderDAIncludedByHash(events[0].Header.Hash().String())
189189
assert.True(t, ok)
190190
assert.Equal(t, uint64(88), hHeight)
191191

192192
// empty data is not marked as data included (the submitter component does handle the empty data case)
193-
_, ok = r.cache.GetDataDAIncluded(events[0].Data.DACommitment().String())
193+
_, ok = r.cache.GetDataDAIncludedByHash(events[0].Data.DACommitment().String())
194194
assert.False(t, ok)
195195
}
196196

‎block/internal/syncing/syncer.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -610,20 +610,20 @@ func (s *Syncer) processHeightEvent(ctx context.Context, event *common.DAHeightE
610610
// empty, nothing to do
611611
case event.DaHeightHints[0] == 0:
612612
// check only data
613-
if _, exists := s.cache.GetDataDAIncluded(event.Data.Hash().String()); !exists {
613+
if _, exists := s.cache.GetDataDAIncludedByHeight(height); !exists {
614614
daHeightHints = []uint64{event.DaHeightHints[1]}
615615
}
616616
case event.DaHeightHints[1] == 0:
617617
// check only header
618-
if _, exists := s.cache.GetHeaderDAIncluded(event.Header.Hash().String()); !exists {
618+
if _, exists := s.cache.GetHeaderDAIncludedByHeight(height); !exists {
619619
daHeightHints = []uint64{event.DaHeightHints[0]}
620620
}
621621
default:
622622
// check both
623-
if _, exists := s.cache.GetHeaderDAIncluded(event.Header.Hash().String()); !exists {
623+
if _, exists := s.cache.GetHeaderDAIncludedByHeight(height); !exists {
624624
daHeightHints = []uint64{event.DaHeightHints[0]}
625625
}
626-
if _, exists := s.cache.GetDataDAIncluded(event.Data.Hash().String()); !exists {
626+
if _, exists := s.cache.GetDataDAIncludedByHeight(height); !exists {
627627
daHeightHints = append(daHeightHints, event.DaHeightHints[1])
628628
}
629629
if len(daHeightHints) == 2 && daHeightHints[0] == daHeightHints[1] {

0 commit comments

Comments
 (0)