-
Couldn't load subscription status.
- Fork 179
[Solidity] Upgrade @openzeppelin/contracts to 5.5.0-rc.0 #681
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughDependency updates to OpenZeppelin Contracts 5.5.0-rc.0 and Hardhat; renaming signer option from ERC7702 to EIP7702 across core, tests, and UI; snapshot and import-path updates for OZ 5.5.0-rc.0; configuration tweaks for upgradeable handling and compatible version constant. Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
All alerts resolved. Learn more about Socket for GitHub. This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. |
| "@nomicfoundation/hardhat-toolbox": "^6.1.0", | ||
| "hardhat": "^2.16.1" | ||
| "@openzeppelin/contracts": "^5.5.0-rc.0", | ||
| "hardhat": "^2.22.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
5.5.0-rc.0 requires Cancun support for mcopy. This is supported by hardhat since 2.22.0.
| "@openzeppelin/contracts": "^5.5.0-rc.0", | ||
| "@openzeppelin/contracts-upgradeable": "^5.5.0-rc.0", | ||
| "@openzeppelin/hardhat-upgrades": "^3.0.0", | ||
| "hardhat": "^2.22.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 9
🧹 Nitpick comments (4)
packages/core/solidity/src/zip-hardhat.test.ts.md (2)
58-61: Consider pinning RC versions for deterministic snapshots.Using caret with pre-releases can drift to newer rc tags and cause snapshot churn. For the zipped sample, pin "@openzeppelin/contracts(-upgradeable)" to "5.5.0-rc.0" to avoid flakiness, unless you intentionally want auto-drift.
Please confirm your desired policy for RC ranges in sample package.json files (pin vs caret).
Also applies to: 131-134, 206-211
169-176: Use upgradeable-variant imports for Initializable and UUPSUpgradeable
Both contracts are exposed in v5.5.0-rc.0 under@openzeppelin/contractsand@openzeppelin/contracts-upgradeable; upgradeable templates should import them from@openzeppelin/contracts-upgradeable/proxy/utils/{Initializable,UUPSUpgradeable}.solto include initializer modifiers and storage gaps.packages/ui/src/solidity/AccountControls.svelte (1)
28-35: Auto-clear upgradeability when selecting EIP7702 to keep state coherent.UI disables the section but keeps any previously selected upgradeability in opts. To avoid invalid state leaking into generation, clear it when EIP7702 is chosen.
$: { - if (opts.signer === 'EIP7702') { + if (opts.signer === 'EIP7702') { + // Ensure incompatible state is cleared + opts.upgradeable = false; upgradeNotSupported = true; upgradeNotSupportedReason = 'EOAs can upgrade by redelegating to a new account'; } else { upgradeNotSupported = false; upgradeNotSupportedReason = ''; } }packages/core/solidity/src/erc721.test.ts.md (1)
560-565: Use upgradeable package imports for both Initializable and UUPSUpgradeable
Both contracts exist in “@openzeppelin/contracts” and “@openzeppelin/contracts-upgradeable” in v5.5.0-rc.0; for upgradeable implementations import from@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.soland@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.solfor consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (13)
packages/core/solidity/src/account.test.ts.snapis excluded by!**/*.snappackages/core/solidity/src/contract.test.ts.snapis excluded by!**/*.snappackages/core/solidity/src/custom.test.ts.snapis excluded by!**/*.snappackages/core/solidity/src/environments/hardhat/package-lock.jsonis excluded by!**/package-lock.jsonpackages/core/solidity/src/environments/hardhat/upgradeable/package-lock.jsonis excluded by!**/package-lock.jsonpackages/core/solidity/src/erc1155.test.ts.snapis excluded by!**/*.snappackages/core/solidity/src/erc20.test.ts.snapis excluded by!**/*.snappackages/core/solidity/src/erc721.test.ts.snapis excluded by!**/*.snappackages/core/solidity/src/governor.test.ts.snapis excluded by!**/*.snappackages/core/solidity/src/stablecoin.test.ts.snapis excluded by!**/*.snappackages/core/solidity/src/zip-foundry.test.ts.snapis excluded by!**/*.snappackages/core/solidity/src/zip-hardhat.test.ts.snapis excluded by!**/*.snapyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (19)
packages/core/solidity/package.json(2 hunks)packages/core/solidity/src/account.test.ts(1 hunks)packages/core/solidity/src/contract.test.ts.md(19 hunks)packages/core/solidity/src/custom.test.ts.md(10 hunks)packages/core/solidity/src/environments/hardhat/package.json(1 hunks)packages/core/solidity/src/environments/hardhat/upgradeable/package.json(1 hunks)packages/core/solidity/src/erc1155.test.ts.md(14 hunks)packages/core/solidity/src/erc20.test.ts.md(43 hunks)packages/core/solidity/src/erc721.test.ts.md(23 hunks)packages/core/solidity/src/generate/account.ts(1 hunks)packages/core/solidity/src/get-imports.test.ts(2 hunks)packages/core/solidity/src/governor.test.ts.md(20 hunks)packages/core/solidity/src/set-upgradeable.ts(2 hunks)packages/core/solidity/src/signer.ts(4 hunks)packages/core/solidity/src/stablecoin.test.ts.md(20 hunks)packages/core/solidity/src/utils/version.ts(1 hunks)packages/core/solidity/src/zip-foundry.test.ts.md(21 hunks)packages/core/solidity/src/zip-hardhat.test.ts.md(6 hunks)packages/ui/src/solidity/AccountControls.svelte(2 hunks)
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
Learnt from: ernestognw
PR: OpenZeppelin/contracts-wizard#609
File: .changeset/sour-hats-grow.md:2-6
Timestamp: 2025-08-15T22:49:25.653Z
Learning: In OpenZeppelin contracts-wizard, breaking changes that have concrete migration paths (like dependency migrations from Community Contracts to OpenZeppelin Contracts) can be handled as minor version bumps instead of major bumps, per maintainer ernestognw's versioning policy.
Learnt from: ernestognw
PR: OpenZeppelin/contracts-wizard#609
File: packages/core/solidity/src/signer.ts:45-49
Timestamp: 2025-08-19T01:15:58.945Z
Learning: The OpenZeppelin contracts-upgradeable package includes upgradeable versions of signer contracts like SignerECDSAUpgradeable, SignerP256Upgradeable, SignerRSAUpgradeable, MultiSignerERC7913Upgradeable, and MultiSignerERC7913WeightedUpgradeable in the contracts/utils/cryptography/signers directory.
Learnt from: ernestognw
PR: OpenZeppelin/contracts-wizard#609
File: packages/core/solidity/src/signer.ts:45-49
Timestamp: 2025-08-19T01:15:58.945Z
Learning: All OpenZeppelin signer contracts have upgradeable variants available in the contracts-upgradeable package: SignerECDSAUpgradeable, SignerP256Upgradeable, SignerRSAUpgradeable, MultiSignerERC7913Upgradeable, and MultiSignerERC7913WeightedUpgradeable all exist in contracts/utils/cryptography/signers/ directory.
📚 Learning: 2025-08-15T23:23:13.097Z
Learnt from: ernestognw
PR: OpenZeppelin/contracts-wizard#609
File: packages/core/solidity/src/signer.ts:31-38
Timestamp: 2025-08-15T23:23:13.097Z
Learning: In OpenZeppelin contracts-wizard, the `setUpgradeableAccount` function deliberately sets `c.upgradeable = false` after upgradeable setup to exclude EIP712Upgradeable and ERC7739Upgradeable variants (avoiding per-call SLOAD overhead). This architectural pattern necessitates manual `_disableInitializers()` setup in both account.ts and signer.ts since the standard upgradeable constructor logic doesn't apply when upgradeability is disabled post-setup.
Applied to files:
packages/core/solidity/src/generate/account.tspackages/ui/src/solidity/AccountControls.sveltepackages/core/solidity/src/signer.tspackages/core/solidity/src/erc721.test.ts.mdpackages/core/solidity/src/set-upgradeable.tspackages/core/solidity/src/zip-hardhat.test.ts.mdpackages/core/solidity/src/zip-foundry.test.ts.mdpackages/core/solidity/src/governor.test.ts.mdpackages/core/solidity/src/erc1155.test.ts.mdpackages/core/solidity/src/account.test.tspackages/core/solidity/src/erc20.test.ts.mdpackages/core/solidity/src/custom.test.ts.md
📚 Learning: 2025-08-19T01:15:58.945Z
Learnt from: ernestognw
PR: OpenZeppelin/contracts-wizard#609
File: packages/core/solidity/src/signer.ts:45-49
Timestamp: 2025-08-19T01:15:58.945Z
Learning: All OpenZeppelin signer contracts have upgradeable variants available in the contracts-upgradeable package: SignerECDSAUpgradeable, SignerP256Upgradeable, SignerRSAUpgradeable, MultiSignerERC7913Upgradeable, and MultiSignerERC7913WeightedUpgradeable all exist in contracts/utils/cryptography/signers/ directory.
Applied to files:
packages/ui/src/solidity/AccountControls.sveltepackages/core/solidity/src/signer.tspackages/core/solidity/src/erc721.test.ts.mdpackages/core/solidity/src/zip-foundry.test.ts.mdpackages/core/solidity/src/account.test.tspackages/core/solidity/src/erc20.test.ts.md
📚 Learning: 2025-08-19T01:15:58.945Z
Learnt from: ernestognw
PR: OpenZeppelin/contracts-wizard#609
File: packages/core/solidity/src/signer.ts:45-49
Timestamp: 2025-08-19T01:15:58.945Z
Learning: The OpenZeppelin contracts-upgradeable package includes upgradeable versions of signer contracts like SignerECDSAUpgradeable, SignerP256Upgradeable, SignerRSAUpgradeable, MultiSignerERC7913Upgradeable, and MultiSignerERC7913WeightedUpgradeable in the contracts/utils/cryptography/signers directory.
Applied to files:
packages/ui/src/solidity/AccountControls.sveltepackages/core/solidity/src/signer.tspackages/core/solidity/src/erc721.test.ts.mdpackages/core/solidity/src/zip-foundry.test.ts.mdpackages/core/solidity/src/account.test.tspackages/core/solidity/src/erc20.test.ts.md
📚 Learning: 2025-08-15T22:49:25.653Z
Learnt from: ernestognw
PR: OpenZeppelin/contracts-wizard#609
File: .changeset/sour-hats-grow.md:2-6
Timestamp: 2025-08-15T22:49:25.653Z
Learning: In OpenZeppelin contracts-wizard, breaking changes that have concrete migration paths (like dependency migrations from Community Contracts to OpenZeppelin Contracts) can be handled as minor version bumps instead of major bumps, per maintainer ernestognw's versioning policy.
Applied to files:
packages/core/solidity/src/environments/hardhat/upgradeable/package.jsonpackages/core/solidity/src/erc721.test.ts.mdpackages/core/solidity/src/zip-hardhat.test.ts.mdpackages/core/solidity/src/zip-foundry.test.ts.mdpackages/core/solidity/src/custom.test.ts.md
📚 Learning: 2025-08-20T20:23:30.259Z
Learnt from: ericglau
PR: OpenZeppelin/contracts-wizard#609
File: packages/core/solidity/src/set-upgradeable.ts:0-0
Timestamp: 2025-08-20T20:23:30.259Z
Learning: In OpenZeppelin contracts-wizard, upgradeable contracts use two different strategies for Initializable imports: Account contracts directly import from contracts-upgradeable/proxy/utils/Initializable.sol for explicit clarity, while other upgradeable contracts (ERC20, ERC721, Governor, etc.) use helpers to automatically transpile imports to upgradeable versions. This architectural separation is intentional due to the special ERC-4337 requirements of account contracts.
Applied to files:
packages/core/solidity/src/erc721.test.ts.mdpackages/core/solidity/src/set-upgradeable.tspackages/core/solidity/src/zip-hardhat.test.ts.mdpackages/core/solidity/src/zip-foundry.test.ts.mdpackages/core/solidity/src/get-imports.test.tspackages/core/solidity/src/governor.test.ts.mdpackages/core/solidity/src/erc1155.test.ts.mdpackages/core/solidity/src/erc20.test.ts.mdpackages/core/solidity/src/custom.test.ts.md
📚 Learning: 2025-08-21T19:44:06.797Z
Learnt from: ericglau
PR: OpenZeppelin/contracts-wizard#609
File: packages/core/solidity/src/account.ts:191-204
Timestamp: 2025-08-21T19:44:06.797Z
Learning: Initializable is available in both openzeppelin/contracts and openzeppelin/contracts-upgradeable packages, so conditional imports like `openzeppelin/${opts.upgradeable ? 'contracts-upgradeable' : 'contracts'}/proxy/utils/Initializable.sol` are valid and will resolve correctly.
Applied to files:
packages/core/solidity/src/erc721.test.ts.mdpackages/core/solidity/src/set-upgradeable.tspackages/core/solidity/src/zip-hardhat.test.ts.mdpackages/core/solidity/src/zip-foundry.test.ts.mdpackages/core/solidity/src/get-imports.test.tspackages/core/solidity/src/governor.test.ts.mdpackages/core/solidity/src/erc20.test.ts.mdpackages/core/solidity/src/custom.test.ts.md
📚 Learning: 2025-08-15T22:52:34.129Z
Learnt from: ernestognw
PR: OpenZeppelin/contracts-wizard#609
File: packages/core/solidity/src/account.ts:89-0
Timestamp: 2025-08-15T22:52:34.129Z
Learning: In OpenZeppelin contracts-wizard, non-upgradeable accounts still require initializer logic (Initializable, _disableInitializers(), and initialize() function) because ERC-4337 accounts are typically deployed by factories as minimal clone proxies, which cannot use constructors effectively for initialization. This is the intended deployment pattern for ERC-4337 account abstraction, even for non-upgradeable accounts.
Applied to files:
packages/core/solidity/src/erc721.test.ts.mdpackages/core/solidity/src/set-upgradeable.tspackages/core/solidity/src/zip-hardhat.test.ts.mdpackages/core/solidity/src/zip-foundry.test.ts.mdpackages/core/solidity/src/governor.test.ts.mdpackages/core/solidity/src/erc1155.test.ts.mdpackages/core/solidity/src/erc20.test.ts.mdpackages/core/solidity/src/custom.test.ts.md
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: boostsecurity - boostsecurityio/semgrep-pro
- GitHub Check: mcp
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (26)
packages/core/solidity/src/contract.test.ts.md (1)
11-13: Snapshots updated to OZ ^5.5.0-rc.0 — LGTMHeader bump is consistent with the version constant and dependency updates.
Also applies to: 23-25, 35-37, 49-51, 64-66, 79-81, 95-97, 107-109, 127-129, 151-153, 164-166, 179-181, 194-196, 211-213, 226-228, 242-244, 255-257, 269-271, 281-283
packages/core/solidity/src/utils/version.ts (1)
4-4: Consistent with the RC bumpUpdating
compatibleContractsSemverto^5.5.0-rc.0matches the snapshots and deps. If the plan is to switch to the final 5.5.0 tag on release day, consider tracking a follow-up to drop the-rc.0suffix then.packages/core/solidity/src/stablecoin.test.ts.md (1)
11-13: Stablecoin snapshots align with OZ ^5.5.0-rc.0 and pinned Community Contracts commit — LGTMNo functional diffs beyond headers.
Also applies to: 27-29, 45-47, 84-86, 127-129, 166-167, 206-207, 227-228, 243-244, 297-298, 314-315, 329-331, 365-366, 411-412, 457-458, 493-494, 529-530, 574-575, 591-592
packages/core/solidity/src/get-imports.test.ts (3)
41-41: Bytes.sol added — OKExpected with OZ 5.5 where ERC721Utils depends on Bytes.
81-81: New Bytes and LowLevelCall imports — OKAligns with OZ 5.5 internal refactors (LowLevelCall split).
Also applies to: 83-83
74-75: Ignore non-upgradeable proxy utils imports
get-importsintentionally emits@openzeppelin/contracts/proxy/utils/...paths forInitializable.solandUUPSUpgradeable.solin both modes and relies on the upgradeable‐transformation step inset-upgradeable.tsto swap in the-upgradeablevariants. No changes needed.Likely an incorrect or invalid review comment.
packages/core/solidity/src/erc1155.test.ts.md (3)
12-12: Compatibility banner bumped to ^5.5.0-rc.0 — OKSnapshot headers uniformly updated.
Also applies to: 35-35, 58-58, 95-95, 118-118, 135-135, 159-159, 200-200, 237-237, 290-290, 327-327, 360-360, 444-444, 537-537
367-367: Cannot verify compilation due to npm errors
The Hardhat compile step failed withENOTEMPTYand one environment is missing@openzeppelin/contracts-upgradeable. Ensurepackages/core/solidity/src/environments/hardhat/package.jsonlists both@openzeppelin/contractsand@openzeppelin/contracts-upgradeable, then run:npm ci --force npx hardhat compileIf it still fails, please share that
package.jsonand yourhardhat.config.tsfor further troubleshooting.
374-374: Confirm plugin supports –reachable variant — Verify that your @openzeppelin/hardhat-upgrades version (in your project’s package.json) supports theoz-upgrades-unsafe-allow-reachableconstructor annotation. Applies also at lines 460 and 549.packages/core/solidity/src/governor.test.ts.md (1)
12-12: Compatibility banner bumped to ^5.5.0-rc.0 — OKConsistent across governor snapshots.
Also applies to: 102-102, 189-189, 281-281, 373-373, 462-462, 557-557, 909-909, 995-995, 1085-1085, 1171-1171, 1257-1257, 1346-1346, 1432-1432, 1521-1521, 1607-1607
packages/core/solidity/src/environments/hardhat/package.json (1)
13-14: Approve Hardhat and OpenZeppelin version bumps. Versions^5.5.0-rc.0and^2.22.0match those used in the upgradeable environment.packages/core/solidity/src/environments/hardhat/upgradeable/package.json (1)
13-16: Dependencies validated
No^5.4.xreferences remain, and@openzeppelin/[email protected]’s peer dependency on Hardhat^2.0.2is satisfied by Hardhat^2.22.0.packages/core/solidity/src/generate/account.ts (1)
11-11: Signer list update LGTM.EIP7702 added and ordering aligns with the test matrix. No further action.
packages/core/solidity/src/erc20.test.ts.md (4)
12-12: Version header update looks goodHeader correctly targets OZ ^5.5.0-rc.0; same change elsewhere in this file is fine.
863-863: Initializable import and reachable-constructor tag
- Importing Initializable from @openzeppelin/contracts is valid in v5.x.
- The “@Custom:oz-upgrades-unsafe-allow-reachable constructor” tag is the right form for reachable constructors; please confirm Foundry Upgrades recognizes it in your pipeline. (Based on learnings)
Also applies to: 869-869
900-900: Same notes for superchain upgradeable variantNon-upgradeable Initializable import and the reachable-constructor tag look correct; verify plugin compatibility passes.
Also applies to: 906-906
1035-1038: UUPS import source changeImporting UUPSUpgradeable from @openzeppelin/contracts/proxy/utils is acceptable in v5.x; no issue spotted.
Also applies to: 1046-1049
packages/core/solidity/src/signer.ts (1)
6-6: Rename to EIP7702: check downstream referencesThe option rename (string literal), switch-case label, and OptionsError key (‘eip7702’) look consistent here. Please verify any UI, schema, or error-handling code that consumed ‘ERC7702’/‘erc7702’ has been updated accordingly.
Also applies to: 16-22, 67-67
packages/core/solidity/src/zip-foundry.test.ts.md (8)
40-41: Foundry install linePinning to @vX.Y.Z-rc.0 is aligned with the PR scope; ensure this token resolves to v5.5.0-rc.0 in tests.
333-347: Upgradeable example imports and reachable-constructor tag
- Using Initializable/UUPSUpgradeable from @openzeppelin/contracts is valid in v5.x.
- Reachable-constructor tag looks correct; confirm validations pass under openzeppelin-foundry-upgrades.
Also applies to: 352-352
479-480: contracts-upgradeable install pinMake sure contracts and contracts-upgradeable are pinned to the same rc to keep submodule versions aligned with remappings (as per Foundry upgrades docs). (docs.openzeppelin.com)
574-587: UUPS minimal example header and tagHeader bump and reachable-constructor tag look good.
761-771: ERC721 UUPS exampleInitializable/UUPS imports from base package + reachable-constructor tag are fine in v5.x.
846-847: ERC1155 basic install lineOK to pin contracts to rc here; same note about ensuring token resolves to 5.5.0-rc.0.
1092-1101: ERC1155 transparent exampleHeader and reachable-constructor tag LGTM.
1175-1175: Compatibility headerHeader updated to ^5.5.0-rc.0; looks good here and throughout.
| ␊ | ||
| contract MyGovernor is Initializable, GovernorUpgradeable, GovernorCountingSimpleUpgradeable, GovernorVotesUpgradeable, GovernorVotesQuorumFractionUpgradeable, GovernorTimelockControlUpgradeable, UUPSUpgradeable {␊ | ||
| /// @custom:oz-upgrades-unsafe-allow constructor␊ | ||
| /// @custom:oz-upgrades-unsafe-allow-reachable constructor␊ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
“reachable constructor” annotation — OK
Matches updated guidance; just ensure docs and codegen comments elsewhere use the same wording.
🏁 Script executed:
#!/bin/bash
# Check for legacy “oz-upgrades-unsafe-allow constructor” occurrences
rg -n 'oz-upgrades-unsafe-allow constructor'Length of output: 418
Replace legacy “oz-upgrades-unsafe-allow constructor” annotations
Ensure all docs and codegen comments use @custom:oz-upgrades-unsafe-allow-reachable constructor instead of the old form:
- packages/core/solidity/src/zip-hardhat.test.ts.md:172
- packages/core/solidity/src/custom.test.ts.md:66
- packages/core/solidity/src/print.ts:92, 127
🤖 Prompt for AI Agents
In packages/core/solidity/src/governor.test.ts.md around line 1621, replace the
legacy oz-upgrades annotation with the new form by changing any occurrence of
"oz-upgrades-unsafe-allow constructor" (or variants) to
"@custom:oz-upgrades-unsafe-allow-reachable constructor"; update the comment
line exactly to use the new @custom:... token so docs/codegen pick up the
corrected annotation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Will approve after 5.5.0 is released and we update this to point to the full release.
Fixes #660