ERC-7786 based crosschain bridge for ERC-1155 tokens #6281
Conversation
🦋 Changeset detectedLatest commit: 2c7147d The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughThe changes introduce cross-chain bridging support for ERC-1155 tokens via ERC-7786. Three new contract abstractions are added: BridgeERC1155Core defines the core transfer workflow and message processing, BridgeERC1155 implements custodial bridging with approval enforcement and ERC-1155 receipt hook handling, and ERC1155Crosschain extends ERC-1155 with native bridge integration using burn/mint mechanics. Supporting test files and a shared behavior test helper are added to validate cross-chain transfer scenarios, access control, and error handling. Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @contracts/crosschain/bridges/BridgeERC1155.sol:
- Around line 81-97: The NatSpec on onERC1155Received incorrectly claims the
`data` contains a full InteroperableAddress while the function does not use
`data`; update the comment to reflect the actual behavior (e.g., remove the line
about `data` or state that `data` is ignored) or rework the implementation to
parse `data` if intended; locate the onERC1155Received function and either
delete or correct the stale sentence about `data` in the function-level
docstring so documentation matches the code.
- Around line 71-79: The NatSpec comment in the _onReceive function incorrectly
references IERC721Receiver; update the comment to reference IERC1155Receiver
instead. Locate the _onReceive(address to, uint256[] memory ids, uint256[]
memory values) internal virtual override function and change the documentation
line that mentions {IERC721Receiver} to {IERC1155Receiver}, leaving the
implementation (token().safeBatchTransferFrom(...)) unchanged.
- Around line 99-115: The NatSpec on the onERC1155BatchReceived function is
stale because it claims the `data` must contain the `to` InteroperableAddress
but the function does not use `data`; update or remove that sentence in the doc
comment for onERC1155BatchReceived (and any similar comment text) so it
accurately reflects behavior—either remove the note about `data` entirely or
state that `data` is ignored by this contract; refer to the
onERC1155BatchReceived function and its unused `data` parameter when making the
edit.
🧹 Nitpick comments (1)
contracts/crosschain/bridges/BridgeERC1155.sol (1)
19-19: Unused error declaration.
BridgeERC1155Unauthorizedis declared but never used in the contract. Consider removing it or using it in the authorization checks.♻️ Suggested removal
- error BridgeERC1155Unauthorized(address caller);
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
contracts/crosschain/bridges/BridgeERC1155.solcontracts/crosschain/bridges/BridgeERC1155Core.solcontracts/token/ERC1155/extensions/ERC1155Crosschain.soltest/crosschain/BridgeERC1155.behavior.jstest/crosschain/BridgeERC1155.test.jstest/token/ERC1155/extensions/ERC1155Crosschain.test.js
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: Amxx
Repo: OpenZeppelin/openzeppelin-contracts PR: 5914
File: contracts/crosschain/bridges/BridgeERC20.sol:57-58
Timestamp: 2025-10-03T13:14:57.679Z
Learning: In cross-chain bridge contracts like BridgeERC20, when processing incoming messages in _processMessage, avoid validation checks that revert on malformed addresses. Reverting would create cross-chain inconsistency where tokens are locked/burned on the source chain but never minted on the destination. Instead, use best-effort address extraction (e.g., address(bytes20(toBinary))) to maintain atomicity across chains. If tokens are minted to an incorrect address due to user error, recovery may be possible through admin controls rather than leaving funds permanently locked.
📚 Learning: 2025-10-03T13:14:57.679Z
Learnt from: Amxx
Repo: OpenZeppelin/openzeppelin-contracts PR: 5914
File: contracts/crosschain/bridges/BridgeERC20.sol:57-58
Timestamp: 2025-10-03T13:14:57.679Z
Learning: In cross-chain bridge contracts like BridgeERC20, when processing incoming messages in _processMessage, avoid validation checks that revert on malformed addresses. Reverting would create cross-chain inconsistency where tokens are locked/burned on the source chain but never minted on the destination. Instead, use best-effort address extraction (e.g., address(bytes20(toBinary))) to maintain atomicity across chains. If tokens are minted to an incorrect address due to user error, recovery may be possible through admin controls rather than leaving funds permanently locked.
Applied to files:
test/token/ERC1155/extensions/ERC1155Crosschain.test.jscontracts/token/ERC1155/extensions/ERC1155Crosschain.soltest/crosschain/BridgeERC1155.behavior.jscontracts/crosschain/bridges/BridgeERC1155Core.soltest/crosschain/BridgeERC1155.test.jscontracts/crosschain/bridges/BridgeERC1155.sol
📚 Learning: 2025-08-29T13:16:08.640Z
Learnt from: Amxx
Repo: OpenZeppelin/openzeppelin-contracts PR: 5904
File: contracts/mocks/crosschain/ERC7786RecipientMock.sol:12-14
Timestamp: 2025-08-29T13:16:08.640Z
Learning: In OpenZeppelin contracts, mock contracts (like ERC7786RecipientMock) don't require input validation such as zero-address checks in constructors, as they are only used for testing purposes in controlled environments.
Applied to files:
test/crosschain/BridgeERC1155.behavior.jscontracts/crosschain/bridges/BridgeERC1155Core.sol
🧬 Code graph analysis (3)
test/token/ERC1155/extensions/ERC1155Crosschain.test.js (4)
test/crosschain/BridgeERC1155.behavior.js (3)
require(1-1)require(2-2)require(3-3)test/crosschain/BridgeERC1155.test.js (7)
require(1-1)require(2-2)require(3-3)require(5-5)require(6-6)require(8-8)chain(11-11)test/crosschain/ERC7786Recipient.test.js (1)
getLocalChain(15-15)test/helpers/account.js (1)
impersonate(7-12)
test/crosschain/BridgeERC1155.behavior.js (2)
test/crosschain/BridgeERC1155.test.js (6)
require(1-1)require(2-2)require(3-3)require(5-5)require(6-6)require(8-8)test/token/ERC1155/extensions/ERC1155Crosschain.test.js (6)
require(1-1)require(2-2)require(3-3)require(5-5)require(6-6)require(8-8)
test/crosschain/BridgeERC1155.test.js (3)
test/token/ERC1155/extensions/ERC1155Crosschain.test.js (6)
chain(11-11)gateway(15-15)tokenA(19-19)bridgeA(20-20)tokenB(23-26)bridgeB(27-27)test/crosschain/ERC7786Recipient.test.js (1)
getLocalChain(15-15)test/helpers/account.js (1)
impersonate(7-12)
🪛 Gitleaks (8.30.0)
test/crosschain/BridgeERC1155.behavior.js
[high] 164-164: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 181-181: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
⏰ 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). (9)
- GitHub Check: Redirect rules - solidity-contracts
- GitHub Check: Header rules - solidity-contracts
- GitHub Check: Pages changed - solidity-contracts
- GitHub Check: tests-upgradeable
- GitHub Check: slither
- GitHub Check: tests
- GitHub Check: tests-foundry
- GitHub Check: coverage
- GitHub Check: halmos
🔇 Additional comments (22)
contracts/crosschain/bridges/BridgeERC1155Core.sol (5)
1-21: LGTM!Contract setup, imports, and documentation are well-structured. The NatSpec clearly explains the two required hook implementations.
23-36: LGTM!Events are properly defined with appropriate indexing for efficient log filtering. The
sendId/receiveIdand address fields are indexed for query performance.
42-61: LGTM!The cross-chain transfer workflow is correct: calls
_onSendfirst to lock/burn tokens, then constructs and sends the message, and emits the event with the returnedsendId.
64-80: LGTM!The best-effort address extraction at line 75 (
address(bytes20(toBinary))) correctly avoids reverting on malformed addresses, maintaining cross-chain atomicity as per the established pattern. Based on learnings, reverting here would cause tokens to be locked/burned on the source chain without being minted on the destination.
82-86: LGTM!Abstract hooks are properly declared with clear documentation of their purpose.
contracts/token/ERC1155/extensions/ERC1155Crosschain.sol (4)
1-16: LGTM!The contract structure and documentation clearly explain the compatibility with both
ERC1155Crosschaininstances and legacyERC1155tokens bridged viaBridgeERC1155.
33-42: LGTM!The batch variant correctly enforces the same authorization check before delegating to
_crosschainTransfer.
44-52: LGTM!The burn/mint implementations for
_onSendand_onReceiveare correct and follow the established pattern for non-custodial cross-chain bridges.
18-31: No action needed. The_checkAuthorizedfunction exists in the ERC1155 base contract and is properly inherited by ERC1155Crosschain. It correctly uses the standardisApprovedForAllmechanism for authorization checks, making this implementation consistent with OpenZeppelin's ERC1155 design.contracts/crosschain/bridges/BridgeERC1155.sol (2)
21-28: LGTM!Constructor and token getter are correctly implemented with immutable storage.
30-64: LGTM!The
crosschainTransferFromfunctions correctly replicate the ERC1155 authorization pattern and delegate to the core transfer logic.test/crosschain/BridgeERC1155.test.js (2)
10-35: LGTM!The fixture correctly sets up the test environment with:
- A mock gateway with impersonated EOA for direct message delivery
- Chain A with legacy ERC1155 and custodial bridge
- Chain B with native cross-chain ERC1155
- Proper link registration with event assertion
37-47: LGTM!Test structure correctly uses
beforeEachwithloadFixtureand runs the shared behavior suite with appropriate flags for the custodial bridge scenario.test/token/ERC1155/extensions/ERC1155Crosschain.test.js (2)
10-35: LGTM!The fixture correctly sets up both chains with native cross-chain ERC1155 tokens (burn/mint pattern). The link registration is properly validated.
37-43: LGTM!Test correctly invokes the shared behavior suite without custodial flags, matching the burn/mint nature of both bridges.
test/crosschain/BridgeERC1155.behavior.js (7)
1-17: LGTM!Good setup with helper constants and the
encodePayloadhelper function for constructing cross-chain message payloads.
19-28: LGTM!The bridge setup test properly validates bidirectional link configuration.
30-107: LGTM!Excellent end-to-end test covering bidirectional cross-chain transfers. The test correctly parameterizes expectations based on
chainAIsCustodialandchainBIsCustodialflags to handle both custody (TransferBatch to bridge) and burn/mint (TransferBatch to zero address) scenarios.
109-146: LGTM!Allowance tests properly verify both owner-initiated and delegated transfers.
148-184: Static analysis false positives - no action needed.The Gitleaks warnings on lines 164 and 181 are false positives. The strings
'ERC1155MissingApprovalForAll'and'ERC1155InsufficientBalance'are Solidity custom error names, not API keys.
186-242: LGTM!Restriction tests comprehensively cover gateway authorization, counterpart validation, and replay protection. The custodial-aware token seeding at lines 221-224 correctly handles the bridge's receive hook restrictions.
244-280: LGTM!Reconfiguration tests properly verify link updates, override protection, and gateway validation.
frangio
left a comment
frangio
left a comment
There was a problem hiding this comment.
Most of these comments apply to all previous bridge PRs.
| return | ||
| msg.sender == address(_token) && operator == address(this) | ||
| ? IERC1155Receiver.onERC1155BatchReceived.selector | ||
| : bytes4(0xffffffff); |
There was a problem hiding this comment.
Does 0xffffffff have any specific significance? Wondering why not just return 0.
| bytes calldata payload | ||
| ) internal virtual override { | ||
| // split payload | ||
| (bytes memory from, bytes memory toBinary, uint256[] memory ids, uint256[] memory values) = abi.decode( |
There was a problem hiding this comment.
I think the *Binary naming convention is already in use in other contracts but I've always found it confusing/ambiguous since ERC-7930 are binary interoperable addresses.
I think a much better naming convention would be *Evm. What do you think?
|
|
||
| /// @inheritdoc ERC7786Recipient | ||
| function _processMessage( | ||
| address /*gateway*/, |
There was a problem hiding this comment.
I feel like this should have a note saying where the gateway is validated. (This probably applies to the other bridges.)
| /** | ||
| * @dev Base contract for bridging ERC-1155 between chains using an ERC-7786 gateway. | ||
| * | ||
| * In order to use this contract, two functions must be implemented to link it to the token: |
There was a problem hiding this comment.
One point that isn't mentioned is that you also need to implement external crosschainTransfer functions.
2 participants
Followup to #5659
Fixes #5902
PR Checklist
npx changeset add)