[MegaRedHand]

Motivation

The spec fixtures check the state transition in isolation. Nothing checks that an ethlambda beacon node and the ethlambda validator client work end to end next to another client: building and proposing blocks, attesting, finalizing and staying in sync.

Description

This adds a kurtosis + assertoor battery that runs on every push to main.

• tooling/kurtosis-assertoor/network_params.yaml describes a mainnet-preset devnet run with lambdaclass/ethereum-package, pinned to bbae5eb7. That fork adds cl_type: ethlambda and vc_type: ethlambda, so ethereum-package's built-in assertoor sees ethlambda as an ordinary participant. The devnet has three participants:
  • lighthouse + geth, which is the bootnode
  • lighthouse + geth with the ethlambda VC
  • ethlambda + geth with the ethlambda VC
• Assertoor runs stability-check, block-proposal-check (every pair holds keys) and synchronized-check. The image and the playbooks are both pinned to assertoor 87bbbd6.
• .github/workflows/kurtosis_assertoor.yml picks the image to test:
  • On push to main, it waits for this commit's sha-<7> image from docker_publish.yaml, so no Rust is built in the job.
  • On PRs that touch the battery, it uses unstable.
  • workflow_dispatch takes an image input.
• The workflow uses the kurtosis CLI rather than kurtosis-assertoor-github-action. The fork's kurtosis.yml still names ethpandaops/ethereum-package, so kurtosis refuses github.com/lambdaclass/ethereum-package@<sha>, and the action only runs remote packages. Instead the workflow clones the fork at the pinned SHA.
• The enclave dump and the assertoor results are uploaded on every run. A failure on main is posted to the general Slack channel.

Known failure

Every test currently fails after assertoor's 5-minute client health check. Assertoor compares each client's /eth/v1/config/spec field by field. Lighthouse reports GLOAS_FORK_EPOCH as far future, while ethlambda leaves the key out, as crates/net/rpc/src/beacon/config.rs documents. Assertoor therefore marks the lighthouse nodes offline. This resolves once gloas support merges.

The chain itself was healthy in the local run: all three pairs proposed, the nodes shared a head, and the chain finalized. Until then, synchronized-check passes vacuously, because assertoor has no online clients to check.

How to test

Follow "Run it locally" in tooling/kurtosis-assertoor/README.md: clone the fork at bbae5eb7 and run kurtosis run <clone> --args-file tooling/kurtosis-assertoor/network_params.yaml. This PR's own CI run exercises the battery against unstable.

[github-actions]

🤖 Codex Code Review

I found 2 substantive issues; both are in the CI harness rather than consensus logic itself.

• Missing yq dependency breaks the workflow — .github/workflows/kurtosis_assertoor.yml:117 and .github/workflows/kurtosis_assertoor.yml:133 call yq, but the workflow never installs it. ubuntu-latest does not guarantee yq, so this can fail before the devnet even starts. Add an explicit install step or use an action that provides yq.

• The wait script can falsely fail on successful runs with null timestamps — tooling/kurtosis-assertoor/scripts/wait-assertoor.sh:59 computes (.stop_time - .start_time) for every run, even after timeout or for runs missing timing fields. In jq, null - number errors, and with set -euo pipefail that aborts the script after results were already collected. That turns “we have a meaningful failure report” into a less-informative hard script failure. Guard the expression, e.g. print duration only when both timestamps are numbers.

A few smaller review notes:

• Supply-chain exposure is partly mitigated, but playbooks are still fetched at runtime — tooling/kurtosis-assertoor/network_params.yaml:51, :54, :59 pin raw GitHub URLs to a commit, which is better than floating refs, but CI still depends on external availability/content delivery during execution. For a security-sensitive project, vendoring these playbooks or pulling them from the pinned package checkout would make runs more reproducible.

• Static enclave name can cause collisions in re-runs/manual local use — .github/workflows/kurtosis_assertoor.yml:36 uses a fixed ENCLAVE. GitHub-hosted runners are isolated so this is probably fine in CI, but it makes the local docs/workflow pattern easier to trip over. A run-specific suffix would be safer for maintainability.

• Consensus-layer coverage is good but indirect — the battery exercises finality/sync/block production through multi-client integration, which is valuable, but it does not directly validate fork-choice edge cases, attestation validity corner cases, SSZ/XMSS handling, or state-transition invariants in isolation. That’s not a bug in the PR, just worth being clear about.

Overall, this looks well thought out: pinned package commit, pinned assertoor commit, artifact upload on failure, and safe Slack payload construction are all solid.

---

Automated review by OpenAI Codex · gpt-5.4 · custom prompt

[github-actions]

🤖 Claude Code Review

Review of PR 679: Kurtosis + assertoor battery (CI, tooling and docs only)

I read the diff only and did not run the workflow. No Rust or consensus code changes, so the risk is limited to CI behaviour.

Main concern

1. The workflow will be red on every push to main and will post to Slack each time. tooling/kurtosis-assertoor/README.md ("Known failure") says every test fails because /eth/v1/config/spec omits GLOAS_FORK_EPOCH. Merging this as is makes the job a permanent failure and trains people to ignore the Slack alert. Options:
   • land the spec-endpoint fix first;
   • make the notify job and Wait for the assertoor tests non-blocking until then (continue-on-error, or workflow_dispatch only);
   • open a tracking issue and link it from the README.

Smaller points

2. Timeouts don't add up. The assertoor_params.tests timeouts sum to 140 min (75 + 45 + 20), assuming assertoor runs the tests sequentially, which is its default as far as I know. wait-assertoor.sh gives up at 95 min (kurtosis_assertoor.yml, "Wait for the assertoor tests"), and the job limit is 120 min. A slow but healthy run would be reported as failed at 95 min. Either raise the wait deadline (and the job timeout-minutes) to cover the sum, or lower the per-test timeouts. The README says "about an hour", so the stability timeout may simply be generous. Keep the three numbers consistent either way.

3. Unauthenticated apt source. deb [trusted=yes] https://sdk.kurtosis.com/... skips signature verification for the CLI installed on the runner. Prefer the repo's signing key (signed-by=), or download a pinned release tarball and check its checksum.

4. Heredoc delimiter in $GITHUB_OUTPUT. The "Collect failed tests" step writes failed<<EOF with a fixed delimiter around test names from assertoor. A name containing EOF on its own line would truncate the output or inject extra outputs. The risk is low, because the names come from pinned playbooks, but a random delimiter is cheap.

5. Unused step id. id: tests on "Wait for the assertoor tests" is never referenced.

6. PR runs test :unstable, not the PR's code. The pull_request trigger only watches tooling/** and the workflow file, so this is consistent and documented. It does mean a PR check can pass or fail because of the unstable image. That is fine as a harness smoke test, but don't read it as validating the PR's code.

7. Possible empty failed list. If the assertoor job is cancelled or times out at the job level, needs.assertoor.outputs.failed can be empty. The notify job then posts "Failed: " with no names. A fallback like "see run" would cover it. When the image job fails, notify already substitutes a message.

Looks good

• Pinning the ethereum-package by full commit SHA, with a git rev-parse check after fetch.
• Passing values to the Slack payload through env and building the JSON with jq.
• permissions: contents: read.
• Counting a timed-out or missing run as a failure in wait-assertoor.sh.
• Always dumping and uploading the enclave logs.
• || true only on best-effort cleanup steps.

---

Automated review by Claude (Anthropic) · sonnet · custom prompt

[github-actions]

🤖 Kimi Code Review

I'll review this PR which adds a Kurtosis + Assertoor CI workflow for testing ethlambda in a multi-client devnet.

Overall Assessment

This is a well-structured CI workflow addition with good documentation. The shell scripting is generally careful, and the workflow design handles failure modes appropriately. I found several issues ranging from minor to moderate severity.

---

Critical / Security Issues

1. Potential Command Injection in yq Expression (.github/workflows/kurtosis_assertoor.yml:142-145)

yq -i '
  (.participants[] | select(.cl_type == "ethlambda") | .cl_image) = strenv(IMAGE) |
  (.participants[] | select(.vc_type == "ethlambda") | .vc_image) = strenv(IMAGE)
' "$RUNNER_TEMP/args.yaml"

Issue: The IMAGE environment variable is interpolated by the shell into strenv(IMAGE), but yq's strenv() reads from environment variables. However, if yq were ever changed to use string interpolation, or if the environment variable contains unexpected content, this could be problematic.

Mitigation: The current use of strenv(IMAGE) is actually correct and safe (it passes the env var name, not value, to yq). However, the workflow should validate the image reference format before use. Consider adding:

