From 54b01160b29dcc33dd37dbd38197fb7cef96f0fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mom=C4=8Dilo=20Miladinovi=C4=87?= Date: Tue, 11 Aug 2026 15:05:29 +0200 Subject: [PATCH 1/2] fix(runner): conclude benign ErrNoValidDutiesToExecute as not_required, not failed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A committee or aggregator-committee duty that reaches post-consensus quorum but leaves this operator with no beacon objects to submit is a normal "nothing to do" terminal (divergent validator sets across the committee's operators), yet both runners classified it as a failed outcome — emitting a spurious "duty failed" warning and a false ssv.runner.duty.outcome=failed data point, while the queue simultaneously treated the same sentinel as a benign terminal drop. Conclude the branch as not_required instead, in both runners: - AggregatorCommitteeRunner: direct swap of markDutyFailed for markDutyNotRequired at the len(beaconObjects) == 0 branch. - CommitteeRunner: markDutyNotRequired before the sentinel return; the deferred markDutyFailed becomes a no-op via concludeDuty idempotency. - CommitteeRunner.ProcessConsensus: the zero-valid-duties sentinel had no marker at all (false "stuck"); conclude it as not_required too. The sentinel is still returned in every case, preserving committee_queue's terminal-drop handling. Genuine terminal failures (expected-roots errors, terminal BLS reconstruction, submit failures) remain classified failed. Closes #2903 --- .../v2/ssv/runner/aggregator_committee.go | 9 +- .../ssv/runner/aggregator_committee_test.go | 40 +++++++++ protocol/v2/ssv/runner/committee.go | 13 +++ ...ittee_postconsensus_classification_test.go | 82 +++++++++++++++++++ 4 files changed, 140 insertions(+), 4 deletions(-) diff --git a/protocol/v2/ssv/runner/aggregator_committee.go b/protocol/v2/ssv/runner/aggregator_committee.go index 1d6a34499f..749f803002 100644 --- a/protocol/v2/ssv/runner/aggregator_committee.go +++ b/protocol/v2/ssv/runner/aggregator_committee.go @@ -887,10 +887,11 @@ func (r *AggregatorCommitteeRunner) ProcessPostConsensus( return fmt.Errorf("could not get expected post consensus roots and beacon objects: %w", err) } if len(beaconObjects) == 0 { - // Empty post-quorum (all beacon objects failed to build) is terminal and non-recoverable: - // committee_queue drops the message and terminates the runner on this error. Classify as - // failed (matching CommitteeRunner) rather than leaving the watcher to report a false stuck. - r.markDutyFailed(ErrNoValidDutiesToExecute) + // Benign terminal: consensus reached but this operator has nothing to submit (no aggregators + // or contributors assigned to it in the decided data). Conclude as not_required (matching + // CommitteeRunner) — neither a false "stuck" nor a spurious "failed". The sentinel still + // tells committee_queue to drop the message and terminate the runner. + r.markDutyNotRequired() return ErrNoValidDutiesToExecute } diff --git a/protocol/v2/ssv/runner/aggregator_committee_test.go b/protocol/v2/ssv/runner/aggregator_committee_test.go index 818beb5a60..a8446d4050 100644 --- a/protocol/v2/ssv/runner/aggregator_committee_test.go +++ b/protocol/v2/ssv/runner/aggregator_committee_test.go @@ -172,6 +172,46 @@ func TestAggregatorCommitteeRunnerProcessPostConsensus_MarksFailedOnSubmitError( require.False(t, env.runner.State.Succeeded, "a failed duty must not be marked succeeded") } +// TestAggregatorCommitteeRunnerProcessPostConsensus_MarksNotRequiredOnNoBeaconObjects is the +// regression test for #2903: a post-consensus quorum where the decided data leaves this operator +// with no beacon objects to submit is a benign terminal and must conclude not_required — not failed +// (the previous behavior, surfacing as a spurious "⚠️ duty failed") — while still surfacing the +// sentinel for the queue's terminal-drop handling. The decided value is swapped after consensus for +// one with no aggregators or contributors to model the empty-objects terminal. +func TestAggregatorCommitteeRunnerProcessPostConsensus_MarksNotRequiredOnNoBeaconObjects(t *testing.T) { + ctx := t.Context() + const version = spec.DataVersionElectra + + base := protocoltesting.NewTestingBeaconNodeWrapped().(*protocoltesting.BeaconNodeWrapped) + env := newAggregatorCommitteeRunnerEnv(t, []int{1}, base) + duty := spectestingutils.TestingAggregatorCommitteeDutyForValidators([]int{1}, []int{}, version) + + concluded := env.startAndFeedThroughConsensus(t, ctx, duty, version) + + emptyDecided := &spectypes.AggregatorCommitteeConsensusData{Version: version} + encoded, err := emptyDecided.Encode() + require.NoError(t, err) + env.runner.State.DecidedValue = encoded + + var postConsensusErr error + for _, psig := range postConsensusMsgsFromFixture(duty, env.keySetMap, version) { + if err := env.runner.ProcessPostConsensus(ctx, env.logger, psig); err != nil { + postConsensusErr = err + } + } + + require.ErrorIs(t, postConsensusErr, ErrNoValidDutiesToExecute, "the benign sentinel must surface to the queue") + + select { + case c := <-concluded: + require.Equal(t, dutyOutcomeNotRequired, c.outcome, "no beacon objects to submit must conclude not_required, not failed") + require.NoError(t, c.reason) + default: + t.Fatal("expected a not_required duty conclusion, got none") + } + require.True(t, env.runner.State.Succeeded, "not_required is a correct completion") +} + // TestAggregatorCommitteeRunnerProcessPostConsensus_DoesNotMarkFailedOnInvalidSigs asserts that the // recoverable reconstruct-invalid-signatures case is NOT concluded failed: the root can later re-cross // quorum on a subsequent message, so concluding here would mask a duty that still completes. diff --git a/protocol/v2/ssv/runner/committee.go b/protocol/v2/ssv/runner/committee.go index 47f26d8f50..6cd8a4a1ae 100644 --- a/protocol/v2/ssv/runner/committee.go +++ b/protocol/v2/ssv/runner/committee.go @@ -379,6 +379,10 @@ listener: ) if totalAttestations == 0 && totalSyncCommittee == 0 { + // Benign terminal: the committee decided but this operator ended up with zero valid duties to + // sign. Conclude as not_required so the watcher doesn't report a false "stuck"; the sentinel + // still tells committee_queue to drop the message and terminate the runner. + r.markDutyNotRequired() return ErrNoValidDutiesToExecute } @@ -514,6 +518,9 @@ func (r *CommitteeRunner) ProcessPostConsensus(ctx context.Context, logger *zap. // are tagged recoverableReconstructError and must not be recorded as failed. // Shutdown (context cancellation) needs no special-casing — markDutyFailed drops a context.Canceled // reason, so a submission aborted by shutdown isn't recorded as a failure. + // The benign no-beacon-objects sentinel (ErrNoValidDutiesToExecute) pre-concludes the duty as + // not_required before returning, which makes this deferred markDutyFailed a no-op (concludeDuty + // is idempotent) — it must not be recorded as failed either. defer func() { if err != nil && !isRecoverableReconstructError(err) { r.markDutyFailed(err) @@ -529,6 +536,12 @@ func (r *CommitteeRunner) ProcessPostConsensus(ctx context.Context, logger *zap. return fmt.Errorf("could not get expected post consensus roots and beacon objects: %w", err) } if len(beaconObjects) == 0 { + // Benign terminal: the committee reached consensus but this operator has no beacon objects to + // submit (e.g. divergent validator sets across the committee's operators). Conclude as + // not_required — not failed — before returning the sentinel; concludeDuty is idempotent, so + // the deferred markDutyFailed becomes a no-op. The sentinel still tells committee_queue to + // drop the message and terminate the runner. + r.markDutyNotRequired() return ErrNoValidDutiesToExecute } diff --git a/protocol/v2/ssv/runner/committee_postconsensus_classification_test.go b/protocol/v2/ssv/runner/committee_postconsensus_classification_test.go index f3464ecee6..26a891cfa6 100644 --- a/protocol/v2/ssv/runner/committee_postconsensus_classification_test.go +++ b/protocol/v2/ssv/runner/committee_postconsensus_classification_test.go @@ -122,3 +122,85 @@ func TestCommitteeRunnerProcessPostConsensus_RecoverableInvalidSigsThenSucceeds( t.Fatal("expected a succeeded duty conclusion after recovery, got none") } } + +// invalidateDutiesInGuard marks every validator duty of the committee duty invalid in the guard +// stub, so expectedPostConsensusRootsAndBeaconObjects (and the ProcessConsensus signing loop) skips +// them all. +func invalidateDutiesInGuard(guard *committeeDutyGuardStub, duty *spectypes.CommitteeDuty) { + guard.validErrs = make(map[string]error) + for _, vd := range duty.ValidatorDuties { + key := guard.validKey(vd.Type, spectypes.ValidatorPK(vd.PubKey), vd.DutySlot()) + guard.validErrs[key] = errors.New("duty no longer valid") + } +} + +// TestCommitteeRunnerProcessPostConsensus_MarksNotRequiredOnNoBeaconObjects is the regression test +// for #2903: a post-consensus quorum where this operator ends up with no beacon objects to submit +// (e.g. divergent validator sets across the committee's operators — modeled here by invalidating +// the duties in the guard after consensus) is a benign terminal. It must conclude not_required — +// not failed (the previous behavior, surfacing as a spurious "⚠️ duty failed") and not a silent +// stall — while still surfacing the sentinel for the queue's terminal-drop handling. +func TestCommitteeRunnerProcessPostConsensus_MarksNotRequiredOnNoBeaconObjects(t *testing.T) { + guard := &committeeDutyGuardStub{} + env := newCommitteeRunnerEnv(t, []int{1}, guard, &doppelgangerStub{}) + duty := spectestingutils.TestingCommitteeDuty([]int{1}, nil, spec.DataVersionElectra) + + env.startAndDecideCommitteeDuty(t, duty) + concluded := observeConclusion(env) + + invalidateDutiesInGuard(guard, duty) + + var postConsensusErr error + for id := spectypes.OperatorID(1); id <= 3; id++ { + msg := spectestingutils.PostConsensusCommitteeMsgForDuty(duty, env.keySetMap, id) + if err := env.runner.ProcessPostConsensus(context.Background(), env.logger, msg); err != nil { + postConsensusErr = err + } + } + + require.ErrorIs(t, postConsensusErr, ErrNoValidDutiesToExecute, "the benign sentinel must surface to the queue") + + select { + case c := <-concluded: + require.Equal(t, dutyOutcomeNotRequired, c.outcome, "no beacon objects to submit must conclude not_required, not failed") + require.NoError(t, c.reason) + default: + t.Fatal("expected a not_required duty conclusion, got none") + } + require.True(t, env.runner.State.Succeeded, "not_required is a correct completion") + require.Empty(t, env.beacon.GetBroadcastedRoots(), "nothing should have been submitted") +} + +// TestCommitteeRunnerProcessConsensus_MarksNotRequiredOnNoValidDuties covers the consensus-phase +// sibling of the #2903 sentinel: a committee that decides while this operator has zero valid duties +// to sign (all invalidated in the guard before consensus) previously concluded via no marker at +// all, surfacing as a false "stuck". It must conclude not_required and surface the sentinel. +func TestCommitteeRunnerProcessConsensus_MarksNotRequiredOnNoValidDuties(t *testing.T) { + guard := &committeeDutyGuardStub{} + env := newCommitteeRunnerEnv(t, []int{1}, guard, &doppelgangerStub{}) + duty := spectestingutils.TestingCommitteeDuty([]int{1}, nil, spec.DataVersionElectra) + + ctx := t.Context() + require.NoError(t, env.runner.StartNewDuty(ctx, env.logger, duty, env.sampleKey.Threshold)) + concluded := observeConclusion(env) + + invalidateDutiesInGuard(guard, duty) + + var consensusErr error + for _, msg := range spectestingutils.CommitteeInputForDuty(duty, duty.Slot, env.keySetMap, false) { + if err := env.runner.ProcessConsensus(ctx, env.logger, msg); err != nil { + consensusErr = err + } + } + + require.ErrorIs(t, consensusErr, ErrNoValidDutiesToExecute, "the benign sentinel must surface to the queue") + + select { + case c := <-concluded: + require.Equal(t, dutyOutcomeNotRequired, c.outcome, "deciding with zero valid duties must conclude not_required") + require.NoError(t, c.reason) + default: + t.Fatal("expected a not_required duty conclusion, got none") + } + require.True(t, env.runner.State.Succeeded, "not_required is a correct completion") +} From 5cbe3bc5cc82b9dee463e83aeb3ee07c28762c2e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mom=C4=8Dilo=20Miladinovi=C4=87?= Date: Tue, 11 Aug 2026 15:55:22 +0200 Subject: [PATCH 2/2] fix(validator): quiet the queue's benign no-duties terminal drop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The committee_queue case that catches ErrNoValidDutiesToExecute logged "❗ could not handle message, dropping message and terminating committee-runner" at Error level and marked the trace span as Error — the same false alarm as the failed outcome classification, fired by the same benign trigger. The runner now concludes the duty as not_required on this sentinel, so the drop is a correct completion: log it at Debug with neutral wording and set the span status to Ok. The drop-and- terminate behavior is unchanged. --- protocol/v2/ssv/validator/committee_queue.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/protocol/v2/ssv/validator/committee_queue.go b/protocol/v2/ssv/validator/committee_queue.go index 8b217ea34b..55baca7359 100644 --- a/protocol/v2/ssv/validator/committee_queue.go +++ b/protocol/v2/ssv/validator/committee_queue.go @@ -228,13 +228,16 @@ func (c *Committee) ConsumeQueue( const couldNotHandleMsgLogPrefix = "could not handle message, " switch { case errors.Is(err, runner.ErrNoValidDutiesToExecute): - const droppingMsgDueToNoValidDutiesToExecuteEvent = "❗ " + couldNotHandleMsgLogPrefix + "dropping message and terminating committee-runner" - msgLogger.Error(droppingMsgDueToNoValidDutiesToExecuteEvent, zap.Error(err)) + // Benign terminal, not a handling failure: the committee decided but this operator has + // no duties to execute (the runner already concluded the duty as not_required), so the + // message is dropped and the runner terminated without error-level noise. + const droppingMsgDueToNoValidDutiesToExecuteEvent = "no valid duties to execute, dropping message and terminating committee-runner" + msgLogger.Debug(droppingMsgDueToNoValidDutiesToExecuteEvent, zap.Error(err)) msgState.span.AddEvent(droppingMsgDueToNoValidDutiesToExecuteEvent, trace.WithAttributes( attribute.String("drop_reason", err.Error()), attribute.Int64("attempt", currentAttempt), )) - msgState.span.SetStatus(codes.Error, droppingMsgDueToNoValidDutiesToExecuteEvent) + msgState.span.SetStatus(codes.Ok, "") msgState.span.End() msgStates.Delete(msgKey) return