From b4112cd2e6b88b14bf97947a4f2b8db77b65cff7 Mon Sep 17 00:00:00 2001 From: Jun Kim <64379343+junkim012@users.noreply.github.com> Date: Tue, 10 Dec 2024 21:11:50 -0500 Subject: [PATCH] feat: more hyperlane require statements --- ...inHyperlaneTellerWithMultiAssetSupport.sol | 54 ++++++++++++++++++- ...HyperlaneTellerWithMultiAssetSupport.t.sol | 39 ++++++++++++++ 2 files changed, 92 insertions(+), 1 deletion(-) diff --git a/src/base/Roles/CrossChain/MultiChainHyperlaneTellerWithMultiAssetSupport.sol b/src/base/Roles/CrossChain/MultiChainHyperlaneTellerWithMultiAssetSupport.sol index 63e76c9..7089c4a 100644 --- a/src/base/Roles/CrossChain/MultiChainHyperlaneTellerWithMultiAssetSupport.sol +++ b/src/base/Roles/CrossChain/MultiChainHyperlaneTellerWithMultiAssetSupport.sol @@ -10,6 +10,8 @@ import { import { BridgeData, ERC20 } from "./CrossChainTellerBase.sol"; import { StandardHookMetadata } from "./Hyperlane/StandardHookMetadata.sol"; import { IMailbox } from "../../../interfaces/hyperlane/IMailbox.sol"; +import { IInterchainSecurityModule } from "../../../interfaces/hyperlane/IInterchainSecurityModule.sol"; +import { IPostDispatchHook } from "../../../interfaces/hyperlane/IPostDispatchHook.sol"; /** * @title MultiChainHyperlaneTellerWithMultiAssetSupport @@ -20,19 +22,37 @@ contract MultiChainHyperlaneTellerWithMultiAssetSupport is MultiChainTellerBase // ========================================= STATE ========================================= /** - * @notice The hyperlane mailbox contract. + * @notice The Hyperlane mailbox contract. */ IMailbox public immutable mailbox; + /** + * @notice The Hyperlane interchain security module. + * @dev If `address(0)`, uses the mailbox's default ISM. + */ + IInterchainSecurityModule public interchainSecurityModule; + + /** + * @notice The hook invoked after `dispatch`. + */ + IPostDispatchHook public hook; + /** * @notice A nonce used to generate unique message IDs. */ uint128 public nonce; + //============================== EVENTS =============================== + + event SetInterChainSecurityModule(address _interchainSecurityModule); + event SetPostDispatchHook(address _hook); + //============================== ERRORS =============================== error MultiChainHyperlaneTellerWithMultiAssetSupport_InvalidBridgeFeeToken(); error MultiChainHyperlaneTellerWithMultiAssetSupport_CallerMustBeMailbox(address caller); + error MultiChainHyperlaneTellerWithMultiAssetSupport_InvalidBytes32Address(bytes32 _address); + error MultiChainHyperlaneTellerWithMultiAssetSupport_ZeroAddressDestinationReceiver(); constructor( address _owner, @@ -45,6 +65,22 @@ contract MultiChainHyperlaneTellerWithMultiAssetSupport is MultiChainTellerBase mailbox = _mailbox; } + /** + * @notice Sets the post dispatch hook for Hyperlane mailbox. + */ + function setHook(IPostDispatchHook _hook) external requiresAuth { + hook = _hook; + emit SetPostDispatchHook(address(_hook)); + } + + /** + * @notice Sets a custom interchain security module for Hyperlane. + */ + function setInterchainSecurityModule(IInterchainSecurityModule _interchainSecurityModule) external requiresAuth { + interchainSecurityModule = _interchainSecurityModule; + emit SetInterChainSecurityModule(address(_interchainSecurityModule)); + } + /** * @notice function override to return the fee quote * @param shareAmount to be sent as a message @@ -90,6 +126,14 @@ contract MultiChainHyperlaneTellerWithMultiAssetSupport is MultiChainTellerBase } (uint256 shareAmount, address receiver, bytes32 messageId) = abi.decode(payload, (uint256, address, bytes32)); + + // This should never be the case since zero address + // `destinationChainReceiver` in `_bridge` is not allowed, but we have + // this as a sanity check. + if (receiver == address(0)) { + revert MultiChainHyperlaneTellerWithMultiAssetSupport_ZeroAddressDestinationReceiver(); + } + vault.enter(address(0), ERC20(address(0)), 0, receiver, shareAmount); _afterReceive(shareAmount, receiver, messageId); @@ -113,6 +157,10 @@ contract MultiChainHyperlaneTellerWithMultiAssetSupport is MultiChainTellerBase revert MultiChainHyperlaneTellerWithMultiAssetSupport_InvalidBridgeFeeToken(); } + if (data.destinationChainReceiver == address(0)) { + revert MultiChainHyperlaneTellerWithMultiAssetSupport_ZeroAddressDestinationReceiver(); + } + bytes memory _payload = abi.encode(shareAmount, data.destinationChainReceiver, messageId); // Unlike L0 that has a built in peer check, this contract must @@ -135,6 +183,10 @@ contract MultiChainHyperlaneTellerWithMultiAssetSupport is MultiChainTellerBase } function _bytes32ToAddress(bytes32 _address) internal pure returns (address) { + if (uint256(_address) > uint256(type(uint160).max)) { + revert MultiChainHyperlaneTellerWithMultiAssetSupport_InvalidBytes32Address(_address); + } + return address(uint160(uint256(_address))); } } diff --git a/test/CrossChain/MultiChainHyperlaneTellerWithMultiAssetSupport.t.sol b/test/CrossChain/MultiChainHyperlaneTellerWithMultiAssetSupport.t.sol index 62da01e..bfb94fe 100644 --- a/test/CrossChain/MultiChainHyperlaneTellerWithMultiAssetSupport.t.sol +++ b/test/CrossChain/MultiChainHyperlaneTellerWithMultiAssetSupport.t.sol @@ -226,6 +226,45 @@ contract MultiChainHyperlaneTellerWithMultiAssetSupportTest is MultiChainBaseTes vm.stopPrank(); } + function testRevertOnInvalidBytes32Address() public { + bytes32 invalidSender = bytes32(uint256(type(uint168).max)); + + vm.startPrank(address(ETHEREUM_MAILBOX)); + vm.expectRevert( + abi.encodeWithSelector( + MultiChainHyperlaneTellerWithMultiAssetSupport + .MultiChainHyperlaneTellerWithMultiAssetSupport_InvalidBytes32Address + .selector, + invalidSender + ) + ); + destinationTeller.handle(SOURCE_DOMAIN, invalidSender, ""); + vm.stopPrank(); + } + + /** + * Trying to bridge token to the zero address should fail as it will simply + * burn the token. We don't want to allow this in a bridging context. + */ + function testRevertOnInvalidDestinationReceiver() public { + deal(address(WETH), address(this), 1e18); + + sourceTeller.addChain(DESTINATION_DOMAIN, true, true, destinationTellerAddr, CHAIN_MESSAGE_GAS_LIMIT, 0); + + WETH.approve(address(boringVault), 1e18); + + vm.expectRevert( + abi.encodeWithSelector( + MultiChainHyperlaneTellerWithMultiAssetSupport + .MultiChainHyperlaneTellerWithMultiAssetSupport_ZeroAddressDestinationReceiver + .selector + ) + ); + sourceTeller.depositAndBridge( + WETH, 1e18, 1e18, BridgeData(DESTINATION_DOMAIN, address(0), ERC20(NATIVE), 80_000, "") + ); + } + function _getTypedTeller(address addr) internal returns (MultiChainHyperlaneTellerWithMultiAssetSupport) { return MultiChainHyperlaneTellerWithMultiAssetSupport(addr); }