refactor(syncing): simplify double-sign detection by removing evidence persistence and checking against applied headers

Author: CaelRowley

block/internal/cache/manager.go

@@ -44,11 +44,6 @@ type CacheManager interface {
 	SetHeaderDAIncluded(hash string, daHeight uint64, blockHeight uint64)
 	RemoveHeaderDAIncluded(hash string)
 
-	// Pending signed header operations (in-flight, pre-persistence)
-	SetPendingSignedHeader(h *types.SignedHeader, source string)
-	GetPendingSignedHeader(blockHeight uint64) (*types.SignedHeader, string, bool)
-	RemovePendingSignedHeader(blockHeight uint64)
-
 	// Data operations
 	IsDataSeen(hash string) bool
 	SetDataSeen(hash string, blockHeight uint64)
@@ -98,24 +93,17 @@ type Manager interface {
 var _ Manager = (*implementation)(nil)
 
 type implementation struct {
-	headerCache            *Cache
-	dataCache              *Cache
-	txCache                *Cache
-	txTimestamps           *sync.Map // map[string]time.Time
-	pendingEvents          map[uint64]*common.DAHeightEvent
-	pendingMu              sync.Mutex
-	pendingHeaders         *PendingHeaders
-	pendingData            *PendingData
-	pendingSignedHeaders   map[uint64]pendingSignedHeader
-	pendingSignedHeadersMu sync.RWMutex
-	store                  store.Store
-	config                 config.Config
-	logger                 zerolog.Logger
-}
-
-type pendingSignedHeader struct {
-	header *types.SignedHeader
-	source string
+	headerCache    *Cache
+	dataCache      *Cache
+	txCache        *Cache
+	txTimestamps   *sync.Map // map[string]time.Time
+	pendingEvents  map[uint64]*common.DAHeightEvent
+	pendingMu      sync.Mutex
+	pendingHeaders *PendingHeaders
+	pendingData    *PendingData
+	store          store.Store
+	config         config.Config
+	logger         zerolog.Logger
 }
 
 // NewManager creates a new Manager, restoring or clearing persisted state as configured.
@@ -135,17 +123,16 @@ func NewManager(cfg config.Config, st store.Store, logger zerolog.Logger) (Manag
 	}
 
 	impl := &implementation{
-		headerCache:          headerCache,
-		dataCache:            dataCache,
-		txCache:              txCache,
-		txTimestamps:         new(sync.Map),
-		pendingEvents:        make(map[uint64]*common.DAHeightEvent),
-		pendingHeaders:       pendingHeaders,
-		pendingData:          pendingData,
-		pendingSignedHeaders: make(map[uint64]pendingSignedHeader),
-		store:                st,
-		config:               cfg,
-		logger:               logger,
+		headerCache:    headerCache,
+		dataCache:      dataCache,
+		txCache:        txCache,
+		txTimestamps:   new(sync.Map),
+		pendingEvents:  make(map[uint64]*common.DAHeightEvent),
+		pendingHeaders: pendingHeaders,
+		pendingData:    pendingData,
+		store:          st,
+		config:         cfg,
+		logger:         logger,
 	}
 
 	if cfg.ClearCache {
@@ -192,42 +179,6 @@ func (m *implementation) RemoveHeaderDAIncluded(hash string) {
 	m.headerCache.removeDAIncluded(hash)
 }
 
-// SetPendingSignedHeader records the first SignedHeader seen at this height.
-// First-write-wins: later writes at the same height are ignored so the
-// double-sign detector can match alternates against the original observation.
-func (m *implementation) SetPendingSignedHeader(h *types.SignedHeader, source string) {
-	if h == nil {
-		return
-	}
-	height := h.Height()
-	m.pendingSignedHeadersMu.Lock()
-	defer m.pendingSignedHeadersMu.Unlock()
-	if _, exists := m.pendingSignedHeaders[height]; exists {
-		return
-	}
-	m.pendingSignedHeaders[height] = pendingSignedHeader{header: h, source: source}
-}
-
-// GetPendingSignedHeader returns the first-seen SignedHeader and the source
-// ("da" or "p2p") it was observed from.
-func (m *implementation) GetPendingSignedHeader(blockHeight uint64) (*types.SignedHeader, string, bool) {
-	m.pendingSignedHeadersMu.RLock()
-	defer m.pendingSignedHeadersMu.RUnlock()
-	entry, ok := m.pendingSignedHeaders[blockHeight]
-	if !ok {
-		return nil, "", false
-	}
-	return entry.header, entry.source, true
-}
-
-// RemovePendingSignedHeader evicts the entry once the height is persisted, so
-// the store becomes the authoritative source for double-sign comparison.
-func (m *implementation) RemovePendingSignedHeader(blockHeight uint64) {
-	m.pendingSignedHeadersMu.Lock()
-	delete(m.pendingSignedHeaders, blockHeight)
-	m.pendingSignedHeadersMu.Unlock()
-}
-
 // DaHeight returns the highest DA height seen across header and data caches.
 func (m *implementation) DaHeight() uint64 {
 	return max(m.headerCache.daHeight(), m.dataCache.daHeight())
@@ -318,7 +269,6 @@ func (m *implementation) DeleteHeight(blockHeight uint64) {
 	m.pendingMu.Lock()
 	delete(m.pendingEvents, blockHeight)
 	m.pendingMu.Unlock()
-	m.RemovePendingSignedHeader(blockHeight)
 
 	// Note: txCache is intentionally NOT deleted here because:
 	// 1. Transactions are tracked by hash, not by block height (they use height 0)
@@ -464,9 +414,6 @@ func (m *implementation) ClearFromStore() error {
 	m.dataCache = NewCache(m.store, DataDAIncludedPrefix)
 	m.txCache = NewCache(nil, "")
 	m.pendingEvents = make(map[uint64]*common.DAHeightEvent)
-	m.pendingSignedHeadersMu.Lock()
-	m.pendingSignedHeaders = make(map[uint64]pendingSignedHeader)
-	m.pendingSignedHeadersMu.Unlock()
 
 	// Initialize DA height from store metadata to ensure DaHeight() is never 0.
 	m.initDAHeightFromStore(ctx)

block/internal/cache/manager_test.go

@@ -3,8 +3,6 @@ package cache
 import (
 	"context"
 	"encoding/binary"
-	"sync"
-	"sync/atomic"
 	"testing"
 	"time"
 
@@ -520,205 +518,6 @@ func TestManager_DaHeightAfterCacheClear(t *testing.T) {
 		"DaHeight should be seeded from finalized-tip metadata even after ClearCache")
 }
 
-// builds a minimal SignedHeader; variant differentiates hashes at the same height.
-func signedHeaderForHeight(height uint64, variant byte) *types.SignedHeader {
-	return &types.SignedHeader{
-		Header: types.Header{
-			BaseHeader: types.BaseHeader{ChainID: "pending-signed", Height: height, Time: 1},
-			AppHash:    []byte{variant, variant, variant},
-		},
-	}
-}
-
-func TestManager_PendingSignedHeader_FirstWriteWins(t *testing.T) {
-	t.Parallel()
-	cfg := tempConfig(t)
-	st := testMemStore(t)
-
-	m, err := NewManager(cfg, st, zerolog.Nop())
-	require.NoError(t, err)
-
-	first := signedHeaderForHeight(5, 0x01)
-	second := signedHeaderForHeight(5, 0x02)
-	require.NotEqual(t, first.Hash().String(), second.Hash().String())
-
-	m.SetPendingSignedHeader(first, "p2p")
-	m.SetPendingSignedHeader(second, "da")
-
-	got, source, ok := m.GetPendingSignedHeader(5)
-	require.True(t, ok)
-	require.Equal(t, first.Hash().String(), got.Hash().String())
-	require.Equal(t, "p2p", source)
-}
-
-func TestManager_PendingSignedHeader_NilHeaderIgnored(t *testing.T) {
-	t.Parallel()
-	cfg := tempConfig(t)
-	st := testMemStore(t)
-
-	m, err := NewManager(cfg, st, zerolog.Nop())
-	require.NoError(t, err)
-
-	m.SetPendingSignedHeader(nil, "p2p")
-	_, _, ok := m.GetPendingSignedHeader(5)
-	require.False(t, ok)
-
-	hdr := signedHeaderForHeight(5, 0x01)
-	m.SetPendingSignedHeader(hdr, "p2p")
-	got, _, ok := m.GetPendingSignedHeader(5)
-	require.True(t, ok)
-	require.Equal(t, hdr.Hash().String(), got.Hash().String())
-}
-
-func TestManager_GetPendingSignedHeader_Miss(t *testing.T) {
-	t.Parallel()
-	cfg := tempConfig(t)
-	st := testMemStore(t)
-
-	m, err := NewManager(cfg, st, zerolog.Nop())
-	require.NoError(t, err)
-
-	hdr, source, ok := m.GetPendingSignedHeader(99)
-	require.False(t, ok)
-	require.Nil(t, hdr)
-	require.Empty(t, source)
-}
-
-func TestManager_RemovePendingSignedHeader_Idempotent(t *testing.T) {
-	t.Parallel()
-	cfg := tempConfig(t)
-	st := testMemStore(t)
-
-	m, err := NewManager(cfg, st, zerolog.Nop())
-	require.NoError(t, err)
-
-	require.NotPanics(t, func() { m.RemovePendingSignedHeader(123) })
-
-	hdr := signedHeaderForHeight(5, 0x01)
-	m.SetPendingSignedHeader(hdr, "p2p")
-	m.RemovePendingSignedHeader(5)
-	m.RemovePendingSignedHeader(5)
-	_, _, ok := m.GetPendingSignedHeader(5)
-	require.False(t, ok)
-}
-
-func TestManager_DeleteHeight_EvictsPendingSignedHeader(t *testing.T) {
-	t.Parallel()
-	cfg := tempConfig(t)
-	st := testMemStore(t)
-
-	m, err := NewManager(cfg, st, zerolog.Nop())
-	require.NoError(t, err)
-
-	hdr := signedHeaderForHeight(5, 0x01)
-	m.SetPendingSignedHeader(hdr, "p2p")
-	_, _, ok := m.GetPendingSignedHeader(5)
-	require.True(t, ok)
-
-	m.DeleteHeight(5)
-
-	_, _, ok = m.GetPendingSignedHeader(5)
-	require.False(t, ok)
-}
-
-func TestManager_ClearFromStore_ResetsPendingSignedHeaders(t *testing.T) {
-	t.Parallel()
-	cfg := tempConfig(t)
-	st := testMemStore(t)
-
-	m, err := NewManager(cfg, st, zerolog.Nop())
-	require.NoError(t, err)
-
-	m.SetPendingSignedHeader(signedHeaderForHeight(5, 0x01), "p2p")
-	m.SetPendingSignedHeader(signedHeaderForHeight(6, 0x02), "da")
-
-	impl, ok := m.(*implementation)
-	require.True(t, ok)
-	require.NoError(t, impl.ClearFromStore())
-
-	for _, h := range []uint64{5, 6} {
-		_, _, present := m.GetPendingSignedHeader(h)
-		require.False(t, present, "pending entry at %d must be cleared", h)
-	}
-}
-
-// Race-detector coverage for the pending-signed-header map. Run with -race.
-func TestManager_PendingSignedHeader_Concurrency(t *testing.T) {
-	t.Parallel()
-	cfg := tempConfig(t)
-	st := testMemStore(t)
-
-	m, err := NewManager(cfg, st, zerolog.Nop())
-	require.NoError(t, err)
-
-	const (
-		writers       = 8
-		readers       = 8
-		removers      = 4
-		heightsPerRun = 200
-	)
-
-	headers := make([]*types.SignedHeader, heightsPerRun)
-	for i := range headers {
-		headers[i] = signedHeaderForHeight(uint64(i+1), byte(i&0xff))
-	}
-
-	var (
-		wg        sync.WaitGroup
-		startCh   = make(chan struct{})
-		writerHit atomic.Int64
-		readerHit atomic.Int64
-	)
-
-	for w := range writers {
-		wg.Add(1)
-		go func(seed int) {
-			defer wg.Done()
-			<-startCh
-			for i := range heightsPerRun {
-				idx := (seed*7 + i) % heightsPerRun
-				m.SetPendingSignedHeader(headers[idx], "p2p")
-				writerHit.Add(1)
-			}
-		}(w)
-	}
-
-	for r := range readers {
-		wg.Add(1)
-		go func(seed int) {
-			defer wg.Done()
-			<-startCh
-			for i := range heightsPerRun {
-				h := uint64((seed*11+i)%heightsPerRun + 1)
-				_, _, _ = m.GetPendingSignedHeader(h)
-				readerHit.Add(1)
-			}
-		}(r)
-	}
-
-	for d := range removers {
-		wg.Add(1)
-		go func(seed int) {
-			defer wg.Done()
-			<-startCh
-			for i := range heightsPerRun {
-				h := uint64((seed*13+i)%heightsPerRun + 1)
-				if i%2 == 0 {
-					m.RemovePendingSignedHeader(h)
-				} else {
-					m.DeleteHeight(h)
-				}
-			}
-		}(d)
-	}
-
-	close(startCh)
-	wg.Wait()
-
-	require.Equal(t, int64(writers*heightsPerRun), writerHit.Load())
-	require.Equal(t, int64(readers*heightsPerRun), readerHit.Load())
-}
-
 func TestManager_DaHeightFromStoreOnRestore(t *testing.T) {
 	t.Parallel()