chore: harden CI and run build + non-fork tests on push/PR - #433
Merged
Merged
Conversation
- add top-level permissions: contents: read (least-privilege token) - SHA-pin actions/checkout to v4.2.2 with persist-credentials: false - SHA-pin foundry-toolchain to v1.4.0 Both SHAs verified against their release tags.
The workflow previously ran only on workflow_dispatch, so nothing was checked automatically on PRs (unlike EVK/EVC/EPO). Enable push + pull_request triggers running forge build and the non-fork test suite. Fork tests (7 files needing live RPC/secrets) are excluded from automatic runs via --no-match-path to keep CI deterministic and safe on fork PRs; workflow_dispatch still runs the full suite so they can be exercised on demand with RPC env vars provided.
kasperpawlowski
changed the title
The workflow set FOUNDRY_PROFILE=ci but foundry.toml defines no [profile.ci]. Recent foundry treats a missing selected profile as a fatal error (older versions only warned and fell back), so the build failed immediately once the workflow started running on PRs. Use the default profile, which carries the correct solc/evm_version/fs_permissions.
HookTargetMarketStatus.sol imported ../Chainlink/DatastreamsVerifier.sol (lowercase 's') but the file is DataStreamsVerifier.sol. This compiles on case-insensitive filesystems (macOS) but fails on Linux — surfaced now that CI builds on ubuntu-latest.
forge build --sizes exits non-zero because ERC4626EVCCollateralSecuritizeFactory (25,588 bytes) exceeds the EIP-170 runtime limit (24,576) under the repo's default optimizer settings. That's a pre-existing deployability concern, not a CI regression; enforcing it here would make CI red from day one. Use plain forge build as a compile check (matching euler-vault-kit). Size enforcement can be re-added once the oversized contract is addressed.
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
1 participant
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.
Adds real CI to a repo that previously had none, plus workflow hardening. Now self-validated: this PR's own run is green — 437 non-fork tests pass, 0 failed, 0 skipped, across 57 suites.
1. Enable automatic CI (was
workflow_dispatch-only)Runs on
push(development, master) andpull_request:forge build(compile check)forge test -vvvexcluding the 7 fork-test files.workflow_dispatchstill runs the full suite (incl. fork tests) for on-demand runs with RPC env vars.Fork tests excluded from auto-CI: 7 of 61 test files fork the chain and need live RPC/secrets; 2 fork unconditionally against hardcoded public RPCs (flaky). Excluding all 7 via
--no-match-pathkeeps push/PR CI deterministic and secret-free (safe on fork PRs) — the euler-price-oracle model. Chosen over editing the tests toenvOr+skip because on the repo's foundryvm.skip(true)insetUpfails the contract, so guarding would mean ~34 per-function edits changing local behavior; the glob achieves the same with zero test changes.2. Workflow hardening
permissions: contents: readactions/checkout@v4→ SHA-pinned v4.2.2 +persist-credentials: falsefoundry-toolchain@v1→ SHA-pinned v1.4.0 (both SHAs verified against tags)version: nightlyleft floating (nofmt --checkjob, so pin-drift can't break it)Latent issues this new CI surfaced (and how they're handled)
FOUNDRY_PROFILE: cireferenced a non-existent profile. Recent foundry treats this as fatal (older versions only warned), so the build failed instantly. Removed the env var → default profile (correct solc/evm_version/fs_permissions).HookTargetMarketStatus.solimported../Chainlink/DatastreamsVerifier.sol(lowercase 's') but the file isDataStreamsVerifier.sol. Compiles on macOS, fails on Linux — the repo did not build on any case-sensitive filesystem. One-char fix.forge build --sizesfails becauseERC4626EVCCollateralSecuritizeFactoryis 25,588 bytes runtime, over the EIP-170 limit (24,576) by ~1,012 under default optimizer settings — i.e. currently undeployable to mainnet. To avoid making CI red on a pre-existing issue, the build step uses plainforge build. Re-add--sizesonce that contract is brought under the limit (via_ir / optimizer_runs / splitting).Separately, repo-level secret scanning + push protection were enabled (settings, not in this diff).
Commits are unsigned (automated session; GPG pinentry unavailable) — amend with
-Sif required.