Skip to content

ci: add a release pipeline with release candidates, binaries and promotion - #674

Open
MegaRedHand wants to merge 7 commits into
mainfrom
ci/release-pipeline
Open

MegaRedHand wants to merge 7 commits into
mainfrom
ci/release-pipeline

Conversation

@MegaRedHand

@MegaRedHand MegaRedHand commented Oct 7, 2026 •

Copy link
Copy Markdown
Collaborator

🗒️ Description / Motivation

ethlambda has no versioned releases: images are tagged by hand, there are no
binaries, and latest is moved manually. This adds a release pipeline modeled on
ethrex's:
cut a release candidate from a tag, test it, then promote it to the final version
without rebuilding, so what ships is what was tested. Binaries and images carry
GitHub build provenance attestations.

git push vX.Y.Z-rc.N ──► release.yaml ──► image X.Y.Z-rc.N + binaries + GitHub pre-release (with changelog)
                                          binaries + image attested
                                                  │ test on devnet
edit release: tag -> vX.Y.Z, untick pre-release   ▼
                         ──► release_promote.yaml ──► X.Y.Z + latest (ETHLAMBDA_CHANNEL=stable), attested

What Changed

File Change
.github/workflows/release.yaml (new) Main push: publishes unstable + sha-<7> (and -shadow twins), moved here from docker_publish.yaml. It also builds the binaries and the changelog without publishing them, so a broken release path fails on the commit that broke it. vX.Y.Z-rc.N tag: publishes and attests the X.Y.Z-rc.N image, builds and attests ethlambda-{linux-x86_64,linux-aarch64,macos-aarch64}, and opens a pre-release with SHA256SUMS and a conventional-commit changelog. Manual run: a dry run of an rc_tag input on the chosen ref, i.e. everything a push of that tag would do (version check, release notes), publishing nothing. The assets are kept as a workflow artifact.
.github/workflows/release_promote.yaml (new) One release edit (tag rename + pre-release untick) re-stamps the candidate as X.Y.Z + latest via FROM <rc> / ENV ETHLAMBDA_CHANNEL=stable, then attests the new image. Verifies that latest moved, that the image reports -stable-, and that its attestation verifies. A released tripwire fails if promotion never fired; a manual dispatch recovers.
.github/workflows/docker_publish.yaml Manual only, for devnetX. Refuses latest, unstable, sha-* and version tags, which the release workflows own.
bin/ethlambda/src/version.rs CLIENT_VERSION const → client_version(), which lets a non-empty ETHLAMBDA_CHANNEL replace the baked channel.
Dockerfile, bin/ethlambda/build.rs Pass a non-empty GIT_BRANCH to vergen as VERGEN_GIT_BRANCH (a tag checkout is detached and would report HEAD); add the org.opencontainers.image.version label.
RELEASE.md, README.md The release procedure, tags, binaries, version string, verifying a release, troubleshooting. The testing checklist is a TODO for now.

