Skip to content

chore: add preview-config.nix file#158

Merged
pablodeymo merged 26 commits intomainfrom
tekton-integration
Mar 2, 2026
Merged

chore: add preview-config.nix file#158
pablodeymo merged 26 commits intomainfrom
tekton-integration

Conversation

@MegaRedHand
Copy link
Collaborator

@MegaRedHand MegaRedHand commented Feb 27, 2026

This PR adds a preview-config.nix file for tekton previews. It starts a local devnet with 4 ethlambda nodes using lean-quickstart.

@MegaRedHand MegaRedHand marked this pull request as ready for review February 27, 2026 13:35
@github-actions
Copy link

🤖 Kimi Code Review

Review of preview-config.nix

1. Security Issues

  • Line 302: Root password is hardcoded as "changeme" – this is a critical security vulnerability. Should use a generated secret or disable password auth entirely.
  • Line 300: PermitRootLogin = "yes" combined with the above makes the container trivially compromisable. Recommend PermitRootLogin = "prohibit-password" or better yet, disable SSH entirely for production.

2. Resource & Performance

  • Line 141: TimeoutStartSec = "900" is reasonable for Rust builds, but consider making this configurable via environment variable for slower CI runners.
  • Line 84: All nodes share the same RocksDB data directory structure (${dataDir}/ethlambda_$i). This is good isolation, but ensure RocksDB's background compaction won't interfere across nodes on the same disk.

3. Error Handling & Robustness

  • Lines 149-153: The script checks for binary existence but doesn't verify it's executable. Add chmod +x check or use test -x.
  • Lines 167-172: The genesis verification loop could be more specific – check for non-empty files with test -s instead of just existence.
  • Line 144: The nix build command doesn't specify --print-build-logs which would help debug build failures in CI.

4. Code Quality & Maintainability

  • Line 45: The meta attribute set is well-structured, but consider moving the route definitions to a separate file for easier maintenance.
  • Lines 210-250: The validator private keys are hardcoded in plaintext. While acceptable for devnet, add a prominent comment warning against production use.
  • Line 67: The mkNodeService function is clean and reusable – good abstraction.

5. Nix-specific Improvements

  • Line 318: system.stateVersion = "24.11" should probably be "24.05" unless you're intentionally targeting an unreleased version.
  • Line 271: Consider using lib.mkForce for the Docker enable to make it clearer this overrides host defaults.

6. Minor Issues

  • Line 186: The comment mentions ethlambda_$i but the actual directory uses underscores (ethlambda_0, etc.). Consistency check needed.
  • Line 93: The path attribute in the node services includes bash and coreutils but these are already in the default systemd path.

Recommendations Summary:

  1. Immediate fixes:

    • Remove hardcoded root password
    • Disable root SSH login
    • Add executable check for ethlambda binary
  2. Improvements:

    • Make build timeout configurable
    • Add --print-build-logs to nix build
    • Add validation for non-empty genesis files
  3. Documentation:

    • Add warning comments about hardcoded devnet keys
    • Document the port allocation scheme (9001-9004, 8081-8084)

The configuration is well-structured for a devnet setup. The main concerns are the security issues which should be addressed before any testnet deployment.


Automated review by Kimi (Moonshot AI) · custom prompt

@github-actions
Copy link

🤖 Kimi Code Review

Review of preview-config.nix

Security Issues

  1. Root SSH with weak password (lines 286-292)

    • services.openssh.settings.PermitRootLogin = "yes" combined with users.users.root.password = "changeme" is a critical security vulnerability. This allows password-based root login with a predictable password.
    • Fix: Remove root password and use key-based auth only, or disable root SSH entirely.
  2. Docker socket access (lines 275-280)

    • The preview user is added to the docker group, giving it root-equivalent privileges through the Docker socket. This is acceptable for a devnet but should be documented as a security consideration.
  3. Token exposure in git URL (lines 208-210)

    • The preview token is embedded in the git URL which could appear in process listings. Consider using git credential helper or environment variables.

