feat: add Docker support for running upgrade scripts#65
Merged
Conversation
Add Dockerfile and CI workflow to enable running orbit-actions commands in a containerized environment without requiring local installation of Foundry and Node.js. - Add Dockerfile with Node 18, Foundry, and pre-installed dependencies - Add smoke tests to verify tools and scripts are accessible - Add GitHub Actions workflow to build and test Docker image on PRs - Update README with Docker usage instructions
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds Docker support for running orbit-actions commands in a containerized environment, eliminating the need for local installation of Foundry and Node.js.
Changes:
- Added Dockerfile with Node 18, Foundry installation, and dependency setup
- Created comprehensive smoke test suite to verify Docker image functionality
- Added GitHub Actions workflow for automated Docker image testing on PRs
- Updated documentation with Docker usage examples and configuration instructions
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| Dockerfile | Defines containerized environment with Node 18, Foundry, and pre-built dependencies |
| .dockerignore | Excludes unnecessary files from Docker build context for optimization |
| test/docker/test-docker.bash | Implements smoke tests verifying tools, dependencies, and scripts in Docker image |
| .github/workflows/test-docker.yml | Automates Docker image building and testing on pull requests |
| package.json | Adds test:docker script for running Docker smoke tests |
| README.md | Documents Docker usage with examples for commands, environment variables, and volume mounting |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The Docker build requires lib/ (forge-std, arbitrum-sdk) which comes from git submodules. Update CI to checkout with submodules and document the prerequisite for local builds.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 7 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Add a path-based CLI that allows users to browse and execute scripts from the foundry directory: - Browse directories: `docker run orbit-actions contract-upgrades/1.2.1` - View files: `docker run orbit-actions contract-upgrades/1.2.1/README.md` - Run upgrades: `docker run orbit-actions contract-upgrades/1.2.1/deploy-execute-verify` Structure: - entrypoint.sh: thin shim that sources .env and delegates to router - bin/router: path parsing, directory listing, command dispatch - bin/contract-upgrade: deploy, execute, deploy-execute-verify commands - bin/arbos-upgrade: deploy, execute, verify, deploy-execute-verify commands - lib/common.sh: shared utilities for auth parsing and forge helpers Upgrade commands read configuration from mounted .env file and accept auth flags (--deploy-key, --execute-key, --ledger, etc.) for signing.
Add Verify*.s.sol Forge scripts to each contract-upgrade version folder, replacing hardcoded verification logic with discoverable scripts. - Add verify command to bin/contract-upgrade and bin/router - Create Verify scripts for 1.2.1, 2.1.0, 2.1.2, 2.1.3 - Update READMEs to reference forge script verification
Publish offchainlabs/chain-actions image to Docker Hub: - On push to main: tag as latest - On release tags (v*): tag as version (e.g., 1.2.3, 1.2) - Manual trigger: tag with branch name (for testing) Requires DOCKERHUB_USERNAME and DOCKERHUB_TOKEN secrets.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 21 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Replace bash CLI (bin/router, bin/contract-upgrade, bin/arbos-upgrade, lib/common.sh) with TypeScript implementation in src/cli/. - Add commander for argument parsing - Add execa for subprocess execution - Update Dockerfile to use node entrypoint directly - Remove entrypoint.sh (no longer needed)
- Remove redundant comments that state the obvious - Add named constants for ArbOS precompiles (ARB_OWNER_PUBLIC, ARB_SYS) - Add ARBOS_VERSION_OFFSET constant with explanation - Remove unused AuthArgs interface - Remove unused getRepoRoot import - Fix double findRepoRoot() call in loadEnv()
- Remove deprecated @typescript-eslint/tslint plugin from eslint config - Update docker tests to use --entrypoint for tool access - Apply prettier formatting to src/cli files - Fix no-implicit-coercion lint errors (!! -> Boolean())
Non-Docker tests for the bin/router and related scripts.
Replace verbose Docker documentation with concise CLI section and streamlined Docker examples. Move tooling documentation to end of README to prioritize upgrade guides.
Remove fallback locations for .env files. Now only loads from process.cwd(), matching Forge's behavior for transparency.
Extract executeUpgrade() and verifyUpgrade() helpers to eliminate code duplication between standalone commands and deploy-execute-verify.
Remove redundant subcommand interface - all functionality is accessed via the router's path-based syntax. Also extract deployAction helper to reduce duplication in arbos-upgrade.
Pin foundryup to nightly-2026-02-09 for reproducible Docker builds.
- Pin to actual nightly release (2026-02-10) using commit hash - Add dist/ to eslintignore - Fix prettier formatting in arbos-upgrade.ts
The foundryup --version flag doesn't work correctly in Docker when bootstrapping from the install script. Revert to unpinned foundryup which installs the latest stable release.
godzillaba
reviewed
Mar 4, 2026
Comment on lines
+18
to
+26
| if (arg === '--private-key' || arg === '--account') { | ||
| const value = args[i + 1] | ||
| if (value) { | ||
| return `${arg} ${value}` | ||
| } | ||
| } | ||
| if (arg === '--ledger' || arg === '--interactive') { | ||
| return arg | ||
| } |
Contributor
There was a problem hiding this comment.
do we need to support more than private key? i doubt people will use the others
Collaborator
Author
There was a problem hiding this comment.
we already had these args documented in the readme:
so i assumed they were useful for something
Contributor
There was a problem hiding this comment.
ok gotcha, i guess i don't really have a preference either way
Collaborator
Author
There was a problem hiding this comment.
remove command line args now, use env vars only for foundry with the cli
| env?: Record<string, string> | ||
| } | ||
|
|
||
| export async function runForgeScript( |
Contributor
There was a problem hiding this comment.
we should expose -g, --gas-limit-multiplier since forge scripts can be finicky with L2 gas accounting
Collaborator
Author
There was a problem hiding this comment.
only use env vars now
|
|
||
| ```bash | ||
| # Browse available scripts | ||
| yarn orbit-actions # List top-level directories |
Contributor
There was a problem hiding this comment.
this command doesn't work
yarn orbit-actions
yarn run v1.22.22
warning package.json: License should be a valid SPDX license expression
error Command "orbit-actions" not found.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
Collaborator
Author
There was a problem hiding this comment.
yeh, needed npm link. I've got rid of this now - can use yarn cli locally now
|
|
||
| ```bash | ||
| # Browse available scripts | ||
| yarn orbit-actions # List top-level directories |
Contributor
There was a problem hiding this comment.
also why is the default just ls? personally doesn't feel intuitive
Contributor
There was a problem hiding this comment.
is it because using docker you need to explore to find paths to versions and envs and readme's etc?
if so i think some kind of tree command would be a little better
Collaborator
Author
There was a problem hiding this comment.
added some more help text. we have this so that a user can run any of the scripts in docker via the cli - not hardcoded commands.
I agree that it's not very intuitive - but i dont think tree is great either since you'll still need to pass a path to run the script with that
| return result.stdout.trim() | ||
| } | ||
|
|
||
| export function parseActionAddress( |
Contributor
There was a problem hiding this comment.
this function implies that any action deployment script MUST deploy the action last. which is probably true for most reasonable scripts but still seems like a subtle footgun that should at least be documented
Add comments to Dockerfile and workflow, format PERFORM_SELECTOR expression, use die() instead of manual console.error/process.exit in router.
…v vars
Remove custom auth flag parsing (--private-key, --account, --ledger, etc.)
and the deploy-execute-verify combined command. Forge/cast auth and behavior
is now configured entirely via FOUNDRY_*/ETH_* env vars passed through
process.env.
- Delete src/cli/utils/auth.ts
- Simplify ForgeScriptOptions to {script, rpcUrl, env?}
- Remove authArgs from CastSendOptions
- Gate arbos cast send on FOUNDRY_BROADCAST env var
- Remove parseOptions and combined command routing
- Update env templates with FOUNDRY_* vars
- Update README with env var docs and separate-step workflow
- Log full forge args instead of truncating for easier debugging - runCastCall uses die() on failure instead of returning 'N/A' - Format scheduled upgrade output as (version, timestamp) tuple - Add contextual help text at every CLI browsing level
…text - Trim env templates to FOUNDRY_BROADCAST and ETH_PRIVATE_KEY, link to Foundry config docs for the full list of supported env vars - Remove orbit-actions prefix from CLI help text so it reads correctly regardless of invocation method (yarn cli, Docker, linked binary) - Update README env var table to match
The orbit-actions binary only worked via yarn link and isn't used in Docker (entrypoint is node directly) or local dev (yarn cli).
The [orbit-actions] prefix added no value -- the user already knows what tool they're running. Drop the wrapper and use console.log throughout for consistent, unprefixed output.
Co-authored-by: Henry <11198460+godzillaba@users.noreply.github.com>
…xecute chaining Execute commands now read the deployed action address from Forge's broadcast output when UPGRADE_ACTION_ADDRESS is not set, enabling deploy && execute chaining without manual .env edits. The env var still takes precedence as an explicit override for multisig flows.
Extract duplicate resolveActionAddress from arbos-upgrade and contract-upgrade into a shared function in forge.ts. Make parseActionAddress private and document its last-CREATE assumption.
Test was checking for deploy-execute-verify which no longer exists. Updated to check for deploy/execute/verify as separate commands. Also fixes prettier formatting.
godzillaba
reviewed
Mar 11, 2026
| console.log(`To: ${upgradeExecutor}`) | ||
| console.log(`Calldata: ${executeCalldata}`) | ||
| console.log('') | ||
| console.log('Submit this to your multisig/Safe to execute the upgrade') |
Contributor
There was a problem hiding this comment.
also seems weird to say "submit to your mutlisig" right before going ahead and executing from an EOA
Collaborator
Author
There was a problem hiding this comment.
just deleting that comment. useless ayway
|
|
||
| export async function runCastCall(options: CastCallOptions): Promise<string> { | ||
| try { | ||
| const result = await execa('cast', [ |
Contributor
There was a problem hiding this comment.
is stderr visible to the user if this fails?
Contributor
There was a problem hiding this comment.
should we pass {stderr: 'inherit'}?
Comment on lines
+66
to
+71
| await runCastSend({ | ||
| to: upgradeExecutor, | ||
| sig: 'execute(address,bytes)', | ||
| args: [actionAddress, PERFORM_SELECTOR], | ||
| rpcUrl, | ||
| }) |
Contributor
There was a problem hiding this comment.
we've already generated the actual calldata so imo we should just pass it along directly to cast send.
this is the only place runCastSend is called so i think it's okay to change the function sig.
Suggested change
| await runCastSend({ | |
| to: upgradeExecutor, | |
| sig: 'execute(address,bytes)', | |
| args: [actionAddress, PERFORM_SELECTOR], | |
| rpcUrl, | |
| }) | |
| await runCastSend({ | |
| to: upgradeExecutor, | |
| data: executeCalldata, | |
| rpcUrl, | |
| }) |
Collaborator
Author
There was a problem hiding this comment.
use data in send and call
Skip action address resolution when not broadcasting - forge runs in simulation mode and doesn't produce a broadcast file at the normal path. Also early-return from deploy if UPGRADE_ACTION_ADDRESS is already set. Remove unused getEnv helper from CLI env utils.
Replace cast calldata/sig-based interfaces with pre-encoded --data param. Encode calldata and decode results with ethers Interface, removing the async castCalldata shell-out entirely.
The CLI identifies the action contract by taking the last CREATE from the broadcast file. Add comments to all deploy scripts noting this constraint, and document it in the README for future script authors.
- Block path traversal in router (reject ".." in path args) - Anchor version regex to prevent misrouting on nested paths - Fix .dockerignore excluding .env.sample and env template files - Remove incorrect 2.1.3 verify script (checked nativeTokenDecimals, a 2.1.2 concern; reverts on ETH-native chains) - Add ROLLUP env var to 1.2.1 and 2.1.0 env templates for verify scripts - Use node:22 and foundryup --version stable in Dockerfile - Use submodules: recursive in CI instead of forge install - Add broadcast guidance on arbos execute without FOUNDRY_BROADCAST - Add missing deploy script null check in contract-upgrade cmdExecute - Wrap getChainId with try/catch for consistent error handling - Load .env from repo root instead of cwd for path consistency - Warn on fallback to /app when repo root not found - Drop isCategoryDir messaging in directory listing
foundryup was rewritten and --version now prints the tool version instead of installing a specific Foundry version. The new flag is --install.
godzillaba
reviewed
Mar 23, 2026
godzillaba
approved these changes
Mar 23, 2026
Contributor
godzillaba
left a comment
godzillaba
left a comment
There was a problem hiding this comment.
lgtm with the small comments in last review
Co-authored-by: Henry <11198460+godzillaba@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add Dockerfile and CI workflow to enable running orbit-actions commands in a containerized environment without requiring local installation of Foundry and Node.js.
fixes: