feat(L2toL2CDM): improve gas estimation (ethereum-optimism#12526)

samlaf · Nov 10, 2024 · 9276074 · 9276074
1 parent c78f995
commit 9276074
Show file tree

Hide file tree

Showing 4 changed files with 23 additions and 53 deletions.
diff --git a/packages/contracts-bedrock/semver-lock.json b/packages/contracts-bedrock/semver-lock.json
@@ -104,8 +104,8 @@
     "sourceCodeHash": "0xd08a2e6514dbd44e16aa312a1b27b2841a9eab5622cbd05a39c30f543fad673c"
   },
   "src/L2/L2ToL2CrossDomainMessenger.sol": {
-    "initCodeHash": "0xf760d814018281b36d9a6a0ab16a23348effb33cf0ab299e3022b59283e46160",
-    "sourceCodeHash": "0xe8d99e4702d90814089c4a80e259c891a95f6d4750c7220fc6b2672c26ef2700"
+    "initCodeHash": "0x6eb3718548d97b69c201a75c27a3a5a77400ebd5e0a9bdcc06e2cc28f9b4a689",
+    "sourceCodeHash": "0x4bb08a8a9d5de37d1fb2dd8a9bcc483305510c32508f0be2d54757e6e257aedb"
   },
   "src/L2/OptimismSuperchainERC20.sol": {
     "initCodeHash": "0xd5c84e45746fd741d541a917ddc1cc0c7043c6b21d5c18040d4bc999d6a7b2db",

diff --git a/packages/contracts-bedrock/snapshots/abi/L2ToL2CrossDomainMessenger.json b/packages/contracts-bedrock/snapshots/abi/L2ToL2CrossDomainMessenger.json
@@ -175,31 +175,6 @@
     "stateMutability": "view",
     "type": "function"
   },
-  {
-    "anonymous": false,
-    "inputs": [
-      {
-        "indexed": true,
-        "internalType": "uint256",
-        "name": "source",
-        "type": "uint256"
-      },
-      {
-        "indexed": true,
-        "internalType": "uint256",
-        "name": "messageNonce",
-        "type": "uint256"
-      },
-      {
-        "indexed": true,
-        "internalType": "bytes32",
-        "name": "messageHash",
-        "type": "bytes32"
-      }
-    ],
-    "name": "FailedRelayedMessage",
-    "type": "event"
-  },
   {
     "anonymous": false,
     "inputs": [
@@ -306,5 +281,10 @@
     "inputs": [],
     "name": "ReentrantCall",
     "type": "error"
+  },
+  {
+    "inputs": [],
+    "name": "TargetCallFailed",
+    "type": "error"
   }
 ]
diff --git a/packages/contracts-bedrock/src/L2/L2ToL2CrossDomainMessenger.sol b/packages/contracts-bedrock/src/L2/L2ToL2CrossDomainMessenger.sol
@@ -38,6 +38,9 @@ error MessageAlreadyRelayed();
 /// @notice Thrown when a reentrant call is detected.
 error ReentrantCall();
 
+/// @notice Thrown when a call to the target contract during message relay fails.
+error TargetCallFailed();
+
 /// @custom:proxied true
 /// @custom:predeploy 0x4200000000000000000000000000000000000023
 /// @title L2ToL2CrossDomainMessenger
@@ -64,8 +67,8 @@ contract L2ToL2CrossDomainMessenger is IL2ToL2CrossDomainMessenger, ISemver, Tra
     uint16 public constant messageVersion = uint16(0);
 
     /// @notice Semantic version.
-    /// @custom:semver 1.0.0-beta.8
-    string public constant version = "1.0.0-beta.8";
+    /// @custom:semver 1.0.0-beta.9
+    string public constant version = "1.0.0-beta.9";
 
     /// @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.
@@ -92,12 +95,6 @@ contract L2ToL2CrossDomainMessenger is IL2ToL2CrossDomainMessenger, ISemver, Tra
     /// @param messageHash  Hash of the message that was relayed.
     event RelayedMessage(uint256 indexed source, uint256 indexed messageNonce, bytes32 indexed messageHash);
 