- name: Validate image reference
  run: |
    if [[ ! "$IMAGE" =~ ^[a-z0-9._/-]+(:[a-zA-Z0-9._-]+)?$ ]]; then
      echo "::error::Invalid image reference format: $IMAGE"
      exit 1
    fi

---

Correctness & Robustness Issues

2. Race Condition in Image Wait Loop (.github/workflows/kurtosis_assertoor.yml:97-108)

deadline=$((SECONDS + 75 * 60))
until docker manifest inspect "$IMAGE" > /dev/null 2>&1; do
  if [ "$SECONDS" -ge "$deadline" ]; then

Issue: SECONDS is a bash special variable that increments from shell start. If the Select image step takes significant time, the effective wait time is reduced. More critically, docker manifest inspect can fail for reasons other than "image not found" (registry auth issues, network problems), causing silent infinite retries until timeout.

Fix: Distinguish failure modes:

until response=$(docker manifest inspect "$IMAGE" 2>&1); do
  if [ "$SECONDS" -ge "$deadline" ]; then
    echo "::error::$IMAGE was not published within 75 minutes"
    exit 1
  fi
  # Fail fast on auth errors
  if echo "$response" | grep -qi "unauthorized\|denied"; then
    echo "::error::Registry authentication failed: $response"
    exit 1
  fi
  echo "Waiting for $IMAGE (${SECONDS}s elapsed)"
  sleep 60
done

3. Missing Validation of Pinned Commit (.github/workflows/kurtosis_assertoor.yml:127)

test "$(git rev-parse HEAD)" = "$ETHEREUM_PACKAGE_REF"

Issue: test with = does string comparison. If ETHEREUM_PACKAGE_REF is empty or modified, this fails silently in some contexts. More importantly, the git fetch --depth 1 could fetch a different ref if the remote is compromised or if there's a collision.

Fix: Use git verify-commit if the commit is signed, or at minimum:

actual_ref=$(git rev-parse HEAD)
if [ "$actual_ref" != "$ETHEREUM_PACKAGE_REF" ]; then
  echo "::error::Commit mismatch: expected $ETHEREUM_PACKAGE_REF, got $actual_ref"
  exit 1
fi

4. wait-assertoor.sh Timeout Logic Bug (tooling/kurtosis-assertoor/scripts/wait-assertoor.sh:44-48)

if [ "$SECONDS" -ge "$deadline" ]; then
    echo "::error::timed out after ${TIMEOUT_MIN} minutes waiting for assertoor tests"
    break
fi

Issue: After timeout break, the script continues to the result collection and may report success incorrectly. The break exits the while loop, but the script then proceeds to check if [ -s "$OUT/failed.txt" ]. If no runs ever appeared, failed.txt is empty (initialized with : > "$OUT/failed.txt"), and the check at line 62 only adds to failed.txt if total < EXPECTED.

Actually wait—let me re-trace: The total is computed from runs, which could be '[]' (empty array from initial value) if assertoor never responded. Then total=0, and 0 -lt EXPECTED is true, so "only 0 of N test runs appeared" is appended. This works, but it's fragile.

More serious issue: If assertoor responded with some runs but not all, and timeout hits, the active check is bypassed. The script then writes whatever runs it has to test_runs.json, fetches details, and checks for non-success statuses. Runs still "pending" or "running" will be counted as failures by the jq filter .status != "success". This is correct behavior.

However: The break on timeout doesn't set an error flag. The script relies on failed.txt being non-empty. But if runs is exactly EXPECTED count and all are still "running" at timeout, they'll be written to failed.txt as non-success. Good.

Edge case: If runs has EXPECTED count, all "success", but timeout hit after the break condition was actually met? No, the break condition is total >= EXPECTED && active == 0, so if we hit timeout, either total < EXPECTED or active > 0.

Verdict: The logic is actually correct, but fragile. Consider explicit timeout failure:

timed_out=0
# ... in loop:
if [ "$SECONDS" -ge "$deadline" ]; then
    echo "::error::timed out after ${TIMEOUT_MIN} minutes waiting for assertoor tests"
    timed_out=1
    break
fi
# ... at end:
if [ "$timed_out" -eq 1 ] || [ -s "$OUT/failed.txt" ]; then
    # ...

