diff --git a/.changeset/neat-sloths-agree.md b/.changeset/neat-sloths-agree.md new file mode 100644 index 0000000000..979753ebec --- /dev/null +++ b/.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 diff --git a/solidity/contracts/hooks/ArbL2ToL1Hook.sol b/solidity/contracts/hooks/ArbL2ToL1Hook.sol index 76c67c0ec6..9a6365b240 100644 --- a/solidity/contracts/hooks/ArbL2ToL1Hook.sol +++ b/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{ diff --git a/solidity/contracts/hooks/OPL2ToL1Hook.sol b/solidity/contracts/hooks/OPL2ToL1Hook.sol index e82f4f4abf..165289e57f 100644 --- a/solidity/contracts/hooks/OPL2ToL1Hook.sol +++ b/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{ diff --git a/solidity/contracts/hooks/OPStackHook.sol b/solidity/contracts/hooks/OPStackHook.sol index 0ddb713d3a..695fab9cc9 100644 --- a/solidity/contracts/hooks/OPStackHook.sol +++ b/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)}( diff --git a/solidity/contracts/hooks/PolygonPosHook.sol b/solidity/contracts/hooks/PolygonPosHook.sol index 2959adc3d2..e3b1c76288 100644 --- a/solidity/contracts/hooks/PolygonPosHook.sol +++ b/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); } diff --git a/solidity/contracts/hooks/aggregation/ERC5164Hook.sol b/solidity/contracts/hooks/aggregation/ERC5164Hook.sol index ece1bf80d8..e1156679c1 100644 --- a/solidity/contracts/hooks/aggregation/ERC5164Hook.sol +++ b/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, diff --git a/solidity/contracts/hooks/layer-zero/LayerZeroV2Hook.sol b/solidity/contracts/hooks/layer-zero/LayerZeroV2Hook.sol index 3a0955e592..5e0de4a8e7 100644 --- a/solidity/contracts/hooks/layer-zero/LayerZeroV2Hook.sol +++ b/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(); diff --git a/solidity/contracts/isms/hook/AbstractMessageIdAuthorizedIsm.sol b/solidity/contracts/isms/hook/AbstractMessageIdAuthorizedIsm.sol index 43d8513456..51a896129b 100644 --- a/solidity/contracts/isms/hook/AbstractMessageIdAuthorizedIsm.sol +++ b/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); } diff --git a/solidity/contracts/isms/hook/ArbL2ToL1Ism.sol b/solidity/contracts/isms/hook/ArbL2ToL1Ism.sol index 98b5f9bd61..42593f85c6 100644 --- a/solidity/contracts/isms/hook/ArbL2ToL1Ism.sol +++ b/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( diff --git a/solidity/contracts/isms/hook/ERC5164Ism.sol b/solidity/contracts/isms/hook/ERC5164Ism.sol index 0e0d788c54..d2735039c0 100644 --- a/solidity/contracts/isms/hook/ERC5164Ism.sol +++ b/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; diff --git a/solidity/contracts/isms/hook/OPStackIsm.sol b/solidity/contracts/isms/hook/OPStackIsm.sol index 1db350fec1..504ed0d9d3 100644 --- a/solidity/contracts/isms/hook/OPStackIsm.sol +++ b/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 diff --git a/solidity/contracts/isms/hook/PolygonPosIsm.sol b/solidity/contracts/isms/hook/PolygonPosIsm.sol index 34a40360ee..8a1471d1d1 100644 --- a/solidity/contracts/isms/hook/PolygonPosIsm.sol +++ b/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 diff --git a/solidity/contracts/isms/hook/layer-zero/LayerZeroV2Ism.sol b/solidity/contracts/isms/hook/layer-zero/LayerZeroV2Ism.sol index 1388f32cf8..fedbffcda0 100644 --- a/solidity/contracts/isms/hook/layer-zero/LayerZeroV2Ism.sol +++ b/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) { diff --git a/solidity/contracts/libs/OPL2ToL1Metadata.sol b/solidity/contracts/libs/OPL2ToL1Metadata.sol index 704ce3d4d9..f0e9338912 100644 --- a/solidity/contracts/libs/OPL2ToL1Metadata.sol +++ b/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. diff --git a/solidity/test/isms/ArbL2ToL1Ism.t.sol b/solidity/test/isms/ArbL2ToL1Ism.t.sol index c44665cb5e..ec0fd93921 100644 --- a/solidity/test/isms/ArbL2ToL1Ism.t.sol +++ b/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 diff --git a/solidity/test/isms/ERC5164ISM.t.sol b/solidity/test/isms/ERC5164ISM.t.sol index 57b467fdef..f3900e621d 100644 --- a/solidity/test/isms/ERC5164ISM.t.sol +++ b/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); } } } diff --git a/solidity/test/isms/ExternalBridgeTest.sol b/solidity/test/isms/ExternalBridgeTest.sol index dca5a3c968..937e39311a 100644 --- a/solidity/test/isms/ExternalBridgeTest.sol +++ b/solidity/test/isms/ExternalBridgeTest.sol @@ -50,7 +50,8 @@ abstract contract ExternalBridgeTest is Test { /* ============ Hook.postDispatch ============ */ function test_postDispatch() public { - bytes memory encodedHookData = _encodeHookData(messageId); + bytes memory hookMetadata = testMetadata; + bytes memory encodedHookData = _encodeHookData(messageId, 0); originMailbox.updateLatestDispatchedId(messageId); _expectOriginExternalBridgeCall(encodedHookData); @@ -98,7 +99,7 @@ abstract contract ExternalBridgeTest is Test { vm.deal(address(this), address(this).balance + extraValue); uint256 valueBefore = address(this).balance; - bytes memory encodedHookData = _encodeHookData(messageId); + bytes memory encodedHookData = _encodeHookData(messageId, 0); originMailbox.updateLatestDispatchedId(messageId); _expectOriginExternalBridgeCall(encodedHookData); @@ -112,7 +113,7 @@ abstract contract ExternalBridgeTest is Test { } function test_postDispatch_revertWhen_insufficientValue() public { - bytes memory encodedHookData = _encodeHookData(messageId); + bytes memory encodedHookData = _encodeHookData(messageId, 0); originMailbox.updateLatestDispatchedId(messageId); _expectOriginExternalBridgeCall(encodedHookData); @@ -122,19 +123,16 @@ abstract contract ExternalBridgeTest is Test { hook.postDispatch{value: quote - 1}(testMetadata, encodedMessage); } - /* ============ ISM.verifyMessageId ============ */ + /* ============ ISM.preVerifyMessage ============ */ - function test_verifyMessageId_asyncCall() public { - bytes memory encodedHookData = abi.encodeCall( - AbstractMessageIdAuthorizedIsm.verifyMessageId, - (messageId) - ); + function test_preVerifyMessage_asyncCall() public { + bytes memory encodedHookData = _encodeHookData(messageId, 0); _externalBridgeDestinationCall(encodedHookData, 0); assertTrue(ism.isVerified(encodedMessage)); } - function test_verifyMessageId_externalBridgeCall() public virtual { + function test_preVerifyMessage_externalBridgeCall() public virtual { bytes memory externalCalldata = _encodeExternalDestinationBridgeCall( address(hook), address(ism), @@ -154,7 +152,7 @@ abstract contract ExternalBridgeTest is Test { } function test_verify_msgValue_asyncCall() public virtual { - bytes memory encodedHookData = _encodeHookData(messageId); + bytes memory encodedHookData = _encodeHookData(messageId, MSG_VALUE); _externalBridgeDestinationCall(encodedHookData, MSG_VALUE); assertTrue(ism.verify(new bytes(0), encodedMessage)); @@ -222,7 +220,7 @@ abstract contract ExternalBridgeTest is Test { // async call - native bridges might have try catch block to prevent revert try this.externalBridgeDestinationCallWrapper( - _encodeHookData(incorrectMessageId), + _encodeHookData(incorrectMessageId, 0), 0 ) {} catch {} @@ -232,7 +230,10 @@ abstract contract ExternalBridgeTest is Test { /// forge-config: default.fuzz.runs = 10 function test_verify_valueAlreadyClaimed(uint256 _msgValue) public virtual { _msgValue = bound(_msgValue, 0, MAX_MSG_VALUE); - _externalBridgeDestinationCall(_encodeHookData(messageId), _msgValue); + _externalBridgeDestinationCall( + _encodeHookData(messageId, _msgValue), + _msgValue + ); bool verified = ism.verify(new bytes(0), encodedMessage); assertTrue(verified); @@ -250,6 +251,18 @@ abstract contract ExternalBridgeTest is Test { assertEq(address(testRecipient).balance, _msgValue); } + function test_verify_override_msgValue() public virtual { + bytes memory encodedHookData = _encodeHookData(messageId, MSG_VALUE); + + _externalBridgeDestinationCall(encodedHookData, MSG_VALUE); + + vm.expectRevert("AbstractMessageIdAuthorizedIsm: invalid msg.value"); + _externalBridgeDestinationCall(encodedHookData, 0); + + assertTrue(ism.verify(new bytes(0), encodedMessage)); + assertEq(address(testRecipient).balance, MSG_VALUE); + } + function test_verify_false_arbitraryCall() public virtual { bytes memory incorrectCalldata = _encodeExternalDestinationBridgeCall( address(hook), @@ -275,12 +288,13 @@ abstract contract ExternalBridgeTest is Test { } function _encodeHookData( - bytes32 _messageId + bytes32 _messageId, + uint256 _msgValue ) internal pure returns (bytes memory) { return abi.encodeCall( - AbstractMessageIdAuthorizedIsm.verifyMessageId, - (_messageId) + AbstractMessageIdAuthorizedIsm.preVerifyMessage, + (_messageId, _msgValue) ); } @@ -315,5 +329,5 @@ abstract contract ExternalBridgeTest is Test { receive() external payable {} // meant to be mock an arbitrary successful call made by the external bridge - function verifyMessageId(bytes32 /*messageId*/) public payable {} + function preVerifyMessage(bytes32 /*messageId*/) public payable {} } diff --git a/solidity/test/isms/OPL2ToL1Ism.t.sol b/solidity/test/isms/OPL2ToL1Ism.t.sol index 7287ba0e11..9322713d5e 100644 --- a/solidity/test/isms/OPL2ToL1Ism.t.sol +++ b/solidity/test/isms/OPL2ToL1Ism.t.sol @@ -71,7 +71,7 @@ contract OPL2ToL1IsmTest is ExternalBridgeTest { } function test_postDispatch_childHook() public { - bytes memory encodedHookData = _encodeHookData(messageId); + bytes memory encodedHookData = _encodeHookData(messageId, 0); originMailbox.updateLatestDispatchedId(messageId); _expectOriginExternalBridgeCall(encodedHookData); @@ -117,8 +117,7 @@ contract OPL2ToL1IsmTest is ExternalBridgeTest { } function _externalBridgeDestinationCall( - bytes memory, - /*_encodedHookData*/ + bytes memory _encodedHookData, uint256 _msgValue ) internal override { vm.deal(address(portal), _msgValue); @@ -132,7 +131,7 @@ contract OPL2ToL1IsmTest is ExternalBridgeTest { data: _encodeMessengerCalldata( address(ism), _msgValue, - messageId + _encodedHookData ) }); portal.finalizeWithdrawalTransaction(withdrawal); @@ -141,10 +140,8 @@ contract OPL2ToL1IsmTest is ExternalBridgeTest { function _encodeMessengerCalldata( address _ism, uint256 _value, - bytes32 _messageId + bytes memory _encodedHookData ) internal view returns (bytes memory) { - bytes memory encodedHookData = _encodeHookData(_messageId); - return abi.encodeCall( ICrossDomainMessenger.relayMessage, @@ -154,7 +151,7 @@ contract OPL2ToL1IsmTest is ExternalBridgeTest { _ism, _value, uint256(GAS_QUOTE), - encodedHookData + _encodedHookData ) ); } @@ -164,6 +161,7 @@ contract OPL2ToL1IsmTest is ExternalBridgeTest { uint256 _value, bytes32 _messageId ) internal view returns (bytes memory) { + bytes memory encodedHookData = _encodeHookData(_messageId, _value); return abi.encode( MOCK_NONCE, @@ -171,7 +169,7 @@ contract OPL2ToL1IsmTest is ExternalBridgeTest { l1Messenger, _value, uint256(GAS_QUOTE), - _encodeMessengerCalldata(_ism, _value, _messageId) + _encodeMessengerCalldata(_ism, _value, encodedHookData) ); } } diff --git a/solidity/test/isms/OPStackIsm.t.sol b/solidity/test/isms/OPStackIsm.t.sol index 3230e59b82..8371c3b681 100644 --- a/solidity/test/isms/OPStackIsm.t.sol +++ b/solidity/test/isms/OPStackIsm.t.sol @@ -99,7 +99,10 @@ contract OPStackIsmTest is ExternalBridgeTest { function test_verify_revertsWhen_incorrectMessageId() public override { bytes32 incorrectMessageId = keccak256("incorrect message id"); - _externalBridgeDestinationCall(_encodeHookData(incorrectMessageId), 0); + _externalBridgeDestinationCall( + _encodeHookData(incorrectMessageId, 0), + 0 + ); assertFalse(ism.isVerified(testMessage)); } @@ -142,7 +145,7 @@ contract OPStackIsmTest is ExternalBridgeTest { } // SKIP - no external bridge call - function test_verifyMessageId_externalBridgeCall() public override {} + function test_preVerifyMessage_externalBridgeCall() public override {} function test_verify_msgValue_externalBridgeCall() public override {} @@ -150,12 +153,12 @@ contract OPStackIsmTest is ExternalBridgeTest { function test_verify_false_arbitraryCall() public override {} - /* ============ ISM.verifyMessageId ============ */ + /* ============ ISM.preVerifyMessage ============ */ function test_verify_revertsWhen_notAuthorizedHook() public override { // needs to be called by the canonical messenger on Optimism vm.expectRevert(NotCrossChainCall.selector); - ism.verifyMessageId(messageId); + ism.preVerifyMessage(messageId, 0); vm.startPrank(L2_MESSENGER_ADDRESS); _setExternalOriginSender(address(this)); @@ -164,7 +167,7 @@ contract OPStackIsmTest is ExternalBridgeTest { vm.expectRevert( "AbstractMessageIdAuthorizedIsm: sender is not the hook" ); - ism.verifyMessageId(messageId); + ism.preVerifyMessage(messageId, 0); } function _setExternalOriginSender( @@ -179,10 +182,11 @@ contract OPStackIsmTest is ExternalBridgeTest { function test_verify_tooMuchValue() public { uint256 _msgValue = 2 ** 255 + 1; - vm.expectRevert( - "AbstractMessageIdAuthorizedIsm: msg.value must be less than 2^255" + vm.expectRevert("AbstractMessageIdAuthorizedIsm: invalid msg.value"); + _externalBridgeDestinationCall( + _encodeHookData(messageId, _msgValue), + _msgValue ); - _externalBridgeDestinationCall(_encodeHookData(messageId), _msgValue); assertFalse(ism.isVerified(encodedMessage)); diff --git a/solidity/test/isms/PolygonPosIsm.t.sol b/solidity/test/isms/PolygonPosIsm.t.sol index fd26afea29..7d30dca4fd 100644 --- a/solidity/test/isms/PolygonPosIsm.t.sol +++ b/solidity/test/isms/PolygonPosIsm.t.sol @@ -78,7 +78,7 @@ contract PolygonPosIsmTest is Test { bytes data ); - event ReceivedMessage(bytes32 indexed messageId); + event ReceivedMessage(bytes32 indexed messageId, uint256 msgValue); function setUp() public { // block numbers to fork from, chain data is cached to ../../forge-cache/ @@ -155,8 +155,8 @@ contract PolygonPosIsmTest is Test { vm.selectFork(mainnetFork); bytes memory encodedHookData = abi.encodeCall( - AbstractMessageIdAuthorizedIsm.verifyMessageId, - (messageId) + AbstractMessageIdAuthorizedIsm.preVerifyMessage, + (messageId, 0) ); l1Mailbox.updateLatestDispatchedId(messageId); @@ -228,22 +228,22 @@ contract PolygonPosIsmTest is Test { polygonPosHook.postDispatch(testMetadata, encodedMessage); } - /* ============ ISM.verifyMessageId ============ */ + /* ============ ISM.preVerifyMessage ============ */ - function testFork_verifyMessageId() public { + function testFork_preVerifyMessage() public { deployAll(); vm.selectFork(polygonPosFork); bytes memory encodedHookData = abi.encodeCall( - AbstractMessageIdAuthorizedIsm.verifyMessageId, - (messageId) + AbstractMessageIdAuthorizedIsm.preVerifyMessage, + (messageId, 0) ); vm.startPrank(POLYGON_CROSSCHAIN_SYSTEM_ADDR); vm.expectEmit(true, false, false, false, address(polygonPosISM)); - emit ReceivedMessage(messageId); + emit ReceivedMessage(messageId, 0); // FIX: expect other events fxChild.onStateReceive( @@ -259,14 +259,14 @@ contract PolygonPosIsmTest is Test { vm.stopPrank(); } - function testFork_verifyMessageId_RevertWhen_NotAuthorized() public { + function testFork_preVerifyMessage_RevertWhen_NotAuthorized() public { deployAll(); vm.selectFork(polygonPosFork); // needs to be called by the fxchild on Polygon vm.expectRevert(NotCrossChainCall.selector); - polygonPosISM.verifyMessageId(messageId); + polygonPosISM.preVerifyMessage(messageId, 0); vm.startPrank(MAINNET_FX_CHILD); @@ -274,7 +274,7 @@ contract PolygonPosIsmTest is Test { vm.expectRevert( "AbstractMessageIdAuthorizedIsm: sender is not the hook" ); - polygonPosISM.verifyMessageId(messageId); + polygonPosISM.preVerifyMessage(messageId, 0); } /* ============ ISM.verify ============ */ @@ -349,8 +349,8 @@ contract PolygonPosIsmTest is Test { vm.selectFork(polygonPosFork); bytes memory encodedHookData = abi.encodeCall( - AbstractMessageIdAuthorizedIsm.verifyMessageId, - (_messageId) + AbstractMessageIdAuthorizedIsm.preVerifyMessage, + (_messageId, 0) ); vm.prank(POLYGON_CROSSCHAIN_SYSTEM_ADDR); diff --git a/solidity/test/isms/layer-zero/LayerZeroV2Ism.t.sol b/solidity/test/isms/layer-zero/LayerZeroV2Ism.t.sol index 0d778a4d92..ac3ff38cfb 100644 --- a/solidity/test/isms/layer-zero/LayerZeroV2Ism.t.sol +++ b/solidity/test/isms/layer-zero/LayerZeroV2Ism.t.sol @@ -23,8 +23,8 @@ contract LayerZeroV2IsmTest is Test { ) internal pure returns (bytes memory) { return abi.encodeCall( - AbstractMessageIdAuthorizedIsm.verifyMessageId, - (_messageId) + AbstractMessageIdAuthorizedIsm.preVerifyMessage, + (_messageId, 0) ); } @@ -133,7 +133,7 @@ contract LayerZeroV2IsmTest is Test { vm.stopPrank(); } - function testLzV2Ism_verifyMessageId_SetsCorrectMessageId( + function testLzV2Ism_preVerifyMessage_SetsCorrectMessageId( bytes32 messageId ) public { lZIsm.setAuthorizedHook(hook.addressToBytes32());