Skip to content

Enable the SSV_TEST_BOOLE_FORK=post unit-test matrix (#2964) - #2974

Open
momosh-ssv wants to merge 3 commits into
integration/boole-convergencefrom
fix/2964-boole-post-matrix
Open

Enable the SSV_TEST_BOOLE_FORK=post unit-test matrix (#2964)#2974
momosh-ssv wants to merge 3 commits into
integration/boole-convergencefrom
fix/2964-boole-post-matrix

Conversation

@momosh-ssv

Copy link
Copy Markdown
Contributor

Items 1–3 of #2964.

  • Fork-agnostic p2p fixturesTestP2pNetwork_SubscribeBroadcast's fixtures built MsgIDs from the static netCfg.DomainType, which post-flip receivers reject (the LocalNet nodes run on the global TestNetwork, 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 via DomainTypeAtSlot, 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.
  • Full-suite sweep — all 112 packages were run locally under SSV_TEST_BOOLE_FORK=post: everything passes except network/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.
  • CI matrix leg — new ssv-boole-post job in unit-test.yml, mirroring how spec-test.yml gained spec-test-boole-post: same job with SSV_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-test already excludes spectest, so nothing double-runs).

Item 4 (the RunSyncCommitteeAggProof alan-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.

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.
@momosh-ssv
momosh-ssv requested review from a team as code owners July 30, 2026 12:41
runs-on: ubuntu-latest

steps:
- uses: actions/checkout@v6.0.2

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 security Mutable CI action references

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-apps

greptile-apps Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR adds post-Boole unit-test coverage and makes P2P test-message domains slot-aware.

  • Adds an ssv-boole-post CI job that runs the regular unit-test target with SSV_TEST_BOOLE_FORK=post, a dedicated cache key, and no duplicate Codecov upload.
  • Updates validator, committee, and dummy P2P fixtures to derive message domains through DomainTypeAtSlot.

Confidence Score: 4/5

The 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

Security Review

The new CI job references checkout, setup-go, and cache actions through mutable version tags; pinning full commit SHAs would prevent upstream tag movement from changing code executed by the workflow.

Important Files Changed

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant