Skip to content

fix(devnet): gate L2 startup on beacon readiness and tune healthchecks - #4944

Merged
danyalprout merged 3 commits into
mainfrom
danyal/devnet-healthchecks
Sep 8, 2026
Merged

fix(devnet): gate L2 startup on beacon readiness and tune healthchecks#4944
danyalprout merged 3 commits into
mainfrom
danyal/devnet-healthchecks

Conversation

@danyalprout

@danyalprout danyalprout commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator

Description

Speed up/simplify healthchecks for devnet

@cb-heimdall

cb-heimdall commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator

✅ Heimdall Review Status

Requirement Status More Info
Reviews 1/1
Denominator calculation
Show calculation
1 if user is bot 0
1 if user is external 0
2 if repo is sensitive 0
From .codeflow.yml 1
Additional review requirements
Show calculation
Max 0
0
From CODEOWNERS 0
Global minimum 0
Max 1
1
1 if commit is unverified 0
Sum 1

@depot-code-access

depot-code-access Bot commented Sep 7, 2026

Copy link
Copy Markdown

✅ All benchmarks green — 14 within ±2% (deterministic instruction counts). View run

Benchmark details (14)
Benchmark Base (target) Head (this PR) Δ instructions
batch_queue/drain/drain_cached_span_batches 242,027 242,027 +0.0%
batch_transaction/encode_in_place/encode_in_place 4,199,759 4,199,759 +0.0%
batch_transaction/temporary_frame_buffers/temporary_frame_buffers 8,408,350 8,408,350 +0.0%
flashblock_decode/decode/brotli 3,296,484 3,296,484 +0.0%
flashblock_decode/decode/plain_json 2,280,194 2,280,194 +0.0%
flz/compress_len/real_contract_call 43,148 43,148 +0.0%
flz/compress_len/synthetic_0 38,205 38,205 +0.0%
flz/compress_len/synthetic_1 54,682 54,682 +0.0%
flz/compress_len/synthetic_2 147,976 147,976 +0.0%
flz/data_gas 43,059 43,059 +0.0%
flz/tx_estimated_size 43,056 43,056 +0.0%
frame_parse/decode/single_4kib 1,031 1,031 +0.0%
frame_parse/parse_frames/few_large 1,053,062 1,053,062 +0.0%
frame_parse/parse_frames/many_small 154,763 154,763 +0.0%

@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Review Summary

This PR fixes a race condition in devnet startup where the L2 bootnode could start before Lighthouse's beacon API was actually ready (a listening TCP port doesn't guarantee /eth/v1/beacon/genesis is servable). The changes are well-scoped to devnet Docker Compose infrastructure.

Changes reviewed:

  • Lighthouse healthcheck upgraded from TCP port check to HTTP 200 validation on /eth/v1/beacon/genesis — correct and uses the same bash + /dev/tcp pattern already established by the prior check
  • base-bootnode now explicitly depends on l1-el and l1-cl being healthy, gating L2 startup on L1 readiness
  • Anvil variant correctly overrides depends_on to exclude disabled Lighthouse services
  • Common healthcheck timing extracted into x-healthcheck-defaults anchor, reducing duplication
  • start_interval: 250ms added consistently across HA, ingress, and Anvil compose files

No findings. The dependency graph is sound (diamond through setup-l2l1-el is handled correctly by Compose), the YAML anchor scoping is correct (each file defines its own since anchors don't cross file boundaries), and the raw HTTP approach in the healthcheck avoids a curl dependency while staying within the container's existing bash capabilities.

Comment thread etc/docker/docker-compose.prover.yml
@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Review Summary

Devnet-only Docker Compose changes to gate L2 startup on beacon readiness and normalize healthcheck timing. No Rust code changes.

Finding

docker-compose.prover.yml — missing start_interval (minor inconsistency): The x-healthcheck-defaults anchor in the prover file omits start_interval: 250ms, diverging from the identical anchor defined in docker-compose.yml, docker-compose.ha.yml, and docker-compose.anvil-l1.yml. See inline comment.

Notes

  • The new l1-cl beacon healthcheck using raw bash TCP + HTTP status parsing is a sound approach to avoid a curl dependency in the Lighthouse image. The read -r protocol status rest correctly isolates the numeric status code from the HTTP response line.
  • YAML merge key ordering is correct in all files — explicit keys properly override merged defaults (e.g., conductor healthcheck timeout: 2s, minio start_period: 5s).
  • The Anvil variant correctly overrides base-bootnode depends_on to drop the l1-cl dependency (Lighthouse is disabled there).
  • $$status escaping in the Compose healthcheck is correct (Compose $$ → shell $).

@depot-code-access

depot-code-access Bot commented Sep 7, 2026

Copy link
Copy Markdown

Base Std historical fork tests

Fork Result Passed Failed Skipped base/base base-anvil base-std
Beryl pass 616 0 13 0bd117c6 98e7839c 520d069b
Cobalt pass 725 0 14 0bd117c6 98e7839c 4571b325

View run

@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Review Summary

This PR refactors devnet healthcheck configurations across five Docker Compose files, extracting repeated timing values into YAML anchors and adding a more robust beacon readiness check for L2 startup gating.

No issues found. The changes are correct:

  • YAML merge-key overrides (e.g., conductor-healthcheck overriding timeout/start_period, minio overriding start_period) work as intended — explicit keys take precedence over merged anchor values.
  • The new l1-cl healthcheck correctly validates the beacon genesis API endpoint rather than just a listening TCP port, preventing a race where base-bootnode starts before the beacon API is ready.
  • The anvil-l1 variant correctly uses !override to drop the l1-cl dependency (since Lighthouse is disabled) while keeping l1-el.
  • The previous review finding about docker-compose.prover.yml missing start_interval has been addressed.

Minor note (not blocking): transaction-events-postgres in docker-compose.ingress.yml uses the same defaults as x-healthcheck-defaults (from docker-compose.yml) but spells them out inline rather than using an anchor. The PR only added start_interval to it. This could be refactored for consistency but the values match and it is functionally correct.

@danyalprout
danyalprout requested a review from refcell September 7, 2026 17:41
@danyalprout
danyalprout added this pull request to the merge queue Sep 8, 2026
Merged via the queue into main with commit 10a58da Sep 8, 2026
26 checks passed
@danyalprout
danyalprout deleted the danyal/devnet-healthchecks branch September 8, 2026 13:45
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.

3 participants