Skip to content

Commit 3b3d5e7

Browse files
tac0turtleclaude
andauthored
chore: minor deduplication (#3139)
* simplify * add grpc * lint * fix: distinguish store not-found from errors, persist before advancing state Address PR review feedback: - getMetadataUint64 returns (uint64, bool, error) to distinguish missing keys from backend failures - processDAInclusionLoop persists DAIncludedHeightKey before advancing in-memory state to prevent cache deletion on persist failure - Expose store.ErrNotFound and store.IsNotFound for clean sentinel checks Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent a96974b commit 3b3d5e7

9 files changed

Lines changed: 85 additions & 60 deletions

File tree

.github/workflows/ci.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ jobs:
5555
apps: |
5656
[
5757
{"name": "ev-node-evm", "dockerfile": "apps/evm/Dockerfile"},
58+
{"name": "ev-node-grpc", "dockerfile": "apps/grpc/Dockerfile"},
5859
{"name": "ev-node-testapp", "dockerfile": "apps/testapp/Dockerfile"}
5960
]
6061

.golangci.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ linters:
5555
gosec:
5656
excludes:
5757
- G115
58+
- G118
5859
revive:
5960
rules:
6061
- name: package-comments

apps/grpc/Dockerfile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ WORKDIR /ev-node
1010
# Copy go mod files
1111
COPY go.mod go.sum ./
1212
COPY apps/grpc/go.mod apps/grpc/go.sum ./apps/grpc/
13-
COPY core/go.mod ./core/
13+
COPY core/go.mod core/go.sum ./core/
1414
COPY execution/grpc/go.mod execution/grpc/go.sum ./execution/grpc/
1515

1616
# Download dependencies

block/internal/cache/manager.go

Lines changed: 33 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,9 @@ import (
1515
"github.com/evstack/ev-node/types"
1616
)
1717

18+
// isNotFound is a convenience alias for store.IsNotFound.
19+
var isNotFound = store.IsNotFound
20+
1821
const (
1922
// HeaderDAIncludedPrefix is the store key prefix for header DA inclusion tracking.
2023
HeaderDAIncludedPrefix = "cache/header-da-included/"
@@ -387,23 +390,45 @@ func (m *implementation) ClearFromStore() error {
387390
return nil
388391
}
389392

393+
// getMetadataUint64 reads an 8-byte little-endian uint64 from store metadata.
394+
// Returns (0, false, nil) when the key is absent and a non-nil error for
395+
// genuine backend failures or malformed values.
396+
func getMetadataUint64(ctx context.Context, st store.Store, key string) (uint64, bool, error) {
397+
b, err := st.GetMetadata(ctx, key)
398+
if err != nil {
399+
// Key absent — not an error, just missing.
400+
if isNotFound(err) {
401+
return 0, false, nil
402+
}
403+
return 0, false, fmt.Errorf("read metadata %q: %w", key, err)
404+
}
405+
if len(b) != 8 {
406+
return 0, false, fmt.Errorf("invalid metadata length for %q: %d", key, len(b))
407+
}
408+
return binary.LittleEndian.Uint64(b), true, nil
409+
}
410+
390411
// initDAHeightFromStore seeds maxDAHeight from the HeightToDAHeight metadata
391412
// written by the submitter for the last finalized block. This ensures
392413
// DaHeight() is non-zero on restart even when the in-flight snapshot is empty.
393414
func (m *implementation) initDAHeightFromStore(ctx context.Context) {
394-
daIncludedBytes, err := m.store.GetMetadata(ctx, store.DAIncludedHeightKey)
395-
if err != nil || len(daIncludedBytes) != 8 {
415+
daIncludedHeight, ok, err := getMetadataUint64(ctx, m.store, store.DAIncludedHeightKey)
416+
if err != nil {
417+
m.logger.Error().Err(err).Msg("failed to read DA included height from store")
396418
return
397419
}
398-
daIncludedHeight := binary.LittleEndian.Uint64(daIncludedBytes)
399-
if daIncludedHeight == 0 {
420+
if !ok || daIncludedHeight == 0 {
400421
return
401422
}
402423

403-
if b, err := m.store.GetMetadata(ctx, store.GetHeightToDAHeightHeaderKey(daIncludedHeight)); err == nil && len(b) == 8 {
404-
m.headerCache.setMaxDAHeight(binary.LittleEndian.Uint64(b))
424+
if h, ok, err := getMetadataUint64(ctx, m.store, store.GetHeightToDAHeightHeaderKey(daIncludedHeight)); err != nil {
425+
m.logger.Error().Err(err).Msg("failed to read header DA height from store")
426+
} else if ok {
427+
m.headerCache.setMaxDAHeight(h)
405428
}
406-
if b, err := m.store.GetMetadata(ctx, store.GetHeightToDAHeightDataKey(daIncludedHeight)); err == nil && len(b) == 8 {
407-
m.dataCache.setMaxDAHeight(binary.LittleEndian.Uint64(b))
429+
if h, ok, err := getMetadataUint64(ctx, m.store, store.GetHeightToDAHeightDataKey(daIncludedHeight)); err != nil {
430+
m.logger.Error().Err(err).Msg("failed to read data DA height from store")
431+
} else if ok {
432+
m.dataCache.setMaxDAHeight(h)
408433
}
409434
}

block/internal/cache/manager_test.go

Lines changed: 15 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -23,17 +23,10 @@ func tempConfig(t *testing.T) config.Config {
2323
return cfg
2424
}
2525

26-
// helper to make an in-memory store
27-
func memStore(t *testing.T) pkgstore.Store {
28-
ds, err := pkgstore.NewTestInMemoryKVStore()
29-
require.NoError(t, err)
30-
return pkgstore.New(ds)
31-
}
32-
3326
func TestManager_HeaderDataOperations(t *testing.T) {
3427
t.Parallel()
3528
cfg := tempConfig(t)
36-
st := memStore(t)
29+
st := testMemStore(t)
3730

3831
m, err := NewManager(cfg, st, zerolog.Nop())
3932
require.NoError(t, err)
@@ -55,7 +48,7 @@ func TestManager_HeaderDataOperations(t *testing.T) {
5548
func TestManager_PendingEventsCRUD(t *testing.T) {
5649
t.Parallel()
5750
cfg := tempConfig(t)
58-
st := memStore(t)
51+
st := testMemStore(t)
5952

6053
m, err := NewManager(cfg, st, zerolog.Nop())
6154
require.NoError(t, err)
@@ -89,7 +82,7 @@ func TestManager_PendingEventsCRUD(t *testing.T) {
8982
func TestManager_SaveAndRestoreFromStore(t *testing.T) {
9083
t.Parallel()
9184
cfg := tempConfig(t)
92-
st := memStore(t)
85+
st := testMemStore(t)
9386
ctx := context.Background()
9487

9588
h1, d1 := types.GetRandomBlock(1, 1, "test-chain")
@@ -169,7 +162,7 @@ func TestManager_SaveAndRestoreFromStore(t *testing.T) {
169162
func TestManager_GetNextPendingEvent_NonExistent(t *testing.T) {
170163
t.Parallel()
171164
cfg := tempConfig(t)
172-
st := memStore(t)
165+
st := testMemStore(t)
173166

174167
m, err := NewManager(cfg, st, zerolog.Nop())
175168
require.NoError(t, err)
@@ -181,7 +174,7 @@ func TestManager_GetNextPendingEvent_NonExistent(t *testing.T) {
181174

182175
func TestPendingHeadersAndData_Flow(t *testing.T) {
183176
t.Parallel()
184-
st := memStore(t)
177+
st := testMemStore(t)
185178
ctx := context.Background()
186179
logger := zerolog.Nop()
187180

@@ -247,7 +240,7 @@ func TestPendingHeadersAndData_Flow(t *testing.T) {
247240
func TestManager_TxOperations(t *testing.T) {
248241
t.Parallel()
249242
cfg := tempConfig(t)
250-
st := memStore(t)
243+
st := testMemStore(t)
251244

252245
m, err := NewManager(cfg, st, zerolog.Nop())
253246
require.NoError(t, err)
@@ -268,7 +261,7 @@ func TestManager_TxOperations(t *testing.T) {
268261
func TestManager_CleanupOldTxs(t *testing.T) {
269262
t.Parallel()
270263
cfg := tempConfig(t)
271-
st := memStore(t)
264+
st := testMemStore(t)
272265

273266
m, err := NewManager(cfg, st, zerolog.Nop())
274267
require.NoError(t, err)
@@ -296,7 +289,7 @@ func TestManager_CleanupOldTxs(t *testing.T) {
296289
func TestManager_CleanupOldTxs_SelectiveRemoval(t *testing.T) {
297290
t.Parallel()
298291
cfg := tempConfig(t)
299-
st := memStore(t)
292+
st := testMemStore(t)
300293

301294
m, err := NewManager(cfg, st, zerolog.Nop())
302295
require.NoError(t, err)
@@ -330,7 +323,7 @@ func TestManager_CleanupOldTxs_SelectiveRemoval(t *testing.T) {
330323
func TestManager_CleanupOldTxs_DefaultDuration(t *testing.T) {
331324
t.Parallel()
332325
cfg := tempConfig(t)
333-
st := memStore(t)
326+
st := testMemStore(t)
334327

335328
m, err := NewManager(cfg, st, zerolog.Nop())
336329
require.NoError(t, err)
@@ -359,7 +352,7 @@ func TestManager_CleanupOldTxs_DefaultDuration(t *testing.T) {
359352
func TestManager_CleanupOldTxs_NoTransactions(t *testing.T) {
360353
t.Parallel()
361354
cfg := tempConfig(t)
362-
st := memStore(t)
355+
st := testMemStore(t)
363356

364357
m, err := NewManager(cfg, st, zerolog.Nop())
365358
require.NoError(t, err)
@@ -372,7 +365,7 @@ func TestManager_CleanupOldTxs_NoTransactions(t *testing.T) {
372365
func TestManager_TxCache_NotPersistedToStore(t *testing.T) {
373366
t.Parallel()
374367
cfg := tempConfig(t)
375-
st := memStore(t)
368+
st := testMemStore(t)
376369

377370
// Create first manager and add transactions
378371
m1, err := NewManager(cfg, st, zerolog.Nop())
@@ -400,7 +393,7 @@ func TestManager_TxCache_NotPersistedToStore(t *testing.T) {
400393
func TestManager_DeleteHeight_PreservesTxCache(t *testing.T) {
401394
t.Parallel()
402395
cfg := tempConfig(t)
403-
st := memStore(t)
396+
st := testMemStore(t)
404397

405398
m, err := NewManager(cfg, st, zerolog.Nop())
406399
require.NoError(t, err)
@@ -429,7 +422,7 @@ func TestManager_DeleteHeight_PreservesTxCache(t *testing.T) {
429422
func TestManager_DAInclusionPersistence(t *testing.T) {
430423
t.Parallel()
431424
cfg := tempConfig(t)
432-
st := memStore(t)
425+
st := testMemStore(t)
433426
ctx := context.Background()
434427

435428
// Create blocks and save to store
@@ -484,7 +477,7 @@ func TestManager_DaHeightAfterCacheClear(t *testing.T) {
484477
t.Parallel()
485478

486479
cfg := tempConfig(t)
487-
st := memStore(t)
480+
st := testMemStore(t)
488481
ctx := context.Background()
489482

490483
// Store a block first
@@ -529,7 +522,7 @@ func TestManager_DaHeightFromStoreOnRestore(t *testing.T) {
529522
t.Parallel()
530523

531524
cfg := tempConfig(t)
532-
st := memStore(t)
525+
st := testMemStore(t)
533526
ctx := context.Background()
534527

535528
// Store a block first

block/internal/cache/pending_data_test.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import (
1515
func TestPendingData_BasicFlow(t *testing.T) {
1616
t.Parallel()
1717
ctx := context.Background()
18-
store := memStore(t)
18+
store := testMemStore(t)
1919

2020
// three blocks with transactions
2121
chainID := "pd-basic"
@@ -62,7 +62,7 @@ func TestPendingData_BasicFlow(t *testing.T) {
6262
func TestPendingData_AdvancesPastEmptyData(t *testing.T) {
6363
t.Parallel()
6464
ctx := context.Background()
65-
store := memStore(t)
65+
store := testMemStore(t)
6666

6767
// Create blocks: non-empty, empty, empty, non-empty
6868
chainID := "pd-empty"
@@ -108,7 +108,7 @@ func TestPendingData_AdvancesPastEmptyData(t *testing.T) {
108108
func TestPendingData_AdvancesPastAllEmptyToEnd(t *testing.T) {
109109
t.Parallel()
110110
ctx := context.Background()
111-
store := memStore(t)
111+
store := testMemStore(t)
112112

113113
// Create blocks: non-empty, empty, empty (all remaining are empty)
114114
chainID := "pd-all-empty"
@@ -144,7 +144,7 @@ func TestPendingData_AdvancesPastAllEmptyToEnd(t *testing.T) {
144144
func TestPendingData_AdvancesPastEmptyAtStart(t *testing.T) {
145145
t.Parallel()
146146
ctx := context.Background()
147-
store := memStore(t)
147+
store := testMemStore(t)
148148

149149
// Create blocks: empty, empty, non-empty
150150
chainID := "pd-empty-start"
@@ -206,7 +206,7 @@ func TestPendingData_InitFromMetadata(t *testing.T) {
206206
func TestPendingData_GetPending_PropagatesFetchError(t *testing.T) {
207207
t.Parallel()
208208
ctx := context.Background()
209-
store := memStore(t)
209+
store := testMemStore(t)
210210

211211
// Set height to 1 but do not save any block data
212212
batch, err := store.NewBatch(ctx)

block/internal/cache/pending_headers_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import (
1515
func TestPendingHeaders_BasicFlow(t *testing.T) {
1616
t.Parallel()
1717
ctx := context.Background()
18-
store := memStore(t)
18+
store := testMemStore(t)
1919

2020
// create and persist three blocks
2121
chainID := "ph-basic"
@@ -67,7 +67,7 @@ func TestPendingHeaders_BasicFlow(t *testing.T) {
6767
func TestPendingHeaders_EmptyWhenUpToDate(t *testing.T) {
6868
t.Parallel()
6969
ctx := context.Background()
70-
store := memStore(t)
70+
store := testMemStore(t)
7171

7272
h, d := types.GetRandomBlock(1, 1, "ph-up")
7373
batch, err := store.NewBatch(ctx)

block/internal/submitting/submitter.go

Lines changed: 20 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -349,17 +349,16 @@ func (s *Submitter) processDAInclusionLoop() {
349349
return
350350
}
351351

352+
// Persist DA included height before advancing in-memory state
353+
if err := putUint64Metadata(s.ctx, s.store, store.DAIncludedHeightKey, nextHeight); err != nil {
354+
s.logger.Error().Err(err).Uint64("height", nextHeight).Msg("failed to persist DA included height")
355+
break
356+
}
357+
352358
// Update DA included height
353359
s.SetDAIncludedHeight(nextHeight)
354360
currentDAIncluded = nextHeight
355361

356-
// Persist DA included height
357-
bz := make([]byte, 8)
358-
binary.LittleEndian.PutUint64(bz, nextHeight)
359-
if err := s.store.SetMetadata(s.ctx, store.DAIncludedHeightKey, bz); err != nil {
360-
s.logger.Error().Err(err).Uint64("height", nextHeight).Msg("failed to persist DA included height")
361-
}
362-
363362
// Delete height cache for that height
364363
// This can only be performed after the height has been persisted to store
365364
s.cache.DeleteHeight(nextHeight)
@@ -415,6 +414,13 @@ func (s *Submitter) initializeDAIncludedHeight(ctx context.Context) error {
415414
return nil
416415
}
417416

417+
// putUint64Metadata encodes val as 8-byte little-endian and writes it to the store.
418+
func putUint64Metadata(ctx context.Context, st store.Store, key string, val uint64) error {
419+
bz := make([]byte, 8)
420+
binary.LittleEndian.PutUint64(bz, val)
421+
return st.SetMetadata(ctx, key, bz)
422+
}
423+
418424
// sendCriticalError sends a critical error to the error channel without blocking
419425
func (s *Submitter) sendCriticalError(err error) {
420426
if s.errorCh != nil {
@@ -431,41 +437,33 @@ func (s *Submitter) sendCriticalError(err error) {
431437
func (s *Submitter) setNodeHeightToDAHeight(ctx context.Context, height uint64, data *types.Data, genesisInclusion bool) error {
432438
dataHash := data.DACommitment()
433439

434-
headerDaHeightBytes := make([]byte, 8)
435440
daHeightForHeader, ok := s.cache.GetHeaderDAIncludedByHeight(height)
436441
if !ok {
437442
return fmt.Errorf("header for height %d not found in cache", height)
438443
}
439-
binary.LittleEndian.PutUint64(headerDaHeightBytes, daHeightForHeader)
440444

441-
if err := s.store.SetMetadata(ctx, store.GetHeightToDAHeightHeaderKey(height), headerDaHeightBytes); err != nil {
445+
if err := putUint64Metadata(ctx, s.store, store.GetHeightToDAHeightHeaderKey(height), daHeightForHeader); err != nil {
442446
return err
443447
}
444448

445449
genesisDAIncludedHeight := daHeightForHeader
446-
dataDaHeightBytes := make([]byte, 8)
447-
// For empty transactions, use the same DA height as the header
448-
if bytes.Equal(dataHash, common.DataHashForEmptyTxs) {
449-
binary.LittleEndian.PutUint64(dataDaHeightBytes, daHeightForHeader)
450-
} else {
450+
// For empty transactions, use the same DA height as the header.
451+
dataDAHeight := daHeightForHeader
452+
if !bytes.Equal(dataHash, common.DataHashForEmptyTxs) {
451453
daHeightForData, ok := s.cache.GetDataDAIncludedByHeight(height)
452454
if !ok {
453455
return fmt.Errorf("data for height %d not found in cache", height)
454456
}
455-
binary.LittleEndian.PutUint64(dataDaHeightBytes, daHeightForData)
456-
457+
dataDAHeight = daHeightForData
457458
// if data posted before header, use data da included height for genesis da height
458459
genesisDAIncludedHeight = min(daHeightForData, genesisDAIncludedHeight)
459460
}
460-
if err := s.store.SetMetadata(ctx, store.GetHeightToDAHeightDataKey(height), dataDaHeightBytes); err != nil {
461+
if err := putUint64Metadata(ctx, s.store, store.GetHeightToDAHeightDataKey(height), dataDAHeight); err != nil {
461462
return err
462463
}
463464

464465
if genesisInclusion {
465-
genesisDAIncludedHeightBytes := make([]byte, 8)
466-
binary.LittleEndian.PutUint64(genesisDAIncludedHeightBytes, genesisDAIncludedHeight)
467-
468-
if err := s.store.SetMetadata(ctx, store.GenesisDAHeightKey, genesisDAIncludedHeightBytes); err != nil {
466+
if err := putUint64Metadata(ctx, s.store, store.GenesisDAHeightKey, genesisDAIncludedHeight); err != nil {
469467
return err
470468
}
471469

0 commit comments

Comments
 (0)