fix(contracts): add msg.Value to verifyMessageId (#4541)

### Description

- Added to the msg.value to verifyMessageId to test if the msgValue
passed in is the same as the msgValue used postDispatch

### Drive-by changes

- None

### Related issues

- fixes chainlight-io/2024-08-hyperlane#3

### Backward compatibility

Yes

### Testing

Unit tests

.changeset/neat-sloths-agree.md

@@ -0,0 +1,5 @@
+---
+'@hyperlane-xyz/core': minor
+---
+
+Added msg.value to preverifyMessage to commit it as part of external hook payload

solidity/contracts/hooks/ArbL2ToL1Hook.sol

@@ -78,8 +78,8 @@ contract ArbL2ToL1Hook is AbstractMessageIdAuthHook {
         bytes calldata message
     ) internal override {
         bytes memory payload = abi.encodeCall(
-            AbstractMessageIdAuthorizedIsm.verifyMessageId,
-            message.id()
+            AbstractMessageIdAuthorizedIsm.preVerifyMessage,
+            (message.id(), metadata.msgValue(0))
         );
 
         childHook.postDispatch{

solidity/contracts/hooks/OPL2ToL1Hook.sol

@@ -79,8 +79,8 @@ contract OPL2ToL1Hook is AbstractMessageIdAuthHook {
         bytes calldata message
     ) internal override {
         bytes memory payload = abi.encodeCall(
-            AbstractMessageIdAuthorizedIsm.verifyMessageId,
-            message.id()
+            AbstractMessageIdAuthorizedIsm.preVerifyMessage,
+            (message.id(), metadata.msgValue(0))
         );
 
         childHook.postDispatch{

solidity/contracts/hooks/OPStackHook.sol

@@ -74,8 +74,8 @@ contract OPStackHook is AbstractMessageIdAuthHook {
         bytes calldata message
     ) internal override {
         bytes memory payload = abi.encodeCall(
-            AbstractMessageIdAuthorizedIsm.verifyMessageId,
-            message.id()
+            AbstractMessageIdAuthorizedIsm.preVerifyMessage,
+            (message.id(), metadata.msgValue(0))
         );
 
         l1Messenger.sendMessage{value: metadata.msgValue(0)}(

solidity/contracts/hooks/PolygonPosHook.sol

@@ -76,8 +76,8 @@ contract PolygonPosHook is AbstractMessageIdAuthHook, FxBaseRootTunnel {
         require(msg.value == 0, "PolygonPosHook: does not support msgValue");
 
         bytes memory payload = abi.encodeCall(
-            AbstractMessageIdAuthorizedIsm.verifyMessageId,
-            message.id()
+            AbstractMessageIdAuthorizedIsm.preVerifyMessage,
+            (message.id(), metadata.msgValue(0))
         );
         _sendMessageToChild(payload);
     }

solidity/contracts/hooks/aggregation/ERC5164Hook.sol

@@ -16,6 +16,7 @@ pragma solidity >=0.8.0;
 // ============ Internal Imports ============
 import {TypeCasts} from "../../libs/TypeCasts.sol";
 import {Message} from "../../libs/Message.sol";
+import {StandardHookMetadata} from "../libs/StandardHookMetadata.sol";
 import {IPostDispatchHook} from "../../interfaces/hooks/IPostDispatchHook.sol";
 import {IMessageDispatcher} from "../../interfaces/hooks/IMessageDispatcher.sol";
 import {AbstractMessageIdAuthHook} from "../libs/AbstractMessageIdAuthHook.sol";
@@ -30,6 +31,7 @@ import {Address} from "@openzeppelin/contracts/utils/Address.sol";
  * any of the 5164 adapters.
  */
 contract ERC5164Hook is AbstractMessageIdAuthHook {
+    using StandardHookMetadata for bytes;
     using Message for bytes;
 
     IMessageDispatcher public immutable dispatcher;
@@ -57,15 +59,14 @@ contract ERC5164Hook is AbstractMessageIdAuthHook {
     }
 
     function _sendMessageId(
-        bytes calldata,
-        /* metadata */
+        bytes calldata metadata,
         bytes calldata message
     ) internal override {
         require(msg.value == 0, "ERC5164Hook: no value allowed");
 
         bytes memory payload = abi.encodeCall(
-            AbstractMessageIdAuthorizedIsm.verifyMessageId,
-            message.id()
+            AbstractMessageIdAuthorizedIsm.preVerifyMessage,
+            (message.id(), metadata.msgValue(0))
         );
         dispatcher.dispatchMessage(
             destinationDomain,

solidity/contracts/hooks/layer-zero/LayerZeroV2Hook.sol

@@ -59,8 +59,8 @@ contract LayerZeroV2Hook is AbstractMessageIdAuthHook {
         bytes calldata message
     ) internal override {
         bytes memory payload = abi.encodeCall(
-            AbstractMessageIdAuthorizedIsm.verifyMessageId,
-            message.id()
+            AbstractMessageIdAuthorizedIsm.preVerifyMessage,
+            (message.id(), metadata.msgValue(0))
         );
 
         bytes calldata lZMetadata = metadata.getCustomMetadata();

solidity/contracts/isms/hook/AbstractMessageIdAuthorizedIsm.sol

@@ -53,7 +53,7 @@ abstract contract AbstractMessageIdAuthorizedIsm is
     // ============ Events ============
 
     /// @notice Emitted when a message is received from the external bridge
-    event ReceivedMessage(bytes32 indexed messageId);
+    event ReceivedMessage(bytes32 indexed messageId, uint256 msgValue);
 
     // ============ Initializer ============
 
@@ -101,7 +101,7 @@ abstract contract AbstractMessageIdAuthorizedIsm is
     }
 
     /**
-     * @notice Check if a message is verified through verifyMessageId first.
+     * @notice Check if a message is verified through preVerifyMessage first.
      * @param message Message to check.
      */
     function isVerified(bytes calldata message) public view returns (bool) {
@@ -115,24 +115,31 @@ abstract contract AbstractMessageIdAuthorizedIsm is
      * @dev Only callable by the authorized hook.
      * @param messageId Hyperlane Id of the message.
      */
-    function verifyMessageId(bytes32 messageId) public payable virtual {
+    function preVerifyMessage(
+        bytes32 messageId,
+        uint256 msgValue
+    ) public payable virtual {
         require(
             _isAuthorized(),
             "AbstractMessageIdAuthorizedIsm: sender is not the hook"
         );
         require(
-            msg.value < 2 ** VERIFIED_MASK_INDEX,
-            "AbstractMessageIdAuthorizedIsm: msg.value must be less than 2^255"
+            msg.value < 2 ** VERIFIED_MASK_INDEX && msg.value == msgValue,
+            "AbstractMessageIdAuthorizedIsm: invalid msg.value"
+        );
+        require(
+            verifiedMessages[messageId] == 0,
+            "AbstractMessageIdAuthorizedIsm: message already verified"
         );
 
         verifiedMessages[messageId] = msg.value.setBit(VERIFIED_MASK_INDEX);
-        emit ReceivedMessage(messageId);
+        emit ReceivedMessage(messageId, msgValue);
     }
 
     // ============ Internal Functions ============
 
     /**
-     * @notice Check if sender is authorized to message `verifyMessageId`.
+     * @notice Check if sender is authorized to message `preVerifyMessage`.
      */
     function _isAuthorized() internal view virtual returns (bool);
 }

solidity/contracts/isms/hook/ArbL2ToL1Ism.sol

@@ -44,6 +44,10 @@ contract ArbL2ToL1Ism is
     // arbitrum nitro contract on L1 to forward verification
     IOutbox public arbOutbox;
 
+    uint256 private constant DATA_LENGTH = 68;
+
+    uint256 private constant MESSAGE_ID_END = 36;
+
     // ============ Constructor ============
 
     constructor(address _bridge) CrossChainEnabledArbitrumL1(_bridge) {
@@ -110,13 +114,16 @@ contract ArbL2ToL1Ism is
             l2Sender == TypeCasts.bytes32ToAddress(authorizedHook),
             "ArbL2ToL1Ism: l2Sender != authorizedHook"
         );
-        // this data is an abi encoded call of verifyMessageId(bytes32 messageId)
-        require(data.length == 36, "ArbL2ToL1Ism: invalid data length");
+        // this data is an abi encoded call of preVerifyMessage(bytes32 messageId)
+        require(
+            data.length == DATA_LENGTH,
+            "ArbL2ToL1Ism: invalid data length"
+        );
         bytes32 messageId = message.id();
         bytes32 convertedBytes;
         assembly {
             // data = 0x[4 bytes function signature][32 bytes messageId]
-            convertedBytes := mload(add(data, 36))
+            convertedBytes := mload(add(data, MESSAGE_ID_END))
         }
         // check if the parsed message id matches the message id of the message
         require(

solidity/contracts/isms/hook/ERC5164Ism.sol

@@ -44,7 +44,7 @@ contract ERC5164Ism is AbstractMessageIdAuthorizedIsm {
     }
 
     /**
-     * @notice Check if sender is authorized to message `verifyMessageId`.
+     * @notice Check if sender is authorized to message `preVerifyMessage`.
      */
     function _isAuthorized() internal view override returns (bool) {
         return msg.sender == executor;

solidity/contracts/isms/hook/OPStackIsm.sol

@@ -49,7 +49,7 @@ contract OPStackIsm is
     // ============ Internal function ============
 
     /**
-     * @notice Check if sender is authorized to message `verifyMessageId`.
+     * @notice Check if sender is authorized to message `preVerifyMessage`.
      */
     function _isAuthorized() internal view override returns (bool) {
         return

solidity/contracts/isms/hook/PolygonPosIsm.sol

@@ -49,7 +49,7 @@ contract PolygonPosIsm is
     // ============ Internal function ============
 
     /**
-     * @notice Check if sender is authorized to message `verifyMessageId`.
+     * @notice Check if sender is authorized to message `preVerifyMessage`.
      */
     function _isAuthorized() internal view override returns (bool) {
         return

solidity/contracts/isms/hook/layer-zero/LayerZeroV2Ism.sol

@@ -57,7 +57,7 @@ contract LayerZeroV2Ism is AbstractMessageIdAuthorizedIsm {
     /**
      * @notice Entry point for receiving msg/packet from the LayerZero endpoint.
      * @param _lzMessage The payload of the received message.
-     * @dev Authorization verification is done within verifyMessageId() -> _isAuthorized()
+     * @dev Authorization verification is done within preVerifyMessage() -> _isAuthorized()
      */
     function lzReceive(
         Origin calldata,
@@ -66,25 +66,36 @@ contract LayerZeroV2Ism is AbstractMessageIdAuthorizedIsm {
         address,
         bytes calldata
     ) external payable {
-        verifyMessageId(_messageId(_lzMessage));
+        preVerifyMessage(_messageId(_lzMessage), _msgValue(_lzMessage));
     }
 
     // ============ Internal function ============
 
     /**
      * @notice Slices the messageId from the message delivered from LayerZeroV2Hook
-     * @dev message is created as abi.encodeCall(AbstractMessageIdAuthorizedIsm.verifyMessageId, id)
+     * @dev message is created as abi.encodeCall(AbstractMessageIdAuthorizedIsm.preVerifyMessage, id)
      * @dev _message will be 36 bytes (4 bytes for function selector, and 32 bytes for messageId)
      */
     function _messageId(
         bytes calldata _message
     ) internal pure returns (bytes32) {
-        return bytes32(_message[FUNC_SELECTOR_OFFSET:]);
+        return bytes32(_message[FUNC_SELECTOR_OFFSET:ORIGIN_SENDER_OFFSET]);
+    }
+
+    /**
+     * @notice Slices the msgValue from the message delivered from LayerZeroV2Hook
+     * @dev message is created as abi.encodeCall(AbstractMessageIdAuthorizedIsm.preVerifyMessage, (id,msgValue))
+     * @dev _message will be 68 bytes (4 bytes for function selector, and 32 bytes for messageId, another 32 for msgValue)
+     */
+    function _msgValue(
+        bytes calldata _message
+    ) internal pure returns (uint256) {
+        return uint256(bytes32(_message[ORIGIN_SENDER_OFFSET:]));
     }
 
     /**
      * @notice Validates criteria to verify a message
-     * @dev this is called by AbstractMessageIdAuthorizedIsm.verifyMessageId
+     * @dev this is called by AbstractMessageIdAuthorizedIsm.preVerifyMessage
      * @dev parses msg.value to get parameters from lzReceive()
      */
     function _isAuthorized() internal view override returns (bool) {

solidity/contracts/libs/OPL2ToL1Metadata.sol

@@ -7,7 +7,7 @@ pragma solidity >=0.8.0;
  */
 library OPL2ToL1Metadata {
     // bottom offset to the start of message id in the metadata
-    uint256 private constant MESSAGE_ID_OFFSET = 88;
+    uint256 private constant MESSAGE_ID_OFFSET = 120;
     // from IOptimismPortal.WithdrawalTransaction
     // Σ {
     //      nonce                          = 32 bytes
@@ -20,7 +20,7 @@ library OPL2ToL1Metadata {
     //          LENGTH                      = 32 bytes
     // } = 252 bytes
     uint256 private constant FIXED_METADATA_LENGTH = 252;
-    // metadata here is double encoded call relayMessage(..., verifyMessageId)
+    // metadata here is double encoded call relayMessage(..., preVerifyMessage)
     // Σ {
     //      _selector                       =  4 bytes
     //      _nonce                          = 32 bytes
@@ -31,9 +31,9 @@ library OPL2ToL1Metadata {
     //      _data
     //          OFFSET                      = 32 bytes
     //          LENGTH                      = 32 bytes
-    //          PADDING + verifyMessageId   = 64 bytes
-    // } = 292 bytes
-    uint256 private constant MESSENGER_CALLDATA_LENGTH = 292;
+    //          PADDING + preVerifyMessage   = 96 bytes
+    // } = 324 bytes
+    uint256 private constant MESSENGER_CALLDATA_LENGTH = 324;
 
     /**
      * @notice Returns the message ID.

solidity/test/isms/ArbL2ToL1Ism.t.sol

@@ -63,7 +63,7 @@ contract ArbL2ToL1IsmTest is ExternalBridgeTest {
     }
 
     function test_postDispatch_childHook() public {
-        bytes memory encodedHookData = _encodeHookData(messageId);
+        bytes memory encodedHookData = _encodeHookData(messageId, 0);
         originMailbox.updateLatestDispatchedId(messageId);
         _expectOriginExternalBridgeCall(encodedHookData);
 
@@ -131,10 +131,7 @@ contract ArbL2ToL1IsmTest is ExternalBridgeTest {
         bytes32 _messageId,
         uint256 _value
     ) internal view returns (bytes memory) {
-        bytes memory encodedHookData = abi.encodeCall(
-            AbstractMessageIdAuthorizedIsm.verifyMessageId,
-            (_messageId)
-        );
+        bytes memory encodedHookData = _encodeHookData(_messageId, _value);
 
         bytes32[] memory proof = new bytes32[](16);
         return

solidity/test/isms/ERC5164ISM.t.sol

@@ -134,7 +134,7 @@ contract ERC5164IsmTest is ExternalBridgeTest {
         vm.expectRevert(
             "AbstractMessageIdAuthorizedIsm: sender is not the hook"
         );
-        ism.verifyMessageId(messageId);
+        ism.preVerifyMessage(messageId, 0);
         assertFalse(ism.isVerified(encodedMessage));
     }
 
@@ -150,6 +150,8 @@ contract ERC5164IsmTest is ExternalBridgeTest {
 
     function test_verify_valueAlreadyClaimed(uint256) public override {}
 
+    function test_verify_override_msgValue() public override {}
+
     function testFuzz_postDispatch_refundsExtraValue(uint256) public override {}
 
     function test_verify_false_arbitraryCall() public override {}
@@ -161,7 +163,7 @@ contract ERC5164IsmTest is ExternalBridgeTest {
         uint256 _msgValue
     ) internal override {
         vm.prank(address(executor));
-        ism.verifyMessageId(messageId);
+        ism.preVerifyMessage(messageId, 0);
     }
 
     function _encodeExternalDestinationBridgeCall(
@@ -172,7 +174,7 @@ contract ERC5164IsmTest is ExternalBridgeTest {
     ) internal override returns (bytes memory) {
         if (_from == address(hook)) {
             vm.prank(address(executor));
-            ism.verifyMessageId{value: _msgValue}(messageId);
+            ism.preVerifyMessage{value: _msgValue}(messageId, 0);
         }
     }
 }