diff --git a/core/tracker/inclusion.go b/core/tracker/inclusion.go index 70c9f1960..8d4c77b32 100644 --- a/core/tracker/inclusion.go +++ b/core/tracker/inclusion.go @@ -6,6 +6,7 @@ import ( "context" "fmt" "maps" + "math" "net/http" "slices" "strconv" @@ -589,7 +590,9 @@ func (a *InclusionChecker) Run(ctx context.Context) { } var ( - checkedSlot uint64 + // MaxUint64 sentinel so the first computed slot (0 at chain start) + // is not skipped as already-checked. + checkedSlot = uint64(math.MaxUint64) attesterDuties []*eth2v1.AttesterDuty ) @@ -598,7 +601,15 @@ func (a *InclusionChecker) Run(ctx context.Context) { case <-ctx.Done(): return case <-ticker.C: - slot := uint64(time.Since(a.genesis)/a.slotDuration) - InclCheckLag + // Skip until a slot is old enough to check: the unsigned + // arithmetic below underflows before genesis (negative elapsed + // time) and during the first InclCheckLag slots. + sinceGenesis := time.Since(a.genesis) + if sinceGenesis < a.slotDuration*InclCheckLag { + continue + } + + slot := uint64(sinceGenesis/a.slotDuration) - InclCheckLag if checkedSlot == slot { continue } @@ -656,7 +667,13 @@ func (a *InclusionChecker) Run(ctx context.Context) { } checkedSlot = slot - a.core.Trim(ctx, slot-InclMissedLag) + + // Only trim once a slot is old enough to be declared missed: + // slot-InclMissedLag underflows otherwise, and Trim would then + // report every pending submission as not included on-chain. + if slot >= InclMissedLag { + a.core.Trim(ctx, slot-InclMissedLag) + } } } } diff --git a/core/tracker/inclusion_internal_test.go b/core/tracker/inclusion_internal_test.go index 84911f7e3..df1c6ae59 100644 --- a/core/tracker/inclusion_internal_test.go +++ b/core/tracker/inclusion_internal_test.go @@ -6,7 +6,9 @@ import ( "context" "math/rand" "slices" + "sync" "testing" + "time" "github.com/OffchainLabs/go-bitfield" eth2api "github.com/attestantio/go-eth2-client/api" @@ -579,3 +581,81 @@ func TestInclusion404Handling(t *testing.T) { require.Error(t, err, "checkBlockAndAtts should return an error for non-404 errors") }) } + +// TestRunNoFalseMissesAtChainStart is a regression test for the first slots +// after genesis: the trim-slot arithmetic underflowed, so Trim deleted every +// pending submission and reported fresh duties as "duty not included +// on-chain", and the zero-valued checkedSlot skipped the genesis slot's +// inclusion check entirely. +func TestRunNoFalseMissesAtChainStart(t *testing.T) { + // Run must take the checkBlockFunc path below, not checkBlockAndAttsFunc. + featureset.DisableForT(t, featureset.AttestationInclusion) + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + bmock, err := beaconmock.New(ctx) + require.NoError(t, err) + + var ( + mu sync.Mutex + missed []core.Duty + ) + + incl := &inclusionCore{ + missedFunc: func(ctx context.Context, sub submission) { + mu.Lock() + defer mu.Unlock() + + missed = append(missed, sub.Duty) + }, + trackerInclFunc: func(duty core.Duty, key core.PubKey, data core.SignedData, err error) {}, + submissions: make(map[subkey]submission), + beaconCommittees: make(map[eth2p0.Slot][]*eth2v1.BeaconCommittee), + } + + // Pin the checked slot to 0 for the whole test: genesis InclCheckLag + // slots ago with a slot duration far longer than the test. + checked := make(chan uint64, 1) + checker := &InclusionChecker{ + core: incl, + eth2Cl: bmock, + genesis: time.Now().Add(-InclCheckLag * time.Hour), + slotDuration: time.Hour, + checkBlockFunc: func(ctx context.Context, slot uint64, found bool) { + incl.CheckBlock(ctx, slot, found) + + select { + case checked <- slot: + default: + } + }, + } + + // A pending proposal from slot 3: too recent to be declared missed. + block := testutil.RandomDenebVersionedSignedProposal() + coreBlock, err := core.NewVersionedSignedProposal(block) + require.NoError(t, err) + require.NoError(t, incl.Submitted(core.NewProposerDuty(3), "", coreBlock, 0)) + + var wg sync.WaitGroup + + wg.Go(func() { + checker.Run(ctx) + }) + + select { + case slot := <-checked: + require.Zero(t, slot, "the genesis slot must be checked, not skipped as already-checked") + case <-time.After(10 * time.Second): + t.Fatal("timed out waiting for the genesis slot inclusion check") + } + + cancel() + wg.Wait() // Run exits only after the tick completes, including any trim. + + mu.Lock() + defer mu.Unlock() + + require.Empty(t, missed, "fresh submissions must not be reported missed right after genesis") +}