contracts-bedrock: improve CrossL2Inbox devex (#11322)

* contracts-bedrock: improve `CrossL2Inbox` devex Improve the `CrossL2Inbox` devex by creating an alternative entrypoint. This design was not considered previously because there was a "top level call" restriction, aka the "only EOA" invariant. This was to allow for static analysis of transactions, keeping resource usage lower for validating transactions when building blocks or at the mempool layer. Since 3074/7702 render the enforcement of only eoa impossible, we decided to op/acc and lean into the approach of allowing subcalls to trigger `ExecutingMessage` events. This new interface allows another contract to be the entrypoint, the idea is that the user sends the `Identifier` and the serialized log (message) to whatever contract that they want and then pass it to `CrossL2Inbox.validateMessage` which then emits the event that consensus validates. This allows the calling smart contract to be aware of the schema for the log and deserialize it however they see fit. Since the serialized logs are done with the following algorithm: ```go msg := make([]byte, 0) for _, topic := range log.Topics { msg = append(msg, topic.Bytes()...) } msg = append(msg, log.Data...) ``` It is very easy to use `abi.decode` to decode a log, given that solidity was used to `emit` it. The topics are `bytes32` and then the data is abi encoded given the schema of the event itself. Unused parts like `topic[0]` (hash of the event name) can be dropped when decoding if they are not required. * ctb: fix typo * remove nonReentrant and add tests for validateMessage, rename ENTERED_SLOT preimage * add natspec for _checkIdentifier and update that of validateMessage * update version and semver-lock file * check all topics in crossl2inbox test, run pnpm snapshots * tests: fix --------- Co-authored-by: Michael Amadi <amadimichaeld@gmail.com>
ethereum-optimism · Aug 7, 2024 · 051db54 · 051db54
1 parent cc67d34
commit 051db54
Show file tree

Hide file tree

Showing 7 changed files with 202 additions and 52 deletions.
diff --git a/packages/contracts-bedrock/semver-lock.json b/packages/contracts-bedrock/semver-lock.json
@@ -68,8 +68,8 @@
     "sourceCodeHash": "0x3a725791a0f5ed84dc46dcdae26f6170a759b2fe3dc360d704356d088b76cfd6"
   },
   "src/L2/CrossL2Inbox.sol": {
-    "initCodeHash": "0x318b1e98f1686920e3d309390983454685aa84ed997598ead1b4c1a1938206c4",
-    "sourceCodeHash": "0xb0d2d5944f11bdf44cb6a16a9b00ab76a9b9f5ab2abb081781fb1c27927eb5ab"
+    "initCodeHash": "0x80124454d2127d5ff340b0ef048be6d5bf5984e84c75021b6a1ffa81703a2503",
+    "sourceCodeHash": "0xfb26fc80fbc7febdc91ac73ea91ceb479b238e0e81804a0a21192d78c261a755"
   },
   "src/L2/ETHLiquidity.sol": {
     "initCodeHash": "0x98177562fca0de0dfea5313c9acefe2fdbd73dee5ce6c1232055601f208f0177",
@@ -108,8 +108,8 @@
     "sourceCodeHash": "0x8388b9b8075f31d580fed815b66b45394e40fb1a63cd8cda2272d2c390fc908c"
   },
   "src/L2/L2ToL2CrossDomainMessenger.sol": {
-    "initCodeHash": "0xda499b71aec14976b8e133fad9ece083805f5ff520f0e88f89232fd837451954",
-    "sourceCodeHash": "0x7a9cddf5b54ac72457231f0c09b8e88398202ac29125cd63318b8389c81e119b"
+    "initCodeHash": "0xe390be1390edc38fd879d7620538560076d7fcf3ef9debce327a1877d96d3ff0",
+    "sourceCodeHash": "0x20f77dc5a02869c6885b73347fa9e7d2bbc4eaf8a2313f7e7435e456001f7a75"
   },
   "src/L2/SequencerFeeVault.sol": {
     "initCodeHash": "0xb94145f571e92ee615c6fe903b6568e8aac5fe760b6b65148ffc45d2fb0f5433",

diff --git a/packages/contracts-bedrock/snapshots/abi/CrossL2Inbox.json b/packages/contracts-bedrock/snapshots/abi/CrossL2Inbox.json
@@ -114,6 +114,51 @@
     "stateMutability": "view",
     "type": "function"
   },
