Add CrosschainRemoteExecutor and CrosschainRemoteController #6272
Conversation
🦋 Changeset detectedLatest commit: 415f5d8 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 |
WalkthroughThis pull request introduces two new abstract smart contracts for cross-chain communication: CrosschainRemoteController manages cross-chain linkage by storing gateway and counterpart addresses per chain and dispatching cross-chain messages through registered gateways, while CrosschainRemoteExecutor extends ERC7786Recipient to receive and process incoming cross-chain messages by decoding mode-encoded payloads and executing single calls, batch calls, or delegatecalls. A mock contract and comprehensive test suite are also added to support development and validation. Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ 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: 1
🤖 Fix all issues with AI agents
In @contracts/crosschain/CrosschainRemoteController.sol:
- Around line 26-33: Add an explicit validation in _crosschainExecute: call
getLink(chain) as before, then require that gateway != address(0) (and
optionally counterpart.length > 0) and revert with a descriptive error such as
LinkNotRegistered(chain) (from CrosschainLinked) before calling
IERC7786GatewaySource(gateway).sendMessage; this prevents calling the zero
address and surfaces a clear error when a chain link is unregistered.
🧹 Nitpick comments (1)
test/crosschain/CrosschainExecutor.test.js (1)
131-150: Consider specifying the expected revert reason for direct setter call.The crosschain call path correctly expects
FailedCall, but the direct call (line 133) uses a bare.to.be.revertedwithout specifying the expected error. This could mask unexpected revert reasons.♻️ Suggested improvement
- await expect(this.executor.$_setup(this.eoa.address, this.chain.toErc7930(this.newController))).to.be.reverted; + await expect(this.executor.$_setup(this.eoa.address, this.chain.toErc7930(this.newController))) + .to.be.revertedWithoutReason(); // EOA has no code, so supportsAttribute call failsOr if there's a specific custom error expected, use
revertedWithCustomError.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
contracts/crosschain/CrosschainRemoteController.solcontracts/crosschain/CrosschainRemoteExecutor.solcontracts/mocks/crosschain/CrosschainRemoteControllerMock.soltest/crosschain/CrosschainExecutor.test.js
🧰 Additional context used
🧠 Learnings (4)
📓 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:
contracts/crosschain/CrosschainRemoteController.solcontracts/crosschain/CrosschainRemoteExecutor.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:
contracts/mocks/crosschain/CrosschainRemoteControllerMock.solcontracts/crosschain/CrosschainRemoteExecutor.sol
📚 Learning: 2025-08-28T16:58:18.879Z
Learnt from: Amxx
Repo: OpenZeppelin/openzeppelin-contracts PR: 5904
File: contracts/mocks/crosschain/ERC7786GatewayMock.sol:27-31
Timestamp: 2025-08-28T16:58:18.879Z
Learning: In the OpenZeppelin ERC7786 implementation, the `UnsupportedAttribute(bytes4 selector)` error is defined in the `IERC7786GatewaySource` interface, making it directly accessible to inheriting contracts without needing interface qualification.
Applied to files:
contracts/crosschain/CrosschainRemoteExecutor.sol
🧬 Code graph analysis (1)
test/crosschain/CrosschainExecutor.test.js (2)
test/crosschain/ERC7786Recipient.test.js (1)
getLocalChain(15-15)test/helpers/erc7579.js (6)
encodeMode(15-24)encodeSingle(26-27)CALL_TYPE_BATCH(12-12)encodeBatch(29-39)CALL_TYPE_DELEGATE(13-13)encodeDelegate(41-42)
⏰ 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
- GitHub Check: slither
- GitHub Check: tests-foundry
- GitHub Check: halmos
- GitHub Check: tests-upgradeable
- GitHub Check: coverage
🔇 Additional comments (13)
contracts/mocks/crosschain/CrosschainRemoteControllerMock.sol (1)
8-10: LGTM!Simple mock contract that correctly extends
CrosschainRemoteControllerand forwards the constructor parameters. Appropriate for testing purposes. Based on learnings, mock contracts don't require additional input validation.test/crosschain/CrosschainExecutor.test.js (4)
16-35: LGTM!The fixture correctly establishes the bidirectional link: the executor knows about the controller (via constructor), and the controller knows about the executor (via
$_setLink). Event verification during setup is good practice.
42-52: LGTM!Good verification of the bidirectional relationship between controller and executor. The use of
eventually.deep.equalfor the array comparison is appropriate.
54-95: LGTM!Comprehensive coverage of all three call types (single, batch, delegatecall) plus the invalid mode error path. The delegate mode test correctly expects
this.gatewayas the caller sincedelegatecallexecutes in the executor's context.
152-156: LGTM!Good verification that
reconfigureis restricted to self-calls only.contracts/crosschain/CrosschainRemoteExecutor.sol (5)
10-18: LGTM!State variables are appropriately private with public getters. The
CrosschainControllerSetevent captures both gateway and controller for proper observability.
20-22: LGTM!Constructor delegates to
_setup, ensuring consistent initialization and validation.
32-46: LGTM!The access control pattern (
msg.sender == address(this)) ensures reconfiguration can only happen through the executor itself, typically via a cross-chain call from the authorized controller. The gateway validation viasupportsAttributeis an elegant way to verify ERC-7786 compliance while also rejecting EOAs.
48-54: LGTM!Proper authorization check requiring both gateway instance match and controller sender match. The
equalcomparison fromByteslibrary handles the dynamic bytes comparison correctly.
57-74: Verify behavior with short payloads.If
payloadis shorter than 32 bytes, the slicepayload[0x00:0x20]will revert with an out-of-bounds error. This may be intentional (malformed messages should fail), but consider whether the revert message is clear enough for debugging cross-chain issues.contracts/crosschain/CrosschainRemoteController.sol (3)
14-18: LGTM!The mapping key is
bytesto support flexible chain identifiers. Theusingdeclarations for utility libraries are appropriate.
20-24: LGTM!Constructor correctly iterates through provided links with
allowOverride=false, ensuring no duplicate chain entries are silently overwritten during initialization.
51-64: LGTM!The
_setLinkimplementation correctly:
- Validates the gateway is ERC-7786 compliant
- Derives the chain identifier from the counterpart address
- Enforces single-registration semantics unless explicitly overriding
- Emits the appropriate event
The pattern of deriving the chain key from the counterpart simplifies the API and prevents mismatches.
| function _crosschainExecute(bytes memory chain, Mode mode, bytes memory executionCalldata) internal virtual { | ||
| (address gateway, bytes memory counterpart) = getLink(chain); | ||
| IERC7786GatewaySource(gateway).sendMessage( | ||
| counterpart, | ||
| abi.encodePacked(mode, executionCalldata), | ||
| new bytes[](0) | ||
| ); | ||
| } |
There was a problem hiding this comment.
Missing validation for unregistered chain link.
If getLink(chain) returns (address(0), "") for an unregistered chain, the call to IERC7786GatewaySource(address(0)).sendMessage(...) will revert, but with a generic error. Consider adding an explicit check with a descriptive error.
🔧 Suggested improvement
function _crosschainExecute(bytes memory chain, Mode mode, bytes memory executionCalldata) internal virtual {
(address gateway, bytes memory counterpart) = getLink(chain);
+ require(gateway != address(0), CrosschainLinked.LinkNotRegistered(chain));
IERC7786GatewaySource(gateway).sendMessage(
counterpart,
abi.encodePacked(mode, executionCalldata),
new bytes[](0)
);
}Note: This assumes a LinkNotRegistered error exists or can be added to CrosschainLinked.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In @contracts/crosschain/CrosschainRemoteController.sol around lines 26 - 33,
Add an explicit validation in _crosschainExecute: call getLink(chain) as before,
then require that gateway != address(0) (and optionally counterpart.length > 0)
and revert with a descriptive error such as LinkNotRegistered(chain) (from
CrosschainLinked) before calling IERC7786GatewaySource(gateway).sendMessage;
this prevents calling the zero address and surfaces a clear error when a chain
link is unregistered.
There was a problem hiding this comment.
Questions:
- Do we want CrosschainRemoteExecutor to access calls from one vs multiple controllers ?
- Is the CrosschainRemoteController really usefull, or should be rather have a Library with a function
library CrosschainRemoteExecutorUtils {
function execute(
address gateway,
bytes memory executor,
Mode mode,
bytes memory executionCalldata
) internal returns (bytes32) {
return IERC7786GatewaySource(gateway).sendMessage(executor, abi.encodePacked(mode, executionCalldata), new bytes[](0));
}
}1 participant
Fixes #5903
PR Checklist
npx changeset add)