Correctness & Reliability

  1. Race condition in setup script (lines 176-186)

    • The check for existing setup uses multiple separate file checks that aren't atomic. If the container crashes during setup, it could leave the system in an inconsistent state.
    • Fix: Use a single atomic marker file or implement proper state tracking.
  2. Missing validation of genesis generation (lines 248-254)

    • The script only checks for file existence but doesn't validate the content integrity (checksums, format validity).
    • Consider: Add validation of genesis file contents or at least basic format checks.
  3. Hard-coded validator private keys (lines 134-167)

    • Private keys are hard-coded in the Nix configuration, which is acceptable for a devnet but should be clearly marked as such.

Performance & Resource Management

  1. No resource limits (node services)

    • The systemd services for ethlambda nodes don't specify memory or CPU limits. In a containerized environment, this could lead to resource exhaustion.
    • Add: MemoryMax, CPUQuota, or cgroup limits to each service.
  2. Timeout too short for initial build (line 173)

    • TimeoutStartSec = "900" (15 minutes) may be insufficient for initial builds with cold caches. Consider increasing to 30-45 minutes for first-time builds.

Code Quality & Maintainability

  1. Missing error handling in genesis generation (lines 243-254)

    • The genesis generation script runs without checking exit codes or capturing stderr.
    • Add: set -e and proper error handling around the genesis generation.
  2. Redundant git fetch (lines 195-200)

    • When updating an existing repo, the script does a full fetch then reset. Consider using git pull --ff-only or checking if the branch is already at the desired commit.
  3. Inconsistent naming (lines 88-89 vs 94-95)

    • Service names use hyphens (ethlambda-0) but directory names use underscores (ethlambda_0). This inconsistency could cause confusion.
  4. Missing cleanup for failed builds (lines 215-220)

    • If the build fails, the binary check will prevent retry on next start. Consider cleaning up failed builds or implementing proper retry logic.

Minor Issues

  1. Outdated system state version (line 304)

    • system.stateVersion = "24.11" may be too new for some systems. Ensure this matches your target NixOS version.
  2. Missing health checks (node services)

    • Consider adding ExecStartPost or ExecReload health checks for each node service.
  3. No log rotation

    • The services will accumulate logs indefinitely. Consider adding logrotate configuration or systemd journal size limits.

Recommendations Summary

  • Critical: Fix root SSH security issue
  • High: Add resource limits to services
  • Medium: Improve error handling and validation
  • Low: Address naming inconsistencies and add health checks

Automated review by Kimi (Moonshot AI) · custom prompt

@github-actions
Copy link

🤖 Claude Code Review

PR #158 Review: preview-config.nix (Tekton Devnet Preview)

This PR adds a NixOS container configuration for spinning up a 4-node ethlambda-only devnet via tekton previews. The overall structure is solid and the devnet topology is well-documented. A few security and reliability issues warrant attention.


Security

Root SSH with static password (lines 308–315)

services.openssh = {
  enable = true;
  settings.PermitRootLogin = "yes";
};
users.users.root = {
  password = "changeme";
};

This combines PermitRootLogin = "yes" with a well-known static password and no PasswordAuthentication = "no". Even in a preview environment, this creates a trivially exploitable entry point if the container is reachable. Either disable password auth and use key-based access, or restrict PermitRootLogin to "prohibit-password".

GitHub token embedded in git URL (lines 175–179)

PREVIEW_TOKEN=$(cat /etc/preview-token 2>/dev/null || echo "")
AUTHED_URL=$(echo "$PREVIEW_REPO_URL" | sed "s|https://|https://x-access-token:$PREVIEW_TOKEN@|")
${pkgs.git}/bin/git -C "${appDir}" remote set-url origin "$AUTHED_URL"

The token ends up in the remote URL, which git stores in .git/config and may appear in ps aux output or systemd journal logs if git prints the URL on error. Prefer using GIT_ASKPASS or a credential helper instead:

