diff --git a/solidity/contracts/hooks/MerkleTreeHook.sol b/solidity/contracts/hooks/MerkleTreeHook.sol index e54ae61d29..5af7e47aec 100644 --- a/solidity/contracts/hooks/MerkleTreeHook.sol +++ b/solidity/contracts/hooks/MerkleTreeHook.sol @@ -17,6 +17,7 @@ import {MerkleLib} from "../libs/Merkle.sol"; import {Message} from "../libs/Message.sol"; import {MailboxClient} from "../client/MailboxClient.sol"; import {Indexed} from "../libs/Indexed.sol"; +import {IPostDispatchHook} from "../interfaces/hooks/IPostDispatchHook.sol"; import {AbstractPostDispatchHook} from "./libs/AbstractPostDispatchHook.sol"; import {StandardHookMetadata} from "./libs/StandardHookMetadata.sol"; @@ -49,6 +50,13 @@ contract MerkleTreeHook is AbstractPostDispatchHook, MailboxClient, Indexed { return (root(), count() - 1); } + // ============ External Functions ============ + + /// @inheritdoc IPostDispatchHook + function hookType() external pure override returns (uint8) { + return uint8(IPostDispatchHook.Types.MERKLE_TREE); + } + // ============ Internal Functions ============ /// @inheritdoc AbstractPostDispatchHook diff --git a/solidity/contracts/hooks/OPStackHook.sol b/solidity/contracts/hooks/OPStackHook.sol index 7152a7c84e..02bca6f005 100644 --- a/solidity/contracts/hooks/OPStackHook.sol +++ b/solidity/contracts/hooks/OPStackHook.sol @@ -48,7 +48,7 @@ contract OPStackHook is AbstractMessageIdAuthHook { constructor( address _mailbox, uint32 _destinationDomain, - address _ism, + bytes32 _ism, address _l1Messenger ) AbstractMessageIdAuthHook(_mailbox, _destinationDomain, _ism) { require( @@ -58,8 +58,7 @@ contract OPStackHook is AbstractMessageIdAuthHook { l1Messenger = ICrossDomainMessenger(_l1Messenger); } - // ============ External functions ============ - + // ============ Internal functions ============ function _quoteDispatch(bytes calldata, bytes calldata) internal pure @@ -69,8 +68,6 @@ contract OPStackHook is AbstractMessageIdAuthHook { return 0; // gas subsidized by the L2 } - // ============ Internal functions ============ - /// @inheritdoc AbstractMessageIdAuthHook function _sendMessageId(bytes calldata metadata, bytes memory payload) internal @@ -81,7 +78,7 @@ contract OPStackHook is AbstractMessageIdAuthHook { "OPStackHook: msgValue must be less than 2 ** 255" ); l1Messenger.sendMessage{value: metadata.msgValue(0)}( - ism, + TypeCasts.bytes32ToAddress(ism), payload, GAS_LIMIT ); diff --git a/solidity/contracts/hooks/PausableHook.sol b/solidity/contracts/hooks/PausableHook.sol index fe42df4ff3..db4ce85747 100644 --- a/solidity/contracts/hooks/PausableHook.sol +++ b/solidity/contracts/hooks/PausableHook.sol @@ -14,6 +14,7 @@ pragma solidity >=0.8.0; @@@@@@@@@ @@@@@@@@*/ import {StandardHookMetadata} from "../hooks/libs/StandardHookMetadata.sol"; +import {IPostDispatchHook} from "../interfaces/hooks/IPostDispatchHook.sol"; import {AbstractPostDispatchHook} from "./libs/AbstractPostDispatchHook.sol"; import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol"; @@ -32,6 +33,13 @@ contract PausableHook is AbstractPostDispatchHook, Ownable, Pausable { _unpause(); } + // ============ External Functions ============ + + /// @inheritdoc IPostDispatchHook + function hookType() external pure override returns (uint8) { + return uint8(IPostDispatchHook.Types.PAUSABLE); + } + // ============ Internal functions ============ /// @inheritdoc AbstractPostDispatchHook diff --git a/solidity/contracts/hooks/StaticProtocolFee.sol b/solidity/contracts/hooks/StaticProtocolFee.sol index d41690d939..15611be408 100644 --- a/solidity/contracts/hooks/StaticProtocolFee.sol +++ b/solidity/contracts/hooks/StaticProtocolFee.sol @@ -17,6 +17,7 @@ pragma solidity >=0.8.0; import {Message} from "../libs/Message.sol"; import {StandardHookMetadata} from "./libs/StandardHookMetadata.sol"; import {AbstractPostDispatchHook} from "./libs/AbstractPostDispatchHook.sol"; +import {IPostDispatchHook} from "../interfaces/hooks/IPostDispatchHook.sol"; // ============ External Imports ============ import {Address} from "@openzeppelin/contracts/utils/Address.sol"; @@ -59,6 +60,11 @@ contract StaticProtocolFee is AbstractPostDispatchHook, Ownable { // ============ External Functions ============ + /// @inheritdoc IPostDispatchHook + function hookType() external pure override returns (uint8) { + return uint8(IPostDispatchHook.Types.PROTOCOL_FEE); + } + /** * @notice Sets the protocol fee. * @param _protocolFee The new protocol fee. diff --git a/solidity/contracts/hooks/aggregation/ERC5164Hook.sol b/solidity/contracts/hooks/aggregation/ERC5164Hook.sol index 17edab8355..89be2fd804 100644 --- a/solidity/contracts/hooks/aggregation/ERC5164Hook.sol +++ b/solidity/contracts/hooks/aggregation/ERC5164Hook.sol @@ -33,7 +33,7 @@ contract ERC5164Hook is AbstractMessageIdAuthHook { constructor( address _mailbox, uint32 _destinationDomain, - address _ism, + bytes32 _ism, address _dispatcher ) AbstractMessageIdAuthHook(_mailbox, _destinationDomain, _ism) { require( @@ -43,13 +43,15 @@ contract ERC5164Hook is AbstractMessageIdAuthHook { dispatcher = IMessageDispatcher(_dispatcher); } + // ============ Internal Functions ============ + function _quoteDispatch(bytes calldata, bytes calldata) internal pure override returns (uint256) { - revert("not implemented"); + return 0; // EIP-5164 doesn't enforce a gas abstraction } function _sendMessageId( @@ -58,6 +60,10 @@ contract ERC5164Hook is AbstractMessageIdAuthHook { bytes memory payload ) internal override { require(msg.value == 0, "ERC5164Hook: no value allowed"); - dispatcher.dispatchMessage(destinationDomain, ism, payload); + dispatcher.dispatchMessage( + destinationDomain, + TypeCasts.bytes32ToAddress(ism), + payload + ); } } diff --git a/solidity/contracts/hooks/aggregation/StaticAggregationHook.sol b/solidity/contracts/hooks/aggregation/StaticAggregationHook.sol index 67e68a13f6..ee7b51383e 100644 --- a/solidity/contracts/hooks/aggregation/StaticAggregationHook.sol +++ b/solidity/contracts/hooks/aggregation/StaticAggregationHook.sol @@ -23,6 +23,11 @@ contract StaticAggregationHook is AbstractPostDispatchHook { // ============ External functions ============ + /// @inheritdoc IPostDispatchHook + function hookType() external pure override returns (uint8) { + return uint8(IPostDispatchHook.Types.AGGREGATION); + } + /// @inheritdoc AbstractPostDispatchHook function _postDispatch(bytes calldata metadata, bytes calldata message) internal diff --git a/solidity/contracts/hooks/igp/InterchainGasPaymaster.sol b/solidity/contracts/hooks/igp/InterchainGasPaymaster.sol index c6cf487db5..d2259ee860 100644 --- a/solidity/contracts/hooks/igp/InterchainGasPaymaster.sol +++ b/solidity/contracts/hooks/igp/InterchainGasPaymaster.sol @@ -92,6 +92,11 @@ contract InterchainGasPaymaster is // ============ External Functions ============ + /// @inheritdoc IPostDispatchHook + function hookType() external pure override returns (uint8) { + return uint8(IPostDispatchHook.Types.INTERCHAIN_GAS_PAYMASTER); + } + /** * @param _owner The owner of the contract. * @param _beneficiary The beneficiary. diff --git a/solidity/contracts/hooks/libs/AbstractMessageIdAuthHook.sol b/solidity/contracts/hooks/libs/AbstractMessageIdAuthHook.sol index 1fad507f71..e606507ca9 100644 --- a/solidity/contracts/hooks/libs/AbstractMessageIdAuthHook.sol +++ b/solidity/contracts/hooks/libs/AbstractMessageIdAuthHook.sol @@ -14,6 +14,7 @@ pragma solidity >=0.8.0; @@@@@@@@@ @@@@@@@@*/ // ============ Internal Imports ============ +import {IPostDispatchHook} from "../../interfaces/hooks/IPostDispatchHook.sol"; import {AbstractPostDispatchHook} from "./AbstractPostDispatchHook.sol"; import {AbstractMessageIdAuthorizedIsm} from "../../isms/hook/AbstractMessageIdAuthorizedIsm.sol"; import {TypeCasts} from "../../libs/TypeCasts.sol"; @@ -35,8 +36,8 @@ abstract contract AbstractMessageIdAuthHook is // ============ Constants ============ - // address for ISM to verify messages - address public immutable ism; + // left-padded address for ISM to verify messages + bytes32 public immutable ism; // Domain of chain on which the ISM is deployed uint32 public immutable destinationDomain; @@ -45,9 +46,9 @@ abstract contract AbstractMessageIdAuthHook is constructor( address _mailbox, uint32 _destinationDomain, - address _ism + bytes32 _ism ) MailboxClient(_mailbox) { - require(_ism != address(0), "AbstractMessageIdAuthHook: invalid ISM"); + require(_ism != bytes32(0), "AbstractMessageIdAuthHook: invalid ISM"); require( _destinationDomain != 0, "AbstractMessageIdAuthHook: invalid destination domain" @@ -56,6 +57,11 @@ abstract contract AbstractMessageIdAuthHook is destinationDomain = _destinationDomain; } + /// @inheritdoc IPostDispatchHook + function hookType() external pure returns (uint8) { + return uint8(IPostDispatchHook.Types.ID_AUTH_ISM); + } + // ============ Internal functions ============ /// @inheritdoc AbstractPostDispatchHook diff --git a/solidity/contracts/hooks/routing/DomainRoutingHook.sol b/solidity/contracts/hooks/routing/DomainRoutingHook.sol index 4b07703b4c..e37e896c81 100644 --- a/solidity/contracts/hooks/routing/DomainRoutingHook.sol +++ b/solidity/contracts/hooks/routing/DomainRoutingHook.sol @@ -42,8 +42,15 @@ contract DomainRoutingHook is AbstractPostDispatchHook, MailboxClient { _transferOwnership(_owner); } - function setHook(uint32 destination, address hook) public onlyOwner { - hooks[destination] = IPostDispatchHook(hook); + // ============ External Functions ============ + + /// @inheritdoc IPostDispatchHook + function hookType() external pure virtual override returns (uint8) { + return uint8(IPostDispatchHook.Types.ROUTING); + } + + function setHook(uint32 _destination, address _hook) public onlyOwner { + hooks[_destination] = IPostDispatchHook(_hook); } function setHooks(HookConfig[] calldata configs) external onlyOwner { diff --git a/solidity/contracts/hooks/routing/FallbackDomainRoutingHook.sol b/solidity/contracts/hooks/routing/FallbackDomainRoutingHook.sol index 4d9fc70d43..31e7603674 100644 --- a/solidity/contracts/hooks/routing/FallbackDomainRoutingHook.sol +++ b/solidity/contracts/hooks/routing/FallbackDomainRoutingHook.sol @@ -36,17 +36,25 @@ contract FallbackDomainRoutingHook is DomainRoutingHook { fallbackHook = IPostDispatchHook(_fallback); } + // ============ External Functions ============ + + /// @inheritdoc IPostDispatchHook + function hookType() external pure override returns (uint8) { + return uint8(IPostDispatchHook.Types.FALLBACK_ROUTING); + } + // ============ Internal Functions ============ function _getConfiguredHook(bytes calldata message) internal view override - returns (IPostDispatchHook hook) + returns (IPostDispatchHook) { - hook = hooks[message.destination()]; - if (address(hook) == address(0)) { - hook = fallbackHook; + IPostDispatchHook _hook = hooks[message.destination()]; + if (address(_hook) == address(0)) { + _hook = fallbackHook; } + return _hook; } } diff --git a/solidity/contracts/interfaces/hooks/IPostDispatchHook.sol b/solidity/contracts/interfaces/hooks/IPostDispatchHook.sol index df571916bb..dc4802a3e3 100644 --- a/solidity/contracts/interfaces/hooks/IPostDispatchHook.sol +++ b/solidity/contracts/interfaces/hooks/IPostDispatchHook.sol @@ -14,6 +14,23 @@ pragma solidity >=0.8.0; @@@@@@@@@ @@@@@@@@*/ interface IPostDispatchHook { + enum Types { + UNUSED, + ROUTING, + AGGREGATION, + MERKLE_TREE, + INTERCHAIN_GAS_PAYMASTER, + FALLBACK_ROUTING, + ID_AUTH_ISM, + PAUSABLE, + PROTOCOL_FEE + } + + /** + * @notice Returns an enum that represents the type of hook + */ + function hookType() external view returns (uint8); + /** * @notice Returns whether the hook supports metadata * @param metadata metadata diff --git a/solidity/contracts/isms/hook/AbstractMessageIdAuthorizedIsm.sol b/solidity/contracts/isms/hook/AbstractMessageIdAuthorizedIsm.sol index 6ad1ab0af5..cdfdc9202c 100644 --- a/solidity/contracts/isms/hook/AbstractMessageIdAuthorizedIsm.sol +++ b/solidity/contracts/isms/hook/AbstractMessageIdAuthorizedIsm.sol @@ -18,7 +18,6 @@ pragma solidity >=0.8.0; import {IInterchainSecurityModule} from "../../interfaces/IInterchainSecurityModule.sol"; import {LibBit} from "../../libs/LibBit.sol"; import {Message} from "../../libs/Message.sol"; -import {TypeCasts} from "../../libs/TypeCasts.sol"; // ============ External Imports ============ @@ -46,9 +45,9 @@ abstract contract AbstractMessageIdAuthorizedIsm is /// @dev the first bit is reserved for verification and the rest 255 bits are for the msg.value mapping(bytes32 => uint256) public verifiedMessages; /// @notice Index of verification bit in verifiedMessages - uint256 public constant MASK_INDEX = 255; - /// @notice Address for the authorized hook - address public authorizedHook; + uint256 public constant VERIFIED_MASK_INDEX = 255; + /// @notice address for the authorized hook + bytes32 public authorizedHook; // ============ Events ============ @@ -57,9 +56,9 @@ abstract contract AbstractMessageIdAuthorizedIsm is // ============ Initializer ============ - function setAuthorizedHook(address _hook) external initializer { + function setAuthorizedHook(bytes32 _hook) external initializer { require( - _hook != address(0), + _hook != bytes32(0), "AbstractMessageIdAuthorizedIsm: invalid authorized hook" ); authorizedHook = _hook; @@ -79,12 +78,18 @@ abstract contract AbstractMessageIdAuthorizedIsm is bytes32 messageId = message.id(); // check for the first bit (used for verification) - bool verified = verifiedMessages[messageId].isBitSet(MASK_INDEX); + bool verified = verifiedMessages[messageId].isBitSet( + VERIFIED_MASK_INDEX + ); // rest 255 bits contains the msg.value passed from the hook if (verified) { - payable(message.recipientAddress()).sendValue( - verifiedMessages[messageId].clearBit(MASK_INDEX) + uint256 _msgValue = verifiedMessages[messageId].clearBit( + VERIFIED_MASK_INDEX ); + if (_msgValue > 0) { + verifiedMessages[messageId] -= _msgValue; + payable(message.recipientAddress()).sendValue(_msgValue); + } } return verified; } @@ -99,8 +104,12 @@ abstract contract AbstractMessageIdAuthorizedIsm is _isAuthorized(), "AbstractMessageIdAuthorizedIsm: sender is not the hook" ); + require( + msg.value < 2**VERIFIED_MASK_INDEX, + "AbstractMessageIdAuthorizedIsm: msg.value must be less than 2^255" + ); - verifiedMessages[messageId] = msg.value.setBit(MASK_INDEX); + verifiedMessages[messageId] = msg.value.setBit(VERIFIED_MASK_INDEX); emit ReceivedMessage(messageId); } diff --git a/solidity/contracts/isms/hook/OPStackIsm.sol b/solidity/contracts/isms/hook/OPStackIsm.sol index ba60db08b2..1db350fec1 100644 --- a/solidity/contracts/isms/hook/OPStackIsm.sol +++ b/solidity/contracts/isms/hook/OPStackIsm.sol @@ -52,6 +52,7 @@ contract OPStackIsm is * @notice Check if sender is authorized to message `verifyMessageId`. */ function _isAuthorized() internal view override returns (bool) { - return _crossChainSender() == authorizedHook; + return + _crossChainSender() == TypeCasts.bytes32ToAddress(authorizedHook); } } diff --git a/solidity/contracts/test/TestPostDispatchHook.sol b/solidity/contracts/test/TestPostDispatchHook.sol index ddf357979a..1c50e2049a 100644 --- a/solidity/contracts/test/TestPostDispatchHook.sol +++ b/solidity/contracts/test/TestPostDispatchHook.sol @@ -1,6 +1,7 @@ // SPDX-License-Identifier: MIT OR Apache-2.0 pragma solidity >=0.8.0; +import {IPostDispatchHook} from "../interfaces/hooks/IPostDispatchHook.sol"; import {AbstractPostDispatchHook} from "../hooks/libs/AbstractPostDispatchHook.sol"; contract TestPostDispatchHook is AbstractPostDispatchHook { @@ -9,6 +10,13 @@ contract TestPostDispatchHook is AbstractPostDispatchHook { // test fees for quoteDispatch uint256 public fee = 0; + // ============ External Functions ============ + + /// @inheritdoc IPostDispatchHook + function hookType() external pure override returns (uint8) { + return uint8(IPostDispatchHook.Types.UNUSED); + } + function supportsMetadata(bytes calldata) public pure diff --git a/solidity/test/MerkleTreeHook.t.sol b/solidity/test/MerkleTreeHook.t.sol index 0a86aee5dd..5546523f29 100644 --- a/solidity/test/MerkleTreeHook.t.sol +++ b/solidity/test/MerkleTreeHook.t.sol @@ -7,6 +7,7 @@ import {Message} from "../contracts/libs/Message.sol"; import {TypeCasts} from "../contracts/libs/TypeCasts.sol"; import {TestMerkleTreeHook} from "../contracts/test/TestMerkleTreeHook.sol"; import {TestMailbox} from "../contracts/test/TestMailbox.sol"; +import {IPostDispatchHook} from "../contracts/interfaces/hooks/IPostDispatchHook.sol"; contract MerkleTreeHookTest is Test { using Message for bytes; @@ -68,4 +69,8 @@ contract MerkleTreeHookTest is Test { assertEq(hook.quoteDispatch("", currMessage), 0); } } + + function testHookType() public { + assertEq(hook.hookType(), uint8(IPostDispatchHook.Types.MERKLE_TREE)); + } } diff --git a/solidity/test/hooks/AggregationHook.t.sol b/solidity/test/hooks/AggregationHook.t.sol index fba5d81fa2..a6fa99fff3 100644 --- a/solidity/test/hooks/AggregationHook.t.sol +++ b/solidity/test/hooks/AggregationHook.t.sol @@ -6,6 +6,7 @@ import {Test} from "forge-std/Test.sol"; import {StaticAggregationHook} from "../../contracts/hooks/aggregation/StaticAggregationHook.sol"; import {StaticAggregationHookFactory} from "../../contracts/hooks/aggregation/StaticAggregationHookFactory.sol"; import {TestPostDispatchHook} from "../../contracts/test/TestPostDispatchHook.sol"; +import {IPostDispatchHook} from "../../contracts/interfaces/hooks/IPostDispatchHook.sol"; contract AggregationHookTest is Test { StaticAggregationHookFactory internal factory; @@ -88,4 +89,9 @@ contract AggregationHookTest is Test { address[] memory actualHook = hook.hooks(""); assertEq(actualHook, expectedHooks); } + + function testHookType() public { + deployHooks(1, 0); + assertEq(hook.hookType(), uint8(IPostDispatchHook.Types.AGGREGATION)); + } } diff --git a/solidity/test/hooks/DomainRoutingHook.t.sol b/solidity/test/hooks/DomainRoutingHook.t.sol index d8a3bcddf8..193f227413 100644 --- a/solidity/test/hooks/DomainRoutingHook.t.sol +++ b/solidity/test/hooks/DomainRoutingHook.t.sol @@ -8,6 +8,7 @@ import {DomainRoutingHook} from "../../contracts/hooks/routing/DomainRoutingHook import {FallbackDomainRoutingHook} from "../../contracts/hooks/routing/FallbackDomainRoutingHook.sol"; import {TestPostDispatchHook} from "../../contracts/test/TestPostDispatchHook.sol"; import {TestMailbox} from "../../contracts/test/TestMailbox.sol"; +import {IPostDispatchHook} from "../../contracts/interfaces/hooks/IPostDispatchHook.sol"; import {Strings} from "@openzeppelin/contracts/utils/Strings.sol"; @@ -104,6 +105,10 @@ contract DomainRoutingHookTest is Test { vm.expectRevert(); hook.postDispatch(metadata, testMessage); } + + function testHookType() public virtual { + assertEq(hook.hookType(), uint8(IPostDispatchHook.Types.ROUTING)); + } } contract FallbackDomainRoutingHookTest is DomainRoutingHookTest { @@ -162,4 +167,11 @@ contract FallbackDomainRoutingHookTest is DomainRoutingHookTest { ); hook.postDispatch(metadata, testMessage); } + + function testHookType() public override { + assertEq( + hook.hookType(), + uint8(IPostDispatchHook.Types.FALLBACK_ROUTING) + ); + } } diff --git a/solidity/test/hooks/StaticProtocolFee.t.sol b/solidity/test/hooks/StaticProtocolFee.t.sol index 5f225b1aef..7d9456de6f 100644 --- a/solidity/test/hooks/StaticProtocolFee.t.sol +++ b/solidity/test/hooks/StaticProtocolFee.t.sol @@ -5,6 +5,7 @@ import {Test} from "forge-std/Test.sol"; import {TypeCasts} from "../../contracts/libs/TypeCasts.sol"; import {MessageUtils} from "../isms/IsmTestUtils.sol"; import {StandardHookMetadata} from "../../contracts/hooks/libs/StandardHookMetadata.sol"; +import {IPostDispatchHook} from "../../contracts/interfaces/hooks/IPostDispatchHook.sol"; import {StaticProtocolFee} from "../../contracts/hooks/StaticProtocolFee.sol"; @@ -35,6 +36,10 @@ contract StaticProtocolFeeTest is Test { assertEq(fees.protocolFee(), FEE); } + function testHookType() public { + assertEq(fees.hookType(), uint8(IPostDispatchHook.Types.PROTOCOL_FEE)); + } + function testSetProtocolFee(uint256 fee) public { fee = bound(fee, 0, fees.MAX_PROTOCOL_FEE()); fees.setProtocolFee(fee); diff --git a/solidity/test/igps/InterchainGasPaymaster.t.sol b/solidity/test/igps/InterchainGasPaymaster.t.sol index 66eec452f3..aaa8fd3368 100644 --- a/solidity/test/igps/InterchainGasPaymaster.t.sol +++ b/solidity/test/igps/InterchainGasPaymaster.t.sol @@ -11,6 +11,7 @@ import {TypeCasts} from "../../contracts/libs/TypeCasts.sol"; import {InterchainGasPaymaster} from "../../contracts/hooks/igp/InterchainGasPaymaster.sol"; import {StorageGasOracle} from "../../contracts/hooks/igp/StorageGasOracle.sol"; import {IGasOracle} from "../../contracts/interfaces/IGasOracle.sol"; +import {IPostDispatchHook} from "../../contracts/interfaces/hooks/IPostDispatchHook.sol"; contract InterchainGasPaymasterTest is Test { using StandardHookMetadata for bytes; @@ -534,6 +535,13 @@ contract InterchainGasPaymasterTest is Test { assertEq(_igpBalanceAfter - _igpBalanceBefore, _quote); } + function testHookType() public { + assertEq( + igp.hookType(), + uint8(IPostDispatchHook.Types.INTERCHAIN_GAS_PAYMASTER) + ); + } + // ============ claim ============ function testClaim() public { diff --git a/solidity/test/isms/ERC5164ISM.t.sol b/solidity/test/isms/ERC5164ISM.t.sol index a2e04ffa21..4d7ade6427 100644 --- a/solidity/test/isms/ERC5164ISM.t.sol +++ b/solidity/test/isms/ERC5164ISM.t.sol @@ -9,6 +9,8 @@ import {MessageUtils} from "./IsmTestUtils.sol"; import {TypeCasts} from "../../contracts/libs/TypeCasts.sol"; import {IMessageDispatcher} from "../../contracts/interfaces/hooks/IMessageDispatcher.sol"; +import {IPostDispatchHook} from "../../contracts/interfaces/hooks/IPostDispatchHook.sol"; +import {IInterchainSecurityModule} from "../../contracts/interfaces/IInterchainSecurityModule.sol"; import {ERC5164Hook} from "../../contracts/hooks/aggregation/ERC5164Hook.sol"; import {AbstractMessageIdAuthorizedIsm} from "../../contracts/isms/hook/AbstractMessageIdAuthorizedIsm.sol"; import {ERC5164Ism} from "../../contracts/isms/hook/ERC5164Ism.sol"; @@ -63,10 +65,10 @@ contract ERC5164IsmTest is Test { hook = new ERC5164Hook( address(originMailbox), TEST2_DOMAIN, - address(ism), + address(ism).addressToBytes32(), address(dispatcher) ); - ism.setAuthorizedHook(address(hook)); + ism.setAuthorizedHook(TypeCasts.addressToBytes32(address(hook))); } /////////////////////////////////////////////////////////////////// @@ -81,7 +83,7 @@ contract ERC5164IsmTest is Test { hook = new ERC5164Hook( address(0), 0, - address(ism), + address(ism).addressToBytes32(), address(dispatcher) ); @@ -91,7 +93,7 @@ contract ERC5164IsmTest is Test { hook = new ERC5164Hook( address(originMailbox), 0, - address(ism), + address(ism).addressToBytes32(), address(dispatcher) ); @@ -99,7 +101,7 @@ contract ERC5164IsmTest is Test { hook = new ERC5164Hook( address(originMailbox), TEST2_DOMAIN, - address(0), + address(0).addressToBytes32(), address(dispatcher) ); @@ -107,7 +109,7 @@ contract ERC5164IsmTest is Test { hook = new ERC5164Hook( address(originMailbox), TEST2_DOMAIN, - address(ism), + address(ism).addressToBytes32(), address(0) ); } @@ -132,6 +134,11 @@ contract ERC5164IsmTest is Test { hook.postDispatch(bytes(""), encodedMessage); } + function testTypes() public { + assertEq(hook.hookType(), uint8(IPostDispatchHook.Types.ID_AUTH_ISM)); + assertEq(ism.moduleType(), uint8(IInterchainSecurityModule.Types.NULL)); + } + function test_postDispatch_RevertWhen_ChainIDNotSupported() public { encodedMessage = MessageUtils.formatMessage( VERSION, diff --git a/solidity/test/isms/OPStackIsm.t.sol b/solidity/test/isms/OPStackIsm.t.sol index 9541c8aabd..03ace07a37 100644 --- a/solidity/test/isms/OPStackIsm.t.sol +++ b/solidity/test/isms/OPStackIsm.t.sol @@ -97,7 +97,7 @@ contract OPStackIsmTest is Test { opHook = new OPStackHook( address(l1Mailbox), OPTIMISM_DOMAIN, - address(opISM), + TypeCasts.addressToBytes32(address(opISM)), L1_MESSENGER_ADDRESS ); @@ -119,7 +119,7 @@ contract OPStackIsmTest is Test { vm.selectFork(optimismFork); - opISM.setAuthorizedHook(address(opHook)); + opISM.setAuthorizedHook(TypeCasts.addressToBytes32(address(opHook))); // for sending value vm.deal( AddressAliasHelper.applyL1ToL2Alias(L1_MESSENGER_ADDRESS), @@ -299,25 +299,7 @@ contract OPStackIsmTest is Test { vm.selectFork(optimismFork); - bytes memory encodedHookData = abi.encodeCall( - AbstractMessageIdAuthorizedIsm.verifyMessageId, - (messageId) - ); - - (uint240 nonce, uint16 version) = decodeVersionedNonce( - l2Messenger.messageNonce() - ); - uint256 versionedNonce = encodeVersionedNonce(nonce + 1, version); - - vm.prank(AddressAliasHelper.applyL1ToL2Alias(L1_MESSENGER_ADDRESS)); - l2Messenger.relayMessage( - versionedNonce, - address(opHook), - address(opISM), - 0, - DEFAULT_GAS_LIMIT, - encodedHookData - ); + orchestrateRelayMessage(0, messageId); bool verified = opISM.verify(new bytes(0), encodedMessage); assertTrue(verified); @@ -328,60 +310,61 @@ contract OPStackIsmTest is Test { _msgValue = bound(_msgValue, 0, 2**254); deployAll(); - vm.selectFork(optimismFork); + orchestrateRelayMessage(_msgValue, messageId); - bytes memory encodedHookData = abi.encodeCall( - AbstractMessageIdAuthorizedIsm.verifyMessageId, - (messageId) - ); + bool verified = opISM.verify(new bytes(0), encodedMessage); + assertTrue(verified); - (uint240 nonce, uint16 version) = decodeVersionedNonce( - l2Messenger.messageNonce() - ); - uint256 versionedNonce = encodeVersionedNonce(nonce + 1, version); + assertEq(address(opISM).balance, 0); + assertEq(address(testRecipient).balance, _msgValue); + } - vm.prank(AddressAliasHelper.applyL1ToL2Alias(L1_MESSENGER_ADDRESS)); - l2Messenger.relayMessage{value: _msgValue}( - versionedNonce, - address(opHook), - address(opISM), - _msgValue, - DEFAULT_GAS_LIMIT, - encodedHookData - ); + /// forge-config: default.fuzz.runs = 10 + function testFork_verify_valueAlreadyClaimed(uint256 _msgValue) public { + _msgValue = bound(_msgValue, 0, 2**254); + deployAll(); + + orchestrateRelayMessage(_msgValue, messageId); bool verified = opISM.verify(new bytes(0), encodedMessage); assertTrue(verified); assertEq(address(opISM).balance, 0); assertEq(address(testRecipient).balance, _msgValue); + + // send more value to the ISM + vm.deal(address(opISM), _msgValue); + + verified = opISM.verify(new bytes(0), encodedMessage); + // verified still true + assertTrue(verified); + + assertEq(address(opISM).balance, _msgValue); + // value which was already sent + assertEq(address(testRecipient).balance, _msgValue); } - // sending over invalid message - function testFork_verify_RevertWhen_HyperlaneInvalidMessage() public { + function testFork_verify_tooMuchValue() public { deployAll(); - vm.selectFork(optimismFork); + uint256 _msgValue = 2**255 + 1; - bytes memory encodedHookData = abi.encodeCall( - AbstractMessageIdAuthorizedIsm.verifyMessageId, - (messageId) - ); + vm.expectEmit(false, false, false, false, address(l2Messenger)); + emit FailedRelayedMessage(messageId); + orchestrateRelayMessage(_msgValue, messageId); - (uint240 nonce, uint16 version) = decodeVersionedNonce( - l2Messenger.messageNonce() - ); - uint256 versionedNonce = encodeVersionedNonce(nonce + 1, version); + bool verified = opISM.verify(new bytes(0), encodedMessage); + assertFalse(verified); - vm.prank(AddressAliasHelper.applyL1ToL2Alias(L1_MESSENGER_ADDRESS)); - l2Messenger.relayMessage( - versionedNonce, - address(opHook), - address(opISM), - 0, - DEFAULT_GAS_LIMIT, - encodedHookData - ); + assertEq(address(opISM).balance, 0); + assertEq(address(testRecipient).balance, 0); + } + + // sending over invalid message + function testFork_verify_RevertWhen_HyperlaneInvalidMessage() public { + deployAll(); + + orchestrateRelayMessage(0, messageId); bytes memory invalidMessage = MessageUtils.formatMessage( HYPERLANE_VERSION, @@ -411,26 +394,7 @@ contract OPStackIsmTest is Test { testMessage ); bytes32 _messageId = Message.id(invalidMessage); - - bytes memory encodedHookData = abi.encodeCall( - AbstractMessageIdAuthorizedIsm.verifyMessageId, - (_messageId) - ); - - (uint240 nonce, uint16 version) = decodeVersionedNonce( - l2Messenger.messageNonce() - ); - uint256 versionedNonce = encodeVersionedNonce(nonce + 1, version); - - vm.prank(AddressAliasHelper.applyL1ToL2Alias(L1_MESSENGER_ADDRESS)); - l2Messenger.relayMessage( - versionedNonce, - address(opHook), - address(opISM), - 0, - DEFAULT_GAS_LIMIT, - encodedHookData - ); + orchestrateRelayMessage(0, _messageId); bool verified = opISM.verify(new bytes(0), encodedMessage); assertFalse(verified); @@ -548,4 +512,34 @@ contract OPStackIsmTest is Test { } return (nonce, version); } + + function orchestrateRelayMessage(uint256 _msgValue, bytes32 _messageId) + internal + { + vm.selectFork(optimismFork); + + bytes memory encodedHookData = abi.encodeCall( + AbstractMessageIdAuthorizedIsm.verifyMessageId, + (_messageId) + ); + + (uint240 nonce, uint16 version) = decodeVersionedNonce( + l2Messenger.messageNonce() + ); + uint256 versionedNonce = encodeVersionedNonce(nonce + 1, version); + + vm.deal( + AddressAliasHelper.applyL1ToL2Alias(L1_MESSENGER_ADDRESS), + 2**256 - 1 + ); + vm.prank(AddressAliasHelper.applyL1ToL2Alias(L1_MESSENGER_ADDRESS)); + l2Messenger.relayMessage{value: _msgValue}( + versionedNonce, + address(opHook), + address(opISM), + _msgValue, + DEFAULT_GAS_LIMIT, + encodedHookData + ); + } }