feat: unpredictable messageHash and data location considerations

Author: jaybuidl

contracts/src/bridge/FastBridgeReceiverOnEthereum.sol

@@ -108,10 +108,17 @@ contract FastBridgeReceiverOnEthereum is SafeBridgeReceiverOnEthereum, IFastBrid
         emit ClaimChallenged(_ticketID, ticket.claim.messageHash, block.timestamp);
     }
 
-    function verifyAndRelay(uint256 _ticketID, bytes memory _messageData) external override {
+    function verifyAndRelay(
+        uint256 _ticketID,
+        uint256 blockNumber,
+        bytes calldata _messageData
+    ) external override {
         Ticket storage ticket = tickets[_ticketID];
         require(ticket.claim.bridger != address(0), "Claim does not exist");
-        require(ticket.claim.messageHash == keccak256(abi.encode(_ticketID, _messageData)), "Invalid hash");
+        require(
+            ticket.claim.messageHash == keccak256(abi.encode(_ticketID, blockNumber, _messageData)),
+            "Invalid hash"
+        );
         require(ticket.claim.claimedAt + challengeDuration < block.timestamp, "Challenge period not over");
         require(ticket.challenge.challenger == address(0), "Claim is challenged");
         require(ticket.relayed == false, "Message already relayed");
@@ -121,14 +128,18 @@ contract FastBridgeReceiverOnEthereum is SafeBridgeReceiverOnEthereum, IFastBrid
         require(_relay(_messageData), "Failed to call contract"); // Checks-Effects-Interaction
     }
 
-    function verifyAndRelaySafe(uint256 _ticketID, bytes memory _messageData) external override {
+    function verifyAndRelaySafe(
+        uint256 _ticketID,
+        uint256 blockNumber,
+        bytes calldata _messageData
+    ) external override {
         require(isSentBySafeBridge(), "Access not allowed: SafeBridgeSender only.");
 
         Ticket storage ticket = tickets[_ticketID];
         require(ticket.relayed == false, "Message already relayed");
 
         // Claim assessment if any
-        bytes32 messageHash = keccak256(abi.encode(_ticketID, _messageData));
+        bytes32 messageHash = keccak256(abi.encode(_ticketID, blockNumber, _messageData));
         if (ticket.claim.bridger != address(0) && ticket.claim.messageHash == messageHash) {
             ticket.claim.verified = true;
         }
@@ -196,7 +207,7 @@ contract FastBridgeReceiverOnEthereum is SafeBridgeReceiverOnEthereum, IFastBrid
     // *       Internal       * //
     // ************************ //
 
-    function _relay(bytes memory _messageData) internal returns (bool success) {
+    function _relay(bytes calldata _messageData) internal returns (bool success) {
         // Decode the receiver address from the data encoded by the IFastBridgeSender
         (address receiver, bytes memory data) = abi.decode(_messageData, (address, bytes));
         (success, ) = address(receiver).call(data);

contracts/src/bridge/FastBridgeSenderToEthereum.sol

@@ -25,6 +25,7 @@ contract FastBridgeSenderToEthereum is SafeBridgeSenderToEthereum, IFastBridgeSe
 
     struct Ticket {
         bytes32 messageHash;
+        uint256 blockNumber;
         bool sentSafe;
     }
 
@@ -46,7 +47,13 @@ contract FastBridgeSenderToEthereum is SafeBridgeSenderToEthereum, IFastBridgeSe
      * The bridgers need to watch for these events and
      * relay the messageHash on the FastBridgeReceiverOnEthereum.
      */
-    event OutgoingMessage(uint256 indexed ticketID, address indexed target, bytes32 indexed messageHash, bytes message);
+    event OutgoingMessage(
+        uint256 indexed ticketID,
+        uint256 blockNumber,
+        address target,
+        bytes32 indexed messageHash,
+        bytes message
+    );
 
     // ************************************* //
     // *        Function Modifiers         * //
@@ -80,9 +87,9 @@ contract FastBridgeSenderToEthereum is SafeBridgeSenderToEthereum, IFastBridgeSe
         ticketID = currentTicketID++;
 
         (bytes32 messageHash, bytes memory messageData) = _encode(ticketID, _receiver, _calldata);
-        emit OutgoingMessage(ticketID, _receiver, messageHash, messageData);
+        emit OutgoingMessage(ticketID, block.number, _receiver, messageHash, messageData);
 
-        tickets[ticketID] = Ticket({messageHash: messageHash, sentSafe: false});
+        tickets[ticketID] = Ticket({messageHash: messageHash, blockNumber: block.number, sentSafe: false});
     }
 
     /**
@@ -94,6 +101,8 @@ contract FastBridgeSenderToEthereum is SafeBridgeSenderToEthereum, IFastBridgeSe
      * It may require some ETH (or whichever native token) to pay for the bridging cost,
      * depending on the underlying safe bridge.
      *
+     * TODO: check if keeping _calldata in storage in sendFast() is cheaper than passing it again as a parameter here
+     *
      * @param _ticketID The ticketID as provided by `sendFast()` if any.
      * @param _receiver The L1 contract address who will receive the calldata
      * @param _calldata The receiving domain encoded message data.
@@ -112,7 +121,12 @@ contract FastBridgeSenderToEthereum is SafeBridgeSenderToEthereum, IFastBridgeSe
 
         // Safe Bridge message envelope
         bytes4 methodSelector = IFastBridgeReceiver.verifyAndRelaySafe.selector;
-        bytes memory safeMessageData = abi.encodeWithSelector(methodSelector, _ticketID, messageData);
+        bytes memory safeMessageData = abi.encodeWithSelector(
+            methodSelector,
+            _ticketID,
+            ticket.blockNumber,
+            messageData
+        );
 
         // TODO: how much ETH should be provided for bridging? add an ISafeBridgeSender.bridgingCost() if needed
         _sendSafe(address(fastBridgeReceiver), safeMessageData);
@@ -135,11 +149,11 @@ contract FastBridgeSenderToEthereum is SafeBridgeSenderToEthereum, IFastBridgeSe
         uint256 _ticketID,
         address _receiver,
         bytes memory _calldata
-    ) internal pure returns (bytes32 messageHash, bytes memory messageData) {
+    ) internal view returns (bytes32 messageHash, bytes memory messageData) {
         // Encode the receiver address with the function signature + arguments i.e calldata
         messageData = abi.encode(_receiver, _calldata);
 
-        // Compute the hash over the message header (ticketID) and body (data).
-        messageHash = keccak256(abi.encode(_ticketID, messageData));
+        // Compute the hash over the message header (ticketID, blockNumber) and body (data).
+        messageHash = keccak256(abi.encode(_ticketID, block.number, messageData));
     }
 }

contracts/src/bridge/interfaces/IFastBridgeReceiver.sol

@@ -7,9 +7,17 @@ interface IFastBridgeReceiver {
 
     function challenge(uint256 _ticketID) external payable;
 
-    function verifyAndRelay(uint256 _ticketID, bytes memory _messageData) external;
-
-    function verifyAndRelaySafe(uint256 _ticketID, bytes memory _messageData) external;
+    function verifyAndRelay(
+        uint256 _ticketID,
+        uint256 blockNumber,
+        bytes calldata _messageData
+    ) external;
+
+    function verifyAndRelaySafe(
+        uint256 _ticketID,
+        uint256 blockNumber,
+        bytes calldata _messageData
+    ) external;
 
     function withdrawClaimDeposit(uint256 _ticketID) external;