-    /// @notice Emitted whenever a message fails to be relayed on this chain.
-    /// @param source       Chain ID of the source chain.
-    /// @param messageNonce Nonce associated with the messsage sent
-    /// @param messageHash  Hash of the message that failed to be relayed.
-    event FailedRelayedMessage(uint256 indexed source, uint256 indexed messageNonce, bytes32 indexed messageHash);
-
     /// @notice Retrieves the sender of the current cross domain message. If not entered, reverts.
     /// @return sender_ Address of the sender of the current cross domain message.
     function crossDomainMessageSender() external view onlyEntered returns (address sender_) {
@@ -200,13 +197,13 @@ contract L2ToL2CrossDomainMessenger is IL2ToL2CrossDomainMessenger, ISemver, Tra
 
         bool success = SafeCall.call(target, msg.value, message);
 
-        if (success) {
-            successfulMessages[messageHash] = true;
-            emit RelayedMessage(source, nonce, messageHash);
-        } else {
-            emit FailedRelayedMessage(source, nonce, messageHash);
+        if (!success) {
+            revert TargetCallFailed();
         }
 
+        successfulMessages[messageHash] = true;
+        emit RelayedMessage(source, nonce, messageHash);
+
         _storeMessageMetadata(0, address(0));
     }
 

diff --git a/packages/contracts-bedrock/test/L2/L2ToL2CrossDomainMessenger.t.sol b/packages/contracts-bedrock/test/L2/L2ToL2CrossDomainMessenger.t.sol
@@ -22,7 +22,8 @@ import {
     MessageTargetCrossL2Inbox,
     MessageTargetL2ToL2CrossDomainMessenger,
     MessageAlreadyRelayed,
-    ReentrantCall
+    ReentrantCall,
+    TargetCallFailed
 } from "src/L2/L2ToL2CrossDomainMessenger.sol";
 
 /// @title L2ToL2CrossDomainMessengerWithModifiableTransientStorage
@@ -402,12 +403,6 @@ contract L2ToL2CrossDomainMessengerTest is Test {
         address target = address(this);
         bytes memory message = abi.encodeWithSelector(this.mockTargetReentrant.selector, _source2, _nonce, _sender2);
 
-        // Look for correct emitted event
-        vm.expectEmit(Predeploys.L2_TO_L2_CROSS_DOMAIN_MESSENGER);
-        emit L2ToL2CrossDomainMessenger.FailedRelayedMessage(
-            _source1, _nonce, keccak256(abi.encode(block.chainid, _source1, _nonce, _sender1, target, message))
-        );
-
         // Ensure the target contract is called with the correct parameters
         vm.expectCall({ callee: target, msgValue: _value, data: message });
 
@@ -426,6 +421,8 @@ contract L2ToL2CrossDomainMessengerTest is Test {
             returnData: ""
         });
 
+        // Expect a revert with the TargetCallFailed selector
+        vm.expectRevert(TargetCallFailed.selector);
         hoax(Predeploys.L2_TO_L2_CROSS_DOMAIN_MESSENGER, _value);
         l2ToL2CrossDomainMessenger.relayMessage{ value: _value }(id, sentMessage);
 
@@ -673,12 +670,6 @@ contract L2ToL2CrossDomainMessengerTest is Test {
         // Ensure that the target contract reverts
         vm.mockCallRevert({ callee: _target, msgValue: _value, data: _message, revertData: abi.encode(false) });
 
-        // Look for correct emitted event
-        vm.expectEmit(Predeploys.L2_TO_L2_CROSS_DOMAIN_MESSENGER);
-        emit L2ToL2CrossDomainMessenger.FailedRelayedMessage(
-            _source, _nonce, keccak256(abi.encode(block.chainid, _source, _nonce, _sender, _target, _message))
-        );
-
         ICrossL2Inbox.Identifier memory id =
             ICrossL2Inbox.Identifier(Predeploys.L2_TO_L2_CROSS_DOMAIN_MESSENGER, _blockNum, _logIndex, _time, _source);
         bytes memory sentMessage = abi.encodePacked(
@@ -693,6 +684,8 @@ contract L2ToL2CrossDomainMessengerTest is Test {
             returnData: ""
         });
 
+        // Expect a revert with the TargetCallFailed selector
+        vm.expectRevert(TargetCallFailed.selector);
         hoax(Predeploys.L2_TO_L2_CROSS_DOMAIN_MESSENGER, _value);
         l2ToL2CrossDomainMessenger.relayMessage{ value: _value }(id, sentMessage);
     }