fix:enable DefaultFallbackRoutingIsm through non-factory deployment (…

…#3009) ### Description - `DefaultFallbackRoutingIsm` needs the mailbox address which meant `DefaultFallbackRoutingIsmFactory` needed the mailbox address which is not ideal. So, I switched to non-factory deployments for this specific ISM type. - CLI does the ISM deployment inside core deployment instead of ISM first and core then. - Using the coreDeployer's `cachedAddresses` to store the latest ie toplevel ISM. ### Drive-by changes - fixed bug in `ismFactory.fromAddressMap` to not use the multiprovider filtered by contract map. This is undesirable as CLI user can provide chain selection and artifacts which doesn't include all the chains in the configs (and when we call `multiprovider.getDomainId` for the origin which may now be missing from the filtered MP, we get an error). ### Related issues - fixes #2895 ### Backward compatibility Yes ### Testing Manual through CLI
hyperlane-xyz · Dec 5, 2023 · e06fe0b · e06fe0b
1 parent 8b16ade
commit e06fe0b
Show file tree

Hide file tree

Showing 20 changed files with 301 additions and 161 deletions.
diff --git a/.changeset/large-guests-jump.md b/.changeset/large-guests-jump.md
@@ -0,0 +1,8 @@
+---
+'@hyperlane-xyz/infra': patch
+'@hyperlane-xyz/cli': patch
+'@hyperlane-xyz/sdk': patch
+'@hyperlane-xyz/core': patch
+---
+
+Supporting DefaultFallbackRoutingIsm through non-factory deployments
diff --git a/solidity/contracts/isms/routing/DomainRoutingIsmFactory.sol b/solidity/contracts/isms/routing/DomainRoutingIsmFactory.sol
@@ -18,18 +18,20 @@ abstract contract AbstractDomainRoutingIsmFactory {
 
     /**
      * @notice Deploys and initializes a DomainRoutingIsm using a minimal proxy
+     * @param _owner The owner to set on the ISM
      * @param _domains The origin domains
      * @param _modules The ISMs to use to verify messages
      */
     function deploy(
+        address _owner,
         uint32[] calldata _domains,
         IInterchainSecurityModule[] calldata _modules
     ) external returns (DomainRoutingIsm) {
         DomainRoutingIsm _ism = DomainRoutingIsm(
             MinimalProxy.create(implementation())
         );
         emit ModuleDeployed(_ism);
-        _ism.initialize(msg.sender, _domains, _modules);
+        _ism.initialize(_owner, _domains, _modules);
         return _ism;
     }
 
@@ -51,19 +53,3 @@ contract DomainRoutingIsmFactory is AbstractDomainRoutingIsmFactory {
         return _implementation;
     }
 }
-
-/**
- * @title DefaultFallbackRoutingIsmFactory
- */
-contract DefaultFallbackRoutingIsmFactory is AbstractDomainRoutingIsmFactory {
-    // ============ Immutables ============
-    address internal immutable _implementation;
-
-    constructor(address mailbox) {
-        _implementation = address(new DefaultFallbackRoutingIsm(mailbox));
-    }
-
-    function implementation() public view override returns (address) {
-        return _implementation;
-    }
-}
diff --git a/solidity/test/isms/DomainRoutingIsm.t.sol b/solidity/test/isms/DomainRoutingIsm.t.sol
@@ -5,7 +5,7 @@ import "forge-std/Test.sol";
 
 import {DomainRoutingIsm} from "../../contracts/isms/routing/DomainRoutingIsm.sol";
 import {DefaultFallbackRoutingIsm} from "../../contracts/isms/routing/DefaultFallbackRoutingIsm.sol";
-import {DefaultFallbackRoutingIsmFactory, DomainRoutingIsmFactory} from "../../contracts/isms/routing/DomainRoutingIsmFactory.sol";
+import {DomainRoutingIsmFactory} from "../../contracts/isms/routing/DomainRoutingIsmFactory.sol";
 import {IInterchainSecurityModule} from "../../contracts/interfaces/IInterchainSecurityModule.sol";
 import {MessageUtils, TestIsm} from "./IsmTestUtils.sol";
 import {TestMailbox} from "../../contracts/test/TestMailbox.sol";

diff --git a/typescript/cli/examples/ism-advanced.yaml b/typescript/cli/examples/ism-advanced.yaml
@@ -1,5 +1,5 @@
 anvil1:
-  type: domainRoutingIsm
+  type: defaultFallbackRoutingIsm
   owner: '0xa0ee7a142d267c1f36714e4a8f75612f20a79720'
   domains:
     anvil2:

diff --git a/typescript/cli/src/config/ism.ts b/typescript/cli/src/config/ism.ts
@@ -1,7 +1,7 @@
 import { confirm, input, select } from '@inquirer/prompts';
 import { z } from 'zod';
 
-import { ChainMap, ChainName, IsmType } from '@hyperlane-xyz/sdk';
+import { ChainMap, ChainName, IsmType, ZHash } from '@hyperlane-xyz/sdk';
 
 import { errorRed, log, logBlue, logGreen } from '../../logger.js';
 import { runMultiChainSelectionStep } from '../utils/chains.js';
@@ -15,13 +15,16 @@ const MultisigIsmConfigSchema = z.object({
     z.literal(IsmType.MESSAGE_ID_MULTISIG),
   ]),
   threshold: z.number(),
-  validators: z.array(z.string()),
+  validators: z.array(ZHash),
 });
 
 const RoutingIsmConfigSchema: z.ZodSchema<any> = z.lazy(() =>
   z.object({
-    type: z.literal(IsmType.ROUTING),
-    owner: z.string(),
+    type: z.union([
+      z.literal(IsmType.ROUTING),
+      z.literal(IsmType.FALLBACK_ROUTING),
+    ]),
+    owner: ZHash,
     domains: z.record(IsmConfigSchema),
   }),
 );