Not carried over from ethrex: L2/GPU builds and zkVM guests (no equivalent here), minisign (ethrex signs only its zkVM guest artifacts, which eth-act's guest handbook requires; nothing requires it here, and attestations need no key), crates.io (impossible while leanvm is a git dependency), and apt and Homebrew (future work; noted in RELEASE.md).

Correctness / Behavior Guarantees

  • Version string: unchanged unless ETHLAMBDA_CHANNEL is set and non-empty.
  • latest now means "latest release" rather than "current devnet"; devnetX tags stay manual.
  • Guards ethrex lacks: a tag whose X.Y.Z differs from [workspace.package].version fails the run, and promoting a candidate to a different version is refused.
  • Attestations are verified in the same run with gh attestation verify, before the pre-release is created or the promotion goes green.
  • Dry runs don't attest, since an attestation is public as soon as it is made.
  • Least privilege: only github-release holds contents: write. The OIDC and attestation permissions are granted only to the jobs that publish a release asset, not to the per-push image jobs. The third-party changelog action runs in a read-only job.
  • Release binaries are the candidate's, so they keep reporting rc.N after promotion (same as ethrex).

Tests Added / Run

  • version::tests: override replaces the channel; unset or empty keeps it.
  • Built with VERGEN_GIT_BRANCH=rc.7 and checked --version with ETHLAMBDA_CHANNEL unset / stable / empty → rc.7 / stable / rc.7.
  • actionlint on all workflows.
  • Extracted the workflows' shell steps and ran them on test inputs: main, branch, RC tag, wrong-version tag, final tag (refused), promotion version mismatch (refused), reserved manual tags (refused), previous-release lookup and changelog assembly.
  • Not run on GitHub yet. workflow_dispatch only works once the workflow is on main, so the first full exercise is after merge: a main push builds everything, and a manual run with rc_tag: v0.1.0-rc.1 dry-runs the release.

✅ Verification Checklist

  • Ran make fmt — clean
  • Ran make lint (clippy with -D warnings) — clean
  • Ran make test — only cargo test -p ethlambda --bin ethlambda (the Rust change is confined to the binary)

@github-actions

github-actions Bot commented Oct 7, 2026

Copy link
Copy Markdown

🤖 Codex Code Review

Found 2 actionable issues; the Rust-side version override itself looks sound and low risk for consensus logic.

  • .github/workflows/release.yaml:20
    on.push.tags: ["v*.*.*-*"] is broader than the validation logic in meta, which only accepts release-candidate tags matching ^vX.Y.Z-<suffix>$. This means any hyphenated tag like v1.2.3-test will trigger the workflow, fail late in meta, and create avoidable noisy failed release runs. Tighten the trigger to the intended candidate shape if possible, or add an early guard matching the exact supported format.

  • .github/workflows/docker_publish.yaml:35
    The reserved-tag check does not trim whitespace around comma-separated tags. An input like devnet5, latest leaves the second token as " latest", which bypasses ^(latest|unstable|sha-.*|...)$ and later publishes an invalid/undesired tag string. Since this workflow gates manual image publication, normalize each tag before validation and before manifest creation, e.g. strip leading/trailing spaces and reject empty elements. This is more correctness/supply-chain hygiene than consensus safety, but it affects artifact integrity.

Looks Good

  • bin/ethlambda/src/version.rs:30 uses LazyLock<String> cleanly; no memory-safety concerns, and resolving the channel once avoids repeated env lookups.
  • bin/ethlambda/build.rs:21 correctly adds rerun-if-env-changed=VERGEN_GIT_BRANCH, which is important for reproducible version metadata when tag builds override branch detection.
  • I did not see changes touching fork choice, attestation validation, justification/finalization, state transition, XMSS verification, or SSZ logic. From a consensus-layer perspective, this PR appears operational/release-focused rather than protocol-risky.

If you want, I can turn these into ready-to-post GitHub review comments.


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

@github-actions

github-actions Bot commented Oct 7, 2026

Copy link
Copy Markdown

🤖 Kimi Code Review

I'll review this PR focusing on the areas you specified. This appears to be a release infrastructure PR with workflow changes, Dockerfile updates, and version string handling. Let me analyze each component.

Summary

The PR implements a proper release pipeline with candidate → promotion flow, multi-arch Docker images, release binaries, and runtime channel override for version strings. The core consensus code changes are minimal (only version reporting).


Detailed Review

1. GitHub Workflows

release.yaml — Critical Issue: Regex allows invalid candidate tags

Line 63-68:

if ! [[ "$REF_NAME" =~ ^v[0-9]+\.[0-9]+\.[0-9]+-[0-9A-Za-z.-]+$ ]]; then

The regex [0-9A-Za-z.-]+ allows tags like v1.0.0- (hyphen only suffix) or v1.0.0-. which would produce empty or malformed channel names after ${tag#*-}. Consider tightening to require at least one alphanumeric before any dots/hyphens: [0-9A-Za-z]+[0-9A-Za-z.-]*.

Line 76-80:

cargo_version=$(sed -n '/^\[workspace\.package\]/,/^\[/s/^version = "\(.*\)"/\1/p' Cargo.toml)

This sed is fragile. If [workspace.package] isn't the first section starting with [, or if there are blank lines before the next section header, it could match across unintended sections. More robust:

cargo_version=$(awk '/^\[workspace\.package\]/{found=1} found && /^version *=/{gsub(/.*= *"|"/,""); print; exit}' Cargo.toml)

Line 182-184:

build-args: |
  GIT_COMMIT=${{ github.sha }}
  GIT_BRANCH=${{ needs.meta.outputs.channel }}

GIT_BRANCH receives channel (e.g., rc.1 or main), but the Dockerfile comment says this should be the rc.N suffix. For main builds, channel is main which is correct. However, for dispatch on non-main branches, channel is the branch name. The comment in Dockerfile at line 53 says "CI passes the tag's rc.N suffix here" — but for main/dispatch this passes main or branch name. This is actually correct behavior (branch name as channel), but the comment is slightly misleading.

Line 296:

fromTag: ${{ github.sha }}
toTag: ${{ steps.previous.outputs.base }}

The changelog action generates notes from base to sha, but base is the merge-base, not necessarily an ancestor tag. This produces correct commits but the direction might be confusing. Verify changelog-action handles this correctly (it should since fromTag/toTag are just commit refs).

release_promote.yaml — Security: TOCTOU in image existence check

Line 156-160:

if ! docker buildx imagetools inspect "$IMAGE:$RC_TAG" > /dev/null; then

This checks existence then proceeds to build. Between check and build, the image could be deleted or registry could fail. The docker buildx build --push below will fail anyway, but the error message will be less clear. Acceptable for this use case.

Line 170-174:

printf 'FROM %s\nENV ETHLAMBDA_CHANNEL=stable\n' "$IMAGE:$RC_TAG" > Dockerfile.promote
docker buildx build -f Dockerfile.promote \
  --platform linux/amd64,linux/arm64 \
  -t "$IMAGE:$VERSION" \
  -t "$IMAGE:latest" \
  --push .

Critical: The FROM uses $IMAGE:$RC_TAG which is a manifest list. docker buildx build with multi-platform will pull the correct arch. However, there's no RUN instruction, so this creates a new manifest list pointing to... actually, this re-wraps the image. The layers are reused but the config gets the env var. This is clever and correct for the stated goal.

However: Line 172 uses --platform linux/amd64,linux/arm64 but the Dockerfile has no RUN — this means BuildKit will create a manifest list with the same single-platform image referenced for both? No, actually since FROM resolves to a multi-platform manifest, BuildKit should handle this correctly. But I recommend verifying the resulting manifest's platform descriptors match expectations.

Line 219-228:

for attempt in $(seq 1 10); do
  ...
  sleep 30
done

Hard-coded 5 minute timeout with 30s sleep. If registry is under heavy load, this might not be enough. Consider exponential backoff or longer timeout. Not critical.

docker_publish.yaml — Good

The tag refusal logic is solid. The regex ^(latest|unstable|sha-.*|v?[0-9]+\.[0-9]+\.[0-9]+.*)$ correctly blocks release-pipeline tags. Note: v? allows bare 1.0.0 without v prefix, but your release workflow requires v prefix. Consistency issue: either block 1.0.0 here or allow it in release.yaml.


2. Rust Code

bin/ethlambda/src/version.rs — Design Issue: LazyLock for version string

Lines 28-47:

pub fn client_version() -> &'static str {
    static CLIENT_VERSION: LazyLock<String> = LazyLock::new(|| {
        let channel_override = std::env::var(CHANNEL_ENV).ok();
        let channel = resolve_channel(channel_override.as_deref(), BUILD_CHANNEL);
        format!(...)
    });
    &CLIENT_VERSION
}

This is correct and thread-safe. However, LazyLock::new(|| ...) in a function body creates a hidden static that executes on first call. The std::env::var read happens at first call time, not at program startup. This is fine for version reporting, but be aware that setenv after first call won't affect it.

Line 50-55:

fn resolve_channel<'a>(channel_override: Option<&'a str>, build_channel: &'a str) -> &'a str {
    channel_override
        .filter(|channel| !channel.is_empty())
        .unwrap_or(build_channel)
}

Good: empty string override is treated as unset. The lifetime elision is correct.

Line 58-73 (tests):
Tests are good but basic. Consider adding a test for whitespace-only override (" ") — currently this would be treated as a valid channel " ". Probably not intended, but also not harmful.

bin/ethlambda/build.rs — Minor Issue

Line 23:

println!("cargo:rerun-if-env-changed=VERGEN_GIT_BRANCH");

This is added, but vergen-git2 already emits its own rerun-if-changed directives for the .git directory. The env override is only read in version.rs, not in build.rs. This directive tells Cargo to rebuild when VERGEN_GIT_BRANCH changes, but since build.rs doesn't read it (vergen reads .git directly), this only affects the version.rs compile. Actually, this is correct: version.rs uses env!("VERGEN_GIT_BRANCH") which is set by vergen during build script execution. If VERGEN_GIT_BRANCH env var changes, we need build.rs to re-run so vergen re-executes with the new env.

Wait — vergen's Git2Builder doesn't read VERGEN_GIT_BRANCH from env, it reads from .git. The CI sets VERGEN_GIT_BRANCH in the environment, but vergen-git2 doesn't honor it. Looking at the Dockerfile:

ARG GIT_BRANCH=""
RUN if [ -n "$GIT_BRANCH" ]; then export VERGEN_GIT_BRANCH="$GIT_BRANCH"; fi; \
    cargo build ...

This exports VERGEN_GIT_BRANCH for the cargo build process. But vergen-git2 reads .git, not this env var. However, build.rs uses vergen's Git2Builder which might check this env var? Let me check... Actually, looking at vergen docs, VERGEN_GIT_BRANCH is an output of vergen, not an input. The build script sets it as a cargo instruction.

So how does the override work? The Dockerfile sets VERGEN_GIT_BRANCH as an environment variable during cargo build. build.rs runs, vergen emits cargo:rustc-env=VERGEN_GIT_BRANCH=... based on .git. But if .git says "HEAD" (detached checkout), then env!("VERGEN_GIT_BRANCH") would be "HEAD".

Actually, I think the intent is: the export VERGEN_GIT_BRANCH="$GIT_BRANCH" in Dockerfile sets the environment variable before build.rs runs. But vergen-git2 doesn't read this — it computes the value itself. The env!("VERGEN_GIT_BRANCH") in version.rs reads the value that vergen emitted via cargo:rustc-env, not the shell environment variable.

So does this actually work? Let me trace more carefully:

  1. Dockerfile: export VERGEN_GIT_BRANCH="rc.1" (shell env var)
  2. cargo build starts, runs build.rs
  3. build.rs: Git2Builder::default().add_instructions(&git2)? — this detects detached HEAD, would normally emit VERGEN_GIT_BRANCH=HEAD
  4. But wait — does vergen check if VERGEN_GIT_BRANCH is already set in environment? Looking at vergen source... I believe newer vergen versions have an emit_and_set or check for existing env vars.

Actually, re-reading the build.rs comment:

// Release builds set this to the `rc.N` suffix of their tag, since a tag
// checkout has no branch; vergen honors it but does not watch it.

This says "vergen honors it" — so vergen does check for VERGEN_GIT_BRANCH in env and uses it if present. The rerun-if-env-changed ensures that if this env var changes, build.rs re-runs. This seems correct if vergen has this behavior.

Recommendation: Verify the exact vergen version behavior. If vergen doesn't honor the env var, this entire mechanism fails silently and all tag builds report HEAD as channel.

crates/net/rpc/src/node.rs — Typo in comment

Line 178:

// The binary injects the real `client_version()` and local peer ID; here we

Should be client_version() — already fixed in this PR. ✓


3. Dockerfile

Line 52-57:

ARG GIT_BRANCH=""
RUN if [ -n "$GIT_BRANCH" ]; then export VERGEN_GIT_BRANCH="$GIT_BRANCH"; fi; \
    cargo build ...

This runs in a single RUN layer, so the export is visible to cargo build. Good.

However, the ARG GIT_BRANCH="" default means local builds without --build-arg GIT_BRANCH=... will have empty string, so vergen reads .git normally. Correct.

Line 71:

ARG VERSION=dev

Added but never used in a LABEL or env. Wait, line 78:

LABEL org.opencontainers.image.version=$VERSION

Good. But VERSION is not passed to the binary — the binary uses CARGO_PKG_VERSION. The comment says "Channel reported in the client version" but VERSION is only for OCI label. This is fine but the Dockerfile comment could clarify.


4. General Issues

Missing: ETHLAMBDA_CHANNEL runtime override not documented in --help

The ETHLAMBDA_CHANNEL env var affects version reporting but isn't mentioned in CLI help. Consider adding to command.rs or documenting.

Missing: Release binary checksum verification

SHA256SUMS is generated but no signature (GPG/minisign). For a consensus client, signed binaries are important. Out of scope for this PR but worth noting.

Concurrency concern in release.yaml

concurrency:
  group: ${{ github.workflow }}-${{ github.ref }}
  cancel-in-progress: false

cancel-in-progress: false means if two main pushes happen close together, both runs proceed. The second publish-docker job might overwrite unstable while the first is still running. Since images are immutable by digest, this is actually fine — the manifest just moves. But sha-XXXXXXX tags from both will exist. Acceptable.


Consensus-Layer Specific Assessment

This PR touches no consensus code. The changes are:

  • Version string formatting (no fork choice, attestation, justification, finalization, state transition, XMSS, or SSZ changes)
  • CI/CD infrastructure
  • Documentation

The version string change from const to LazyLock-based function is purely presentational and does not affect consensus validation.


Recommendations (Priority Order)

  1. Verify vergen env override behavior (build.rs + Dockerfile). If vergen doesn't read VERGEN_GIT_BRANCH from environment, tag builds will show HEAD channel. Test with a detached checkout locally.

  2. Tighten release tag regex in release.yaml line 63 to prevent v1.0.0- edge case: ^[0-9]+\.[0-9]+\.[0-9]+-[0-9A-Za-z]+[0-9A-Za-z.-]*$

  3. Fix sed fragility in release.yaml line 76 for Cargo.toml version extraction.

  4. Document ETHLAMBDA_CHANNEL in README or CLI help text.

  5. Consider signed checksums for release binaries (future work).


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

@github-actions

github-actions Bot commented Oct 7, 2026

Copy link
Copy Markdown

🤖 Claude Code Review

Review of PR 674: release candidate and promotion pipeline

I read the diff only and did not run any of the workflows. Overall the design is sound. Promoting by re-stamping the tested image rather than rebuilding it is a good choice. The ETHLAMBDA_CHANNEL override is small and unit-tested, and the version-string refactor is mechanical. The notes below are mostly robustness gaps.

Worth addressing

  1. The final tag's commit is only checked by hand (release_promote.yaml, resolve job; RELEASE.md step 5). The promoted image comes from the rc image, but the vX.Y.Z tag is created by the GitHub release edit with a UI-selected target. If the target is wrong (main instead of release/vX.Y.Z), the tag lands on untested code. The only guard is a manual git ls-remote comparison in the docs.

    • Suggestion: add a step in resolve or promote that fails if git rev-parse vX.Y.Z^{commit} differs from vX.Y.Z-rc.N^{commit}. It could use gh api or git ls-remote.
    • Optionally compare the rc image's org.opencontainers.image.revision label against the tag's commit.
    • This turns a "check the SHAs" instruction into an enforced invariant. It matters most for a release path that moves latest.
  2. A candidate can be promoted when its release run was only partly green (release.yaml, publish-docker vs build-binaries). publish-docker doesn't depend on build-binaries or changelog. If the binaries fail, the rc image still exists, but no pre-release is created, and the promote check (imagetools inspect) would pass anyway. The error text says "was the candidate's release run green?", but nothing checks it.

    • Suggestion: check that the pre-release has the expected assets, or document that promotion is only valid from a fully green run.
    • Today it is unlikely, since the edit itself requires an existing pre-release.
  3. The previous-release lookup sorts by tag creation date (release.yaml:267). git tag --sort=-creatordate can pick the wrong baseline if a patch release (v0.1.1) is cut after v0.2.0.

    • Use --sort=-v:refname to get the semver-highest final tag.
    • Also confirm that requarks/changelog-action accepts commit SHAs for fromTag/toTag. It is documented for tags, and I could not verify SHA support here.

Minor

  • Concurrency on promotion (release_promote.yaml): there is no concurrency group. Two promotions racing (an edited run plus a manual recovery dispatch) could both push latest. A group such as promote with cancel-in-progress: false would serialize them.
  • Floating third-party actions (release.yaml): requarks/changelog-action@v1 and the other actions are pinned to floating tags. Moving to SHA pins would be better for a pipeline that publishes latest and uses contents: write. You already keep the changelog action in a read-only job, which is good.
  • Leftover arch tags (release.yaml): X.Y.Z-rc.N-amd64 and -arm64 stay in GHCR. docker_publish's reserved-tag regex (v?[0-9]+\.[0-9]+\.[0-9]+.*) correctly blocks reusing them, so this is cosmetic.
  • Shadow images (release.yaml:99-105): the comment on -shadow images being unlocked and not reproducible is a good call-out. They are still published under unstable-shadow and sha-*-shadow, so keep them out of anything consumers treat as trusted.

Looks good

  • Untrusted values in the shell: REF_NAME, release tag names and inputs all go through env: and not inline ${{ }} in scripts, so there is no script-injection surface.
  • Changelog output: it is passed through GITHUB_OUTPUT with a random delimiter, which handles PR titles safely.
  • Version-string change: client_version() behaves identically when ETHLAMBDA_CHANNEL is unset or empty. resolve_channel has tests for both cases.
  • Dockerfile and build.rs: the GIT_BRANCH handling is sensible. cargo:rerun-if-env-changed=VERGEN_GIT_BRANCH covers the fact that vergen reads the variable but doesn't watch it.
  • Cargo version guard: the tag-vs-[workspace.package] check and the rc-to-final version check are useful additions.

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

@MegaRedHand
MegaRedHand marked this pull request as draft October 19, 2026 15:47
@MegaRedHand
MegaRedHand marked this pull request as ready for review October 19, 2026 18:20
@github-actions

Copy link
Copy Markdown

🤖 Kimi Code Review

I'll review this PR, which introduces a comprehensive release pipeline with Docker image promotion, binary releases, and attestation workflows. Let me analyze the changes across all files.

Summary

This PR implements a full release management system: automated builds for main and release candidates, Docker image multi-arch manifests, binary distribution, GitHub attestations, and promotion from candidate to final release without rebuilding. The implementation is generally well-structured with good security practices.


Detailed Findings

1. GitHub Workflows — release.yaml

Line 83-84: actions/checkout@v6 — This version does not exist. The latest stable is v4. This will cause workflow failures.

uses: actions/checkout@v6  # ERROR: v6 does not exist

Same issue appears at lines 134, 273, 322, 380.

Line 275: actions/upload-artifact@v7 — This version does not exist. Latest stable is v4.

Line 386: actions/download-artifact@v8 — This version does not exist. Latest stable is v4.

Line 450: actions/attest@v4 — Verify this exists; actions/attest-build-provenance@v1 was the earlier naming. The actions/attest action at v4 seems plausible but should be verified against GitHub's official documentation.

Line 104-105: The regex for candidate tags allows formats like v1.0.0-rc.1 but also v1.0.0-foo.bar.baz. The comment says vX.Y.Z-rc.N but the regex [0-9A-Za-z.-]+ is more permissive. This is intentional flexibility, but document if -alpha.1 or -beta.2 are also valid.

Line 293-294: cargo build --release --locked --bin ethlambda — The --locked flag requires Cargo.lock to be up to date. Since cargo update --workspace is run during release preparation (per RELEASE.md), this should succeed, but if dependencies are updated manually without refreshing the lockfile, this will fail.

Line 311-312: The binary is executed directly after build for version verification:

"dist/ethlambda-${{ matrix.target.name }}" --version

This is good practice, but on cross-compiled targets (e.g., building linux-aarch64 on ubuntu-22.04-arm), execution should work. However, if future targets include cross-compilation without execution capability, this pattern would break.

Line 330-331: fetch-depth: 0 for changelog generation — This fetches all history, which can be slow for large repositories. Consider if --filter=blob:none or partial clones would help, though git merge-base and tag listing do need full history.

Line 343: grep -E '^v[0-9]+\.[0-9]+\.[0-9]+$' — This finds only final releases, not candidates, which is correct per the comment. However, if a repository has lightweight tags vs. annotated tags, --sort=-creatordate behavior differs. Consider git for-each-ref --sort=-creatordate --format '%(refname:short)' refs/tags for more predictable ordering.

Line 360-361: The changelog-action uses fromTag: ${{ github.sha }} and toTag: ${{ steps.previous.outputs.base }}. This is reversed from typical usage (usually fromTag is the older tag). Verify this action's documentation — some tools expect chronological order.

Line 431: needs: [meta, build-binaries, publish-docker, attest-image, changelog] — The if condition on this job uses !cancelled() && !failure(), but publish-docker and attest-image are skipped (not successful) on dry runs. In GitHub Actions, needs with skipped jobs causes this job to be skipped unless if: always() or similar is used. The !failure() handles this correctly (skipped is not failure), but verify behavior.


2. GitHub Workflows — release_promote.yaml

Line 68-69: actions/checkout@v6 — Same version issue as above.

Line 77-79: The if condition for edited events:

if: >-
  github.event_name == 'workflow_dispatch' ||
  (github.event.action == 'edited' &&
   github.event.changes.tag_name.from != '' &&
   github.event.release.prerelease == false)

Security concern: github.event.changes.tag_name.from != '' checks that a change occurred, but github.event.release.prerelease == false checks the new state. The logic assumes that if prerelease is now false and the tag was renamed, it's a promotion. However, what if someone edits a release that was already not a pre-release, changing some other field? The tag_name.from check helps, but this could still trigger on non-promotion edits if the release was never a pre-release.

Consider also checking that github.event.changes.tag_name.from matches a candidate pattern and github.event.release.tag_name matches a final pattern.

Line 95-96: Regex validation allows 0.2.0-rc.1 but the rc prefix is not enforced — 0.2.0-foo.1 passes. Consistent with release.yaml, but ensure this matches your tagging policy.

Line 137-138: The Dockerfile.promote is created inline:

printf 'FROM %s\nENV ETHLAMBDA_CHANNEL=stable\n' "$IMAGE:$RC_TAG" > Dockerfile.promote

Security concern: No digest pinning here. If $IMAGE:$RC_TAG is a mutable tag (though candidates should be immutable), this could be subject to TOCTOU. Candidates use immutable tags, so this is acceptable, but consider documenting this assumption.

Line 140-144: The promotion build:

docker buildx build -f Dockerfile.promote \
  --platform linux/amd64,linux/arm64 \
  -t "$IMAGE:$VERSION" \
  -t "$IMAGE:latest" \
  --push .

This creates a new manifest list. The --push happens immediately. If the latest tag fails to update in the registry but the command reports success, the verify job catches it. Good defense in depth.

Line 206-207: Version string check:

if [[ "$reported" != *"/v$VERSION-stable-"* ]]; then

This is fragile — it assumes the version format never changes. If client_version() format changes, this breaks. Consider a more robust check or document this coupling.

Line 225-226: The assert-latest-promoted job polls for up to 5 minutes (10 attempts × 30 seconds). If registry eventual consistency exceeds this, it fails. This is reasonable but may need tuning based on observed behavior.


3. GitHub Workflows — docker_publish.yaml

Line 28-29: actions/checkout@v6 — Same version issue.

Line 47-48: Tag validation regex:

if [[ "$t" =~ ^(latest|unstable|sha-.*|v?[0-9]+\.[0-9]+\.[0-9]+.*)$ ]]; then

The v?[0-9]+... allows v1.0.0 and 1.0.0, but release.yaml only produces tags without v prefix. The regex is slightly more permissive than necessary. Also, sha-.* would match sha- with nothing after it; consider sha-.+ or sha-[a-f0-9]+.


4. Dockerfile

Line 52-56: The GIT_BRANCH handling:

ARG GIT_BRANCH=""

RUN if [ -n "$GIT_BRANCH" ]; then export VERGEN_GIT_BRANCH="$GIT_BRANCH"; fi; \
    cargo build --profile $BUILD_PROFILE ...

Issue: The ARG GIT_BRANCH is not re-declared in the build stage if this is a multi-stage build with a separate builder stage. Looking at the Dockerfile, this appears to be in the builder stage, but verify ARG propagation. ARG values are only available in the stage where they're declared.

Actually, looking more carefully: the ARG is declared at line 54, then used in the same stage's RUN. This is correct.

Issue: The export in RUN only affects that shell session. vergen reads environment variables at compile time, so this works if vergen is invoked during this cargo build. This is correct.

Line 74: ARG VERSION=dev — This is used for the OCI label but not passed to cargo build. The binary's version comes from CARGO_PKG_VERSION, not this VERSION. This is intentional (the binary version is Cargo's version), but ensure the label and binary version don't diverge unexpectedly.


