Enable the SSV_TEST_BOOLE_FORK=post unit-test matrix (#2964) - #2974
Enable the SSV_TEST_BOOLE_FORK=post unit-test matrix (#2964)#2974momosh-ssv wants to merge 3 commits into
Conversation
TestP2pNetwork_SubscribeBroadcast fixtures built MsgIDs from the static netCfg.DomainType, which post-flip receivers reject: under SSV_TEST_BOOLE_FORK=post the LocalNet nodes run post-fork and operate on the Boole domain while the fixture still carried the Alan one. Derive the domain per-slot via DomainTypeAtSlot, exactly like production (p2p_setup.go), so the fixtures are valid on both sides of the fork - and the matrix leg genuinely exercises the post-fork p2p path instead of pinning it back to pre-fork. Item 1 of #2964.
Adds the unit-test half of the SSV_TEST_BOOLE_FORK matrix (the spec-test half landed with the convergence): a sibling ssv-boole-post job running make unit-test with SSV_TEST_BOOLE_FORK=post. No codecov upload - the primary job owns the core-flagged report. A full local sweep of all 112 packages under post mode found no other failures (network/p2p and network/topics hang locally on mdns in BOTH modes and are verified by this job on CI). Item 3 of #2964.
| runs-on: ubuntu-latest | ||
|
|
||
| steps: | ||
| - uses: actions/checkout@v6.0.2 |
There was a problem hiding this comment.
The new job executes checkout, setup-go, and cache through mutable version tags, allowing upstream tag movement to change code executed with the job's token and repository access. Pin all three references to immutable commit SHAs to preserve workflow integrity.
How this was verified: The new action references at lines 61, 67, and 75 use version tags rather than full commit SHAs.
Greptile SummaryThe PR adds post-Boole unit-test coverage and makes P2P test-message domains slot-aware.
Confidence Score: 4/5The PR appears safe to merge, with the non-blocking exception that the new workflow dependencies should be pinned to immutable commits. The post-fork environment reaches the intended test configuration, and the fixture domain changes match production slot handling; the remaining concern is supply-chain hardening for the newly added mutable action references. Files Needing Attention: .github/workflows/unit-test.yml
|
| Filename | Overview |
|---|---|
| .github/workflows/unit-test.yml | Adds a correctly propagated post-Boole test leg, but its newly added action dependencies are not integrity-pinned. |
| network/p2p/p2p_test.go | Aligns fixture MsgID domains with production's slot-based fork selection, with callers and broadcast routing using the same QBFT-height-to-slot mapping. |
Reviews (1): Last reviewed commit: "ci: run the unit suite under the post-bo..." | Re-trigger Greptile
The ssv-boole-post CI leg's first run caught what the local sweep could not (mdns hangs the package locally in both modes): - The trim-score tests built their network on the global TestNetwork; post-fork the node's own subnets land on the Boole side while the fixtures present Alan-shaped topics, collapsing every score to zero. Pin the trim network to an explicit pre-fork config (the Boole side of the scoring math is covered fork-independently by the TestSubnetPeers_Score tests) and add a Boole-topic scoring case so the post leg isn't a pure re-run. - TestP2pNetwork_SubscribeBroadcast's hardcoded persistent-subnet set covered the test committee's Alan subnet (114) but not its Boole subnet (11), so post-fork nodes never joined the topic the broadcast lands on. Derive the set from the configured shares under both mappings. Subscribe topic, broadcast topic and the validation gate's expected topic all reduce to the same committee-subnet functions, so the chain is closed on both sides of the fork. Completes item 1 of #2964.
Items 1–3 of #2964.
TestP2pNetwork_SubscribeBroadcast's fixtures built MsgIDs from the staticnetCfg.DomainType, which post-flip receivers reject (the LocalNet nodes run on the globalTestNetwork, so under the flip they're post-fork on the Boole domain while the fixture carried Alan). The three fixture sites now derive the domain per-slot viaDomainTypeAtSlot, exactly like production (p2p_setup.go). Chosen over pinning the test pre-fork: unlike the validation/topics pins, p2p has no separate post-fork subtest, so pinning would leave the matrix with zero post-fork p2p coverage.SSV_TEST_BOOLE_FORK=post: everything passes exceptnetwork/p2p/network/topics, which hang identically in default mode locally (the known mdns env-hang, not a fork gap) and are verified by this new CI leg on Linux. Notably the spec-test packages (LFS fixtures present) also pass post-mode locally.ssv-boole-postjob inunit-test.yml, mirroring howspec-test.ymlgainedspec-test-boole-post: same job withSSV_TEST_BOOLE_FORK: post, its own cache key (falling back to the primary's), and no codecov upload (the primary job owns the core-flagged report;make unit-testalready excludes spectest, so nothing double-runs).Item 4 (the
RunSyncCommitteeAggProofalan-spec port) is deliberately not included — it's a ~half-day non-mechanical port (the alan build predates the committee/aggregator-committee duty split); leaving it tracked on the issue.Why this matters (from the issue): until this leg exists, the global-config flip path stays permanently unexercised in CI — exactly the gap that let the metadata-syncer fixture regress unnoticed.