GIT_ASKPASS_SCRIPT=$(mktemp)
echo -e "#!/bin/sh\necho \"$PREVIEW_TOKEN\"" > "$GIT_ASKPASS_SCRIPT"
chmod +x "$GIT_ASKPASS_SCRIPT"
GIT_ASKPASS="$GIT_ASKPASS_SCRIPT" git clone "https://x-access-token@${PREVIEW_REPO_URL#https://}" ...

Or set GITHUB_TOKEN and use a credential store that never serialises to disk.

Hardcoded secp256k1 P2P private keys (lines 211–247)

The four node private keys are committed to the repository. These are P2P identity keys, not validator signing keys, so the consensus security impact is limited — but anyone with repo access can impersonate these nodes on the network. Worth a comment clarifying they are dev-only and ephemeral.


Reliability

Unpinned lean-quickstart clone (lines 256–259)

${pkgs.git}/bin/git clone --depth 1 --single-branch \
  https://github.com/blockblaz/lean-quickstart.git "${quickstartDir}"

No commit hash is pinned. An upstream change to generate-genesis.sh or the genesis tooling will silently affect all future previews. Add a --branch <tag> or post-clone git checkout <commit> to pin to a known-good revision.

No cleanup of stale genesis on force-rebuild (lines 165–172)

When /tmp/force-rebuild exists, the setup proceeds but the existing ${genesisDir} is not cleared first. If a previous genesis run partially succeeded, stale files may pass the [ -f ... ] guards and generate-genesis.sh may produce unexpected results. Add rm -rf "${genesisDir}" at the top of the force-rebuild path.

source /etc/preview.env called twice (lines 144 and 157)

The second source /etc/preview.env after sourcing $SECRETS_FILE is redundant. Environment variables from preview.env are already active. Minor noise but can be removed.


Questionable Abstraction

SECRETS_FILE only writes a constant (lines 145–153)

if [ ! -f "$SECRETS_FILE" ]; then
  echo "Generating preview secrets..."
  {
    echo "RUST_LOG=info"
  } > "$SECRETS_FILE"
  chmod 600 "$SECRETS_FILE"
fi

The comment says "Generate stable per-preview secrets on first run" but the file only ever contains RUST_LOG=info, which is not a secret and could be set elsewhere. Either remove this file and set RUST_LOG directly in the service environment, or use it to actually generate and persist non-deterministic values (e.g., a random network identifier). As written it adds complexity without benefit.


Minor / Nit

DNS configuration may conflict (lines 100–105)

services.resolved = {
  enable = true;
  settings.Resolve.FallbackDNS = [ "8.8.8.8" "1.1.1.1" ];
};
networking.nameservers = [ "8.8.8.8" "1.1.1.1" ];

With systemd-resolved enabled, /etc/resolv.conf should point to 127.0.0.53. Setting networking.nameservers to external IPs overrides this and can produce conflicting resolver configurations. With resolved enabled, drop networking.nameservers and use resolved's DNS or FallbackDNS settings exclusively.


What Looks Good

  • set -euo pipefail throughout the setup script.
  • Verification loop for all expected genesis outputs before proceeding.
  • Idiomatic mkNodeService helper avoids repetition; port arithmetic is straightforward.
  • Correct use of RemainAfterExit = true and requires/after ordering for oneshot → long-running dependency chain.
  • TimeoutStartSec = "900" is a sensible upper bound for a cold Nix build.
  • --node-id and WorkingDirectory consistently use the underscore form (ethlambda_N), matching what the data directory creation step produces.

Automated review by Claude (Anthropic) · custom prompt

@github-actions
Copy link

🤖 Claude Code Review

Here is my review of PR 158:


PR 158 — preview-config.nix Review

This adds a NixOS container config for tekton preview deployments, spinning up a 4-node ethlambda devnet. The overall structure is solid and well-commented. A few issues to address:


Security

Root password in plaintext (L320)

users.users.root = {
  password = "changeme";
};

