diff --git a/pkg/beacon/beacon.go b/pkg/beacon/beacon.go index 0b1eb83..d58202b 100644 --- a/pkg/beacon/beacon.go +++ b/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 { diff --git a/pkg/beacon/metrics_beacon.go b/pkg/beacon/metrics_beacon.go index c485b50..e4ade28 100644 --- a/pkg/beacon/metrics_beacon.go +++ b/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 } diff --git a/pkg/beacon/slot_arithmetic_test.go b/pkg/beacon/slot_arithmetic_test.go new file mode 100644 index 0000000..4f6049a --- /dev/null +++ b/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) + } + }) + } +}