[DON'T MERGE] draft-v31 to zksync-os-stable#1947
Open
kelemeno wants to merge 683 commits intozksync-os-stablefrom
Open
[DON'T MERGE] draft-v31 to zksync-os-stable#1947kelemeno wants to merge 683 commits intozksync-os-stablefrom
kelemeno wants to merge 683 commits intozksync-os-stablefrom
Conversation
kelemeno
requested review from
Deniallugo,
StanislavBreadless and
vladbochok
as code owners
![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: 46a554be96
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| ); | ||
|
|
||
| _deployedContracts.stateTransition.diamondInit = address(new DiamondInit{salt: _salt}(_isZKsyncOS)); | ||
| _deployedContracts.stateTransition.diamondInit = address(new DiamondInit{salt: _salt}(false)); |
There was a problem hiding this comment.
In GatewayCTMDeployer._deployFacetsAndUpgrades, DiamondInit is always constructed with false, so s.zksyncOS is forced off even when _config.isZKsyncOS is true. DiamondInit persists this flag in chain storage, and ZKChainBase/TransactionValidator use it to select OS-specific transaction types and validation paths; a ZKsync OS deployment will therefore run in the wrong mode and reject/accept transactions incorrectly. This only manifests when _config.isZKsyncOS is true, but in that case it breaks OS chains. Consider threading the config flag through and using it in the constructor.
Useful? React with 👍 / 👎.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Comment on lines
+8
to
+59
| runs-on: ubuntu-latest | ||
|
|
||
| steps: | ||
| - name: Checkout the repository | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| submodules: recursive | ||
|
|
||
| - name: Install foundry-zksync | ||
| run: | | ||
| mkdir ./foundry-zksync | ||
| curl -LO https://github.com/matter-labs/foundry-zksync/releases/download/foundry-zksync-v0.0.30/foundry_zksync_v0.0.30_linux_amd64.tar.gz | ||
| tar zxf foundry_zksync_v0.0.30_linux_amd64.tar.gz -C ./foundry-zksync | ||
| chmod +x ./foundry-zksync/forge ./foundry-zksync/cast | ||
| echo "$PWD/foundry-zksync" >> $GITHUB_PATH | ||
|
|
||
| - name: Use Node.js | ||
| uses: actions/setup-node@v3 | ||
| with: | ||
| node-version: 18.20.8 | ||
|
|
||
| - name: Install Yarn | ||
| run: npm install -g yarn | ||
|
|
||
| - name: Build da-contracts artifacts | ||
| run: yarn da build:foundry | ||
|
|
||
| - name: Build artifacts | ||
| working-directory: system-contracts | ||
| run: | | ||
| yarn install | ||
| yarn build | ||
| yarn build:foundry | ||
|
|
||
| - name: Build artifacts | ||
| working-directory: l1-contracts | ||
| run: | | ||
| yarn install | ||
| yarn build:foundry | ||
|
|
||
| - name: Create cache | ||
| uses: actions/cache/save@v4 | ||
| with: | ||
| key: artifacts-system-${{ github.sha }} | ||
| path: | | ||
| system-contracts/zkout | ||
| system-contracts/cache-forge | ||
| system-contracts/bootloader/build | ||
| system-contracts/artifacts-zk | ||
| system-contracts/cache-zk | ||
| system-contracts/typechain | ||
| system-contracts/contracts-preprocessed |
Check warning
Code scanning / CodeQL
Workflow does not contain permissions Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 1 month ago
In general, the fix is to add an explicit permissions: block that grants only the minimal required access to GITHUB_TOKEN. For this workflow, all jobs only need to read repository contents (for actions/checkout) and interact with caches/artifacts; no step needs to write to code, issues, or PRs. Therefore contents: read is sufficient as a baseline. Declaring it at the top (root) level of the workflow applies to all jobs that don’t override it.
The best minimal-impact change is:
- Add a root-level
permissions:block right after thename:(oron:) declaration:contents: read
- Do not add any broader permissions since none of the listed steps require them.
- No other functional changes are necessary; the jobs will continue to run as before but with a restricted
GITHUB_TOKEN.
Concretely, in .github/workflows/system-contracts-ci.yaml, insert:
permissions:
contents: readbetween line 2 and line 3 (before on:), keeping indentation consistent.
Suggested changeset
1
.github/workflows/system-contracts-ci.yaml
| @@ -1,5 +1,8 @@ | ||
| name: System contracts CI | ||
|
|
||
| permissions: | ||
| contents: read | ||
|
|
||
| on: | ||
| pull_request: | ||
|
|
Copilot is powered by AI and may make mistakes. Always verify output.
…ontracts into draft-v31
Kl/merge zkstable (#1967)
# What ❔ <!-- What are the changes this PR brings about? --> <!-- Example: This PR adds a PR template to the repo. --> <!-- (For bigger PRs adding more context is appreciated) --> ## Why ❔ <!-- Why are these changes done? What goal do they contribute to? What are the principles behind them? --> <!-- Example: PR templates ensure PR reviewers, observers, and future iterators are in context about the evolution of repos. --> ## Checklist <!-- Check your PR fulfills the following items. --> <!-- For draft PRs check the boxes as you complete them. --> - [ ] PR title corresponds to the body of PR (we generate changelog entries from PRs). - [ ] Tests for the changes have been added / updated. - [ ] Documentation comments have been added / updated. --------- Signed-off-by: Danil <deniallugo@gmail.com> Co-authored-by: Danil <deniallugo@gmail.com> # What ❔ <!-- What are the changes this PR brings about? --> <!-- Example: This PR adds a PR template to the repo. --> <!-- (For bigger PRs adding more context is appreciated) --> ## Why ❔ <!-- Why are these changes done? What goal do they contribute to? What are the principles behind them? --> <!-- Example: PR templates ensure PR reviewers, observers, and future iterators are in context about the evolution of repos. --> ## Checklist <!-- Check your PR fulfills the following items. --> <!-- For draft PRs check the boxes as you complete them. --> - [ ] PR title corresponds to the body of PR (we generate changelog entries from PRs). - [ ] Tests for the changes have been added / updated. - [ ] Documentation comments have been added / updated.
# What ❔ <!-- What are the changes this PR brings about? --> <!-- Example: This PR adds a PR template to the repo. --> <!-- (For bigger PRs adding more context is appreciated) --> ## Why ❔ <!-- Why are these changes done? What goal do they contribute to? What are the principles behind them? --> <!-- Example: PR templates ensure PR reviewers, observers, and future iterators are in context about the evolution of repos. --> ## Checklist <!-- Check your PR fulfills the following items. --> <!-- For draft PRs check the boxes as you complete them. --> - [ ] PR title corresponds to the body of PR (we generate changelog entries from PRs). - [ ] Tests for the changes have been added / updated. - [ ] Documentation comments have been added / updated. --------- Signed-off-by: Danil <deniallugo@gmail.com> Co-authored-by: zkzoomer <sergio@zkzoomer.io> Co-authored-by: 0xValera <55665573+0xValera@users.noreply.github.com> Co-authored-by: 0xValera <valera7743@icloud.com> Co-authored-by: Danil <deniallugo@gmail.com> Co-authored-by: Marcin M <128217157+mm-zk@users.noreply.github.com> Co-authored-by: Raid Ateir <ateirraid@gmail.com> Co-authored-by: Raid5594 <52794079+Raid5594@users.noreply.github.com> Co-authored-by: Vladislav Volosnikov <vladyan18@gmail.com> Co-authored-by: EmilLuta <evl@matterlabs.dev> Co-authored-by: EmilLuta <EmilLuta@users.noreply.github.com> Co-authored-by: AntonD3 <74021421+AntonD3@users.noreply.github.com> Co-authored-by: Stanislav Bezkorovainyi <stanislavbezkor@gmail.com> Co-authored-by: mm <mm@matterlabs.dev> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Alexander Melnikov <sanekmelnikov@gmail.com> Co-authored-by: nikitastupin-matterlabs <178824211+nikitastupin-matterlabs@users.noreply.github.com> Co-authored-by: Anton Baliasnikov <aba@matterlabs.dev> Co-authored-by: Claude <noreply@anthropic.com>
# What ❔ <!-- What are the changes this PR brings about? --> <!-- Example: This PR adds a PR template to the repo. --> <!-- (For bigger PRs adding more context is appreciated) --> ## Why ❔ <!-- Why are these changes done? What goal do they contribute to? What are the principles behind them? --> <!-- Example: PR templates ensure PR reviewers, observers, and future iterators are in context about the evolution of repos. --> ## Checklist <!-- Check your PR fulfills the following items. --> <!-- For draft PRs check the boxes as you complete them. --> - [ ] PR title corresponds to the body of PR (we generate changelog entries from PRs). - [ ] Tests for the changes have been added / updated. - [ ] Documentation comments have been added / updated.
# What ❔ <!-- What are the changes this PR brings about? --> <!-- Example: This PR adds a PR template to the repo. --> <!-- (For bigger PRs adding more context is appreciated) --> ## Why ❔ <!-- Why are these changes done? What goal do they contribute to? What are the principles behind them? --> <!-- Example: PR templates ensure PR reviewers, observers, and future iterators are in context about the evolution of repos. --> ## Checklist <!-- Check your PR fulfills the following items. --> <!-- For draft PRs check the boxes as you complete them. --> - [ ] PR title corresponds to the body of PR (we generate changelog entries from PRs). - [ ] Tests for the changes have been added / updated. - [ ] Documentation comments have been added / updated.
# What ❔ <!-- What are the changes this PR brings about? --> <!-- Example: This PR adds a PR template to the repo. --> <!-- (For bigger PRs adding more context is appreciated) --> ## Why ❔ <!-- Why are these changes done? What goal do they contribute to? What are the principles behind them? --> <!-- Example: PR templates ensure PR reviewers, observers, and future iterators are in context about the evolution of repos. --> ## Checklist <!-- Check your PR fulfills the following items. --> <!-- For draft PRs check the boxes as you complete them. --> - [ ] PR title corresponds to the body of PR (we generate changelog entries from PRs). - [ ] Tests for the changes have been added / updated. - [ ] Documentation comments have been added / updated.
…ness Fix test suite flakiness
# What ❔ Adds a CI check to ensure that `l1-contracts/zkstack-out` is kept up to date, plus some other cosmetic fixes This is a sister PR to matter-labs/zksync-era#4616 <!-- What are the changes this PR brings about? --> <!-- Example: This PR adds a PR template to the repo. --> <!-- (For bigger PRs adding more context is appreciated) --> ## Why ❔ <!-- Why are these changes done? What goal do they contribute to? What are the principles behind them? --> <!-- Example: PR templates ensure PR reviewers, observers, and future iterators are in context about the evolution of repos. --> ## Checklist <!-- Check your PR fulfills the following items. --> <!-- For draft PRs check the boxes as you complete them. --> - [ ] PR title corresponds to the body of PR (we generate changelog entries from PRs). - [ ] Tests for the changes have been added / updated. - [ ] Documentation comments have been added / updated.
# What ❔ * [x] Unify zksync-foundry installation in CI * [x] Use latest stable version `v0.1.5` <!-- What are the changes this PR brings about? --> <!-- Example: This PR adds a PR template to the repo. --> <!-- (For bigger PRs adding more context is appreciated) --> ## Why ❔ To remove old version of foundry and simplify CI and fix a bug with MacOS and Linux hash mismatches. <!-- Why are these changes done? What goal do they contribute to? What are the principles behind them? --> <!-- Example: PR templates ensure PR reviewers, observers, and future iterators are in context about the evolution of repos. --> ## Checklist <!-- Check your PR fulfills the following items. --> <!-- For draft PRs check the boxes as you complete them. --> - [x] PR title corresponds to the body of PR (we generate changelog entries from PRs). - [x] Tests for the changes have been added / updated. - [x] Documentation comments have been added / updated.
fix: ZKsyncOSDualVerifier as non-testnet verifier for ZKSyncOS
# What ❔ <!-- What are the changes this PR brings about? --> <!-- Example: This PR adds a PR template to the repo. --> <!-- (For bigger PRs adding more context is appreciated) --> ## Why ❔ <!-- Why are these changes done? What goal do they contribute to? What are the principles behind them? --> <!-- Example: PR templates ensure PR reviewers, observers, and future iterators are in context about the evolution of repos. --> ## Checklist <!-- Check your PR fulfills the following items. --> <!-- For draft PRs check the boxes as you complete them. --> - [ ] PR title corresponds to the body of PR (we generate changelog entries from PRs). - [ ] Tests for the changes have been added / updated. - [ ] Documentation comments have been added / updated.
…ive costs (#2111) # What ❔ Update zksync-os constants related to pubdata and intrinsic native costs. Add comment to zksync-era mint to acknowledge that logic is replicated in zksync-os. <!-- What are the changes this PR brings about? --> <!-- Example: This PR adds a PR template to the repo. --> <!-- (For bigger PRs adding more context is appreciated) --> ## Why ❔ As L2_ASSET_TRACKER sub-call was added to bootloader in zksync-os, which significantly affected the worst-case scenario in terms of intrinsic native costs. The pubdata constant had to be updated both due to the above call and due to addition of treasury account. <!-- Why are these changes done? What goal do they contribute to? What are the principles behind them? --> <!-- Example: PR templates ensure PR reviewers, observers, and future iterators are in context about the evolution of repos. --> ## Checklist <!-- Check your PR fulfills the following items. --> <!-- For draft PRs check the boxes as you complete them. --> - [x] PR title corresponds to the body of PR (we generate changelog entries from PRs). - [ ] Tests for the changes have been added / updated. - [x] Documentation comments have been added / updated. --------- Co-authored-by: zksync[bot] <zksync[bot]@users.noreply.github.com>
## Summary - Add comprehensive **testing guidelines** for agents: test structure, coverage requirements, mock usage, regression tests, and readability standards. - Add **PR description maintenance** rule: agents must keep PR descriptions up to date and follow existing styling. - Add **fork-based PR workflow** note: agents must push to forks, not directly to the main repo. ## Test plan - [ ] Review AGENTS.md diff for accuracy and completeness of guidelines - [ ] Verify no unrelated changes were introduced 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: developer <developer@example.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…2114) ## Summary - Adds a `dangerousContracts` mapping to `GatewayTransactionFilterer` that restricts specific L2 contracts to be callable only by whitelisted senders, regardless of their address - Complements the existing `MIN_ALLOWED_ADDRESS` blanket restriction with fine-grained per-contract control — useful for high-address contracts that should still be gated - Registers `L2_CREATE2_FACTORY_ADDR` (the built-in Create2Factory deployed during ZKsync OS genesis) as dangerous on initialization, to prevent unauthorized contract deployments through the Gateway - Adds `addDangerousContract` / `removeDangerousContract` owner-only functions with corresponding events and custom errors (`AlreadyDangerousContract`, `NotDangerousContract`) ## Test plan - [x] `test_initialize_marksCreate2FactoryAsDangerous` — Create2Factory is in the dangerous list after init - [x] `test_initialize_emitsDangerousContractAddedForCreate2Factory` — event emitted during init - [x] `test_addDangerousContract_marksContractAsDangerous` — contract added correctly - [x] `test_addDangerousContract_emitsEvent` — `DangerousContractAdded` event emitted - [x] `test_addDangerousContract_revertsIfNotOwner` — access control enforced - [x] `test_addDangerousContract_revertsIfAlreadyDangerous` — double-add reverts - [x] `test_removeDangerousContract_unmarksContract` — contract removed correctly - [x] `test_removeDangerousContract_emitsEvent` — `DangerousContractRemoved` event emitted - [x] `test_removeDangerousContract_revertsIfNotOwner` — access control enforced - [x] `test_removeDangerousContract_revertsIfNotDangerous` — removing non-dangerous reverts - [x] `test_isTransactionAllowed_blocksDangerousContractForNonWhitelisted` — non-whitelisted sender blocked - [x] `test_isTransactionAllowed_allowsDangerousContractForWhitelisted` — whitelisted sender allowed - [x] `test_isTransactionAllowed_blocksCreate2FactoryForNonWhitelisted` — Create2Factory blocked - [x] `test_isTransactionAllowed_allowsCreate2FactoryForWhitelisted` — whitelisted can call Create2Factory - [x] `test_isTransactionAllowed_highAddressStillAllowedIfNotDangerous` — non-dangerous high addresses unaffected - [x] `test_isTransactionAllowed_dangerousContractTakesPrecedenceOverHighAddress` — dangerous check runs before high-address shortcut - [x] `test_isTransactionAllowed_afterRemovingDangerousContract_highAddressAllowed` — removal restores access All 40 tests pass (17 new + 23 existing). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: StanislavBreadfulAI <stanislav@breadful.ai> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
# What ❔ <!-- What are the changes this PR brings about? --> <!-- Example: This PR adds a PR template to the repo. --> <!-- (For bigger PRs adding more context is appreciated) --> ## Why ❔ <!-- Why are these changes done? What goal do they contribute to? What are the principles behind them? --> <!-- Example: PR templates ensure PR reviewers, observers, and future iterators are in context about the evolution of repos. --> ## Checklist <!-- Check your PR fulfills the following items. --> <!-- For draft PRs check the boxes as you complete them. --> - [ ] PR title corresponds to the body of PR (we generate changelog entries from PRs). - [ ] Tests for the changes have been added / updated. - [ ] Documentation comments have been added / updated. --------- Co-authored-by: Alexander Melnikov <sanekmelnikov@gmail.com> Co-authored-by: Stanislav Breadless <stanislavbezkor@gmail.com> Co-authored-by: kelemeno <34402761+kelemeno@users.noreply.github.com> Co-authored-by: zksync[bot] <zksync[bot]@users.noreply.github.com> Co-authored-by: kelemeno <kl@matterlabs.dev>
Resolve conflicts: accept draft-v31 for l1-contracts and generated files, keep gateway command additions in protocol-ops. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ework chore: reentrancy regression test rework
- Fix migrate step ordering: notify-server before submit - Derive shouldSend from !simulate instead of hardcoding true - Extract shared build_admin_functions_script to gateway/mod.rs - Replace manual JSON parsing with typed serde structs in migrate.rs - Move token multiplier setter inside !skip_priority_txs in init.rs Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…/matter-labs/era-contracts into gateway-commands-in-protocol-ops
# What ❔ AI review folder will serve as a basis for testing and sharing approaches wrt to AI reviews ## Why ❔ <!-- Why are these changes done? What goal do they contribute to? What are the principles behind them? --> <!-- Example: PR templates ensure PR reviewers, observers, and future iterators are in context about the evolution of repos. --> ## Checklist <!-- Check your PR fulfills the following items. --> <!-- For draft PRs check the boxes as you complete them. --> - [ ] PR title corresponds to the body of PR (we generate changelog entries from PRs). - [ ] Tests for the changes have been added / updated. - [ ] Documentation comments have been added / updated.
`DeployCTMUtils.initializeConfig` accepted a `permanentValuesPath` parameter but never read from it — only `configPath` was used. Remove the dead parameter and update all callers: - DeployCTM.s.sol: drop from runInner, runWithBridgehub, _runConfiguredTest; remove PERMANENT_VALUES_INPUT env read - GatewayVotePreparation.s.sol: drop param + env read - foundry.toml: remove fs_permissions for permanent-values paths - .env: remove PERMANENT_VALUES_INPUT default - Test files: update callers to match new signatures Note: the TypeScript upgrade tooling (upgrade-script-utils.ts, copy-permanent-values.ts, collect-gateway-migration-intervals.ts) still uses upgrade-envs/permanent-values/ — that directory and its contents are intentionally preserved. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…l-ops gateway commands
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
19 participants
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 ❔
Why ❔
Checklist