A hardcoded plaintext password committed to a public repository is a serious issue even in a preview environment. Use hashedPassword with a value generated by mkpasswd, or disable password auth entirely and rely on SSH keys. At minimum, use initialPassword if you want a one-time default, but the current pattern is dangerous.

SSH root login enabled (L315)

settings.PermitRootLogin = "yes";

Combined with the hardcoded password above, this means any attacker who can reach the SSH port gets root. Consider whether root login is actually needed for debugging — a preview user with sudo would be safer. If you need root for debugging, at least restrict to prohibit-password and add an SSH authorized key.

Hardcoded P2P private keys (L230-L260)

The four secp256k1 private keys are committed to the public repo. The comment says "reused from the standard devnet config", which presumably means they are well-known keys, but this should be explicitly documented so future maintainers don't treat them as secret.


Functional Issues

Nodes have no bootnode configuration

The ExecStart for each node passes --custom-network-config-dir, --gossipsub-port, --node-id, --node-key, and metrics flags — but no bootnode/peer flags. If ethlambda doesn't read peer addresses from the genesis config directory, the four nodes will have no way to discover each other and the devnet will be broken from the start. Verify that generate-genesis.sh produces something in genesisDir that ethlambda reads for bootstrapping, or add explicit peer flags (e.g. --bootnodes).

nix build may fail for unprivileged user

The preview user runs nix build but is not listed in nix.settings.trusted-users. In some NixOS configurations this can prevent builds from evaluating impure expressions or accessing the daemon properly. Adding this avoids subtle failures:

nix.settings.trusted-users = [ "preview" ];

lean-quickstart is not pinned

git clone --depth 1 --single-branch \
  https://github.com/blockblaz/lean-quickstart.git "${quickstartDir}"

No branch, tag, or commit is specified, so this always fetches the default branch HEAD. An upstream change to genesis format or generate-genesis.sh could silently break previews. Pin to a specific commit or tag.


Minor Issues

Stale template comment (L3)

# Copy this file to the root of your repo as preview-config.nix and adapt it.

This is the repo root — the file is already in place. The comment is leftover from a template and should be removed.

Silent failure for missing auth token (L185-L186)

PREVIEW_TOKEN=$(cat /etc/preview-token 2>/dev/null || echo "")
AUTHED_URL=$(echo "$PREVIEW_REPO_URL" | sed "s|https://|https://x-access-token:$PREVIEW_TOKEN@|")

If /etc/preview-token is missing, PREVIEW_TOKEN is empty and the URL becomes https://x-access-token:@github.com/.... For a public repo this may work, but for a private repo it will fail with a confusing auth error. A check and early exit with a clear error message would help:

if [ -z "$PREVIEW_TOKEN" ] && [[ "$PREVIEW_REPO_URL" == *"github.com"* ]]; then
  echo "WARNING: No preview token found; clone may fail for private repos"
fi

WorkingDirectory naming vs. systemd unit names

The systemd unit names use dashes (ethlambda-0) while the name variable in mkNodeService uses underscores (ethlambda_0) for the data directory path and --node-id. This is intentional (Linux service naming convention vs. node identifier), but it is worth a comment in mkNodeService to avoid confusion for future readers.


What Looks Good

  • The oneshot + RemainAfterExit pattern for setup-devnet is correct for a setup service that guards multiple dependent units.
  • The restart-idempotency guard (checking for binary + genesis files) is a clean approach.
  • Firewall rules correctly open only the needed TCP/UDP ports.
  • DNS fallback configuration is correct.
  • The TimeoutStartSec = "900" is appropriately generous for a cold Rust build.
  • The meta structure cleanly separates routing, service names, and database config for tekton.

Automated review by Claude (Anthropic) · custom prompt

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 27, 2026

Greptile Summary

This PR adds tekton preview environment support by introducing preview-config.nix, which configures a 4-node ethlambda devnet for PR previews, and updates flake.nix to include HTML files needed by the RPC server.

