diff --git a/block/internal/cache/generic_cache.go b/block/internal/cache/generic_cache.go index 0cac0ae215..43a70ccbdb 100644 --- a/block/internal/cache/generic_cache.go +++ b/block/internal/cache/generic_cache.go @@ -16,6 +16,8 @@ type Cache[T any] struct { hashes *sync.Map // daIncluded tracks the DA inclusion height for a given hash daIncluded *sync.Map + // hashByHeight tracks the hash associated with each height for pruning + hashByHeight *sync.Map } // NewCache returns a new Cache struct @@ -24,6 +26,7 @@ func NewCache[T any]() *Cache[T] { itemsByHeight: new(sync.Map), hashes: new(sync.Map), daIncluded: new(sync.Map), + hashByHeight: new(sync.Map), } } @@ -46,22 +49,6 @@ func (c *Cache[T]) setItem(height uint64, item *T) { c.itemsByHeight.Store(height, item) } -// rangeByHeight iterates over items keyed by height in an unspecified order and calls fn for each. -// If fn returns false, iteration stops early. -func (c *Cache[T]) rangeByHeight(fn func(height uint64, item *T) bool) { - c.itemsByHeight.Range(func(k, v any) bool { - height, ok := k.(uint64) - if !ok { - return true - } - it, ok := v.(*T) - if !ok { - return true - } - return fn(height, it) - }) -} - // getNextItem returns the item at the specified height and removes it from cache if found. // Returns nil if not found. func (c *Cache[T]) getNextItem(height uint64) *T { @@ -85,9 +72,10 @@ func (c *Cache[T]) isSeen(hash string) bool { return seen.(bool) } -// setSeen sets the hash as seen -func (c *Cache[T]) setSeen(hash string) { +// setSeen sets the hash as seen and tracks its height for pruning +func (c *Cache[T]) setSeen(hash string, height uint64) { c.hashes.Store(hash, true) + c.hashByHeight.Store(height, hash) } // getDAIncluded returns the DA height if the hash has been DA-included, otherwise it returns 0. @@ -99,9 +87,10 @@ func (c *Cache[T]) getDAIncluded(hash string) (uint64, bool) { return daIncluded.(uint64), true } -// setDAIncluded sets the hash as DA-included with the given DA height -func (c *Cache[T]) setDAIncluded(hash string, daHeight uint64) { +// setDAIncluded sets the hash as DA-included with the given DA height and tracks block height for pruning +func (c *Cache[T]) setDAIncluded(hash string, daHeight uint64, blockHeight uint64) { c.daIncluded.Store(hash, daHeight) + c.hashByHeight.Store(blockHeight, hash) } // removeDAIncluded removes the DA-included status of the hash @@ -109,6 +98,17 @@ func (c *Cache[T]) removeDAIncluded(hash string) { c.daIncluded.Delete(hash) } +// deleteAll removes all items and their associated data from the cache at the given height +func (c *Cache[T]) deleteAllForHeight(height uint64) { + c.itemsByHeight.Delete(height) + hash, ok := c.hashByHeight.Load(height) + if ok { + c.hashes.Delete(hash) + c.hashByHeight.Delete(height) + // c.daIncluded.Delete(hash) // we actually do not want to delete the DA-included status here + } +} + const ( itemsByHeightFilename = "items_by_height.gob" hashesFilename = "hashes.gob" diff --git a/block/internal/cache/generic_cache_test.go b/block/internal/cache/generic_cache_test.go index 11ed249015..f2af03b8e3 100644 --- a/block/internal/cache/generic_cache_test.go +++ b/block/internal/cache/generic_cache_test.go @@ -14,31 +14,14 @@ func init() { gob.Register(&testItem{}) } -// TestCache_TypeSafety ensures methods gracefully handle invalid underlying types. -func TestCache_TypeSafety(t *testing.T) { - c := NewCache[testItem]() - - // Inject invalid value types directly into maps (bypassing typed methods) - c.itemsByHeight.Store(uint64(1), "not-a-*testItem") - - if got := c.getItem(1); got != nil { - t.Fatalf("expected nil for invalid stored type, got %#v", got) - } - - // Range should skip invalid entries and not panic - ran := false - c.rangeByHeight(func(_ uint64, _ *testItem) bool { ran = true; return true }) - _ = ran // ensure no panic -} - // TestCache_SaveLoad_ErrorPaths covers SaveToDisk and LoadFromDisk error scenarios. func TestCache_SaveLoad_ErrorPaths(t *testing.T) { c := NewCache[testItem]() for i := 0; i < 5; i++ { v := &testItem{V: i} c.setItem(uint64(i), v) - c.setSeen(fmt.Sprintf("s%d", i)) - c.setDAIncluded(fmt.Sprintf("d%d", i), uint64(i)) + c.setSeen(fmt.Sprintf("s%d", i), uint64(i)) + c.setDAIncluded(fmt.Sprintf("d%d", i), uint64(i), uint64(i)) } // Normal save/load roundtrip diff --git a/block/internal/cache/manager.go b/block/internal/cache/manager.go index 471ce1d1a3..c63d60fa29 100644 --- a/block/internal/cache/manager.go +++ b/block/internal/cache/manager.go @@ -40,16 +40,16 @@ func registerGobTypes() { type Manager interface { // Header operations IsHeaderSeen(hash string) bool - SetHeaderSeen(hash string) + SetHeaderSeen(hash string, blockHeight uint64) GetHeaderDAIncluded(hash string) (uint64, bool) - SetHeaderDAIncluded(hash string, daHeight uint64) + SetHeaderDAIncluded(hash string, daHeight uint64, blockHeight uint64) RemoveHeaderDAIncluded(hash string) // Data operations IsDataSeen(hash string) bool - SetDataSeen(hash string) + SetDataSeen(hash string, blockHeight uint64) GetDataDAIncluded(hash string) (uint64, bool) - SetDataDAIncluded(hash string, daHeight uint64) + SetDataDAIncluded(hash string, daHeight uint64, blockHeight uint64) // Pending operations GetPendingHeaders(ctx context.Context) ([]*types.SignedHeader, error) @@ -60,13 +60,16 @@ type Manager interface { NumPendingData() uint64 // Pending events syncing coordination - GetNextPendingEvent(height uint64) *common.DAHeightEvent - SetPendingEvent(height uint64, event *common.DAHeightEvent) + GetNextPendingEvent(blockHeight uint64) *common.DAHeightEvent + SetPendingEvent(blockHeight uint64, event *common.DAHeightEvent) - // Cleanup operations + // Disk operations SaveToDisk() error LoadFromDisk() error ClearFromDisk() error + + // Cleanup operations + DeleteHeight(blockHeight uint64) } var _ Manager = (*implementation)(nil) @@ -100,6 +103,7 @@ func NewManager(cfg config.Config, store store.Store, logger zerolog.Logger) (Ma return nil, fmt.Errorf("failed to create pending data: %w", err) } + registerGobTypes() impl := &implementation{ headerCache: headerCache, dataCache: dataCache, @@ -130,16 +134,16 @@ func (m *implementation) IsHeaderSeen(hash string) bool { return m.headerCache.isSeen(hash) } -func (m *implementation) SetHeaderSeen(hash string) { - m.headerCache.setSeen(hash) +func (m *implementation) SetHeaderSeen(hash string, blockHeight uint64) { + m.headerCache.setSeen(hash, blockHeight) } func (m *implementation) GetHeaderDAIncluded(hash string) (uint64, bool) { return m.headerCache.getDAIncluded(hash) } -func (m *implementation) SetHeaderDAIncluded(hash string, daHeight uint64) { - m.headerCache.setDAIncluded(hash, daHeight) +func (m *implementation) SetHeaderDAIncluded(hash string, daHeight uint64, blockHeight uint64) { + m.headerCache.setDAIncluded(hash, daHeight, blockHeight) } func (m *implementation) RemoveHeaderDAIncluded(hash string) { @@ -151,16 +155,24 @@ func (m *implementation) IsDataSeen(hash string) bool { return m.dataCache.isSeen(hash) } -func (m *implementation) SetDataSeen(hash string) { - m.dataCache.setSeen(hash) +func (m *implementation) SetDataSeen(hash string, blockHeight uint64) { + m.dataCache.setSeen(hash, blockHeight) } func (m *implementation) GetDataDAIncluded(hash string) (uint64, bool) { return m.dataCache.getDAIncluded(hash) } -func (m *implementation) SetDataDAIncluded(hash string, daHeight uint64) { - m.dataCache.setDAIncluded(hash, daHeight) +func (m *implementation) SetDataDAIncluded(hash string, daHeight uint64, blockHeight uint64) { + m.dataCache.setDAIncluded(hash, daHeight, blockHeight) +} + +// DeleteHeight removes from all caches the given height. +// This can be done when a height has been da included. +func (m *implementation) DeleteHeight(blockHeight uint64) { + m.headerCache.deleteAllForHeight(blockHeight) + m.dataCache.deleteAllForHeight(blockHeight) + m.pendingEventsCache.deleteAllForHeight(blockHeight) } // Pending operations diff --git a/block/internal/cache/manager_test.go b/block/internal/cache/manager_test.go index 06c93d130d..1cb441db2f 100644 --- a/block/internal/cache/manager_test.go +++ b/block/internal/cache/manager_test.go @@ -39,13 +39,13 @@ func TestManager_HeaderDataOperations(t *testing.T) { require.NoError(t, err) // seen & DA included flags - m.SetHeaderSeen("h1") - m.SetDataSeen("d1") + m.SetHeaderSeen("h1", 1) + m.SetDataSeen("d1", 1) assert.True(t, m.IsHeaderSeen("h1")) assert.True(t, m.IsDataSeen("d1")) - m.SetHeaderDAIncluded("h1", 10) - m.SetDataDAIncluded("d1", 11) + m.SetHeaderDAIncluded("h1", 10, 2) + m.SetDataDAIncluded("d1", 11, 2) _, ok := m.GetHeaderDAIncluded("h1") assert.True(t, ok) _, ok = m.GetDataDAIncluded("d1") @@ -102,10 +102,10 @@ func TestManager_SaveAndLoadFromDisk(t *testing.T) { // populate caches hdr := &types.SignedHeader{Header: types.Header{BaseHeader: types.BaseHeader{ChainID: "c", Height: 2}}} dat := &types.Data{Metadata: &types.Metadata{ChainID: "c", Height: 2}} - m1.SetHeaderSeen("H2") - m1.SetDataSeen("D2") - m1.SetHeaderDAIncluded("H2", 100) - m1.SetDataDAIncluded("D2", 101) + m1.SetHeaderSeen("H2", 2) + m1.SetDataSeen("D2", 2) + m1.SetHeaderDAIncluded("H2", 100, 2) + m1.SetDataDAIncluded("D2", 101, 2) m1.SetPendingEvent(2, &common.DAHeightEvent{Header: hdr, Data: dat, DaHeight: 99}) // persist diff --git a/block/internal/submitting/da_submitter.go b/block/internal/submitting/da_submitter.go index 8b3d780b47..baccd069b9 100644 --- a/block/internal/submitting/da_submitter.go +++ b/block/internal/submitting/da_submitter.go @@ -225,7 +225,7 @@ func (s *DASubmitter) SubmitHeaders(ctx context.Context, cache cache.Manager) er }, func(submitted []*types.SignedHeader, res *coreda.ResultSubmit, gasPrice float64) { for _, header := range submitted { - cache.SetHeaderDAIncluded(header.Hash().String(), res.Height) + cache.SetHeaderDAIncluded(header.Hash().String(), res.Height, header.Height()) } // Update last submitted height if l := len(submitted); l > 0 { @@ -270,7 +270,7 @@ func (s *DASubmitter) SubmitData(ctx context.Context, cache cache.Manager, signe }, func(submitted []*types.SignedData, res *coreda.ResultSubmit, gasPrice float64) { for _, sd := range submitted { - cache.SetDataDAIncluded(sd.Data.DACommitment().String(), res.Height) + cache.SetDataDAIncluded(sd.Data.DACommitment().String(), res.Height, sd.Height()) } if l := len(submitted); l > 0 { lastHeight := submitted[l-1].Height() diff --git a/block/internal/submitting/submitter.go b/block/internal/submitting/submitter.go index 76ae5b69cc..303158cb84 100644 --- a/block/internal/submitting/submitter.go +++ b/block/internal/submitting/submitter.go @@ -234,6 +234,10 @@ func (s *Submitter) processDAInclusionLoop() { if err := s.store.SetMetadata(s.ctx, store.DAIncludedHeightKey, bz); err != nil { s.logger.Error().Err(err).Uint64("height", nextHeight).Msg("failed to persist DA included height") } + + // Delete height cache for that height + // This can only be performed after the height has been persisted to store + s.cache.DeleteHeight(nextHeight) } } } diff --git a/block/internal/submitting/submitter_test.go b/block/internal/submitting/submitter_test.go index c05ebe3297..9da50af2af 100644 --- a/block/internal/submitting/submitter_test.go +++ b/block/internal/submitting/submitter_test.go @@ -120,9 +120,9 @@ func TestSubmitter_IsHeightDAIncluded(t *testing.T) { h1, d1 := newHeaderAndData("chain", 3, true) h2, d2 := newHeaderAndData("chain", 4, true) - cm.SetHeaderDAIncluded(h1.Hash().String(), 100) - cm.SetDataDAIncluded(d1.DACommitment().String(), 100) - cm.SetHeaderDAIncluded(h2.Hash().String(), 101) + cm.SetHeaderDAIncluded(h1.Hash().String(), 100, 2) + cm.SetDataDAIncluded(d1.DACommitment().String(), 100, 2) + cm.SetHeaderDAIncluded(h2.Hash().String(), 101, 4) // no data for h2 specs := map[string]struct { @@ -166,8 +166,8 @@ func TestSubmitter_setSequencerHeightToDAHeight(t *testing.T) { h, d := newHeaderAndData("chain", 1, true) // set DA included heights in cache - cm.SetHeaderDAIncluded(h.Hash().String(), 100) - cm.SetDataDAIncluded(d.DACommitment().String(), 90) + cm.SetHeaderDAIncluded(h.Hash().String(), 100, 1) + cm.SetDataDAIncluded(d.DACommitment().String(), 90, 1) headerKey := fmt.Sprintf("%s/%d/h", store.HeightToDAHeightKey, 1) dataKey := fmt.Sprintf("%s/%d/d", store.HeightToDAHeightKey, 1) @@ -200,7 +200,7 @@ func TestSubmitter_setSequencerHeightToDAHeight_Errors(t *testing.T) { assert.Error(t, s.setSequencerHeightToDAHeight(ctx, 1, h, d, false)) // Add header, missing data - cm.SetHeaderDAIncluded(h.Hash().String(), 10) + cm.SetHeaderDAIncluded(h.Hash().String(), 10, 1) assert.Error(t, s.setSequencerHeightToDAHeight(ctx, 1, h, d, false)) } @@ -263,10 +263,10 @@ func TestSubmitter_processDAInclusionLoop_advances(t *testing.T) { require.NoError(t, batch2.SetHeight(2)) require.NoError(t, batch2.Commit()) - cm.SetHeaderDAIncluded(h1.Hash().String(), 100) - cm.SetDataDAIncluded(d1.DACommitment().String(), 100) - cm.SetHeaderDAIncluded(h2.Hash().String(), 101) - cm.SetDataDAIncluded(d2.DACommitment().String(), 101) + cm.SetHeaderDAIncluded(h1.Hash().String(), 100, 1) + cm.SetDataDAIncluded(d1.DACommitment().String(), 100, 1) + cm.SetHeaderDAIncluded(h2.Hash().String(), 101, 2) + cm.SetDataDAIncluded(d2.DACommitment().String(), 101, 2) s.ctx, s.cancel = ctx, cancel require.NoError(t, s.initializeDAIncludedHeight(ctx)) @@ -408,3 +408,109 @@ type fakeSigner struct{} func (f *fakeSigner) Sign(msg []byte) ([]byte, error) { return append([]byte(nil), msg...), nil } func (f *fakeSigner) GetPublic() (crypto.PubKey, error) { return nil, nil } func (f *fakeSigner) GetAddress() ([]byte, error) { return []byte("addr"), nil } + +func TestSubmitter_CacheClearedOnHeightInclusion(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + cm, st := newTestCacheAndStore(t) + + cfg := config.DefaultConfig() + cfg.DA.BlockTime.Duration = 5 * time.Millisecond + metrics := common.NopMetrics() + + exec := testmocks.NewMockExecutor(t) + exec.On("SetFinal", mock.Anything, uint64(1)).Return(nil).Once() + exec.On("SetFinal", mock.Anything, uint64(2)).Return(nil).Once() + + daSub := NewDASubmitter(nil, cfg, genesis.Genesis{}, common.DefaultBlockOptions(), metrics, zerolog.Nop()) + s := NewSubmitter(st, exec, cm, metrics, cfg, genesis.Genesis{}, daSub, nil, zerolog.Nop(), nil) + + // Create test blocks + h1, d1 := newHeaderAndData("chain", 1, true) + h2, d2 := newHeaderAndData("chain", 2, true) + h3, d3 := newHeaderAndData("chain", 3, true) + + sig := types.Signature([]byte("sig")) + + // Save blocks to store + blocks := []struct { + header *types.SignedHeader + data *types.Data + height uint64 + }{ + {h1, d1, 1}, + {h2, d2, 2}, + {h3, d3, 3}, + } + + for _, block := range blocks { + batch, err := st.NewBatch(ctx) + require.NoError(t, err) + require.NoError(t, batch.SaveBlockData(block.header, block.data, &sig)) + require.NoError(t, batch.SetHeight(block.height)) + require.NoError(t, batch.Commit()) + } + + // Set up cache with headers and data seen for all heights + cm.SetHeaderSeen(h1.Hash().String(), 1) + cm.SetDataSeen(d1.DACommitment().String(), 1) + cm.SetHeaderSeen(h2.Hash().String(), 2) + cm.SetDataSeen(d2.DACommitment().String(), 2) + cm.SetHeaderSeen(h3.Hash().String(), 3) + cm.SetDataSeen(d3.DACommitment().String(), 3) + + // Verify items are seen in cache before processing + assert.True(t, cm.IsHeaderSeen(h1.Hash().String())) + assert.True(t, cm.IsDataSeen(d1.DACommitment().String())) + assert.True(t, cm.IsHeaderSeen(h2.Hash().String())) + assert.True(t, cm.IsDataSeen(d2.DACommitment().String())) + assert.True(t, cm.IsHeaderSeen(h3.Hash().String())) + assert.True(t, cm.IsDataSeen(d3.DACommitment().String())) + + // Set DA inclusion for heights 1 and 2 only (height 3 will remain unprocessed) + cm.SetHeaderDAIncluded(h1.Hash().String(), 100, 1) + cm.SetDataDAIncluded(d1.DACommitment().String(), 100, 1) + cm.SetHeaderDAIncluded(h2.Hash().String(), 101, 2) + cm.SetDataDAIncluded(d2.DACommitment().String(), 101, 2) + + s.ctx, s.cancel = ctx, cancel + require.NoError(t, s.initializeDAIncludedHeight(ctx)) + require.Equal(t, uint64(0), s.GetDAIncludedHeight()) + + // Start submitter to process DA inclusions + require.NoError(t, s.Start(ctx)) + + // Wait for heights 1 and 2 to be processed + require.Eventually(t, func() bool { + return s.GetDAIncludedHeight() == 2 + }, 1*time.Second, 10*time.Millisecond) + + require.NoError(t, s.Stop()) + + // Verify cache is cleared for processed heights (1 and 2) + assert.False(t, cm.IsHeaderSeen(h1.Hash().String()), "height 1 header should be cleared from cache") + assert.False(t, cm.IsDataSeen(d1.DACommitment().String()), "height 1 data should be cleared from cache") + assert.False(t, cm.IsHeaderSeen(h2.Hash().String()), "height 2 header should be cleared from cache") + assert.False(t, cm.IsDataSeen(d2.DACommitment().String()), "height 2 data should be cleared from cache") + + // Verify DA inclusion status remains for processed heights + _, h1DAIncluded := cm.GetHeaderDAIncluded(h1.Hash().String()) + _, d1DAIncluded := cm.GetDataDAIncluded(d1.DACommitment().String()) + _, h2DAIncluded := cm.GetHeaderDAIncluded(h2.Hash().String()) + _, d2DAIncluded := cm.GetDataDAIncluded(d2.DACommitment().String()) + assert.True(t, h1DAIncluded, "height 1 header DA inclusion status should remain") + assert.True(t, d1DAIncluded, "height 1 data DA inclusion status should remain") + assert.True(t, h2DAIncluded, "height 2 header DA inclusion status should remain") + assert.True(t, d2DAIncluded, "height 2 data DA inclusion status should remain") + + // Verify unprocessed height 3 cache remains intact + assert.True(t, cm.IsHeaderSeen(h3.Hash().String()), "height 3 header should remain in cache") + assert.True(t, cm.IsDataSeen(d3.DACommitment().String()), "height 3 data should remain in cache") + + // Verify height 3 has no DA inclusion status since it wasn't processed + _, h3DAIncluded := cm.GetHeaderDAIncluded(h3.Hash().String()) + _, d3DAIncluded := cm.GetDataDAIncluded(d3.DACommitment().String()) + assert.False(t, h3DAIncluded, "height 3 header should not have DA inclusion status") + assert.False(t, d3DAIncluded, "height 3 data should not have DA inclusion status") +} diff --git a/block/internal/syncing/da_retriever.go b/block/internal/syncing/da_retriever.go index dcb7f2047b..de67e1fd1c 100644 --- a/block/internal/syncing/da_retriever.go +++ b/block/internal/syncing/da_retriever.go @@ -247,7 +247,7 @@ func (r *DARetriever) tryDecodeHeader(bz []byte, daHeight uint64) *types.SignedH // This has to be done for all fetched DA headers prior to validation because P2P does not confirm // da inclusion. This is not an issue, as an invalid header will be rejected. There cannot be hash collisions. headerHash := header.Hash().String() - r.cache.SetHeaderDAIncluded(headerHash, daHeight) + r.cache.SetHeaderDAIncluded(headerHash, daHeight, header.Height()) r.logger.Info(). Str("header_hash", headerHash). @@ -278,7 +278,7 @@ func (r *DARetriever) tryDecodeData(bz []byte, daHeight uint64) *types.Data { // Mark as DA included dataHash := signedData.Data.DACommitment().String() - r.cache.SetDataDAIncluded(dataHash, daHeight) + r.cache.SetDataDAIncluded(dataHash, daHeight, signedData.Height()) r.logger.Info(). Str("data_hash", dataHash). diff --git a/block/internal/syncing/syncer.go b/block/internal/syncing/syncer.go index 0a9443c319..bff287864c 100644 --- a/block/internal/syncing/syncer.go +++ b/block/internal/syncing/syncer.go @@ -462,9 +462,9 @@ func (s *Syncer) trySyncNextBlock(event *common.DAHeightEvent) error { s.metrics.Height.Set(float64(newState.LastBlockHeight)) // Mark as seen - s.cache.SetHeaderSeen(header.Hash().String()) + s.cache.SetHeaderSeen(header.Hash().String(), header.Height()) if !bytes.Equal(header.DataHash, common.DataHashForEmptyTxs) { - s.cache.SetDataSeen(data.DACommitment().String()) + s.cache.SetDataSeen(data.DACommitment().String(), newState.LastBlockHeight) } return nil diff --git a/block/internal/syncing/syncer_test.go b/block/internal/syncing/syncer_test.go index 65ecfb674c..a3dfccb392 100644 --- a/block/internal/syncing/syncer_test.go +++ b/block/internal/syncing/syncer_test.go @@ -235,10 +235,10 @@ func TestSequentialBlockSync(t *testing.T) { s.processHeightEvent(&evt2) // Mark DA inclusion in cache (as DA retrieval would) - cm.SetDataDAIncluded(data1.DACommitment().String(), 10) - cm.SetDataDAIncluded(data2.DACommitment().String(), 11) // empty data still needs cache entry - cm.SetHeaderDAIncluded(hdr1.Header.Hash().String(), 10) - cm.SetHeaderDAIncluded(hdr2.Header.Hash().String(), 11) + cm.SetDataDAIncluded(data1.DACommitment().String(), 10, 1) + cm.SetDataDAIncluded(data2.DACommitment().String(), 11, 2) // empty data still needs cache entry + cm.SetHeaderDAIncluded(hdr1.Header.Hash().String(), 10, 1) + cm.SetHeaderDAIncluded(hdr2.Header.Hash().String(), 11, 2) // Verify both blocks were synced correctly finalState, _ := st.GetState(context.Background())