@@ -160,6 +163,12 @@ export async function createIsmConfig(
         description:
           'Each origin chain can be verified by the specified ISM type via RoutingISM',
       },
+      {
+        value: IsmType.FALLBACK_ROUTING,
+        name: IsmType.FALLBACK_ROUTING,
+        description:
+          "You can specify ISM type for specific chains you like and fallback to mailbox's default ISM for other chains via DefaultFallbackRoutingISM",
+      },
       {
         value: IsmType.AGGREGATION,
         name: IsmType.AGGREGATION,
@@ -180,8 +189,11 @@ export async function createIsmConfig(
     moduleType === IsmType.MERKLE_ROOT_MULTISIG
   ) {
     lastConfig = await createMultisigConfig(moduleType);
-  } else if (moduleType === IsmType.ROUTING) {
-    lastConfig = await createRoutingConfig(remote, origins);
+  } else if (
+    moduleType === IsmType.ROUTING ||
+    moduleType === IsmType.FALLBACK_ROUTING
+  ) {
+    lastConfig = await createRoutingConfig(moduleType, remote, origins);
   } else if (moduleType === IsmType.AGGREGATION) {
     lastConfig = await createAggregationConfig(remote, origins);
   } else if (moduleType === IsmType.TEST_ISM) {
@@ -241,6 +253,7 @@ export async function createAggregationConfig(
 }
 
 export async function createRoutingConfig(
+  type: IsmType.ROUTING | IsmType.FALLBACK_ROUTING,
   remote: ChainName,
   chains: ChainName[],
 ): Promise<ZodIsmConfig> {
@@ -259,7 +272,7 @@ export async function createRoutingConfig(
     domainsMap[chain] = config;
   }
   return {
-    type: IsmType.ROUTING,
+    type,
     owner: ownerAddress,
     domains: domainsMap,
   };

diff --git a/typescript/cli/src/config/multisig.ts b/typescript/cli/src/config/multisig.ts
@@ -1,7 +1,7 @@
 import { confirm, input } from '@inquirer/prompts';
 import { z } from 'zod';
 
-import { ChainMap, MultisigConfig } from '@hyperlane-xyz/sdk';
+import { ChainMap, MultisigConfig, ZHash } from '@hyperlane-xyz/sdk';
 import {
   Address,
   isValidAddress,
@@ -19,7 +19,7 @@ import { readChainConfigsIfExists } from './chain.js';
 const MultisigConfigMapSchema = z.object({}).catchall(
   z.object({
     threshold: z.number(),
-    validators: z.array(z.string()),
+    validators: z.array(ZHash),
   }),
 );
 export type MultisigConfigMap = z.infer<typeof MultisigConfigMapSchema>;

diff --git a/typescript/cli/src/config/warp.ts b/typescript/cli/src/config/warp.ts
@@ -2,7 +2,7 @@ import { confirm, input } from '@inquirer/prompts';
 import { ethers } from 'ethers';
 import { z } from 'zod';
 
-import { TokenType } from '@hyperlane-xyz/sdk';
+import { TokenType, ZHash } from '@hyperlane-xyz/sdk';
 
 import { errorRed, logBlue, logGreen } from '../../logger.js';
 import {
@@ -14,17 +14,17 @@ import { FileFormat, readYamlOrJson, writeYamlOrJson } from '../utils/files.js';
 import { readChainConfigsIfExists } from './chain.js';
 
 const ConnectionConfigSchema = {
-  mailbox: z.string().optional(),
-  interchainGasPaymaster: z.string().optional(),
-  interchainSecurityModule: z.string().optional(),
+  mailbox: ZHash.optional(),
+  interchainGasPaymaster: ZHash.optional(),
+  interchainSecurityModule: ZHash.optional(),
   foreignDeployment: z.string().optional(),
 };
 
 export const WarpRouteConfigSchema = z.object({
   base: z.object({
     type: z.literal(TokenType.native).or(z.literal(TokenType.collateral)),
     chainName: z.string(),
-    address: z.string().optional(),
+    address: ZHash.optional(),
     isNft: z.boolean().optional(),
     name: z.string().optional(),
     symbol: z.string().optional(),

diff --git a/typescript/cli/src/deploy/core.ts b/typescript/cli/src/deploy/core.ts
@@ -279,24 +279,15 @@ async function executeDeploy({
     mergedContractAddrs,
     multiProvider,
   );
-
-  // 3. Deploy ISM contracts to remote deployable chains
-  logBlue('Deploying ISMs');
+  // 3. Construct ISM configs for all deployable chains
   const ismContracts: ChainMap<{ interchainSecurityModule: DeployedIsm }> = {};
-  const defaultIsms: ChainMap<Address> = {};
+  const defaultIsms: ChainMap<IsmConfig> = {};
   for (const ismOrigin of chains) {
-    logBlue(`Deploying ISM to ${ismOrigin}`);
-    const ismConfig =
+    defaultIsms[ismOrigin] =
       ismConfigs[ismOrigin] ??
       buildIsmConfig(owner, ismOrigin, chains, multisigConfigs);
-    ismContracts[ismOrigin] = {
-      interchainSecurityModule: await ismFactory.deploy(ismOrigin, ismConfig),
-    };
-    defaultIsms[ismOrigin] =
-      ismContracts[ismOrigin].interchainSecurityModule.address;
   }
   artifacts = writeMergedAddresses(contractsFilePath, artifacts, ismContracts);
-  logGreen('ISM contracts deployed');
 
   // 4. Deploy core contracts to chains
   logBlue(`Deploying core contracts to ${chains.join(', ')}`);
@@ -310,6 +301,16 @@ async function executeDeploy({
     multisigConfigs,
   );
   const coreContracts = await coreDeployer.deploy(coreConfigs);
+
+  // 4.5 recover the toplevel ISM address
+  const isms: HyperlaneAddressesMap<any> = {};
+  for (const chain of chains) {
+    isms[chain] = {
+      interchainSecurityModule:
+        coreDeployer.cachedAddresses[chain].interchainSecurityModule,
+    };
+  }
+  artifacts = objMerge(artifacts, isms);
   artifacts = writeMergedAddresses(contractsFilePath, artifacts, coreContracts);
   logGreen('Core contracts deployed');
 
@@ -358,7 +359,7 @@ function buildIsmConfig(
 function buildCoreConfigMap(
   owner: Address,
   chains: ChainName[],
-  defaultIsms: ChainMap<Address>,
+  defaultIsms: ChainMap<IsmConfig>,
   hooksConfig: ChainMap<HooksConfig>,
   multisigConfigs: ChainMap<MultisigConfig>,
 ): ChainMap<CoreConfig> {
@@ -381,7 +382,7 @@ function buildCoreConfigMap(
   }, {});
 }
 
-function buildTestRecipientConfigMap(
+export function buildTestRecipientConfigMap(
   chains: ChainName[],
   addressesMap: HyperlaneAddressesMap<any>,
 ): ChainMap<TestRecipientConfig> {

diff --git a/typescript/cli/src/tests/ism.test.ts b/typescript/cli/src/tests/ism.test.ts
@@ -10,7 +10,7 @@ describe('readIsmConfig', () => {
 
     const exampleIsmConfig: ChainMap<IsmConfig> = {
       anvil1: {
-        type: IsmType.ROUTING,
+        type: IsmType.FALLBACK_ROUTING,
         owner: '0xa0ee7a142d267c1f36714e4a8f75612f20a79720',
         domains: {
           anvil2: {

diff --git a/typescript/cli/src/tests/multisig.test.ts b/typescript/cli/src/tests/multisig.test.ts
@@ -36,6 +36,6 @@ describe('readMultisigConfig', () => {
   it('invalid address', () => {
     expect(function () {
       readMultisigConfig('src/tests/multisig/invalid-address-fail.yaml');
-    }).to.throw('Invalid address 0xa0ee7a142d267c1n36714e4a8f7561f20a79720');
+    }).to.throw('Invalid multisig config: anvil2,validators,0 => Invalid');
   });
 });
diff --git a/typescript/infra/src/govern/HyperlaneCoreGovernor.ts b/typescript/infra/src/govern/HyperlaneCoreGovernor.ts
@@ -29,10 +29,10 @@ export class HyperlaneCoreGovernor extends HyperlaneAppGovernor<
       case MailboxViolationType.DefaultIsm: {
         let ismAddress: string;
         if (typeof violation.expected === 'object') {
-          const ism = await this.checker.ismFactory.deploy(
-            violation.chain,
-            violation.expected,
-          );
+          const ism = await this.checker.ismFactory.deploy({
+            destination: violation.chain,
+            config: violation.expected,
+          });
           ismAddress = ism.address;
         } else if (typeof violation.expected === 'string') {
           ismAddress = violation.expected;

diff --git a/typescript/sdk/hardhat.config.ts b/typescript/sdk/hardhat.config.ts
@@ -7,7 +7,7 @@ import '@typechain/hardhat';
  */
 module.exports = {
   solidity: {
-    version: '0.7.6',
+    version: '0.8.19',
     settings: {
       optimizer: {
         enabled: true,

diff --git a/typescript/sdk/src/core/HyperlaneCoreDeployer.ts b/typescript/sdk/src/core/HyperlaneCoreDeployer.ts
@@ -22,7 +22,6 @@ export class HyperlaneCoreDeployer extends HyperlaneDeployer<
   CoreConfig,
   CoreFactories
 > {
-  startingBlockNumbers: ChainMap<number | undefined> = {};
   hookDeployer: HyperlaneHookDeployer;
 
   constructor(
@@ -69,8 +68,13 @@ export class HyperlaneCoreDeployer extends HyperlaneDeployer<
     );
     if (!matches) {
       this.logger('Deploying default ISM');
-      defaultIsm = await this.deployIsm(chain, config.defaultIsm);
+      defaultIsm = await this.deployIsm(
+        chain,
+        config.defaultIsm,
+        mailbox.address,
+      );
     }
+    this.cachedAddresses[chain].interchainSecurityModule = defaultIsm;
 
     const hookAddresses = { mailbox: mailbox.address, proxyAdmin };
 
@@ -168,8 +172,16 @@ export class HyperlaneCoreDeployer extends HyperlaneDeployer<
     return hooks[config.type].address;
   }
 
-  async deployIsm(chain: ChainName, config: IsmConfig): Promise<Address> {
-    const ism = await this.ismFactory.deploy(chain, config);
+  async deployIsm(
+    chain: ChainName,
+    config: IsmConfig,
+    mailbox: Address,
+  ): Promise<Address> {
+    const ism = await this.ismFactory.deploy({
+      destination: chain,
+      config,
+      mailbox,
+    });
     this.addDeployedContracts(chain, this.ismFactory.deployedIsms[chain]);
     return ism.address;
   }
@@ -187,10 +199,6 @@ export class HyperlaneCoreDeployer extends HyperlaneDeployer<
 
     const mailbox = await this.deployMailbox(chain, config, proxyAdmin.address);
 
-    // TODO: remove once agents fetch deployedBlock from mailbox
-    const deployedBlock = await mailbox.deployedBlock();
-    this.startingBlockNumbers[chain] = deployedBlock.toNumber();
-
     const validatorAnnounce = await this.deployValidatorAnnounce(
       chain,
       mailbox.address,

diff --git a/typescript/sdk/src/deploy/HyperlaneDeployer.ts b/typescript/sdk/src/deploy/HyperlaneDeployer.ts
@@ -220,7 +220,8 @@ export abstract class HyperlaneDeployer<
         this.multiProvider,
         ismFactory.getContracts(chain),
       );
-      targetIsm = (await ismFactory.deploy(chain, config)).address;
+      targetIsm = (await ismFactory.deploy({ destination: chain, config }))
+        .address;
     }
     if (!matches) {
       await this.runIfOwner(chain, contract, async () => {
@@ -269,7 +270,6 @@ export abstract class HyperlaneDeployer<
     config: MailboxClientConfig,
   ): Promise<void> {
     this.logger(`Initializing mailbox client (if not already) on ${local}...`);
-    this.logger(`MailboxClient Config: ${JSON.stringify(config)}`);
     if (config.hook) {
       await this.configureHook(
         local,

diff --git a/typescript/sdk/src/deploy/contracts.ts b/typescript/sdk/src/deploy/contracts.ts
@@ -13,9 +13,6 @@ export const proxyFactoryFactories = {
   aggregationIsmFactory: new StaticAggregationIsmFactory__factory(),
   aggregationHookFactory: new StaticAggregationHookFactory__factory(),
   routingIsmFactory: new DomainRoutingIsmFactory__factory(),
-  // TODO: https://github.com/hyperlane-xyz/hyperlane-monorepo/issues/2895
-  // defaultFallbackRoutingIsmFactory:
-  //   new DefaultFallbackRoutingIsmFactory__factory(),
 };
 
 export type ProxyFactoryFactories = typeof proxyFactoryFactories;
@@ -29,5 +26,4 @@ export const proxyFactoryImplementations: ProxyFactoryImplementations = {
   aggregationIsmFactory: 'StaticAggregationIsm',
   aggregationHookFactory: 'StaticAggregationHook',
   routingIsmFactory: 'DomaingRoutingIsm',
-  // defaultFallbackRoutingIsmFactory: 'DefaultFallbackRoutingIsm',
 };
diff --git a/typescript/sdk/src/hook/HyperlaneHookDeployer.ts b/typescript/sdk/src/hook/HyperlaneHookDeployer.ts
@@ -182,11 +182,11 @@ export class HyperlaneHookDeployer extends HyperlaneDeployer<
       origin: chain,
       nativeBridge: l2Messenger,
     };
-    const opstackIsm = (await this.ismFactory.deploy(
-      config.destinationChain,
-      ismConfig,
-      chain,
-    )) as OPStackIsm;
+    const opstackIsm = (await this.ismFactory.deploy({
+      destination: config.destinationChain,
+      config: ismConfig,
+      origin: chain,
+    })) as OPStackIsm;
     // deploy opstack hook
     const hook = await this.deployContract(chain, HookType.OP_STACK, [
       mailbox,