feat: return the returned data from the target call on l2 to l2 relay message (#13389)

* feat: adds return data to L2toL2 relay message

* feat: add test for the return data

* chore: update interface and run scripts

* chore: polish test natspec

* fix: declare target instead of fuzzing on test

* fix: semgrep

* fix: unrelated flake tests adding check over target address

* refactor: fuzz the target address again on return data test

---------
Co-authored-by: Skeletor Spaceman <skeletorspaceman@gmail.com>
Co-authored-by: Skeletor Spaceman <92943766+skeletor-spaceman@users.noreply.github.com>

packages/contracts-bedrock/interfaces/L2/IL2ToL2CrossDomainMessenger.sol

@@ -104,7 +104,14 @@ interface IL2ToL2CrossDomainMessenger {
     ///         already received once and is currently being replayed.
     /// @param _id          Identifier of the SentMessage event to be relayed
     /// @param _sentMessage Message payload of the `SentMessage` event
-    function relayMessage(Identifier calldata _id, bytes calldata _sentMessage) external payable;
+    /// @return returnData_ Return data from the target contract call.
+    function relayMessage(
+        Identifier calldata _id,
+        bytes calldata _sentMessage
+    )
+        external
+        payable
+        returns (bytes memory returnData_);
 
     function messageVersion() external view returns (uint16);
 

packages/contracts-bedrock/snapshots/abi/L2ToL2CrossDomainMessenger.json

@@ -110,7 +110,13 @@
       }
     ],
     "name": "relayMessage",
-    "outputs": [],
+    "outputs": [
+      {
+        "internalType": "bytes",
+        "name": "returnData_",
+        "type": "bytes"
+      }
+    ],
     "stateMutability": "payable",
     "type": "function"
   },

packages/contracts-bedrock/snapshots/semver-lock.json