5. bin/ethlambda/src/version.rs

Line 1: use std::sync::LazyLock; — Good, LazyLock is stable in Rust 1.80+.

Line 28-29: client_version() returns &'static str via LazyLock. This is correct and thread-safe.

Line 31-42: The LazyLock closure captures BUILD_CHANNEL (a const &str). This is fine — const values are effectively inlined.

Line 44-49: resolve_channel function:

fn resolve_channel<'a>(channel_override: Option<&'a str>, build_channel: &'a str) -> &'a str {
    channel_override
        .filter(|channel| !channel.is_empty())
        .unwrap_or(build_channel)
}

Correctness: Good handling of empty strings. The lifetime elision is correct.

Line 51-65: Tests are present and cover the key cases. Consider adding a test for whitespace-only override (" ") — currently it would return " " (not empty), which might be unintended. The filter(|channel| !channel.is_empty()) doesn't trim.

Potential issue: If ETHLAMBDA_CHANNEL=" stable " (with spaces), the version string becomes "v1.0.0- stable -abc123...". Consider trimming, or document that the variable must be clean.


6. bin/ethlambda/build.rs

Line 23-25: rerun-if-env-changed=VERGEN_GIT_BRANCH — Good. This ensures that changing the environment variable triggers a rebuild.

Missing: Should also emit rerun-if-changed=../Cargo.lock or similar for emit_crypto_revs() if it reads Cargo.lock. Not changed in this PR, so out of scope.


