ci(security): scope EIP-170 size gate to deployable contracts#230
Conversation
…cript) Stage 1's `forge build --sizes` exits non-zero if ANY built contract exceeds 24576 bytes, including test-only helpers. SuperPaymasterV2Reinit (a thin SuperPaymaster subclass in SupplementaryLifecycle.t.sol, used purely for UUPS reinitializer testing and never deployed) is ~25.6 KB and was tripping the gate, even though the real deployable SuperPaymaster is 24,159 B (under the limit). Add --skip "*.t.sol" --skip "*.s.sol" so the gate enforces EIP-170 on exactly the contracts we deploy.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ed8b236cc8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # gate enforces the limit on contracts we actually deploy — and is not tripped by | ||
| # test-only helpers (e.g. SuperPaymasterV2Reinit, a thin SuperPaymaster subclass | ||
| # used purely for UUPS reinitializer testing) that are never deployed. | ||
| run: forge build --sizes --skip "*.t.sol" --skip "*.s.sol" |
There was a problem hiding this comment.
In the security workflow this still builds the oversized test helper because Foundry's current forge build --skip docs define the argument as a substring filter, with test and script as aliases for .t.sol and .s.sol (not shell-style globs): https://getfoundry.sh/forge/reference/build/. On GitHub Actions, "*.t.sol" is passed literally when no root-level file matches, so files like contracts/test/.../SupplementaryLifecycle.t.sol are not skipped and the Stage 1 EIP-170 gate remains blocked in the same scenario this commit is trying to fix.
Useful? React with 👍 / 👎.
Codex flagged the previous `--skip "*.t.sol" --skip "*.s.sol"` as undocumented glob strings. Switch to forge's documented aliases: `forge build --help` states 'test and script are aliases for .t.sol and .s.sol'. Same effect (skip test/script so the EIP-170 gate only checks deployable contracts), but on the officially-supported flag form.
fanhousanbu
left a comment
fanhousanbu
left a comment
There was a problem hiding this comment.
✅ APPROVE — CI-only, no contract changes
Change verified: .github/workflows/security.yml only.
Before: forge build --sizes (includes ALL contracts incl. .t.sol) + a fragile awk filter trying to isolate SuperPaymaster lines.
After: forge build --sizes --skip test --skip script — forge's documented aliases for excluding .t.sol / .s.sol files. The EIP-170 gate now checks only deployable contracts, so SuperPaymasterV2Reinit (test-only UUPS reinit helper, ~25.6KB, never deployed) no longer trips it.
SuperPaymaster itself is 24,159B — well within the 24,576B limit. ✅
Also removes the duplicate forge build call (the old script built twice). Clean.
2 participants
Stage 1's
forge build --sizesfails if any built contract exceeds EIP-170 — including test-only helpers.SuperPaymasterV2Reinit(a thinSuperPaymastersubclass inSupplementaryLifecycle.t.sol, used only for UUPS reinitializer testing, never deployed) is ~25.6 KB and tripped the gate. The real deployableSuperPaymasteris 24,159 B (under limit). Fix:--skip "*.t.sol" --skip "*.s.sol"so the gate checks only deployable src. No code change.