From cb4008e66f263d796c0494829249842bf350f809 Mon Sep 17 00:00:00 2001 From: samliok Date: Tue, 29 Sep 2026 00:23:49 +0100 Subject: [PATCH] race fix --- adapters.go | 11 +++++++++-- adapters_test.go | 22 ++++++++++++++++++++++ 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/adapters.go b/adapters.go index dfea4e33..7322a432 100644 --- a/adapters.go +++ b/adapters.go @@ -181,13 +181,15 @@ func (cs *CachedStorage) Retrieve(seq uint64, digest common.Digest) (common.Veri } func (cs *CachedStorage) Index(ctx context.Context, block common.VerifiedBlock, certificate common.Finalization) error { + // Holding the lock across indexing and pruning prevents Retrieve from serving a cached block at an indexed seq. + cs.lock.Lock() + defer cs.lock.Unlock() + err := cs.Storage.Index(ctx, block, certificate) if err == nil { // We delete the block from the cache after it has been indexed because now that it is persisted, // we can just lookup by sequence number instead of digest. - cs.lock.Lock() - defer cs.lock.Unlock() delete(cs.cache, block.BlockHeader().Digest) // We also delete all blocks that are older than the indexed block, including the finalized block because they are now finalized and persisted. @@ -205,6 +207,11 @@ func (cs *CachedStorage) insertBlock(block *ParsedBlock) { cs.lock.Lock() defer cs.lock.Unlock() + // A verification that completes after its seq was indexed must not shadow the finalized block. + if block.BlockHeader().Seq < cs.Storage.NumBlocks() { + return + } + cs.cache[block.Digest()] = cachedBlock{ ParsedBlock: block, } diff --git a/adapters_test.go b/adapters_test.go index 00ddeba3..b4259d3a 100644 --- a/adapters_test.go +++ b/adapters_test.go @@ -140,6 +140,28 @@ func TestCachedStorageIndexEvictsSameSeqFork(t *testing.T) { require.NotNil(t, fin) } +// TestCachedStorageLateVerify asserts that a fork whose +// verification completes after its seq was indexed is not cached. +func TestCachedStorageLateVerifyDoesNotShadowIndexed(t *testing.T) { + cs := NewCachedStorage(newTestStorage(), 0) + require.NoError(t, cs.Index(t.Context(), newTestParsedBlock(0, "genesis"), common.Finalization{})) + + finalized := newTestParsedBlock(1, "finalized") + require.NoError(t, cs.Index(t.Context(), finalized, common.Finalization{})) + + delayedVerificationBlock := &cachedBlock{ + ParsedBlock: newTestParsedBlock(1, "fork"), + cache: cs, + } + _, err := delayedVerificationBlock.Verify(t.Context(), common.OnlyVMVerifyOpt) + require.NoError(t, err) + + retrievedBlock, fin, err := cs.Retrieve(1, common.Digest{}) + require.NoError(t, err) + require.Equal(t, finalized.BlockHeader().Digest, retrievedBlock.BlockHeader().Digest) + require.NotNil(t, fin) +} + // TestCachedStoragePopulatedByWal asserts that a block restored from the WAL on // startup ends up in the instance's CachedStorage, retrievable by seq before it // is finalized and indexed.