Mark ready on populated cache, not on epoch event

Author: nickgs1337

core/process/peer/peerTypeProvider.go

@@ -55,10 +55,10 @@ func NewPeerTypeProvider(arg ArgPeerTypeProvider) (*PeerTypeProvider, error) {
 	return ptp, nil
 }
 
-// IsCachePopulated returns true once the cache has been refreshed by an epoch-start event.
-// During the bootstrap window before the first epoch-start notification fires, the cache
-// is either empty or seeded from a NodesCoordinator that is not yet fully populated, so
-// callers should treat its peer-type result as unreliable.
+// IsCachePopulated returns true once at least one updateCache call has produced a
+// non-empty cache. Until then, the NodesCoordinator has not yet exposed any
+// validator keys for the active epoch, so callers should treat the peer-type result
+// as unreliable and avoid overwriting startup-seeded values.
 func (ptp *PeerTypeProvider) IsCachePopulated() bool {
 	ptp.mutCache.RLock()
 	defer ptp.mutCache.RUnlock()
@@ -103,7 +103,6 @@ func (ptp *PeerTypeProvider) epochStartEventHandler() sharding.EpochStartActionH
 				"slot", hdr.GetSlot(),
 				"epoch", hdr.GetEpoch())
 			ptp.updateCache(hdr.GetEpoch())
-			ptp.markReady()
 		},
 		func(_ data.HeaderHandler) {},
 		core.IndexerOrder,
@@ -117,12 +116,9 @@ func (ptp *PeerTypeProvider) updateCache(epoch uint32) {
 
 	ptp.mutCache.Lock()
 	ptp.cache = newCache
-	ptp.mutCache.Unlock()
-}
-
-func (ptp *PeerTypeProvider) markReady() {
-	ptp.mutCache.Lock()
-	ptp.isReady = true
+	if len(newCache) > 0 {
+		ptp.isReady = true
+	}
 	ptp.mutCache.Unlock()
 }
 

core/process/peer/peerTypeProvider_test.go

@@ -205,7 +205,7 @@ func TestNewPeerTypeProvider_ComputeForKeyNotFoundInCacheReturnsObserver(t *test
 	assert.Nil(t, err)
 }
 
-func TestPeerTypeProvider_IsCachePopulated_FalseAtConstruction(t *testing.T) {
+func TestPeerTypeProvider_IsCachePopulated_FalseWhenCoordinatorReturnsEmpty(t *testing.T) {
 	arg := createDefaultArgPeerTypeProvider()
 
 	ptp, err := NewPeerTypeProvider(arg)
@@ -215,21 +215,57 @@ func TestPeerTypeProvider_IsCachePopulated_FalseAtConstruction(t *testing.T) {
 	assert.False(t, ptp.IsCachePopulated())
 }
 
-func TestPeerTypeProvider_IsCachePopulated_TrueAfterEpochStart(t *testing.T) {
+func TestPeerTypeProvider_IsCachePopulated_TrueAtConstructionWhenCachePopulated(t *testing.T) {
+	arg := createDefaultArgPeerTypeProvider()
+	arg.NodesCoordinator = &mock.NodesCoordinatorMock{
+		GetAllElectedValidatorsKeysCalled: func() ([][]byte, error) {
+			return [][]byte{[]byte("validator-pk")}, nil
+		},
+	}
+
+	ptp, err := NewPeerTypeProvider(arg)
+	require.Nil(t, err)
+	require.NotNil(t, ptp)
+
+	assert.True(t, ptp.IsCachePopulated())
+}
+
+func TestPeerTypeProvider_IsCachePopulated_TrueAfterEpochStartFillsCache(t *testing.T) {
 	arg := createDefaultArgPeerTypeProvider()
 	epochStartNotifier := &mock.EpochStartNotifierStub{}
 	arg.EpochStartEventNotifier = epochStartNotifier
 
+	coordinator := &mock.NodesCoordinatorMock{}
+	arg.NodesCoordinator = coordinator
+
 	ptp, err := NewPeerTypeProvider(arg)
 	require.Nil(t, err)
 	require.NotNil(t, ptp)
 	require.False(t, ptp.IsCachePopulated())
 
+	coordinator.GetAllElectedValidatorsKeysCalled = func() ([][]byte, error) {
+		return [][]byte{[]byte("validator-pk")}, nil
+	}
 	epochStartNotifier.NotifyAll(&block.Block{Header: &block.BlockHeader{Nonce: 1, Epoch: 1}})
 
 	assert.True(t, ptp.IsCachePopulated())
 }
 
+func TestPeerTypeProvider_IsCachePopulated_StaysFalseWhenEpochStartFiresButCacheEmpty(t *testing.T) {
+	arg := createDefaultArgPeerTypeProvider()
+	epochStartNotifier := &mock.EpochStartNotifierStub{}
+	arg.EpochStartEventNotifier = epochStartNotifier
+
+	ptp, err := NewPeerTypeProvider(arg)
+	require.Nil(t, err)
+	require.NotNil(t, ptp)
+	require.False(t, ptp.IsCachePopulated())
+
+	epochStartNotifier.NotifyAll(&block.Block{Header: &block.BlockHeader{Nonce: 1, Epoch: 1}})
+
+	assert.False(t, ptp.IsCachePopulated())
+}
+
 func TestNewPeerTypeProvider_IsInterfaceNil(t *testing.T) {
 	arg := createDefaultArgPeerTypeProvider()