ci(l1): make the daily reports fail loudly and attribute honestly

[ilitteri]

Motivation

The 2026-09-03 daily report looked like a regression on the head of main and led to a revert evaluation. It was not one: the same commit had passed the previous seven snapsync runs and a 100% P2P day, the P2P collapse came from an unpinned go-ethereum checkout inside hive's devp2p simulator, and 20k tests had silently dropped out of the totals. Two reporting defects made that harder to see than it needed to be.

Description

• Daily hive report: each simulator's artifact is downloaded into its own directory and checked against the workflow's own matrix. When one is missing, a warning is prepended to the report and the Slack blocks, and the job fails last so the report still goes out. Previously a missing artifact became a 0/0 line on a green job.
• Snapsync alerts: on scheduled runs the SHA is labelled as the head of main at run start (not "Commit:"), the commit's age is shown, and this workflow's earlier conclusions on the same commit are listed (success ×6, failure ×1). The history lookup needs actions: read on the token.

The geth pin for the devp2p simulator is handled separately: it needs a hive fork or an upstream change, since the simulator's Dockerfile clones geth master at build time.

Checklist

• No Store changes.

Testing

• The modified hive-report job ran on this branch (run 33769918540): the coverage check passed with every simulator present, and the flag/fail steps were skipped as intended. The missing-artifact path is exercised only by a missing artifact, so it is verified by reading.
• The two red checks are Hive - Devp2p tests and its aggregator. They fail identically on main: hive's devp2p simulator clones go-ethereum master at build time and geth moved on 2026-09-02. Pinning geth is a separate change (needs a hive repo we control).
• The snapsync workflow's run for this branch was displaced from the ethrex-sync-server concurrency queue by the scheduled run before it started, so the new alert text has not run in CI yet.

[github-actions]

⚠️ Known Issues — intentionally skipped tests

Source: docs/known_issues.md

rpc-compat log-bearing cases excluded

Where: KNOWN_EXCLUDED_TESTS in .github/scripts/check-hive-results.sh counts out
eight hive rpc-compat cases — the four eth_getLogs cases, eth_getBlockReceipts/get-block-receipts-latest,
and three eth_getTransactionReceipt cases. They are exactly the cases whose recorded
response contains at least one log object; every case with an empty log array still runs.
Note this leaves eth_getLogs with no rpc-compat coverage at all, since all four of its
cases are in the set.

Why: ethrex populates blockTimestamp on log objects, as geth, besu, nethermind, reth
and erigon all do. hive's rpc-compat compares responses byte-exactly (jsondiff.FullMatch;
the lenient checkJSONStructure path applies only to cases upstream marks speconly), and
the corpus is pinned to execution-apis d08382ae (2025-02-10), whose recordings predate the
field — it entered the schema in execution-apis#639 and the fixtures in #846 (2026-07-22).
So the extra key cannot match, and this is a property of the pin rather than of the response.

The pin cannot move, and this is not temporary. The pin sits one commit before
execution-apis#627, which moved the test chain to a pre-merge genesis: the current corpus has
~36 proof-of-work blocks before its terminal total difficulty. ethrex does not support
pre-merge chains and will not, so importing that chain.rlp fails at block 1 —
validate_block_header has no pre-London base-fee path. Every revision carrying
blockTimestamp in its fixtures also carries that chain, so there is no revision that
satisfies both. Nor can the corpus be patched locally: rpc-compat's Dockerfile clones
ethereum/execution-apis by hard-coded URL, so the branch buildarg cannot point at a fork.

Coverage: the field itself is pinned by
block_timestamp_is_on_the_log_and_not_on_the_receipt in
crates/networking/rpc/types/receipt.rs, which asserts it is present on each log and absent
from the receipt level.

Removal: delete the entries if ethrex ever gains pre-merge chain import, or if upstream
marks these cases speconly so they are type-checked instead of compared byte-for-byte.

---

The stateless schema id does not identify the encoding

Where: STATELESS_INPUT_SCHEMA_ID in crates/common/types/stateless_ssz.rs.

Upstream keeps the stateless input schema id at 0x1501
(fork_index 0x15 << 8 | revision 0x01) across incompatible body changes. Three
encodings have now shipped under it: tests-zkevm@v0.6.2, then #3248 + #3278,
then #3356, which moved state, codes and public_keys from SszList to
ProgressiveList. ethrex speaks the last one.

The consequence is that the 2-byte prefix cannot be used to detect a stale or
mismatched bundle. A wrong-dialect input is accepted by the id check and then
fails later — in SSZ decode, or on a root that does not match — rather than being
rejected up front for what it is. only_amsterdam_schema_id_decodes therefore
proves less than its name suggests.

