diff --git a/.gitmodules b/.gitmodules index f94071e535..502fd7060f 100644 --- a/.gitmodules +++ b/.gitmodules @@ -15,3 +15,6 @@ [submodule "lib/@matterlabs/zksync-contracts"] path = lib/@matterlabs/zksync-contracts url = https://github.com/matter-labs/v2-testnet-contracts +[submodule "l1-contracts/lib/openzeppelin-contracts-master"] + path = l1-contracts/lib/openzeppelin-contracts-master + url = https://github.com/OpenZeppelin/openzeppelin-contracts.git diff --git a/l1-contracts/contracts/bridge/L2SharedBridgeLegacy.sol b/l1-contracts/contracts/bridge/L2SharedBridgeLegacy.sol index 150f1035ae..c986c01650 100644 --- a/l1-contracts/contracts/bridge/L2SharedBridgeLegacy.sol +++ b/l1-contracts/contracts/bridge/L2SharedBridgeLegacy.sol @@ -16,7 +16,7 @@ import {IL2AssetRouter} from "./asset-router/IL2AssetRouter.sol"; import {IL2NativeTokenVault} from "./ntv/IL2NativeTokenVault.sol"; import {IL2SharedBridgeLegacy} from "./interfaces/IL2SharedBridgeLegacy.sol"; -import {AmountMustBeGreaterThanZero, DeployFailed, EmptyBytes32, Unauthorized, ZeroAddress, InvalidCaller} from "../common/L1ContractErrors.sol"; +import {AmountMustBeGreaterThanZero, DeployFailed, EmptyBytes32, Unauthorized, ZeroAddress} from "../common/L1ContractErrors.sol"; /// @author Matter Labs /// @custom:security-contact security@matterlabs.dev @@ -113,7 +113,7 @@ contract L2SharedBridgeLegacy is IL2SharedBridgeLegacy, Initializable { AddressAliasHelper.undoL1ToL2Alias(msg.sender) != l1Bridge && AddressAliasHelper.undoL1ToL2Alias(msg.sender) != l1SharedBridge ) { - revert InvalidCaller(msg.sender); + revert Unauthorized(msg.sender); } IL2AssetRouter(L2_ASSET_ROUTER_ADDR).finalizeDepositLegacyBridge({ diff --git a/l1-contracts/contracts/bridge/asset-router/AssetRouterBase.sol b/l1-contracts/contracts/bridge/asset-router/AssetRouterBase.sol index dd647c7a41..19fceb32bb 100644 --- a/l1-contracts/contracts/bridge/asset-router/AssetRouterBase.sol +++ b/l1-contracts/contracts/bridge/asset-router/AssetRouterBase.sol @@ -109,14 +109,14 @@ abstract contract AssetRouterBase is IAssetRouterBase, Ownable2StepUpgradeable, bytes32 _assetId, address _originalCaller, uint256 _amount - ) public payable virtual; + ) external payable virtual; function _bridgehubDepositBaseToken( uint256 _chainId, bytes32 _assetId, address _originalCaller, uint256 _amount - ) public payable virtual { + ) internal virtual { address assetHandler = assetHandlerAddress[_assetId]; require(assetHandler != address(0), AssetHandlerDoesNotExist(_assetId)); @@ -220,7 +220,7 @@ abstract contract AssetRouterBase is IAssetRouterBase, Ownable2StepUpgradeable, //////////////////////////////////////////////////////////////*/ /// @inheritdoc IAssetRouterBase - function finalizeDeposit(uint256 _chainId, bytes32 _assetId, bytes calldata _transferData) public payable virtual; + function finalizeDeposit(uint256 _chainId, bytes32 _assetId, bytes calldata _transferData) external payable virtual; function _finalizeDeposit( uint256 _chainId, diff --git a/l1-contracts/contracts/bridge/asset-router/IL1AssetRouter.sol b/l1-contracts/contracts/bridge/asset-router/IL1AssetRouter.sol index cc3dc2f148..ec02640d28 100644 --- a/l1-contracts/contracts/bridge/asset-router/IL1AssetRouter.sol +++ b/l1-contracts/contracts/bridge/asset-router/IL1AssetRouter.sol @@ -8,7 +8,7 @@ import {IAssetRouterBase} from "./IAssetRouterBase.sol"; import {L2TransactionRequestTwoBridgesInner} from "../../bridgehub/IBridgehub.sol"; import {IL1SharedBridgeLegacy} from "../interfaces/IL1SharedBridgeLegacy.sol"; import {IL1ERC20Bridge} from "../interfaces/IL1ERC20Bridge.sol"; -import {IL1CrossChainSender} from "./IL1CrossChainSender.sol"; +import {IL1CrossChainSender} from "../interfaces/IL1CrossChainSender.sol"; /// @title L1 Bridge contract interface /// @author Matter Labs diff --git a/l1-contracts/contracts/bridge/asset-router/IL2AssetRouter.sol b/l1-contracts/contracts/bridge/asset-router/IL2AssetRouter.sol index b9dcf1fdec..8d0fa229b6 100644 --- a/l1-contracts/contracts/bridge/asset-router/IL2AssetRouter.sol +++ b/l1-contracts/contracts/bridge/asset-router/IL2AssetRouter.sol @@ -3,11 +3,11 @@ pragma solidity ^0.8.20; import {IAssetRouterBase} from "./IAssetRouterBase.sol"; -import {InteropCallStarter} from "../../common/Messaging.sol"; +import {IL2CrossChainSender} from "../interfaces/IL2CrossChainSender.sol"; /// @author Matter Labs /// @custom:security-contact security@matterlabs.dev -interface IL2AssetRouter is IAssetRouterBase { +interface IL2AssetRouter is IAssetRouterBase, IL2CrossChainSender { event WithdrawalInitiatedAssetRouter( uint256 chainId, address indexed l2Sender, @@ -37,19 +37,4 @@ interface IL2AssetRouter is IAssetRouterBase { /// a legacy asset. /// @param _assetId The assetId of the legacy token. function setLegacyTokenAssetHandler(bytes32 _assetId) external; - - /// @notice Function that returns an InteropCallStarter corresponding to the interop call. Effectively this initiates bridging, - /// BH part is processed within this function via `_bridgehubDeposit` call which also returns the data for an l2 call - /// on the destination chain (which will be processed with the returned InteropCallStarter from this function). - /// @param _chainId Destination chain ID. - /// @param _originalCaller The `msg.sender` address from the external call that initiated current one. - /// @param _value The `msg.value` to be deposited on the target chain. - /// @param _data The calldata for the second bridge deposit. - /// @return interopCallStarter InteropCallStarter corresponding to the second bridge call. - function interopCenterInitiateBridge( - uint256 _chainId, - address _originalCaller, - uint256 _value, - bytes calldata _data - ) external payable returns (InteropCallStarter memory interopCallStarter); } diff --git a/l1-contracts/contracts/bridge/asset-router/L1AssetRouter.sol b/l1-contracts/contracts/bridge/asset-router/L1AssetRouter.sol index b826e515ef..8eac33d87c 100644 --- a/l1-contracts/contracts/bridge/asset-router/L1AssetRouter.sol +++ b/l1-contracts/contracts/bridge/asset-router/L1AssetRouter.sol @@ -9,6 +9,7 @@ import {IL1AssetRouter} from "./IL1AssetRouter.sol"; import {IL2AssetRouter} from "./IL2AssetRouter.sol"; import {IAssetRouterBase, LEGACY_ENCODING_VERSION, SET_ASSET_HANDLER_COUNTERPART_ENCODING_VERSION} from "./IAssetRouterBase.sol"; import {AssetRouterBase} from "./AssetRouterBase.sol"; +import {IL1CrossChainSender} from "../interfaces/IL1CrossChainSender.sol"; import {IL1AssetHandler} from "../interfaces/IL1AssetHandler.sol"; import {IL1ERC20Bridge} from "../interfaces/IL1ERC20Bridge.sol"; @@ -195,7 +196,7 @@ contract L1AssetRouter is AssetRouterBase, IL1AssetRouter, ReentrancyGuard { _bridgehubDepositBaseToken(_chainId, _assetId, _originalCaller, _amount); } - /// @inheritdoc IL1AssetRouter + /// @inheritdoc IL1CrossChainSender function bridgehubDeposit( uint256 _chainId, address _originalCaller, diff --git a/l1-contracts/contracts/bridge/asset-router/L2AssetRouter.sol b/l1-contracts/contracts/bridge/asset-router/L2AssetRouter.sol index 762b91c018..c9f846d933 100644 --- a/l1-contracts/contracts/bridge/asset-router/L2AssetRouter.sol +++ b/l1-contracts/contracts/bridge/asset-router/L2AssetRouter.sol @@ -3,6 +3,7 @@ pragma solidity 0.8.28; import {IL2AssetRouter} from "./IL2AssetRouter.sol"; +import {IL2CrossChainSender} from "../interfaces/IL2CrossChainSender.sol"; import {IAssetRouterBase} from "./IAssetRouterBase.sol"; import {AssetRouterBase} from "./AssetRouterBase.sol"; @@ -20,15 +21,16 @@ import {InteropCallStarter} from "../../common/Messaging.sol"; import {L2_BRIDGEHUB_ADDR, L2_INTEROP_CENTER_ADDR, L2_NATIVE_TOKEN_VAULT_ADDR} from "../../common/l2-helpers/L2ContractAddresses.sol"; import {L2ContractHelper} from "../../common/l2-helpers/L2ContractHelper.sol"; import {DataEncoding} from "../../common/libraries/DataEncoding.sol"; -import {AmountMustBeGreaterThanZero, AssetIdNotSupported, EmptyAddress, InvalidCaller, Unauthorized, TokenNotLegacy, InvalidSelector, PayloadTooShort, ExecuteMessageFailed} from "../../common/L1ContractErrors.sol"; -import {IERC7786Receiver} from "../../interop/IERC7786Receiver.sol"; +import {AmountMustBeGreaterThanZero, AssetIdNotSupported, EmptyAddress, Unauthorized, TokenNotLegacy, InvalidSelector, PayloadTooShort, ExecuteMessageFailed} from "../../common/L1ContractErrors.sol"; +import {IERC7786Recipient} from "../../interop/IERC7786Recipient.sol"; import {IERC7786Attributes} from "../../interop/IERC7786Attributes.sol"; +import {InteroperableAddress} from "@openzeppelin/contracts-master/utils/draft-InteroperableAddress.sol"; /// @author Matter Labs /// @custom:security-contact security@matterlabs.dev /// @notice The "default" bridge implementation for the ERC20 tokens. Note, that it does not /// support any custom token logic, i.e. rebase tokens' functionality is not supported. -contract L2AssetRouter is AssetRouterBase, IL2AssetRouter, ReentrancyGuard, IERC7786Receiver { +contract L2AssetRouter is AssetRouterBase, IL2AssetRouter, ReentrancyGuard, IERC7786Recipient { /// @dev The address of the L2 legacy shared bridge. address public immutable L2_LEGACY_SHARED_BRIDGE; @@ -42,9 +44,9 @@ contract L2AssetRouter is AssetRouterBase, IL2AssetRouter, ReentrancyGuard, IERC modifier onlyAssetRouterCounterpart(uint256 _originChainId) { if (_originChainId == L1_CHAIN_ID) { // Only the L1 Asset Router counterpart can initiate and finalize the deposit. - require(AddressAliasHelper.undoL1ToL2Alias(msg.sender) == L1_ASSET_ROUTER, InvalidCaller(msg.sender)); + require(AddressAliasHelper.undoL1ToL2Alias(msg.sender) == L1_ASSET_ROUTER, Unauthorized(msg.sender)); } else { - revert InvalidCaller(msg.sender); // xL2 messaging not supported for now + revert Unauthorized(msg.sender); } _; } @@ -53,27 +55,25 @@ contract L2AssetRouter is AssetRouterBase, IL2AssetRouter, ReentrancyGuard, IERC modifier onlyAssetRouterCounterpartOrSelf(uint256 _chainId) { if (_chainId == L1_CHAIN_ID) { // Only the L1 Asset Router counterpart can initiate and finalize the deposit. - if ( - (AddressAliasHelper.undoL1ToL2Alias(msg.sender) != L1_ASSET_ROUTER) && - msg.sender != address(this) && - (AddressAliasHelper.undoL1ToL2Alias(msg.sender) != address(this)) - ) { - revert InvalidCaller(msg.sender); + if ((AddressAliasHelper.undoL1ToL2Alias(msg.sender) != L1_ASSET_ROUTER) && msg.sender != address(this)) { + revert Unauthorized(msg.sender); } } else { - revert InvalidCaller(msg.sender); // xL2 messaging not supported for now + if (msg.sender != address(this)) { + revert Unauthorized(msg.sender); + } } _; } /// @notice Checks that the message sender is the legacy L2 bridge. modifier onlyLegacyBridge() { - require(msg.sender == L2_LEGACY_SHARED_BRIDGE, InvalidCaller(msg.sender)); + require(msg.sender == L2_LEGACY_SHARED_BRIDGE, Unauthorized(msg.sender)); _; } modifier onlyNTV() { - require(msg.sender == L2_NATIVE_TOKEN_VAULT_ADDR, InvalidCaller(msg.sender)); + require(msg.sender == L2_NATIVE_TOKEN_VAULT_ADDR, Unauthorized(msg.sender)); _; } @@ -130,25 +130,17 @@ contract L2AssetRouter is AssetRouterBase, IL2AssetRouter, ReentrancyGuard, IERC } /// @notice Executes cross-chain interop messages following ERC-7786 standard - /// @param messageId Gateway-specific message identifier (currently unused) - /// @param sourceChain CAIP-2 chain identifier where the message originated - /// @param sender CAIP-10 account address that initiated the cross-chain message + /// @param sender ERC-7930 Address of the message sender /// @param payload Encoded function call data (must be finalizeDeposit) - /// @param attributes ERC-7786 message attributes (currently unused) /// @return Function selector confirming successful execution per ERC-7786 - function executeMessage( - // kl todo: change back to strings - // solhint-disable-next-line no-unused-vars - bytes32 messageId, // Gateway-specific message identifier - uint256 sourceChain, // [CAIP-2] chain identifier - address sender, // [CAIP-10] account address - bytes calldata payload, - // solhint-disable-next-line no-unused-vars - bytes[] calldata attributes + function receiveMessage( + bytes32 /* receiveId */, // Unique identifier + bytes calldata sender, // ERC-7930 address + bytes calldata payload ) external payable returns (bytes4) { // This function serves as the L2AssetRouter's entry point for processing cross-chain bridge operations // initiated through the InteropCenter system. It implements critical security validations: - // - L1->L2 calls: Only L1_ASSET_ROUTER can send messages from L1_CHAIN_ID + // - L1->L2 calls: Currently Interop can only be initiated on L2, so this case shouldn't be covered. // - L2->L2 calls: Only this contract (L2AssetRouter) can send messages from other L2 chains // // This dual validation prevents attackers from spoofing cross-chain messages by requiring @@ -156,16 +148,14 @@ contract L2AssetRouter is AssetRouterBase, IL2AssetRouter, ReentrancyGuard, IERC // // INDIRECT CALL PATTERN (L2->L2 interop flow): // 1. User calls InteropCenter on source L2 - // 2. InteropCenter calls interopCenterInitiateBridge() on source chain's L2AssetRouter + // 2. InteropCenter calls initiateBridging() on source chain's L2AssetRouter // 3. Source L2AssetRouter becomes the "sender" for the destination L2 call - // 4. Destination L2 validates sender == address(this) for non-L1 sources + // 4. Destination L2 validates senderAddress == address(this) for non-L1 sources // (L2AssetRouter address is equal for all ZKsync chains) - require( - (sourceChain == L1_CHAIN_ID && sender == L1_ASSET_ROUTER) || - (sourceChain != L1_CHAIN_ID && sender == address(this)), - InvalidCaller(sender) - ); + (uint256 senderChainId, address senderAddress) = InteroperableAddress.parseEvmV1Calldata(sender); + + require((senderChainId != L1_CHAIN_ID && senderAddress == address(this)), Unauthorized(senderAddress)); // The payload must contain a valid finalizeDeposit selector to ensure only legitimate // bridge operations are executed. This prevents arbitrary function calls through the interop system. @@ -177,7 +167,7 @@ contract L2AssetRouter is AssetRouterBase, IL2AssetRouter, ReentrancyGuard, IERC (bool success, ) = address(this).call(payload); require(success, ExecuteMessageFailed()); - return IERC7786Receiver.executeMessage.selector; + return IERC7786Recipient.receiveMessage.selector; } /*////////////////////////////////////////////////////////////// @@ -219,13 +209,13 @@ contract L2AssetRouter is AssetRouterBase, IL2AssetRouter, ReentrancyGuard, IERC emit DepositFinalizedAssetRouter(_originChainId, _assetId, _transferData); } - /// @inheritdoc IL2AssetRouter - function interopCenterInitiateBridge( + /// @inheritdoc IL2CrossChainSender + function initiateBridging( uint256 _chainId, address _originalCaller, uint256 _value, bytes calldata _data - ) external payable returns (InteropCallStarter memory interopCallStarter) { + ) external payable onlyL2InteropCenter returns (InteropCallStarter memory interopCallStarter) { // This function is called by the InteropCenter when processing indirect interop calls. // It prepares the bridge operation for cross-chain execution through these steps: // 1. Processing the deposit through the standard bridgehub flow @@ -252,7 +242,7 @@ contract L2AssetRouter is AssetRouterBase, IL2AssetRouter, ReentrancyGuard, IERC bytes[] memory attributes = new bytes[](1); attributes[0] = abi.encode(IERC7786Attributes.interopCallValue.selector, _value); interopCallStarter = InteropCallStarter({ - nextContract: request.l2Contract, + to: request.l2Contract, data: request.l2Calldata, callAttributes: attributes }); diff --git a/l1-contracts/contracts/bridge/asset-tracker/AssetTracker.sol b/l1-contracts/contracts/bridge/asset-tracker/AssetTracker.sol index b5fa4ed6cd..90b8f70518 100644 --- a/l1-contracts/contracts/bridge/asset-tracker/AssetTracker.sol +++ b/l1-contracts/contracts/bridge/asset-tracker/AssetTracker.sol @@ -11,7 +11,7 @@ import {L2_ASSET_ROUTER_ADDR, L2_ASSET_TRACKER_ADDR, L2_INTEROP_CENTER_ADDR, L2_ import {DataEncoding} from "../../common/libraries/DataEncoding.sol"; import {IAssetRouterBase} from "../asset-router/IAssetRouterBase.sol"; import {INativeTokenVault} from "../ntv/INativeTokenVault.sol"; -import {InsufficientChainBalanceAssetTracker, InvalidInteropCalldata, InvalidMessage, InvalidProof, ReconstructionMismatch, Unauthorized, ChainIdNotRegistered} from "../../common/L1ContractErrors.sol"; +import {InsufficientChainBalanceAssetTracker, InvalidInteropCalldata, InvalidMessage, InvalidProof, ReconstructionMismatch, Unauthorized, ChainIdNotRegistered, InvalidChainId} from "../../common/L1ContractErrors.sol"; import {IMessageRoot} from "../../bridgehub/IMessageRoot.sol"; import {ProcessLogsInput} from "../../state-transition/chain-interfaces/IExecutor.sol"; import {DynamicIncrementalMerkleMemory} from "../../common/libraries/DynamicIncrementalMerkleMemory.sol"; @@ -24,7 +24,7 @@ import {FinalizeL1DepositParams} from "../../bridge/interfaces/IL1Nullifier.sol" import {TransientPrimitivesLib} from "../../common/libraries/TransientPrimitves/TransientPrimitives.sol"; import {AddressAliasHelper} from "../../vendor/AddressAliasHelper.sol"; // import {IChainAssetHandler} from "../../bridgehub/IChainAssetHandler.sol"; -import {NotMigratedChain, InvalidAssetId, InvalidAmount, InvalidChainId, InvalidSender} from "./AssetTrackerErrors.sol"; +import {NotMigratedChain, InvalidAssetId, InvalidAmount, InvalidSender} from "./AssetTrackerErrors.sol"; contract AssetTracker is IAssetTracker, Ownable2StepUpgradeable, AssetHandlerModifiers { using DynamicIncrementalMerkleMemory for DynamicIncrementalMerkleMemory.Bytes32PushTree; diff --git a/l1-contracts/contracts/bridge/asset-tracker/AssetTrackerErrors.sol b/l1-contracts/contracts/bridge/asset-tracker/AssetTrackerErrors.sol index 8e3f22f372..3f6bd77f5a 100644 --- a/l1-contracts/contracts/bridge/asset-tracker/AssetTrackerErrors.sol +++ b/l1-contracts/contracts/bridge/asset-tracker/AssetTrackerErrors.sol @@ -5,8 +5,6 @@ pragma solidity ^0.8.21; error InvalidAmount(); // 0xfafca5a0 error InvalidAssetId(); -// 0x7a47c9a2 -error InvalidChainId(); // 0x4ecc0587 error InvalidMigrationNumber(); // 0xddb5de5e diff --git a/l1-contracts/contracts/bridge/asset-router/IL1CrossChainSender.sol b/l1-contracts/contracts/bridge/interfaces/IL1CrossChainSender.sol similarity index 100% rename from l1-contracts/contracts/bridge/asset-router/IL1CrossChainSender.sol rename to l1-contracts/contracts/bridge/interfaces/IL1CrossChainSender.sol diff --git a/l1-contracts/contracts/bridge/interfaces/IL2CrossChainSender.sol b/l1-contracts/contracts/bridge/interfaces/IL2CrossChainSender.sol new file mode 100644 index 0000000000..28e601a95d --- /dev/null +++ b/l1-contracts/contracts/bridge/interfaces/IL2CrossChainSender.sol @@ -0,0 +1,25 @@ +// SPDX-License-Identifier: MIT +// We use a floating point pragma here so it can be used within other projects that interact with the ZKsync ecosystem without using our exact pragma version. +pragma solidity ^0.8.21; + +import {InteropCallStarter} from "../../common/Messaging.sol"; + +/// @title L2 Cross Chain Sender interface +/// @author Matter Labs +/// @custom:security-contact security@matterlabs.dev +interface IL2CrossChainSender { + /// @notice Function that returns an InteropCallStarter corresponding to the interop call. Effectively this initiates bridging, + /// BH part is processed within this function via `_bridgehubDeposit` call which also returns the data for an l2 call + /// on the destination chain (which will be processed with the returned InteropCallStarter from this function). + /// @param _chainId Destination chain ID. + /// @param _originalCaller The `msg.sender` address from the external call that initiated current one. + /// @param _value The `msg.value` to be deposited on the target chain. + /// @param _data The calldata for the second bridge deposit. + /// @return interopCallStarter InteropCallStarter corresponding to the second bridge call. + function initiateBridging( + uint256 _chainId, + address _originalCaller, + uint256 _value, + bytes calldata _data + ) external payable returns (InteropCallStarter memory interopCallStarter); +} diff --git a/l1-contracts/contracts/bridgehub/Bridgehub.sol b/l1-contracts/contracts/bridgehub/Bridgehub.sol index b241a1caef..3d79252506 100644 --- a/l1-contracts/contracts/bridgehub/Bridgehub.sol +++ b/l1-contracts/contracts/bridgehub/Bridgehub.sol @@ -23,8 +23,9 @@ import {BridgehubL2TransactionRequest, L2Log, L2Message, TxStatus} from "../comm import {AddressAliasHelper} from "../vendor/AddressAliasHelper.sol"; import {IMessageRoot} from "./IMessageRoot.sol"; import {ICTMDeploymentTracker} from "./ICTMDeploymentTracker.sol"; -import {AlreadyCurrentSL, NotChainAssetHandler, NotInGatewayMode, NotL1, NotRelayedSender, SLNotWhitelisted, SecondBridgeAddressTooLow} from "./L1BridgehubErrors.sol"; -import {AssetHandlerNotRegistered, AssetIdAlreadyRegistered, AssetIdNotSupported, BridgeHubAlreadyRegistered, CTMAlreadyRegistered, CTMNotRegistered, ChainIdAlreadyExists, ChainIdCantBeCurrentChain, ChainIdMismatch, ChainIdNotRegistered, ChainIdTooBig, EmptyAssetId, IncorrectBridgeHubAddress, MigrationPaused, MsgValueMismatch, NoCTMForAssetId, NotCurrentSettlementLayer, SettlementLayersMustSettleOnL1, SharedBridgeNotSet, Unauthorized, WrongMagicValue, ZKChainLimitReached, ZeroAddress, ZeroChainId} from "../common/L1ContractErrors.sol"; +import {AlreadyCurrentSL, NotChainAssetHandler, NotInGatewayMode, NotRelayedSender, SLNotWhitelisted, SecondBridgeAddressTooLow} from "./L1BridgehubErrors.sol"; +import {AssetHandlerNotRegistered, AssetIdAlreadyRegistered, AssetIdNotSupported, BridgeHubAlreadyRegistered, CTMAlreadyRegistered, CTMNotRegistered, ChainIdAlreadyExists, ChainIdCantBeCurrentChain, ChainIdMismatch, ChainIdNotRegistered, ChainIdTooBig, EmptyAssetId, IncorrectBridgeHubAddress, MigrationPaused, MsgValueMismatch, NoCTMForAssetId, NotCurrentSettlementLayer, SettlementLayersMustSettleOnL1, SharedBridgeNotSet, Unauthorized, WrongMagicValue, ZKChainLimitReached, ZeroAddress, ZeroChainId, NotL1} from "../common/L1ContractErrors.sol"; +import {IL1CrossChainSender} from "../bridge/interfaces/IL1CrossChainSender.sol"; /// @author Matter Labs /// @custom:security-contact security@matterlabs.dev @@ -538,7 +539,7 @@ contract Bridgehub is IBridgehub, ReentrancyGuard, Ownable2StepUpgradeable, Paus } // slither-disable-next-line arbitrary-send-eth - L2TransactionRequestTwoBridgesInner memory outputRequest = IL1AssetRouter(_request.secondBridgeAddress) + L2TransactionRequestTwoBridgesInner memory outputRequest = IL1CrossChainSender(_request.secondBridgeAddress) .bridgehubDeposit{value: _request.secondBridgeValue}( _request.chainId, msg.sender, @@ -566,7 +567,7 @@ contract Bridgehub is IBridgehub, ReentrancyGuard, Ownable2StepUpgradeable, Paus }) ); - IL1AssetRouter(_request.secondBridgeAddress).bridgehubConfirmL2Transaction( + IL1CrossChainSender(_request.secondBridgeAddress).bridgehubConfirmL2Transaction( _request.chainId, outputRequest.txDataHash, canonicalTxHash diff --git a/l1-contracts/contracts/bridgehub/CTMDeploymentTracker.sol b/l1-contracts/contracts/bridgehub/CTMDeploymentTracker.sol index 3f0be80a6c..4195cbdfed 100644 --- a/l1-contracts/contracts/bridgehub/CTMDeploymentTracker.sol +++ b/l1-contracts/contracts/bridgehub/CTMDeploymentTracker.sol @@ -6,7 +6,7 @@ import {Ownable2StepUpgradeable} from "@openzeppelin/contracts-upgradeable-v4/ac import {IBridgehub, L2TransactionRequestTwoBridgesInner} from "./IBridgehub.sol"; import {ICTMDeploymentTracker} from "./ICTMDeploymentTracker.sol"; -import {IL1CrossChainSender} from "../bridge/asset-router/IL1CrossChainSender.sol"; +import {IL1CrossChainSender} from "../bridge/interfaces/IL1CrossChainSender.sol"; import {IAssetRouterBase} from "../bridge/asset-router/IAssetRouterBase.sol"; import {TWO_BRIDGES_MAGIC_VALUE} from "../common/Config.sol"; diff --git a/l1-contracts/contracts/bridgehub/ChainAssetHandler.sol b/l1-contracts/contracts/bridgehub/ChainAssetHandler.sol index bd6b03a52e..fe530e8e7a 100644 --- a/l1-contracts/contracts/bridgehub/ChainAssetHandler.sol +++ b/l1-contracts/contracts/bridgehub/ChainAssetHandler.sol @@ -15,8 +15,8 @@ import {IZKChain} from "../state-transition/chain-interfaces/IZKChain.sol"; import {ETH_TOKEN_ADDRESS, L1_SETTLEMENT_LAYER_VIRTUAL_ADDRESS} from "../common/Config.sol"; import {IMessageRoot} from "./IMessageRoot.sol"; -import {HyperchainNotRegistered, IncorrectChainAssetId, IncorrectSender, NotAssetRouter, NotL1} from "./L1BridgehubErrors.sol"; -import {ChainIdNotRegistered, MigrationPaused} from "../common/L1ContractErrors.sol"; +import {HyperchainNotRegistered, IncorrectChainAssetId, IncorrectSender, NotAssetRouter} from "./L1BridgehubErrors.sol"; +import {ChainIdNotRegistered, MigrationPaused, NotL1} from "../common/L1ContractErrors.sol"; import {AssetHandlerModifiers} from "../bridge/interfaces/AssetHandlerModifiers.sol"; import {IChainAssetHandler} from "./IChainAssetHandler.sol"; diff --git a/l1-contracts/contracts/bridgehub/ChainRegistrationSender.sol b/l1-contracts/contracts/bridgehub/ChainRegistrationSender.sol index 08c5d00273..e21aa90dec 100644 --- a/l1-contracts/contracts/bridgehub/ChainRegistrationSender.sol +++ b/l1-contracts/contracts/bridgehub/ChainRegistrationSender.sol @@ -6,10 +6,9 @@ import {Ownable2StepUpgradeable} from "@openzeppelin/contracts-upgradeable-v4/ac import {IChainRegistrationSender} from "./IChainRegistrationSender.sol"; import {ReentrancyGuard} from "../common/ReentrancyGuard.sol"; -import {IL1CrossChainSender} from "../bridge/asset-router/IL1CrossChainSender.sol"; +import {IL1CrossChainSender} from "../bridge/interfaces/IL1CrossChainSender.sol"; import {IBridgehub, L2TransactionRequestTwoBridgesInner} from "./IBridgehub.sol"; -import {IL1CrossChainSender} from "../bridge/asset-router/IL1CrossChainSender.sol"; import {IMailbox} from "../state-transition/chain-interfaces/IMailbox.sol"; import {L2_BRIDGEHUB_ADDR} from "../common/l2-helpers/L2ContractAddresses.sol"; diff --git a/l1-contracts/contracts/bridgehub/L1BridgehubErrors.sol b/l1-contracts/contracts/bridgehub/L1BridgehubErrors.sol index 001c604271..ae609e56cd 100644 --- a/l1-contracts/contracts/bridgehub/L1BridgehubErrors.sol +++ b/l1-contracts/contracts/bridgehub/L1BridgehubErrors.sol @@ -24,8 +24,6 @@ error NotChainAssetHandler(address sender, address chainAssetHandler); error NotCurrentSettlementLayer(uint256 currentSettlementLayer, uint256 newSettlementLayer); // 0x472477e2 error NotInGatewayMode(); -// 0xecb34449 -error NotL1(uint256 l1ChainId, uint256 blockChainId); // 0x23295f0e error NotOwner(address sender, address owner); // 0x693cd3dc diff --git a/l1-contracts/contracts/common/L1ContractErrors.sol b/l1-contracts/contracts/common/L1ContractErrors.sol index 288ca9dafa..d197e00e18 100644 --- a/l1-contracts/contracts/common/L1ContractErrors.sol +++ b/l1-contracts/contracts/common/L1ContractErrors.sol @@ -140,12 +140,10 @@ error IncorrectBridgeHubAddress(address bridgehub); error IncorrectTokenAddressFromNTV(bytes32 assetId, address tokenAddress); // 0xddb74934 error InsufficientChainBalanceAssetTracker(uint256 chainId, bytes32 assetId, uint256 amount); -// 0x826fb11e -error InsufficientChainBalance(); // 0x9bf8b9aa error InvalidBatchNumber(uint256 provided, uint256 expected); -// 0xcbd9d2e0 -error InvalidCaller(address); +// 0x7a47c9a2 +error InvalidChainId(); // 0x92daded2 error InvalidDAForPermanentRollup(); // 0x4fbe5dba @@ -256,6 +254,10 @@ error NotAZKChain(address addr); error NotCurrentSettlementLayer(); // 0xdd7e3621 error NotInitializedReentrancyGuard(); +// 0xecb34449 +error NotL1(uint256 l1ChainId, uint256 blockChainId); +// 0xc5441a63 +error NotL2ToL2(uint256 sourceChainId, uint256 destinationChainId); // 0xdf17e316 error NotWhitelisted(address); // 0xf3ed9dfa diff --git a/l1-contracts/contracts/common/Messaging.sol b/l1-contracts/contracts/common/Messaging.sol index a001f95390..cfb38aa142 100644 --- a/l1-contracts/contracts/common/Messaging.sol +++ b/l1-contracts/contracts/common/Messaging.sol @@ -155,7 +155,7 @@ struct InteropRoot { /// @param chainId The chain ID of the transaction to check. /// @param l2BatchNumber The L2 batch number where the withdrawal was processed. /// @param l2MessageIndex The position in the L2 logs Merkle tree of the l2Log that was sent with the message. -/// @param l2sender The address of the message sender on L2 (base token system contract address or asset handler) +/// @param l2Sender The address of the message sender on L2 (base token system contract address or asset handler) /// @param l2TxNumberInBatch The L2 transaction number in the batch, in which the log was sent. /// @param message The L2 withdraw data, stored in an L2 -> L1 message. /// @param merkleProof The Merkle proof of the inclusion L2 -> L1 message about withdrawal initialization. @@ -170,44 +170,46 @@ struct FinalizeL1DepositParams { } /// @dev Struct used to define parameters for adding a single call in an interop bundle. -/// @param nextContract Address of the contract to call on the destination chain. -/// @param data Calldata payload to send to nextContract. +/// @param to Address to call on the destination chain. +/// @param data Calldata payload to send to `to` address on the destination chain. /// @param callAttributes EIP-7786 Attributes. struct InteropCallStarter { - address nextContract; + address to; bytes data; bytes[] callAttributes; } /// @dev Internal representation of an InteropCallStarter after parsing its parameters. -/// @param nextContract Address of the contract to call on the destination chain. +/// @param to Address to call on the destination chain. /// @param data Calldata payload to send. /// @param callAttributes EIP-7786 Attributes. struct InteropCallStarterInternal { - address nextContract; + address to; bytes data; CallAttributes callAttributes; } /// @param interopCallValue Base token value on destination chain to send for interop call. -/// @param directCall True for direct interop, false if routed through bridge. +/// @param indirectCall True if routed through bridge, false for direct interop. /// @param indirectCallMessageValue Base token value on sending chain to send for indirect call. struct CallAttributes { uint256 interopCallValue; - bool directCall; + bool indirectCall; uint256 indirectCallMessageValue; } -/// @param executionAddress Address allowed to execute on remote side. If set to 0 -> execution is permissionless. -/// @param unbundlerAddress Address allowed to unbundle. Note, that here no permissionless mode is available, unlike above. +/// @param executionAddress ERC-7930 Address allowed to execute the bundle on the destination chain. If the byte array is empty then execution is permissionless. +/// @param unbundlerAddress ERC-7930 Address allowed to unbundle the bundle on the destination chain. Note, that it is required to be nonempty, unlike `executionAddress`. struct BundleAttributes { - address executionAddress; - address unbundlerAddress; + bytes executionAddress; + bytes unbundlerAddress; } /// @dev A single call. /// @param version Version of the InteropCall. -/// @param shadowAccount If true, execute via a shadow account, otherwise normal. In current release always false. +/// @param shadowAccount If true, execute via a shadow account, otherwise normal. In current release always false, as it's not yet implemented. +/// Shadow accounts help with interop when `to` doesn't support 7786. In this case, a "shadow" account could be deployed, allowing +/// the user to hold funds securely on the destination chain, and interact with anything on destination chain using this shadow account. /// @param to Destination contract address on the target chain. /// @param from Original sender address that initiated the call. /// @param value Amount of base token to send with the call. @@ -237,8 +239,7 @@ enum CallStatus { /// @param interopBundleSalt Salt of the interopBundle. It's required to ensure that all bundles have distinct hashes. /// It's equal to the keccak256(abi.encodePacked(senderOfTheBundle, NumberOfBundleSentByTheSender)) /// @param calls Array of InteropCall structs to execute. -/// @param executionAddress Optional address authorized to execute this bundle; zero-address means permissionless. -/// @param unbundlerAddress Address authorized to unbundle this bundle. +/// @param bundleAttributes Bundle execution and unbundling attributes. struct InteropBundle { bytes1 version; uint256 destinationChainId; diff --git a/l1-contracts/contracts/interop/AttributesDecoder.sol b/l1-contracts/contracts/interop/AttributesDecoder.sol index 2068aa00e0..e987f7287e 100644 --- a/l1-contracts/contracts/interop/AttributesDecoder.sol +++ b/l1-contracts/contracts/interop/AttributesDecoder.sol @@ -7,10 +7,14 @@ pragma solidity ^0.8.24; /// https://github.com/ethereum/ERCs/blob/023a7d657666308568d3d1391c578d5972636093/ERCS/erc-7786.md library AttributesDecoder { function decodeAddress(bytes calldata _data) internal pure returns (address) { - return (address(uint160(bytes20(_data[4:24])))); + return (address(uint160(bytes20(_data[16:36])))); } function decodeUint256(bytes calldata _data) internal pure returns (uint256) { return (uint256(bytes32(_data[4:36]))); } + + function decodeInteroperableAddress(bytes calldata _data) internal pure returns (bytes calldata) { + return _data[4:]; + } } diff --git a/l1-contracts/contracts/interop/IERC7786.sol b/l1-contracts/contracts/interop/IERC7786.sol deleted file mode 100644 index 92fb352d37..0000000000 --- a/l1-contracts/contracts/interop/IERC7786.sol +++ /dev/null @@ -1,47 +0,0 @@ -// SPDX-License-Identifier: MIT - -pragma solidity ^0.8.24; - -/// @title IERC7786GatewaySource -/// @notice Interface for the ERC7786 gateway source -/// https://github.com/ethereum/ERCs/blob/023a7d657666308568d3d1391c578d5972636093/ERCS/erc-7786.md -interface IERC7786GatewaySource { - event MessagePosted( - bytes32 indexed outboxId, - string sender, - string receiver, - bytes payload, - uint256 value, - bytes[] attributes - ); - - error UnsupportedAttribute(bytes4 selector); - - function supportsAttribute(bytes4 selector) external view returns (bool); - - function sendMessage( - string calldata destinationChain, // [CAIP-2] chain identifier - string calldata receiver, // [CAIP-10] account address - bytes calldata payload, - bytes[] calldata attributes - ) external payable returns (bytes32 outboxId); - - /// kl todo decide how to merge this and sendMessage, i.e. CAIP. Also put value in an attribute. - function sendCall( - uint256 destinationChain, - address destinationAddress, - bytes calldata data, - bytes[] calldata attributes - ) external payable returns (bytes32 outboxId); - - function quoteRelay( - string calldata destinationChain, - string calldata receiver, - bytes calldata payload, - bytes[] calldata attributes, - uint256 gasLimit, - string[] calldata refundReceivers - ) external returns (uint256); - - function requestRelay(bytes32 outboxId, uint256 gasLimit, string[] calldata refundReceivers) external payable; -} diff --git a/l1-contracts/contracts/interop/IERC7786Attributes.sol b/l1-contracts/contracts/interop/IERC7786Attributes.sol index af6f972c10..ec45ecaefb 100644 --- a/l1-contracts/contracts/interop/IERC7786Attributes.sol +++ b/l1-contracts/contracts/interop/IERC7786Attributes.sol @@ -4,13 +4,16 @@ pragma solidity ^0.8.24; /// @title IERC778Attributes /// @notice Interface for the ERC7786 gateway source +/// @dev When adding/removing a function here the InteropCenter must be updated to reflect the changes. /// https://github.com/ethereum/ERCs/blob/023a7d657666308568d3d1391c578d5972636093/ERCS/erc-7786.md interface IERC7786Attributes { function indirectCall(uint256 _indirectCallMessageValue) external pure; function interopCallValue(uint256 _interopCallValue) external pure; - function executionAddress(address _executionAddress) external pure; + // Attribute assumes that _executionAddress is an ERC-7930 address. + function executionAddress(bytes calldata _executionAddress) external pure; - function unbundlerAddress(address _unbundlerAddress) external pure; + // Attribute assumes that _executionAddress is an ERC-7930 address. + function unbundlerAddress(bytes calldata _unbundlerAddress) external pure; } diff --git a/l1-contracts/contracts/interop/IERC7786GatewaySource.sol b/l1-contracts/contracts/interop/IERC7786GatewaySource.sol new file mode 100644 index 0000000000..2864a6842f --- /dev/null +++ b/l1-contracts/contracts/interop/IERC7786GatewaySource.sol @@ -0,0 +1,27 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.24; + +/// @title IERC7786GatewaySource +/// @notice Interface for the ERC7786 gateway source +/// https://github.com/ethereum/ERCs/blob/023a7d657666308568d3d1391c578d5972636093/ERCS/erc-7786.md +interface IERC7786GatewaySource { + event MessageSent( + bytes32 indexed sendId, + bytes sender, // ERC-7930 address + bytes recipient, // ERC-7930 address + bytes payload, + uint256 value, + bytes[] attributes + ); + + error UnsupportedAttribute(bytes4 selector); + + function supportsAttribute(bytes4 selector) external view returns (bool); + + function sendMessage( + bytes calldata recipient, // ERC-7930 address + bytes calldata payload, + bytes[] calldata attributes + ) external payable returns (bytes32 sendId); +} diff --git a/l1-contracts/contracts/interop/IERC7786Receiver.sol b/l1-contracts/contracts/interop/IERC7786Receiver.sol deleted file mode 100644 index 64a59b27b7..0000000000 --- a/l1-contracts/contracts/interop/IERC7786Receiver.sol +++ /dev/null @@ -1,17 +0,0 @@ -// SPDX-License-Identifier: MIT - -pragma solidity ^0.8.24; - -/// @title IERC7786Receiver -/// @notice Interface for the ERC7786 receiver -/// https://github.com/ethereum/ERCs/blob/023a7d657666308568d3d1391c578d5972636093/ERCS/erc-7786.md -interface IERC7786Receiver { - function executeMessage( - // kl todo: change back to strings to be CAIP-2 compatible. - bytes32 messageId, // gateway specific, empty or unique - uint256 sourceChain, // [CAIP-2] chain identifier - address sender, // [CAIP-10] account address - bytes calldata payload, - bytes[] calldata attributes - ) external payable returns (bytes4); -} diff --git a/l1-contracts/contracts/interop/IERC7786Recipient.sol b/l1-contracts/contracts/interop/IERC7786Recipient.sol new file mode 100644 index 0000000000..d2882ddc54 --- /dev/null +++ b/l1-contracts/contracts/interop/IERC7786Recipient.sol @@ -0,0 +1,14 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.24; + +/// @title IERC7786Recipient +/// @notice Interface for the ERC7786 recipient +/// https://github.com/ethereum/ERCs/blob/023a7d657666308568d3d1391c578d5972636093/ERCS/erc-7786.md +interface IERC7786Recipient { + function receiveMessage( + bytes32 receiveId, // Unique identifier + bytes calldata sender, // ERC-7930 address + bytes calldata payload + ) external payable returns (bytes4); +} diff --git a/l1-contracts/contracts/interop/IInteropCenter.sol b/l1-contracts/contracts/interop/IInteropCenter.sol index 84513df678..76e7cc09dd 100644 --- a/l1-contracts/contracts/interop/IInteropCenter.sol +++ b/l1-contracts/contracts/interop/IInteropCenter.sol @@ -10,6 +10,9 @@ import {IAssetTracker} from "../bridge/asset-tracker/IAssetTracker.sol"; interface IInteropCenter { event InteropBundleSent(bytes32 l2l1MsgHash, bytes32 interopBundleHash, InteropBundle interopBundle); + event NewAssetRouter(address indexed oldAssetRouter, address indexed newAssetRouter); + event NewAssetTracker(address indexed oldAssetTracker, address indexed newAssetTracker); + /// @notice Restrictions for parsing attributes. /// @param OnlyInteropCallValue: Only attribute for interop call value is allowed. /// @param OnlyCallAttributes: Only call attributes are allowed. diff --git a/l1-contracts/contracts/interop/InteropCenter.sol b/l1-contracts/contracts/interop/InteropCenter.sol index 2f9c11574e..2a34e83f50 100644 --- a/l1-contracts/contracts/interop/InteropCenter.sol +++ b/l1-contracts/contracts/interop/InteropCenter.sol @@ -16,22 +16,30 @@ import {L2_ASSET_TRACKER_ADDR, L2_BASE_TOKEN_SYSTEM_CONTRACT, L2_TO_L1_MESSENGER import {ETH_TOKEN_ADDRESS, SETTLEMENT_LAYER_RELAY_SENDER} from "../common/Config.sol"; import {BUNDLE_IDENTIFIER, InteropBundle, InteropCall, InteropCallStarter, InteropCallStarterInternal, CallAttributes, BundleAttributes, INTEROP_BUNDLE_VERSION, INTEROP_CALL_VERSION} from "../common/Messaging.sol"; -import {MsgValueMismatch, Unauthorized} from "../common/L1ContractErrors.sol"; +import {MsgValueMismatch, Unauthorized, NotL1, NotL2ToL2} from "../common/L1ContractErrors.sol"; import {NotInGatewayMode} from "../bridgehub/L1BridgehubErrors.sol"; import {IAssetTracker} from "../bridge/asset-tracker/IAssetTracker.sol"; -import {AttributeAlreadySet, AttributeNotForCall, AttributeNotForBundle, IndirectCallValueMismatch, AttributeNotForInteropCallValue} from "./InteropErrors.sol"; +import {AttributeAlreadySet, AttributeViolatesRestriction, IndirectCallValueMismatch} from "./InteropErrors.sol"; -import {IERC7786GatewaySource} from "./IERC7786.sol"; +import {IERC7786GatewaySource} from "./IERC7786GatewaySource.sol"; import {IERC7786Attributes} from "./IERC7786Attributes.sol"; import {AttributesDecoder} from "./AttributesDecoder.sol"; import {InteropDataEncoding} from "./InteropDataEncoding.sol"; +import {InteroperableAddress} from "@openzeppelin/contracts-master/utils/draft-InteroperableAddress.sol"; +import {IL2CrossChainSender} from "../bridge/interfaces/IL2CrossChainSender.sol"; /// @title InteropCenter /// @author Matter Labs /// @custom:security-contact security@matterlabs.dev -/// @dev This contract serves as the primary entry point for L1<->L2 communication, facilitating interactions between end user and bridges. -contract InteropCenter is IInteropCenter, ReentrancyGuard, Ownable2StepUpgradeable, PausableUpgradeable { +/// @dev This contract serves as the primary entry point for communication between chains connected to the interop, facilitating interactions between end user and bridges. +contract InteropCenter is + IInteropCenter, + IERC7786GatewaySource, + ReentrancyGuard, + Ownable2StepUpgradeable, + PausableUpgradeable +{ /// @notice The bridgehub, responsible for registering chains. IBridgehub public immutable override BRIDGE_HUB; @@ -42,7 +50,7 @@ contract InteropCenter is IInteropCenter, ReentrancyGuard, Ownable2StepUpgradeab /// @notice The asset ID of ETH on L1. bytes32 internal immutable ETH_TOKEN_ASSET_ID; - /// @notice All of the ETH and ERC20 tokens are held by NativeVaultToken managed by this AssetRouter. + /// @notice All of the ETH and ERC20 tokens are held by NativeTokenVault managed by this AssetRouter. address public assetRouter; /// @notice AssetTracker component address on L1. On L2 the address is L2_ASSET_TRACKER_ADDR. @@ -57,23 +65,17 @@ contract InteropCenter is IInteropCenter, ReentrancyGuard, Ownable2StepUpgradeab mapping(address sender => uint256 numberOfBundlesSent) public interopBundleNonce; modifier onlyL1() { - if (L1_CHAIN_ID != block.chainid) { - revert Unauthorized(msg.sender); - } + require(L1_CHAIN_ID == block.chainid, NotL1(L1_CHAIN_ID, block.chainid)); _; } - modifier onlyL2NotToL1(uint256 _destinationChainId) { - if (L1_CHAIN_ID == block.chainid || _destinationChainId == L1_CHAIN_ID) { - revert Unauthorized(msg.sender); - } + modifier onlyL2ToL2(uint256 _destinationChainId) { + _ensureL2ToL2(_destinationChainId); _; } modifier onlySettlementLayerRelayedSender() { - if (msg.sender != SETTLEMENT_LAYER_RELAY_SENDER) { - revert Unauthorized(msg.sender); - } + require(msg.sender == SETTLEMENT_LAYER_RELAY_SENDER, Unauthorized(msg.sender)); _; } @@ -88,8 +90,7 @@ contract InteropCenter is IInteropCenter, ReentrancyGuard, Ownable2StepUpgradeab } /// @notice Used to initialize the contract - /// This contract is also deployed on L2 as a system contract. - /// On the L2 owner and its related functions will not be used. + /// InteropCenter is deployed on L2 as a system contract without a proxy thus initialization is needed only on L1. /// @param _owner the owner of the contract function initialize(address _owner) external reentrancyGuardInitializer onlyL1 { _transferOwnership(_owner); @@ -100,93 +101,69 @@ contract InteropCenter is IInteropCenter, ReentrancyGuard, Ownable2StepUpgradeab /// @param _assetRouter Address of the AssetRouter component. /// @param _assetTracker Address of the AssetTracker component on L1. function setAddresses(address _assetRouter, address _assetTracker) external onlyOwner { + address oldAssetRouter = assetRouter; + address oldAssetTracker = address(assetTracker); + assetRouter = _assetRouter; assetTracker = IAssetTracker(_assetTracker); + + emit NewAssetRouter(oldAssetRouter, _assetRouter); + emit NewAssetTracker(oldAssetTracker, _assetTracker); } /*////////////////////////////////////////////////////////////// - Bundle interface + InteropCenter entry points //////////////////////////////////////////////////////////////*/ - /// @notice Finalizes, serializes, and sends a message corresponding to the bundle via the L2 to L1 messenger. - /// @param _bundle InteropBundle struct corresponding to the bundle that is being sent. - /// @param _bundleCallsTotalValue Total base token value for all calls. - /// @return interopBundleHash keccak256 hash of the encoded bundle. - function _finalizeAndSendBundle( - InteropBundle memory _bundle, - uint256 _bundleCallsTotalValue - ) internal returns (bytes32 interopBundleHash) { - // Ensure that tokens required for bundle execution were received. - _ensureCorrectTotalValue(_bundle.destinationChainId, _bundleCallsTotalValue); - - bytes memory interopBundleBytes = abi.encode(_bundle); - - // Send the message corresponding to the relevant InteropBundle to L1. - bytes32 msgHash = L2_TO_L1_MESSENGER_SYSTEM_CONTRACT.sendToL1( - bytes.concat(BUNDLE_IDENTIFIER, interopBundleBytes) - ); - interopBundleHash = InteropDataEncoding.encodeInteropBundleHash(block.chainid, interopBundleBytes); - - // Emit event stating that the bundle was sent out successfully. - emit InteropBundleSent(msgHash, interopBundleHash, _bundle); - } - - /// @notice Ensures the received base token value matches expected for the destination chain. - /// @param _destinationChainId Destination chain ID. - /// @param _totalValue Sum of requested interop call values. - function _ensureCorrectTotalValue(uint256 _destinationChainId, uint256 _totalValue) internal { - bytes32 destinationChainBaseTokenAssetId = BRIDGE_HUB.baseTokenAssetId(_destinationChainId); - // We burn the value that is passed along the bundle here, on source chain. - bytes32 thisChainBaseTokenAssetId = BRIDGE_HUB.baseTokenAssetId(block.chainid); - if (destinationChainBaseTokenAssetId == thisChainBaseTokenAssetId) { - if (msg.value != _totalValue) { - revert MsgValueMismatch(_totalValue, msg.value); - } - // slither-disable-next-line arbitrary-send-eth - L2_BASE_TOKEN_SYSTEM_CONTRACT.burnMsgValue{value: _totalValue}(); - } else { - if (msg.value != 0) { - revert MsgValueMismatch(0, msg.value); - } - IL2AssetRouter(assetRouter).bridgehubDepositBaseToken( - _destinationChainId, - destinationChainBaseTokenAssetId, - msg.sender, - _totalValue - ); - } - } + /// @notice Sends a single ERC-7786 message to another chain. + /// @param recipient ERC-7930 address corresponding to the destination of a message. It must be corresponding to an EIP-155 chain. + /// @param payload Payload to send. + /// @param attributes Attributes of the call. + /// @return sendId Hash of the sent bundle containing a single call. + function sendMessage( + bytes calldata recipient, + bytes calldata payload, + bytes[] calldata attributes + ) external payable whenNotPaused returns (bytes32 sendId) { + (uint256 recipientChainId, address recipientAddress) = InteroperableAddress.parseEvmV1Calldata(recipient); - /*////////////////////////////////////////////////////////////// - EOA helpers - //////////////////////////////////////////////////////////////*/ + _ensureL2ToL2(recipientChainId); - /// @notice Sends a single call to another chain. - /// @param _destinationChainId Chain ID to send to. - /// @param _destinationAddress Address on remote chain. - /// @param _data Calldata payload to send. - /// @param _attributes Attributes of the call. - /// @return bundleHash Hash of the sent bundle containing a single call. - /// Todo use interoperable addresses using this: https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/utils/draft-InteroperableAddress.sol - function sendCall( - uint256 _destinationChainId, - address _destinationAddress, - bytes calldata _data, - bytes[] calldata _attributes - ) public payable onlyL2NotToL1(_destinationChainId) returns (bytes32 bundleHash) { (CallAttributes memory callAttributes, BundleAttributes memory bundleAttributes) = parseAttributes( - _attributes, + attributes, AttributeParsingRestrictions.CallAndBundleAttributes ); + // If the unbundler was not set for a call, we set the unbundler to be equal to the original sender, so that it's + // still possible to unbundle the bundle containing the call. If the original sender is the contract, it'll still + // be able to unbundle the bundle either via direct call to `unbundleBundle`, or via `sendMessage` to `InteropHandler`, + // with specific payload. Refer to `InteropHandler` for details. + if (bundleAttributes.unbundlerAddress.length == 0) { + bundleAttributes.unbundlerAddress = InteroperableAddress.formatEvmV1(block.chainid, msg.sender); + } + InteropCallStarterInternal[] memory callStartersInternal = new InteropCallStarterInternal[](1); callStartersInternal[0] = InteropCallStarterInternal({ - nextContract: _destinationAddress, - data: _data, + to: recipientAddress, + data: payload, callAttributes: callAttributes }); - bundleHash = _sendBundle(_destinationChainId, callStartersInternal, bundleAttributes); + // Prepare original attributes array for the single call + bytes[][] memory originalCallAttributes = new bytes[][](1); + originalCallAttributes[0] = attributes; + + bytes32 bundleHash = _sendBundle( + recipientChainId, + callStartersInternal, + bundleAttributes, + originalCallAttributes + ); + + // We return the sendId of the only message that was sent in the bundle above. We always send messages in bundles, even if there's only one message being sent. + // Note, that bundleHash is unique for every bundle. Each sendId is determined as keccak256 of bundleHash where the message (call) is contained, + // and the index of the call inside the bundle. + sendId = keccak256(abi.encodePacked(bundleHash, uint256(0))); } /// @notice Sends an interop bundle. @@ -199,19 +176,26 @@ contract InteropCenter is IInteropCenter, ReentrancyGuard, Ownable2StepUpgradeab uint256 _destinationChainId, InteropCallStarter[] calldata _callStarters, bytes[] calldata _bundleAttributes - ) public payable onlyL2NotToL1(_destinationChainId) returns (bytes32 bundleHash) { + ) external payable onlyL2ToL2(_destinationChainId) whenNotPaused returns (bytes32 bundleHash) { InteropCallStarterInternal[] memory callStartersInternal = new InteropCallStarterInternal[]( _callStarters.length ); uint256 callStartersLength = _callStarters.length; + + // Prepare original attributes array for all calls + bytes[][] memory originalCallAttributes = new bytes[][](callStartersLength); + for (uint256 i = 0; i < callStartersLength; ++i) { + // Store original attributes for MessageSent event emission + originalCallAttributes[i] = _callStarters[i].callAttributes; + // solhint-disable-next-line no-unused-vars (CallAttributes memory callAttributes, ) = parseAttributes( _callStarters[i].callAttributes, AttributeParsingRestrictions.OnlyCallAttributes ); callStartersInternal[i] = InteropCallStarterInternal({ - nextContract: _callStarters[i].nextContract, + to: _callStarters[i].to, data: _callStarters[i].data, callAttributes: callAttributes }); @@ -221,88 +205,67 @@ contract InteropCenter is IInteropCenter, ReentrancyGuard, Ownable2StepUpgradeab _bundleAttributes, AttributeParsingRestrictions.OnlyBundleAttributes ); + + // If the unbundler was not set for a bundle, we set the unbundler to be equal to the original sender, so + // that it's still possible to unbundle the bundle. If the original sender is the contract, it'll still be + // able to unbundle the bundle either via direct call to `unbundleBundle`, or via `sendMessage` to `InteropHandler`, + // with specific payload. Refer to `InteropHandler` for details. + if (bundleAttributes.unbundlerAddress.length == 0) { + bundleAttributes.unbundlerAddress = InteroperableAddress.formatEvmV1(block.chainid, msg.sender); + } + bundleHash = _sendBundle({ _destinationChainId: _destinationChainId, _callStarters: callStartersInternal, - _bundleAttributes: bundleAttributes + _bundleAttributes: bundleAttributes, + _originalCallAttributes: originalCallAttributes }); } - /// @notice Parses the attributes of the call or bundle. - /// @param _attributes EIP-7786 Attributes of the call. - /// @param _restriction Restriction for parsing attributes. - function parseAttributes( - bytes[] calldata _attributes, - AttributeParsingRestrictions _restriction - ) public pure returns (CallAttributes memory callAttributes, BundleAttributes memory bundleAttributes) { - // Default value is direct call. - callAttributes.directCall = true; + /*////////////////////////////////////////////////////////////// + Internal functions + //////////////////////////////////////////////////////////////*/ - bytes4[4] memory ATTRIBUTE_SELECTORS = _getERC7786AttributeSelectors(); - // We can only pass each attribute once. - bool[] memory attributeUsed = new bool[](4); + function _ensureL2ToL2(uint256 _destinationChainId) internal view { + require( + L1_CHAIN_ID != block.chainid && _destinationChainId != L1_CHAIN_ID, + NotL2ToL2(block.chainid, _destinationChainId) + ); + } - uint256 attributesLength = _attributes.length; - for (uint256 i = 0; i < attributesLength; ++i) { - bytes4 selector = bytes4(_attributes[i]); - /// Finding the matching attribute selector. - uint256 attributeSelectorsLength = ATTRIBUTE_SELECTORS.length; - uint256 indexInSelectorsArray = attributeSelectorsLength; - for (uint256 j = 0; j < attributeSelectorsLength; ++j) { - if (selector == ATTRIBUTE_SELECTORS[j]) { - /// check if the attribute was already set. - require(!attributeUsed[j], AttributeAlreadySet(j)); - attributeUsed[j] = true; - indexInSelectorsArray = j; - break; - } - } - // Revert if the selector does not match any of the known attributes. - if (indexInSelectorsArray == attributeSelectorsLength) { - revert IERC7786GatewaySource.UnsupportedAttribute(selector); - } - // Checking whether selectors satisfy the restrictions. - if (_restriction == AttributeParsingRestrictions.OnlyInteropCallValue) { - require(indexInSelectorsArray == 0, AttributeNotForInteropCallValue(selector)); - } - if (indexInSelectorsArray < 2) { - require( - _restriction != AttributeParsingRestrictions.OnlyBundleAttributes, - AttributeNotForBundle(selector) - ); - } else { - require( - _restriction != AttributeParsingRestrictions.OnlyInteropCallValue, - AttributeNotForCall(selector) - ); - } - // setting the attributes - if (indexInSelectorsArray == 0) { - callAttributes.interopCallValue = AttributesDecoder.decodeUint256(_attributes[i]); - } else if (indexInSelectorsArray == 1) { - callAttributes.directCall = false; - callAttributes.indirectCallMessageValue = AttributesDecoder.decodeUint256(_attributes[i]); - } else if (indexInSelectorsArray == 2) { - bundleAttributes.executionAddress = AttributesDecoder.decodeAddress(_attributes[i]); - } else if (indexInSelectorsArray == 3) { - bundleAttributes.unbundlerAddress = AttributesDecoder.decodeAddress(_attributes[i]); - } + /// @notice Ensures the received base token value matches expected for the destination chain. + /// @param _destinationChainId Destination chain ID. + /// @param _totalValue Sum of requested interop call values. + function _ensureCorrectTotalValue(uint256 _destinationChainId, uint256 _totalValue) internal { + bytes32 destinationChainBaseTokenAssetId = BRIDGE_HUB.baseTokenAssetId(_destinationChainId); + // We burn the value that is passed along the bundle here, on source chain. + bytes32 thisChainBaseTokenAssetId = BRIDGE_HUB.baseTokenAssetId(block.chainid); + if (destinationChainBaseTokenAssetId == thisChainBaseTokenAssetId) { + require(msg.value == _totalValue, MsgValueMismatch(_totalValue, msg.value)); + // slither-disable-next-line arbitrary-send-eth + L2_BASE_TOKEN_SYSTEM_CONTRACT.burnMsgValue{value: _totalValue}(); + } else { + require(msg.value == 0, MsgValueMismatch(0, msg.value)); + IL2AssetRouter(assetRouter).bridgehubDepositBaseToken( + _destinationChainId, + destinationChainBaseTokenAssetId, + msg.sender, + _totalValue + ); } } - /*////////////////////////////////////////////////////////////// - Internal functions - //////////////////////////////////////////////////////////////*/ - - /// @notice Constructs and sends an InteropBundle. + /// @notice Constructs and sends an InteropBundle, that includes sending a message corresponding to the bundle via the L2 to L1 messenger. /// @param _destinationChainId Chain ID to send to. - /// @param _callStarters Array of InteropCallStarterInternal structs, corresponding the the calls in bundle. + /// @param _callStarters Array of InteropCallStarterInternal structs, corresponding to the calls in bundle. /// @param _bundleAttributes Attributes of the bundle. + /// @param _originalCallAttributes Original ERC-7786 attributes for each call to emit in MessageSent events. /// @return bundleHash Hash of the sent bundle. function _sendBundle( uint256 _destinationChainId, InteropCallStarterInternal[] memory _callStarters, - BundleAttributes memory _bundleAttributes + BundleAttributes memory _bundleAttributes, + bytes[][] memory _originalCallAttributes ) internal returns (bytes32 bundleHash) { // This will calculate how much value does all of the calls use cumulatively. uint256 totalCallsValue; @@ -325,10 +288,39 @@ contract InteropCenter is IInteropCenter, ReentrancyGuard, Ownable2StepUpgradeab InteropCall memory interopCall = _processCallStarter(_callStarters[i], _destinationChainId, msg.sender); bundle.calls[i] = interopCall; totalCallsValue += _callStarters[i].callAttributes.interopCallValue; + // For indirect calls, also account for the bridge message value that gets sent to the AssetRouter + if (_callStarters[i].callAttributes.indirectCall) { + totalCallsValue += _callStarters[i].callAttributes.indirectCallMessageValue; + } + } + + // Ensure that tokens required for bundle execution were received. + _ensureCorrectTotalValue(bundle.destinationChainId, totalCallsValue); + + bytes memory interopBundleBytes = abi.encode(bundle); + + // Send the message corresponding to the relevant InteropBundle to L1. + bytes32 msgHash = L2_TO_L1_MESSENGER_SYSTEM_CONTRACT.sendToL1( + bytes.concat(BUNDLE_IDENTIFIER, interopBundleBytes) + ); + + bundleHash = InteropDataEncoding.encodeInteropBundleHash(block.chainid, interopBundleBytes); + + // Emit ERC-7786 MessageSent event for each call in the bundle + for (uint256 i = 0; i < callStartersLength; ++i) { + InteropCall memory currentCall = bundle.calls[i]; + emit MessageSent({ + sendId: keccak256(abi.encodePacked(bundleHash, i)), + sender: InteroperableAddress.formatEvmV1(block.chainid, currentCall.from), + recipient: InteroperableAddress.formatEvmV1(_destinationChainId, currentCall.to), + payload: _callStarters[i].data, + value: _callStarters[i].callAttributes.interopCallValue, + attributes: _originalCallAttributes[i] + }); } - // Send the bundle. - bundleHash = _finalizeAndSendBundle({_bundle: bundle, _bundleCallsTotalValue: totalCallsValue}); + // Emit event stating that the bundle was sent out successfully. + emit InteropBundleSent(msgHash, bundleHash, bundle); } function _processCallStarter( @@ -336,15 +328,11 @@ contract InteropCenter is IInteropCenter, ReentrancyGuard, Ownable2StepUpgradeab uint256 _destinationChainId, address _sender ) internal returns (InteropCall memory interopCall) { - if (!_callStarter.callAttributes.directCall) { + if (_callStarter.callAttributes.indirectCall) { // slither-disable-next-line arbitrary-send-eth - InteropCallStarter memory actualCallStarter = IL2AssetRouter(_callStarter.nextContract) - .interopCenterInitiateBridge{value: _callStarter.callAttributes.indirectCallMessageValue}( - _destinationChainId, - _sender, - _callStarter.callAttributes.interopCallValue, - _callStarter.data - ); + InteropCallStarter memory actualCallStarter = IL2CrossChainSender(_callStarter.to).initiateBridging{ + value: _callStarter.callAttributes.indirectCallMessageValue + }(_destinationChainId, _sender, _callStarter.callAttributes.interopCallValue, _callStarter.data); // solhint-disable-next-line no-unused-vars // slither-disable-next-line unused-return (CallAttributes memory indirectCallAttributes, ) = this.parseAttributes( @@ -361,16 +349,16 @@ contract InteropCenter is IInteropCenter, ReentrancyGuard, Ownable2StepUpgradeab interopCall = InteropCall({ version: INTEROP_CALL_VERSION, shadowAccount: false, - to: actualCallStarter.nextContract, + to: actualCallStarter.to, data: actualCallStarter.data, value: _callStarter.callAttributes.interopCallValue, - from: _callStarter.nextContract + from: _callStarter.to }); } else { interopCall = InteropCall({ version: INTEROP_CALL_VERSION, shadowAccount: false, - to: _callStarter.nextContract, + to: _callStarter.to, data: _callStarter.data, value: _callStarter.callAttributes.interopCallValue, from: _sender @@ -397,9 +385,7 @@ contract InteropCenter is IInteropCenter, ReentrancyGuard, Ownable2StepUpgradeab bytes32 _assetId, uint256 _amount ) external override onlySettlementLayerRelayedSender { - if (L1_CHAIN_ID == block.chainid) { - revert NotInGatewayMode(); - } + require(L1_CHAIN_ID != block.chainid, NotInGatewayMode()); if (_baseTokenAmount > 0) { IAssetTracker(L2_ASSET_TRACKER_ADDR).handleChainBalanceIncrease( _chainId, @@ -419,6 +405,68 @@ contract InteropCenter is IInteropCenter, ReentrancyGuard, Ownable2StepUpgradeab ERC 7786 //////////////////////////////////////////////////////////////*/ + /// @notice Parses the attributes of the call or bundle. + /// @param _attributes ERC-7786 Attributes of the call or bundle. + /// @param _restriction Restriction for parsing attributes. + function parseAttributes( + bytes[] calldata _attributes, + AttributeParsingRestrictions _restriction + ) public pure returns (CallAttributes memory callAttributes, BundleAttributes memory bundleAttributes) { + // Default value is direct call. + callAttributes.indirectCall = false; + + bytes4[4] memory ATTRIBUTE_SELECTORS = _getERC7786AttributeSelectors(); + // We can only pass each attribute once. + bool[] memory attributeUsed = new bool[](ATTRIBUTE_SELECTORS.length); + + uint256 attributesLength = _attributes.length; + for (uint256 i = 0; i < attributesLength; ++i) { + bytes4 selector = bytes4(_attributes[i]); + + if (selector == IERC7786Attributes.interopCallValue.selector) { + require(!attributeUsed[0], AttributeAlreadySet(selector)); + require( + _restriction == AttributeParsingRestrictions.OnlyInteropCallValue || + _restriction == AttributeParsingRestrictions.OnlyCallAttributes || + _restriction == AttributeParsingRestrictions.CallAndBundleAttributes, + AttributeViolatesRestriction(selector, uint256(_restriction)) + ); + attributeUsed[0] = true; + callAttributes.interopCallValue = AttributesDecoder.decodeUint256(_attributes[i]); + } else if (selector == IERC7786Attributes.indirectCall.selector) { + require(!attributeUsed[1], AttributeAlreadySet(selector)); + require( + _restriction == AttributeParsingRestrictions.OnlyCallAttributes || + _restriction == AttributeParsingRestrictions.CallAndBundleAttributes, + AttributeViolatesRestriction(selector, uint256(_restriction)) + ); + attributeUsed[1] = true; + callAttributes.indirectCall = true; + callAttributes.indirectCallMessageValue = AttributesDecoder.decodeUint256(_attributes[i]); + } else if (selector == IERC7786Attributes.executionAddress.selector) { + require(!attributeUsed[2], AttributeAlreadySet(selector)); + require( + _restriction == AttributeParsingRestrictions.OnlyBundleAttributes || + _restriction == AttributeParsingRestrictions.CallAndBundleAttributes, + AttributeViolatesRestriction(selector, uint256(_restriction)) + ); + attributeUsed[2] = true; + bundleAttributes.executionAddress = AttributesDecoder.decodeInteroperableAddress(_attributes[i]); + } else if (selector == IERC7786Attributes.unbundlerAddress.selector) { + require(!attributeUsed[3], AttributeAlreadySet(selector)); + require( + _restriction == AttributeParsingRestrictions.OnlyBundleAttributes || + _restriction == AttributeParsingRestrictions.CallAndBundleAttributes, + AttributeViolatesRestriction(selector, uint256(_restriction)) + ); + attributeUsed[3] = true; + bundleAttributes.unbundlerAddress = AttributesDecoder.decodeInteroperableAddress(_attributes[i]); + } else { + revert IERC7786GatewaySource.UnsupportedAttribute(selector); + } + } + } + /// @notice Checks if the attribute selector is supported by the InteropCenter. /// @param _attributeSelector The attribute selector to check. /// @return True if the attribute selector is supported, false otherwise. diff --git a/l1-contracts/contracts/interop/InteropErrors.sol b/l1-contracts/contracts/interop/InteropErrors.sol index 3626e6fb97..f6e732b589 100644 --- a/l1-contracts/contracts/interop/InteropErrors.sol +++ b/l1-contracts/contracts/interop/InteropErrors.sol @@ -1,14 +1,10 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.21; -// 0x04bb396e -error AttributeAlreadySet(uint256 index); -// 0xcd73770b -error AttributeNotForBundle(bytes4 selector); -// 0x2531ea93 -error AttributeNotForCall(bytes4 selector); -// 0xec11242f -error AttributeNotForInteropCallValue(bytes4 selector); +// 0x9031f751 +error AttributeAlreadySet(bytes4 selector); +// 0xbcb41ec7 +error AttributeViolatesRestriction(bytes4 selector, uint256 restriction); // 0x5bba5111 error BundleAlreadyProcessed(bytes32 bundleHash); // 0xa43d2953 @@ -19,13 +15,17 @@ error CallAlreadyExecuted(bytes32 bundleHash, uint256 callIndex); error CallNotExecutable(bytes32 bundleHash, uint256 callIndex); // 0xf729f26d error CanNotUnbundle(bytes32 bundleHash); -// 0x44fe431f -error ExecutingNotAllowed(bytes32 bundleHash, address callerAddress, address executionAddress); +// 0xe845be4c +error ExecutingNotAllowed(bytes32 bundleHash, bytes callerAddress, bytes executionAddress); // 0x62d214aa error IndirectCallValueMismatch(uint256 expected, uint256 actual); // 0x32c2e156 error MessageNotIncluded(); -// 0x924f9fb1 -error UnbundlingNotAllowed(bytes32 bundleHash, address callerAddress, address unbundlerAddress); +// 0x89fd2c76 +error UnauthorizedMessageSender(address expected, address actual); +// 0x0345c281 +error UnbundlingNotAllowed(bytes32 bundleHash, bytes callerAddress, bytes unbundlerAddress); // 0x801534e9 error WrongCallStatusLength(uint256 bundleCallsLength, uint256 providedCallStatusLength); +// 0x4534e972 +error WrongDestinationChainId(bytes32 bundleHash, uint256 expected, uint256 actual); diff --git a/l1-contracts/contracts/interop/InteropHandler.sol b/l1-contracts/contracts/interop/InteropHandler.sol index c0e5bf33a2..5df3898553 100644 --- a/l1-contracts/contracts/interop/InteropHandler.sol +++ b/l1-contracts/contracts/interop/InteropHandler.sol @@ -2,14 +2,15 @@ pragma solidity ^0.8.24; -import {L2_BASE_TOKEN_SYSTEM_CONTRACT, L2_MESSAGE_VERIFICATION} from "../common/l2-helpers/L2ContractAddresses.sol"; +import {L2_BASE_TOKEN_SYSTEM_CONTRACT, L2_MESSAGE_VERIFICATION, L2_INTEROP_CENTER_ADDR} from "../common/l2-helpers/L2ContractAddresses.sol"; import {IInteropHandler} from "./IInteropHandler.sol"; import {BUNDLE_IDENTIFIER, InteropBundle, InteropCall, MessageInclusionProof, CallStatus, BundleStatus} from "../common/Messaging.sol"; -import {IERC7786Receiver} from "./IERC7786Receiver.sol"; +import {IERC7786Recipient} from "./IERC7786Recipient.sol"; import {ReentrancyGuard} from "../common/ReentrancyGuard.sol"; import {InteropDataEncoding} from "./InteropDataEncoding.sol"; -import {MessageNotIncluded, BundleAlreadyProcessed, CanNotUnbundle, CallAlreadyExecuted, CallNotExecutable, WrongCallStatusLength, UnbundlingNotAllowed, ExecutingNotAllowed, BundleVerifiedAlready} from "./InteropErrors.sol"; -import {InvalidSelector} from "../common/L1ContractErrors.sol"; +import {InteroperableAddress} from "@openzeppelin/contracts-master/utils/draft-InteroperableAddress.sol"; +import {MessageNotIncluded, BundleAlreadyProcessed, CanNotUnbundle, CallAlreadyExecuted, CallNotExecutable, WrongCallStatusLength, UnbundlingNotAllowed, ExecutingNotAllowed, BundleVerifiedAlready, UnauthorizedMessageSender, WrongDestinationChainId} from "./InteropErrors.sol"; +import {Unauthorized, InvalidSelector} from "../common/L1ContractErrors.sol"; /// @title InteropHandler /// @author Matter Labs @@ -36,22 +37,40 @@ contract InteropHandler is IInteropHandler, ReentrancyGuard { _proof.chainId ); - // Verify that the caller has permission to execute the bundle. - // Note, that in case the executionAddress wasn't specified in the bundle then executing is permissionless, as documented in Messaging.sol + // Verify that the destination chainId of the bundle is equal to the chainId where it's trying to get executed require( - (interopBundle.bundleAttributes.executionAddress == address(0) || - msg.sender == interopBundle.bundleAttributes.executionAddress), - ExecutingNotAllowed(bundleHash, msg.sender, interopBundle.bundleAttributes.executionAddress) + interopBundle.destinationChainId == block.chainid, + WrongDestinationChainId(bundleHash, interopBundle.destinationChainId, block.chainid) ); - // We shouldn't process bundles which are either fully executed, or were unbundled here. - // If the bundle if fully executed, it's not expected that anything else should be done with the bundle, it's finalized already. - // If the bundle were unbundled, it's either fully finalized (all calls are cancelled or executed), in which case nothing else could be done, similar to above, - // or some of the calls are still unprocessed, in this case they should be processed via unbundling. - if (status == BundleStatus.FullyExecuted || status == BundleStatus.Unbundled) { - revert BundleAlreadyProcessed(bundleHash); + // If the execution address is not specified then the execution is permissionless. + if (interopBundle.bundleAttributes.executionAddress.length != 0) { + (uint256 executionChainId, address executionAddress) = InteroperableAddress.parseEvmV1( + interopBundle.bundleAttributes.executionAddress + ); + + // Verify that the caller has permission to execute the bundle. + // Note, that in case the executionAddress wasn't specified in the bundle then executing is permissionless, as documented in Messaging.sol + // It's also possible that the caller is InteropHandler itself, in case the execution was initiated through receiveMessage. + require( + (msg.sender == address(this) || + ((executionChainId == block.chainid || executionChainId == 0) && executionAddress == msg.sender)), + ExecutingNotAllowed( + bundleHash, + InteroperableAddress.formatEvmV1(block.chainid, msg.sender), + interopBundle.bundleAttributes.executionAddress + ) + ); } + // We can only process bundles that are either unreceived (first time processing) or verified (already verified but not executed). + // This whitelist approach ensures that if new bundle statuses are added in the future, they will be explicitly rejected + // until they are explicitly allowed, preventing potential security vulnerabilities. + require( + status == BundleStatus.Unreceived || status == BundleStatus.Verified, + BundleAlreadyProcessed(bundleHash) + ); + // Verify the bundle inclusion, if not done yet. if (status != BundleStatus.Verified) _verifyBundle(_bundle, _proof, bundleHash); @@ -85,12 +104,22 @@ contract InteropHandler is IInteropHandler, ReentrancyGuard { /// @param _proof Inclusion proof for the bundle message. The bundle message itself gets broadcasted by InteropCenter contract whenever a bundle is sent. function verifyBundle(bytes memory _bundle, MessageInclusionProof memory _proof) public nonReentrant { // Decode the bundle data, calculate its hash and get the current status of the bundle. - (, bytes32 bundleHash, BundleStatus status) = _getBundleData(_bundle, _proof.chainId); + (InteropBundle memory interopBundle, bytes32 bundleHash, BundleStatus status) = _getBundleData( + _bundle, + _proof.chainId + ); + + // Verify that the destination chainId of the bundle is equal to the chainId where it's trying to get verified + require( + interopBundle.destinationChainId == block.chainid, + WrongDestinationChainId(bundleHash, interopBundle.destinationChainId, block.chainid) + ); // If the bundle was already fully executed or unbundled, we revert stating that it was processed already. - if (status != BundleStatus.Unreceived && status != BundleStatus.Verified) { - revert BundleAlreadyProcessed(bundleHash); - } + require( + status == BundleStatus.Unreceived || status == BundleStatus.Verified, + BundleAlreadyProcessed(bundleHash) + ); // Revert if the bundle was verified already. require(status != BundleStatus.Verified, BundleVerifiedAlready(bundleHash)); @@ -115,10 +144,26 @@ contract InteropHandler is IInteropHandler, ReentrancyGuard { _sourceChainId ); + // Verify that the destination chainId of the bundle is equal to the chainId where it's trying to get unbundled + require( + interopBundle.destinationChainId == block.chainid, + WrongDestinationChainId(bundleHash, interopBundle.destinationChainId, block.chainid) + ); + + (uint256 unbundlerChainId, address unbundlerAddress) = InteroperableAddress.parseEvmV1( + interopBundle.bundleAttributes.unbundlerAddress + ); + // Verify that the caller has permission to unbundle the bundle. + // It's also possible that the caller is InteropHandler itself, in case the unbundling was initiated through receiveMessage. require( - msg.sender == interopBundle.bundleAttributes.unbundlerAddress, - UnbundlingNotAllowed(bundleHash, msg.sender, interopBundle.bundleAttributes.unbundlerAddress) + msg.sender == address(this) || + ((unbundlerChainId == block.chainid || unbundlerChainId == 0) && unbundlerAddress == msg.sender), + UnbundlingNotAllowed( + bundleHash, + InteroperableAddress.formatEvmV1(block.chainid, msg.sender), + interopBundle.bundleAttributes.unbundlerAddress + ) ); // Verify that the provided call statuses array has the same length as the number of calls in the bundle. @@ -130,9 +175,7 @@ contract InteropHandler is IInteropHandler, ReentrancyGuard { // The bundle status have to be either verified (we know that it's received, but not processed yet), or unbundled. // Note, that on the first call to unbundle the status of the bundle should be verified. - if (status == BundleStatus.Unreceived || status == BundleStatus.FullyExecuted) { - revert CanNotUnbundle(bundleHash); - } + require(status == BundleStatus.Verified || status == BundleStatus.Unbundled, CanNotUnbundle(bundleHash)); // Mark the given bundle as unbundled, following CEI pattern. bundleStatus[bundleHash] = BundleStatus.Unbundled; @@ -143,7 +186,7 @@ contract InteropHandler is IInteropHandler, ReentrancyGuard { CallStatus recordedCallStatus = callStatus[bundleHash][i]; CallStatus requestedCallStatus = _providedCallStatus[i]; if (requestedCallStatus == CallStatus.Executed) { - // We can only executed unprocessed calls. + // We can only execute unprocessed calls. require(recordedCallStatus == CallStatus.Unprocessed, CallNotExecutable(bundleHash, i)); callStatus[bundleHash][i] = CallStatus.Executed; emit CallProcessed(bundleHash, i, CallStatus.Executed); @@ -215,14 +258,12 @@ contract InteropHandler is IInteropHandler, ReentrancyGuard { L2_BASE_TOKEN_SYSTEM_CONTRACT.mint(address(this), interopCall.value); // slither-disable-next-line arbitrary-send-eth - bytes4 selector = IERC7786Receiver(interopCall.to).executeMessage{value: interopCall.value}({ - messageId: _bundleHash, - sourceChain: _sourceChainId, - sender: interopCall.from, - payload: interopCall.data, - attributes: new bytes[](0) + bytes4 selector = IERC7786Recipient(interopCall.to).receiveMessage{value: interopCall.value}({ + receiveId: keccak256(abi.encodePacked(_bundleHash, i)), + sender: InteroperableAddress.formatEvmV1(_sourceChainId, interopCall.from), + payload: interopCall.data }); // attributes are not supported yet - require(selector == IERC7786Receiver.executeMessage.selector, InvalidSelector(selector)); + require(selector == IERC7786Recipient.receiveMessage.selector, InvalidSelector(selector)); } } @@ -232,6 +273,12 @@ contract InteropHandler is IInteropHandler, ReentrancyGuard { /// @param _bundleHash Hash corresponding to the bundle that is to be verified. /// That message gets sent to L1 by origin chain in InteropCenter contract, and is picked up and included in receiving chain by sequencer. function _verifyBundle(bytes memory _bundle, MessageInclusionProof memory _proof, bytes32 _bundleHash) internal { + // Verify that the message came from the legitimate InteropCenter + require( + _proof.message.sender == L2_INTEROP_CENTER_ADDR, + UnauthorizedMessageSender(L2_INTEROP_CENTER_ADDR, _proof.message.sender) + ); + // Substitute provided message data with data corresponding to the bundle currently being verified. _proof.message.data = bytes.concat(BUNDLE_IDENTIFIER, _bundle); @@ -250,4 +297,101 @@ contract InteropHandler is IInteropHandler, ReentrancyGuard { // Emit event stating that the bundle was verified. emit BundleVerified(_bundleHash); } + + /// @notice The sole purpose of this function is to serve as a rescue mechanism in case the sender is a contract, + /// the unbundler chainid is set to the sender chainid and the unbundler address is set to the contract's address. + /// In particular, this happens when the unbundler is not specified. + /// In such a case, the contract might nol be able to call `InteropHandler.unbundleBundle` directly. + /// Instead, it's able to send another bundle which calls `InteropHandler.unbundleBundle` via the `receiveMessage` function. + /// @dev Implements ERC-7786 recipient interface. The payload must be encoded using abi.encodeCall + /// with one of the following function selectors: + /// - executeBundle: payload = abi.encodeCall(InteropHandler.executeBundle, (bundle, proof)) + /// - unbundleBundle: payload = abi.encodeCall(InteropHandler.unbundleBundle, (sourceChainId, bundle, providedCallStatus)) + /// The sender must have appropriate permissions (executionAddress or unbundlerAddress) which are + /// validated before calling the respective internal functions. Since this function validates + /// permissions, the called functions (executeBundle/unbundleBundle) will bypass their own + /// permission checks when called from this contract (msg.sender == address(this)). + /// @param sender ERC-7930 interoperable address of the message sender. + /// @param payload ABI-encoded function call data with selector and parameters. + /// @return selector The function selector of this receiveMessage function, as per ERC-7786. + function receiveMessage( + bytes32 /* receiveId */, + bytes calldata sender, + bytes calldata payload + ) external payable nonReentrant returns (bytes4) { + // Verify that call to this function is a result of a call being executed, meaning this message came from a valid bundle. + // This is the only way receiveMessage can be invoked on InteropHandler by itself. + require(msg.sender == address(this), Unauthorized(msg.sender)); + + bytes4 selector = bytes4(payload[:4]); + + (uint256 senderChainId, address senderAddress) = InteroperableAddress.parseEvmV1Calldata(sender); + + if (selector == this.executeBundle.selector) { + _handleExecuteBundle(payload, senderChainId, senderAddress, sender); + } else if (selector == this.unbundleBundle.selector) { + _handleUnbundleBundle(payload, senderChainId, senderAddress, sender); + } else { + revert InvalidSelector(selector); + } + + return IERC7786Recipient.receiveMessage.selector; + } + + function _handleExecuteBundle( + bytes calldata payload, + uint256 senderChainId, + address senderAddress, + bytes calldata sender + ) internal { + (bytes memory bundle, MessageInclusionProof memory proof) = abi.decode( + payload[4:], + (bytes, MessageInclusionProof) + ); + + // Decode the bundle to get execution permissions + (InteropBundle memory interopBundle, , ) = _getBundleData(bundle, proof.chainId); + + // If the execution address is not specified then the execution is permissionless. + if (interopBundle.bundleAttributes.executionAddress.length != 0) { + (uint256 executionChainId, address executionAddress) = InteroperableAddress.parseEvmV1( + interopBundle.bundleAttributes.executionAddress + ); + + // Verify sender has execution permission + require( + (executionChainId == senderChainId || executionChainId == 0) && executionAddress == senderAddress, + ExecutingNotAllowed(keccak256(bundle), sender, interopBundle.bundleAttributes.executionAddress) + ); + } + + this.executeBundle(bundle, proof); + } + + function _handleUnbundleBundle( + bytes calldata payload, + uint256 senderChainId, + address senderAddress, + bytes calldata sender + ) internal { + (uint256 sourceChainId, bytes memory bundle, CallStatus[] memory providedCallStatus) = abi.decode( + payload[4:], + (uint256, bytes, CallStatus[]) + ); + + // Decode the bundle to get unbundling permissions + (InteropBundle memory interopBundle, , ) = _getBundleData(bundle, sourceChainId); + + (uint256 unbundlerChainId, address unbundlerAddress) = InteroperableAddress.parseEvmV1( + interopBundle.bundleAttributes.unbundlerAddress + ); + + // Verify sender has unbundling permission + require( + (unbundlerChainId == senderChainId || unbundlerChainId == 0) && unbundlerAddress == senderAddress, + UnbundlingNotAllowed(keccak256(bundle), sender, interopBundle.bundleAttributes.unbundlerAddress) + ); + + this.unbundleBundle(sourceChainId, bundle, providedCallStatus); + } } diff --git a/l1-contracts/contracts/state-transition/L1StateTransitionErrors.sol b/l1-contracts/contracts/state-transition/L1StateTransitionErrors.sol index 5c51c506f2..5fe2f49035 100644 --- a/l1-contracts/contracts/state-transition/L1StateTransitionErrors.sol +++ b/l1-contracts/contracts/state-transition/L1StateTransitionErrors.sol @@ -20,8 +20,6 @@ error ExecutedIsNotConsistentWithVerified(uint256 batchesExecuted, uint256 batch error InitialForceDeploymentMismatch(bytes32 forceDeploymentHash, bytes32 initialForceDeploymentHash); // 0xfbd630b8 error InvalidBatchesDataLength(uint256 batchesDataLength, uint256 priorityOpsDataLength); -// 0x7a47c9a2 -error InvalidChainId(); // 0xc06789fa error InvalidCommitment(); // 0xd2531c15 diff --git a/l1-contracts/contracts/state-transition/chain-deps/facets/Mailbox.sol b/l1-contracts/contracts/state-transition/chain-deps/facets/Mailbox.sol index e1cc550fe6..e4865db227 100644 --- a/l1-contracts/contracts/state-transition/chain-deps/facets/Mailbox.sol +++ b/l1-contracts/contracts/state-transition/chain-deps/facets/Mailbox.sol @@ -25,8 +25,8 @@ import {L2_BOOTLOADER_ADDRESS, L2_INTEROP_CENTER_ADDR} from "../../../common/l2- import {IL1AssetRouter} from "../../../bridge/asset-router/IL1AssetRouter.sol"; -import {BaseTokenGasPriceDenominatorNotSet, BatchNotExecuted, GasPerPubdataMismatch, MsgValueTooLow, OnlyEraSupported, TooManyFactoryDeps, TransactionNotAllowed} from "../../../common/L1ContractErrors.sol"; -import {InvalidChainId, LocalRootIsZero, LocalRootMustBeZero, NotHyperchain, NotL1, NotSettlementLayer} from "../../L1StateTransitionErrors.sol"; +import {BaseTokenGasPriceDenominatorNotSet, BatchNotExecuted, GasPerPubdataMismatch, MsgValueTooLow, OnlyEraSupported, TooManyFactoryDeps, TransactionNotAllowed, InvalidChainId} from "../../../common/L1ContractErrors.sol"; +import {LocalRootIsZero, LocalRootMustBeZero, NotHyperchain, NotL1, NotSettlementLayer} from "../../L1StateTransitionErrors.sol"; // While formally the following import is not used, it is needed to inherit documentation from it import {IZKChainBase} from "../../chain-interfaces/IZKChainBase.sol"; diff --git a/l1-contracts/foundry.toml b/l1-contracts/foundry.toml index 2ca6df022b..ec571581bd 100644 --- a/l1-contracts/foundry.toml +++ b/l1-contracts/foundry.toml @@ -1,5 +1,5 @@ [profile.default] -allow_paths = ["../l2-contracts/contracts"] +allow_paths = ["../l2-contracts/contracts", "../lib"] src = "contracts" out = "out" libs = ["./lib"] @@ -42,6 +42,7 @@ remappings = [ "l2-contracts/=../l2-contracts/contracts/", "@openzeppelin/contracts-v4/=lib/openzeppelin-contracts-v4/contracts/", "@openzeppelin/contracts-upgradeable-v4/=lib/openzeppelin-contracts-upgradeable-v4/contracts/", + "@openzeppelin/contracts-master/=lib/openzeppelin-contracts-master/contracts/", ] optimizer = true optimizer_runs = 9999999 diff --git a/l1-contracts/lib/openzeppelin-contracts-master b/l1-contracts/lib/openzeppelin-contracts-master new file mode 160000 index 0000000000..32e7a6ffbc --- /dev/null +++ b/l1-contracts/lib/openzeppelin-contracts-master @@ -0,0 +1 @@ +Subproject commit 32e7a6ffbc5af9ab0e6dfdbc58508511d0f0b4a2 diff --git a/l1-contracts/test/foundry/l1/integration/l2-tests-abstract/L2Erc20TestAbstract.t.sol b/l1-contracts/test/foundry/l1/integration/l2-tests-abstract/L2Erc20TestAbstract.t.sol index 9d969d7413..ccbdd5de85 100644 --- a/l1-contracts/test/foundry/l1/integration/l2-tests-abstract/L2Erc20TestAbstract.t.sol +++ b/l1-contracts/test/foundry/l1/integration/l2-tests-abstract/L2Erc20TestAbstract.t.sol @@ -32,13 +32,17 @@ import {SystemContractsArgs} from "./Utils.sol"; import {DeployUtils} from "deploy-scripts/DeployUtils.s.sol"; import {TestnetERC20Token} from "contracts/dev-contracts/TestnetERC20Token.sol"; import {IERC7786Attributes} from "contracts/interop/IERC7786Attributes.sol"; -import {IERC7786GatewaySource} from "contracts/interop/IERC7786.sol"; +import {IERC7786GatewaySource} from "contracts/interop/IERC7786GatewaySource.sol"; +import {InteroperableAddress} from "@openzeppelin/contracts-master/utils/draft-InteroperableAddress.sol"; import {SharedL2ContractDeployer} from "./_SharedL2ContractDeployer.sol"; import {BUNDLE_IDENTIFIER, BridgehubL2TransactionRequest, InteropBundle, InteropCall, InteropCallStarter, L2CanonicalTransaction, L2Log, L2Message, TxStatus} from "contracts/common/Messaging.sol"; import {GasFields, InteropTrigger, TRIGGER_IDENTIFIER} from "contracts/dev-contracts/test/Utils.sol"; abstract contract L2Erc20TestAbstract is Test, SharedL2ContractDeployer { + address constant UNBUNDLER_ADDRESS = address(0x1); + address constant EXECUTION_ADDRESS = address(0x2); + function performDeposit(address depositor, address receiver, uint256 amount) internal { vm.prank(aliasedL1AssetRouter); L2AssetRouter(L2_ASSET_ROUTER_ADDR).finalizeDeposit({ @@ -139,12 +143,13 @@ abstract contract L2Erc20TestAbstract is Test, SharedL2ContractDeployer { ); InteropCallStarter[] memory calls = new InteropCallStarter[](1); - bytes[] memory attributes = new bytes[](1); - attributes[0] = abi.encodeCall(IERC7786Attributes.indirectCall, (0)); + bytes[] memory callAttributes = new bytes[](1); + callAttributes[0] = abi.encodeCall(IERC7786Attributes.indirectCall, (0)); + calls[0] = InteropCallStarter({ - nextContract: L2_ASSET_ROUTER_ADDR, + to: L2_ASSET_ROUTER_ADDR, data: secondBridgeCalldata, - callAttributes: attributes + callAttributes: callAttributes }); uint256 destinationChainId = 271; @@ -164,11 +169,14 @@ abstract contract L2Erc20TestAbstract is Test, SharedL2ContractDeployer { abi.encodeWithSelector(L2_BASE_TOKEN_SYSTEM_CONTRACT.burnMsgValue.selector), abi.encode(bytes("")) ); - l2InteropCenter.sendBundle(271, calls, new bytes[](0)); - } - address constant UNBUNDLER_ADDRESS = address(0x1); - address constant EXECUTION_ADDRESS = address(0x2); + bytes[] memory bundleAttributes = new bytes[](1); + bundleAttributes[0] = abi.encodeCall( + IERC7786Attributes.unbundlerAddress, + (InteroperableAddress.formatEvmV1(UNBUNDLER_ADDRESS)) + ); + l2InteropCenter.sendBundle(271, calls, bundleAttributes); + } function test_requestSendCall() public { address l2TokenAddress = initializeTokenByDeposit(); @@ -183,10 +191,16 @@ abstract contract L2Erc20TestAbstract is Test, SharedL2ContractDeployer { InteropCallStarter[] memory calls = new InteropCallStarter[](1); bytes[] memory attributes = new bytes[](3); attributes[0] = abi.encodeCall(IERC7786Attributes.indirectCall, (0)); - attributes[1] = abi.encodeCall(IERC7786Attributes.executionAddress, (EXECUTION_ADDRESS)); - attributes[2] = abi.encodeCall(IERC7786Attributes.unbundlerAddress, (UNBUNDLER_ADDRESS)); + attributes[1] = abi.encodeCall( + IERC7786Attributes.executionAddress, + (InteroperableAddress.formatEvmV1(EXECUTION_ADDRESS)) + ); + attributes[2] = abi.encodeCall( + IERC7786Attributes.unbundlerAddress, + (InteroperableAddress.formatEvmV1(UNBUNDLER_ADDRESS)) + ); calls[0] = InteropCallStarter({ - nextContract: L2_ASSET_ROUTER_ADDR, + to: L2_ASSET_ROUTER_ADDR, data: secondBridgeCalldata, callAttributes: attributes }); @@ -208,9 +222,8 @@ abstract contract L2Erc20TestAbstract is Test, SharedL2ContractDeployer { abi.encodeWithSelector(L2_BASE_TOKEN_SYSTEM_CONTRACT.burnMsgValue.selector), abi.encode(bytes("")) ); - IERC7786GatewaySource(address(l2InteropCenter)).sendCall( - 271, - calls[0].nextContract, + IERC7786GatewaySource(address(l2InteropCenter)).sendMessage( + InteroperableAddress.formatEvmV1(271, calls[0].to), calls[0].data, calls[0].callAttributes ); diff --git a/l1-contracts/test/foundry/l1/integration/l2-tests-abstract/L2InteropTestAbstract.t.sol b/l1-contracts/test/foundry/l1/integration/l2-tests-abstract/L2InteropTestAbstract.t.sol index fef01a0374..685dcbfb91 100644 --- a/l1-contracts/test/foundry/l1/integration/l2-tests-abstract/L2InteropTestAbstract.t.sol +++ b/l1-contracts/test/foundry/l1/integration/l2-tests-abstract/L2InteropTestAbstract.t.sol @@ -26,8 +26,11 @@ import {IL1Nullifier} from "contracts/bridge/interfaces/IL1Nullifier.sol"; import {IL1AssetRouter} from "contracts/bridge/asset-router/IL1AssetRouter.sol"; import {IAssetRouterBase} from "contracts/bridge/asset-router/IAssetRouterBase.sol"; import {IInteropCenter} from "contracts/interop/IInteropCenter.sol"; +import {InteropCenter} from "contracts/interop/InteropCenter.sol"; import {IInteropHandler, CallStatus} from "contracts/interop/IInteropHandler.sol"; import {IERC7786Attributes} from "contracts/interop/IERC7786Attributes.sol"; +import {UnauthorizedMessageSender, WrongDestinationChainId} from "contracts/interop/InteropErrors.sol"; +import {InteroperableAddress} from "@openzeppelin/contracts-master/utils/draft-InteroperableAddress.sol"; import {IAssetRouterBase, NEW_ENCODING_VERSION} from "contracts/bridge/asset-router/IAssetRouterBase.sol"; import {IChainTypeManager} from "contracts/state-transition/IChainTypeManager.sol"; @@ -42,6 +45,8 @@ import {DummyL2StandardTriggerAccount} from "../../../../../contracts/dev-contra import {IMessageVerification} from "contracts/state-transition/chain-interfaces/IMessageVerification.sol"; import {L2_INTEROP_ACCOUNT_ADDR, L2_STANDARD_TRIGGER_ACCOUNT_ADDR} from "./Utils.sol"; import {GasFields, InteropTrigger, TRIGGER_IDENTIFIER} from "contracts/dev-contracts/test/Utils.sol"; +import {InteropDataEncoding} from "contracts/interop/InteropDataEncoding.sol"; +import {InteropHandler} from "contracts/interop/InteropHandler.sol"; abstract contract L2InteropTestAbstract is Test, SharedL2ContractDeployer { address constant UNBUNDLER_ADDRESS = address(0x1); @@ -86,7 +91,8 @@ abstract contract L2InteropTestAbstract is Test, SharedL2ContractDeployer { // (bool success, ) = recipient.call(data); // assertTrue(success); } - function getInclusionProof() public view returns (MessageInclusionProof memory) { + + function getInclusionProof(address messageSender) public view returns (MessageInclusionProof memory) { bytes32[] memory proof = new bytes32[](27); proof[0] = bytes32(0x010f050000000000000000000000000000000000000000000000000000000000); proof[1] = bytes32(0x72abee45b59e344af8a6e520241c4744aff26ed411f4c4b00f8af09adada43ba); @@ -123,7 +129,7 @@ abstract contract L2InteropTestAbstract is Test, SharedL2ContractDeployer { l2MessageIndex: 0, message: L2Message( 0, - address(0x0000000000000000000000000000000000010003), + address(messageSender), hex"9c884fd1000000000000000000000000000000000000000000000000000000000000010f76b59944c0e577e988c1b823ef4ad168478ddfe6044cca433996ade7637ec70d00000000000000000000000083aeb38092d5f5a5cf7fb8ccf94c981c1d37d81300000000000000000000000083aeb38092d5f5a5cf7fb8ccf94c981c1d37d813000000000000000000000000ee0dcf9b8c3048530fd6b2211ae3ba32e8590905000000000000000000000000000000000000000000000000000000000000000100000000000000000000000000000000000000000000000000000000000000a000000000000000000000000000000000000000000000000000000000000001c1010000000000000000000000000000000000000000000000000000000000000009000000000000000000000000000000000000000000000000000000000000008000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000000000180000000000000000000000000000000000000000000000000000000000000006000000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000004574254430000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000600000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000000457425443000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000000800000000000000000000000000000000000000000000000000000000000000" ), proof: proof @@ -131,7 +137,7 @@ abstract contract L2InteropTestAbstract is Test, SharedL2ContractDeployer { } function test_l2MessageVerification() public { - MessageInclusionProof memory proof = getInclusionProof(); + MessageInclusionProof memory proof = getInclusionProof(L2_INTEROP_CENTER_ADDR); L2_MESSAGE_VERIFICATION.proveL2MessageInclusionShared( proof.chainId, proof.l1BatchNumber, @@ -153,7 +159,7 @@ abstract contract L2InteropTestAbstract is Test, SharedL2ContractDeployer { function test_executeBundle() public { InteropBundle memory interopBundle = getInteropBundle(1); bytes memory bundle = abi.encode(interopBundle); - MessageInclusionProof memory proof = getInclusionProof(); + MessageInclusionProof memory proof = getInclusionProof(L2_INTEROP_CENTER_ADDR); vm.mockCall( address(L2_MESSAGE_VERIFICATION), abi.encodeWithSelector(L2_MESSAGE_VERIFICATION.proveL2MessageInclusionShared.selector), @@ -164,14 +170,31 @@ abstract contract L2InteropTestAbstract is Test, SharedL2ContractDeployer { abi.encodeWithSelector(L2_BASE_TOKEN_SYSTEM_CONTRACT.mint.selector), abi.encode(bytes("")) ); + bytes32 bundleHash = InteropDataEncoding.encodeInteropBundleHash(proof.chainId, bundle); + // Expect event + vm.expectEmit(true, false, false, false); + emit IInteropHandler.BundleExecuted(bundleHash); vm.prank(EXECUTION_ADDRESS); IInteropHandler(L2_INTEROP_HANDLER_ADDR).executeBundle(bundle, proof); + // Check storage changes + assertEq( + uint256(InteropHandler(L2_INTEROP_HANDLER_ADDR).bundleStatus(bundleHash)), + 2, + "BundleStatus should be FullyExecuted" + ); + for (uint256 i = 0; i < interopBundle.calls.length; ++i) { + assertEq( + uint256(InteropHandler(L2_INTEROP_HANDLER_ADDR).callStatus(bundleHash, i)), + 1, + "CallStatus should be Executed" + ); + } } function test_unbundleBundle() public { InteropBundle memory interopBundle = getInteropBundle(3); bytes memory bundle = abi.encode(interopBundle); - MessageInclusionProof memory proof = getInclusionProof(); + MessageInclusionProof memory proof = getInclusionProof(L2_INTEROP_CENTER_ADDR); vm.mockCall( address(L2_MESSAGE_VERIFICATION), abi.encodeWithSelector(L2_MESSAGE_VERIFICATION.proveL2MessageInclusionShared.selector), @@ -182,6 +205,7 @@ abstract contract L2InteropTestAbstract is Test, SharedL2ContractDeployer { abi.encodeWithSelector(L2_BASE_TOKEN_SYSTEM_CONTRACT.mint.selector), abi.encode(bytes("")) ); + bytes32 bundleHash = InteropDataEncoding.encodeInteropBundleHash(proof.chainId, bundle); IInteropHandler(L2_INTEROP_HANDLER_ADDR).verifyBundle(bundle, proof); CallStatus[] memory callStatuses1 = new CallStatus[](3); callStatuses1[0] = CallStatus.Unprocessed; @@ -191,10 +215,64 @@ abstract contract L2InteropTestAbstract is Test, SharedL2ContractDeployer { callStatuses2[0] = CallStatus.Executed; callStatuses2[1] = CallStatus.Cancelled; callStatuses2[2] = CallStatus.Unprocessed; + // Expect events for first unbundle + vm.expectEmit(true, false, false, false); + emit IInteropHandler.CallProcessed(bundleHash, 1, CallStatus.Cancelled); + vm.expectEmit(true, false, false, false); + emit IInteropHandler.CallProcessed(bundleHash, 2, CallStatus.Executed); + vm.expectEmit(true, false, false, false); + emit IInteropHandler.BundleUnbundled(bundleHash); vm.prank(UNBUNDLER_ADDRESS); IInteropHandler(L2_INTEROP_HANDLER_ADDR).unbundleBundle(proof.chainId, bundle, callStatuses1); + // Check storage changes after first unbundle + assertEq( + uint256(InteropHandler(L2_INTEROP_HANDLER_ADDR).callStatus(bundleHash, 0)), + 0, + "Call 0 should be Unprocessed" + ); + assertEq( + uint256(InteropHandler(L2_INTEROP_HANDLER_ADDR).callStatus(bundleHash, 1)), + 2, + "Call 1 should be Cancelled" + ); + assertEq( + uint256(InteropHandler(L2_INTEROP_HANDLER_ADDR).callStatus(bundleHash, 2)), + 1, + "Call 2 should be Executed" + ); + assertEq( + uint256(InteropHandler(L2_INTEROP_HANDLER_ADDR).bundleStatus(bundleHash)), + 3, + "BundleStatus should be Unbundled" + ); + // Expect events for second unbundle + vm.expectEmit(true, false, false, false); + emit IInteropHandler.CallProcessed(bundleHash, 0, CallStatus.Executed); + vm.expectEmit(true, false, false, false); + emit IInteropHandler.BundleUnbundled(bundleHash); vm.prank(UNBUNDLER_ADDRESS); IInteropHandler(L2_INTEROP_HANDLER_ADDR).unbundleBundle(proof.chainId, bundle, callStatuses2); + // Check storage changes after second unbundle + assertEq( + uint256(InteropHandler(L2_INTEROP_HANDLER_ADDR).callStatus(bundleHash, 0)), + 1, + "Call 0 should be Executed" + ); + assertEq( + uint256(InteropHandler(L2_INTEROP_HANDLER_ADDR).callStatus(bundleHash, 1)), + 2, + "Call 1 should be Cancelled" + ); + assertEq( + uint256(InteropHandler(L2_INTEROP_HANDLER_ADDR).callStatus(bundleHash, 2)), + 1, + "Call 2 should be Executed" + ); + assertEq( + uint256(InteropHandler(L2_INTEROP_HANDLER_ADDR).bundleStatus(bundleHash)), + 3, + "BundleStatus should be Unbundled" + ); } function getInteropBundle(uint256 amount) public returns (InteropBundle memory) { @@ -268,14 +346,151 @@ abstract contract L2InteropTestAbstract is Test, SharedL2ContractDeployer { }); InteropBundle memory interopBundle = InteropBundle({ version: INTEROP_BUNDLE_VERSION, - destinationChainId: 271, + destinationChainId: 31337, interopBundleSalt: keccak256(abi.encodePacked(depositor, bytes32(0))), calls: calls, bundleAttributes: BundleAttributes({ - executionAddress: EXECUTION_ADDRESS, - unbundlerAddress: UNBUNDLER_ADDRESS + executionAddress: InteroperableAddress.formatEvmV1(EXECUTION_ADDRESS), + unbundlerAddress: InteroperableAddress.formatEvmV1(UNBUNDLER_ADDRESS) }) }); return interopBundle; } + + /// @notice Regression test to ensure bundles can only be verified from InteropCenter + /// @dev This test verifies that the fix for unauthorized bundle verification is working + function test_verifyBundle_revertWhen_messageNotFromInteropCenter() public { + address nonInteropCenter = makeAddr("nonInteropCenter"); + + InteropBundle memory interopBundle = getInteropBundle(1); + bytes memory bundle = abi.encode(interopBundle); + + MessageInclusionProof memory proof = getInclusionProof(nonInteropCenter); + + vm.mockCall( + address(L2_MESSAGE_VERIFICATION), + abi.encodeWithSelector(L2_MESSAGE_VERIFICATION.proveL2MessageInclusionShared.selector), + abi.encode(true) + ); + + vm.expectRevert( + abi.encodeWithSelector(UnauthorizedMessageSender.selector, L2_INTEROP_CENTER_ADDR, nonInteropCenter) + ); + + IInteropHandler(L2_INTEROP_HANDLER_ADDR).verifyBundle(bundle, proof); + } + + /// @notice Regression test to ensure bundles can only be executed on the correct destination chain + /// @dev This test verifies that the fix for destination chain ID validation is working + function test_verifyBundle_revertWhen_wrongDestinationChainId() public { + InteropBundle memory interopBundle = getInteropBundle(1); + uint256 wrongChainId = 12345; + interopBundle.destinationChainId = wrongChainId; + + bytes memory bundle = abi.encode(interopBundle); + MessageInclusionProof memory proof = getInclusionProof(L2_INTEROP_CENTER_ADDR); + + vm.mockCall( + address(L2_MESSAGE_VERIFICATION), + abi.encodeWithSelector(L2_MESSAGE_VERIFICATION.proveL2MessageInclusionShared.selector), + abi.encode(true) + ); + + bytes32 bundleHash = InteropDataEncoding.encodeInteropBundleHash(proof.chainId, bundle); + + vm.expectRevert( + abi.encodeWithSelector(WrongDestinationChainId.selector, bundleHash, wrongChainId, block.chainid) + ); + + IInteropHandler(L2_INTEROP_HANDLER_ADDR).verifyBundle(bundle, proof); + } + + /// @notice Test pause functionality in InteropCenter + function test_interopCenter_pause() public { + address interopCenterOwner = InteropCenter(L2_INTEROP_CENTER_ADDR).owner(); + + vm.prank(interopCenterOwner); + InteropCenter(L2_INTEROP_CENTER_ADDR).pause(); + + assertTrue(InteropCenter(L2_INTEROP_CENTER_ADDR).paused(), "InteropCenter should be paused"); + + bytes memory recipient = abi.encodePacked(uint256(271), address(0x123)); + bytes memory payload = abi.encode("test"); + bytes[] memory attributes = new bytes[](0); + + vm.expectRevert("Pausable: paused"); + InteropCenter(L2_INTEROP_CENTER_ADDR).sendMessage(recipient, payload, attributes); + } + + /// @notice Test unpause functionality in InteropCenter + function test_interopCenter_unpause() public { + address interopCenterOwner = InteropCenter(L2_INTEROP_CENTER_ADDR).owner(); + + vm.prank(interopCenterOwner); + InteropCenter(L2_INTEROP_CENTER_ADDR).pause(); + assertTrue(InteropCenter(L2_INTEROP_CENTER_ADDR).paused(), "InteropCenter should be paused"); + + vm.prank(interopCenterOwner); + InteropCenter(L2_INTEROP_CENTER_ADDR).unpause(); + + assertFalse(InteropCenter(L2_INTEROP_CENTER_ADDR).paused(), "InteropCenter should be unpaused"); + } + + /// @notice Test setAddresses functionality in InteropCenter + function test_interopCenter_setAddresses() public { + address interopCenterOwner = InteropCenter(L2_INTEROP_CENTER_ADDR).owner(); + + address newAssetRouter = makeAddr("newAssetRouter"); + address newAssetTracker = makeAddr("newAssetTracker"); + + address oldAssetRouter = InteropCenter(L2_INTEROP_CENTER_ADDR).assetRouter(); + address oldAssetTracker = address(InteropCenter(L2_INTEROP_CENTER_ADDR).assetTracker()); + + vm.expectEmit(true, true, false, false); + emit IInteropCenter.NewAssetRouter(oldAssetRouter, newAssetRouter); + vm.expectEmit(true, true, false, false); + emit IInteropCenter.NewAssetTracker(oldAssetTracker, newAssetTracker); + + vm.prank(interopCenterOwner); + InteropCenter(L2_INTEROP_CENTER_ADDR).setAddresses(newAssetRouter, newAssetTracker); + + assertEq(InteropCenter(L2_INTEROP_CENTER_ADDR).assetRouter(), newAssetRouter, "Asset router not updated"); + assertEq( + address(InteropCenter(L2_INTEROP_CENTER_ADDR).assetTracker()), + newAssetTracker, + "Asset tracker not updated" + ); + } + + /// @notice Test that only owner can pause InteropCenter + function test_interopCenter_pause_onlyOwner() public { + address nonOwner = makeAddr("nonOwner"); + + vm.prank(nonOwner); + vm.expectRevert("Ownable: caller is not the owner"); + InteropCenter(L2_INTEROP_CENTER_ADDR).pause(); + } + + /// @notice Test that only owner can unpause InteropCenter + function test_interopCenter_unpause_onlyOwner() public { + address interopCenterOwner = InteropCenter(L2_INTEROP_CENTER_ADDR).owner(); + vm.prank(interopCenterOwner); + InteropCenter(L2_INTEROP_CENTER_ADDR).pause(); + + address nonOwner = makeAddr("nonOwner"); + vm.prank(nonOwner); + vm.expectRevert("Ownable: caller is not the owner"); + InteropCenter(L2_INTEROP_CENTER_ADDR).unpause(); + } + + /// @notice Test that only owner can call setAddresses + function test_interopCenter_setAddresses_onlyOwner() public { + address nonOwner = makeAddr("nonOwner"); + address newAssetRouter = makeAddr("newAssetRouter"); + address newAssetTracker = makeAddr("newAssetTracker"); + + vm.prank(nonOwner); + vm.expectRevert("Ownable: caller is not the owner"); + InteropCenter(L2_INTEROP_CENTER_ADDR).setAddresses(newAssetRouter, newAssetTracker); + } } diff --git a/system-contracts/contracts/libraries/Messaging.sol b/system-contracts/contracts/libraries/Messaging.sol index 6de010d959..d271c14b75 100644 --- a/system-contracts/contracts/libraries/Messaging.sol +++ b/system-contracts/contracts/libraries/Messaging.sol @@ -139,7 +139,7 @@ struct BridgehubL2TransactionRequest { } struct InteropCallStarter { - bool directCall; + bool indirectCall; address nextContract; bytes data; uint256 value; @@ -151,7 +151,7 @@ struct InteropCallStarter { } struct InteropCall { - bool directCall; + bool indirectCall; address to; address from; uint256 value;