fix:enable DefaultFallbackRoutingIsm through non-factory deployment

.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

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;
-    }
-}

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";

typescript/cli/examples/ism-advanced.yaml

@@ -1,5 +1,5 @@
 anvil1:
-  type: domainRoutingIsm
+  type: defaultFallbackRoutingIsm
   owner: '0xa0ee7a142d267c1f36714e4a8f75612f20a79720'
   domains:
     anvil2:

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),
   }),
 );
@@ -156,6 +159,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,
@@ -176,8 +185,11 @@ export async function createIsmConfig(
     moduleType === IsmType.MERKLE_ROOT_MULTISIG
   ) {
     lastConfig = await createMultisigConfig(moduleType);
-  } else if (moduleType === IsmType.ROUTING) {
-    lastConfig = await createRoutingConfig(chain, chainConfigPath);
+  } else if (
+    moduleType === IsmType.ROUTING ||
+    moduleType === IsmType.FALLBACK_ROUTING
+  ) {
+    lastConfig = await createRoutingConfig(moduleType, chain, chainConfigPath);
   } else if (moduleType === IsmType.AGGREGATION) {
     lastConfig = await createAggregationConfig(chain, chainConfigPath);
   } else if (moduleType === IsmType.TEST_ISM) {
@@ -237,6 +249,7 @@ export async function createAggregationConfig(
 }
 
 export async function createRoutingConfig(
+  type: IsmType.ROUTING | IsmType.FALLBACK_ROUTING,
   chain: ChainName,
   chainConfigPath: string,
 ): Promise<ZodIsmConfig> {
@@ -261,7 +274,7 @@ export async function createRoutingConfig(
     domainsMap[chain] = config;
   }
   return {
-    type: IsmType.ROUTING,
+    type,
     owner: ownerAddress,
     domains: domainsMap,
   };

typescript/cli/src/config/multisig.ts

@@ -1,7 +1,7 @@
 import { 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,
@@ -18,7 +18,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>;

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(),

typescript/cli/src/deploy/core.ts

@@ -214,7 +214,7 @@ async function runHookStep(
   log(`Found hook configs for chains: ${Object.keys(configs).join(', ')}`);
 }
 
-interface DeployParams {
+export interface DeployParams {
   chains: string[];
   signer: ethers.Signer;
   multiProvider: MultiProvider;
@@ -250,7 +250,7 @@ async function runDeployPlanStep({
   if (!isConfirmed) throw new Error('Deployment cancelled');
 }
 
-async function executeDeploy({
+export async function executeDeploy({
   chains,
   signer,
   multiProvider,
@@ -293,24 +293,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(', ')}`);
@@ -323,6 +314,16 @@ async function executeDeploy({
     multisigConfigs ?? defaultMultisigConfigs, // TODO: fix https://github.com/hyperlane-xyz/issues/issues/773
   );
   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');
 
@@ -348,6 +349,8 @@ async function executeDeploy({
   logBlue('Deployment is complete!');
   logBlue(`Contract address artifacts are in ${contractsFilePath}`);
   logBlue(`Agent configs are in ${agentFilePath}`);
+
+  return artifacts;
 }
 
 function buildIsmConfig(
@@ -371,7 +374,7 @@ function buildIsmConfig(
 function buildCoreConfigMap(
   owner: Address,
   chains: ChainName[],
-  defaultIsms: ChainMap<Address>,
+  defaultIsms: ChainMap<IsmConfig>,
   multisigConfig: ChainMap<MultisigConfig>,
 ): ChainMap<CoreConfig> {
   return chains.reduce<ChainMap<CoreConfig>>((config, chain) => {
@@ -403,7 +406,7 @@ function buildCoreConfigMap(
   }, {});
 }
 
-function buildTestRecipientConfigMap(
+export function buildTestRecipientConfigMap(
   chains: ChainName[],
   addressesMap: HyperlaneAddressesMap<any>,
 ): ChainMap<TestRecipientConfig> {

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: {

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');
   });
 });

typescript/cli/src/tests/routingIsm.test.ts

@@ -0,0 +1,49 @@
+import { expect } from 'chai';
+
+import { DomainRoutingIsm__factory } from '@hyperlane-xyz/core';
+import { IsmConfig, IsmType } from '@hyperlane-xyz/sdk';
+
+import { getContextWithSigner } from '../context.js';
+import { DeployParams, executeDeploy } from '../deploy/core.js';
+
+describe('readFallbackRoutingIsmConfig', () => {
+  let deployParams: DeployParams;
+  const key =
+    '0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80';
+  const { multiProvider, signer } = getContextWithSigner(
+    key,
+    './examples/anvil-chains.yaml',
+  );
+  const ismConfig: IsmConfig = {
+    type: IsmType.ROUTING,
+    owner: '0xa0ee7a142d267c1f36714e4a8f75612f20a79720',
+    domains: {
+      anvil2: {
+        type: IsmType.MESSAGE_ID_MULTISIG,
+        threshold: 1,
+        validators: ['0xa0ee7a142d267c1f36714e4a8f75612f20a79720'],
+      },
+    },
+  };
+
+  it('deploys and right module for the origin specified', async () => {
+    deployParams = {
+      chains: ['anvil1'],
+      signer,
+      multiProvider,
+      artifacts: {},
+      ismConfigs: { anvil1: ismConfig },
+      multisigConfigs: {},
+      outPath: '/tmp',
+      skipConfirmation: true,
+    };
+    const artifacts = await executeDeploy(deployParams);
+    const ism = DomainRoutingIsm__factory.connect(
+      artifacts.anvil1.interchainSecurityModule,
+      multiProvider.getSigner('anvil1'),
+    );
+
+    const actualIsm = await ism.module(multiProvider.getChainId('anvil2'));
+    expect(actualIsm).to.equal(artifacts.anvil2.messageIdMultisigIsm);
+  }).timeout(250000);
+});

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({
+            chain: violation.chain,
+            config: violation.expected,
+          });
           ismAddress = ism.address;
         } else if (typeof violation.expected === 'string') {
           ismAddress = violation.expected;