Fix uint64 underflow in slot arithmetic

pkg/beacon/beacon.go

@@ -431,7 +431,12 @@ func (n *node) subscribeDownstream(ctx context.Context) error {
 			return
 		}
 
-		_, err := n.FetchBlock(ctx, fmt.Sprintf("%v", slot.Number()-1))
+		previousStateID, ok := previousSlotStateID(slot.Number())
+		if !ok {
+			return
+		}
+
+		_, err := n.FetchBlock(ctx, previousStateID)
 		if err != nil {
 			if strings.Contains(err.Error(), "404") {
 				n.publishEmptySlot(ctx, phase0.Slot(slot.Number()))
@@ -454,6 +459,18 @@ func (n *node) subscribeDownstream(ctx context.Context) error {
 	return nil
 }
 
+// previousSlotStateID returns the state ID for the slot before slotNumber,
+// and false for slot 0, which has no previous slot. slotNumber is a
+// uint64, so slotNumber-1 at slot 0 would otherwise underflow to the
+// maximum uint64 value instead of a meaningful state ID.
+func previousSlotStateID(slotNumber uint64) (string, bool) {
+	if slotNumber == 0 {
+		return "", false
+	}
+
+	return fmt.Sprintf("%v", slotNumber-1), true
+}
+
 func (n *node) fetchIsHealthy(ctx context.Context) error {
 	provider, isProvider := n.client.(eth2client.NodeSyncingProvider)
 	if !isProvider {

pkg/beacon/metrics_beacon.go

@@ -336,6 +336,15 @@ func (b *BeaconMetrics) handleEmptySlot(ctx context.Context, event *EmptySlotEve
 	return nil
 }
 
+// blockTooOldForProposerDelay reports whether a block's slot is more than
+// 2 slots behind the current wallclock slot. Both slot numbers are
+// uint64, so a block at or ahead of the current slot must be checked
+// separately from the subtraction, otherwise it underflows and looks
+// like a block from far in the past instead of a current or future one.
+func blockTooOldForProposerDelay(currSlotNumber, blockSlotNumber uint64) bool {
+	return currSlotNumber > blockSlotNumber && currSlotNumber-blockSlotNumber > 2
+}
+
 func (b *BeaconMetrics) handleBlock(ctx context.Context, event *v1.BlockEvent) error {
 	syncState, err := b.beaconNode.SyncState()
 	if err != nil {
@@ -354,7 +363,7 @@ func (b *BeaconMetrics) handleBlock(ctx context.Context, event *v1.BlockEvent) e
 	}
 
 	// We don't care about blocks that are more than 2 slots in the past.
-	if currSlot.Number()-slot.Number() > 2 {
+	if blockTooOldForProposerDelay(currSlot.Number(), slot.Number()) {
 		return nil
 	}
 

pkg/beacon/slot_arithmetic_test.go

@@ -0,0 +1,106 @@
+package beacon
+
+import "testing"
+
+func TestPreviousSlotStateID(t *testing.T) {
+	tests := []struct {
+		name       string
+		slotNumber uint64
+		expectedID string
+		expectedOK bool
+	}{
+		{
+			name:       "slot 0 has no previous slot",
+			slotNumber: 0,
+			expectedID: "",
+			expectedOK: false,
+		},
+		{
+			name:       "slot 1 returns slot 0",
+			slotNumber: 1,
+			expectedID: "0",
+			expectedOK: true,
+		},
+		{
+			name:       "an ordinary slot returns the slot before it",
+			slotNumber: 12345,
+			expectedID: "12344",
+			expectedOK: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			id, ok := previousSlotStateID(tt.slotNumber)
+			if ok != tt.expectedOK {
+				t.Fatalf("expected ok=%v, got %v", tt.expectedOK, ok)
+			}
+
+			if id != tt.expectedID {
+				t.Fatalf("expected id=%q, got %q", tt.expectedID, id)
+			}
+		})
+	}
+}
+
+func TestBlockTooOldForProposerDelay(t *testing.T) {
+	tests := []struct {
+		name            string
+		currSlotNumber  uint64
+		blockSlotNumber uint64
+		expected        bool
+	}{
+		{
+			name:            "block at the current slot is not too old",
+			currSlotNumber:  1000,
+			blockSlotNumber: 1000,
+			expected:        false,
+		},
+		{
+			name:            "block 1 slot in the future is not too old",
+			currSlotNumber:  1000,
+			blockSlotNumber: 1001,
+			expected:        false,
+		},
+		{
+			name:            "block far in the future is not too old",
+			currSlotNumber:  1000,
+			blockSlotNumber: 5000,
+			expected:        false,
+		},
+		{
+			name:            "block 2 slots in the past is not too old",
+			currSlotNumber:  1000,
+			blockSlotNumber: 998,
+			expected:        false,
+		},
+		{
+			name:            "block 3 slots in the past is too old",
+			currSlotNumber:  1000,
+			blockSlotNumber: 997,
+			expected:        true,
+		},
+		{
+			name:            "block at slot 0 with current slot far ahead is too old",
+			currSlotNumber:  1000,
+			blockSlotNumber: 0,
+			expected:        true,
+		},
+		{
+			name:            "current slot at 0 with a future block is not too old",
+			currSlotNumber:  0,
+			blockSlotNumber: 5,
+			expected:        false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := blockTooOldForProposerDelay(tt.currSlotNumber, tt.blockSlotNumber)
+			if got != tt.expected {
+				t.Fatalf("blockTooOldForProposerDelay(%d, %d) = %v, want %v",
+					tt.currSlotNumber, tt.blockSlotNumber, got, tt.expected)
+			}
+		})
+	}
+}