@@ -100,8 +100,8 @@
     "sourceCodeHash": "0xaef8ea36c5b78cd12e0e62811d51db627ccf0dfd2cc5479fb707a10ef0d42048"
   },
   "src/L2/L2ToL2CrossDomainMessenger.sol": {
-    "initCodeHash": "0x45564b97c63419cc12eadc60425c6d001857a3eea688ecaf1439ae7ede6aa9aa",
-    "sourceCodeHash": "0xed64736338b43a42f6bc6a88cca734403e1bb9ceafa55e4738605dfdedd1a99f"
+    "initCodeHash": "0xc56db8cb569efa0467fd53ab3fa218af3051e54f5517d7fafb7b5831b4350618",
+    "sourceCodeHash": "0x72062343a044e9c56f4143dcfc71706286eb205902006c2afcf6a4cd90c3e9f8"
   },
   "src/L2/OptimismSuperchainERC20.sol": {
     "initCodeHash": "0xdac32a1057a6bc8a8d2ffdce1db8f34950cd0ffd1454d2133865736d21869192",

packages/contracts-bedrock/src/L2/L2ToL2CrossDomainMessenger.sol

@@ -5,7 +5,6 @@ pragma solidity 0.8.25;
 import { Encoding } from "src/libraries/Encoding.sol";
 import { Hashing } from "src/libraries/Hashing.sol";
 import { Predeploys } from "src/libraries/Predeploys.sol";
-import { SafeCall } from "src/libraries/SafeCall.sol";
 import { TransientReentrancyAware } from "src/libraries/TransientContext.sol";
 
 // Interfaces
@@ -72,8 +71,8 @@ contract L2ToL2CrossDomainMessenger is ISemver, TransientReentrancyAware {
     uint16 public constant messageVersion = uint16(0);
 
     /// @notice Semantic version.
-    /// @custom:semver 1.0.0-beta.13
-    string public constant version = "1.0.0-beta.13";
+    /// @custom:semver 1.0.0-beta.14
+    string public constant version = "1.0.0-beta.14";
 
     /// @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.
@@ -159,7 +158,16 @@ contract L2ToL2CrossDomainMessenger is ISemver, TransientReentrancyAware {
     ///         currently being replayed.
     /// @param _id          Identifier of the SentMessage event to be relayed
     /// @param _sentMessage Message payload of the `SentMessage` event
-    function relayMessage(Identifier calldata _id, bytes calldata _sentMessage) external payable nonReentrant {
+    /// @return returnData_ Return data from the target contract call.
+    function relayMessage(
+        Identifier calldata _id,
+        bytes calldata _sentMessage
+    )
+        external
+        payable
+        nonReentrant
+        returns (bytes memory returnData_)
+    {
         // Ensure the log came from the messenger. Since the log origin is the CDM, there isn't a scenario where
         // this can be invoked from the CrossL2Inbox as the SentMessage log is not calldata for this function
         if (_id.origin != Predeploys.L2_TO_L2_CROSS_DOMAIN_MESSENGER) {
@@ -194,7 +202,8 @@ contract L2ToL2CrossDomainMessenger is ISemver, TransientReentrancyAware {
 
         _storeMessageMetadata(source, sender);
 
-        bool success = SafeCall.call(target, msg.value, message);
+        bool success;
+        (success, returnData_) = target.call{ value: msg.value }(message);
 
         if (!success) {
             revert TargetCallFailed();

packages/contracts-bedrock/test/L2/L2ToL2CrossDomainMessenger.t.sol

@@ -67,6 +67,8 @@ contract L2ToL2CrossDomainMessengerWithModifiableTransientStorage is L2ToL2Cross
 /// @title L2ToL2CrossDomainMessengerTest
 /// @dev Contract for testing the L2ToL2CrossDomainMessenger contract.
 contract L2ToL2CrossDomainMessengerTest is Test {
+    address internal foundryVMAddress = 0x7109709ECfa91a80626fF3989D68f67F5b1DD12D;
+
     /// @dev L2ToL2CrossDomainMessenger contract instance with modifiable transient storage.
     L2ToL2CrossDomainMessengerWithModifiableTransientStorage l2ToL2CrossDomainMessenger;
 
@@ -246,8 +248,11 @@ contract L2ToL2CrossDomainMessengerTest is Test {
     )
         external
     {
-        // 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);
+        // Ensure that the target contract is not CrossL2Inbox or L2ToL2CrossDomainMessenger or the foundry VM
+        vm.assume(
+            _target != Predeploys.CROSS_L2_INBOX && _target != Predeploys.L2_TO_L2_CROSS_DOMAIN_MESSENGER
+                && _target != foundryVMAddress
+        );
 
         // Ensure that the target call is payable if value is sent
         if (_value > 0) assumePayable(_target);
@@ -396,6 +401,57 @@ contract L2ToL2CrossDomainMessengerTest is Test {
         assertEq(l2ToL2CrossDomainMessenger.crossDomainMessageSender(), address(0));
     }
 
+    /// @dev Tests the `relayMessage` function returns the expected return data from the call to the target contract.
+    function testFuzz_relayMessage_returnData_succeeds(
+        uint256 _source,
+        uint256 _nonce,
+        address _sender,
+        uint256 _value,
+        uint256 _blockNum,
+        uint256 _logIndex,
+        uint256 _time,
+        address _target,
+        bytes memory _mockedReturnData
+    )
+        public
+    {
+        // Ensure the target is not CrossL2Inbox or L2ToL2CrossDomainMessenger or the foundry VM
+        vm.assume(
+            _target != Predeploys.CROSS_L2_INBOX && _target != Predeploys.L2_TO_L2_CROSS_DOMAIN_MESSENGER
+                && _target != foundryVMAddress
+        );
+
+        // ensure the target has 0 balance to avoid an overflow
+        vm.deal(_target, 0);
+
+        // Declare a random call to be made over the target
+        bytes memory message = abi.encodePacked("randomCall()");
+
+        // Construct the message
+        Identifier memory id =
+            Identifier(Predeploys.L2_TO_L2_CROSS_DOMAIN_MESSENGER, _blockNum, _logIndex, _time, _source);
+        bytes memory sentMessage = abi.encodePacked(
+            abi.encode(L2ToL2CrossDomainMessenger.SentMessage.selector, block.chainid, _target, _nonce), // topics
+            abi.encode(_sender, message) // data
+        );
+
+        // Ensure the CrossL2Inbox validates this message
+        vm.mockCall({
+            callee: Predeploys.CROSS_L2_INBOX,
+            data: abi.encodeCall(ICrossL2Inbox.validateMessage, (id, keccak256(message))),
+            returnData: ""
+        });
+
+        // Mock the random call over the target with the expected return data
+        vm.mockCall({ callee: _target, data: message, returnData: _mockedReturnData });
+
+        hoax(Predeploys.L2_TO_L2_CROSS_DOMAIN_MESSENGER, _value);
+        bytes memory returnData = l2ToL2CrossDomainMessenger.relayMessage{ value: _value }(id, sentMessage);
+
+        // Check that the return data is the mocked one
+        assertEq(returnData, _mockedReturnData);
+    }
+
     /// @dev Mock reentrant function that calls the `relayMessage` function.
     /// @param _source Source chain ID of the message.
     /// @param _nonce Nonce of the message.
@@ -648,8 +704,11 @@ contract L2ToL2CrossDomainMessengerTest is Test {
         // Ensure that the target call is payable if value is sent
         if (_value > 0) assumePayable(_target);
 
-        // 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);
+        // Ensure that the target contract is not CrossL2Inbox or L2ToL2CrossDomainMessenger or the foundry VM
+        vm.assume(
+            _target != Predeploys.CROSS_L2_INBOX && _target != Predeploys.L2_TO_L2_CROSS_DOMAIN_MESSENGER
+                && _target != foundryVMAddress
+        );
 
         // Ensure that the target contract does not revert
         vm.mockCall({ callee: _target, msgValue: _value, data: _message, returnData: abi.encode(true) });