From 9276074f8f96b36f5676848ea2051265f39c186a Mon Sep 17 00:00:00 2001 From: tre Date: Sun, 20 Oct 2024 21:30:51 -0700 Subject: [PATCH] feat(L2toL2CDM): improve gas estimation (#12526) --- packages/contracts-bedrock/semver-lock.json | 4 +-- .../abi/L2ToL2CrossDomainMessenger.json | 30 ++++--------------- .../src/L2/L2ToL2CrossDomainMessenger.sol | 23 +++++++------- .../test/L2/L2ToL2CrossDomainMessenger.t.sol | 19 ++++-------- 4 files changed, 23 insertions(+), 53 deletions(-) diff --git a/packages/contracts-bedrock/semver-lock.json b/packages/contracts-bedrock/semver-lock.json index 11f387a9ba9dc..695a8611c80be 100644 --- 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 index f6d2692bae538..daefa3a74386e 100644 --- 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" } ] \ No newline at end of file diff --git a/packages/contracts-bedrock/src/L2/L2ToL2CrossDomainMessenger.sol b/packages/contracts-bedrock/src/L2/L2ToL2CrossDomainMessenger.sol index 0d7b46080fc93..c8afe8be5d924 100644 --- 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 index e0149e8aab781..36b36973af8da 100644 --- 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); }