Add cross-chain options for ERC721, ERC1155, and Governor (Contracts 5.7) - #825
Conversation
…thorityUtils import transpilation
…ors have non-address arguments
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughAdds ERC721/ERC1155 ERC-7786 bridging and Governor cross-chain execution options, wires them through Solidity builders, generators, MCP, AI, CLI, and UI surfaces, updates deployment templates, and upgrades generated compatibility metadata to OpenZeppelin Contracts 5.7.0. ChangesSolidity cross-chain generation
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant SolidityUI
participant MCPTool
participant ContractBuilder
participant GeneratedOutput
User->>SolidityUI: select cross-chain option
SolidityUI->>MCPTool: submit option values
MCPTool->>ContractBuilder: pass ERC or Governor options
ContractBuilder->>GeneratedOutput: add cross-chain extension and deployment wiring
GeneratedOutput-->>User: return Solidity project output
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. |
…and fix import hyperlink version
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/core/solidity/src/zip-foundry.ts (1)
194-230: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winComment out
run()wheninitialOwneris required
args.lengthmisses the transparent-proxyinitialOwnerplaceholder. For contracts with no constructor args, this emits an uncommentedrun()containingaddress initialOwner = <Set initialOwner address here>;, which makes the generated script invalid. IncludeneedsInitialOwnerVariable(c, opts)in the guard.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/solidity/src/zip-foundry.ts` around lines 194 - 230, The getScript function currently only comments out run() when constructor arguments exist, missing the initialOwner placeholder for transparent proxies. Update the run() guard to also account for needsInitialOwnerVariable(c, opts), while preserving the existing addTodoAndCommentOut behavior and deployment flow.
🧹 Nitpick comments (1)
packages/core/solidity/src/generate/erc1155.ts (1)
18-24: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
crossChainLinkAllowOverridenever generates thetruebranch.Both
blueprintandcrossChainBridgingUpgradeableBlueprintfixcrossChainLinkAllowOverride: [false], so the generated compile-coverage matrix never exercises_setLink(gateway, counterpart, true)fromaddERC1155Crosschaininerc1155.ts. Consider covering both values for at least the non-upgradeable variants.♻️ Proposed fix
- crossChainLinkAllowOverride: [false], + crossChainLinkAllowOverride: booleans,Also applies to: 25-47
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/solidity/src/generate/erc1155.ts` around lines 18 - 24, Update the ERC1155 compile-coverage option definitions in blueprint and crossChainBridgingUpgradeableBlueprint so crossChainLinkAllowOverride covers both false and true, ensuring non-upgradeable variants exercise the _setLink(..., true) branch while preserving existing upgradeable coverage.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/core/solidity/src/zip-hardhat.test.ts.md`:
- Around line 625-633: The generated Ignition module templates use undeclared
constructor arguments, so deployment code cannot be evaluated or type-checked.
In packages/core/solidity/src/zip-hardhat.test.ts.md lines 625-633, update the
MyToken deployment call to use typed placeholder values or comment out the call
until links and initialOwner are supplied; apply the same treatment in lines
789-797 for _token and _timelock, preserving the surrounding module structure.
In `@packages/ui/src/solidity/GovernorControls.svelte`:
- Line 263: Update the helpLink value in GovernorControls to reference a
currently published OpenZeppelin governance documentation anchor; if no valid
anchor exists, remove the link rather than retaining the dead
`#GovernorCrosschain` fragment.
---
Outside diff comments:
In `@packages/core/solidity/src/zip-foundry.ts`:
- Around line 194-230: The getScript function currently only comments out run()
when constructor arguments exist, missing the initialOwner placeholder for
transparent proxies. Update the run() guard to also account for
needsInitialOwnerVariable(c, opts), while preserving the existing
addTodoAndCommentOut behavior and deployment flow.
---
Nitpick comments:
In `@packages/core/solidity/src/generate/erc1155.ts`:
- Around line 18-24: Update the ERC1155 compile-coverage option definitions in
blueprint and crossChainBridgingUpgradeableBlueprint so
crossChainLinkAllowOverride covers both false and true, ensuring non-upgradeable
variants exercise the _setLink(..., true) branch while preserving existing
upgradeable coverage.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: ac2fa320-9c7b-4b4c-a183-2570d4bb5ac3
⛔ Files ignored due to path filters (20)
packages/cli/src/cli.test.ts.snapis excluded by!**/*.snappackages/core/confidential/src/erc7984.test.ts.snapis excluded by!**/*.snappackages/core/confidential/src/get-versioned-remappings.test.ts.snapis excluded by!**/*.snappackages/core/confidential/src/zip-hardhat.test.ts.snapis excluded by!**/*.snappackages/core/solidity/src/account.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/get-versioned-remappings.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-polkadot.test.ts.snapis excluded by!**/*.snappackages/core/solidity/src/zip-hardhat.test.ts.snapis excluded by!**/*.snappackages/core/uniswap-hooks/src/get-versioned-remappings.test.ts.snapis excluded by!**/*.snappackages/core/uniswap-hooks/src/hooks.test.ts.snapis excluded by!**/*.snapyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (52)
.changeset/solidity-57-crosschain.mdpackages/cli/src/cli.test.ts.mdpackages/common/src/ai/descriptions/solidity.tspackages/common/src/ai/schemas/solidity.tspackages/core/confidential/src/erc7984.test.ts.mdpackages/core/confidential/src/get-versioned-remappings.test.ts.mdpackages/core/confidential/src/zip-hardhat.test.ts.mdpackages/core/solidity/package.jsonpackages/core/solidity/src/account.test.ts.mdpackages/core/solidity/src/account.tspackages/core/solidity/src/custom.test.ts.mdpackages/core/solidity/src/environments/hardhat/package.jsonpackages/core/solidity/src/environments/hardhat/upgradeable/package.jsonpackages/core/solidity/src/erc1155.test.tspackages/core/solidity/src/erc1155.test.ts.mdpackages/core/solidity/src/erc1155.tspackages/core/solidity/src/erc20.test.ts.mdpackages/core/solidity/src/erc20.tspackages/core/solidity/src/erc721.test.tspackages/core/solidity/src/erc721.test.ts.mdpackages/core/solidity/src/erc721.tspackages/core/solidity/src/generate/erc1155.tspackages/core/solidity/src/generate/erc20.tspackages/core/solidity/src/generate/erc721.tspackages/core/solidity/src/generate/governor.tspackages/core/solidity/src/get-versioned-remappings.test.ts.mdpackages/core/solidity/src/governor.test.tspackages/core/solidity/src/governor.test.ts.mdpackages/core/solidity/src/governor.tspackages/core/solidity/src/stablecoin.test.ts.mdpackages/core/solidity/src/utils/version.tspackages/core/solidity/src/zip-foundry.test.tspackages/core/solidity/src/zip-foundry.test.ts.mdpackages/core/solidity/src/zip-foundry.tspackages/core/solidity/src/zip-hardhat-polkadot.test.ts.mdpackages/core/solidity/src/zip-hardhat.test.tspackages/core/solidity/src/zip-hardhat.test.ts.mdpackages/core/solidity/src/zip-hardhat.tspackages/core/uniswap-hooks/src/get-versioned-remappings.test.ts.mdpackages/core/uniswap-hooks/src/hooks.test.ts.mdpackages/mcp/src/solidity/tools/erc1155.test.tspackages/mcp/src/solidity/tools/erc1155.tspackages/mcp/src/solidity/tools/erc721.test.tspackages/mcp/src/solidity/tools/erc721.tspackages/mcp/src/solidity/tools/governor.test.tspackages/mcp/src/solidity/tools/governor.tspackages/ui/api/ai-assistant/function-definitions/solidity.tspackages/ui/src/solidity/ERC1155Controls.sveltepackages/ui/src/solidity/ERC721Controls.sveltepackages/ui/src/solidity/GovernorControls.sveltepackages/ui/src/solidity/crosschain-incremental-tooltip.tspackages/ui/src/solidity/inject-hyperlinks.ts
Cleanups over the cross-chain bridging change, with no change to generated
output:
- Extract the shared `CrosschainLinked` wiring and the `setLink` function
definition into `set-crosschain-linked.ts`. ERC20, ERC721 and ERC1155 had
three byte-identical copies differing only in the parent extension.
- Drop the single-case `switch` dispatchers in `erc721.ts`/`erc1155.ts`; the
call sites already guard on the option being set.
- Extract `hasNonAddressArgs` and `addTodoAndCommentOut` into `zip-shared.ts`.
The predicate was spelled out four times across the Foundry and Hardhat
generators, and the TODO wording was duplicated between them.
`addTodoAndCommentOut` now derives its wording from the contract instead of
taking a boolean each caller had to re-derive.
- Collapse the two near-identical `getVariables` closures in `zip-foundry.ts`
into one function parameterized by the address initializer, and hoist
`withHelpers` out of the per-argument loop.
- Add a `manualNoticeTooltipProps` factory so the Superchain and crosschain
incremental tooltips share their tippy configuration.
- Hoist the thrice-duplicated `crossChainLinkAllowOverride` description into a
single constant.
- Replace the redundant `let x = false; $: { x = ... }` derived state in the
token controls with a plain reactive assignment, and move the mid-script
imports in `ERC721Controls.svelte` up to the import block.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01V6YuEiAxTBpG1AMfF57Bqy
The single-case `switch` with a `const _: never` default in erc721.ts and erc1155.ts is deliberate: it makes adding a bridging mode a compile error until the new mode is handled here. Removing it as dead code lost that forcing function. The shared `addCrosschainLinked` extraction is kept — `addERC721Crosschain` and `addERC1155Crosschain` remain as the per-kind wrappers the dispatch targets. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V6YuEiAxTBpG1AMfF57Bqy
Merging the two `getVariables` closures left `needsInitialOwnerVariable` and `getLocalVariableType` with one caller each, and I inlined them on that basis. That was the wrong call: those helpers exist to name a non-obvious condition, not to avoid duplication, so inlining traded a readable predicate for a dense inline expression. Both are restored. The merged function body is now the original closure body with only the address initializer swapped for the injected callback, which is the diff this change should have been. This also drops the `withHelpers(c)` hoist out of the loop. That was a negligible saving on a once-per-download path, and keeping the per-argument call avoids invoking it for contracts that have no non-address arguments. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V6YuEiAxTBpG1AMfF57Bqy
|
Thank you for your contribution to OpenZeppelin Contracts Wizard. Before being able to integrate those changes, we would like you to sign our Contributor License Agreement. I confirm that I have read and hereby agree to the OpenZeppelin Contributor License Agreement You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot. |
ernestognw
left a comment
ernestognw
left a comment
There was a problem hiding this comment.
LGTM, few nits after a quick pass. Thanks @ericglau
| * Semantic version string representing of the minimum compatible version of Contracts to display in output. | ||
| */ | ||
| export const compatibleContractsSemver = '^5.6.0'; | ||
| export const compatibleContractsSemver = '^5.7.0'; |
There was a problem hiding this comment.
In inject-hyperlinks.ts, we get the version from openzeppelin-contracts-version.json. Shall we do it here too?
There was a problem hiding this comment.
This is intentionally kept separate. openzeppelin-contracts-version.json is the exact version we build and test against — it resolves to whatever the devDependency range installs, e.g. 5.7.1 from ^5.7.0 — whereas this semver describes the minimum version the generated code needs. Deriving one from the other would raise the stated minimum on every upstream patch release, and we may also want to bundle a newer version while the generated code stays compatible with an older one.
| </ExpandableToggleRadio> | ||
|
|
||
| <ExpandableToggleRadio | ||
| label="Cross-Chain Bridging" |
There was a problem hiding this comment.
We use "cross-chain" in some sections and "crosschain" and others. Shall we standardize to "crosschain"?
There was a problem hiding this comment.
Agreed, we should standardize on "crosschain". Opened #831 to do it across all the tabs, since the mix exists in ERC20's shipped text too and this PR adds more of it.
Note that renaming ERC20's existing crossChain* option keys in the API would be breaking, so we kept the new options aligned with that naming — the same concept spelled two ways across tabs is a footgun for anything consuming the options API, especially LLMs filling these in via the MCP tools and AI assistant. #831 suggests updating user-facing labels but leaves the keys as-is.
Co-authored-by: Ernesto García <ernestognw@gmail.com>
Co-authored-by: Ernesto García <ernestognw@gmail.com>
…zard into 57bridgeable
3 participants
Adds Wizard support for the new cross-chain contracts in OpenZeppelin Contracts 5.7.
New options
crossChainBridging(ERC-7786 Native) embeds a bridge directly in the token viaERC721Crosschain/ERC1155Crosschain— outbound transfers burn, inbound transfers mint. Adds aCrosschainLinked.Link[]constructor argument and an access-controlledsetLinkfunction, with thecrossChainLinkAllowOverridesub-option. Mirrors the existing ERC20erc7786nativeoption.crossChainExecutionadds theGovernorCrosschainextension, letting passed proposals relay execution to other chains through ERC-7786 gateways. TheCrosschainRemoteExecutoron target chains is deployed by the user (not generated); gateway and executor are per-proposal arguments torelayCrosschain.Combining ERC721 auto-increment IDs with bridging is allowed but shows a warning (IDs increment independently per chain; mint on a single chain and link non-minting counterparts to avoid collisions that strand bridged tokens).
Covers core codegen, UI, AI assistant descriptions/schemas/function definitions, MCP tools, and CLI.
Fixes
Upgradeable
crossChainBridgingcombinations were previously excluded from the exhaustive compile matrix; they are now covered via reduced blueprints, which surfaced two existing compile errors in upgradeable ERC20 bridging variants (fixed here):erc7786nativereferenced the untranspiledCrosschainLinked.Link[]in the initializer.customwith managed access transpiled theAuthorityUtilslibrary import to a nonexistent file.Two fixes outside the cross-chain surface:
IERC4337import path dropped itsdraft-prefix in Contracts 5.7, so generated account contracts importinterfaces/IERC4337.sol.@openzeppelin/contractsfrom a hoisted copy innode_modulesand pointed at v5.4.0; they now use the version the Wizard's output is generated against. Affects all Solidity tabs.Additionally, downloaded Foundry and Hardhat projects now compile when constructors have non-address arguments (e.g. cross-chain links, or Governor's token and timelock): the generated test and script files declare typed placeholders and comment out the deployment with a TODO, instead of calling the constructor with missing arguments.
Before merging
@openzeppelin/contractsand@openzeppelin/contracts-upgradeableto^5.7.0(core devDependencies and both Hardhat environments) and regenerate lockfiles/snapshot viaprepareandupdate-envcompatibleContractsSemverto^5.7.0ERC721Crosschain,ERC1155Crosschain, andGovernorCrosschainresolve — the anchors are not published yet; they land with [CI] Update API docs for OpenZeppelin/openzeppelin-contracts v5.7.0 docs#226. The help links already use the correct URLs and currently resolve to the right page without scrolling to the anchor.🤖 Generated with Claude Code