5. curl Failures Silently Ignored (tooling/kurtosis-assertoor/scripts/wait-assertoor.sh:56)

curl -sf --max-time 30 "$URL/api/v1/test_run/$id/details" > "$OUT/run-$id.json" || true

Issue: If curl fails (network blip, assertoor restarted), the run detail is missing but the script continues. This could mask important debugging information.

Fix: At minimum log the failure:

if ! curl -sf --max-time 30 "$URL/api/v1/test_run/$id/details" > "$OUT/run-$id.json"; then
    echo "Warning: failed to fetch details for run $id" >&2
fi

Or make it non-fatal but tracked.

---

Performance & Resource Issues

6. Aggressive Polling Interval (tooling/kurtosis-assertoor/scripts/wait-assertoor.sh:49)

sleep 30

Issue: 30-second polling against assertoor's API is reasonable, but during devnet startup, assertoor may take minutes to become ready. Consider exponential backoff or longer initial sleep to reduce log noise and API load.

Suggestion:

sleep_duration=30
# ... in loop, after successful fetch with no progress:
sleep "$sleep_duration"
sleep_duration=$((sleep_duration < 60 ? sleep_duration + 10 : 60))

7. Missing Resource Cleanup on Timeout (.github/workflows/kurtosis_assertoor.yml)

Issue: The assertoor job has timeout-minutes: 120, but GitHub Actions' job-level timeout kills the job without running always() steps if the entire job exceeds timeout. Actually, GitHub's documentation states that job-level timeouts do allow if: always() steps to run. However, if a step hangs (e.g., kurtosis run hangs), the step's own timeout isn't set.

Fix: Add step-level timeout to long-running steps:

- name: Start the devnet
  timeout-minutes: 30
  run: |
    kurtosis run ...

---

Rust / Code Quality (N/A for this PR)

This PR contains no Rust code. The shell scripts and YAML are generally well-written.

---

Documentation & Maintainability

8. Typo in README (tooling/kurtosis-assertoor/README.md:59)

Until ethlambda's `/eth/v1/config/spec` reports `GLOAS_FORK_EPOCH`...

Should be GLOAS_FORK_EPOCH → GLOAS_FORK_EPOCH is actually correct as the typo name (it's GLOAS not GLOAS?). Wait, let me re-read: The actual fork name in Ethereum specs is "GLOAS" (Gloas). The typo is that ethlambda omits it. Actually checking: Ethereum consensus specs have GLOAS_FORK_EPOCH? No, looking at current specs, there's no "GLOAS" fork. The next fork after Fulu is not yet named in mainnet. The README says "gloas stays unscheduled" in network_params.yaml.

Actually, I think GLOAS might be a typo for GLOAS or the actual placeholder name. Regardless, the documentation clearly explains the issue. Not a bug.

9. Hardcoded Fork Commit in Multiple Places

The commit bbae5eb7e7b5b938c625b3b2bb2167e997b2d0de appears in:

• .github/workflows/kurtosis_assertoor.yml:28
• tooling/kurtosis-assertoor/README.md:25
• tooling/kurtosis-assertoor/network_params.yaml:7

Issue: Risk of desynchronization. Consider a single source of truth or a CI check that validates consistency.

---

Positive Acknowledgments

1. Good use of set -euo pipefail in wait-assertoor.sh
2. Proper secret handling — Slack webhook uses secrets., not hardcoded
3. Concurrency control prevents redundant runs
4. if: always() on cleanup steps ensures enclave removal
5. Environment variable quoting in shell scripts is generally correct
6. The strenv(IMAGE) pattern in yq avoids injection vulnerabilities
7. Commit verification with git rev-parse HEAD comparison, even if could be stronger

---

Summary Table

Priority	Item	Location	Issue
Medium	2	Workflow line 97	Race condition / silent failures in image wait
Medium	3	Workflow line 127	Weak commit verification
Medium	4	Script line 44	Timeout logic fragile (functional but unclear)
Low	1	Workflow line 142	Image reference validation missing
Low	5	Script line 56	Silent curl failure for run details
Low	6	Script line 49	Could use backoff for polling
Low	7	Workflow	Missing step-level timeouts
Low	9	Multiple	Commit hash duplication

---

Automated review by Kimi (Moonshot AI) · kimi-k2.6 · custom prompt