Skip to content

make rainix-sol-artifacts hermetic: own anvil when no ETH_RPC_URL (fixes main red) - #284

Merged
thedavidmeister merged 3 commits into
mainfrom
fix-artifacts-rpc-secret
Jul 26, 2026
Merged

make rainix-sol-artifacts hermetic: own anvil when no ETH_RPC_URL (fixes main red)#284
thedavidmeister merged 3 commits into
mainfrom
fix-artifacts-rpc-secret

Conversation

@thedavidmeister

@thedavidmeister thedavidmeister commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Problem

The rainix-sol-artifacts CI job (runs in test/fixture/) depended on a live external RPC:

  • main is red: the CI_DEPLOY_SEPOLIA_RPC_URL secret was deleted, so ETH_RPC_URL is empty → Connection refused.
  • Repointing at RPC_URL_ETHEREUM_FORK (this PR's first commit) cannot work: the RPC connects, but the first attempt fails (drpc free-plan 408s / unfunded key), and the retry's --resume then looks for a recorded broadcast for chain 1 that doesn't exist → Error: Deployment not found for chain 1. Any external RPC keeps the job hostage to secrets, key rotation, and rate limits.

Fix: hermetic by default

rainix-sol-artifacts now starts its own ephemeral anvil when no ETH_RPC_URL is provided (port 18545, killed on exit), with anvil's first funded dev account as DEPLOYMENT_KEY. An explicit ETH_RPC_URL — a real deploy — always wins and leaves DEPLOYMENT_KEY untouched. This mirrors the existing precedent in test/bats/task/skip-simulation.test.bats, which already spins anvil with the same dev key.

  • test/fixture/script/Deploy.sol now reads DEPLOYMENT_KEY exactly like consumer deploy scripts, so the fixture exercises the task as consumers run it (bare vm.broadcast() hits foundry's default-sender refusal under broadcast).
  • test.yml drops all secret plumbing from the job (PRIVATE_KEY, RPC URL, etherscan key): the artifacts job now needs no secrets and no external network.

Verification (local, repo's own nix shells)

  • Hermetic simulation run (the CI path): rainix-sol-artifacts exit 0, SIMULATION COMPLETE, anvil reaped by trap.
  • Hermetic broadcast run (DEPLOY_BROADCAST=1): exit 0, ONCHAIN EXECUTION COMPLETE & SUCCESSFUL.
  • Explicit-RPC mode (own anvil on another port + ETH_RPC_URL/DEPLOYMENT_KEY set): task exit 0 — explicit config wins as before. The retry loop (where --resume lives) is untouched by this diff; with a healthy local chain the first attempt succeeds so the retry path never fires in CI.
  • bats test/bats/task/skip-simulation.test.bats: 2/2 ok.
  • Commit-time pre-commit hooks: all green (yamlfmt, nixfmt, deadnix, statix, nil, shellcheck). The --all-files rustfmt failure is pre-existing on an untouched main clone (no repo-root Cargo.toml) and untriggered by this diff (no .rs changes).

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Chores
    • Updated CI so the artifacts job no longer requires external RPC/API secrets and runs hermetically.
    • Enhanced the deployment task to auto-start a local anvil instance (and set the needed RPC/deployment key) when no ETH_RPC_URL is provided.
  • Tests
    • Updated deployment test helpers and fixtures to provide the deployment key via the DEPLOYMENT_KEY environment variable, ensuring consistent broadcasting behavior.

The artifacts job read ETH_RPC_URL from CI_DEPLOY_SEPOLIA_RPC_URL, a legacy org
secret removed in the RPC-secret consolidation to the canonical RPC_URL_*_FORK
scheme. With it empty, forge treated the empty --rpc-url as a local path and
"connection refused" against the cwd (test/fixture/) — main went red with a
misleading error (see #283). The job only SIMULATES script/Deploy.sol, so any
eth fork RPC works; use the surviving RPC_URL_ETHEREUM_FORK.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@thedavidmeister thedavidmeister self-assigned this Jul 24, 2026
@coderabbitai

coderabbitai Bot commented Jul 24, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@thedavidmeister, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 44 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: d9054459-e3d5-4d36-9426-d93685a7581e

📥 Commits

Reviewing files that changed from the base of the PR and between 0353d29 and 5d8a665.

📒 Files selected for processing (1)
  • flake.nix
📝 Walkthrough

Walkthrough

Artifact deployments now use an environment-provided deployment key and start a local Anvil when no RPC URL is configured. CI no longer supplies RPC, Etherscan, or deployment-key secrets for the hermetic artifact task.

Changes

Hermetic artifact deployment

Layer / File(s) Summary
Local RPC fallback
flake.nix
rainix-sol-artifacts starts Anvil on port 18545, exports the local RPC URL and deployment key, and waits for the chain when ETH_RPC_URL is empty.
Environment-based deployment key
test/fixture/script/Deploy.sol, test/bats/task/skip-simulation.test.bats
The deployment fixture reads DEPLOYMENT_KEY for explicit broadcasting, and tests provide that variable while retaining local RPC and simulation behavior.
CI secret removal
.github/workflows/test.yml
The hermetic artifact job no longer injects deployment, RPC, or Etherscan secrets.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant CI as Rainix CI
  participant Task as rainix-sol-artifacts
  participant Anvil
  participant Deploy as Deploy.sol
  CI->>Task: Run artifact task without RPC/key secrets
  Task->>Anvil: Start local node on port 18545
  Task->>Deploy: Set ETH_RPC_URL and DEPLOYMENT_KEY
  Deploy->>Anvil: Broadcast Counter deployment
Loading

Possibly related PRs

🚥 Pre-merge checks | ✅ 2 | ❌ 3

❌ Failed checks (3 warnings)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The issue asks to restore CI via a valid configured RPC secret, but the PR changes workflow and fixture behavior instead of fixing that secret. Update the CI workflow to use the configured ETH_RPC_URL secret or variable, or split the hermetic fallback into a separate follow-up PR.
Out of Scope Changes check ⚠️ Warning The workflow, flake, test, and fixture changes go beyond the linked issue's scope, which only asked to fix the empty ETH_RPC_URL secret or var. Limit this PR to the secret or variable fix, and move the hermetic anvil and fixture harness changes to a separate PR.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: making rainix-sol-artifacts hermetic by starting its own Anvil when ETH_RPC_URL is missing.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix-artifacts-rpc-secret

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

The CI artifacts job depended on a live external RPC via secrets that have
been deleted (empty ETH_RPC_URL, connection refused on main) and no surviving
secret can satisfy it (the retry --resume needs the recorded chain, and a
mainnet fork URL yields 'Deployment not found for chain 1'). Instead of
repointing at another external RPC, the task now runs against its own
ephemeral anvil when no ETH_RPC_URL is provided: no secrets, no external
network, no rate limits. An explicit ETH_RPC_URL (a real deploy) always wins.

The fixture Deploy.sol now reads DEPLOYMENT_KEY like consumer deploy scripts
do, so the fixture exercises the task exactly as consumers run it (broadcast
included); the skip-simulation bats test supplies the key via env accordingly.
CI drops the PRIVATE_KEY / RPC / etherscan secret plumbing from the job.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@thedavidmeister thedavidmeister changed the title fix(ci): point rainix-sol-artifacts at RPC_URL_ETHEREUM_FORK (fixes main red, #283) make rainix-sol-artifacts hermetic: own anvil when no ETH_RPC_URL (fixes main red) Jul 26, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@flake.nix`:
- Around line 246-254: Bound the Anvil readiness loop in the startup flow by
adding a retry deadline and checking that anvil_pid remains alive before each
retry. If the deadline expires or Anvil exits before cast chain-id succeeds,
fail the artifact job with a nonzero status instead of continuing to wait
indefinitely.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: ea83b41f-52f4-453c-bcf3-f607bc82b3a9

📥 Commits

Reviewing files that changed from the base of the PR and between 9fda95e and 0353d29.

📒 Files selected for processing (4)
  • .github/workflows/test.yml
  • flake.nix
  • test/bats/task/skip-simulation.test.bats
  • test/fixture/script/Deploy.sol

Comment thread flake.nix
The readiness wait was unbounded, so an anvil that exited during startup (or
never bound the port) hung the task until the surrounding job timed out.
Bound the wait to 20s and check the anvil process is still alive each
iteration, so both cases exit non-zero with a clear message.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@thedavidmeister

Copy link
Copy Markdown
Contributor Author

Reviewed 5d8a665: approve

Makes rainix-sol-artifacts hermetic — with no ETH_RPC_URL the task runs against its own ephemeral anvil, so the job needs no RPC/deploy/etherscan secrets. This fixes main's red at the root: the old failure was an empty ETH_RPC_URL after CI_DEPLOY_SEPOLIA_RPC_URL was deleted, and pointing it at mainnet only traded that for a chain-mismatch. It also removes the whole failure class (dead keys, deleted secrets, third-party rate limits).

Verification:

  • All 12 checks green, including rainix-sol-artifacts — the empirical proof the hermetic path works end to end in CI.
  • Independently reviewed the secret removals: --verify and --etherscan-api-key are \${VAR:+…} conditionals (so dropping them just omits the flags, and DEPLOY_VERIFY was never set in CI), and DEPLOYMENT_KEY is consumed only by this task — no other matrix task regresses. The embedded key is anvil's well-known public dev account, exported only on the local-anvil path, and was already present in the repo's bats test.
  • CodeRabbit's Major (unbounded readiness loop) fixed in 5d8a665 and verified in both directions locally: happy path exits 0 on chain 31337 (SIMULATION COMPLETE); with port 18545 held by a non-RPC listener the task fails after exactly one cast chain-id attempt via the kill -0 liveness check (anvil exited during startup) instead of hanging to the job timeout.

Merging with --admin (branch protection; sole maintainer cannot self-approve).

@thedavidmeister
thedavidmeister merged commit 4c95ffb into main Jul 26, 2026
13 checks passed
@github-actions

Copy link
Copy Markdown

@coderabbitai assess this PR size classification for the totality of the PR with the following criterias and report it in your comment:

S/M/L PR Classification Guidelines:

This guide helps classify merged pull requests by effort and complexity rather than just line count. The goal is to assess the difficulty and scope of changes after they have been completed.

Small (S)

Characteristics:

  • Simple bug fixes, typos, or minor refactoring
  • Single-purpose changes affecting 1-2 files
  • Documentation updates
  • Configuration tweaks
  • Changes that require minimal context to review

Review Effort: Would have taken 5-10 minutes

Examples:

  • Fix typo in variable name
  • Update README with new instructions
  • Adjust configuration values
  • Simple one-line bug fixes
  • Import statement cleanup

Medium (M)

Characteristics:

  • Feature additions or enhancements
  • Refactoring that touches multiple files but maintains existing behavior
  • Breaking changes with backward compatibility
  • Changes requiring some domain knowledge to review

Review Effort: Would have taken 15-30 minutes

Examples:

  • Add new feature or component
  • Refactor common utility functions
  • Update dependencies with minor breaking changes
  • Add new component with tests
  • Performance optimizations
  • More complex bug fixes

Large (L)

Characteristics:

  • Major feature implementations
  • Breaking changes or API redesigns
  • Complex refactoring across multiple modules
  • New architectural patterns or significant design changes
  • Changes requiring deep context and multiple review rounds

Review Effort: Would have taken 45+ minutes

Examples:

  • Complete new feature with frontend/backend changes
  • Protocol upgrades or breaking changes
  • Major architectural refactoring
  • Framework or technology upgrades

Additional Factors to Consider

When deciding between sizes, also consider:

  • Test coverage impact: More comprehensive test changes lean toward larger classification
  • Risk level: Changes to critical systems bump up a size category
  • Team familiarity: Novel patterns or technologies increase complexity

Notes:

  • the assessment must be for the totality of the PR, that means comparing the base branch to the last commit of the PR
  • the assessment output must be exactly one of: S, M or L (single-line comment) in format of: SIZE={S/M/L}
  • do not include any additional text, only the size classification
  • your assessment comment must not include tips or additional sections
  • do NOT tag me or anyone else on your comment

@coderabbitai

coderabbitai Bot commented Jul 26, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the complete PR delta against its merge base: changed files and diff statistics.
base="$(git merge-base HEAD origin/HEAD 2>/dev/null || true)"
if [ -z "$base" ]; then
  base="$(git merge-base HEAD main 2>/dev/null || true)"
fi
git diff --stat "$base"...HEAD
git diff --name-status "$base"...HEAD
git diff --numstat "$base"...HEAD

Length of output: 509


SIZE=M

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