Skip to content

Commit 401b5d0

Browse files
committed
feedback
1 parent fd73169 commit 401b5d0

4 files changed

Lines changed: 23 additions & 17 deletions

File tree

block/internal/cache/generic_cache.go

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package cache
33
import (
44
"context"
55
"encoding/binary"
6+
"fmt"
67
"sync"
78
"sync/atomic"
89

@@ -148,14 +149,14 @@ func (c *Cache[T]) setDAIncluded(hash string, daHeight uint64, blockHeight uint6
148149
c.daIncluded.Add(hash, daHeight)
149150
c.hashByHeight.Add(blockHeight, hash)
150151
c.setMaxDAHeight(daHeight)
151-
c.persistSnapshot(context.Background())
152+
_ = c.persistSnapshot(context.Background())
152153
}
153154

154155
// removeDAIncluded removes the DA-included status of the hash from the cache
155156
// and rewrites the window snapshot.
156157
func (c *Cache[T]) removeDAIncluded(hash string) {
157158
c.daIncluded.Remove(hash)
158-
c.persistSnapshot(context.Background())
159+
_ = c.persistSnapshot(context.Background())
159160
}
160161

161162
// daHeight returns the maximum DA height from all DA-included items.
@@ -200,15 +201,15 @@ func (c *Cache[T]) deleteAllForHeight(height uint64) {
200201
c.daIncluded.Remove(hash)
201202
}
202203

203-
c.persistSnapshot(context.Background())
204+
_ = c.persistSnapshot(context.Background())
204205
}
205206

206207
// persistSnapshot writes all current in-flight [blockHeight, daHeight] pairs
207208
// to the store under a single key. Called on every mutation; payload is tiny
208209
// (typically <10 entries × 16 bytes).
209-
func (c *Cache[T]) persistSnapshot(ctx context.Context) {
210+
func (c *Cache[T]) persistSnapshot(ctx context.Context) error {
210211
if c.store == nil || c.storeKeyPrefix == "" {
211-
return
212+
return nil
212213
}
213214

214215
heights := c.hashByHeight.Keys()
@@ -225,7 +226,7 @@ func (c *Cache[T]) persistSnapshot(ctx context.Context) {
225226
entries = append(entries, snapshotEntry{blockHeight: h, daHeight: daH})
226227
}
227228

228-
_ = c.store.SetMetadata(ctx, c.snapshotKey(), encodeSnapshot(entries))
229+
return c.store.SetMetadata(ctx, c.snapshotKey(), encodeSnapshot(entries))
229230
}
230231

231232
// encodeSnapshot serialises a slice of snapshotEntry values into a byte slice.
@@ -298,7 +299,9 @@ func (c *Cache[T]) SaveToStore(ctx context.Context) error {
298299
if c.store == nil {
299300
return nil
300301
}
301-
c.persistSnapshot(ctx)
302+
if err := c.persistSnapshot(ctx); err != nil {
303+
return fmt.Errorf("saving cache snapshot: %w", err)
304+
}
302305
return nil
303306
}
304307

@@ -307,6 +310,8 @@ func (c *Cache[T]) ClearFromStore(ctx context.Context) error {
307310
if c.store == nil {
308311
return nil
309312
}
310-
_ = c.store.DeleteMetadata(ctx, c.snapshotKey())
313+
if err := c.store.DeleteMetadata(ctx, c.snapshotKey()); err != nil {
314+
return fmt.Errorf("clearing cache snapshot: %w", err)
315+
}
311316
return nil
312317
}

block/internal/cache/generic_cache_test.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -325,10 +325,14 @@ func TestCache_ClearFromStore(t *testing.T) {
325325
c.setDAIncluded("hash1", 100, 1)
326326
c.setDAIncluded("hash2", 200, 2)
327327

328+
// Verify the snapshot key was written before clearing.
329+
_, err := st.GetMetadata(ctx, "clear-test/__snap")
330+
require.NoError(t, err, "snapshot key should exist before ClearFromStore")
331+
328332
require.NoError(t, c.ClearFromStore(ctx))
329333

330-
_, err := st.GetMetadata(ctx, "clear-test/hash1")
331-
assert.Error(t, err, "key should have been removed from store")
334+
_, err = st.GetMetadata(ctx, "clear-test/__snap")
335+
assert.Error(t, err, "snapshot key should have been removed from store")
332336
}
333337

334338
// ---------------------------------------------------------------------------

block/internal/executing/executor.go

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -422,10 +422,7 @@ func (e *Executor) executionLoop() {
422422
txsAvailable = false
423423
// reset timer accounting for time spent producing the block
424424
elapsed := time.Since(start)
425-
remaining := e.config.Node.BlockTime.Duration - elapsed
426-
if remaining <= 0 {
427-
remaining = 0
428-
}
425+
remaining := max(e.config.Node.BlockTime.Duration-elapsed, 0)
429426
blockTimer.Reset(remaining)
430427

431428
case <-lazyTimerCh:

pkg/sequencers/single/sequencer_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1368,7 +1368,7 @@ func TestSequencer_CatchUp_SkipsMempoolDuringCatchUp(t *testing.T) {
13681368
require.NoError(t, err)
13691369

13701370
// Submit several mempool transactions
1371-
for i := 0; i < 5; i++ {
1371+
for range 5 {
13721372
_, err = seq.SubmitBatchTxs(ctx, coresequencer.SubmitBatchTxsRequest{
13731373
Id: []byte("test-chain"),
13741374
Batch: &coresequencer.Batch{Transactions: [][]byte{[]byte("mempool-tx")}},
@@ -1741,7 +1741,7 @@ func TestSequencer_CatchUp_MultiEpochReplay(t *testing.T) {
17411741
}
17421742

17431743
// Process the 3 old epochs — all should be catch-up (no mempool)
1744-
for i := 0; i < 3; i++ {
1744+
for i := range 3 {
17451745
resp, err := seq.GetNextBatch(ctx, req)
17461746
require.NoError(t, err)
17471747
assert.True(t, seq.isCatchingUp(), "should be catching up during epoch %d", 100+i)
@@ -2005,7 +2005,7 @@ func TestSequencer_CatchUp_MonotonicTimestamps(t *testing.T) {
20052005

20062006
// Produce 3 blocks from epoch 100 (1 tx each due to gas filter)
20072007
var timestamps []time.Time
2008-
for i := 0; i < 3; i++ {
2008+
for i := range 3 {
20092009
resp, err := seq.GetNextBatch(ctx, req)
20102010
require.NoError(t, err)
20112011
assert.True(t, seq.isCatchingUp(), "should be catching up during block %d", i)

0 commit comments

Comments
 (0)