Boole → stage convergence — final consolidation - #2941
Open
momosh-ssv wants to merge 195 commits into
Open
Conversation
…T-LOAD-0)
Step 0 of the boole-fork -> stage convergence (base: integration/boole-convergence).
ssv-spec v1.2.3 removes RoleAggregator/RoleSyncCommitteeContribution (their wire
values 1/3 are left vacant; new RoleAggregatorCommittee=6) and renames
ValidatorConsensusData -> ProposerConsensusData (same SSZ shape). Stage referenced
the removed symbols across ~25 production files, so the pin bump alone cannot build.
Port boole's compat shim rather than pulling the runner merge forward:
- protocol/v2/types/{runner_role,consensus_data,partial_sig_message}.go: legacy
roles pinned at their v1.2.2 wire values (RoleAggregator=1,
RoleSyncCommitteeContribution=3) + fork-aware mapping helpers (boole-faithful).
- Every duty->runner-role call site uses RunnerRoleForValidatorDuty(duty, false)
(literal false = legacy mapping; TODO(convergence unit 5) threads the real fork
bit), so behavior is IDENTICAL to stage today: two separate legacy runners, same
wire bytes.
- committee.go validateMessage: add the v1.2.3-required wrong-role rejection
(spectypes.CommitteeWrongRoleErrorCode) -- the one intended behavior change.
- spectest: skip the 528 AggregatorCommittee-scoped vectors (525 msg-processing/
valcheck/construction + 3 SyncCommitteeAggregatorProof) that require the
AggregatorCommitteeRunner (unit 5). The 3 SyncCommitteeAggregatorProof skips are
a temporary coverage regression (green on v1.2.2) -- TODO-tagged. Also handles
the v1.2.3 Committee.Runners -> CommitteeRunners fixture rename (legacy fallback).
Wire-compat verified: no surviving role renumbered, 1/3 not recycled,
ProposerConsensusData SSZ byte-identical to v1.2.2 ValidatorConsensusData.
Deep-reviewed (behavior identity, wire compat, skip scoping): APPROVE.
go build/vet clean; qbft + ssv spec-tests green; golangci-lint 0 issues.
Unit 1 of the boole-fork -> stage convergence (stacks on FRONT-LOAD-0 / #2924). Adds boole's fork layer to networkconfig, boole-faithful (the worktree-vs-boole diff on networkconfig/ reduces to exactly the intentional naming delta): - ssv.go: NextDomainType + SSVForks{Boole phase0.Epoch} (was empty) + marshal/ unmarshal handling (absent next_domain_type defaults to DomainType; 4-byte check). - network.go: DomainTypeAtSlot, CurrentDomainType, BooleFork, BooleForkAtEpoch, BooleForkAtSlot, InBooleTransitionWindow + private window helpers. - Per-network values: prod nets (mainnet/holesky/holesky-stage/hoodi/hoodi-stage/ sepolia) pin Boole=MaxUint64 (inert); testnet/local-testnet use 0. NextDomainType = DomainType+1 everywhere. - test-network.go: TestNetwork Boole default + SSV_TEST_BOOLE_FORK pre/post env flip; test-network_alan.go (alan_spec builds pin MaxUint64). - Ports boole's fork tests (TestDomainTypeAtSlot, transition-window tests, marshal tests). Naming rule (converge toward stage): beacon.go keeps stage's ForkAtEpoch; boole's BeaconForkAtEpoch rename is intentionally NOT ported. The API is deliberately DORMANT on this branch (verified: zero consumers outside networkconfig/) until units 2 (networking) and 5 (runner) wire it. Deep-reviewed APPROVE: upgrade contract safe (ConfigLock compares only NetworkName/UsingLocalEvents/UsingSSVSigner; StorageName unchanged -> no boot-mismatch on upgrade). Carry-forward before wiring BooleFork (unit 2/5): a custom-network YAML omitting `forks` deserializes to Boole=0 (fork-at-genesis) -- resolve (require explicit forks, or default absent to MaxUint64) before the first unit that consumes BooleFork. go build clean; networkconfig tests green; golangci-lint 0 issues.
…gateAndProof v1.2.3 changed testingutils.TestingPhase0AggregateAndProof from a value to a func(aggregatorIndex phase0.ValidatorIndex) *phase0.AggregateAndProof; invoke it so SignBeaconObject receives an ssz.HashRoot. ssvsigner is a separate Go module, so `go build ./...` from the main module never compiled it — which is why this slipped past local validation and only CI (lint/ssv/ssvsigner) caught it.
…gateAndProof v1.2.3 changed testingutils.TestingPhase0AggregateAndProof from a value to a func(aggregatorIndex phase0.ValidatorIndex) *phase0.AggregateAndProof; invoke it so SignBeaconObject receives an ssz.HashRoot. ssvsigner is a separate Go module, so `go build ./...` from the main module never compiled it — which is why this slipped past local validation and only CI (lint/ssv/ssvsigner) caught it.
…tisfy deadcode CommitteeRunnerRoleForBeaconRole, CommitteeSignerBucketForBeaconRole (+ its CommitteeSignerBucket type/consts) and the duplicate types.RunnerRoleToString were ported ahead of their consumers. deadcode analyses reachability from ./cmd/ssvnode and flagged all three as unreachable. The used RunnerRoleToString lives in protocol/v2/message/msg.go; the committee-bucket helpers land with the aggregator-committee trace wiring in a later unit. Removing them here keeps front-load-0 to spec-bump + reachable compat shim only.
…tisfy deadcode CommitteeRunnerRoleForBeaconRole, CommitteeSignerBucketForBeaconRole (+ its CommitteeSignerBucket type/consts) and the duplicate types.RunnerRoleToString were ported ahead of their consumers. deadcode analyses reachability from ./cmd/ssvnode and flagged all three as unreachable. The used RunnerRoleToString lives in protocol/v2/message/msg.go; the committee-bucket helpers land with the aggregator-committee trace wiring in a later unit. Removing them here keeps front-load-0 to spec-bump + reachable compat shim only.
…max (5048) v1.2.3 raised the PartialSignatureMessages.Messages SSZ list bound from 1512 to 5048, so the stale test's 1513-element message now encodes fine and the expected encode error never fired. Append past 5048 and assert the new bound.
…max (5048) v1.2.3 raised the PartialSignatureMessages.Messages SSZ list bound from 1512 to 5048, so the stale test's 1513-element message now encodes fine and the expected encode error never fired. Append past 5048 and assert the new bound.
…-pin convergence(step-0): ssv-spec v1.2.3 + boole compat shim
…-fork-api convergence(unit 1): networkconfig fork API
Consolidated into main session. Broad draft (network/ + qbft/runner spill); preserved wholesale before rebasing onto integration/boole-convergence.
…on done
Port boole-fork's slot-gated receive-side checks, stage-idiomatically (uint64 subnets,
not boole's Subnet type):
- validateTopicAtSlot: expected topic = Alan or Boole per BooleForkAtSlot(slot); exact
match, byte-mirrors p2pNetwork.BroadcastAtSlot (committee + validator roles).
- validateDomainAtSlot: exact match vs netCfg.DomainTypeAtSlot(slot).
- Called after decode (slot known) from consensus (Height) and partial (Slot) validators.
- Pre-decode: committeeChecks topic block removed; signed_ssv_message domain check relaxed
to a {DomainType, NextDomainType} allowlist (exact per-slot enforced later).
Production compiles/vets/deadcode clean. TEST MIGRATION PENDING: message/validation
fixtures still use base-name topics + static Alan domain; TestNetwork defaults Boole=0
(post-fork, matches boole-fork), so ~90 subtests need fork-aware fixtures mirroring
boole-fork's validation_test.go (full topics + fork-aware domains).
…fork coverage
Mirror boole-fork's test structure:
- Fixtures passed topic BASE names ('114'); fork-aware validation (like production
pMsg.GetTopic()) needs FULL names ('ssv.v2.114'). Wrap CommitteeTopicID(...)[0] in
GetTopicFullName at all committee-topic sites (the old lenient committeeChecks hid this).
- Default TestNetwork to pre-Boole-fork (Boole=MaxUint64) so Alan-oriented fixtures stay
valid; post-fork is opt-in via SSV_TEST_BOOLE_FORK=post or explicit per-test configs.
- validateTopicAtSlot Alan branch reuses commons.CommitteeTopicID (== SubnetTopicID(
AlanCommitteeSubnet), keeps it reachable for deadcode, byte-matches publish side).
- Add explicit post-fork Boole receive tests: committee consensus accepted on the Boole
topic, and rejected on an Alan topic / with an Alan domain at a post-fork slot.
Default suite green (pre-fork); build/vet(main+ssvsigner)/deadcode all clean.
…mmetry Deep review found committeeRole() returned RoleCommittee only, while the publish side (BroadcastAtSlot) and messageLateness already treat RoleAggregatorCommittee as committee- backed, and origin/boole-fork's committeeRole() includes it. Left as-is, once unit 5 starts producing RoleAggregatorCommittee, 100% of those messages would hit the validator lookup and be rejected (ErrUnknownValidator). Align committeeRole() with boole-fork (no runtime effect in unit 2 — role not produced yet — but removes the half-wired asymmetry). Tests: TestCommitteeRole locks the publish/receive symmetry; add a pre-fork reverse-domain subtest (Boole domain on a pre-fork slot -> ErrWrongDomain) to pin the slot-exact domain check symmetrically. build/vet/deadcode/golangci-lint(message/validation) all clean.
Networking review found no blockers/majors; the transition-window subscription state machine, topic derivation, peer scoring, and qbft/runner ripple are correct and match origin/boole-fork. Two defensive items: - BooleCommitteeSubnet now returns UnknownSubnetId on an empty committee instead of panicking on committee[0] (real shares are non-empty; guards a future-caller footgun). - Document persistentSubnets' concurrency invariant: written only during single-threaded startup before the goroutine readers act on it. A -race-verified mutex is deferred to a follow-up (the p2p integration tests that would exercise it can't run in this sandbox).
CI (first run of the real networking gate) surfaced two real bugs the sandbox couldn't: - Double-prefixed topics (ssv.v2.ssv.v2.64): the draft ported the full-topic-passing callers but not boole-fork's controller.go change, so topicsCtrl re-wrapped an already- full topic. Ported boole-fork's controller (Subscribe/Unsubscribe/Broadcast/Peers/Topics/ Close use topic names verbatim; callers pass full Alan/Boole names). This also fixes ParseTopicSubnet in buildPeerTrimScores, which needs the full name. - TestBuildPeerTrimScores panicked (nil NetworkConfig) then mismatched: buildPeerTrimScores needs NetworkConfig for fork-aware subnets; the trim test built p2pNetwork without it and its mock controller returned base topic names. Set cfg.NetworkConfig=TestNetwork and made the mock present full topic names (base stored, wrapped outward) to match production. Also gofmt helpers_test.go (lint). build/vet/deadcode/golangci-lint + message/validation + network/commons + network/topics + trim tests all green. Full-libp2p tests (SubscribeBroadcast, TestTopicManager) still can't run in sandbox — CI is the gate.
Follow-up to the controller de-wrap: two network/topics tests still used base topic names.
- msg_validator_test.go: fed the validator base topics ('114'); production pubsub delivers
full names, which fork-aware validateTopicAtSlot expects. Wrap in GetTopicFullName.
- controller_test.go (TestTopicManager): committeeTopic() returned a base name, so with the
now-verbatim controller, Subscribe/Broadcast used base topics while getCount looked up full
-> count never matched. Ported boole-fork's shape: committeeTopic returns the full topic;
drop the now-redundant GetTopicFullName wraps; Peers(committeeTopic(pk)).
TestMsgValidator + build/gofmt/deadcode/golangci-lint(network/topics) green locally.
TestTopicManager needs real libp2p/mdns (hangs in sandbox) -> ported to match boole-fork
exactly; CI is the gate.
…ENDING)
Progress on exporter2 convergence (stacked on unit-2):
- Ported boole-fork's migration_9 (4 files) + registered in migrations.go.
- KEY DISCOVERY: exporter packages diverged — stage keeps the duty-trace model in
exporter/traces (package traces), boole in exporter. So this is an ADAPTATION, not a
file-copy: adapted migration_9 to use traces.* instead of exporter.*.
- Added Role spectypes.RunnerRole to traces.CommitteeDutyTrace (matches boole; only that
one type gained Role).
NOT DONE (build currently broken): SSZ encoding for traces.CommitteeDutyTrace needs regen —
RunnerRole is int32 which sszgen can't encode; boole's generated code casts to uint64
(ssz.MarshalUint64(dst, uint64(c.Role))) and shifts all subsequent field offsets by 8.
Next: port boole-fork's proven CommitteeDutyTrace SSZ methods from exporter/model_encoding.go
into exporter/traces/model_encoding.go (offset-critical — verify via migration_9 round-trip
test), then the 15 conflict files + new api files (exporter/api/{decided/stream,msg,
query_handlers}, api/types.go).
…sets correct) Data-format core of the exporter2 convergence, verified: - Ported boole-fork's model_encoding.go into exporter/traces (package traces) — the structs are identical modulo package + the new Role field, so boole's proven, offset-correct SSZ encoding drops in (RunnerRole marshaled as uint64; all subsequent field offsets shift +8). - Added Role to traces.CommitteeDutyTrace + its DeepCopy. - migration_9 (+test) adapted to the traces package; added the CommitteeRunnerRoleToPrefix helper (+ role-prefix consts) to exporter/store. - migration_9 round-trip TEST PASSES → confirms the SSZ-offset data-format risk is handled. go build ./... + deadcode + gofmt clean. REMAINING: the broader role-aware exporter/store keying (read/write paths) + the 15 conflict files (api handlers/node/server, broadcaster, operators/recipients) + 6 net-new api files, then validation + code-reviewer-deep + PR.
…tCommitteeDuty/Duties) Port boole-fork's role-aware committee-duty key scheme onto stage's exporter/store/store.go (adapted to the traces package): committee-duty keys now include a runner-role byte via CommitteeRunnerRoleToPrefix, and Save/GetCommitteeDuty(ies) take an explicit spectypes.RunnerRole param (GetCommitteeDuties defaults to both Committee+AggregatorCommittee when no roles given). Update every caller across stage's dutytracer package (which has no boole-fork equivalent) to pass spectypes.RoleCommittee explicitly, since AggregatorCommittee-producing runners are gated to a later unit: - exporter/dutytracer/collector.go: two late-SaveCommitteeDuty sites, both already gated on RoleCommittee message paths - exporter/dutytracer/eviction.go: periodic SaveCommitteeDuties dump - exporter/dutytracer/store.go: DutyTraceStore interface + disk-fallback reads in Collector.GetCommitteeDuties/GetCommitteeDuty - exporter/dutytracer/store_metrics.go: metrics-wrapping passthroughs - test mocks/harnesses: dutytracer/collector_test.go mockDutyTraceStore, dutytracer/store_test.go direct store assertions, operator/node_query_test.go
Stage's exporter/v1/api stamped a STATIC domain on all exported participant data; boole computes DomainTypeAtSlot(msg.Slot) per-message, so a batch spanning the Boole fork gets wrong domains/MsgIDs for post-fork messages. Ported boole's fix: - ParticipantsAPIData / NewParticipantsAPIMsg now take *networkconfig.Network and derive the domain per-message via netCfg.DomainTypeAtSlot(msg.Slot). - Threaded netCfg through all callers: operator/node.go, exporter/v1/api/query_handlers.go, exporter/v1/api/decided/stream.go (+ its NewStreamPublisher/NewDecidedListener ctors and the cli/operator/node.go wiring), and updated query_handlers_test.go. Full matrix green: build/vet(main+ssvsigner)/deadcode/gofmt/golangci-lint + exporter + api tests. NOTE: aggregator-committee query-layer (Collector cache-keying) intentionally deferred to unit 5; api/server.go + the old exporter/api/* path NOT ported (stage is ahead).
…iew nit) Deep review found the stored trace's Role relied on RoleCommittee being the zero value matching the 0x00 key byte, rather than an explicit assignment. Safe today, but a unit-5 footgun (the AggregatorCommittee path must set 0x06). Set Role=RoleCommittee explicitly at both collector build sites to remove the implicit dependency.
Convergence of cli/operator/node.go. Most of unit 4's anticipated scope is already stage-ahead: #2866 (node.go split) and #2891 (ApplyDefaults config-in-code) exist on stage and NOT on boole; exporter fixedSubnets=AllSubnets default is present (node.go:364); the #2914 teardown race is re-solved via ctx-cancel-before-close (documented in close()). The only genuine fork delta: applyDynamicMaxPeers counted Alan subnets only. Now mirrors boole — counts each validator's Boole committee subnet, plus its Alan subnet before/through the transition (!booleFork), using the SSVShare.BooleCommitteeSubnet/AlanCommitteeSubnet helpers added in unit 2. This is the peer-budget heuristic; resolves the nit deferred from unit-2 review. build ./... + vet(main+ssvsigner) + deadcode + gofmt + golangci-lint(cli) + cli tests green.
Foundation for unit 5. Ports boole's fork-gated QBFT proposer selection: - protocol/v2/qbft/round_robin_proposer.go: RoundRobinProposer changes signature to (height, round, committee, netCfg) and adds ETH-epoch variability + committee sorting; old impl renamed RoundRobinProposerPreBooleFork. Co-lands round_robin_proposer_test.go (pre-fork matches stage logic; post-fork epoch offset; committee-order independence). - operator/validator/controller.go buildController: fork-gate the ProposerF — BooleForkAtSlot(state.Height) ? RoundRobinProposer(new) : RoundRobinProposerPreBooleFork. Uses ssvtypes.OperatorIDsFromOperators (already present). Runner construction stays for 5c. - message/validation/consensus_validation.go leader check: fork-gate identically; rename the unit-2 helper roundRobinProposer -> roundRobinProposerPreBooleFork so validation and the actual proposer agree per-fork (else post-fork proposals rejected as 'not leader'). - validation_test.go: pre-fork leader test renamed; the unit-3 post-fork acceptance test now builds a proposal signed by the fork-aware leader (stronger). Foundation only: aggregator-committee helpers (value_check checker, msg/runner_role role bits) deferred to their consumers in 5b/5c to avoid deadcode. Full matrix green: build ./... + vet(main+ssvsigner) + deadcode + gofmt + golangci-lint + qbft/message-validation/ operator-validator tests.
Ports boole-fork's additive runner changes for group 1: fork-aware domain selection (DomainTypeAtSlot) on partial-sig broadcasts (#2915), recoverableReconstructError tagging so a recoverable post-quorum BLS-reconstruction failure isn't recorded as a failed duty (#2918/#2919 class) in both AggregatorRunner and CommitteeRunner, AggregatorCommitteeDuty switch-cases in currentDutySlot/State (marshal) and the RoleAggregatorCommittee branches in runner_validations.go (dead until the runner itself lands in a later unit, but these are reachable switch/if branches, not unused symbols), consolidation of VerifyBeaconPartialSignature into BaseRunner.verifyBeaconPartialSignature/ resolveDuplicateSignature, and misc test-kit deduplication (shared runnerTestKit replaced by inlined per-file setup + standalone sortBySubnet unit test per PR #2899 review). Deliberately NOT ported (out of unit-5b scope): - aggregator_committee.go / aggregator_committee_test.go: the new AggregatorCommitteeRunner itself is deferred to unit 5c. - networkconfig Beacon.ForkAtEpoch -> BeaconForkAtEpoch rename: that lives in the networkconfig package (outside this unit's file list); kept the existing ForkAtEpoch name, which is functionally identical. - NewVRSubmitter(ctx, ...) constructor/Start-merge refactor in validator_registration.go: its only caller (cli/operator/node.go) is outside this unit's scope and still uses the old 4-arg signature + explicit .Start(ctx) call; porting the rename would break that build. validator_registration.go and its test are left untouched. - Redundant per-runner GetShare() overrides on AggregatorRunner/ ProposerRunner/SyncCommitteeAggregatorRunner: BaseRunner.GetShare() (already present) provides identical behavior via promotion. faultyAggregateSubmitBeacon (needed by aggregator_postconsensus_classification_test.go) is defined locally in that file since its original home, aggregator_committee_test.go, is deferred to 5c; flagged with a comment for dedup when that file lands. Build: go build ./protocol/v2/ssv/runner/... && go vet ./protocol/v2/ssv/runner/... Test: go test ./protocol/v2/ssv/runner/... -count=1 (ok) Also verified: go build ./... (full repo), gofmt -l (clean), deadcode.sh (clean)
Ports boole-fork's additive scheduler changes: threads the real Boole fork bit through RunnerRoleForValidatorDuty (resolves the "TODO(convergence unit 5)" markers in scheduler.go and observability/log/fields/fields.go) instead of a literal false, and gates the legacy Alan-era execution paths in AttesterHandler (executeAggregatorDuties) and SyncCommitteeHandler (processExecution) behind !netCfg.BooleForkAtSlot so they stop firing once Boole activates (the post-fork successor is the AggregatorCommitteeHandler, deferred - see below). Widens baseHandler (and every handler embedding it) from a bare *networkconfig.Beacon to *networkconfig.Network (renamed beaconConfig -> netCfg) because BooleForkAtSlot only exists on Network; this is the prerequisite for the fork-bit threading above and ripples through all of operator/duties/*.go plus a one-line companion fix in operator/node.go and fee-recipient-adjacent duties.SchedulerOptions construction (the only two callers of the renamed SchedulerOptions.BeaconConfig/duties.NewScheduler field). fields.Duties gained a runnerRole callback parameter (fork-aware role resolution for log lines); adapted its 3 call sites (attester.go, proposer.go, scheduler.go). Dropped the unrelated fields.Subnet/FieldSubnet addition that rode along in the same upstream file diff - its only consumers are network/p2p/*, outside this unit's scope, and it was flagged by deadcode.sh as unreachable. Deliberately NOT ported (out of unit-5b scope, coupled to the AggregatorCommitteeRunner wiring deferred to a later unit): - CommitteeHandler.isAggregator / NewAggregatorCommitteeHandler / newCommitteeDuty and widening committeeDuty.duty from *spectypes.CommitteeDuty to spectypes.Duty: the scheduler would then hand AggregatorCommitteeDuty values to DutyExecutor.ExecuteCommitteeDuty, but that interface's sole real implementer, operator/validator.Controller.ExecuteCommitteeDuty, is out of scope and still only accepts *spectypes.CommitteeDuty - widening the interface without it would break operator/node.go's duties.DutyExecutor(opts.ValidatorController) conversion. - DutyExecutor.ExecuteCommitteeDuty(duty spectypes.Duty) widening in scheduler.go/executor_noop.go/scheduler_mock.go, for the same reason (those two files are consequently untouched). - fields.BuildCommitteeDutyID gaining a role parameter: it has 5 call sites across 4 packages, 3 of them out of scope (operator/validator/controller.go, protocol/v2/ssv/validator/*); kept the 3-arg signature and scheduler.go's existing call. - ExecuteCommitteeDuties/loggerWithCommitteeDutyContext role recomputation via RunnerRoleForDuty: inert without the widened duty type (always resolves to RoleCommittee today), so skipped rather than adding dead-weight complexity. committeeDuty gained no new accessor methods; committee_test.go/ scheduler_test.go were adapted to use committeeDuty.duty.ValidatorDuties directly (an accessor method would only be called from tests, which deadcode.sh flags as unreachable from the cmd/ssvnode entrypoint). Build: go build ./... && go vet ./... Test: go test ./operator/duties/... ./observability/... ./operator/... -count=1 (ok) Also verified: gofmt -l (clean), deadcode.sh (clean)
Ports boole-fork's additive RoleAggregatorCommittee handling in the message-validation pipeline: - signed_ssv_message.go: renamed validRole -> validRoleAtSlot(role, slot), now fork-gated: RoleAggregatorCommittee only valid post-Boole, the deprecated Alan RoleAggregator/RoleSyncCommitteeContribution only valid pre-Boole. The role check moved out of validateSSVMessage (which runs before the slot is known) into validateConsensusMessageSemantics/ validatePartialSignatureMessageSemantics, once the message is decoded far enough to know its slot. - validation.go: restored the domain-allowlist check validateSSVMessage used to do inline as its own validateDomainAllowlist method (called right after validateSSVMessage in handleSignedSSVMessage), and inlined the one-line committeeChecks wrapper into a direct belongsToCommittee call - both needed since validateSSVMessage lost those two checks. - common_checks.go, partial_validation.go, consensus_validation.go: RoleAggregatorCommittee threaded through dutyLimit, maxRound, validPartialSigMsgType/partialSignatureTypeMatchesRole, and the partial-sig message/duplicate-index limits (widened from the RoleCommittee-only min(2V, V+SyncCommitteeSize) formula to a role-parameterized min(scSubnets*V, V+scSubnets*SyncCommitteeSize) covering AggregatorCommittee's wider fan-out); replaced `role != spectypes.RoleCommittee` checks with the already-existing mv.committeeRole(role) helper so both committee roles are covered. - consensus_validation.go: switched the full-data-hash check from the local protocol/v2/qbft.HashDataRoot ([32]byte, no error) to the spec's qbft.HashDataRoot ([32]byte, error) - byte-for-byte identical (sha256.Sum256, error always nil) at the pinned ssv-spec v1.2.3, so this is a behavior-neutral dedup; added ErrFullDataHash for the (dead) error path. - seen_msg_types.go, errors.go: added spectypes.AggregatorCommitteePartialSig to the pre-consensus message-type set; renamed ErrTripleValidatorIndexInPartialSignatures -> ErrTooManyEqualValidatorIndicesInPartialSignatures (same check, V<=2, for RoleCommittee; the generalized name reflects the new role-parameterized limit above); added ErrFullDataHash. - logger_fields.go: addDutyIDField now also builds committee-style duty IDs for RoleAggregatorCommittee; uses the existing ssvtypes.RunnerRoleToString (added the function to protocol/v2/types/runner_role.go - it was already needed here to stringify the deprecated Alan RoleAggregator/RoleSyncCommitteeContribution values, which spectypes.RunnerRole.String() doesn't know about). - protocol/v2/types/runner_role.go: added RunnerRoleToString only (the CommitteeSignerBucket/CommitteeRunnerRoleForBeaconRole additions that rode along in the same upstream file are exporter/observability-domain and out of this unit's scope). Deliberately NOT ported (out of unit-5b scope): - committee_info.go's committeeInfoFromCommittee/committeeInfoFromShare rename and new CommitteeInfo.subnet/subnetAlan (commons.Subnet-typed) fields, and validation.go's corresponding switch to committeeInfo.subnet.BooleTopic()/.subnetAlan.AlanTopic(): both ride on network/commons's new `type Subnet uint64` (with BooleTopic/AlanTopic methods), a ~150-line rework of network/commons/subnets.go that also touches network/p2p/*, registry/storage, and protocol/v2/types/ssvshare.go - all out of scope. validateTopicAtSlot already computes the fork-aware expected topic correctly via the existing commons.BooleCommitteeSubnet()/commons.GetTopicFullName(commons.CommitteeTopicID()) functions (added by an earlier unit), so this is a pure implementation-detail refactor with no behavior gap - left untouched. - const.go's wholesale switch from hand-computed max-message-size constants to github.com/ssvlabs/ssv-spec/.../maxmsgsize-derived ones: unrelated to fork-awareness, touches the wire message-size-limit logic, and carries real risk if ported without independent scrutiny - out of scope for this unit. - fields.BuildCommitteeDutyID gaining a role parameter (kept the 3-arg call in logger_fields.go): see unit-5b group-2 commit for why - it has 5 call sites across 4 packages, 3 of them out of scope. - common_checks_test.go: boole-fork deletes it, presumably folded into its validation_test.go rewrite; kept it since committeeRole() itself is unchanged and it's cheap, focused, still-valid coverage. - The ~900-line validation_test.go rewrite: deeply coupled to the two skipped refactors above (commons.Subnet, const.go maxmsgsize) via shared test fixtures/helpers. Instead, fixed the 2 fallout spots in the existing test file: renamed one assertion to ErrTooManyEqualValidatorIndicesInPartialSignatures, and updated the "invalid role" test to key off a real validator pubkey instead of a synthetic committee ID (the role check moved past getCommitteeAndValidatorIndices/belongsToCommittee, so the old fixture now fails earlier with "unknown validator" instead of reaching the role check). No new dedicated tests were added for the new RoleAggregatorCommittee validation branches themselves. Build: go build ./message/validation/... && go vet ./message/validation/... Test: go test ./message/... ./protocol/v2/types/... -count=1 (ok) Also verified: go build ./... (full repo), gofmt -l (clean), deadcode.sh (clean)
…eview nit) The fork bit is now threaded for real (group 2); the TODO is obsolete.
Port protocol/v2/ssv/runner/aggregator_committee.go from boole-fork in its final #2919-correct form (terminal/recoverable classify closure, errCh drain, code-81 recoverable classification), plus its test suite. Dedup faultyAggregateSubmitBeacon with the classification test file per unit5b's forward-looking NOTE. Dependencies pulled in to make the runner compile and function: - ssv.NewAggregatorCommitteeChecker (value_check.go), from the original aggregator-committee PR (c8c54ee), including its accompanying reorder of the WrongBeaconRoleType/DutyEpochTooFarFuture checks. - beacon.BeaconNode: add IsAggregator/GetAggregateAttestation (client.go + regenerated mocks), and their GoClient implementation (aggregator.go), reusing the existing attestedDataRoot/computeAttestationDataRoot helpers already present in this base. Kept the existing ForkAtEpoch name (boole renamed it to BeaconForkAtEpoch as an unrelated, out-of-scope rename) and left checkPtrResponse/nil-check style alone (separate infra #2905 unit). - protocol/v2/testing/temp_testing_beacon_network.go: BeaconNodeWrapped gains IsAggregator/GetAggregateAttestation delegating to the spec TestingBeaconNode (previously fell through to a nil embedded beacon.BeaconNode interface and panicked). Left the unrelated Alan-spec-test SyncCommitteeSubnetID file-split out of scope. - protocol/v2/ssv/testing/{runner,validator}.go: wire AggregatorCommitteeRunner into the test harness construction paths (both fixed-share and share-map variants) and BaseValidator's role map. Build: go build ./protocol/v2/ssv/... ./protocol/v2/blockchain/beacon/... ./beacon/... Test: go test ./protocol/v2/ssv/... ./beacon/goclient/... -count=1 (all pass)
Widen Committee (protocol/v2/ssv/validator/committee.go) to run both
CommitteeDuty and AggregatorCommitteeDuty side by side: parallel
Queues/AggregatorQueues and Runners/AggregatorRunners maps, role-routed
queueContainer lookup, ProcessMessage pre-consensus routing for the
aggregator-committee partial-sig subtype, and runnerForDuty/runnerForRole/
createRunner/extractValidatorDuties helpers. committee_queue.go and timer.go
route enqueue/timeout events to the role-specific queue via the new
queueContainer wrapper. committee_observer.go adds aggregator/sync-committee-
contribution root tracking (saveAggregatorRoots/saveSyncCommContribRoots) and
its own resolveDuplicateSignature/verifyBeaconPartialSignature (copied from
BaseRunner, since CommitteeObserver lives outside the runner package).
Resolves the three unit5b "TODO(convergence unit 5): thread real fork bit"
markers in duty_executor.go and validator.go's StartDuty, and the aggregator
runner-construction spec-test skip.
Threads the committeeDuty.duty widening (*spectypes.CommitteeDuty ->
spectypes.Duty) end to end: operator/duties/committee.go gains
isAggregator/NewAggregatorCommitteeHandler/newCommitteeDuty plus
validatorDuties()/appendValidatorDuty() duty-type-switch helpers;
operator/duties/scheduler.go's DutyExecutor.ExecuteCommitteeDuty and
loggerWithCommitteeDutyContext follow; the scheduler now also registers
NewAggregatorCommitteeHandler alongside NewCommitteeHandler. Regenerated
scheduler_mock.go. operator/validator/controller.go's ExecuteCommitteeDuty
widens to match; SetupCommitteeRunners' CommitteeRunnerFunc signature widens
to (spectypes.Duty, ...) (runner.Runner, error) but its closure still only
constructs CommitteeRunner for now — the AggregatorCommitteeDuty case is
deferred to group 3, which is what makes NewAggregatorCommitteeRunner
reachable (confirmed via deadcode.sh: only NewAggregatorCommitteeRunner and
NewAggregatorCommitteeChecker remain flagged, as expected at this stage).
fields.BuildCommitteeDutyID gains a role parameter (formats via
FormatRunnerRole instead of a hardcoded "COMMITTEE" prefix) so committee vs
aggregator-committee duty IDs are distinguishable; all call sites updated
(message/validation, operator/duties, operator/validator, protocol/v2/ssv/
validator). protocol/v2/ssv/queue/observability.go gains the
AggregatorCommitteeQueueMetricType constant needed by the new queue.
Deliberately NOT ported (out of scope, from unrelated bundled history):
- committee.go's local `CommitteeWrongRoleErrorCode = 85` override and
committee_queue.go's "drop_reason" -> "ssv.queue.drop_reason" attribute
rename (kept base's existing spectypes.CommitteeWrongRoleErrorCode and
"drop_reason" instead).
- qbft controller's identifier -> identifierFn (fork-aware QBFT msg ID)
refactor and the associated protocol/v2/ssv/spectest/msg_processing_type.go
rewrite (IdentifierFn, SetAggregators, adjustActualErrorForRunner, etc.) —
applied only the minimal CommitteeRunnerFunc signature fix needed to keep
spectest compiling, since the full rewrite depends on the out-of-scope
qbft controller change.
- ssv_mapping_test.go's "TODO(convergence unit 5): also read
AggregatorCommitteeRunners" is left as-is (non-blocking; no fixture in
this suite currently exercises RoleAggregatorCommittee runners).
Fixed one JSON-encoding regression this widening caused in
committee_msg_processing_type.go: the reference committee built from a
state-comparison fixture didn't initialize the new AggregatorRunners map, so
its root diverged from the always-non-nil map the Committee constructor
produces (nil vs {} in JSON) — TestSSVMapping's committee-runner-state-root
comparisons were failing; initializing it to an empty map fixed all of them.
Build: go build ./... ; go vet ./...
Test: go test ./protocol/v2/ssv/... ./operator/duties/... ./operator/validator/...
./message/validation/... ./observability/... -count=1 (all pass)
deadcode.sh: only NewAggregatorCommitteeRunner + NewAggregatorCommitteeChecker
flagged (expected until group 3 wires controller construction)
iurii-ssv
reviewed
Jul 29, 2026
iurii-ssv
reviewed
Jul 29, 2026
iurii-ssv
reviewed
Jul 29, 2026
iurii-ssv
reviewed
Jul 29, 2026
badger's transaction limit is ~15% of MemTableSize (~9.6MB under the DefaultOptions this repo opens with), and a single CommitteeDutyTrace can reach ~4MB via ProposalData — so a 5000-record count cap alone can blow ErrTxnTooBig. Flush at 2MB of accumulated payload as well, and stop implying the batching bounds keys on badger (GetAll materializes them up front).
migration_9's source model is frozen, but its write side marshals the live traces.CommitteeDutyTrace and derives the key byte from the live role→prefix map. Pin both with a golden-bytes fixture so a later shape change fails loudly and directs the finder to freeze the target into a V2 model instead of silently emitting the newest format.
…-base#2 The rationale and blast radius for skipping the validator-index check on committee roles are settled in ssvlabs/knowledge-base#2; leave a pointer at the relaxation site so it survives the merge.
Committee.Indices carries one entry per share, positionally aligned with Shares — a metadata-less share contributes a literal 0 until beacon sync fills its real index. Both the committee duty limit and validation's len()==0 branch rely on that cardinality, so filtering the zeros would tighten against legitimate traffic mid-sync. Say so at the field, and mark the ErrNoValidators branch as the defensive assertion it is (currently unreachable: empty committees are deleted outright).
…ator index Committee-role message validation deliberately does not assert that a partial-signature validator index belongs to a validator this operator holds a share for (knowledge-base#2), so a duplicate partial signature carrying an unheld index reached resolveDuplicateSignature and dereferenced b.Share[idx].Committee on a nil *Share — crashing the whole node from a single authenticated committee message. Look the share up once, drop the unverifiable entry when it is absent. Adds RoleAggregator- Committee as a second role exposed to this path; the RoleCommittee exposure predates the fork work. Regression test seeds a duplicate for an unheld index and asserts no panic.
… uniqueness
holesky's naive Boole domain (Alan+1 = {0,0,5,3}) was byte-identical to
hoodi's live Alan DomainType, and holesky-stage's ({0,0,0x31,0x14}) to
hoodi-stage's — the testnets share an ad-hoc 0x05/0x31 lane with adjacent
Alan domains, so Alan+1 walks onto the neighbour. DomainType feeds
NewMsgID and the handshake network-ID filter, so post-Boole holesky would
be protocol-indistinguishable from pre-Boole hoodi. Move both holesky
Boole domains clear of the neighbour's cluster (safe: Boole is unscheduled
on holesky, so NextDomainType is not live). Add TestBuiltinNetworkDomains-
AreUnique to enforce global uniqueness of every {Alan, Boole} domain going
forward — the existing config tests only asserted DomainType equality.
The specific replacement bytes (0x22 / 0x23) are a proposal; the domain
registry owner should confirm them.
…artial-sig cap Post-consensus emits one partial signature per Aggregators/Contributors entry (GetAggregateAndProofs / GetSyncCommitteeContributions are one-per-entry, matched by committee/subnet index rather than position). Message validation caps partial signatures at 5 per validator index and rejects the whole message beyond it. Two gaps: - Honest overflow: a validator holding several sync-committee indices in one subnet was appended to Contributors once per index, so a validator with >5 indices in a subnet made its own post-consensus message self-reject. Dedup Contributors by (validator, subnet) and Aggregators by (validator, committee index) at build time — bounding honest traffic to <=4 sync subnets + 1 aggregator = 5 per index, exactly the cap. - Byzantine amplification: spec Validate() permits duplicate entries, so a malicious proposer could make every honest operator sign data that emits duplicate partial sigs and gets rejected network-wide, draining their gossip score. Reject duplicate (validator, committee index) within each set in the value check, so honest operators refuse the proposal at QBFT time. Deduped independently per set: an aggregator committee index and a contributor subnet id share a namespace but are distinct assignments.
…ss allowance The Alan topic set was retired one slot after the fork (booleSubsequentWindowSlots = 1), but message validation keeps accepting messages for a pre-fork slot for maxStoredSlots (SlotsPerEpoch + LateSlotAllowance) slots. A committee duty for the last pre-fork slot whose QBFT is still advancing rounds publishes its decided and post-consensus messages on the Alan topic keyed by that pre-fork slot — after F+1 the whole fleet has unsubscribed and de-whitelisted Alan, so those messages are dropped by the subscription filter before validation sees them and the duty is silently lost. Derive the subsequent window from SlotsPerEpoch + a lateness margin that mirrors message/validation.LateSlotAllowance, so Alan stays subscribed as long as those late pre-fork messages remain valid.
…ittee root computation expectedPreConsensusRoots and expectedPostConsensusRootsAndBeaconObjects swallowed DomainData/root-compute errors with a Debug log and continued. Those errors come from the beacon node (DomainData) or from malformed already-decided data — never a per-validator 'no duty' condition, and DomainData fails identically for every validator in the slot. Swallowing them left the expected-root maps empty, so pre-consensus concluded the duty 'not required' with a success-flavoured log (dropping every aggregation for the slot), and post-consensus fell through to ErrNoValidDutiesToExecute, which terminates the queue consumer on what is a transient error. Both callers already markDutyFailed and return on a non-nil error, so propagate the wrapped error instead of continuing. Drop the now-unused logger parameter from both helpers.
…ittee roles GetCommitteeDecideds loops over the committee runner roles and continues on a miss, but only recognized the package-local ErrNotFound. A disk miss wraps exporter/store.ErrNotFound instead, so the loop aborted on the first role's miss rather than trying the next — a validator whose only trace at that slot is an aggregator-committee duty would report nothing once a caller passes more than one role. Match both sentinels.
The role-aware committee scan uses the slot+role prefix, so obj.Key is the committee ID (32 bytes). A pre-migration legacy key whose committee ID starts with the RoleCommittee byte 0x00 aliases into the scan and leaves a shorter remainder, so UnmarshalSSZ fails and poisons the whole slot's result with an error. Such keys can only appear after a post-migration downgrade writes fresh legacy keys (migration_9 is forward-only). Skip any entry whose trimmed key is not committee-ID length so a rollback degrades to invisible stragglers rather than permanent per-slot query errors.
The committee round-timer routing msg ID used CurrentDomainType() (current wall-clock slot) while every other MsgID in the fork cutover uses DomainTypeAtSlot(dutySlot). Only GetRoleType() is read from this ID downstream, so this is a consistency fix rather than a behavior change, but it stops a pre-fork duty still running after the fork from keying its timer events under the wrong domain.
…gators A GetAggregateAttestation failure dropped the aggregator from the proposal and set anyErr, but the loss was effectively silent: message replay never recovers it (pre-consensus quorum is reported only once, so a retried message never re-reaches this fetch), and post-consensus derives its expectation from the decided data, so the duty was later reported as a full success with the validator's aggregation missing. Retry the fetch in place with a short bounded, context-aware backoff — the realistic failure is a transient beacon hiccup at the 2/3-into-slot fetch, and fetches are already deduped per committee index so this runs at most once per index. When retries are exhausted, log the dropped aggregator at WARN so the loss is visible rather than folded into a success conclusion. Does not restructure the post-consensus success derivation; a genuine sustained beacon outage still drops the aggregator (nothing can fetch it), but it is now surfaced.
…tted-duty roots Two aggregator-committee robustness fixes, sharing a generalized retryBeaconFetch helper (extracted from the aggregate-attestation retry): - Sync-committee contribution fetch (processSyncCommitteeSelectionProof) had the same shape as the aggregate path: a transient GetSyncCommittee- Contribution failure left the validator's selection unchecked, so the duty hung to slot end (message replay never re-reaches the fetch — quorum is reported once). Retry it in place with the same bounded backoff; the caller already logs the give-up. - HasSubmittedAllDuties re-derived the expected-root maps by calling expectedPostConsensusRootsAndBeaconObjects again, re-issuing O(validators) DomainData calls per post-consensus message. Since that derivation can now fail on a transient beacon error, a hiccup at the final check mislabeled a fully-submitted duty as 'partial success'. Pass in the maps ProcessPost- Consensus already computed instead (mirrors CommitteeRunner).
…xecutor ID validRole was replaced by the slot-dependent validRoleAtSlot, called only after getCommitteeAndValidatorIndices. So a role outside the union of all forks' valid roles, carried on a message whose executor ID does not resolve to a known validator, took the unknown-validator path and returned ErrUnknownValidator (Ignore) instead of ErrInvalidRole (Reject) — a lost punishment signal for malformed traffic. Add a fork-independent validRoleUnion check in validateSSVMessage (which runs before the executor ID is resolved); validRoleAtSlot still narrows valid roles per fork afterward. Regression test covers the unknown-executor -ID case that previously degraded to Ignore.
validateBeaconDuty asserts per-validator duty assignment only for RoleProposer and the pre-Boole RoleSyncCommitteeContribution; RoleCommittee and RoleAggregatorCommittee fall through unchecked. This is deliberate, not an oversight: committee roles are exempt from per-validator duty assertions for the same reason the ValidatorIndex check is relaxed (knowledge-base#2) — operators are not assumed synced on each other's validator sets, so asserting a per-validator duty would reject legitimate messages from nodes mid-sync and penalize the peers they need to catch up from. The pre-Boole SCC role had the assertion because it was a per-validator role; AggregatorCommittee inherits that traffic post-fork as a committee role, so the drop is by design. Record it so it is not re-filed as a regression. Residual spam is insider-only and bounded by dutyLimit.
…orting startup A single legacy 'cd' value that fails UnmarshalSSZ aborted the whole migration, and migration failure aborts node startup — so one bit-rotted record among the tens of millions a mainnet exporter can hold would brick the node with no clean operator remedy. These are observability duty traces, not validator-critical state, and a record that won't decode is already unreadable, so skipping it loses nothing recoverable. Log the unreadable record loudly (ERROR, with its key) and continue, counting skips in the completion log. The marshal of the converted trace is left fatal: a failure there would indicate a conversion bug rather than input corruption, and must not be silently skipped. Test seeds a corrupt record beside a good one and asserts the migration completes.
qbft.Config.Domain was set to the static (Alan) DomainType at runner construction and read only by GetSignatureDomainType(), which is declared on the local IConfig interface but invoked nowhere — not in our controller/ instance, not in ssv-spec's qbft. Production signing-domain selection flows through the fork-aware IdentifierFn. So the field was inert but latent: a fork value frozen to Alan that any future caller of GetSignatureDomainType would sign with post-fork, wrong, uncaught. Deriving it fork-aware is meaningless (it is set once at construction, spanning the fork, and nothing reads it), so delete the field, the interface method, and its two production plus two test construction sites.
iurii-ssv
previously approved these changes
Jul 29, 2026
This was referenced Jul 30, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Boole → stage convergence — final consolidation
Merges the entire
boole-forkprotocol-fork layer intostage, assembled unit-by-unit onintegration/boole-convergence. This is the consolidation merge — every unit below was already implemented, deep-reviewed, and merged intointegrationas its own PR; this PR lands them instageas one coherent set.What's included
SSVForks,BooleForkAtSlot,DomainTypeAtSlot).applyDynamicMaxPeers.qbft.Proposerentry point).AggregatorCommitteeRunner+ consensus wiring.Status / before merge
integration(under review) — they will appear in this diff automatically as they merge. This PR should merge only once they're in.operator/node.go(stage commita7a02cc73"Exporter refactoring threshold cmd to generate keys #3 (Exporter refactoring #3: Move websocket query bridge into exporter v1 API #2928)" vs the convergence Scheduler-widening/fork-wiring). Resolved at the final step by syncingstageintointegration(once convergence: aggregator-committee message-validation test coverage #2939/convergence: fork-aware exporter validator-path duty routing (NewExporter networkConfig) #2940 land), then full validation — not a plain button-merge.Follow-up (tracked, not in this PR)
sync committee aggregator selection proof) against a reconciled fork schedule.