Address code review comments IV

Signed-off-by: Yacov Manevich <[email protected]>

ids/node_weight.go

@@ -4,6 +4,6 @@
 package ids
 
 type NodeWeight struct {
-	Node   NodeID
+	ID     NodeID
 	Weight uint64
 }

snow/engine/common/tracker/peers.go

@@ -274,7 +274,7 @@ func (p *peerData) SampleValidator() (ids.NodeID, bool) {
 func (p *peerData) GetValidators() set.Set[ids.NodeWeight] {
 	res := set.NewSet[ids.NodeWeight](len(p.validators))
 	for k, v := range p.validators {
-		res.Add(ids.NodeWeight{Node: k, Weight: v})
+		res.Add(ids.NodeWeight{ID: k, Weight: v})
 	}
 	return res
 }
@@ -285,7 +285,7 @@ func (p *peerData) ConnectedValidators() set.Set[ids.NodeWeight] {
 	copied := set.NewSet[ids.NodeWeight](len(p.connectedValidators))
 	for _, vdrID := range p.connectedValidators.List() {
 		weight := p.validators[vdrID]
-		copied.Add(ids.NodeWeight{Node: vdrID, Weight: weight})
+		copied.Add(ids.NodeWeight{ID: vdrID, Weight: weight})
 	}
 	return copied
 }

snow/engine/common/tracker/peers_test.go

@@ -58,10 +58,10 @@ func TestConnectedValidators(t *testing.T) {
 
 	require.NoError(p.Connected(context.Background(), nodeID2, version.CurrentApp))
 	require.Equal(uint64(11), p.ConnectedWeight())
-	require.True(set.Of(ids.NodeWeight{Node: nodeID1, Weight: 5}, ids.NodeWeight{Node: nodeID2, Weight: 6}).Equals(p.GetValidators()))
-	require.True(set.Of(ids.NodeWeight{Node: nodeID1, Weight: 5}, ids.NodeWeight{Node: nodeID2, Weight: 6}).Equals(p.ConnectedValidators()))
+	require.True(set.Of(ids.NodeWeight{ID: nodeID1, Weight: 5}, ids.NodeWeight{ID: nodeID2, Weight: 6}).Equals(p.GetValidators()))
+	require.True(set.Of(ids.NodeWeight{ID: nodeID1, Weight: 5}, ids.NodeWeight{ID: nodeID2, Weight: 6}).Equals(p.ConnectedValidators()))
 
 	require.NoError(p.Disconnected(context.Background(), nodeID2))
-	require.True(set.Of(ids.NodeWeight{Node: nodeID1, Weight: 5}, ids.NodeWeight{Node: nodeID2, Weight: 6}).Equals(p.GetValidators()))
-	require.True(set.Of(ids.NodeWeight{Node: nodeID1, Weight: 5}).Equals(p.ConnectedValidators()))
+	require.True(set.Of(ids.NodeWeight{ID: nodeID1, Weight: 5}, ids.NodeWeight{ID: nodeID2, Weight: 6}).Equals(p.GetValidators()))
+	require.True(set.Of(ids.NodeWeight{ID: nodeID1, Weight: 5}).Equals(p.ConnectedValidators()))
 }

snow/engine/snowman/engine_decorator.go

@@ -21,9 +21,30 @@ type decoratedEngineWithStragglerDetector struct {
 
 func NewDecoratedEngineWithStragglerDetector(e *Engine, time func() time.Time, f func(time.Duration)) common.Engine {
 	minConfRatio := float64(e.Params.AlphaConfidence) / float64(e.Params.K)
-	sd := newStragglerDetector(time, e.Config.Ctx.Log, minConfRatio, e.Consensus.LastAccepted,
-		e.Config.ConnectedValidators.ConnectedValidators, e.Config.ConnectedValidators.ConnectedPercent,
-		e.Consensus.Processing, e.acceptedFrontiers.LastAccepted)
+
+	sa := &snapshotAnalyzer{
+		log:        e.Config.Ctx.Log,
+		processing: e.Consensus.Processing,
+	}
+
+	s := &snapshotter{
+		log:                      e.Config.Ctx.Log,
+		connectedValidators:      e.Config.ConnectedValidators.ConnectedValidators,
+		minConfirmationThreshold: minConfRatio,
+		lastAcceptedByNodeID:     e.acceptedFrontiers.LastAccepted,
+		lastAccepted:             dropHeight(e.Consensus.LastAccepted),
+	}
+
+	conf := stragglerDetectorConfig{
+		getSnapshot:               s.getNetworkSnapshot,
+		areWeBehindTheRest:        sa.areWeBehindTheRest,
+		minStragglerCheckInterval: minStragglerCheckInterval,
+		log:                       e.Config.Ctx.Log,
+		getTime:                   time,
+	}
+
+	sd := newStragglerDetector(conf)
+
 	return &decoratedEngineWithStragglerDetector{
 		Engine: e,
 		f:      f,

snow/engine/snowman/engine_decorator_test.go

@@ -13,37 +13,28 @@ import (
 	"github.com/ava-labs/avalanchego/ids"
 	"github.com/ava-labs/avalanchego/snow/consensus/snowman"
 	"github.com/ava-labs/avalanchego/snow/consensus/snowman/snowmantest"
+	"github.com/ava-labs/avalanchego/utils/timer/mockable"
 )
 
 func TestEngineStragglerDetector(t *testing.T) {
 	require := require.New(t)
 
-	fakeClock := make(chan time.Time, 1)
+	var fakeClock mockable.Clock
 
 	conf := DefaultConfig(t)
 	peerID, _, sender, vm, engine := setup(t, conf)
 
 	parent := snowmantest.BuildChild(snowmantest.Genesis)
 	require.NoError(conf.Consensus.Add(parent))
 
-	listenerShouldInvokeWith := []time.Duration{0, 0, time.Second * 2}
-
-	fakeTime := func() time.Time {
-		select {
-		case now := <-fakeClock:
-			return now
-		default:
-			require.Fail("should have a time.Time in the channel")
-			return time.Time{}
-		}
-	}
+	listenerShouldInvokeWith := []time.Duration{0, 0, minStragglerCheckInterval * 2}
 
 	f := func(duration time.Duration) {
 		require.Equal(listenerShouldInvokeWith[0], duration)
 		listenerShouldInvokeWith = listenerShouldInvokeWith[1:]
 	}
 
-	decoratedEngine := NewDecoratedEngineWithStragglerDetector(engine, fakeTime, f)
+	decoratedEngine := NewDecoratedEngineWithStragglerDetector(engine, fakeClock.Time, f)
 
 	vm.GetBlockF = func(_ context.Context, blkID ids.ID) (snowman.Block, error) {
 		switch blkID {
@@ -62,13 +53,13 @@ func TestEngineStragglerDetector(t *testing.T) {
 	}
 
 	now := time.Now()
-	fakeClock <- now
+	fakeClock.Set(now)
 	require.NoError(decoratedEngine.Chits(context.Background(), peerID, 0, parent.ID(), parent.ID(), parent.ID()))
-	now = now.Add(time.Second * 2)
-	fakeClock <- now
+	now = now.Add(minStragglerCheckInterval * 2)
+	fakeClock.Set(now)
 	require.NoError(decoratedEngine.Chits(context.Background(), peerID, 0, parent.ID(), parent.ID(), parent.ID()))
-	now = now.Add(time.Second * 2)
-	fakeClock <- now
+	now = now.Add(minStragglerCheckInterval * 2)
+	fakeClock.Set(now)
 	require.NoError(decoratedEngine.Chits(context.Background(), peerID, 0, parent.ID(), parent.ID(), parent.ID()))
 	require.Empty(listenerShouldInvokeWith)
 }