Key changes:

  • Created comprehensive NixOS configuration for preview containers with 4 ethlambda nodes
  • Each node runs as a systemd service with unique QUIC (9001-9004) and RPC/metrics (8081-8084) ports
  • Genesis generation uses lean-quickstart tooling with Podman containers
  • Node 0 configured as aggregator to enable block finalization
  • Updated build source filter to include .html files (needed for fork_choice.html in RPC static files)
  • SSH access secured with key-only authentication (previous password issue resolved)

Confidence Score: 4/5

  • Safe to merge - well-documented preview environment config with secure SSH setup
  • High confidence due to well-structured, thoroughly documented code with proper error handling and security practices. Score is 4/5 (not 5) because this is a complex new infrastructure configuration that should be validated in the actual tekton preview environment to ensure all components integrate correctly.
  • No files require special attention - code is well-documented and follows best practices

Important Files Changed

Filename Overview
flake.nix Updated source filter to include HTML files needed by RPC server
preview-config.nix Comprehensive NixOS config for 4-node devnet with proper systemd services, key-only SSH auth, and genesis generation

Last reviewed commit: 6a499ba

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 file reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

@MegaRedHand MegaRedHand marked this pull request as draft February 27, 2026 17:10
@MegaRedHand MegaRedHand marked this pull request as ready for review March 2, 2026 16:18
@github-actions
Copy link

github-actions bot commented Mar 2, 2026

🤖 Kimi Code Review

Review Summary

The PR adds NixOS container configuration for a 4-node ethlambda devnet preview environment. The code is well-structured and follows NixOS best practices. Here are the key findings:

Issues Found

  1. Security: Hardcoded private keys (preview-config.nix:120-150)

    • Four secp256k1 private keys are hardcoded in the validator configuration. These should be generated dynamically or loaded from secure storage to prevent key compromise in preview environments.
  2. Error handling: Missing validation (preview-config.nix:180-190)

    • The genesis generation script runs without checking if required container images are available. Add a pre-check for hash-sig-cli and eth-beacon-genesis images to fail fast with clear error messages.
  3. Resource limits: No memory/CPU constraints (preview-config.nix:90-110)

    • The systemd services for ethlambda nodes lack resource limits. Add MemoryMax, CPUQuota, or TasksMax to prevent runaway processes from consuming all container resources.
  4. Port collision: Potential conflict (preview-config.nix:75-80)

    • The firewall configuration uses ports 8081-8084 and 9001-9004. Consider making these configurable via environment variables to avoid conflicts when running multiple preview containers.
  5. Cleanup: Incomplete state reset (preview-config.nix:160-170)

    • The force-rebuild mechanism only removes genesis files but doesn't clear RocksDB data directories. Add rm -rf "${dataDir}"/* to ensure complete state reset.

Positive Aspects

  • Clean separation between build-time (Nix evaluation) and runtime (container execution)
  • Proper use of systemd dependencies between setup and node services
  • Good documentation of ports and endpoints
  • Non-root execution for all services
  • Comprehensive validation of genesis generation outputs

Minor Suggestions

  • Consider adding a health check endpoint aggregator that reports status for all 4 nodes
  • Add logging configuration to capture ethlambda logs in a structured format
  • Consider adding a simple web interface to view node status at the root path

Automated review by Kimi (Moonshot AI) · custom prompt

@github-actions
Copy link

github-actions bot commented Mar 2, 2026