Worth raising upstream: a revision field that does not move across a body change
provides no version negotiation at all.

---

ZisK guest program hash changes with the unsync_cell gate

Where: crates/common/types/block.rs, transaction.rs.

The gate on the single-threaded unsync_cell::OnceCell moved from
all(feature = "eip-8025", target_arch = "riscv64") to
all(feature = "zisk", target_arch = "riscv64") when the eip-8025 feature was removed.

The guest ELFs were previously built --features "<zkvm>-build-elf,ci", which never enabled
eip-8025, so they compiled the atomic once_cell variant. bin/zisk/Cargo.toml does enable
ethrex-common/zisk, so the ZisK guest now compiles the unsafe impl Sync cell instead.
That changes the ELF bytes and therefore the program hash and verification key.

This is intended (the guest is single-threaded, so the unsync cell is sound and cheaper), but it
is a VK change rather than a no-op refactor, and the diffstat presents it as a file rename
(eip8025_cell.rs → unsync_cell.rs). Anyone pinning a ZisK VK across this change must
re-register it. The stateless-validator crate now forwards ethrex-common/zisk from its own
zisk feature so the two ZisK guests do not disagree on the cell type.

---

Release signing key is an unprotected repository secret

Where: .github/workflows/tag_release.yaml.

MINISIGN_SECRET_KEY is a plain repository secret. There is no environment: on
finalize-release or dry-run-release-assets, and gh api repos/lambdaclass/ethrex/rulesets
shows only branch-targeted rulesets, so the github.ref_type == 'tag' condition is a workflow
check rather than an enforced boundary: anyone who can push a tag can reach the signing key.

This is a repository-settings change, not a code change, so it is recorded here rather than
fixed in the tree. Recommended:

1. Move MINISIGN_SECRET_KEY / MINISIGN_PASSWORD into a GitHub Environment with required
   reviewers, and add environment: to the two jobs that sign.
2. Add a ruleset targeting refs/tags/v* restricting who may create release tags.

Until then, the compromise of that key is silent and durable: signatures would still verify
against the committed .github/minisign.pub.

[github-actions]

🤖 Kimi Code Review

I'll review this PR which contains GitHub workflow and shell script changes for CI/CD notifications and Hive test reporting.

File: .github/scripts/notify_snapsync_run.sh

Line 95-100: Commit age calculation has integer overflow risk on 32-bit systems

COMMIT_TS=$(git log -1 --format=%ct "$HEAD_SHA" 2>/dev/null || true)
if [[ "$COMMIT_TS" =~ ^[0-9]+$ ]] && (( EPOCHSECONDS >= COMMIT_TS )); then
  AGE_SECS=$((EPOCHSECONDS - COMMIT_TS))

The EPOCHSECONDS variable is a bash 5.0+ feature. If this script runs on systems with older bash (macOS default is 3.2), it will fail. Consider adding a fallback:

COMMIT_TS=$(git log -1 --format=%ct "$HEAD_SHA" 2>/dev/null || true)
if [[ "$COMMIT_TS" =~ ^[0-9]+$ ]]; then
  NOW=${EPOCHSECONDS:-$(date +%s)}
  if (( NOW >= COMMIT_TS )); then
    AGE_SECS=$((NOW - COMMIT_TS))

Line 108-112: HISTORY_LINE construction has JSON injection risk via prior

HISTORY_LINE="*Earlier runs on this commit:* ${prior:-none}"

The prior variable comes from jq output, but jq with join(", ") could theoretically contain newlines or control characters if GitHub API returns malformed data. While slack_escape isn't applied here, this is later placed in a jq payload. The outer jq construction at line 130+ should handle this, but verify no double-escaping issues.

Actually, looking more carefully: prior is inserted into DETAILS which becomes a jq --arg, so it's properly escaped. This is fine.

Line 102-106: Division by zero not possible, but verify 86400 constant

The age calculation:

COMMIT_AGE=" (committed $((AGE_SECS / 86400))d $(((AGE_SECS % 86400) / 3600))h ago)"

This is correct. No division by zero risk since AGE_SECS >= 0.

Line 22-23: Variable defaults could mask unset required variables

EVENT_NAME=${EVENT_NAME:-}
GH_TOKEN=${GH_TOKEN:-}

These are fine as optional variables with documented fallbacks.

File: .github/workflows/daily_hive_report.yaml

Line 227: merge-multiple: true removal changes behavior

        with:
          path: hive/workspace/logs
          pattern: "*_daily-results.zip"
-         merge-multiple: true

