Repository navigation
Conversation
Agents must not regenerate hashes/selectors/zkstack-out/chain-states per commit or debug their red CI on draft PRs; one Update All Generated Artifacts dispatch right before marking ready for review clears them all. Also flags stale chain-state snapshots as the first suspect when interop tests fail after a bytecode change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
kelemeno
requested review from
StanislavBreadless and
vladbochok
as code owners
August 28, 2026 10:36
Auto-generated by CI with foundry v1.5.1 and FOUNDRY_PROFILE=anvil-interop.
kelemeno
added a commit
that referenced
this pull request
Sep 9, 2026
Dispatching the regen against #2445 (a docs-only PR) verified the local-action fix from the previous commit, and surfaced this: the writer decided 'did anything change?' with git diff, while the verifier gunzips and tolerates block-number drift. Since the snapshots are gzipped dumps of a wall-clock chain, every regeneration differs byte-for-byte, so the dispatch always pushed a commit — it pushed six +0/-0 .json.gz files onto that PR. The writer now asks compare-chain-states, the same comparator state-generation-check uses. Also drops the temporary marker file used to exercise the draft-skip path. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This reverts commit 1b3d956.
kelemeno
added a commit
that referenced
this pull request
Sep 9, 2026
## What Splits PR CI into two tiers so that checks requiring an artifact regeneration only have to pass once, right before merging, instead of on every push. The tiers are tied to the PR's **draft status**: on a draft PR only per-commit CI runs; once the PR is marked ready for review, all CI runs. ### Per-commit CI (runs on every push, drafts included) - `l1-contracts-ci`: **removed** `check-hashes`, `check-selectors`, `check-zkstack-out` (moved to pre-merge). Build, foundry tests, zkfoundry tests, verifier-gen check, and coverage stay. - `anvil-interop-ci`: the interop suite keeps running from the committed chain-state snapshots (fast, parallel). If a change alters genesis state it may run against stale bytecode (or fail) until a regen — accepted trade-off; the pre-merge `state-generation-check` gates snapshot freshness before merge. The v31→v33 upgrade test stays (it uses historical v31 states, no regen needed). - All other workflows (lint, slither, l2/system contracts, foundry-ci, licenses, secrets) are untouched. ### Pre-merge CI (new `pre-merge-checks.yaml`, skips on draft PRs) All checks that compare committed generated files against a fresh regeneration: - `check-hashes` (`AllContractsHashes.json`) - `check-selectors` (`l1-contracts/selectors`) - `check-zkstack-out` (`l1-contracts/zkstack-out`, reuses the build artifacts for an incremental rebuild) - `state-generation-check` (chain-state determinism gate) They run when the PR is **not a draft** — on the `ready_for_review` transition and every later push — and on `workflow_dispatch`. `merge_group` is also wired up, but it is forward-looking only: the repo has no merge queue today, and enabling one should not require a workflow edit. A `changes` job decides first whether the PR can affect the artifacts at all, so a documentation-only PR does not pay a 9-minute state regeneration. It is exclusion-based on purpose: an unrecognized path counts as relevant, so a new directory runs the checks rather than silently bypassing them. Only paths that cannot reach a compiler or a generator belong in that list, which is why `protocol-ops/` came back out of it after review: chain-state generation reaches only forge scripts today, but code that *writes* a committed artifact qualifies whatever language it is in — #2457's genesis check is exactly that shape, and nothing structural keeps protocol-ops out of state setup tomorrow. ### `pre-merge-verified` — the check to require **This is the one context to add to the base branch's required checks, and it needs an org-ruleset edit to take effect.** Today nothing enforces the tier: ruleset 18020683 (org-sourced, `Hardened - Protected Branches`) has no `required_status_checks` rule, classic protection is off, and every check on this PR reports `isRequired: null` — so a PR can be merged the moment it is marked ready, while the checks are still queued. Requiring the four validators individually would **not** fix that, which is the trap: GitHub [counts a skipped required check as satisfied](https://docs.github.com/en/pull-requests/how-tos/merge-and-close-pull-requests/troubleshooting-required-status-checks), and the validators are skipped whenever their producer fails. That combination already happened on this PR — [pre-merge attempt 1](https://github.com/matter-labs/era-contracts/actions/runs/33162317525/attempts/1) failed downloading zksolc, skipped all three validators, while [L1 CI](https://github.com/matter-labs/era-contracts/actions/runs/33162317466) passed. So `pre-merge-verified` always runs (`if: always()`), inspects every job's result, and fails on any failure, cancellation, or *unexpected* skip. It passes only when the checks really passed, or when they were skipped for a reason it can name: the PR is a draft, or nothing artifact-affecting changed. Note for whoever configures this: the build job is deliberately named `build-artifacts` here, because both callers of the reusable build otherwise report a check literally named `build / build` (`l1-contracts-ci`'s and this workflow's), which is ambiguous as a required context. ### One-click regen **Update All Generated Artifacts** performs the complete pre-merge regen in one dispatch: hashes + selectors + zkstack-out (via `update-hashes-on-demand`), then chain states. `regenerate-chain-states` needed two fixes to keep that dispatch trustworthy. First, `uses: ./…` resolves from the workspace, which the workflow rewinds to the *target PR's* head — a branch that predates the composite action this revision calls, so the chain-state stage could fail to find it after the hashes stage had already committed. The dispatched revision's actions are now stashed across that checkout and used to fill in only what the target branch lacks — the PR's own copy wins wherever it exists, so a PR that updates the build recipe regenerates with the recipe its own `state-generation-check` will verify against. Verified on exactly that shape: [dispatched from this branch against #2445](https://github.com/matter-labs/era-contracts/actions/runs/34333262063), whose head does not contain the action — stash, checkout, restore and build all succeed. That run also showed the dispatch pushing a chain-states commit onto that docs-only PR — six `+0/-0` `.json.gz` files (since reverted). The cause is benign: the writer commits on any byte difference, and these gzipped dumps of a wall-clock-dependent chain always differ. Suppressing it with `compare-chain-states` was tried and **reverted**, because that comparator skips the *entire* storage of every chain diamond, so a change confined to diamond storage compares as "up to date" — a writer trusting that verdict would leave the committed snapshots stale while reporting success. A redundant commit per dispatch is the cheaper mistake. Worth flagging for the same reason, since this PR makes `state-generation-check` the merge gate: that exclusion is a **known blind spot of the gate itself**, and it predates this PR. Narrowing it to the genuinely volatile slots is follow-up work that would both let the writer be smarter and make the gate strictly stronger. What this PR does fix in the comparator is the version-directory scan: it iterated only the committed tree, so a protocol bump adding a generated-only directory compared as up to date; it now takes the union (fixture-checked — identical trees pass, generated-only and committed-only versions both fail). ### Shared build (dedup) The all-projects foundry build + artifacts cache has a single definition: `build-contract-artifacts.yaml` (`workflow_call`), used by both `l1-contracts-ci` and `pre-merge-checks`, carrying the ci-speedups build steps from #2441 (solc-cache warming, prune unit test). It now declares `permissions: contents: read` instead of inheriting `l1-contracts-ci`'s `pull-requests: write`. Cache coordination is race-free by construction rather than by waiting. The build takes the key it saves under — run-scoped for pre-merge, so no other run ever writes it — and *outputs* the key consumers must restore: the reused key when the reuse hit (already finalized, since the build just downloaded it), otherwise its own. `reuse-key` is the cheap path: when `pre-merge-checks` runs on a commit `l1-contracts-ci` has already finished building, the build collapses to a cache download ([63 seconds, measured](https://github.com/matter-labs/era-contracts/actions/runs/33162317525/job/98821679690)) — see the caveat under "The gating, demonstrated" for when it does not. Similarly, the deterministic chain-state build (plain foundry, `anvil-interop` profile, no warm cache) has a single definition in the `build-anvil-interop-artifacts` composite action, used by both `regenerate-chain-states` (the snapshot writer) and `state-generation-check` (the verifier) so the two cannot drift apart. ## Why a separate workflow, rather than `if: !draft` on the four jobs in place? Gating the four jobs where they were — and adding `ready_for_review` to `l1-contracts-ci` and `anvil-interop-ci` — is simpler, and it keeps `state-generation-check` behind the anvil path filter. It was rejected for one reason: `ready_for_review` re-runs those workflows *whole*. The per-commit tier's ~20 minutes of builds, foundry tests, zkfoundry tests and coverage would run a second time on a commit where they already passed, and the PR's check rollup would be replaced by that second run. Suppressing the tier-1 jobs on that event (`if: github.event.action != 'ready_for_review'`) trades the waste for something worse: `test-foundry` and friends re-reporting as *skipped*, which — per the same GitHub behaviour described above — reads as satisfied. A separate workflow re-runs only the four checks that need re-running, and leaves tier-1's verdicts standing. The cost is the shared build definition and the cache handshake above, which is why both are kept small and why the build is skipped outright on a reuse hit. ## The gating, demonstrated Reviewers noted the draft and `ready_for_review` paths had never actually run. They have now, on this branch: | Case | Run | Result | | --- | --- | --- | | Push while draft | [34333147132](https://github.com/matter-labs/era-contracts/actions/runs/34333147132) | `changes` green (the pushed path was *not* excluded, so the tier wanted to run), `build-artifacts` and all four validators skipped, `pre-merge-verified` green — skips recognised as deliberate | | `ready_for_review` | [34333263535](https://github.com/matter-labs/era-contracts/actions/runs/34333263535) | full tier green on real results | | Push while non-draft | [34334332690](https://github.com/matter-labs/era-contracts/actions/runs/34334332690) | tier runs again | One honest caveat on the reuse path: in the `ready_for_review` run above the build did *not* hit the cache, because the ready transition came ~2 minutes after the push while `l1-contracts-ci` was still building the same commit — both missed and both built, which is the concurrent case the run-scoped save key makes safe ([consumers restored `artifacts-l1-pre-merge-<sha>-<run_id>`](https://github.com/matter-labs/era-contracts/actions/runs/34251002816)). The reuse hit is demonstrated separately, by a rerun against an already-built commit: [a 63-second build](https://github.com/matter-labs/era-contracts/actions/runs/33162317525/job/98821679690) that skipped the compile entirely. In practice the ready transition usually lands after tier-1 has finished, which is when reuse pays. ## Merge flow 1. Open the PR as a **draft** and iterate; per-commit CI must be green. 2. When ready: dispatch **Update All Generated Artifacts** with the PR number. 3. Mark the PR **ready for review**; merge once `pre-merge-verified` is green. If review forces bytecode changes, regen again (or convert back to draft until done). Documented in `docs/ai-review/docs/ci-green.md` under "CI tiers" — the real CI doc, whose job lists and v31→v32 references were stale. `.github/workflows/README.md` (which is about calldata generation) keeps a pointer instead of the section. #2445 writes the same expectations into `AGENTS.md` for agents. ## Coordination with #2457 #2457 adds `check-zksync-os-genesis` to `l1-contracts-ci` with `needs: [build]` and the restore action. It is a committed-generated-artifact check by the same definition as the rest (it regenerates `configs/genesis/zksync-os/latest.json` and fails on the diff), so under this policy it belongs in the pre-merge tier, and **Update All Generated Artifacts** needs a leg that regenerates it — otherwise the one-click regen leaves it red. The two PRs also conflict in `l1-contracts-ci.yaml` (this PR deletes the block #2457 inserts into). **Whichever lands second adapts**: if #2457 lands first, this PR moves the job into `pre-merge-checks.yaml` and adds the regen leg; if this lands first, #2457 targets the new workflow. > Note for the squash message: the first commit in this branch describes the gate as a `pre-merge` label, which was replaced by draft-status gating later in the branch. Prefer this description over the concatenated commit messages. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
3 tasks done
0xValera
added a commit
that referenced
this pull request
Sep 21, 2026
## What ❔ The regeneration workflows commit through the GitHub API instead of `git commit && git push` as `zksync[bot]`, so their commits are signed. - New composite action `.github/actions/signed-commit`: stages the given paths and creates the commit with the GraphQL `createCommitOnBranch` mutation, which GitHub signs. It refuses to run on a branch other than the requested one, and `expectedHeadOid` fails the call instead of overwriting a newer push. - `update-hashes-on-demand.yaml` and `regenerate-chain-states.yaml` mint a `matterlabs-bots` App token (`contents: write` on this repository, revoked at job end) once the PR's own build has run, then call the action. The action is taken from the dispatched ref into `.ci-actions/`, so PRs branched before it existed can still be regenerated. - `RELEASE_TOKEN` is no longer used, checkouts run with `persist-credentials: false`, and `contents: write` is dropped where nothing pushes any more. Commits are **Verified** and attributed to `matterlabs-bots[bot]`; unlike pushes with `GITHUB_TOKEN`, they trigger the PR's workflows. ## Why ❔ The org ruleset requires signed commits on `main`, `release/**` and `draft/**` and checks every commit on the PR head, so each regeneration left an unsigned bot commit to re-sign and force-push before merging (#2461, #2432, #2479, #2445). [EVM-1688](https://linear.app/matterlabs/issue/EVM-1688/bot-signed-commits-for-ci-jobs). Signing through the API keeps key material out of CI; the App and its org secrets already exist and are installed here, and `helm-charts` uses the same path. The other bases carrying these workflows (`draft/v0.34.0`, `draft-v31`, `atomic-imt-reduced`, `release/v0.33.0-atomic-interop`) need the change adapted to their copies. ## Validation - `update-hashes-on-demand.yaml` dispatched from this branch against a `main`-based scratch PR with one corrupted hash and no action in its tree ([run](https://github.com/matter-labs/era-contracts/actions/runs/34475450705), #2495): the restoring commit is authored by `matterlabs-bots[bot]` with `verification.reason == valid`, and the PR's workflows re-ran on it. - `regenerate-chain-states.yaml` is not exercised, since `main` has no anvil-interop tree. Its commit step is identical; the ~3.5 MB payload gets covered on `draft/v0.34.0`. ## Checklist - [x] PR title corresponds to the body of PR (we generate changelog entries from PRs). - [x] Validated through the CI run linked above; workflow-only change, no test suite applies. - [x] Documentation comments have been added / updated. --------- Co-authored-by: Anton Baliasnikov <aba@matterlabs.dev> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.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.
What
Adds a Generated Artifacts and Draft-PR CI Policy section to
AGENTS.md, so coding agents stop wasting time on the regen-dependent checks during development:check-hashes/check-selectors/check-zkstack-out/state-generation-checkis expected and must NOT be investigated, fixed, or regenerated per commit.ANVIL_INTEROP_FRESH_DEPLOY=1tests the actual code locally). This is the failure mode that previously caused phantom debugging (e.g. around chore(verifier): regenerate ZKsyncOSVerifierPlonk for the zksync-os v0.5.3-private VK #2433).The existing "zkstack-out out of date" / "selectors out of date" Common Issues entries now point at this policy and at the one-click dispatch.
Why
Docs-only, mergeable now: it lets us trial the "don't chase regen checks on drafts" behavior for a few days while #2437 (which additionally makes those checks skip on draft PRs) waits for review. The policy text is written to stay correct after #2437 merges.
🤖 Generated with Claude Code