Add ERC-20 option for embedding ERC-7786 based crosschain bridge #747
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 WalkthroughThis PR adds support for ERC-7786 native cross-chain bridging option across OpenZeppelin Wizard for ERC20, stablecoin, and Real World Asset contracts. It introduces a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Comment |
|
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. Ignoring alerts on:
|
packages/core/solidity/src/erc20.ts
Outdated
| args: [ | ||
| { name: 'gateway', type: 'address' }, | ||
| { name: 'counterpart', type: 'bytes memory' }, | ||
| { name: 'allowOverride', type: 'bool' }, |
There was a problem hiding this comment.
I don't think this should be an argument in the public function. The contract has to hardcode a value IIUC. CC @Amxx
There was a problem hiding this comment.
From @Amxx: allowOverride should be false by default so that links are immutable once set.
There was a problem hiding this comment.
Also consider adding a comment explaining the choice of false and when to consider changing it.
There was a problem hiding this comment.
Added an option for this with default as false.
|
I think "ERC-7786 Native" instead of "Embedded" is a clearer and more compelling name. Open to other suggestions involving ERC-7786. |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
![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/hardhat.config.js (1)
32-40: Potential off-by-one bug when chunkingcompilationJobsThe loop condition
i < compilationJobs.length - 1will drop the last job whencompilationJobs.length > CHUNK_SIZE(e.g., length 101 → job at index 100 is never included in any chunk).Consider iterating up to the full length instead:
task(TASK_COMPILE_SOLIDITY_MERGE_COMPILATION_JOBS, async ({ compilationJobs }, _, runSuper) => { const CHUNK_SIZE = 100; const chunks = []; - for (let i = 0; i < compilationJobs.length - 1; i += CHUNK_SIZE) { + for (let i = 0; i < compilationJobs.length; i += CHUNK_SIZE) { chunks.push(compilationJobs.slice(i, i + CHUNK_SIZE)); } const mergedChunks = await Promise.all(chunks.map(cj => runSuper({ compilationJobs: cj }))); return mergedChunks.flat(); });This avoids skipping the tail of the array when there are more than
CHUNK_SIZEjobs.
🧹 Nitpick comments (3)
packages/core/solidity/src/generate/stablecoin.ts (1)
22-23: Stablecoin presets correctly surface crossChainLinkAllowOverrideWiring
crossChainLinkAllowOverrideinto both basic (false) and full (true) presets keeps the generator aligned with the Stablecoin options type and the new ERC-7786-native flow. If, in the future, you want to forbidallowOverridefor non‑erc7786nativemodes, this would be the place to specialize those combinations.Also applies to: 40-42
packages/core/solidity/src/zip-foundry.test.ts.md (1)
394-397: Consider tightening or documenting setTokenBridge behavior
setTokenBridgelets the owner set the bridge to any address (including zero) without emitting an event. If disabling the bridge via zero is not desired, mirroring the non‑zerorequirefrominitializeand/or emitting aTokenBridgeSetevent would make misconfiguration and upgrades easier to audit; otherwise, a short comment that zero is allowed to disable bridging would clarify intent.packages/core/solidity/package.json (1)
31-31: Avoid long‑term dependency on @openzeppelin/contracts masterPointing
@openzeppelin/contractsatgit+https://github.com/OpenZeppelin/openzeppelin-contracts.git#masteris reasonable while you depend on unreleased ERC‑7786 changes, but it makes builds non‑reproducible as master evolves. For anything beyond a short transitional window, consider pinning to a specific commit hash and then switching back to a tagged version once the target release is available.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
packages/core/solidity/src/erc20.test.ts.snapis excluded by!**/*.snappackages/core/solidity/src/zip-foundry.test.ts.snapis excluded by!**/*.snapyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (27)
.changeset/ninety-ears-slide.md(1 hunks).changeset/sad-cases-yawn.md(1 hunks)packages/common/src/ai/descriptions/solidity.ts(1 hunks)packages/core/solidity/hardhat.config.js(1 hunks)packages/core/solidity/package.json(2 hunks)packages/core/solidity/remappings.txt(1 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/erc20.test.ts(1 hunks)packages/core/solidity/src/erc20.test.ts.md(12 hunks)packages/core/solidity/src/erc20.ts(10 hunks)packages/core/solidity/src/generate/erc20.ts(1 hunks)packages/core/solidity/src/generate/stablecoin.ts(2 hunks)packages/core/solidity/src/stablecoin.ts(2 hunks)packages/core/solidity/src/zip-foundry.test.ts.md(5 hunks)packages/core/uniswap-hooks/remappings.txt(1 hunks)packages/mcp/src/solidity/schemas.ts(1 hunks)packages/mcp/src/solidity/tools/erc20.test.ts(2 hunks)packages/mcp/src/solidity/tools/erc20.ts(2 hunks)packages/mcp/src/solidity/tools/rwa.test.ts(2 hunks)packages/mcp/src/solidity/tools/rwa.ts(2 hunks)packages/mcp/src/solidity/tools/stablecoin.test.ts(2 hunks)packages/mcp/src/solidity/tools/stablecoin.ts(2 hunks)packages/ui/api/ai-assistant/function-definitions/solidity.ts(1 hunks)packages/ui/src/solidity/ERC20Controls.svelte(2 hunks)packages/ui/src/solidity/RealWorldAssetControls.svelte(2 hunks)packages/ui/src/solidity/StablecoinControls.svelte(2 hunks)
🧰 Additional context used
🧠 Learnings (16)
📓 Common learnings
Learnt from: ernestognw
Repo: OpenZeppelin/contracts-wizard PR: 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: typicalHuman
Repo: OpenZeppelin/contracts-wizard PR: 715
File: packages/core/solidity/src/stablecoin.ts:74-77
Timestamp: 2025-11-03T12:18:33.779Z
Learning: In packages/core/solidity/src/stablecoin.ts, the ERC20Restricted base contract from openzeppelin/community-contracts provides a default implementation of isUserAllowed that returns true for non-BLOCKED users (blocklist logic). Only allowlist mode needs to override isUserAllowed to check for Restriction.ALLOWED instead. Blocklist mode uses the default implementation without override.
📚 Learning: 2025-11-03T12:18:33.779Z
Learnt from: typicalHuman
Repo: OpenZeppelin/contracts-wizard PR: 715
File: packages/core/solidity/src/stablecoin.ts:74-77
Timestamp: 2025-11-03T12:18:33.779Z
Learning: In packages/core/solidity/src/stablecoin.ts, the ERC20Restricted base contract from openzeppelin/community-contracts provides a default implementation of isUserAllowed that returns true for non-BLOCKED users (blocklist logic). Only allowlist mode needs to override isUserAllowed to check for Restriction.ALLOWED instead. Blocklist mode uses the default implementation without override.
Applied to files:
packages/ui/src/solidity/StablecoinControls.sveltepackages/ui/src/solidity/ERC20Controls.sveltepackages/mcp/src/solidity/tools/rwa.test.tspackages/mcp/src/solidity/tools/stablecoin.tspackages/mcp/src/solidity/tools/erc20.test.tspackages/core/solidity/src/erc20.test.ts.mdpackages/ui/src/solidity/RealWorldAssetControls.sveltepackages/mcp/src/solidity/tools/rwa.tspackages/mcp/src/solidity/tools/stablecoin.test.tspackages/core/solidity/src/generate/stablecoin.tspackages/core/solidity/src/generate/erc20.tspackages/core/solidity/src/stablecoin.tspackages/core/solidity/src/erc20.ts
📚 Learning: 2025-08-15T22:49:25.653Z
Learnt from: ernestognw
Repo: OpenZeppelin/contracts-wizard PR: 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/uniswap-hooks/remappings.txtpackages/core/solidity/package.jsonpackages/core/solidity/src/environments/hardhat/upgradeable/package.jsonpackages/core/solidity/src/environments/hardhat/package.json.changeset/ninety-ears-slide.mdpackages/core/solidity/remappings.txt.changeset/sad-cases-yawn.mdpackages/core/solidity/src/erc20.tspackages/core/solidity/src/zip-foundry.test.ts.md
📚 Learning: 2025-08-19T15:18:09.410Z
Learnt from: ericglau
Repo: OpenZeppelin/contracts-wizard PR: 638
File: packages/core/cairo_alpha/src/scripts/update-scarb-project.ts:62-79
Timestamp: 2025-08-19T15:18:09.410Z
Learning: In the cairo_alpha package (packages/core/cairo_alpha), the OpenZeppelin dependency in test_project/Scarb.toml is expected to be in { git, tag } form rather than a simple registry-style string, so the update script only needs to update the tag value, not convert between formats.
Applied to files:
packages/core/solidity/package.jsonpackages/core/solidity/src/environments/hardhat/upgradeable/package.jsonpackages/core/solidity/src/environments/hardhat/package.jsonpackages/core/solidity/remappings.txt
📚 Learning: 2025-09-12T15:07:08.673Z
Learnt from: ericglau
Repo: OpenZeppelin/contracts-wizard PR: 663
File: packages/core/cairo_alpha/src/custom.test.ts.md:12-12
Timestamp: 2025-09-12T15:07:08.673Z
Learning: In the OpenZeppelin contracts-wizard cairo_alpha package changelog (packages/core/cairo_alpha/CHANGELOG.md), each alpha version gets its own separate entry under the "Unreleased" section rather than updating a single entry. This allows tracking of changes across different alpha releases (e.g., v3.0.0-alpha.0, v3.0.0-alpha.1, v3.0.0-alpha.2 all have separate entries).
Applied to files:
packages/core/solidity/package.jsonpackages/core/solidity/src/environments/hardhat/upgradeable/package.jsonpackages/core/solidity/src/environments/hardhat/package.json.changeset/ninety-ears-slide.mdpackages/core/solidity/remappings.txt
📚 Learning: 2025-10-22T16:23:53.055Z
Learnt from: ericglau
Repo: OpenZeppelin/contracts-wizard PR: 680
File: packages/core/solidity/src/environments/hardhat/polkadot/package.json:17-17
Timestamp: 2025-10-22T16:23:53.055Z
Learning: In packages/core/solidity/src/environments/hardhat/polkadot/package.json, the "license" field is a placeholder that gets dynamically overwritten with the actual contract license when packaging the JSON into the zip file during zip generation.
Applied to files:
packages/core/solidity/package.jsonpackages/core/solidity/src/environments/hardhat/upgradeable/package.jsonpackages/core/solidity/src/environments/hardhat/package.json
📚 Learning: 2025-09-12T20:50:43.101Z
Learnt from: CoveMB
Repo: OpenZeppelin/contracts-wizard PR: 644
File: packages/ui/api/services/open-ai.ts:43-56
Timestamp: 2025-09-12T20:50:43.101Z
Learning: In the OpenZeppelin contracts-wizard project, the current streaming implementation in packages/ui/api/services/open-ai.ts intentionally handles text deltas and function_calls with different formats (raw text chunks vs JSON objects). While this can cause JSON.parse issues, harmonizing the response format is considered outside scope of dependency updates as it requires UI refactoring to handle unified streaming responses.
Applied to files:
packages/core/solidity/package.json
📚 Learning: 2025-08-19T15:21:06.991Z
Learnt from: ericglau
Repo: OpenZeppelin/contracts-wizard PR: 638
File: packages/core/cairo_alpha/src/account.test.ts.md:18-18
Timestamp: 2025-08-19T15:21:06.991Z
Learning: In the OpenZeppelin contracts-wizard repository, the cairo_alpha package (packages/core/cairo_alpha) and the stable cairo package (packages/core/cairo) are separate implementations that do not need to have the same code or matching dependency versions. The cairo_alpha package targets dependencies that are either newer than or the same as the stable cairo package, allowing it to test and support newer Cairo/Scarb/OpenZeppelin versions while the stable version maintains compatibility with stable releases.
Applied to files:
packages/core/solidity/package.jsonpackages/core/solidity/src/environments/hardhat/upgradeable/package.jsonpackages/core/solidity/src/environments/hardhat/package.json
📚 Learning: 2025-09-18T20:17:09.709Z
Learnt from: ericglau
Repo: OpenZeppelin/contracts-wizard PR: 652
File: packages/mcp/src/confidential/schemas.ts:40-46
Timestamp: 2025-09-18T20:17:09.709Z
Learning: MCP tool schemas for confidential contracts can be more restrictive than the core API. For the votes field in confidential fungible tokens, the MCP schema only accepts 'blocknumber' or 'timestamp' strings because undefined behaves functionally equivalent to false, eliminating the need to explicitly accept boolean values in the tool definition.
Applied to files:
packages/mcp/src/solidity/tools/rwa.test.tspackages/mcp/src/solidity/tools/erc20.test.tspackages/mcp/src/solidity/schemas.tspackages/mcp/src/solidity/tools/stablecoin.test.ts
📚 Learning: 2025-08-20T20:23:30.259Z
Learnt from: ericglau
Repo: OpenZeppelin/contracts-wizard PR: 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/environments/hardhat/upgradeable/package.jsonpackages/core/solidity/remappings.txtpackages/core/solidity/src/erc20.test.ts.mdpackages/core/solidity/src/zip-foundry.test.ts.md
📚 Learning: 2025-08-19T01:15:58.945Z
Learnt from: ernestognw
Repo: OpenZeppelin/contracts-wizard PR: 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/core/solidity/remappings.txtpackages/core/solidity/src/erc20.test.ts.mdpackages/core/solidity/src/zip-foundry.test.ts.md
📚 Learning: 2025-08-19T01:15:58.945Z
Learnt from: ernestognw
Repo: OpenZeppelin/contracts-wizard PR: 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/core/solidity/remappings.txtpackages/core/solidity/src/erc20.test.ts.mdpackages/core/solidity/src/zip-foundry.test.ts.md
📚 Learning: 2025-08-21T19:44:06.797Z
Learnt from: ericglau
Repo: OpenZeppelin/contracts-wizard PR: 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/remappings.txtpackages/core/solidity/src/erc20.test.ts.mdpackages/core/solidity/src/zip-foundry.test.ts.md
📚 Learning: 2025-09-18T20:18:23.799Z
Learnt from: ericglau
Repo: OpenZeppelin/contracts-wizard PR: 652
File: packages/ui/api/ai-assistant/function-definitions/confidential.ts:32-38
Timestamp: 2025-09-18T20:18:23.799Z
Learning: In OpenZeppelin Wizard, the AI Assistant defines its own function definitions separately and does not use the MCP tools. This means the AI Assistant function definitions and MCP schemas can have different shapes without causing validation conflicts.
Applied to files:
packages/ui/api/ai-assistant/function-definitions/solidity.ts
📚 Learning: 2025-08-15T22:52:34.129Z
Learnt from: ernestognw
Repo: OpenZeppelin/contracts-wizard PR: 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/erc20.test.ts.mdpackages/core/solidity/src/zip-foundry.test.ts.md
📚 Learning: 2025-08-15T23:23:13.097Z
Learnt from: ernestognw
Repo: OpenZeppelin/contracts-wizard PR: 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/erc20.test.ts.mdpackages/core/solidity/src/erc20.tspackages/core/solidity/src/zip-foundry.test.ts.md
🧬 Code graph analysis (3)
packages/mcp/src/solidity/tools/rwa.test.ts (1)
packages/mcp/src/helpers.test.ts (1)
assertAPIEquivalence(46-88)
packages/ui/api/ai-assistant/function-definitions/solidity.ts (3)
packages/ui/api/ai-assistant/types/helpers.ts (1)
extractStringEnumValues(107-110)packages/core/solidity/src/erc20.ts (1)
CrossChainBridging(20-20)packages/common/src/ai/descriptions/solidity.ts (1)
solidityERC20Descriptions(25-40)
packages/mcp/src/solidity/tools/stablecoin.test.ts (1)
packages/mcp/src/helpers.test.ts (1)
assertAPIEquivalence(46-88)
🔇 Additional comments (34)
packages/core/solidity/hardhat.config.js (1)
61-71:viaIR: trueplacement looks correct; verify compiler/Hardhat support and CI impactAdding
viaIR: trueas a sibling tooptimizerundersolidity.settingsmatches the expected compiler config shape and is consistent with using the IR pipeline for all Solidity compilation here.Please double‑check that:
- The
SOLIDITY_VERSIONin./src/solidity-version.jsonand the Hardhat version in this repo both supportviaIR.- CI still passes across all targets (including any older sample projects) with the IR pipeline enabled globally.
If everything is green in CI, this change seems fine to ship.
packages/core/solidity/src/stablecoin.ts (1)
44-46: Reuse of ERC20 access-control helper looks goodDelegating to
erc20isAccessControlRequiredand then OR-ingrestrictions/freezablekeeps stablecoin in sync with ERC20’s evolving requirements (e.g., new cross-chain modes) while still enforcing its own extension-specific constraints.packages/core/uniswap-hooks/remappings.txt (1)
1-1: @openzeppelin/contracts remapping is consistentPointing the alias to
node_modules/@openzeppelin/contracts/contracts/matches the pattern used in other remapping files and aligns with the import paths in the snapshots.packages/core/solidity/src/zip-foundry.test.ts.md (4)
320-324: Script updated to match new initializer orderingThe Foundry script now encodes
(tokenBridge_, initialOwner, recipient)forMyToken.initialize, which matches the contract signature below; deployment scaffolding stays consistent.
348-348: Inheritance order change is still soundMoving
OwnableUpgradeableearlier in the base list (while keepingInitializablefirst) preserves a valid linearization alongside the ERC20, bridgeable, and UUPS bases; no override conflicts are apparent.
364-375: Initializer now cleanly separates owner and recipientThe new
initialize(address tokenBridge_, address initialOwner, address recipient)plus__Ownable_init(initialOwner)keeps the tokenBridge non‑zero requirement and chain‑specific premint while decoupling ownership from the recipient address. This looks correct.
448-453: Test fixture aligns with new initializer signatureThe UUPS ERC20 test now deploys the proxy using
(tokenBridge_, initialOwner, recipient)in the same order as the updatedinitializefunction, so the snapshot remains consistent end‑to‑end.packages/mcp/src/solidity/tools/rwa.test.ts (1)
41-49: RWA MCP tests correctly exercise crossChainLinkAllowOverrideAdding the
crosschain erc7786native allowOverridescase and wiringcrossChainLinkAllowOverrideinto the “all” scenario ensures the MCP schema and tool stay aligned withStablecoinOptionsand the new ERC‑7786 native bridging behavior.Also applies to: 65-66
packages/mcp/src/solidity/tools/erc20.test.ts (1)
41-49: ERC20 MCP tests cover ERC‑7786 allowOverrides pathThe added
crosschain erc7786native allowOverridestest plus thecrossChainLinkAllowOverride: falsefield in the “all” case give good coverage that the MCP schema exposes the new flag and that it round‑trips correctly througherc20.print.Also applies to: 65-66
packages/core/solidity/remappings.txt (1)
3-3: Core remapping for @openzeppelin/contracts looks correctAdding
@openzeppelin/contracts/=node_modules/@openzeppelin/contracts/contracts/is consistent with the other entries here and with how the generated Solidity imports are written.packages/core/solidity/src/erc20.test.ts (1)
171-206: ERC-7786 native combinations are well coveredThe new snapshot cases exercise the main erc7786native variants (base, allowOverride, access modes, and upgradeable) and look consistent with existing crossChainBridging coverage. No issues from the test wiring side.
packages/core/solidity/src/environments/hardhat/upgradeable/package.json (1)
13-13: Git master dependency is fine as a temporary measurePointing
@openzeppelin/contractsto the master branch matches the PR’s intent to consume unreleased ERC-7786 features. Since this reduces reproducibility, just ensure this is reverted to a tagged release when the corresponding contracts version is published, as already noted in the PR checklist.packages/ui/src/solidity/ERC20Controls.svelte (1)
54-57: UI wiring for ERC-7786 Native and Allow Link Overrides looks correctThe reactive
showAllowOverrideflag and the new ERC-7786 Native radio/checkbox correctly bind toopts.crossChainBridgingandopts.crossChainLinkAllowOverride, with tooltips pointing to the right docs. Behavior is consistent with existing Superchain controls.Also applies to: 195-211
packages/ui/src/solidity/StablecoinControls.svelte (1)
53-56: Stablecoin ERC-7786 Native controls are consistent and correctly boundThe new ERC-7786 Native option and conditional Allow Link Overrides checkbox correctly mirror the ERC20 UI behavior and bind to the expected Stablecoin options. No issues spotted.
Also applies to: 229-245
packages/mcp/src/solidity/tools/stablecoin.test.ts (1)
41-49: MCP Stablecoin tests correctly cover crossChainLinkAllowOverrideThe new erc7786native allowOverrides test and the explicit
crossChainLinkAllowOverride: falsein the “all” case properly exercise the new option through the MCP path and keep the DeepRequired typing in sync withStablecoinOptions. Looks good.Also applies to: 65-65
packages/core/solidity/src/environments/hardhat/package.json (1)
13-13: Consistent switch to contracts master for the non-upgradeable Hardhat envAligning this env with the upgradeable one by using
git+https://github.com/OpenZeppelin/openzeppelin-contracts.git#masteris reasonable for testing unreleased features. Just confirm you’ll move back to a tagged version once the next contracts release is out.packages/mcp/src/solidity/tools/erc20.ts (1)
26-27: ERC20 MCP tool correctly propagates crossChainLinkAllowOverrideThe new
crossChainLinkAllowOverrideparameter is cleanly threaded from the tool input intoERC20Optionsand on toerc20.print, matching the patterns used for other ERC20 options. No issues here.Also applies to: 44-45
packages/core/solidity/src/generate/erc20.ts (1)
23-23: Generator blueprint now accounts for crossChainLinkAllowOverrideAdding
crossChainLinkAllowOverride: [false]keepsgenerateERC20Optionsaligned with the updatedERC20Optionsshape and ensures all generated combinations explicitly set the new flag. This fits well with dedicated tests that cover thetruecase.packages/ui/src/solidity/RealWorldAssetControls.svelte (2)
55-59: LGTM!The reactive block correctly tracks when to show the Allow Link Overrides option based on the ERC-7786 native bridging selection.
236-252: LGTM!The new ERC-7786 Native radio option and conditional Allow Link Overrides checkbox follow the established UI patterns in this file and are consistent with similar implementations in ERC20Controls.svelte and StablecoinControls.svelte.
packages/mcp/src/solidity/tools/rwa.ts (1)
26-26: LGTM!The
crossChainLinkAllowOverrideoption is correctly wired through the destructured parameters and propagated into theStablecoinOptionsobject, consistent with the schema definition whererwaSchema = stablecoinSchema.Also applies to: 45-45
packages/mcp/src/solidity/schemas.ts (1)
66-72: LGTM!The schema correctly extends
crossChainBridgingwith the new'erc7786native'option and adds thecrossChainLinkAllowOverrideboolean field. The inheritance viaerc20SchemaOmitUpgradeable→stablecoinSchema→rwaSchemaensures consistency across all token types.packages/mcp/src/solidity/tools/stablecoin.ts (1)
26-26: LGTM!The
crossChainLinkAllowOverrideoption is correctly wired through the stablecoin tool, consistent with the ERC20 and RWA tool implementations.Also applies to: 45-45
packages/common/src/ai/descriptions/solidity.ts (1)
33-36: LGTM!The descriptions are clear and comprehensive. The
crossChainBridgingdescription now accurately covers all three options (custom, ERC-7786 native, and SuperchainERC20), and the newcrossChainLinkAllowOverridedescription properly documents its conditional applicability..changeset/sad-cases-yawn.md (1)
1-7: Verify version bump type for breaking changes.The changeset uses
patchversion bumps, but the description documents breaking changes (access control requirement, constructor parameter order changes). Based on learnings from this repo, breaking changes with concrete migration paths can use minor bumps instead of major, but apatchseems too conservative for breaking changes.Consider whether this should be a
minorbump instead ofpatchgiven the breaking changes to the custom bridging option.packages/ui/api/ai-assistant/function-definitions/solidity.ts (1)
60-70: LGTM!The new
erc7786nativeoption andcrossChainLinkAllowOverrideproperty are correctly added to the AI function definition. The enum values are properly extracted usingextractStringEnumValues<CrossChainBridging>()maintaining type safety, and the description is correctly sourced from the common descriptions catalog. This is consistent with the existing pattern used for other properties.packages/core/solidity/src/erc20.test.ts.md (4)
723-749: LGTM! New ERC-7786 native bridging test snapshots.The new test snapshots for
erc7786nativebridging correctly demonstrate:
- Import of
ERC20CrosschainandCrosschainLinked- Constructor accepting
CrosschainLinked.Link[] memory links- Access-controlled
setLinkfunction with_setLink(gateway, counterpart, false)by default- Proper inheritance chain with access control (Ownable)
751-777: LGTM! Allow override variant correctly hardcodestrue.The
allowOverridevariant snapshot correctly shows_setLink(gateway, counterpart, true)which aligns with the past review discussion thatallowOverrideshould be hardcoded in the contract rather than passed as a function argument.
549-570: LGTM! Breaking change to custom bridging correctly applied.The custom bridging snapshot now correctly includes:
Ownableimport and inheritance (breaking change as documented)initialOwnerconstructor parameter- New
setTokenBridgepublic function withonlyOwnermodifierThis aligns with the PR objectives stating custom bridging now requires access control.
903-939: Remove or investigate the source ofCrosschainLinkedtype reference.
CrosschainLinkeddoes not exist in OpenZeppelin's official contracts or contracts-upgradeable packages. The review snapshot usesCrosschainLinked.Link[]but imports onlyCrosschainLinkedUpgradeable, which itself appears to be non-existent in OpenZeppelin. Verify whether this is an experimental feature, a custom type for contracts-wizard, or whether the test snapshot should use standard OpenZeppelin cross-chain types likeERC7786ReceiverorCrossChainEnabledinstead.Likely an incorrect or invalid review comment.
packages/core/solidity/src/erc20.ts (4)
390-433:setTokenBridgeonly added forownableaccess mode.The
setTokenBridgefunction is added forownableaccess (both upgradeable and non-upgradeable paths), but not forrolesormanagedaccess modes. Forroles, the bridge is managed viaTOKEN_BRIDGE_ROLEgrants/revokes, and formanaged, via the authority. This is likely intentional since role-based systems don't need a separate setter, but confirm this design choice.
341-357: LGTM! ERC-7786 native bridging implementation.The
addERC20Crosschainfunction correctly:
- Adds the
ERC20Crosschainparent contract- Sets up
CrosschainLinkedwith links constructor argument- Uses
requireAccessControlto protectsetLinkwith theCROSSCHAIN_LINKERrole- Hardcodes
crossChainLinkAllowOverridein the function body as requested in past reviews
551-563: LGTM! New function definitions added.The
setTokenBridgeandsetLinkfunction definitions are correctly structured with appropriate argument types (addressfor tokenBridge,addressandbytes memoryfor gateway/counterpart).
84-92: LGTM! Access control requirement logic updated.The
isAccessControlRequiredfunction correctly includes'erc7786native'alongside'custom'as bridging options that require access control. This ensures the UI and validation logic properly enforces access control for both bridging modes.
|
@SocketSecurity ignore-all |
| import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol";␊ | ||
| ␊ | ||
| contract MyToken is ERC20, ERC20Bridgeable, ERC20Permit {␊ | ||
| contract MyToken is ERC20, ERC20Bridgeable, Ownable, ERC20Permit {␊ |
There was a problem hiding this comment.
What would it take to join the ERC20XXX inheritance and have Ownable at the end. Not really an issue, but it would be cleaner if we could achieve that easily
There was a problem hiding this comment.
I started to do this but realized that would make it have a different order than the parent initializer calls. We document that the parent initializer calls should be in linearization order, and the Upgrades Plugins would give a warning if they are not.
We should aim to eventually stabilize and reorganize these, but it may require a larger change in how the code is generated in order to do this properly.
Opened #750 to track this as an enhancement request.
Co-authored-by: Hadrien Croubois <hadrien.croubois@gmail.com>
3 participants
Overview
In Solidity ERC-20's Cross-Chain Bridging section, adds an ERC-7866 Native option to use
ERC20Crosschain. When this is selected, it also shows a sub-option Allow Link Overrides which defaults tofalse.Both the ERC-7866 Native (
ERC20Crosschain) and Custom (ERC20Bridgeable) bridging options will require access control.Breaking Changes
Custom bridging option now requires access control to allow privileged roles to update the token bridge address after deployment.
Rationale: Tokens may want to update their bridge address to use a
BridgeERC7802, which requires the token address as a constructor argument to set an immutable variable. This implies the token must be deployed before the bridge.New Features
ERC-7786 Native (
ERC20Crosschain) embeds an ERC-7786 based bridge directly in the token contract. This option requires access control to manage crosschain links.false.To do
packages/core/solidity/package.jsonpackages/core/solidity/src/environments/hardhat/package.jsonpackages/core/solidity/src/environments/hardhat/upgradeable/package.jsonyarn.lockpackages/core/solidity/remappings.txtpackages/core/uniswap-hooks/remappings.txt