7. General Rust Code Changes

bin/ethlambda/src/main.rs and others: Changing version::CLIENT_VERSION to version::client_version() is correct. The function call is cheap (returns cached &'static str).

crates/net/rpc/src/node.rs: Test comment updated. The sentinel VERSION string in the test doesn't match the new format with channel resolution, but it's a test sentinel so that's fine.


8. Documentation

RELEASE.md: Comprehensive and well-written. The warning about single-edit promotion is critical and appropriately emphasized.

Line 165-166: git ls-remote origin refs/tags/vX.Y.Z refs/tags/vX.Y.Z-rc.N — Good verification step.

Line 220: cargo update --workspace — This updates all workspace crates. If there are path dependencies outside the workspace, they won't be updated. Ensure all ethlambda-* crates are in the workspace.


Critical Issues

Priority Issue Location
Blocking actions/checkout@v6 does not exist All workflow files
Blocking actions/upload-artifact@v7 does not exist release.yaml:275
Blocking actions/download-artifact@v8 does not exist release.yaml:386
High Verify actions/attest@v4 exists release.yaml:450, release_promote.yaml:176

Security Observations

  1. Attestation scope: The attest-image job only runs for candidates (candidate != ''), not for main builds. This is intentional — main builds are unstable and not attested as release artifacts. Good.

  2. OIDC permissions: The id-token: write permission is narrowly scoped to jobs that need it (attest-image, github-release, promote). Good principle of least privilege.

  3. Registry authentication: Uses secrets.GITHUB_TOKEN with packages: write scoped to specific jobs. Correct.

  4. Promotion integrity: The re-stamp build uses FROM $IMAGE:$RC_TAG without digest verification. Consider adding an integrity check:

    docker buildx imagetools inspect "$IMAGE:$RC_TAG" --format '{{.Manifest.Digest}}'

    and logging it, or even pinning the FROM with digest. However, since candidates are immutable tags, this is acceptable.

Recommendations

  1. Fix action versions immediately — this PR will break all workflows.

  2. Add whitespace trimming for ETHLAMBDA_CHANNEL:

    .map(|s| s.trim())
    .filter(|channel| !channel.is_empty())
  3. Consider stricter regex in docker_publish.yaml for version tags to match release.yaml exactly.

  4. Add timeout to release_promote.yaml polling loop to prevent infinite hangs if seq fails (defensive, though set -euo pipefail helps).

  5. Document the v prefix handling — release.yaml strips v from tags for image tags, but docker_publish.yaml allows v? prefix. Ensure consistency.


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

@github-actions

Copy link
Copy Markdown

🤖 Codex Code Review

Overall, the Rust changes look low-risk and do not touch consensus-critical paths like fork choice, attestation validation, justification/finalization, state transition, XMSS verification, or SSZ logic. I did not find a correctness or memory-safety bug in the code shown.

  • bin/ethlambda/src/version.rs:30 uses LazyLock<String> appropriately; returning &'static str via deref is fine, thread-safe, and avoids repeated allocation.
  • bin/ethlambda/src/version.rs:49 correctly treats ETHLAMBDA_CHANNEL= as unset, which avoids malformed version strings.
  • bin/ethlambda/src/main.rs:151, bin/ethlambda/src/main.rs:158, and bin/ethlambda/src/main.rs:163 consistently route all runtime version reporting through client_version(), so metrics, RPC, and logs stay aligned.
  • bin/ethlambda/src/command.rs:41 and bin/ethlambda/src/benchmark/report.rs:64 also use the same source of truth, which improves maintainability.

A couple of review notes:

  • bin/ethlambda/src/version.rs:12
    Security/operationally, ETHLAMBDA_CHANNEL is now fully trusted runtime input. That is probably intentional for release promotion, but it also means any operator can make the node identify itself as stable, main, or arbitrary text in logs/RPC/metrics. If peer-facing or observability consumers rely on this string for release provenance, consider constraining accepted override values (for example only stable) or documenting clearly that it is informational and unauthenticated.

  • bin/ethlambda/src/version.rs:31
    Because the value is cached on first use, any later mutation of ETHLAMBDA_CHANNEL in-process is ignored. That is probably fine, but it would be worth calling out explicitly in the doc comment since this behavior is subtle and test coverage currently only exercises resolve_channel(), not the one-time caching behavior.

If you want, I can also do a second pass focused just on the workflow changes in .github/workflows/release.yaml and docker_publish.yaml, since most of this PR appears to be release-pipeline logic rather than consensus code.


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

@github-actions

Copy link
Copy Markdown

🤖 Claude Code Review

Review of PR 674: release pipeline (RCs, binaries, promotion)

The design is sound. Promoting by re-stamping FROM <rc> keeps the tested layers, and the same-run gh attestation verify steps and the least-privilege permissions are good. The Rust change is small and correct: empty overrides are ignored, LazyLock resolves it once, and rerun-if-env-changed is present. I found no blocking bugs, only the points below.

Points worth addressing

  1. The final tag may not point at the tested commit (release_promote.yaml, release: edited). Renaming vX.Y.Z-rc.N to vX.Y.Z in the release UI creates a new tag. If vX.Y.Z doesn't already exist, GitHub points it at the release's target_commitish, which is usually the default branch HEAD and not the RC commit. The image is promoted from the RC digest, so it is fine. The vX.Y.Z git tag, and the source archive GitHub generates from it, can still differ from what was tested. Either have the procedure push vX.Y.Z to the RC's commit first, or add a check in resolve that git rev-parse vX.Y.Z^{commit} equals the RC tag's commit. The release only pushes -rc tags, so a vX.Y.Z tag doesn't re-trigger release.yaml, and that part is fine.

  2. latest can move backwards (release_promote.yaml, "Re-stamp"). Promoting a patch for an older line, such as 0.1.5 after 0.2.0 is out, overwrites latest. Either compare against the current latest version or only tag latest when the version is the highest release.

  3. The binaries are not the image's binary. build-binaries runs cargo build --release, while the Docker image uses $BUILD_PROFILE in the Dockerfile. The "what ships is what was tested" guarantee therefore covers the image only. If devnet testing is done on the image, the attached binaries are untested builds. Please state this in RELEASE.md, or align the profiles.

  4. The tag filter accepts any suffix. In release.yaml meta, ^v[0-9]+\.[0-9]+\.[0-9]+-[0-9A-Za-z.-]+$ also accepts v1.0.0-foo. The error message and RELEASE.md say -rc.N. If only RCs are intended, tighten it to -rc\.[0-9]+. Promotion has the same loose regex.

Minor

  • docker_publish.yaml check-tags: an empty or whitespace entry in the comma list passes the check and fails later in the build. Reject empty entries explicitly.
  • release_promote.yaml assert-latest-promoted: the released event fires for any pre-release to release transition, including a manual one that skips the rename. In that case the tripwire polls for 5 minutes and then fails. That is the intended signal, but the failure message could say that no promotion run exists, so people don't wait on a run that never started.
  • release.yaml image tags: extra_tags is keyed on REF_TYPE == branch. This is correct today, but it silently drops the sha- tag if the trigger set grows. Keying on candidate == '' would be more robust.
  • GIT_BRANCH=${{ needs.meta.outputs.channel }} reaches build-args through an expression. It is safe today because the tag regex and main bound the characters. If meta ever accepts other refs, route it through env like the other fields.

Not verified: the workflows haven't run on GitHub (as the PR notes). The first main push and a dry run with rc_tag should be watched closely after merge, especially the macos-latest and ubuntu-22.04-arm binary builds and the requarks/changelog-action inputs (fromTag/toTag given as SHAs).


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

…ntime

The release pipeline promotes a tested release candidate by re-stamping its
image instead of rebuilding it, so the binary that ships is the one that was
tested. That binary has its candidate channel (`rc.N`) baked in at build time;
reading the channel from `ETHLAMBDA_CHANNEL` when set lets the promoted image
report `stable` without touching the executable. An empty value is ignored, so
a blank `-e ETHLAMBDA_CHANNEL=` cannot drop the channel from the string.

`CLIENT_VERSION` becomes `client_version()`, built once on first use, since a
`concat!` constant cannot read the environment.
ethlambda shipped only hand-tagged Docker images, with no versioned releases,
no binaries, and `latest` moved by hand. This adopts ethrex's release flow:

- release.yaml: a `vX.Y.Z-rc.N` tag publishes the `X.Y.Z-rc.N` image, builds
  Linux (x86_64, aarch64) and macOS (aarch64) binaries, and opens a GitHub
  pre-release with them and a conventional-commit changelog. Pushes to main
  keep publishing `unstable` and `sha-<7>`, moved here from docker_publish.yaml
  so one workflow owns every automatic tag. A manual run rehearses the whole
  candidate path without publishing.
- release_promote.yaml: renaming the candidate's tag to `vX.Y.Z` and unticking
  pre-release, in one edit, re-publishes the tested images as `X.Y.Z` and
  `latest` with `ETHLAMBDA_CHANNEL=stable`, then checks `latest` moved and the
  image reports `-stable-`. A `released` tripwire fails if the promotion never
  fired; a manual run recovers it.
- docker_publish.yaml: manual only, for devnet tags. It refuses the tags the
  release workflows own, so two writers never race over `latest`.

Two guards ethrex lacks: a tag whose `X.Y.Z` differs from the Cargo version is
refused, as is promoting a candidate to a different version, since the binary
reports its Cargo version rather than the tag either way.

A tag checkout is detached, so vergen would report the branch as "HEAD". The
Dockerfile and the binary jobs now pass the channel to vergen through
VERGEN_GIT_BRANCH, which build.rs watches. RELEASE.md documents the procedure.
The binaries and the changelog only ran on a release-candidate tag or a
manual run, so a change that broke them surfaced on the next tag, long after
the commit that caused it. Main pushes now build both too, publishing neither:
the binaries stay workflow artifacts and the changelog goes to the run
summary.
The manual run built the chosen branch as a branch, so it never reached what
only a tag runs: the tag-versus-Cargo version check, the `rc.N` channel, and
the pre-release. Now that main pushes build the binaries, it rehearsed
nothing a push to main does not.

It now takes an `rc_tag` input and runs that tag's path on the chosen ref,
publishing nothing: images are built but not pushed, and the pre-release job
writes the notes and assets it would publish to the run summary and a
`dry-run-release-assets` artifact. Running it on `release/vX.Y.Z` before
tagging catches a bad version bump or a broken asset step without cutting a
candidate.
Release assets were published with only a SHA256SUMS beside them, which
proves integrity against the release page itself and nothing more. They
are now verifiable two independent ways:

- minisign, as ethrex does: every asset, SHA256SUMS included, gets a
  `.minisig` whose trusted comment names the asset, tag and commit. The
  script checks the secret key derives to the committed .github/minisign.pub
  and verifies each signature against it, so a key rotated in one place only
  fails the run instead of shipping signatures nobody can check. The
  canonical repository refuses to publish unsigned; a fork without a key
  publishes unsigned with a warning.
- GitHub build provenance attestations: the binaries and the candidate
  image on tag push, and the promoted image at promotion, since re-stamping
  it makes a new manifest the candidate's attestation does not cover. Each
  is verified with `gh attestation verify` before the run goes green.

A dry run signs with the real key (marked DRY-RUN in the trusted comment)
so a broken key or script fails before a tag does, but skips attesting,
since an attestation is public the moment it is made. The OIDC and
attestation permissions are granted only to the jobs that publish a release
asset, not to the per-push image jobs.

The key itself is not part of this change: RELEASE.md describes creating it
and setting the two secrets, and until that is done a candidate tag fails at
signing.
…ed channels

Step 4 says to test a candidate on the current devnet, but not which checks
or hosts that means, so each release would be tested differently. A TODO
marks where that checklist goes until it is written.

The new distribution section records why crates.io is out of reach (the
workspace depends on leanvm from git, which crates.io rejects) and that apt
and Homebrew are future work, so the question does not have to be
re-investigated each release.
minisign was added on the premise that it matched ethrex, but ethrex signs
only its zkVM guest ELFs and verification keys, because eth-act's zkEVM
guest handbook requires those to ship as signed assets. Its binaries and
images are unsigned. Nothing consumes a minisign signature of ethlambda's
binaries, and the build provenance attestations already let a user verify a
download, without a long-lived key to generate, store, rotate, and set up
before the first release candidate can be cut.

This branch has not been deployed

No deployments
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.

1 participant