ci: add rsk-contracts-test workflow and fix PATH-sensitive forge binary test by fedejinich · Pull Request #40 · rsksmart/optimism

fedejinich · 2026-07-29T22:41:52Z

Adds .github/workflows/rsk-contracts-test.yml — the Foundry-enabled CI lane PAYROLLUP-80 left open — plus a one-line test fix required to make it green. Closes PAYROLLUP-90.

What the workflow does

One job on ubuntu-latest (SHA-pinned actions, permissions: contents: read, timeout-minutes: 30):

Checks out the repo (persist-credentials: false) and scope-initializes the 11 Solidity submodules under packages/contracts-bedrock/lib — deliberately not submodules: recursive on checkout, which would also clone the unrelated repo-root Rust submodules (op-rbuilder, rollup-boost).
Installs Foundry 1.2.3 via foundry-rs/foundry-toolchain (pinned to the v1.9.0 commit SHA; v1.9.1 is 1 day old and the repo's Dependabot cooldown is 10 days). forge --version is echoed and guarded — the job fails loudly if the pin didn't take. 1.2.3 is the version the repo already pins in mise.toml (lines 49–51) and op-deployer/pkg/deployer/forge/version.json.
forge build + forge test in packages/contracts-rootstock (19 tests).
go test ./op-deployer/pkg/deployer/forge/... on the same runner — the package rsk-test.yml intentionally excludes because it needs a real forge binary.

The test fix (`binary_test.go`, +4 lines)

TestStandardBinary_Downloads never sanitized PATH, unlike its sibling TestStandardBinary_ForgeBins. StandardBin.Ensure() prefers a PATH forge whose version equals StandardVersion (v1.2.3) before falling back to the download path the test exercises — so on any machine with forge exactly 1.2.3 on PATH (i.e. precisely what this workflow sets up), both subtests fail. Reproduced locally: FAIL without the fix, ok with it. The fix is the same t.Setenv("PATH", "") pattern the sibling test already uses. Upstream develop has the same latent bug; this is upstreamable as-is.

Path filters — two deliberate choices

The filter includes this workflow file itself. Without it, the PR adding the workflow triggers nothing and its greenness is unprovable (workflow_dispatch only works once the file is on the default branch).
The filter includes op-deployer/pkg/deployer/forge/**, going beyond the ticket's literal "packages/contracts-** changes": this job is the only CI that runs that Go package's tests, so changes to the package must trigger it.

Local verification (forge 1.2.3, darwin_arm64)

forge build: clean build in ~16 s; forge test: 19/19 passed (~0.5 s).
go test ./op-deployer/pkg/deployer/forge/...: ok in ~19 s with forge 1.2.3 first on PATH (including TestStandardBinary_ForgeBins downloading all six release tarballs).

…ry test Adds a Foundry-pinned (1.2.3, matching mise.toml and op-deployer/pkg/deployer/forge/version.json) CI job that runs forge build and forge test for packages/contracts-rootstock plus the op-deployer forge Go tests that rsk-test.yml deliberately excludes for lack of a Foundry-enabled runner. The path filters include the workflow file itself and op-deployer/pkg/deployer/forge/** so the PR adding it — and future changes to either — actually trigger it. TestStandardBinary_Downloads now sanitizes PATH, mirroring its sibling test, so a standard-version forge on PATH cannot short-circuit Ensure() before the download path under test is reached. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Copilot

Pull request overview

Adds a Foundry-enabled GitHub Actions workflow for the RSK fork to run forge build/test for packages/contracts-rootstock and to execute the previously-excluded op-deployer/pkg/deployer/forge/... Go tests on a runner with forge installed, plus a small Go test tweak to make those tests deterministic when forge is present on PATH.

Changes:

Add .github/workflows/rsk-contracts-test.yml to run contracts-rootstock Foundry builds/tests and the op-deployer forge Go test suite with Foundry pinned to v1.2.3.
Fix TestStandardBinary_Downloads to clear PATH so StandardBin.Ensure() exercises the download path consistently in CI.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
op-deployer/pkg/deployer/forge/binary_test.go	Clears `PATH` in a download-focused test to avoid accidentally satisfying `Ensure()` via a `forge` already on `PATH`.
.github/workflows/rsk-contracts-test.yml	Introduces a CI workflow that installs a pinned Foundry version, initializes needed Solidity submodules, runs `forge` build/test, then runs the Go tests that require `forge` on `PATH`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (2)

.github/workflows/rsk-contracts-test.yml:51

The job id contracts-rootstock is shown in the Actions UI, but this job also runs go test ./op-deployer/pkg/deployer/forge/.... Consider adding an explicit job name: (or renaming the job id) so it’s clear the workflow covers both workloads when scanning CI results.

jobs:
  contracts-rootstock:
    runs-on: ubuntu-latest

op-deployer/pkg/deployer/forge/binary_test.go:56

The new test comment hard-codes v1.2.3, which can become stale if StandardVersion is bumped later. Prefer wording that doesn’t embed a specific version (or refers to the pinned StandardVersion conceptually).

	// Clear out the PATH env var so the standard forge on PATH (v1.2.3 in CI)
	// can't satisfy Ensure() before the download path under test is reached.

illuque · 2026-08-07T18:30:35Z

+    paths:
+      - 'packages/contracts-rootstock/**'
+      - 'packages/contracts-bedrock/**'
+      - 'op-deployer/pkg/deployer/forge/**'
+      - '.github/workflows/rsk-contracts-test.yml'


Missing a few paths that also affect build correctness here: go.mod, go.sum, op-service/httputil/**, op-service/ioutil/** (the Go package under test imports these), and mise.toml (where the 1.2.3 pin this job asserts also lives). A change to any of these right now wouldn't retrigger this job. Same list is duplicated under push: below, needs the same fix there.

illuque · 2026-08-07T18:30:35Z

+on:
+  pull_request:
+    branches: [rsk/**]
+    paths:


A required check gated by paths: never runs on PRs outside those paths, so it stays stuck on Pending forever for them. rsk-test.yml has no path filters, probably why it's safe to require. Since this one's going into required status checks, drop the filters here too (or add a skip job reporting the same check name) before that happens.

illuque · 2026-08-07T18:44:36Z

+      # forge.PathBinary() resolves and what StandardBin.Ensure() compares
+      # against StandardVersion ("v1.2.3") before falling back to a download.
+      - name: Test op-deployer forge package
+        run: go test -timeout 20m ./op-deployer/pkg/deployer/forge/...


This is the first CI run of TestStandardBinary_ForgeBins, which downloads six unauthenticated Foundry release tarballs from GitHub with no retry. New flake vector this job introduces, worth being aware of if it starts failing intermittently on network blips.

illuque · 2026-08-07T18:44:36Z

+    # optimizer_runs = 5000, and the Go tests download six Foundry release
+    # tarballs (TestStandardBinary_ForgeBins). Expected wall time is ~8-15 min;
+    # 30 is a ceiling, not a target.
+    timeout-minutes: 30


With go test -timeout 20m below (line 125), a hang in the Go tests hits this job's 30 min ceiling before its own 20m timeout fires, so you lose the goroutine dump that would show where it's stuck. Bumping this to ~45m (or dropping the go test timeout to ~15m) would let the more specific timeout fire first.

illuque · 2026-08-07T18:44:36Z

+      # No --depth: the pinned commits are not all branch tips, and a shallow
+      # submodule fetch can fail to reach them.
+      - name: Check out contracts-bedrock submodules
+        run: git submodule update --init --recursive --jobs 4 packages/contracts-bedrock


nit: nothing here is cached beyond setup-go's Go module cache. Submodule clone, solc download, and the bedrock compile at optimizer_runs=5000 all happen from scratch every run. Not a correctness issue, just wall clock and repeated network calls.

illuque · 2026-08-07T18:44:36Z

+      - 'op-deployer/pkg/deployer/forge/**'
+      - '.github/workflows/rsk-contracts-test.yml'
+
+permissions:


nit: no concurrency: group, so superseded runs on rapid pushes keep burning minutes instead of getting cancelled. Same as the other rsk-* workflows, not specific to this PR.

illuque · 2026-08-07T18:44:36Z

+      - name: Show forge version
+        run: |
+          forge --version
+          forge --version | grep -Eq '(^|[^0-9])1\.2\.3([^0-9]|$)' || {


nit: this greps the whole multi-line forge --version output, so a 1.2.3 inside a commit hash or build timestamp line would also match. Piping through head -1 first would be tighter, though practical risk is low.

illuque · 2026-08-07T18:44:36Z

+
+jobs:
+  contracts-rootstock:
+    runs-on: ubuntu-latest


nit: runs-on: ubuntu-latest isn't pinned to a specific image. Same as the other rsk-* workflows, so not a regression here.

fedejinich marked this pull request as ready for review July 31, 2026 01:23

Copilot AI review requested due to automatic review settings July 31, 2026 01:23

Copilot started reviewing on behalf of fedejinich July 31, 2026 01:23 View session

Copilot AI reviewed Jul 31, 2026

View reviewed changes

Comment thread .github/workflows/rsk-contracts-test.yml Outdated

illuque self-requested a review July 31, 2026 09:16

fix(ci): require numeric Forge version boundaries

4d660df

Copilot AI review requested due to automatic review settings August 3, 2026 22:14

fedejinich requested a review from a team as a code owner August 3, 2026 22:14

Copilot started reviewing on behalf of fedejinich August 3, 2026 22:14 View session

Copilot AI reviewed Aug 3, 2026

View reviewed changes

illuque reviewed Aug 7, 2026

View reviewed changes

Navigation Menu

Uh oh!

ci: add rsk-contracts-test workflow and fix PATH-sensitive forge binary test - #40

ci: add rsk-contracts-test workflow and fix PATH-sensitive forge binary test#40
fedejinich wants to merge 2 commits into
rsk/developfrom
rsk/payrollup-90-foundry-ci

fedejinich commented Jul 29, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

illuque Aug 7, 2026

Uh oh!

illuque Aug 7, 2026

Uh oh!

illuque Aug 7, 2026

Uh oh!

illuque Aug 7, 2026

Uh oh!

illuque Aug 7, 2026

Uh oh!

illuque Aug 7, 2026

Uh oh!

illuque Aug 7, 2026

Uh oh!

illuque Aug 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

fedejinich commented Jul 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What the workflow does

The test fix (binary_test.go, +4 lines)

Path filters — two deliberate choices

Local verification (forge 1.2.3, darwin_arm64)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

fedejinich commented Jul 29, 2026 •

edited

Loading

The test fix (`binary_test.go`, +4 lines)