Repository navigation
feat(fork): reorder fork ordinality so CStar can activate without Boole - #1087
shane-moore wants to merge 2 commits into
Conversation
SSV core deprioritized Boole indefinitely; CStar (the SSV-side Gloas rollout) now ships first. Reorder the Fork enum to Alan, CStar, Boole so the derived Ord becomes Alan < CStar < Boole. This single change flips every ordinal gate at once: `>= Fork::Boole` behaviors (MinHash subnets, AggregatorCommittee wire roles, QBFT epoch shift) stay dormant when CStar is active, and a future Boole is forced to schedule after CStar. Re-key the two subnet topology match arms that grouped Boole and CStar so CStar inherits Alan committee-based topology. Boole logic is retained for possible later activation. Update built-in fork-schedule example comments to the new chronology (cstar domain 0x00000002, boole 0x00000003). Closes sigp#1085
|
@claude, please take a look |
|
Claude finished @shane-moore's task in 4m 58s —— View job PR Review —
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## epbs #1087 +/- ##
=======================================
Coverage ? 63.80%
=======================================
Files ? 162
Lines ? 28113
Branches ? 0
=======================================
Hits ? 17938
Misses ? 10175
Partials ? 0
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
The fork machinery relies on Fork::all() being ordered identically to the derived Ord: ForkSchedule::new's take_while and from_fork_configs's BTreeMap iteration both assume it. The existing order tests pin only the current three forks, so a future fork added in the wrong enum position relative to all() could slip through. Assert the invariant directly. Addresses review feedback on sigp#1085.
|
@claude, thanks for the review. Addressed Finding 1 in Finding 1 (validation rests on enum order): Added Findings 2, 3, 4: no code change, agreed.
|
|
Claude finished @shane-moore's task in 59s —— View job Follow-up review —
|
|
closing in favor of #1090 |
Problem, Evidence, and Context (Required)
SSV core has deprioritized the Boole fork indefinitely. CStar (the SSV-side Gloas rollout) now ships first, on a hard deadline tied to Ethereum's Gloas fork. Boole may still happen later, so its logic must be retained.
Anchor's fork machinery assumes the chronology
Alan < Boole < CStar:ForkderivesOrdfrom variant order, and every behavior gate is an ordinal comparison (active_fork(epoch) >= Fork::X). With that ordering, activating CStar makes every>= Fork::Boolegate evaluate true, silently switching on Boole behaviors (MinHash subnet topology,AggregatorCommitteewire roles, the QBFT epoch shift) that the network never agreed to. The scheduling layer already supports skipping Boole; the problem is purely the behavior layer.Closes #1085.
Change Overview (Required)
Reorder the
Forkenum toAlan, CStar, Booleso the derivedOrdbecomesAlan < CStar < Boole. Because every gate is an ordinal comparison, this single change flips them all at once:>= Fork::Boolebehaviors stay dormant while Boole is unscheduled, and the new ordering forces any future Boole activation to land after CStar. CStar therefore lands directly on Alan-era behavior.Only one site needed a behavior re-key: the two subnet-topology match arms that explicitly grouped
Boole | CStar. They now readFork::Alan | Fork::CStar => committee-based,Fork::Boole => MinHash. No other ordinal gate needed a code change.Reading order:
common/fork/src/fork.rs— the enum reorder (the conceptual core).subnet_service/src/{subnet,service}.rs— the one behavior re-key.common/fork/src/schedule.rs— schedule-validation tests (cstar-without-boole accepted; boole-before-cstar rejected). Notefrom_fork_configsneeds no code change: it iterates aBTreeMap<Fork, _>in enum order, so the reorder makes the validation correct automatically.message_validatorandssv_network_config, plus the built-in fork-schedule example comments.What did not change: Boole logic is fully retained (explicitly not removed). Name-based parsing,
Display, and gossip topic strings are order-free and unchanged. Pre-CStar (Alan-only) behavior is byte-for-byte identical. No wire/serialization format changes (Forkserde is name-based; it is never serialized by ordinal).Risks, Trade-offs, and Mitigations (Required)
Fork, which is why the diff spans 4 crates. It is one atomic change and cannot be split. Each affected gate was audited; only the subnet match arms required a code change.ForkSchedule::new(fork)semantics shift: it seeds every fork<= fork, sonew(Fork::Boole)now co-activates CStar. The validator_store committee-attestation harness uses a Boole schedule and consequently now exercises the realGloasBeaconVotepath. This is the genuinely-correct behavior for a Boole network (which is necessarily post-CStar), so it is improved coverage, not a regression. Documented in thenewdoc comment.0x00000002(the increment after Alan), Boole takes0x00000003if later activated. Anchor is the first mover; go-ssv has not started Gloas-fork planning. These anchor-side choices should be memorialized for eventual go-ssv parity. No built-in network schedules either fork today (they carry both only as commented examples), so nothing live is affected.Validation (Required)
make cargo-fmt-checkandmake lint(clippy-D warnings): clean.cargo teston every affected crate passes:fork,subnet_service(55),ssv_network_config,message_validator(67),qbft_manager(31),anchor_validator_store(41),network(52).alan@0 + cstar@N(no boole) validates andactive_fork(N) == CStar; a boole-before-cstar schedule is rejected; CStar inherits committee-based subnet topology; legacyAggregator/SyncCommitteewire roles are accepted andAggregatorCommitteeis rejected under CStar; no epoch shift in round-robin proposer selection under CStar.spec_tests, which requires Go-generated fixtures absent in a fresh checkout (environmental, unrelated to this diff).Rollback (Required for behavior or runtime changes; optional otherwise)
Revert the commit. No config, data, or operational impact: no production network schedules Boole or CStar today, and the change is a pure in-memory ordering with no wire-format effect.
Additional Info / Next Steps (Optional)
The issue also proposed centralizing the scattered ordinal gates into
ForkSchedulecapability methods (min_hash_subnets_active(epoch), etc.). That hardening is intentionally deferred to a follow-up PR to keep this one behavior-only and independently reviewable. The reorder is fully correct without it.