From e3ed942c4a714259f962baee75e2e36e95a07238 Mon Sep 17 00:00:00 2001 From: AgusDuha <81362284+agusduha@users.noreply.github.com> Date: Thu, 21 Nov 2024 16:22:39 -0300 Subject: [PATCH] feat: add dependency set check to L2ToL2CDM (#134) --- .../abi/L2ToL2CrossDomainMessenger.json | 5 +++ .../snapshots/semver-lock.json | 4 +- .../src/L2/L2ToL2CrossDomainMessenger.sol | 9 ++++- .../IL2ToL2CrossDomainMessenger.sol | 3 ++ .../test/L2/L2ToL2CrossDomainMessenger.t.sol | 39 ++++++++++++++++++- 5 files changed, 55 insertions(+), 5 deletions(-) diff --git a/packages/contracts-bedrock/snapshots/abi/L2ToL2CrossDomainMessenger.json b/packages/contracts-bedrock/snapshots/abi/L2ToL2CrossDomainMessenger.json index 6434233930db..9f51cc1c3a27 100644 --- a/packages/contracts-bedrock/snapshots/abi/L2ToL2CrossDomainMessenger.json +++ b/packages/contracts-bedrock/snapshots/abi/L2ToL2CrossDomainMessenger.json @@ -247,6 +247,11 @@ "name": "IdOriginNotL2ToL2CrossDomainMessenger", "type": "error" }, + { + "inputs": [], + "name": "InvalidChainId", + "type": "error" + }, { "inputs": [], "name": "MessageAlreadyRelayed", diff --git a/packages/contracts-bedrock/snapshots/semver-lock.json b/packages/contracts-bedrock/snapshots/semver-lock.json index d069a2753933..b31fd1438a4d 100644 --- a/packages/contracts-bedrock/snapshots/semver-lock.json +++ b/packages/contracts-bedrock/snapshots/semver-lock.json @@ -100,8 +100,8 @@ "sourceCodeHash": "0xd08a2e6514dbd44e16aa312a1b27b2841a9eab5622cbd05a39c30f543fad673c" }, "src/L2/L2ToL2CrossDomainMessenger.sol": { - "initCodeHash": "0x2a1a1ee4f47175ce661ee8e4e50cfa879b082dcb5278b1d66ddda00ed77bb744", - "sourceCodeHash": "0xa76133db7f449ae742f9ba988ad86ccb5672475f61298b9fefe411b63b63e9f6" + "initCodeHash": "0xc1c80c662aafebb639f62f17d9fefd6954947fd43dc31c278950727491471a94", + "sourceCodeHash": "0xac12ab96ffe91c75bfe74768271a725e1cbe3996b16284171440dd71bcc215b6" }, "src/L2/OptimismSuperchainERC20.sol": { "initCodeHash": "0x5bc5824030ecdb531e1f615d207cb73cdaa702e198769445d0ddbe717271eba9", diff --git a/packages/contracts-bedrock/src/L2/L2ToL2CrossDomainMessenger.sol b/packages/contracts-bedrock/src/L2/L2ToL2CrossDomainMessenger.sol index 6b1d7327dbc0..bf53133484d6 100644 --- a/packages/contracts-bedrock/src/L2/L2ToL2CrossDomainMessenger.sol +++ b/packages/contracts-bedrock/src/L2/L2ToL2CrossDomainMessenger.sol @@ -8,6 +8,7 @@ import { CrossL2Inbox, Identifier } from "src/L2/CrossL2Inbox.sol"; import { ISemver } from "src/universal/interfaces/ISemver.sol"; import { SafeCall } from "src/libraries/SafeCall.sol"; import { TransientReentrancyAware } from "src/libraries/TransientContext.sol"; +import { IDependencySet } from "src/L2/interfaces/IDependencySet.sol"; /// @notice Thrown when a non-written slot in transient storage is attempted to be read from. error NotEntered(); @@ -39,6 +40,9 @@ error ReentrantCall(); /// @notice Thrown when a call to the target contract during message relay fails. error TargetCallFailed(); +/// @notice Thrown when attempting to use a chain ID that is not in the dependency set. +error InvalidChainId(); + /// @custom:proxied true /// @custom:predeploy 0x4200000000000000000000000000000000000023 /// @title L2ToL2CrossDomainMessenger @@ -65,8 +69,8 @@ contract L2ToL2CrossDomainMessenger is ISemver, TransientReentrancyAware { uint16 public constant messageVersion = uint16(0); /// @notice Semantic version. - /// @custom:semver 1.0.0-beta.10 - string public constant version = "1.0.0-beta.10"; + /// @custom:semver 1.0.0-beta.11 + string public constant version = "1.0.0-beta.11"; /// @notice Mapping of message hashes to boolean receipt values. Note that a message will only be present in this /// mapping if it has successfully been relayed on this chain, and can therefore not be relayed again. @@ -130,6 +134,7 @@ contract L2ToL2CrossDomainMessenger is ISemver, TransientReentrancyAware { if (_destination == block.chainid) revert MessageDestinationSameChain(); if (_target == Predeploys.CROSS_L2_INBOX) revert MessageTargetCrossL2Inbox(); if (_target == Predeploys.L2_TO_L2_CROSS_DOMAIN_MESSENGER) revert MessageTargetL2ToL2CrossDomainMessenger(); + if (!IDependencySet(Predeploys.L1_BLOCK_ATTRIBUTES).isInDependencySet(_destination)) revert InvalidChainId(); uint256 nonce = messageNonce(); emit SentMessage(_destination, _target, nonce, msg.sender, _message); diff --git a/packages/contracts-bedrock/src/L2/interfaces/IL2ToL2CrossDomainMessenger.sol b/packages/contracts-bedrock/src/L2/interfaces/IL2ToL2CrossDomainMessenger.sol index 00ca4906b5c4..89311cf18f01 100644 --- a/packages/contracts-bedrock/src/L2/interfaces/IL2ToL2CrossDomainMessenger.sol +++ b/packages/contracts-bedrock/src/L2/interfaces/IL2ToL2CrossDomainMessenger.sol @@ -42,6 +42,9 @@ interface IL2ToL2CrossDomainMessenger { /// @notice Thrown when a call to the target contract during message relay fails. error TargetCallFailed(); + /// @notice Thrown when attempting to use a chain ID that is not in the dependency set. + error InvalidChainId(); + /// @notice Emitted whenever a message is sent to a destination /// @param destination Chain ID of the destination chain. /// @param target Target contract or wallet address. diff --git a/packages/contracts-bedrock/test/L2/L2ToL2CrossDomainMessenger.t.sol b/packages/contracts-bedrock/test/L2/L2ToL2CrossDomainMessenger.t.sol index 1e5e04edc25b..0775eb588d2a 100644 --- a/packages/contracts-bedrock/test/L2/L2ToL2CrossDomainMessenger.t.sol +++ b/packages/contracts-bedrock/test/L2/L2ToL2CrossDomainMessenger.t.sol @@ -22,7 +22,9 @@ import { MessageTargetL2ToL2CrossDomainMessenger, MessageAlreadyRelayed, ReentrantCall, - TargetCallFailed + TargetCallFailed, + IDependencySet, + InvalidChainId } from "src/L2/L2ToL2CrossDomainMessenger.sol"; /// @title L2ToL2CrossDomainMessengerWithModifiableTransientStorage @@ -85,6 +87,13 @@ contract L2ToL2CrossDomainMessengerTest is Test { // Ensure that the target contract is not CrossL2Inbox or L2ToL2CrossDomainMessenger vm.assume(_target != Predeploys.CROSS_L2_INBOX && _target != Predeploys.L2_TO_L2_CROSS_DOMAIN_MESSENGER); + // Mock the call over the `isInDependencySet` function to return true + vm.mockCall( + Predeploys.L1_BLOCK_ATTRIBUTES, + abi.encodeCall(IDependencySet.isInDependencySet, (_destination)), + abi.encode(true) + ); + // Get the current message nonce uint256 messageNonce = l2ToL2CrossDomainMessenger.messageNonce(); @@ -193,6 +202,34 @@ contract L2ToL2CrossDomainMessengerTest is Test { }); } + /// @notice Tests the `sendMessage` function reverts when the `destination` is not in the dependency set. + function testFuzz_sendMessage_notInDependencySet_reverts( + uint256 _destination, + address _target, + bytes calldata _message + ) + external + { + // Ensure the destination is not the same as the source, otherwise the function will revert + vm.assume(_destination != block.chainid); + + // Ensure that the target contract is not CrossL2Inbox or L2ToL2CrossDomainMessenger + vm.assume(_target != Predeploys.CROSS_L2_INBOX && _target != Predeploys.L2_TO_L2_CROSS_DOMAIN_MESSENGER); + + // Mock the call over the `isInDependencySet` function to return false + vm.mockCall( + Predeploys.L1_BLOCK_ATTRIBUTES, + abi.encodeCall(IDependencySet.isInDependencySet, (_destination)), + abi.encode(false) + ); + + // Expect a revert with the InvalidChainId selector + vm.expectRevert(InvalidChainId.selector); + + // Call `sendMessage` with a destination that is not in the dependency set to provoke revert + l2ToL2CrossDomainMessenger.sendMessage(_destination, _target, _message); + } + /// @dev Tests that the `relayMessage` function succeeds and emits the correct RelayedMessage event. function testFuzz_relayMessage_succeeds( uint256 _source,