fix(contracts): InterchainAccountRouter minor audit remediation

solidity/contracts/client/MailboxClient.sol

@@ -1,6 +1,18 @@
 // SPDX-License-Identifier: MIT OR Apache-2.0
 pragma solidity >=0.6.11;
 
+/*@@@@@@@       @@@@@@@@@
+ @@@@@@@@@       @@@@@@@@@
+  @@@@@@@@@       @@@@@@@@@
+   @@@@@@@@@       @@@@@@@@@
+    @@@@@@@@@@@@@@@@@@@@@@@@@
+     @@@@@  HYPERLANE  @@@@@@@
+    @@@@@@@@@@@@@@@@@@@@@@@@@
+   @@@@@@@@@       @@@@@@@@@
+  @@@@@@@@@       @@@@@@@@@
+ @@@@@@@@@       @@@@@@@@@
+@@@@@@@@@       @@@@@@@@*/
+
 // ============ Internal Imports ============
 import {IMailbox} from "../interfaces/IMailbox.sol";
 import {IPostDispatchHook} from "../interfaces/hooks/IPostDispatchHook.sol";
@@ -14,6 +26,9 @@ import {OwnableUpgradeable} from "@openzeppelin/contracts-upgradeable/access/Own
 abstract contract MailboxClient is OwnableUpgradeable {
     using Message for bytes;
 
+    event HookSet(address _hook);
+    event IsmSet(address _ism);
+
     IMailbox public immutable mailbox;
 
     uint32 public immutable localDomain;
@@ -62,8 +77,11 @@ abstract contract MailboxClient is OwnableUpgradeable {
      * @notice Sets the address of the application's custom hook.
      * @param _hook The address of the hook contract.
      */
-    function setHook(address _hook) public onlyContractOrNull(_hook) onlyOwner {
+    function setHook(
+        address _hook
+    ) public virtual onlyContractOrNull(_hook) onlyOwner {
         hook = IPostDispatchHook(_hook);
+        emit HookSet(_hook);
     }
 
     /**
@@ -74,6 +92,7 @@ abstract contract MailboxClient is OwnableUpgradeable {
         address _module
     ) public onlyContractOrNull(_module) onlyOwner {
         interchainSecurityModule = IInterchainSecurityModule(_module);
+        emit IsmSet(_module);
     }
 
     // ======== Initializer =========

solidity/contracts/hooks/ProtocolFee.sol

@@ -32,6 +32,9 @@ contract ProtocolFee is AbstractPostDispatchHook, Ownable {
     using Address for address payable;
     using Message for bytes;
 
+    event ProtocolFeeSet(uint256 protocolFee);
+    event BeneficiarySet(address beneficiary);
+
     // ============ Constants ============
 
     /// @notice The maximum protocol fee that can be set.
@@ -126,6 +129,7 @@ contract ProtocolFee is AbstractPostDispatchHook, Ownable {
             "ProtocolFee: exceeds max protocol fee"
         );
         protocolFee = _protocolFee;
+        emit ProtocolFeeSet(_protocolFee);
     }
 
     /**
@@ -135,5 +139,6 @@ contract ProtocolFee is AbstractPostDispatchHook, Ownable {
     function _setBeneficiary(address _beneficiary) internal {
         require(_beneficiary != address(0), "ProtocolFee: invalid beneficiary");
         beneficiary = _beneficiary;
+        emit BeneficiarySet(_beneficiary);
     }
 }

solidity/contracts/middleware/InterchainAccountRouter.sol

@@ -22,6 +22,7 @@ import {TypeCasts} from "../libs/TypeCasts.sol";
 import {StandardHookMetadata} from "../hooks/libs/StandardHookMetadata.sol";
 import {EnumerableMapExtended} from "../libs/EnumerableMapExtended.sol";
 import {Router} from "../client/Router.sol";
+import {IPostDispatchHook} from "../interfaces/hooks/IPostDispatchHook.sol";
 
 // ============ External Imports ============
 import {Create2} from "@openzeppelin/contracts/utils/Create2.sol";
@@ -160,6 +161,12 @@ contract InterchainAccountRouter is Router {
         }
     }
 
+    function setHook(
+        address _hook
+    ) public override onlyInitializing onlyContractOrNull(_hook) onlyOwner {
+        hook = IPostDispatchHook(_hook);
+    }
+
     // ============ External Functions ============
     /**
      * @notice Dispatches a single remote call to be made by an owner's
@@ -600,7 +607,14 @@ contract InterchainAccountRouter is Router {
     ) private returns (bytes32) {
         require(_router != bytes32(0), "no router specified for destination");
         emit RemoteCallDispatched(_destination, msg.sender, _router, _ism);
-        return mailbox.dispatch{value: msg.value}(_destination, _router, _body);
+        return
+            mailbox.dispatch{value: msg.value}(
+                _destination,
+                _router,
+                _body,
+                new bytes(0),
+                hook
+            );
     }
 
     /**
@@ -625,7 +639,8 @@ contract InterchainAccountRouter is Router {
                 _destination,
                 _router,
                 _body,
-                _hookMetadata
+                _hookMetadata,
+                hook
             );
     }
 
@@ -665,7 +680,13 @@ contract InterchainAccountRouter is Router {
     function quoteGasPayment(
         uint32 _destination
     ) external view returns (uint256 _gasPayment) {
-        return _quoteDispatch(_destination, "");
+        return
+            _Router_quoteDispatch(
+                _destination,
+                new bytes(0),
+                new bytes(0),
+                address(hook)
+            );
     }
 
     /**
@@ -679,13 +700,12 @@ contract InterchainAccountRouter is Router {
         bytes calldata _messageBody,
         uint256 gasLimit
     ) external view returns (uint256 _gasPayment) {
-        bytes32 _router = _mustHaveRemoteRouter(_destination);
         return
-            mailbox.quoteDispatch(
+            _Router_quoteDispatch(
                 _destination,
-                _router,
                 _messageBody,
-                StandardHookMetadata.overrideGasLimit(gasLimit)
+                StandardHookMetadata.overrideGasLimit(gasLimit),
+                address(hook)
             );
     }
 }

solidity/contracts/test/TestPostDispatchHook.sol

@@ -1,5 +1,6 @@
 // SPDX-License-Identifier: MIT OR Apache-2.0
 pragma solidity >=0.8.0;
+
 import {Message} from "../libs/Message.sol";
 
 import {IPostDispatchHook} from "../interfaces/hooks/IPostDispatchHook.sol";
@@ -35,15 +36,15 @@ contract TestPostDispatchHook is AbstractPostDispatchHook {
 
     // ============ Internal functions ============
     function _postDispatch(
-        bytes calldata /*metadata*/,
-        bytes calldata message
+        bytes calldata,
+        /*metadata*/ bytes calldata message
     ) internal override {
         messageDispatched[message.id()] = true;
     }
 
     function _quoteDispatch(
-        bytes calldata /*metadata*/,
-        bytes calldata /*message*/
+        bytes calldata,
+        /*metadata*/ bytes calldata /*message*/
     ) internal view override returns (uint256) {
         return fee;
     }

solidity/test/InterchainAccountRouter.t.sol

@@ -13,6 +13,8 @@ import {TestInterchainGasPaymaster} from "../contracts/test/TestInterchainGasPay
 import {IPostDispatchHook} from "../contracts/interfaces/hooks/IPostDispatchHook.sol";
 import {CallLib, OwnableMulticall, InterchainAccountRouter} from "../contracts/middleware/InterchainAccountRouter.sol";
 import {InterchainAccountIsm} from "../contracts/isms/routing/InterchainAccountIsm.sol";
+import {AbstractPostDispatchHook} from "../contracts/hooks/libs/AbstractPostDispatchHook.sol";
+import {TestPostDispatchHook} from "../contracts/test/TestPostDispatchHook.sol";
 
 contract Callable {
     mapping(address => bytes32) public data;
@@ -106,10 +108,11 @@ contract InterchainAccountRouterTestBase is Test {
         address owner = address(this);
         originIcaRouter = deployProxiedIcaRouter(
             environment.mailboxes(origin),
-            environment.igps(destination),
+            environment.igps(origin),
             icaIsm,
             owner
         );
+
         destinationIcaRouter = deployProxiedIcaRouter(
             environment.mailboxes(destination),
             environment.igps(destination),
@@ -152,6 +155,15 @@ contract InterchainAccountRouterTest is InterchainAccountRouterTestBase {
         assertEq(_account.owner(), address(destinationIcaRouter));
     }
 
+    function test_setHook_revertsWhen_reInitializing() public {
+        TestInterchainGasPaymaster newIgp = new TestInterchainGasPaymaster();
+
+        vm.expectRevert("Initializable: contract is not initializing");
+        originIcaRouter.setHook(address(newIgp));
+
+        assertEq(address(originIcaRouter.hook()), address(0x0));
+    }
+
     function testFuzz_getRemoteInterchainAccount(
         address _localOwner,
         address _ism
@@ -384,6 +396,25 @@ contract InterchainAccountRouterTest is InterchainAccountRouterTestBase {
         );
     }
 
+    function test_quoteDispatch_differentHook() public {
+        // arrange
+        TestPostDispatchHook testHook = new TestPostDispatchHook();
+        originIcaRouter = deployProxiedIcaRouter(
+            environment.mailboxes(origin),
+            testHook,
+            icaIsm,
+            address(this)
+        );
+        originIcaRouter.enrollRemoteRouterAndIsm(
+            destination,
+            routerOverride,
+            ismOverride
+        );
+
+        // assert
+        assertEq(originIcaRouter.quoteGasPayment(destination), 0);
+    }
+
     function testFuzz_singleCallRemoteWithDefault(bytes32 data) public {
         // arrange
         originIcaRouter.enrollRemoteRouterAndIsm(
@@ -429,6 +460,32 @@ contract InterchainAccountRouterTest is InterchainAccountRouterTestBase {
         assertIgpPayment(balanceBefore, balanceAfter, igp.getDefaultGasUsage());
     }
 
+    function testFuzz_callRemoteWithDefault_differentHook(bytes32 data) public {
+        // arrange
+        TestPostDispatchHook testHook = new TestPostDispatchHook();
+        originIcaRouter = deployProxiedIcaRouter(
+            environment.mailboxes(origin),
+            testHook,
+            icaIsm,
+            address(this)
+        );
+        originIcaRouter.enrollRemoteRouterAndIsm(
+            destination,
+            routerOverride,
+            ismOverride
+        );
+
+        // assert
+        vm.expectCall(
+            address(testHook),
+            0,
+            abi.encodePacked(AbstractPostDispatchHook.postDispatch.selector)
+        );
+
+        // act
+        originIcaRouter.callRemote(destination, getCalls(data));
+    }
+
     function testFuzz_overrideAndCallRemote(bytes32 data) public {
         // arrange
         originIcaRouter.enrollRemoteRouterAndIsm(
@@ -534,6 +591,7 @@ contract InterchainAccountRouterTest is InterchainAccountRouterTestBase {
         uint256 balanceAfter = address(this).balance;
         assertRemoteCallReceived(data);
         assertIgpPayment(balanceBefore, balanceAfter, igp.getDefaultGasUsage());
+        assertEq(address(originIcaRouter.hook()), address(0));
     }
 
     function testFuzz_callRemoteWithOverrides_metadata(
@@ -560,6 +618,35 @@ contract InterchainAccountRouterTest is InterchainAccountRouterTestBase {
         assertIgpPayment(balanceBefore, balanceAfter, gasLimit);
     }
 
+    function testFuzz_callRemoteWithOverrides_withHook(bytes32 data) public {
+        TestPostDispatchHook testHook = new TestPostDispatchHook();
+
+        originIcaRouter = deployProxiedIcaRouter(
+            environment.mailboxes(origin),
+            testHook,
+            icaIsm,
+            address(this)
+        );
+        originIcaRouter.enrollRemoteRouterAndIsm(
+            destination,
+            routerOverride,
+            ismOverride
+        );
+
+        vm.expectCall(
+            address(testHook),
+            0,
+            abi.encodePacked(AbstractPostDispatchHook.postDispatch.selector)
+        );
+        originIcaRouter.callRemoteWithOverrides(
+            destination,
+            routerOverride,
+            ismOverride,
+            getCalls(data),
+            new bytes(0)
+        );
+    }
+
     function testFuzz_callRemoteWithFailingIsmOverride(bytes32 data) public {
         // arrange
         string memory failureMessage = "failing ism";

solidity/test/hooks/ProtocolFee.t.sol

@@ -42,6 +42,9 @@ contract ProtocolFeeTest is Test {
 
     function testSetProtocolFee(uint256 fee) public {
         fee = bound(fee, 0, fees.MAX_PROTOCOL_FEE());
+
+        vm.expectEmit(true, true, true, true);
+        emit ProtocolFee.ProtocolFeeSet(fee);
         fees.setProtocolFee(fee);
         assertEq(fees.protocolFee(), fee);
     }
@@ -69,6 +72,13 @@ contract ProtocolFeeTest is Test {
         assertEq(fees.protocolFee(), FEE);
     }
 
+    function testSetBeneficiary(address beneficiary) public {
+        vm.expectEmit(true, true, true, true);
+        emit ProtocolFee.BeneficiarySet(beneficiary);
+        fees.setBeneficiary(beneficiary);
+        assertEq(fees.beneficiary(), beneficiary);
+    }
+
     function testSetBeneficiary_revertWhen_notOwner() public {
         vm.prank(charlie);