build(ci)!: move the solidity toolchain to Node 24 - #4205
Conversation
📝 WalkthroughWalkthroughThe PR updates Solidity package engine requirements, Docker base images, and related GitHub Actions workflows from Node.js 22 to Node.js 24. The workflows use Node.js 24.11.1, and both Solidity packages require Node.js 24 or newer. ChangesNode.js 24 runtime upgrade
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
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. Comment |
The end of the chain, and the reason for it. Five workflows, two Dockerfiles and two `engines` fields go from Node 18.15.0 -- end of life since 2025-04-30 -- straight to 24.11.1, the current LTS, supported until 2028-04-30. This was not reachable before. Node 24 needs hardhat >= 2.26, because below it Hardhat's solc download fails with HH502: npm's undici and Node 24's bundled undici share a global-dispatcher symbol, so the download reaches Node's internal dispatcher, which rejects `maxRedirections`. hardhat >= 2.21 in turn broke @defi-wonderland/smock, and no smock version ever reached 2.26. The dependency was archived, so no version ever would. That is why #4203 removed it and #4204 raised hardhat before this commit could exist. The failure that started it, now passing on the same machine and the same runtime -- cold compiler cache, which is what a CI runner always has: before Error HH502: Couldn't download compiler versions list. after Compiled 75 Solidity files successfully Verified, whole suite, both packages, on Node 24.11.1: ecdsa 673 passing / 44 pending / 0 failing random-beacon 955 passing / 0 pending / 0 failing Docker base images are pinned to `node:24.11.1-alpine` rather than the floating `node:24-alpine`, matching the exact version CI pins, so the two runtimes cannot drift apart. This supersedes #4201, which moved the same nine files from 18 to 22 while 24 was still out of reach. If that lands first this needs a one-token rebase per site; if this chain lands, #4201 can be closed. BREAKING CHANGE: the solidity packages now require Node >= 24.0.0. Contributors on Node 18 or 20 will need to upgrade. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1ac138d to
770d35b
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@solidity/ecdsa/Dockerfile`:
- Line 1: Update both solidity/ecdsa/Dockerfile lines 1-1 and
solidity/random-beacon/Dockerfile lines 1-1: after copying the project, change
ownership of $WORK_DIR to the node user, then set USER node before the Hardhat
entrypoint so both images run non-root.
In `@solidity/ecdsa/package.json`:
- Line 81: Regenerate the tracked package-lock.json for the solidity/ecdsa
package using the updated package.json engines declaration, ensuring its root
metadata records Node >=24.0.0 instead of >=14.0.0. Do not alter the manifest’s
engine contract.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 41b8c3c1-bf5f-4639-bfb3-d77c1b397c1a
📒 Files selected for processing (9)
.github/workflows/contracts-ecdsa.yml.github/workflows/contracts-random-beacon.yml.github/workflows/npm-ecdsa.yml.github/workflows/npm-random-beacon.yml.github/workflows/reusable-solidity-docs.ymlsolidity/ecdsa/Dockerfilesolidity/ecdsa/package.jsonsolidity/random-beacon/Dockerfilesolidity/random-beacon/package.json
| @@ -1,4 +1,4 @@ | |||
| FROM node:22.23.1-alpine | |||
| FROM node:24.11.1-alpine | |||
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Use a non-root runtime user in both Hardhat images.
Both Dockerfiles omit USER, so their Hardhat entrypoints run as root. Chown $WORK_DIR after copying the project, then set USER node before the entrypoint.
solidity/ecdsa/Dockerfile#L1-L1: add the ownership change andUSER node.solidity/random-beacon/Dockerfile#L1-L1: add the same ownership change andUSER node.
🧰 Tools
🪛 Trivy (0.72.0)
[error] 1-1: Image user should not be 'root'
Specify at least 1 USER command in Dockerfile with non-root user as argument
Rule: DS-0002
(IaC/Dockerfile)
[error] 1-1: Image user should not be 'root'
Specify at least 1 USER command in Dockerfile with non-root user as argument
Rule: DS-0002
(IaC/Dockerfile)
📍 Affects 2 files
solidity/ecdsa/Dockerfile#L1-L1(this comment)solidity/random-beacon/Dockerfile#L1-L1
🤖 Prompt for 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.
In `@solidity/ecdsa/Dockerfile` at line 1, Update both solidity/ecdsa/Dockerfile
lines 1-1 and solidity/random-beacon/Dockerfile lines 1-1: after copying the
project, change ownership of $WORK_DIR to the node user, then set USER node
before the Hardhat entrypoint so both images run non-root.
Source: Linters/SAST tools
| }, | ||
| "engines": { | ||
| "node": ">=22.0.0" | ||
| "node": ">=24.0.0" |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Synchronize the lockfile with the new engine contract.
solidity/ecdsa/package-lock.json:53-55 still records "node": ">= 14.0.0", while this manifest requires >=24.0.0. Regenerate the tracked lockfile so npm tooling does not use conflicting runtime metadata.
🤖 Prompt for 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.
In `@solidity/ecdsa/package.json` at line 81, Regenerate the tracked
package-lock.json for the solidity/ecdsa package using the updated package.json
engines declaration, ensuring its root metadata records Node >=24.0.0 instead of
>=14.0.0. Do not alter the manifest’s engine contract.
Stacked on #4204. The end of the chain, and the reason for it.
Five workflows, two Dockerfiles and two
enginesfields go from Node 18.15.0— end of life since 2025-04-30 — straight to 24.11.1, the current LTS,
supported until 2028-04-30.
Why this needed three PRs in front of it
It was not reachable before. The dependency chain, each link measured:
HH502— npm's undici and Node 24's bundled undici share a global-dispatchersymbol, so the download reaches Node's internal dispatcher, which rejects
maxRedirections.@defi-wonderland/smock, and no smock version everreached 2.26. The package is archived, so no version ever would.
So smock had to go (#4203) and hardhat had to rise (#4204) before this commit
could exist.
The failure that started the whole investigation, now passing on the same
machine and runtime — cold compiler cache, which is what a CI runner always
has:
Verified
Whole suite, both packages, on Node 24.11.1:
ecdsarandom-beaconDocker images are pinned
node:24.11.1-alpine, not the floatingnode:24-alpine, matching the exactversion CI pins so the two runtimes cannot drift apart.
Relationship to #4201
This supersedes it. #4201 moved the same nine files from 18 to 22, which was
the right call while 24 was out of reach — it no longer is. Either:
one-token rebase per site (
22 → 24instead of18 → 24)Both work. The first is less total churn.
Summary by CodeRabbit