Stabilize Issue #38 CI races and sync preflight - #47
Merged
Conversation
…#38, defect 3) - Fail on missing/empty ALCHEMY_KEY in a 5-minute preflight job before the ~20 minute release build (GitHub passes a missing secret as an empty string, which also slipped past the test's env::var skip-guard). - Gate the workflow to workflow_dispatch: since the Tsuki migration the 15k consolidation test uses DOGEOS_CHIKYU, whose genesis lacks scroll.l1Config.startL1Block, so NodeConfig::from_chainspec fails before any networking and no secret can make the workflow green. A follow-up issue re-points the test at chikyu infrastructure before re-enabling push.
…(issue #38) test_should_consolidate_after_optimistic_sync armed the sequencer's 20ms auto-build timer while driving 200 manual build_block() calls asserting exact block numbers. A manual BuildBlock silently replaces an in-flight auto job (payload building = 40ms > the inter-iteration gap), which normally hides the timer; on a slow runner an auto job completed inside the gap, claimed the expected number, its event was consumed by an interleaved waiter, and the exact-number wait timed out after 30s — the CI failure in issue #38 (defect 1). The timer now stays off during the exact-numbered setup loop and is enabled via enable_automatic_sequencing() for the sync/consolidation phase, whose one-shot gossip waits rely on its continuous block stream (removing it entirely made those waits flaky the other way — verified locally). Assertions are unchanged. Also in this file: - treat an empty ALCHEMY_KEY as unset in the 15k consolidation test's skip-guard (GitHub passes a missing secret as an empty string; issue #38, defect 3), and - bound wait_n_events with a 60s timeout so an unmet expectation fails with a diagnosis instead of hanging the binary for the whole job timeout (test_chain_orchestrator_l1_reorg was observed doing exactly that on a pre-existing, unrelated race). Verified: rollup-node::sync suite 14/14 locally; the fixed test passed 3/3 standalone plus the full-suite run.
…act-number waits (issue #38) The event waiter consumed at most one event per 10ms sleep (~100 events/s). Under load that let the broadcast event channel (capacity 5000, silently lossy on lag via reth's EventStream) back up and drop events, turning a backlog into a lost-event hang. The waiter now drains every immediately-available event before sleeping. block_sequenced(target) additionally fails fast with a diagnosis when it observes a BlockSequenced above the target: numbers are monotone per node, so the awaited event can no longer arrive and a silent 30s timeout would just hide the story.
… payload job (issue #38) A manual BuildBlock command called start_payload_building directly and silently replaced any in-flight (typically timer-triggered) payload job, discarding its engine work and making block numbering timing-dependent whenever the build timer and manual triggers race — the mechanism behind the optimistic-sync test flake. The command now coalesces with the in-flight job; callers wait on BlockSequenced/BlockBuildingSkipped, which that job emits. A black-box red test for the old behavior is not possible in isolation (a replaced job emits nothing, so replace and coalesce look identical without the timer); the added test pins the coalescing contract: two rapid build_block() calls yield exactly one block and contiguous numbering. Note: an A/B experiment (guard present 1/3 vs absent 2/6 failures) confirmed the pre-existing test_chain_orchestrator_l1_reorg flake is unrelated to this change — it panics in reth's nippy-jar static-file reader after the L1-reorg unwind (tracked separately).
… remote down (issue #38) Adds a remote_source_url() override to the test fixture builder and a test that points the remote-source node at a dead port, asserts the node still launches, then brings the port up via a TCP proxy to the sequencer RPC and asserts the node imports and builds on top. Red until the next commit: RemoteBlockSourceAddOn::new() currently probes the remote during launch_add_ons, and a connection-refused error (which alloy's default retry policy does not retry) aborts the entire node — the docker_test_remote_block_source_basic startup race from issue #38 (defect 2).
…nch (issue #38) RemoteBlockSourceAddOn::new() called get_block_number() plus the common-ancestor walk inside launch_add_ons. alloy's default retry policy retries only rate-limit/temporarily-unavailable responses — connection-refused is not retried — so a remote that wasn't up yet (the sequencer container racing the remote-source container in docker-compose.remote-source.yml, which orders neither) aborted the whole node and its own RPC on 8546 never came up. The resume point (last_imported_block) is now Option<u64>, determined on the first successful poll inside the run loop, where errors are already logged and retried at poll cadence. Construction performs no remote I/O, so node launch never depends on remote availability; restart-time resume detection is unchanged (still exercised by test_remote_block_source_resumes_from_correct_head). Fixes docker_test_remote_block_source_basic and the same one-shot race previously seen in docker_test_remote_block_source_recovery.
…o-fail-fast lanes Discovered while collecting repeated-green evidence for issue #38: a rerun of the docker lane failed in docker_test_heterogeneous_client_sync_and_sequencer_handoff with anvil rejecting L1 txs ('max fee per gas less than block base fee'). The L1 messenger/gateway helpers pinned gas_price at 0.1 gwei, so whenever the continuous senders pushed the EIP-1559 base fee above that, every subsequent L1 message failed. Fees are now estimated per-send. Both integration lanes also run with --no-fail-fast so a single flaky test cannot cancel the rest of the lane's evidence (the failing rerun cancelled the remote-source pair before it ran).
This was referenced Sep 7, 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
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.
Closes #38.
This is PR 1 extracted from #45. It contains only the root-cause fixes needed for the three reported Issue #38 failures, plus one tightly scoped Docker test reliability fix discovered while reproducing them.
What this fixes
BuildBlockcommand with an in-flight payload job instead of replacing that job.ALCHEMY_KEYas missing.--no-fail-fastso one flaky integration test cannot erase evidence from the rest of its lane.Scope
This PR intentionally excludes the review-tail changes from #45: cancellation/rewind and CAS logic, fatal divergence policy, finalized-marker replay, L2 sync latching, broader remote transport/config validation, credential redaction, genesis/startup forkchoice work, DB restore classification, FCU policy, and soak automation. Those changes have been separated into local follow-up branches for individual review. The original #45 remains open as the preserved reference.
Known pre-existing behavior is not widened here: a cancelled remote build can still leave an outcome wait unbounded, and an unavailable remote can still log once per poll interval. Both are deferred rather than mixed into this fix.
Verification
cargo +nightly-2026-01-05 fmt --all -- --checkcargo +nightly-2026-01-05 clippy --workspace --lib --examples --tests --benches --all-features -- -D warningscargo +nightly-2026-01-05 check --workspace --all-features --all-targets --locked --quieton every prepared split branchNo breaking configuration changes are included in this PR.