proposer tests: deterministic BN response ordering via Release channels - #2865
Draft
iurii-ssv wants to merge 37 commits into
Draft
proposer tests: deterministic BN response ordering via Release channels#2865iurii-ssv wants to merge 37 commits into
iurii-ssv wants to merge 37 commits into
Conversation
Reframe MEV configuration guidance around PBS-side timing games (mev-boost >= v1.11 with -config, or commit-boost) as the recommended approach. Soft-deprecate SSV ProposerDelay to a legacy appendix; no removal date, no behavior change. Code-side changes are doc-comment alignment only: - config/config.example.yaml: MEV configuration block reframed. - cli/operator/node.go: ProposerDelay env-description softened; AllowDangerousProposerDelay unit consistency (s -> ms). - protocol/v2/ssv/runner/proposer.go: ProposerRunner.proposerDelay and ProposerRunnerOptions.ProposerDelay doc comments reframed; internal field comment DRYed to reference the exported field.
Address review feedback on #2855: - cli/operator/node.go: ms-unit consistency in validateProposerDelayConfig error message and warn-log fields. The zap.Duration fields are renamed to proposer_delay_ms / max_safe_proposer_delay_ms (Int64) so the unit is explicit in structured log output. - docs/MEV_CONSIDERATIONS.md: - Configuration knobs: state enable_timing_games defaults to false. - Example A: parenthetical noting the 50ms overhead assumes BN and PBS co-located with SSV. - Example B: add second relay to match Example A's structure. - Appendix A: clarify that the 200ms MEVBoostRelayTimeout figure assumes legacy single-shot PBS behavior; not relevant when running timing-games-capable PBS.
Test_validateProposerDelayConfig was asserting on the old zap field names (proposer_delay, max_safe_proposer_delay) and time.Duration values. The previous follow-up commit renamed these to proposer_delay_ms / max_safe_proposer_delay_ms with int64 type. Update the assertions to match. Fixes the unit-test job on #2855.
The original doc claimed QBFT round 1 times out 2000ms after slot start and used this to derive a "tighter constraint" formula. In reality the proposer round timer is round-relative (2000ms from when the round starts), not slot-relative — see protocol/v2/qbft/roundtimer/timer.go line 151-158 and #2429. Changes: - §2 background: expand the 4000ms budget equation to include QBFTRound1Time + QBFTRound2Time + PostConsensusSigningTime + BlockSubmissionTime; drop the now-removed "tighter constraint" 2000ms equation entirely. - Example A intro: replace "round-1 deadline" rationale with a reference to the 4000ms slot deadline. - Tuning section: replace the "round-1 deadline" bullet with a safety-margin framing for the ~2000ms cutoff guidance. - Appendix A: update the legacy formula to use the new term breakdown; remove the incorrect "1200ms = 2000ms minus everything else" derivation; keep the same ~1200ms practical ceiling but rejustify it as a buffer against variance in QBFT, submission, and relay payload-reveal latencies. No behavior change.
The previous Example B framed itself as "close equivalent of ProposerDelay = 1000ms" using the wrong dimension: it matched header-arrival time at SSV (~1050ms), whereas legacy ProposerDelay=1000ms actually delivers the header at SSV at ~1500-2000ms (after mev-boost's full getHeaderTimeout). The correct equivalence dimension is when relay bids are sampled. Both the previous Example B (last poll at ~1000ms) and legacy ProposerDelay=1000ms (single query at t=1000ms) sample bids at the same moment — PBS-timing-games just returns the result earlier, freeing slot budget. Restructure: - New Example A = previous Example B's numbers (cutoff=1050ms), reframed as the bid-sample equivalent of legacy ProposerDelay=1000ms. Now the recommended starting point. - New Example B = aggressive (cutoff=1800ms). Fully utilizes SSV's proposalSoftTimeout buffer; lands last relay poll at ~1600ms, header at SSV by ~1850ms. Captures more intra-slot bid growth at the cost of less variance margin. - Tuning section: update the "Example A starting point" reference to the new numbers.
The original 1000ms figure (inherited from the pre-rewrite doc) was conservative. Updating to ~200ms to better reflect typical observed latency for SSV → BN → relay submission + payload-reveal. Derived numbers in Appendix A propagated: - Theoretical ProposerDelay max: 2200ms → 3000ms. - Headroom buffer at the recommended ~1200ms practical ceiling: ~1000ms → ~1800ms. The recommended ~1200ms ProposerDelay ceiling is preserved.
The "Where the auction window should land" bullet used the term "cutoff" without explicit definition and omitted the ~70ms post-PBS, pre-QBFT overhead from the slot-budget arithmetic. - Replace "cutoff" with the explicit config field name `late_in_slot_time_ms`. - Add the ~70ms (BN→SSV transport + pre-QBFT blinding) to the math so the inequality is actually right.
The specific relay-data API endpoints and the delivered_value/max_bid_at_T metric aren't really SSV-doc material — operators interested in measuring MEV capture at that level can find this information in upstream relay docs.
Example A's late_in_slot_time_ms is 1050ms; with ~50ms BN→SSV transport overhead, the header arrives at SSV at ~1100ms, not ~1050ms. - Example A description: "~1050ms" → "~1100ms (1050ms PBS cutoff + ~50ms BN→SSV transport)". - Example B trade-off math: Example A's remaining slot budget at 4000ms − 1100ms = ~2900ms (was stated as ~2950ms). The "Example A's ~1050ms cutoff" reference in the Tuning section is unchanged — it correctly refers to the late_in_slot_time_ms value, not the header arrival time.
The original Appendix A values block treated QBFTRound2Time as "typically not needed" — implying it's safe to skip in the budget. That's wrong: for the slot deadline to hold in any realistic scenario (round 1 succeeds OR round 1 fails and round 2 runs), the equation must always include the round-2-fallback path. Changes: - §2 narrative: drop the "typical case round 2 = 0" framing; state explicitly that the equation must hold for the worst case where round 1 fails and round 2 runs. - Appendix A values: QBFTRound1Time set to 2000ms (the round-1 timer worst case), QBFTRound2Time set to 350ms (typical round-2 success). Theoretical ProposerDelay max accordingly drops from 3000ms to 1000ms. - Appendix A narrative: practical ProposerDelay ceiling lowered from ~1200ms to ~800ms (~200ms variance buffer below the new 1000ms theoretical max). - Example B trade-off: explicitly note that Example B's 2150ms remaining slot budget falls below the ~2700ms required for the worst-case 2-round scenario, so Example B accepts "round 1 must succeed" as an operational constraint. - Tuning section reference: Example B trade-off rephrased to match.
The round-change step between round 1 and round 2 was previously absorbed implicitly. Make it explicit as its own term so the equation reflects the actual sequence: round 1 fails (2000ms timer) -> ROUND-CHANGE handshake -> round 2 starts Updates: - §2 bullet, equation, and narrative include the new term and explain what QBFTRoundChange covers. - Appendix A: equation and values block include QBFTRoundChange ~ 150ms. Theoretical ProposerDelay max: 1000ms -> 850ms. Practical ceiling recommendation lowered from ~800ms to ~700ms (~150ms variance headroom against the theoretical max). - Example B trade-off: required post-cutoff budget for worst-case 2-round scenario updated from ~2700ms to ~2850ms (= 2000 R1 + 150 RC + 350 R2 + 150 signing + 200 submission).
The "Where the auction window should land" bullets still referenced an older ~2000ms cutoff guideline that was tied to the round-1-only budget framing. With the 2-round-budget framing now used throughout the doc, the meaningful threshold is ~1080ms (above which round-2 fallback no longer fits). Restructured to three bullets: - Round-2 viability inequality, with worst-case values plugged in to derive late_in_slot_time_ms ≲ ~1080ms. - ~1080ms boundary semantics — cutoffs above this accept "round 1 must succeed"; Example B (1800ms) is in this regime. - Higher ~2500ms threshold where even round-1-only path becomes risky due to latency variance.
The equation variables read more cleanly without the redundant Time suffix, and the three QBFT terms (Round1Time + RoundChange + Round2Time) are easier to reason about as a single QBFT bucket whose value covers the worst-case 2-round scenario (~2500ms). Renames applied in §2, Example B trade-off, Tuning bullets, and Appendix A: - RANDAOTime -> RANDAO - QBFTRound1Time + QBFTRoundChange + QBFTRound2Time -> QBFT - PostConsensusSigningTime -> PostConsensusSigning - BlockSubmissionTime -> BlockSubmission MEVBoostRelayTimeout is unchanged — its suffix is "Timeout", not "Time", and it refers to mev-boost's actual timeout setting. The §2 narrative now states the QBFT breakdown inline (2000ms R1 timer + ~150ms round change + ~350ms R2 ≈ 2500ms) so the worst-case 2-round budget remains visible. No numerical changes.
The §5 tuning bullet was the only equation in the doc that included pre-QBFT blinding (~20ms) in its overhead figure. The rest of the doc (§2 equation, Example A header-arrival, Example B trade-off math) already excluded blinding implicitly, so the bullet was the outlier. Drop the blinding term: post-cutoff overhead is now ~50ms (BN→SSV transport only), and the round-2 viability threshold accordingly becomes ~1100ms (was ~1080ms). Example A's 1050ms cutoff still fits the worst-case 2-round scenario, now with 50ms margin (was 30ms).
… paths Introduces a three-path model for the multi-BN block-header fetch: - Path 0 (legacy): preserves the original ProposerDelay/ProposalSoftTimeout behavior bit-for-bit. Selected when an operator sets either of those legacy knobs. - Path 1 (safe, default): multi-BN parallel fetch with early-exit on the first blinded response; falls back at slot-relative ProposalSoftDeadline (default 1000ms). Selected when no MEV-related knobs are set. - Path 2 (MEV-optimized, opt-in): no early-exit on blinded; collects all responses until slot-relative ProposalSoftDeadline. Selected when an operator sets ProposalSoftDeadline explicitly. Behavior change for default operators: the previous 1800ms relative- duration proposalSoftTimeout is replaced for default-config operators by a slot-relative 1000ms ProposalSoftDeadline. This is a deliberate safety improvement — the 1800ms default could push slot budget past the 4000ms deadline in worst-case multi-BN all-vanilla scenarios. Operators who explicitly set the legacy knobs keep the old behavior unchanged. Validation: - (ProposerDelay > 0 || ProposalSoftTimeout set) && ProposalSoftDeadline set -> startup rejected with an error. - Path 2: ProposalSoftDeadline must be in [1000ms, 3600ms]; values above 1800ms emit a startup warning (round-2 fallback no longer fits). - Path 0: startup logs a WARN nudging migration to the new model. Files: - beacon/goclient/options.go: BlockFetchPath type, DetermineBlockFetchPath, ValidateProposalSoftDeadline, NewOptions takes the path. - beacon/goclient/goclient.go: new fields on GoClient (proposalSoftDeadline, blockFetchPath). - beacon/goclient/proposer.go: rename getProposalParallel -> getProposalParallelLegacy; add getProposalParallelSafe and getProposalParallelMEVOptimized + shared helpers; dispatch from GetBeaconBlock. - cli/operator/node.go: path determination, validation, startup logging. - config/config.example.yaml: ProposalSoftDeadline + ProposalSoftTimeout comment blocks documenting the path-selection model. - docs/BLOCK_FETCH_PATHS_PLAN.md: design doc for the path split. Existing tests updated for the NewOptions signature change. Path-specific behavior tests follow in a separate commit, as do MEV_CONSIDERATIONS.md updates.
Covers: - DetermineBlockFetchPath across all input combinations, including the hard-fail case where legacy knobs and ProposalSoftDeadline are both set. - ValidateProposalSoftDeadline range checks (lower bound 1000ms, hard upper bound 3600ms, plus the safe-max boundary at 1800ms). - NewOptions path-specific defaulting for safe / legacy / MEV-optimized. - BlockFetchPath.String() formatting. Per-path behavior tests for getProposalParallelSafe and getProposalParallelMEVOptimized are not added here — the implementations share most of their structure with the existing getProposalParallelLegacy (already covered) and only differ in two specific places (slot-relative deadline + early-exit gating). Worth revisiting if the path implementations diverge further.
The doc previously referenced a single proposalSoftTimeout model. Update to reflect the path-selection model introduced by the beacon/goclient/options.go split (safe / legacy / MEV-optimized). Changes: - TL;DR: add a brief paragraph noting the three paths, linking to the new Configuration paths section. - Example A + B: add SSV-side YAML snippets with ProposalSoftDeadline, tied to each example's PBS-side late_in_slot_time_ms + ~50ms. - Example B: drop the "fully use SSV's ~1800ms header-fetch buffer" framing (which assumed the legacy proposalSoftTimeout default). Reframe as "PBS-side cutoff at 1800ms; round 1 must succeed." - Tuning bullet: add a note about matching ProposalSoftDeadline to late_in_slot_time_ms + ~50ms for Path-2 operators. - §6 "Interaction with ProposerDelay": replaced wholesale by a new "Configuration paths" section that explains the path-selection algorithm, the three paths individually, and how to opt into Path 2 for multi-BN cross-bid scoring. - Appendix A header: re-labeled "Path 0 (ProposerDelay, legacy approach)" for consistency with the path-selection terminology.
…cy paths The "Multi-BN caveat" bullet under the Tuning section described the early-exit-on-first-blinded behavior as if it always applied. After the three-path split, it only applies to Path 0 (legacy) and Path 1 (safe). Path 2 (MEV-optimized) explicitly disables that early-exit to enable cross-BN bid scoring. Update the bullet to make this scope-of-applicability explicit and direct operators wanting true cross-BN scoring to opt into Path 2 via ProposalSoftDeadline.
Add per-path behavior tests covering the dispatch and the key behavioral difference between paths 1 and 2: - TestNew_StoresBlockFetchPath: verifies the selected path and its associated timing field (proposalSoftDeadline / proposalSoftTimeout) propagate from Options into the resulting GoClient. - TestGetBeaconBlock_MultiBN_Path1_EarlyExitOnBlinded: with one fast and one slow BN both returning blinded proposals, asserts the safe path returns in <250ms (early-exit on first blinded). - TestGetBeaconBlock_MultiBN_Path2_NoEarlyExit: same setup; asserts the MEV-optimized path waits for the slow BN (>=400ms) before returning. The multi-BN tests use semicolon-separated BN URLs and a slot two slots in the future to ensure the slot-relative ProposalSoftDeadline lands after both BN responses — so the early-exit behavior, not the deadline firing, is what's being observed. Also delete docs/BLOCK_FETCH_PATHS_PLAN.md — the plan-doc is no longer needed now that the implementation has landed and the public-facing content lives in docs/MEV_CONSIDERATIONS.md.
The block-fetch path-determination comment referenced docs/BLOCK_FETCH_PATHS_PLAN.md which was deleted in the previous commit. Keep the docs/MEV_CONSIDERATIONS.md reference, which now covers all the operator-relevant content.
Restructure the doc to open with a "Definitions and typical values" section: a table defining each stage of the proposer-duty timeline (RANDAO, auction window, MEVBoostRelayTimeout, QBFT, PostConsensusSigning, BlockSubmission) with typical values for a healthy mainnet SSV cluster, explicitly framed as illustrative rather than authoritative. Slim duplications throughout: - §2 background no longer redefines variables — references the table. - The QBFT worst-case decomposition (R1 timer + round change + R2) lives only in the QBFT row of the table; previous duplications in §2 narrative, Example B trade-off, and Appendix A values block are removed. - Appendix A drops the realistic-numbers values block in favor of a one-line derivation using the typical values from the table. - Example A's "not equivalent in header-arrival time" paragraph collapsed to one sentence. Remove the "Multi-BN caveat" subsection from the Tuning section entirely — its content was an explanatory note about the early-exit-on-blinded behavior that the Configuration paths section already covers. Net: 297 -> 280 lines, with significantly less repetition.
#1 (math bug): SafeMaxProposalSoftDeadline 1800ms -> 1100ms. The previous 1800ms claimed "round-2 QBFT fits" but the math doesn't: with QBFT worst-case 2-round = 2500ms, signing 150ms, submission 200ms, plus 50ms BN->SSV transport, deadline must be <= 1100ms for round-2 to fit within the 4000ms slot deadline. Updated the comment in options.go, the warning text in node.go, and references in MEV_CONSIDERATIONS.md (Path 2 section, Example B narrative + SSV-side note). options_test.go updated for the new safe-max boundary. #5: waitForFirstValidProposal now joins accumulated BN failure errors with ctx.Err() on slot deadline, preserving diagnostic context. #6 (doc): clarify that 700-1000ms ProposerDelay is permitted by the safety guard but is risky and not recommended. Doc-only change; the 1000ms cap stays as the hard safety threshold. #7: re-add concrete measurement function references (measurements.PreConsensusTime / ConsensusTime in protocol/v2/ssv/runner/) and the SubmitBeaconBlock entry-point to the "What to measure first" section. Operators without Grafana exports need these to instrument their stack. #8 partial: add TestGetBeaconBlock_MultiBN_SoftDeadlineFires_FallsBackToFirstValid verifying that paths 1/2 fall through to waitForFirstValidProposal when the slot-relative soft deadline has already fired. Uses a slot in the past so softCtx is done immediately at collection-loop entry. #9 (doc): tighten Example A's legacy-arrival range (~1300-2000ms, not ~1500-2000ms), and rewrite Example B's "latest practical value" framing to match the corrected 1100ms threshold from #1. #2 + #3: add a multi-BN pointer to TL;DR directing operators with multiple Beacon nodes to opt into Path 2 by setting ProposalSoftDeadline. Pushed back on (with rationale): - Reviewer claim that single-BN sees the "did not receive any valid proposals" log: incorrect. Single-BN goes through the direct fetchProposal path in GetBeaconBlock (proposer.go:103) and never enters getProposalParallel*; the log is multi-BN only. - Lowering MinProposalSoftDeadline below 1000ms: marginal value. Operators with low PBS cutoffs can still opt into Path 2 with the 1000ms floor (just wastes some wait time, not broken). - Lowering the ProposerDelay safety cap from 1000ms to 700ms: behavior change affecting existing operators using 700-1000ms. Clarified the gap in doc instead. - Startup-fatal end-to-end test for ProposerDelay+ProposalSoftDeadline combo: more infra for marginal return; unit test on DetermineBlockFetchPath already covers the logic. - Code-duplication parameterization for getProposalParallelSafe vs getProposalParallelMEVOptimized: the explicit split was deliberate per design discussion; merging would add control-flow complexity.
P1: createProposalResponseSafe mutates pointer-shared ssv-spec fixture blocks (TestingBlindedBeaconBlockV / TestingBeaconBlockV return wrappers that point at cached structs). Single-BN tests never hit this because only one server-handler goroutine runs at a time. The new multi-BN tests trigger two server goroutines concurrently, racing on block.Slot and block.Body.*.FeeRecipient. Confirmed locally with `go test -race`. Fix: package-level sync.Mutex around createProposalResponseSafe. Serializes the mutation+marshal so each call's bytes capture its own intended state. Verified: `go test -race ./beacon/goclient` now passes. P2: DetermineBlockFetchPath used `> 0` checks for path selection, which silently treated negative values as "unset". A negative ProposalSoftDeadline (-100ms for instance) would route through the safe path with proposalSoftDeadline preserved as-is, then slotStart.Add(negative) produces a deadline in the past — softCtx fires immediately, multi-BN scoring is skipped, the operator gets no warning. Fix: reject negative values for ProposerDelay, ProposalSoftTimeout, and ProposalSoftDeadline upfront in DetermineBlockFetchPath with a clear "must be non-negative" error. Operators get a startup fatal instead of confusing silent behavior. Three new test cases cover each variable. Cleanup note (reviewer): the Safe vs MEV-optimized collectors are nearly identical except for the blinded early-exit. Keeping them split is intentional per design discussion; revisit if they drift. No action needed now.
…vs legacy The reader-facing model now is just two approaches and the interaction between them — no internal path numbers. - Rename "Configuration paths" section to "SSV-side block-fetch configuration". Restructure into "New approach (recommended)" / "Legacy approach" / "Interaction" subsections. - Drop "Path 1 — Safe (default)", "Path 0 — Legacy", "Path 2 — MEV-optimized (opt-in)" headings. The default-vs-cross-BN-scoring variants of the new approach are now described in prose within the "New approach" subsection. - Rename Appendix A header from "Path 0 (ProposerDelay, legacy approach)" to "Legacy ProposerDelay approach". - TL;DR drops the "MEV-optimized fetch path" framing for the multi-BN pointer; just says "set ProposalSoftDeadline to opt into cross-BN bid scoring." - Example A / Example B SSV-side notes and the Tuning section bullet rephrased to avoid "Path 2" / "Path-2 operators" labels. - All anchor links updated to the new header slugs. Internal code names (BlockFetchPathSafe / Legacy / MEVOptimized) are unchanged — this is a doc-terminology cleanup only.
… test Drop "path 0/1/2" terminology throughout code/comments/tests to match the docs framing in MEV_CONSIDERATIONS.md. Rename proposer_paths_test.go to proposer_path_dispatch_test.go. config.example.yaml: warn threshold 1800ms -> 1100ms (matches SafeMax); note that ProposalSoftDeadline=1000ms is not a no-op. options.go: document MaxProposalSoftDeadline=3600ms rationale. Block-fetch dispatcher in GetBeaconBlock: drop case+fallthrough; explicit case for safe, default returns an error for unknown paths. cli/operator: scope-of-validation comment on the legacy switch arm; split the SafeMax startup warning into clauses. Existing 'races multiple clients' test now exercises the multi-BN logic again - switch to dynamic response generation with a future slot so the safe path's slot-relative deadline doesn't fire before the collection loop starts. Add TestGetBeaconBlock_MultiBN_MEVOptimizedPath_HighestScoringBlindedWins covering the MEV-optimized path's defining behavior. Extract writeProposalHeaders helper and plumb optional ExecutionValue header through the test server.
TL;DR: rephrase "doesn't consume slot budget" — the auction wait happens either way; the real PBS advantage is multi-polling per relay within a slot-relative window. Add note that ProposalSoftDeadline and the legacy ProposerDelay / ProposalSoftTimeout are mutually exclusive (pointer to Interaction section). PBS-side timing games section: replace the same misleading bullet with two accurate ones (multi-poll within the window; slot-relative cutoff → predictable QBFT start). Drop the round-relative-vs-slot-relative #2429 aside from the QBFT row. Drop the 'Mainnet vs testnet' and 'Iteration discipline' subsections.
The 3600ms upper bound on ProposalSoftDeadline exists precisely so hyper-tuned clusters can still benefit from late auction windows. Several spots in the docs, options.go, the cli warning, and config.example.yaml were stating the round-2 fallback "can no longer fit" / "will not fit" / "is missed" as if it were universal; reality is "for typical clusters, may not fit / may be missed — depends on your QBFT and submission speed". Edits use "may not fit" / "may be missed" / "for typical clusters", adding an explicit note in a couple of places that clusters with measurably faster QBFT + submission can still leave room for round 2. The "round 1 must succeed" phrase is kept where it frames the operator's acknowledged trade-off (Example B, the cli warning's parenthetical, the options.go comment), since there it describes the configuration intent rather than asserting a hard fact about timing.
Single section "What to measure" grouped by data source (SSV side / PBS side / end-to-end). Drop implementation-detail references (function names like measurements.PreConsensusTime, file paths like protocol/v2/ssv/runner/, beacon/goclient/proposer.go) — operator-facing doc should describe the signals, not where they live in the code.
Rename "SSV-side block-fetch configuration" to "Multi-BN setup" and restructure so the recommended action (set ProposalSoftDeadline) leads; add a blockquote at the top telling single-BN operators to skip the section entirely. Rewrite the TL;DR multi-BN paragraph as a direct instruction: ProposalSoftDeadline = your PBS late_in_slot_time_ms + ~50ms transport with a pointer to the new section. Drop the sprinkled multi-BN/cross-BN parentheticals in: - TL;DR PBS-side paragraph (replace "cross-BN bid scoring" wording and update link) - Example A and Example B SSV-side prefaces (now read "multi-BN setups only — see Multi-BN setup; single-BN operators skip this") - Tuning bullet (Round-2 fallback) — the "For operators who set ProposalSoftDeadline..." parenthetical is redundant now that the Multi-BN setup section leads with that instruction - Interaction pseudocode comment (describe behavior instead of using the dropped "cross-BN bid scoring" term)
… 1450ms docs/MEV_CONSIDERATIONS: - RANDAO 100 -> 50, QBFTRoundChange 150 -> 100, QBFTRound2Time 350 -> 250 (QBFT worst-case 2-round: 2500 -> 2350), PostConsensusSigning 150 -> 50, BlockSubmission 200 -> 100. - Re-derive thresholds throughout: post-cutoff budget needed 2850ms -> 2500ms, safe-max threshold 1100ms -> 1450ms. - Appendix A theoretical ProposerDelay max 850ms -> 1250ms (recommended ~700ms unchanged; headroom grows from ~150ms to ~550ms). beacon/goclient/options.go: - SafeMaxProposalSoftDeadline 1100ms -> 1450ms with new derivation in the comment (the "largest safest" deadline under the tightened estimates). - DefaultProposalSoftDeadline now equals SafeMax (was 1000ms) - the safe path defaults to the largest deadline that still fits worst-case 2-round QBFT. - MinProposalSoftDeadline decoupled from Default and held at 1000ms - Min exists to floor the BN-response window, not to mirror Default. Lets operators opt into MEV-optimized with a tighter-than-default deadline (e.g., to match an early PBS cutoff like Example A's 1100ms). Tests + config.example.yaml updated to track the new thresholds.
… variance heuristic - docs/MEV_CONSIDERATIONS.md "Default behavior" subsection still cited "(default 1000ms)" — update to "(1450ms — the largest safest deadline for typical clusters)" with a pointer to the Tuning guidance section. - Tuning bullet variance heuristic "much beyond ~2500ms" → "~3000ms" to align with the tightened typical values (more headroom for round-1-only cutoffs now that post-deadline budget shrank from ~2850ms to ~2500ms). - beacon/goclient/options.go: reorder the ProposalSoftDeadline const block so SafeMaxProposalSoftDeadline is declared first and DefaultProposalSoftDeadline = SafeMaxProposalSoftDeadline references an already-declared constant. Functionally identical (Go const blocks permit forward references), but easier to read top-to-bottom.
- Consolidate getProposalParallelSafe + getProposalParallelMEVOptimized into getProposalParallelByDeadline(..., earlyExitOnBlinded bool). Behavior unchanged; shared helper's doc folds in the in-flight-goroutine cleanup note. - proposer_path_dispatch_test.go: widen timing bounds (250->350ms upper for EarlyExitOnBlinded; 400->450ms upper for SoftDeadlineFires) to reduce CI flake; add a fee-recipient identity assertion to the past-slot fallback test so the primary signal isn't elapsed-time-only. - Replace remaining "default 1000ms" with DefaultProposalSoftDeadline references (4 sites: BlockFetchPathSafe const doc, ProposalSoftDeadline struct field doc, NewOptions safe-path case, config.example.yaml). The default has been 1450ms since the typical-values tightening commit; these doc/yaml sites had been overlooked. - Document the defensive 'if options.ProposalSoftDeadline == 0' branch in NewOptions case BlockFetchPathSafe, the intentional strict '>' (not '>=') in the SafeMax warning, and that range validation via ValidateProposalSoftDeadline is the caller's responsibility (cli/operator runs it; NewOptions does not enforce it). - Tighten GoClient.blockFetchPath comment to describe the consolidated dispatch; drop leftover "path-0/path-2" terminology in the DetermineBlockFetchPath doc.
The first commit's SafeMaxProposalSoftDeadline doc-comment carried a math inconsistency flagged on review: the +50ms BN->SSV transport term made sense only if "deadline" meant PBS cutoff, but the constant is applied to the SSV-side ProposalSoftDeadline, and the doc tells operators to set ProposalSoftDeadline = PBS cutoff + 50ms. So an operator setting PBS cutoff to the documented ceiling (1450ms) computed ProposalSoftDeadline = 1500ms and tripped the >1450ms startup warning despite following the docs. Fix: rewrite the doc-comment with the honest strict-math derivation (ProposalSoftDeadline <= 1500ms) and explain the 1450ms threshold sits 50ms tighter as a variance buffer. Update the Tuning guidance section to recommend PBS cutoff <= ~1400ms (matches the SSV warning threshold via +50ms transport), while preserving the 1450ms strict-bound reference for the round-2-fit cliff. Constant value (1450ms) unchanged. Also addresses cleanup #1: NewOptions returned an error type but had no error path. Add a default arm that rejects unknown BlockFetchPath at startup rather than at per-slot dispatch in proposer.go. Test added for the new error path; the proposer.go default arm stays as defense-in-depth.
The MultiBN path-dispatch tests previously relied on time.Sleep delays
in the mock BN server combined with wall-clock elapsed-time bounds in
assertions. That made them CI-sensitive (per the review feedback that
flagged the timing-bound assertions as flake-prone) and slowed the suite
— each test ran in 200–500ms purely from sleeping.
Add a Release <-chan struct{} field to beaconProposalServerOptions. When
non-nil, the proposal endpoint handler blocks until the channel receives
a value, is closed, or the request context cancels. ProposalResponseDuration
remains the default for tests that don't need ordering control.
Refactor the four MultiBN path-dispatch tests
(SafePath_EarlyExitOnBlinded, MEVOptimizedPath_NoEarlyExit,
MEVOptimizedPath_HighestScoringBlindedWins, SoftDeadlineFires_FallsBackToFirstValid)
to:
- run GetBeaconBlock in a background goroutine via launchGetBeaconBlock,
- close Release channels to deterministically order BN responses,
- assert response identity (fee recipient) instead of elapsed time,
- bound the wait on a 2s safety timeout, not a tight CI-sensitive bound.
The NoEarlyExit test additionally uses a short (100ms) non-return check
after releasing BN1 — strictly weaker than the old 400ms lower bound and
sufficient to detect a regression to early-exit behavior on the
MEV-optimized path.
Net effect: each affected test runs in ~10ms instead of 200–500ms, and
none depend on tight wall-clock bounds.
Codecov Report✅ All modified and coverable lines are covered by tests. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
iurii-ssv
force-pushed
the
mev-considerations-revised
branch
7 times, most recently
from
June 5, 2026 12:06
4054c1d to
e40a5fe
Compare
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
1 participant
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.
Summary
Follow-up to #2855 addressing reviewer cleanup #2: the MultiBN path-dispatch tests in
beacon/goclient/proposer_path_dispatch_test.gopreviously depended on wall-clock timing bounds (elapsed >= 400ms,elapsed < 350ms) combined withtime.Sleepdelays in the mock BN server. CI-sensitive, slow, and brittle under load.This PR gates BN responses on per-test
Release <-chan struct{}channels so the assertions become identity/ordering-based instead of timing-based.What's changed
beacon/goclient/proposer_test.go—beaconProposalServerOptionsgains aReleasefield. When non-nil, the proposal-endpoint handler waits on it (also unblocking on request context cancellation) before responding.ProposalResponseDurationremains the default for tests that don't need ordering control, so existing tests are unaffected.beacon/goclient/proposer_path_dispatch_test.go— four tests refactored:TestGetBeaconBlock_MultiBN_SafePath_EarlyExitOnBlinded— release BN1 only; assert response identity == BN1.TestGetBeaconBlock_MultiBN_MEVOptimizedPath_NoEarlyExit— release BN1, assert no return within 100ms (vs old 400ms lower bound), then release BN2.TestGetBeaconBlock_MultiBN_MEVOptimizedPath_HighestScoringBlindedWins— release both; assert response identity == BN2 (high-bid winner).TestGetBeaconBlock_MultiBN_SoftDeadlineFires_FallsBackToFirstValid— release BN1 only (past slot → softCtx already fired →waitForFirstValidProposal); assert response identity == BN1.Helpers introduced:
launchGetBeaconBlock— spawnsGetBeaconBlockin a background goroutine, returns a result channel + cancel func.requireBeaconBlockResult— blocks on the result channel with a 2s safety timeout.assertNoReturnWithin— fails if a result arrives within a short window (used to verify non-early-exit).assertProposalFeeRecipient— extracts and compares fee recipient.Effect
Each of the four refactored tests runs in ~10ms instead of 200–500ms. None depend on tight wall-clock bounds. The only remaining timing knob is the 2s safety timeout on the result channel — comfortably above any reasonable test runtime, and serves as a "GetBeaconBlock never returned" failure signal.
Test plan
go vet ./...clean.go build ./...clean.go test -race -count=1 ./beacon/goclient/... ./cli/operator/...passes locally.Releasemechanism increateProposalBeaconServeris the right abstraction (vs. e.g. injecting a delay function).Base
Targets
mev-considerations-revised(the head branch of #2855). Will rebase ontostageautomatically when #2855 merges.