Do not wake-up notifier if preference has been set to its previous one

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

Author: yacovm

snow/engine/snowman/block/notifier.go

@@ -31,6 +31,9 @@ type ChangeNotifier struct {
 	// This is needed in case a block has been accepted that changes when a VM considers the need to build a block.
 	// In order for the subscription to be correlated to the latest data, it needs to be retried.
 	OnChange func()
+	// lastPref is the last block ID that was set as preferred via SetPreference.
+	lastPref ids.ID
+	invoked  bool
 }
 
 func (cn *ChangeNotifier) GetAncestors(ctx context.Context, blkID ids.ID, maxBlocksNum int, maxBlocksSize int, maxBlocksRetrivalTime time.Duration) ([][]byte, error) {
@@ -83,7 +86,13 @@ func (cn *ChangeNotifier) GetStateSummary(ctx context.Context, summaryHeight uin
 }
 
 func (cn *ChangeNotifier) SetPreference(ctx context.Context, blkID ids.ID) error {
-	defer cn.OnChange()
+	// Only call OnChange if the preference has changed.
+	if !cn.invoked || cn.lastPref != blkID {
+		cn.lastPref = blkID
+		cn.invoked = true
+		defer cn.OnChange()
+	}
+
 	return cn.ChainVM.SetPreference(ctx, blkID)
 }
 

snow/engine/snowman/block/notifier_test.go

@@ -224,3 +224,38 @@ func TestChangeNotifierNormal(t *testing.T) {
 		})
 	}
 }
+
+func TestChangeNotifierSetPreference(t *testing.T) {
+	ctrl := gomock.NewController(t)
+
+	tvm := blockmock.NewFullVM(ctrl)
+	tvm.EXPECT().SetPreference(gomock.Any(), gomock.Any()).Return(nil).AnyTimes()
+
+	var invoked bool
+	nf := block.ChangeNotifier{
+		OnChange: func() {
+			invoked = true
+		},
+		ChainVM: tvm,
+	}
+
+	// First time SetPreference is called, it should invoke OnChange
+	require.NoError(t, nf.SetPreference(context.Background(), ids.Empty), "expected SetPreference to succeed")
+	require.True(t, invoked, "expected to have been invoked on first SetPreference call")
+
+	invoked = false
+	// Second time SetPreference is called with the same block ID, it should not invoke OnChange
+	require.NoError(t, nf.SetPreference(context.Background(), ids.Empty), "expected SetPreference to succeed on second call with same block ID")
+	require.False(t, invoked, "expected not to have been invoked on second SetPreference call with same block ID")
+
+	invoked = false
+	// Third time SetPreference is called with a different block ID, it should invoke OnChange again
+	testID := ids.GenerateTestID()
+	require.NoError(t, nf.SetPreference(context.Background(), testID), "expected SetPreference to succeed on third call with different block ID")
+	require.True(t, invoked, "expected to have been invoked on third SetPreference call with different block ID")
+
+	invoked = false
+	// Fourth time SetPreference is called with the same block ID, it should not invoke OnChange
+	require.NoError(t, nf.SetPreference(context.Background(), testID), "expected SetPreference to succeed on fourth call with same block ID")
+	require.False(t, invoked, "expected not to have been invoked on fourth SetPreference call with same block ID")
+}