🤖 Codex Code Review

  1. High: unpinned external genesis tooling introduces supply-chain and nondeterminism risk
    /home/runner/work/ethlambda/ethlambda/preview-config.nix:74 fetches lean-quickstart from ref = "main" with no fixed revision/hash. A force-push or compromise on that repo can silently change genesis artifacts and preview behavior.
    Suggested change: pin to a specific commit (rev) and lock content (narHash/fixed-output style).

  2. High: silent fallback to tekton-integration can run previews against the wrong code
    /home/runner/work/ethlambda/ethlambda/preview-config.nix:68 defaults PREVIEW_BRANCH to "tekton-integration" when unset. If CI metadata injection fails, reviewers may validate a different branch without noticing.
    Suggested change: fail fast when PREVIEW_BRANCH is empty instead of defaulting.

  3. Medium: root SSH access is enabled with a baked-in public key
    /home/runner/work/ethlambda/ethlambda/preview-config.nix:297, /home/runner/work/ethlambda/ethlambda/preview-config.nix:305. Even for preview infra, static root keys in repo expand blast radius if leaked/reused and can violate least-privilege expectations.
    Suggested change: disable root SSH for previews, or inject short-lived keys from CI secrets at deploy time.

  4. Medium: setup cache check can leave stale genesis/state across updates
    /home/runner/work/ethlambda/ethlambda/preview-config.nix:197 skips regeneration if two files exist. Changes to validator config/genesis tooling may not apply unless /tmp/force-rebuild is manually set, causing misleading consensus behavior in previews.
    Suggested change: include a deterministic config/tooling fingerprint check (or always regenerate in PR preview environments).

No direct changes to Rust consensus code paths (fork choice, attestation validation, state transition, XMSS, SSZ) are present in this PR, so I did not find consensus-logic regressions in the diff itself.


Automated review by OpenAI Codex · custom prompt

@github-actions
Copy link

github-actions bot commented Mar 2, 2026

🤖 Claude Code Review

Now I have a complete picture of the PR. Let me write the review.


PR #158 Review: preview-config.nix (Tekton Devnet Preview)

Note on existing comments: The automated reviews already posted are based on an earlier version of this file that included PermitRootLogin = "yes", a root password "changeme", and a runtime git clone pattern. The current diff has already addressed all of those issues (key-only SSH, pre-built binary from Nix eval). The points below focus only on the code as it actually exists.


Functional / Correctness

Unpinned builtins.fetchGit (line 74-77)

quickstartSrc = builtins.fetchGit {
  url = "https://github.com/blockblaz/lean-quickstart";
  ref = "main";
};

No rev is pinned. Nix will fetch the current HEAD of main at eval time, making preview builds non-reproducible — a force-push or upstream change to generate-genesis.sh or the genesis tooling silently changes devnet behavior in all future previews. Pin to a specific commit:

quickstartSrc = builtins.fetchGit {
  url = "https://github.com/blockblaz/lean-quickstart";
  ref = "main";
  rev = "<commit-sha>";
};

Peer discovery is unspecified (lines 107-115)

The ExecStart for every node passes --custom-network-config-dir, --node-id, --node-key, and ports — but no --bootnodes or equivalent peer-discovery argument. The four loopback nodes need some mechanism to find each other. If --custom-network-config-dir causes ethlambda to read a nodes.yaml (generated by generate-genesis.sh) for peer addresses, that should be documented in a comment. If it doesn't, the nodes will never connect and the devnet will be silently broken. This needs either explicit peer flags or a confirming comment.


Data directories not cleared on force-rebuild (lines 203-204)

rm -f /tmp/force-rebuild
rm -rf "${genesisDir}"

When force-rebuild is triggered, genesis is regenerated from scratch — but ${dataDir}/ethlambda_N (the RocksDB directories) are not cleared. Old chain state from the previous genesis will be present and will likely cause node startup failures or mismatches since the genesis keys/state change each time genesis is regenerated. Add:

rm -rf "${genesisDir}" "${dataDir}"

genesis.ssz missing from verification list (lines 256-269)

The comment at line 257 lists genesis.ssz as a genesis output. The verification loop below it does not check for it:

for f in config.yaml validators.yaml nodes.yaml genesis.json annotated_validators.yaml; do

If ethlambda reads genesis.ssz at startup (which is likely for initializing the genesis state), missing this check means the failure surfaces with a cryptic I/O error from the node rather than a clear "Genesis generation did not produce genesis.ssz" message from setup.


Security

Hardcoded SSH authorized key (line 306)