The removal of merge-multiple: true is intentional and correct per the comment, but verify the actions/download-artifact v4 behavior: without merge-multiple, multiple matching artifacts create subdirectories. This is the desired behavior for the subsequent check.

Line 236-246: Simulator detection regex is fragile

          for sim in $(grep -oE 'file_name: [a-z0-9-]+' .github/workflows/daily_hive_report.yaml | awk '{print $2}' | sort -u); do

Bug risk: This regex [a-z0-9-]+ doesn't match underscores. If any simulator name contains underscores, they'll be missed. Also, grep on the workflow file itself is fragile—if the YAML format changes (e.g., file_name:[space][space]value), it breaks.

Suggestion: Define the expected simulators explicitly or use a more robust extraction. At minimum, allow underscores:

grep -oE 'file_name:\s*[a-zA-Z0-9_-]+'

Wait—looking at the actual matrix in this workflow, the simulators appear to use hyphens. But engine-api or similar might use other characters. The regex also doesn't allow uppercase, which could be an issue.

Line 242: compgen -G glob pattern has subtle bug

if ! compgen -G "hive/workspace/logs/${sim}_daily-results.zip/*.json" > /dev/null; then

The artifact download creates directories named after the artifact, not .zip files. The directory would be hive/workspace/logs/consume-engine-rest_daily-results.zip/ (a directory, despite the .zip suffix from the artifact name). The glob checks for .json inside that directory.

However, compgen -G returns non-zero if no match, but also if the directory doesn't exist. This seems correct for detecting missing artifacts.

Line 250: shopt -s nullglob without restoring previous state

          shopt -s nullglob

This changes shell options for the remainder of the step. While not harmful in a GitHub Actions step that exits, it's poor practice. Consider:

          shopt -s nullglob
          # ... commands ...
          shopt -u nullglob  # restore

Or use a subshell: (shopt -s nullglob; ...) — but that complicates variable capture.

Line 252-258: mv without error handling for duplicate basenames

          for f in hive/workspace/logs/*/*.json hive/workspace/logs/*/*.log; do
            mv "$f" "hive/workspace/logs/$(basename "$f")"
          done

Bug: If two different subdirectories contain files with the same basename, the second mv will silently overwrite the first. The previous globstar approach had the same issue, but this is worth noting.

Suggestion: Add collision detection:

          for f in hive/workspace/logs/*/*.json hive/workspace/logs/*/*.log; do
            target="hive/workspace/logs/$(basename "$f")"
            if [[ -e "$target" ]]; then
              echo "::warning::Collision: $target already exists, skipping $f"
              continue
            fi
            mv "$f" "$target"
          done

Line 285-290: jq mutation of Slack blocks assumes array structure

          jq --arg t "$note" '.blocks |= ([.[0], {type: "section", text: {type: "mrkdwn", text: $t}}] + .[1:])' \
            hive_slack_blocks.json > blocks.tmp && mv blocks.tmp hive_slack_blocks.json

This assumes .blocks is a non-empty array. If hive_slack_blocks.json has an unexpected structure, .[0] and .[1:] will fail or produce bad results. Given this is generated by a known tool (hive_report), the risk is low.

Line 308-310: Job failure after artifact upload

      - name: Fail when a simulator did not report
        if: steps.coverage.outputs.missing != ''
        run: exit 1

Good pattern—artifacts and reports are uploaded before failing. However, the hive-report job will be marked failed, and post-daily-report has needs: [hive-report] with if: ${{ always() }}.

Potential issue: always() causes the job to run even if hive-report is skipped (e.g., due to earlier job failure). This might post partial reports. Consider if: ${{ !cancelled() && (needs.hive-report.result == 'success' || needs.hive-report.result == 'failure') }} to avoid running on cancellation or earlier dependency failures.

Actually, always() is likely intentional to ensure reports go out. The if: ${{ always() }} is fine for this use case but will run even if the workflow is cancelled.

File: .github/workflows/daily_snapsync.yaml

Line 24: actions: read permission addition

permissions:
  contents: read
+  actions: read # notify_snapsync_run.sh lists earlier runs on the same commit

Correct and minimal permission for the GH_TOKEN usage. Good security practice.

Lines 134-135, 188-189: Token exposure

+          EVENT_NAME: ${{ github.event_name }}
+          GH_TOKEN: ${{ github.token }}

The github.token is automatically masked in logs. This is safe.

Summary

Priority	Item	Location
Medium	Bash 5.0 EPOCHSECONDS compatibility	notify_snapsync_run.sh:99
Medium	Fragile regex for simulator names	daily_hive_report.yaml:238
Medium	Silent file collision on mv	daily_hive_report.yaml:254
Low	nullglob not restored	daily_hive_report.yaml:250
Low	always() vs !cancelled() consideration	daily_hive_report.yaml:316

