Support hook rotation in core deployer (#2834)

solidity/contracts/mock/MockHyperlaneEnvironment.sol

@@ -3,7 +3,7 @@ pragma solidity ^0.8.13;
 
 import "./MockMailbox.sol";
 import "../test/TestInterchainGasPaymaster.sol";
-import "../test/TestMultisigIsm.sol";
+import "../test/TestIsm.sol";
 
 import {TypeCasts} from "../libs/TypeCasts.sol";
 
@@ -25,19 +25,15 @@ contract MockHyperlaneEnvironment {
         originMailbox.addRemoteMailbox(_destinationDomain, destinationMailbox);
         destinationMailbox.addRemoteMailbox(_originDomain, originMailbox);
 
-        isms[originDomain] = new TestMultisigIsm();
-        isms[destinationDomain] = new TestMultisigIsm();
+        isms[originDomain] = new TestIsm();
+        isms[destinationDomain] = new TestIsm();
 
         originMailbox.setDefaultIsm(address(isms[originDomain]));
         destinationMailbox.setDefaultIsm(address(isms[destinationDomain]));
 
         igps[originDomain] = new TestInterchainGasPaymaster();
         igps[destinationDomain] = new TestInterchainGasPaymaster();
 
-        // TODO: update routers with IGP paymentss
-        // originMailbox.setDefaultHook(address(igps[originDomain]));
-        // destinationMailbox.setDefaultHook(address(igps[destinationDomain]));
-
         originMailbox.transferOwnership(msg.sender);
         destinationMailbox.transferOwnership(msg.sender);
 

solidity/test/isms/OPStackIsm.t.sol

@@ -11,7 +11,6 @@ import {AbstractMessageIdAuthorizedIsm} from "../../contracts/isms/hook/Abstract
 import {TestMailbox} from "../../contracts/test/TestMailbox.sol";
 import {Message} from "../../contracts/libs/Message.sol";
 import {MessageUtils} from "./IsmTestUtils.sol";
-import {TestMultisigIsm} from "../../contracts/test/TestMultisigIsm.sol";
 import {OPStackIsm} from "../../contracts/isms/hook/OPStackIsm.sol";
 import {OPStackHook} from "../../contracts/hooks/OPStackHook.sol";
 import {TestRecipient} from "../../contracts/test/TestRecipient.sol";

typescript/sdk/src/core/CoreDeployer.hardhat-test.ts

@@ -3,10 +3,13 @@ import { assert, expect } from 'chai';
 import { ethers } from 'hardhat';
 import sinon from 'sinon';
 
+import { objMap, promiseObjAll } from '@hyperlane-xyz/utils';
+
 import { TestChains } from '../consts/chains';
 import { HyperlaneContractsMap } from '../contracts/types';
 import { HyperlaneProxyFactoryDeployer } from '../deploy/HyperlaneProxyFactoryDeployer';
 import { HyperlaneIsmFactory } from '../ism/HyperlaneIsmFactory';
+import { AggregationIsmConfig, ModuleType } from '../ism/types';
 import { MultiProvider } from '../providers/MultiProvider';
 import { testCoreConfig } from '../test/testUtils';
 import { ChainMap } from '../types';
@@ -24,29 +27,97 @@ describe('core', async () => {
   let contracts: HyperlaneContractsMap<CoreFactories>;
   let coreConfig: ChainMap<CoreConfig>;
   let ismFactory: HyperlaneIsmFactory;
+
   before(async () => {
     const [signer] = await ethers.getSigners();
     multiProvider = MultiProvider.createTestMultiProvider({ signer });
     const proxyFactoryDeployer = new HyperlaneProxyFactoryDeployer(
       multiProvider,
     );
-    coreConfig = testCoreConfig(TestChains);
+    coreConfig = testCoreConfig(TestChains, signer.address);
     const ismFactories = await proxyFactoryDeployer.deploy(coreConfig);
     ismFactory = new HyperlaneIsmFactory(ismFactories, multiProvider);
-  });
-
-  beforeEach(async () => {
-    const [signer] = await ethers.getSigners();
-    // This is kind of awkward and really these tests shouldn't live here
-    multiProvider = MultiProvider.createTestMultiProvider({ signer });
+    deployer = new HyperlaneCoreDeployer(multiProvider, ismFactory);
   });
 
   it('deploys', async () => {
-    deployer = new HyperlaneCoreDeployer(multiProvider, ismFactory);
     contracts = await deployer.deploy(coreConfig);
     core = new HyperlaneCore(contracts, multiProvider);
   });
 
+  describe('idempotency', () => {
+    beforeEach(async () => {
+      contracts = await deployer.deploy(coreConfig);
+    });
+
+    it('rotates default and required hooks and recovers artifacts', async () => {
+      const getHooks = async (
+        contracts: HyperlaneContractsMap<CoreFactories>,
+      ) =>
+        promiseObjAll(
+          objMap(contracts, async (_, { mailbox }) => ({
+            default: await mailbox.defaultHook(),
+            required: await mailbox.requiredHook(),
+          })),
+        );
+
+      const hooksBefore = await getHooks(contracts);
+
+      const updatedConfig = objMap(coreConfig, (_, config) => ({
+        ...config,
+        defaultHook: config.requiredHook,
+        requiredHook: config.defaultHook,
+      }));
+
+      const [signer] = await ethers.getSigners();
+      const nonceBefore = await signer.getTransactionCount();
+
+      const updatedContracts = await deployer.deploy(updatedConfig);
+
+      const hooksAfter = await getHooks(updatedContracts);
+      expect(hooksBefore).to.deep.equal(
+        objMap(hooksAfter, (_, res) => ({
+          required: res.default,
+          default: res.required,
+        })),
+      );
+
+      // number of set hook transactions
+      const numTransactions = 2 * TestChains.length;
+      const nonceAfter = await signer.getTransactionCount();
+      expect(nonceAfter).to.equal(nonceBefore + numTransactions);
+    });
+
+    it('rotates default ISMs', async () => {
+      const testIsm = await contracts.test1.mailbox.defaultIsm();
+
+      const updatedConfig: ChainMap<CoreConfig> = objMap(
+        coreConfig,
+        (_, config) => {
+          const ismConfig: AggregationIsmConfig = {
+            type: ModuleType.AGGREGATION,
+            modules: [testIsm, testIsm],
+            threshold: 2,
+          };
+          return {
+            ...config,
+            defaultIsm: ismConfig,
+          };
+        },
+      );
+
+      const [signer] = await ethers.getSigners();
+      const nonceBefore = await signer.getTransactionCount();
+
+      await deployer.deploy(updatedConfig);
+
+      // one aggregation ISM deploy and one set ISM transaction per chain
+      const numTransactions = 2 * TestChains.length;
+      const nonceAfter = await signer.getTransactionCount();
+      expect(nonceAfter).to.equal(nonceBefore + numTransactions);
+    });
+  });
+
   describe('failure modes', async () => {
     beforeEach(async () => {
       deployer = new HyperlaneCoreDeployer(multiProvider, ismFactory);

typescript/sdk/src/core/HyperlaneCoreDeployer.ts

@@ -32,6 +32,7 @@ export class HyperlaneCoreDeployer extends HyperlaneDeployer<
     super(multiProvider, coreFactories, {
       logger: debug('hyperlane:CoreDeployer'),
       chainTimeoutMs: 1000 * 60 * 10, // 10 minutes
+      ismFactory,
     });
     this.hookDeployer = new HyperlaneHookDeployer(
       multiProvider,
@@ -73,11 +74,14 @@ export class HyperlaneCoreDeployer extends HyperlaneDeployer<
 
     const hookAddresses = { mailbox: mailbox.address, proxyAdmin };
 
+    this.logger('Deploying default hook');
     const defaultHook = await this.deployHook(
       chain,
       config.defaultHook,
       hookAddresses,
     );
+
+    this.logger('Deploying required hook');
     const requiredHook = await this.deployHook(
       chain,
       config.requiredHook,
@@ -93,9 +97,35 @@ export class HyperlaneCoreDeployer extends HyperlaneDeployer<
     } catch (e: any) {
       if (!e.message.includes('already initialized')) {
         throw e;
-      } else {
-        this.logger('Mailbox already initialized');
       }
+
+      this.logger('Mailbox already initialized');
+
+      await this.configureHook(
+        chain,
+        mailbox,
+        defaultHook,
+        (_mailbox) => _mailbox.defaultHook(),
+        (_mailbox, _hook) => _mailbox.populateTransaction.setDefaultHook(_hook),
+      );
+
+      await this.configureHook(
+        chain,
+        mailbox,
+        requiredHook,
+        (_mailbox) => _mailbox.requiredHook(),
+        (_mailbox, _hook) =>
+          _mailbox.populateTransaction.setRequiredHook(_hook),
+      );
+
+      await this.configureIsm(
+        chain,
+        mailbox,
+        defaultIsm,
+        (_mailbox) => _mailbox.defaultIsm(),
+        (_mailbox, _module) =>
+          _mailbox.populateTransaction.setDefaultIsm(_module),
+      );
     }
 
     return mailbox;