users.users.root.openssh.authorizedKeys.keys = [
  "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIEYfIsBaa3Jr/0Ij7QOwWcLp1I1fnNp86yW0Bzsg4Ylg"
];

A single static public key is committed to the repository. This is reasonable for a controlled preview environment, but it means root SSH access to all preview containers is permanently gated on this single key. If the holder's private key is rotated, lost, or if another maintainer needs access, the config must be updated. Injecting authorized keys from CI secrets at deploy time is more flexible. This is a low-severity operational concern, not a blocker.

Hardcoded secp256k1 P2P keys (lines 220-249)

Mentioned in the comment as "reused from the standard devnet config." These are now permanently in git history as well-known keys. The comment could be strengthened to say explicitly: "these are publicly known devnet-only keys and are not secrets." As P2P identity keys (not validator signing keys), the consensus impact is limited.


Nix / DNS

networking.nameservers conflicts with systemd-resolved (line 137)

services.resolved = { enable = true; settings.Resolve.FallbackDNS = [ "8.8.8.8" "1.1.1.1" ]; };
networking.nameservers = [ "8.8.8.8" "1.1.1.1" ];

With systemd-resolved active, /etc/resolv.conf points to 127.0.0.53. Setting networking.nameservers to external IPs installs those addresses directly in /etc/resolv.conf, bypassing the resolver stub and potentially producing conflicting resolution depending on NixOS version. Drop networking.nameservers and rely on resolved's FallbackDNS alone.


Silent fallback branch (line 68-69)

previewBranch = let b = builtins.getEnv "PREVIEW_BRANCH"; in
  if b == "" then "tekton-integration" else b;

If PREVIEW_BRANCH is unset (e.g., CI metadata injection fails), previews silently evaluate against tekton-integration instead of the actual PR branch. A reviewer can approve behavior from the wrong code without noticing. Consider surfacing this as a Nix abort when the variable is unset:

previewBranch =
  let b = builtins.getEnv "PREVIEW_BRANCH"; in
  if b == "" then abort "PREVIEW_BRANCH env var must be set (use --impure)" else b;

Minor

  • quickstartDir is a redundant alias (line 85): quickstartDir = "${quickstartSrc}" is identical to quickstartSrc when interpolated in the script. quickstartSrc already coerces to a path string in string interpolation context. Not wrong, just slightly noisy.

  • flake.nix HTML filter (line 38): The .*\\.html$ addition is syntactically correct. The PR description doesn't mention what HTML files this supports. A brief note on what embeds HTML (a metrics dashboard, the RPC UI?) would help future readers.


What Looks Good

  • Excellent architecture decision: build-time builtins.getFlake + builtins.fetchGit eliminates all runtime git/nix dependencies. The container only needs Podman for genesis image pulls.
  • set -euo pipefail throughout the setup script.
  • Correct oneshot + RemainAfterExit + ordered requires/after/before for the setup→nodes dependency chain.
  • Idempotency check (skip setup on container restart) is clean and useful.
  • --is-aggregator on node 0 only is correctly implemented and well-commented — matches the CLAUDE.md note about justified/finalized slots staying at 0 without it.
  • Node naming consistency: ethlambda_N (underscore) for --node-id, WorkingDirectory, and data dirs; ethlambda-N (hyphen) for systemd unit names. Intentional and consistent throughout.
  • Firewall rules are tight: only the four TCP and four UDP ports are opened.
  • SSH is key-only with PermitRootLogin = "prohibit-password" — the security model here is correct.
  • subUidRanges/subGidRanges for rootless Podman user namespaces is correctly configured.

Automated review by Claude (Anthropic) · custom prompt

@MegaRedHand MegaRedHand requested a review from pablodeymo March 2, 2026 20:23
@pablodeymo pablodeymo merged commit ca5aa1b into main Mar 2, 2026
2 checks passed
@pablodeymo pablodeymo deleted the tekton-integration branch March 2, 2026 20:24
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