The changes are generally well-structured with good comments explaining the rationale. The commit age labeling and run history features are useful for distinguishing flaky failures from regressions.

---

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

[github-actions]

🤖 Claude Code Review

Review: PR 7252 — "make the daily reports fail loudly and attribute honestly"

This is a CI/tooling-only change (no Rust/EVM/consensus code), so the usual EVM/consensus checklist doesn't apply. The scripts are written defensively (set -euo pipefail, guarded || true around subshells that could fail, nullglob), and the fixes address real, well-diagnosed problems. A few things worth double-checking before/after merge:

1. Flatten step narrowed from recursive to fixed depth-1 (.github/workflows/daily_hive_report.yaml:249-259)

The old step used shopt -s globstar and matched **/*.json at any depth; the new one matches exactly hive/workspace/logs/*/*.json (one level under the per-artifact directory). If any simulator's zip ever nests its result JSON more than one directory deep, those files will silently not be picked up by the flatten step — the same class of bug this PR is fixing, just relocated.

This is largely self-correcting: the new coverage check (added just above, lines 233-247) uses the same one-level assumption, so if it's wrong, every simulator gets flagged as missing on the very first run rather than failing silently. Worth a one-line comment confirming the artifact's internal layout is verified to be exactly one level deep, since that assumption is now load-bearing in two places.

2. Coverage/history checks operate at workflow-run granularity, not job granularity

• daily_hive_report.yaml line 239: the expected-simulator set is scraped by grepping file_name: [a-z0-9-]+ out of the workflow YAML itself. It works correctly today (and cleverly avoids self-matching its own grep line, since [ isn't in the [a-z0-9-] class), but it's fragile to reformatting (e.g., switching that matrix entry to block style, or renaming the key) with no test that would catch drift other than the check going silent.
• notify_snapsync_run.sh:125-133: the "earlier runs on this commit" history is queried by matching .name == $GITHUB_WORKFLOW, i.e. at the whole-workflow-run level. daily_snapsync.yaml has two separate jobs (sync-lighthouse, sync-prysm) posting independent Slack alerts under the same workflow name. A failure alert for lighthouse-hoodi will show history built from the overall run's conclusion, which can't distinguish "lighthouse failed but prysm passed" from "prysm failed but lighthouse passed" on a prior run. Not wrong, just coarser than the per-job story the alert is about — worth being aware of when reading "success ×6, failure ×1" next to a specific client's alert.

3. Minor: block-insertion assumes hive_slack_blocks.json has ≥1 block

daily_hive_report.yaml:291: [.[0], {...}] + .[1:] — if .blocks were ever empty, .[0] evaluates to null, producing an invalid Slack block that would make the whole payload rejected by Slack's API (silently breaking delivery of the very warning this step exists to surface). Very unlikely given hive_report presumably always emits a header block, but cheap to guard against, e.g. .blocks = [.blocks[0]] + [{...}] + .blocks[1:] behaves the same but is more explicit that it depends on a non-empty array, or add a fallback when .blocks | length == 0.

Correctly handled (worth calling out since they're easy to get wrong)

• post-daily-report job gets if: ${{ always() }} added (line 317) — necessary and correct, since without it, the new deliberate failure in hive-report (added by this PR) would silently stop the Slack report from ever being posted, undoing the PR's own goal.
• Ordering in hive-report: artifact upload happens before the "Fail when a simulator did not report" step, so the report/artifact still ships even though the job goes red.
• actions: read is scoped precisely where needed (daily_snapsync.yaml:24) and the token is only used for a read-only API call — no privilege escalation.
• notify_snapsync_run.sh guards against clock skew (EPOCHSECONDS >= COMMIT_TS) before computing age, and wraps the whole curl|jq history lookup in || true so a transient GitHub API failure can't crash the notifier under set -e.

Nit

• notify_snapsync_run.sh:132: if the GitHub API call itself fails (bad token, rate limit, network error) rather than just returning zero prior runs, prior ends up empty and the line prints *Earlier runs on this commit:* none — indistinguishable from "genuinely no history." Not worth extra complexity for a Slack-only nicety, but if this ever needs debugging, that's the first place to look.

Overall: solid, well-scoped fix for a real reporting gap, with good comments explaining the why (especially the 2026-09-03 incident context). Nothing here blocks merging; the flatten-depth assumption (Item 1) is the only one I'd actually confirm against real artifact contents before trusting the coverage check long-term.

---

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

[github-actions]

🤖 Codex Code Review

⚠️ The review job did not complete (result: cancelled). See the job log

---

Automated review by OpenAI Codex