ConnectionClient v2 (#1288)

* Add ISM to ConnectionClient

* Add Router#enrollRemoteRouters (#1310)

* Use batch enroll remote routers in deployer

* Fix zero address tests

Co-authored-by: Nam Chu Hoai <nambrot@googlemail.com>

solidity/contracts/HyperlaneConnectionClient.sol

@@ -3,12 +3,17 @@ pragma solidity >=0.6.11;
 
 // ============ Internal Imports ============
 import {IInterchainGasPaymaster} from "../interfaces/IInterchainGasPaymaster.sol";
+import {ISpecifiesInterchainSecurityModule, IInterchainSecurityModule} from "../interfaces/IInterchainSecurityModule.sol";
 import {IMailbox} from "../interfaces/IMailbox.sol";
 
 // ============ External Imports ============
 import {OwnableUpgradeable} from "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";
+import {Address} from "@openzeppelin/contracts/utils/Address.sol";
 
-abstract contract HyperlaneConnectionClient is OwnableUpgradeable {
+abstract contract HyperlaneConnectionClient is
+    OwnableUpgradeable,
+    ISpecifiesInterchainSecurityModule
+{
     // ============ Mutable Storage ============
 
     IMailbox public mailbox;
@@ -17,10 +22,11 @@ abstract contract HyperlaneConnectionClient is OwnableUpgradeable {
     // otherwise payments made to the paymaster will not result in relayed messages.
     IInterchainGasPaymaster public interchainGasPaymaster;
 
+    IInterchainSecurityModule public interchainSecurityModule;
+
     uint256[48] private __GAP; // gap for upgrade safety
 
     // ============ Events ============
-
     /**
      * @notice Emitted when a new mailbox is set.
      * @param mailbox The address of the mailbox contract
@@ -33,6 +39,8 @@ abstract contract HyperlaneConnectionClient is OwnableUpgradeable {
      */
     event InterchainGasPaymasterSet(address indexed interchainGasPaymaster);
 
+    event InterchainSecurityModuleSet(address indexed module);
+
     // ============ Modifiers ============
 
     /**
@@ -43,6 +51,14 @@ abstract contract HyperlaneConnectionClient is OwnableUpgradeable {
         _;
     }
 
+    /**
+     * @notice Only accept addresses that at least have contract code
+     */
+    modifier onlyContract(address _contract) {
+        require(Address.isContract(_contract), "!contract");
+        _;
+    }
+
     // ======== Initializer =========
 
     function __HyperlaneConnectionClient_initialize(address _mailbox)
@@ -61,6 +77,18 @@ abstract contract HyperlaneConnectionClient is OwnableUpgradeable {
         __HyperlaneConnectionClient_initialize(_mailbox);
     }
 
+    function __HyperlaneConnectionClient_initialize(
+        address _mailbox,
+        address _interchainGasPaymaster,
+        address _interchainSecurityModule
+    ) internal onlyInitializing {
+        _setInterchainSecurityModule(_interchainSecurityModule);
+        __HyperlaneConnectionClient_initialize(
+            _mailbox,
+            _interchainGasPaymaster
+        );
+    }
+
     // ============ External functions ============
 
     /**
@@ -83,6 +111,14 @@ abstract contract HyperlaneConnectionClient is OwnableUpgradeable {
         _setInterchainGasPaymaster(_interchainGasPaymaster);
     }
 
+    function setInterchainSecurityModule(address _module)
+        external
+        virtual
+        onlyOwner
+    {
+        _setInterchainSecurityModule(_module);
+    }
+
     // ============ Internal functions ============
 
     /**
@@ -91,6 +127,7 @@ abstract contract HyperlaneConnectionClient is OwnableUpgradeable {
      */
     function _setInterchainGasPaymaster(address _interchainGasPaymaster)
         internal
+        onlyContract(_interchainGasPaymaster)
     {
         interchainGasPaymaster = IInterchainGasPaymaster(
             _interchainGasPaymaster
@@ -102,8 +139,16 @@ abstract contract HyperlaneConnectionClient is OwnableUpgradeable {
      * @notice Modify the contract the Application uses to validate Inbox contracts
      * @param _mailbox The address of the mailbox contract
      */
-    function _setMailbox(address _mailbox) internal {
+    function _setMailbox(address _mailbox) internal onlyContract(_mailbox) {
         mailbox = IMailbox(_mailbox);
         emit MailboxSet(_mailbox);
     }
+
+    function _setInterchainSecurityModule(address _module)
+        internal
+        onlyContract(_module)
+    {
+        interchainSecurityModule = IInterchainSecurityModule(_module);
+        emit InterchainSecurityModuleSet(_module);
+    }
 }

solidity/contracts/Mailbox.sol

@@ -218,9 +218,11 @@ contract Mailbox is
         try _recipient.interchainSecurityModule() returns (
             IInterchainSecurityModule _val
         ) {
-            return _val;
-        } catch {
-            return defaultIsm;
-        }
+            // If the recipient specifies a zero address, use the default ISM.
+            if (address(_val) != address(0)) {
+                return _val;
+            }
+        } catch {}
+        return defaultIsm;
     }
 }

solidity/contracts/Router.sol

@@ -69,6 +69,21 @@ abstract contract Router is HyperlaneConnectionClient, IMessageRecipient {
         _enrollRemoteRouter(_domain, _router);
     }
 
+    /**
+     * @notice Batch version of `enrollRemoteRouter`
+     * @param _domains The domaisn of the remote Application Routers
+     * @param _routers The addresses of the remote Application Routers
+     */
+    function enrollRemoteRouters(
+        uint32[] calldata _domains,
+        bytes32[] calldata _routers
+    ) external virtual onlyOwner {
+        require(_domains.length == _routers.length, "!length");
+        for (uint256 i = 0; i < _domains.length; i += 1) {
+            _enrollRemoteRouter(_domains[i], _routers[i]);
+        }
+    }
+
     /**
      * @notice Handles an incoming message
      * @param _origin The origin domain

solidity/contracts/middleware/liquidity-layer/LiquidityLayerRouter.sol

@@ -24,11 +24,11 @@ contract LiquidityLayerRouter is Router {
         address _mailbox,
         address _interchainGasPaymaster
     ) public initializer {
-        // Transfer ownership of the contract to deployer
+        __HyperlaneConnectionClient_initialize(
+            _mailbox,
+            _interchainGasPaymaster
+        );
         _transferOwnership(_owner);
-        // Alternatively, this could be done later in an initialize method
-        _setMailbox(_mailbox);
-        _setInterchainGasPaymaster(_interchainGasPaymaster);
     }
 
     function dispatchWithTokens(

solidity/contracts/middleware/liquidity-layer/adapters/CircleBridgeAdapter.sol

@@ -74,9 +74,6 @@ contract CircleBridgeAdapter is ILiquidityLayerAdapter, Router {
         // Transfer ownership of the contract to deployer
         _transferOwnership(_owner);
 
-        // Set the addresses for the IGP to address(0) - it's not used.
-        _setInterchainGasPaymaster(address(0));
-
         circleBridge = ICircleBridge(_circleBridge);
         circleMessageTransmitter = ICircleMessageTransmitter(
             _circleMessageTransmitter

solidity/test/hyperlaneConnectionClient.test.ts

@@ -18,6 +18,7 @@ const ONLY_OWNER_REVERT_MSG = 'Ownable: caller is not the owner';
 describe('HyperlaneConnectionClient', async () => {
   let connectionClient: TestHyperlaneConnectionClient,
     mailbox: Mailbox,
+    newMailbox: Mailbox,
     signer: SignerWithAddress,
     nonOwner: SignerWithAddress;
 
@@ -29,6 +30,7 @@ describe('HyperlaneConnectionClient', async () => {
     const mailboxFactory = new Mailbox__factory(signer);
     const domain = 1000;
     mailbox = await mailboxFactory.deploy(domain);
+    newMailbox = await mailboxFactory.deploy(domain);
 
     const connectionClientFactory = new TestHyperlaneConnectionClient__factory(
       signer,
@@ -39,23 +41,22 @@ describe('HyperlaneConnectionClient', async () => {
 
   it('Cannot be initialized twice', async () => {
     await expect(
-      connectionClient.initialize(ethers.constants.AddressZero),
+      connectionClient.initialize(mailbox.address),
     ).to.be.revertedWith('Initializable: contract is already initialized');
   });
 
   it('owner can set mailbox', async () => {
-    const newMailbox = signer.address;
-    expect(await connectionClient.mailbox()).to.not.equal(newMailbox);
-    await expect(connectionClient.setMailbox(newMailbox)).to.emit(
+    expect(await connectionClient.mailbox()).to.not.equal(newMailbox.address);
+    await expect(connectionClient.setMailbox(newMailbox.address)).to.emit(
       connectionClient,
       'MailboxSet',
     );
-    expect(await connectionClient.mailbox()).to.equal(newMailbox);
+    expect(await connectionClient.mailbox()).to.equal(newMailbox.address);
   });
 
   it('non-owner cannot set mailbox', async () => {
     await expect(
-      connectionClient.connect(nonOwner).setMailbox(signer.address),
+      connectionClient.connect(nonOwner).setMailbox(newMailbox.address),
     ).to.be.revertedWith(ONLY_OWNER_REVERT_MSG);
   });
 

solidity/test/router.test.ts

@@ -59,10 +59,10 @@ describe('Router', async () => {
     });
 
     it('cannot be initialized twice', async () => {
-      await router.initialize(ethers.constants.AddressZero);
-      await expect(
-        router.initialize(ethers.constants.AddressZero),
-      ).to.be.revertedWith('Initializable: contract is already initialized');
+      await router.initialize(mailbox.address);
+      await expect(router.initialize(mailbox.address)).to.be.revertedWith(
+        'Initializable: contract is already initialized',
+      );
     });
   });
 
@@ -110,6 +110,21 @@ describe('Router', async () => {
       expect(await router.mustHaveRemoteRouter(origin)).to.equal(remoteBytes);
     });
 
+    it('owner can enroll remote router using batch function', async () => {
+      const remote = nonOwner.address;
+      const remoteBytes = utils.addressToBytes32(nonOwner.address);
+      expect(await router.isRemoteRouter(origin, remoteBytes)).to.equal(false);
+      await expect(router.mustHaveRemoteRouter(origin)).to.be.revertedWith(
+        `No router enrolled for domain. Did you specify the right domain ID?`,
+      );
+      await router.enrollRemoteRouters(
+        [origin],
+        [utils.addressToBytes32(remote)],
+      );
+      expect(await router.isRemoteRouter(origin, remoteBytes)).to.equal(true);
+      expect(await router.mustHaveRemoteRouter(origin)).to.equal(remoteBytes);
+    });
+
     it('non-owner cannot enroll remote router', async () => {
       await expect(
         router

typescript/sdk/src/core/HyperlaneCore.ts

@@ -80,6 +80,7 @@ export class HyperlaneCore<
     return {
       mailbox: contracts.mailbox.address,
       interchainGasPaymaster: contracts.interchainGasPaymaster.address,
+      interchainSecurityModule: contracts.multisigIsm.address, // default ism
     };
   }
 

typescript/sdk/src/deploy/router/HyperlaneRouterDeployer.ts

@@ -4,6 +4,7 @@ import { ethers } from 'ethers';
 import { utils } from '@hyperlane-xyz/utils';
 
 import { chainMetadata } from '../../consts/chainMetadata';
+import { DomainIdToChainName } from '../../domains';
 import { MultiProvider } from '../../providers/MultiProvider';
 import { RouterContracts, RouterFactories } from '../../router';
 import { ChainMap, ChainName } from '../../types';
@@ -72,34 +73,44 @@ export abstract class HyperlaneRouterDeployer<
     );
     // Make all routers aware of each other.
     const deployedChains = Object.keys(contractsMap);
-    await promiseObjAll(
-      objMap(contractsMap, async (local, contracts) => {
-        const chainConnection = this.multiProvider.getChainConnection(local);
-        // only enroll chains which are deployed
-        const enrollChains = this.multiProvider
-          .remoteChains(local)
-          .filter((c) => deployedChains.includes(c));
-        for (const remote of enrollChains) {
+    for (const [chain, contracts] of Object.entries<RouterContracts>(
+      contractsMap,
+    )) {
+      const local = chain as Chain;
+      const chainConnection = this.multiProvider.getChainConnection(local);
+      // only enroll chains which are deployed
+      const deployedRemoteChains = this.multiProvider
+        .remoteChains(local)
+        .filter((c) => deployedChains.includes(c));
+
+      const enrollEntries = await Promise.all(
+        deployedRemoteChains.map(async (remote) => {
           const remoteDomain = chainMetadata[remote].id;
           const current = await contracts.router.routers(remoteDomain);
           const expected = utils.addressToBytes32(
             contractsMap[remote].router.address,
           );
-          if (current !== expected) {
-            await super.runIfOwner(local, contracts.router, async () => {
-              this.logger(`Enroll ${remote}'s router on ${local}`);
-              await chainConnection.handleTx(
-                contracts.router.enrollRemoteRouter(
-                  chainMetadata[remote].id,
-                  expected,
-                  chainConnection.overrides,
-                ),
-              );
-            });
-          }
-        }
-      }),
-    );
+          return current !== expected ? [remoteDomain, expected] : undefined;
+        }),
+      );
+      const entries = enrollEntries.filter(
+        (entry): entry is [number, string] => entry !== undefined,
+      );
+      const domains = entries.map(([id]) => id);
+      const addresses = entries.map(([, address]) => address);
+
+      await super.runIfOwner(local, contracts.router, async () => {
+        const chains = domains.map((id) => DomainIdToChainName[id]);
+        this.logger(`Enroll remote (${chains}) routers on ${local}`);
+        await chainConnection.handleTx(
+          contracts.router.enrollRemoteRouters(
+            domains,
+            addresses,
+            chainConnection.overrides,
+          ),
+        );
+      });
+    }
   }
 
   async transferOwnership(

typescript/sdk/src/router.ts

@@ -23,6 +23,7 @@ export type RouterFactories<RouterContract extends Router = Router> =
 export type ConnectionClientConfig = {
   mailbox: types.Address;
   interchainGasPaymaster: types.Address;
+  interchainSecurityModule?: types.Address;
 };
 
 export { Router } from '@hyperlane-xyz/core';