+  {
+    "inputs": [
+      {
+        "components": [
+          {
+            "internalType": "address",
+            "name": "origin",
+            "type": "address"
+          },
+          {
+            "internalType": "uint256",
+            "name": "blockNumber",
+            "type": "uint256"
+          },
+          {
+            "internalType": "uint256",
+            "name": "logIndex",
+            "type": "uint256"
+          },
+          {
+            "internalType": "uint256",
+            "name": "timestamp",
+            "type": "uint256"
+          },
+          {
+            "internalType": "uint256",
+            "name": "chainId",
+            "type": "uint256"
+          }
+        ],
+        "internalType": "struct ICrossL2Inbox.Identifier",
+        "name": "_id",
+        "type": "tuple"
+      },
+      {
+        "internalType": "bytes32",
+        "name": "_msgHash",
+        "type": "bytes32"
+      }
+    ],
+    "name": "validateMessage",
+    "outputs": [],
+    "stateMutability": "nonpayable",
+    "type": "function"
+  },
   {
     "inputs": [],
     "name": "version",
@@ -188,6 +233,11 @@
     "name": "NotEntered",
     "type": "error"
   },
+  {
+    "inputs": [],
+    "name": "ReentrantCall",
+    "type": "error"
+  },
   {
     "inputs": [],
     "name": "TargetCallFailed",

diff --git a/packages/contracts-bedrock/snapshots/abi/TransientReentrancyAware.json b/packages/contracts-bedrock/snapshots/abi/TransientReentrancyAware.json
@@ -1 +1,12 @@
-[]
+[
+  {
+    "inputs": [],
+    "name": "NotEntered",
+    "type": "error"
+  },
+  {
+    "inputs": [],
+    "name": "ReentrantCall",
+    "type": "error"
+  }
+]
diff --git a/packages/contracts-bedrock/src/L2/CrossL2Inbox.sol b/packages/contracts-bedrock/src/L2/CrossL2Inbox.sol
@@ -56,21 +56,14 @@ contract CrossL2Inbox is ICrossL2Inbox, ISemver, TransientReentrancyAware {
     bytes32 internal constant CHAINID_SLOT = 0x6e0446e8b5098b8c8193f964f1b567ec3a2bdaeba33d36acb85c1f1d3f92d313;
 
     /// @notice Semantic version.
-    /// @custom:semver 1.0.0-beta.3
-    string public constant version = "1.0.0-beta.3";
+    /// @custom:semver 1.0.0-beta.4
+    string public constant version = "1.0.0-beta.4";
 
     /// @notice Emitted when a cross chain message is being executed.
     /// @param msgHash Hash of message payload being executed.
     /// @param id Encoded Identifier of the message.
     event ExecutingMessage(bytes32 indexed msgHash, Identifier id);
 
-    /// @notice Enforces that cross domain message sender and source are set. Reverts if not.
-    ///         Used to differentiate between 0 and nil in transient storage.
-    modifier notEntered() {
-        if (TransientContext.callDepth() == 0) revert NotEntered();
-        _;
-    }
-
     /// @notice Returns the origin address of the Identifier. If not entered, reverts.
     /// @return Origin address of the Identifier.
     function origin() external view notEntered returns (address) {
@@ -114,10 +107,8 @@ contract CrossL2Inbox is ICrossL2Inbox, ISemver, TransientReentrancyAware {
         payable
         reentrantAware
     {
-        if (_id.timestamp > block.timestamp) revert InvalidTimestamp();
-        if (!IDependencySet(Predeploys.L1_BLOCK_ATTRIBUTES).isInDependencySet(_id.chainId)) {
-            revert InvalidChainId();
-        }
+        // Check the Identifier.
+        _checkIdentifier(_id);
 
         // Store the Identifier in transient storage.
         _storeIdentifier(_id);
@@ -131,6 +122,30 @@ contract CrossL2Inbox is ICrossL2Inbox, ISemver, TransientReentrancyAware {
         emit ExecutingMessage(keccak256(_message), _id);
     }
 
+    /// @notice Validates a cross chain message on the destination chain
+    ///         and emits an ExecutingMessage event. This function is useful
+    ///         for applications that understand the schema of the _message payload and want to
+    ///         process it in a custom way.
+    /// @param _id      Identifier of the message.
+    /// @param _msgHash Hash of the message payload to call target with.
+    function validateMessage(Identifier calldata _id, bytes32 _msgHash) external {
+        // Check the Identifier.
+        _checkIdentifier(_id);
+
+        emit ExecutingMessage(_msgHash, _id);
+    }
+
+    /// @notice Validates that for a given cross chain message identifier,
+    ///         it's timestamp is not in the future and the source chainId
+    ///         is in the destination chain's dependency set.
+    /// @param _id Identifier of the message.
+    function _checkIdentifier(Identifier calldata _id) internal view {
+        if (_id.timestamp > block.timestamp) revert InvalidTimestamp();
+        if (!IDependencySet(Predeploys.L1_BLOCK_ATTRIBUTES).isInDependencySet(_id.chainId)) {
+            revert InvalidChainId();
+        }
+    }
+
     /// @notice Stores the Identifier in transient storage.
     /// @param _id Identifier to store.
     function _storeIdentifier(Identifier calldata _id) internal {

diff --git a/packages/contracts-bedrock/src/L2/L2ToL2CrossDomainMessenger.sol b/packages/contracts-bedrock/src/L2/L2ToL2CrossDomainMessenger.sol
@@ -7,6 +7,7 @@ import { CrossL2Inbox } from "src/L2/CrossL2Inbox.sol";
 import { IL2ToL2CrossDomainMessenger } from "src/L2/IL2ToL2CrossDomainMessenger.sol";
 import { ISemver } from "src/universal/ISemver.sol";
 import { SafeCall } from "src/libraries/SafeCall.sol";
+import { TransientReentrancyAware } from "src/libraries/TransientContext.sol";
 
 /// @notice Thrown when a non-written slot in transient storage is attempted to be read from.
 error NotEntered();
@@ -41,11 +42,7 @@ error ReentrantCall();
 /// @notice The L2ToL2CrossDomainMessenger is a higher level abstraction on top of the CrossL2Inbox that provides
 ///         features necessary for secure transfers ERC20 tokens between L2 chains. Messages sent through the
 ///         L2ToL2CrossDomainMessenger on the source chain receive both replay protection as well as domain binding.
-contract L2ToL2CrossDomainMessenger is IL2ToL2CrossDomainMessenger, ISemver {
-    /// @notice Storage slot for `entered` value.
-    ///         Equal to bytes32(uint256(keccak256("l2tol2crossdomainmessenger.entered")) - 1)
-    bytes32 internal constant ENTERED_SLOT = 0xf53fc38c5e461bdcbbeb47887fecf014abd399293109cd50f65e5f9078cfd025;
-
+contract L2ToL2CrossDomainMessenger is IL2ToL2CrossDomainMessenger, ISemver, TransientReentrancyAware {
     /// @notice Storage slot for the sender of the current cross domain message.
     ///         Equal to bytes32(uint256(keccak256("l2tol2crossdomainmessenger.sender")) - 1)
     bytes32 internal constant CROSS_DOMAIN_MESSAGE_SENDER_SLOT =
@@ -80,25 +77,6 @@ contract L2ToL2CrossDomainMessenger is IL2ToL2CrossDomainMessenger, ISemver {
     /// @param messageHash Hash of the message that failed to be relayed.
     event FailedRelayedMessage(bytes32 indexed messageHash);
 
-    /// @notice Enforces that a function cannot be re-entered.
-    modifier nonReentrant() {
-        if (_entered()) revert ReentrantCall();
-        assembly {
-            tstore(ENTERED_SLOT, 1)
-        }
-        _;
-        assembly {
-            tstore(ENTERED_SLOT, 0)
-        }
-    }
-
-    /// @notice Enforces that cross domain message sender and source are set. Reverts if not.
-    ///         Used to differentiate between 0 and nil in transient storage.
-    modifier onlyEntered() {
-        if (!_entered()) revert NotEntered();
-        _;
-    }
-
     /// @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) {
@@ -193,16 +171,6 @@ contract L2ToL2CrossDomainMessenger is IL2ToL2CrossDomainMessenger, ISemver {
         return Encoding.encodeVersionedNonce(msgNonce, messageVersion);
     }
 
-    /// @notice Retrieves whether the contract is currently entered or not.
-    /// @return True if the contract is entered, and false otherwise.
-    function _entered() internal view returns (bool) {
-        uint256 value;
-        assembly {
-            value := tload(ENTERED_SLOT)
-        }
-        return value != 0;
-    }
-
     /// @notice Stores message data such as sender and source in transient storage.
     /// @param _source Chain ID of the source chain.
     /// @param _sender Address of the sender of the message.

diff --git a/packages/contracts-bedrock/src/libraries/TransientContext.sol b/packages/contracts-bedrock/src/libraries/TransientContext.sol
@@ -61,10 +61,55 @@ library TransientContext {
 /// @notice Reentrancy-aware modifier for transient storage, which increments and
 ///         decrements the call depth when entering and exiting a function.
 contract TransientReentrancyAware {
+    /// @notice Thrown when a non-written transient storage slot is attempted to be read from.
+    error NotEntered();
+
+    /// @notice Thrown when a reentrant call is detected.
+    error ReentrantCall();
+
+    /// @notice Storage slot for `entered` value.
+    ///         Equal to bytes32(uint256(keccak256("transientreentrancyaware.entered")) - 1)
+    bytes32 internal constant ENTERED_SLOT = 0xf13569814868ede994184d5a425471fb19e869768a33421cb701a2ba3d420c0a;
+
     /// @notice Modifier to make a function reentrancy-aware.
     modifier reentrantAware() {
         TransientContext.increment();
         _;
         TransientContext.decrement();
     }
+
+    /// @notice Enforces that a function cannot be re-entered.
+    modifier nonReentrant() {
+        if (_entered()) revert ReentrantCall();
+        assembly {
+            tstore(ENTERED_SLOT, 1)
+        }
+        _;
+        assembly {
+            tstore(ENTERED_SLOT, 0)
+        }
+    }
+
+    /// @notice Enforces that cross domain message sender and source are set. Reverts if not.
+    ///         Used to differentiate between 0 and nil in transient storage.
+    modifier notEntered() {
+        if (TransientContext.callDepth() == 0) revert NotEntered();
+        _;
+    }
+
+    /// @notice Enforces that cross domain message sender and source are set. Reverts if not.
+    ///         Used to differentiate between 0 and nil in transient storage.
+    modifier onlyEntered() {
+        if (!_entered()) revert NotEntered();
+        _;
+    }
+
+    /// @notice Retrieves whether the contract is currently entered or not.
+    /// @return entered_ True if the contract is entered, and false otherwise.
+    function _entered() internal view returns (bool entered_) {
+        assembly {
+            let value := tload(ENTERED_SLOT)
+            entered_ := gt(value, 0)
+        }
+    }
 }
diff --git a/packages/contracts-bedrock/test/L2/CrossL2Inbox.t.sol b/packages/contracts-bedrock/test/L2/CrossL2Inbox.t.sol
@@ -271,6 +271,67 @@ contract CrossL2InboxTest is Test {
         crossL2Inbox.executeMessage{ value: _value }({ _id: _id, _target: _target, _message: _message });
     }
 
+    function testFuzz_validateMessage_succeeds(ICrossL2Inbox.Identifier memory _id, bytes32 _messageHash) external {
+        // Ensure that the id's timestamp is valid (less than or equal to the current block timestamp)
+        _id.timestamp = bound(_id.timestamp, 1, block.timestamp);
+
+        // Ensure that the chain ID is in the dependency set
+        vm.mockCall({
+            callee: Predeploys.L1_BLOCK_ATTRIBUTES,
+            data: abi.encodeWithSelector(L1BlockIsInDependencySetSelector, _id.chainId),
+            returnData: abi.encode(true)
+        });
+
+        // Look for the emit ExecutingMessage event
+        vm.expectEmit(Predeploys.CROSS_L2_INBOX);
+        emit CrossL2Inbox.ExecutingMessage(_messageHash, _id);
+
+        // Call the validateMessage function
+        crossL2Inbox.validateMessage(_id, _messageHash);
+    }
+
+    /// @dev Tests that the `validateMessage` function reverts when called with an identifier with an invalid timestamp.
+    function testFuzz_validateMessage_invalidTimestamp_reverts(
+        ICrossL2Inbox.Identifier calldata _id,
+        bytes32 _messageHash
+    )
+        external
+    {
+        // Ensure that the id's timestamp is invalid (greater thsan the current block timestamp)
+        vm.assume(_id.timestamp > block.timestamp);
+
+        // Expect a revert with the InvalidTimestamp selector
+        vm.expectRevert(InvalidTimestamp.selector);
+
+        // Call the validateMessage function
+        crossL2Inbox.validateMessage(_id, _messageHash);
+    }
+
+    /// @dev Tests that the `validateMessage` function reverts when called with an identifier with a chain ID not in the
+    /// dependency set.
+    function testFuzz_validateMessage_invalidChainId_reverts(
+        ICrossL2Inbox.Identifier memory _id,
+        bytes32 _messageHash
+    )
+        external
+    {
+        // Ensure that the timestamp is valid (less than or equal to the current block timestamp)
+        _id.timestamp = bound(_id.timestamp, 0, block.timestamp);
+
+        // Ensure that the chain ID is NOT in the dependency set.
+        vm.mockCall({
+            callee: Predeploys.L1_BLOCK_ATTRIBUTES,
+            data: abi.encodeWithSelector(L1BlockIsInDependencySetSelector, _id.chainId),
+            returnData: abi.encode(false)
+        });
+
+        // Expect a revert with the InvalidChainId selector
+        vm.expectRevert(InvalidChainId.selector);
+
+        // Call the validateMessage function
+        crossL2Inbox.validateMessage(_id, _messageHash);
+    }
+
     /// @dev Tests that the `origin` function returns the correct value.
     function testFuzz_origin_succeeds(address _origin) external {
         // Increment the call depth to prevent NotEntered revert