diff --git a/solidity/contracts/mock/MockHyperlaneEnvironment.sol b/solidity/contracts/mock/MockHyperlaneEnvironment.sol index e56325d1e..9f51b61ad 100644 --- a/solidity/contracts/mock/MockHyperlaneEnvironment.sol +++ b/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,8 +25,8 @@ 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])); @@ -34,10 +34,6 @@ contract MockHyperlaneEnvironment { 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); diff --git a/solidity/contracts/test/TestMultisigIsm.sol b/solidity/contracts/test/TestMultisigIsm.sol deleted file mode 100644 index 92e332496..000000000 --- a/solidity/contracts/test/TestMultisigIsm.sol +++ /dev/null @@ -1,39 +0,0 @@ -// SPDX-License-Identifier: MIT OR Apache-2.0 -pragma solidity >=0.8.0; - -import {IInterchainSecurityModule} from "../interfaces/IInterchainSecurityModule.sol"; -import {IMultisigIsm} from "../interfaces/isms/IMultisigIsm.sol"; - -contract TestMultisigIsm is IMultisigIsm { - // solhint-disable-next-line const-name-snakecase - uint8 public constant moduleType = - uint8(IInterchainSecurityModule.Types.MERKLE_ROOT_MULTISIG); - - bool public accept; - - constructor() { - accept = true; - } - - function validatorsAndThreshold(bytes calldata) - external - pure - returns (address[] memory, uint8) - { - address[] memory validators = new address[](1); - validators[0] = address(0); - return (validators, 1); - } - - function setAccept(bool _val) external { - accept = _val; - } - - function verify(bytes calldata, bytes calldata) - external - view - returns (bool) - { - return accept; - } -} diff --git a/solidity/test/isms/OPStackIsm.t.sol b/solidity/test/isms/OPStackIsm.t.sol index 0afc733da..9541c8aab 100644 --- a/solidity/test/isms/OPStackIsm.t.sol +++ b/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"; diff --git a/typescript/sdk/src/core/CoreDeployer.hardhat-test.ts b/typescript/sdk/src/core/CoreDeployer.hardhat-test.ts index 6cb17c072..c466cac29 100644 --- a/typescript/sdk/src/core/CoreDeployer.hardhat-test.ts +++ b/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; let coreConfig: ChainMap; 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, + ) => + 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 = 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); diff --git a/typescript/sdk/src/core/HyperlaneCoreDeployer.ts b/typescript/sdk/src/core/HyperlaneCoreDeployer.ts index 6e2432c7d..acb811b23 100644 --- a/typescript/sdk/src/core/HyperlaneCoreDeployer.ts +++ b/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; diff --git a/typescript/sdk/src/deploy/HyperlaneDeployer.ts b/typescript/sdk/src/deploy/HyperlaneDeployer.ts index 7e19acfc0..a438a71f3 100644 --- a/typescript/sdk/src/deploy/HyperlaneDeployer.ts +++ b/typescript/sdk/src/deploy/HyperlaneDeployer.ts @@ -1,10 +1,8 @@ import { Debugger, debug } from 'debug'; -import { Contract, ethers } from 'ethers'; +import { Contract, PopulatedTransaction, ethers } from 'ethers'; import { - Mailbox, MailboxClient, - Mailbox__factory, Ownable, ProxyAdmin, ProxyAdmin__factory, @@ -30,6 +28,7 @@ import { HyperlaneIsmFactory, moduleMatchesConfig, } from '../ism/HyperlaneIsmFactory'; +import { IsmConfig } from '../ism/types'; import { MultiProvider } from '../providers/MultiProvider'; import { MailboxClientConfig } from '../router/types'; import { ChainMap, ChainName } from '../types'; @@ -195,75 +194,90 @@ export abstract class HyperlaneDeployer< } } + protected async configureIsm( + chain: ChainName, + contract: C, + config: IsmConfig, + getIsm: (contract: C) => Promise
, + setIsm: (contract: C, ism: Address) => Promise, + ): Promise { + if (this.options?.ismFactory === undefined) { + throw new Error('No ISM factory provided'); + } + const ismFactory = this.options.ismFactory; + + const configuredIsm = await getIsm(contract); + const matches = await moduleMatchesConfig( + chain, + configuredIsm, + config, + this.multiProvider, + ismFactory.getContracts(chain), + ); + if (!matches) { + await this.runIfOwner(chain, contract, async () => { + const targetIsm = await ismFactory.deploy(chain, config); + this.logger(`Set ISM on ${chain}`); + await this.multiProvider.sendTransaction( + chain, + setIsm(contract, targetIsm.address), + ); + if (targetIsm.address !== (await getIsm(contract))) { + throw new Error(`Set ISM failed on ${chain}`); + } + }); + } + } + + protected async configureHook( + chain: ChainName, + contract: C, + targetHook: Address, + getHook: (contract: C) => Promise
, + setHook: (contract: C, hook: Address) => Promise, + ): Promise { + const configuredHook = await getHook(contract); + if (targetHook !== configuredHook) { + await this.runIfOwner(chain, contract, async () => { + this.logger(`Set hook on ${chain}`); + await this.multiProvider.sendTransaction( + chain, + setHook(contract, targetHook), + ); + if (targetHook !== (await getHook(contract))) { + throw new Error(`Set hook failed on ${chain}`); + } + }); + } + } + protected async initMailboxClient( local: ChainName, client: MailboxClient, config: MailboxClientConfig, ): Promise { this.logger(`Initializing mailbox client (if not already) on ${local}...`); - await this.runIfOwner(local, client, async () => { - const txOverrides = this.multiProvider.getTransactionOverrides(local); - - // set hook if not already set (and configured) - if (config.hook && config.hook !== (await client.hook())) { - this.logger(`Set hook on ${local}`); - await this.multiProvider.handleTx( - local, - client.setHook(config.hook, txOverrides), - ); - } - - let currentIsm = await client.interchainSecurityModule(); - // in case the above returns zero address, fetch the defaultISM from the mailbox - if (currentIsm === ethers.constants.AddressZero) { - const mailbox: Mailbox = Mailbox__factory.connect( - config.mailbox, - client.signer, - ); - currentIsm = await mailbox.defaultIsm(); - } - - // set interchain security module if not already set (and configured) - if (config.interchainSecurityModule) { - let configuredIsm: string; - if (typeof config.interchainSecurityModule === 'string') { - configuredIsm = config.interchainSecurityModule; - } else if (this.options?.ismFactory) { - const matches = await moduleMatchesConfig( - local, - currentIsm, - config.interchainSecurityModule, - this.multiProvider, - this.options.ismFactory.chainMap[local], - ); - if (matches) { - // when the ISM recursively matches the IsmConfig, we don't need to deploy a new ISM - this.logger( - `ISM matches config for chain ${local}, skipping deploy`, - ); - return; - } - const ism = await this.options.ismFactory.deploy( - local, - config.interchainSecurityModule, - ); - configuredIsm = ism.address; - } else { - throw new Error('No ISM factory provided'); - } + if (config.hook) { + await this.configureHook( + local, + client, + config.hook, + (_client) => _client.hook(), + (_client, _hook) => _client.populateTransaction.setHook(_hook), + ); + } - if (!eqAddress(currentIsm, configuredIsm)) { - this.logger( - `Set interchain security module on ${local} at ${configuredIsm}`, - ); + if (config.interchainSecurityModule) { + await this.configureIsm( + local, + client, + config.interchainSecurityModule, + (_client) => _client.interchainSecurityModule(), + (_client, _module) => + _client.populateTransaction.setInterchainSecurityModule(_module), + ); + } - await this.multiProvider.handleTx( - local, - client.setInterchainSecurityModule(configuredIsm, txOverrides), - ); - } - } - }); this.logger(`Connection client on ${local} initialized...`); } @@ -418,7 +432,6 @@ export abstract class HyperlaneDeployer< return implementation; } - this.logger(`Deploying transparent upgradable proxy`); const constructorArgs = proxyConstructorArgs( implementation, proxyAdmin, diff --git a/typescript/sdk/src/ism/HyperlaneIsmFactory.ts b/typescript/sdk/src/ism/HyperlaneIsmFactory.ts index f4117735f..4556f89c5 100644 --- a/typescript/sdk/src/ism/HyperlaneIsmFactory.ts +++ b/typescript/sdk/src/ism/HyperlaneIsmFactory.ts @@ -10,7 +10,7 @@ import { StaticAddressSetFactory, StaticAggregationIsm__factory, StaticThresholdAddressSetFactory, - TestMultisigIsm__factory, + TestIsm__factory, } from '@hyperlane-xyz/core'; import { Address, eqAddress, formatMessage, warn } from '@hyperlane-xyz/utils'; @@ -108,8 +108,13 @@ export class HyperlaneIsmFactory extends HyperlaneApp { } else if (config.type === ModuleType.AGGREGATION) { this.logger(`Deploying Aggregation ISM to ${chain}`); contract = await this.deployAggregationIsm(chain, config, origin); - } else if (config.type === ModuleType.TEST_ISM) { - contract = await this.deployTestIsm(chain); + } else if (config.type === ModuleType.NULL) { + this.logger(`Deploying Test ISM to ${chain}`); + contract = await this.multiProvider.handleDeploy( + chain, + new TestIsm__factory(), + [], + ); } else { throw new Error(`Unsupported ISM type`); } @@ -217,13 +222,6 @@ export class HyperlaneIsmFactory extends HyperlaneApp { return IAggregationIsm__factory.connect(address, signer); } - private async deployTestIsm(chain: ChainName) { - const signer = this.multiProvider.getSigner(chain); - const factory = new TestMultisigIsm__factory(signer); - const contract = await factory.deploy(); - return contract; - } - async deployStaticAddressSet( chain: ChainName, factory: StaticThresholdAddressSetFactory | StaticAddressSetFactory, @@ -365,7 +363,7 @@ export async function moduleCanCertainlyVerify( } return verified >= destModule.threshold; } - case ModuleType.TEST_ISM: { + case ModuleType.NULL: { return true; } } @@ -485,6 +483,11 @@ export async function moduleMatchesConfig( } break; } + case ModuleType.NULL: { + // This is just a TestISM + matches = true; + break; + } default: { throw new Error('Unsupported ModuleType'); } @@ -526,7 +529,7 @@ export function collectValidators( aggregatedValidators.forEach((set) => { validators = validators.concat([...set]); }); - } else if (config.type === ModuleType.TEST_ISM) { + } else if (config.type === ModuleType.NULL) { // This is just a TestISM return new Set([]); } else { diff --git a/typescript/sdk/src/ism/types.ts b/typescript/sdk/src/ism/types.ts index 9e9c64758..ed42fb83f 100644 --- a/typescript/sdk/src/ism/types.ts +++ b/typescript/sdk/src/ism/types.ts @@ -5,7 +5,7 @@ import { IRoutingIsm, StaticMerkleRootMultisigIsm, StaticMessageIdMultisigIsm, - TestMultisigIsm, + TestIsm, } from '@hyperlane-xyz/core'; import type { Address } from '@hyperlane-xyz/utils'; @@ -18,7 +18,7 @@ export type DeployedIsm = | IRoutingIsm | StaticMessageIdMultisigIsm | StaticMerkleRootMultisigIsm - | TestMultisigIsm; + | TestIsm; export enum ModuleType { UNUSED, @@ -27,7 +27,7 @@ export enum ModuleType { LEGACY_MULTISIG, // DEPRECATED MERKLE_ROOT_MULTISIG, MESSAGE_ID_MULTISIG, - TEST_ISM, + NULL, } export type MultisigConfig = { @@ -39,8 +39,8 @@ export type MultisigIsmConfig = MultisigConfig & { type: ModuleType.MERKLE_ROOT_MULTISIG | ModuleType.MESSAGE_ID_MULTISIG; }; -export type TestMultisigIsmConfig = MultisigConfig & { - type: ModuleType.TEST_ISM; +export type TestIsmConfig = { + type: ModuleType.NULL; }; export type RoutingIsmConfig = { @@ -60,4 +60,4 @@ export type IsmConfig = | RoutingIsmConfig | MultisigIsmConfig | AggregationIsmConfig - | TestMultisigIsmConfig; + | TestIsmConfig; diff --git a/typescript/sdk/src/providers/MultiProvider.ts b/typescript/sdk/src/providers/MultiProvider.ts index a7f26ea5b..bef14b999 100644 --- a/typescript/sdk/src/providers/MultiProvider.ts +++ b/typescript/sdk/src/providers/MultiProvider.ts @@ -369,9 +369,9 @@ export class MultiProvider extends ChainMetadataManager { */ async sendTransaction( chainNameOrId: ChainName | number, - tx: PopulatedTransaction, + tx: PopulatedTransaction | Promise, ): Promise { - const txReq = await this.prepareTx(chainNameOrId, tx); + const txReq = await this.prepareTx(chainNameOrId, await tx); const signer = this.getSigner(chainNameOrId); const response = await signer.sendTransaction(txReq); this.logger(`Sent tx ${response.hash}`); diff --git a/typescript/sdk/src/test/testUtils.ts b/typescript/sdk/src/test/testUtils.ts index f75b64d89..d99742978 100644 --- a/typescript/sdk/src/test/testUtils.ts +++ b/typescript/sdk/src/test/testUtils.ts @@ -14,7 +14,7 @@ import { CoinGeckoSimplePriceParams, } from '../gas/token-prices'; import { HookType } from '../hook/types'; -import { ModuleType, TestMultisigIsmConfig } from '../ism/types'; +import { ModuleType } from '../ism/types'; import { RouterConfig } from '../router/types'; import { ChainMap, ChainName } from '../types'; @@ -44,53 +44,28 @@ export function createRouterConfigMap( const nonZeroAddress = ethers.constants.AddressZero.replace('00', '01'); // dummy config as TestInbox and TestOutbox do not use deployed ISM -export function testCoreConfig(chains: ChainName[]): ChainMap { - const multisigIsm: TestMultisigIsmConfig = { - type: ModuleType.TEST_ISM, - validators: [nonZeroAddress], - threshold: 1, - }; - - const config: ChainMap = Object.fromEntries( - chains.map((local) => [ - local, - { - owner: nonZeroAddress, - defaultIsm: { - type: ModuleType.ROUTING, - owner: nonZeroAddress, - domains: Object.fromEntries( - chains - .filter((c) => c !== local) - .map((remote) => [remote, multisigIsm]), - ), - }, - defaultHook: { - type: HookType.MERKLE_TREE, - }, - requiredHook: { - type: HookType.PROTOCOL_FEE, - maxProtocolFee: ethers.utils.parseUnits('1', 'gwei'), // 1 gwei of native token - protocolFee: BigNumber.from(1), // 1 wei - beneficiary: nonZeroAddress, - owner: nonZeroAddress, - }, - }, - ]), - ); - - // test partial timelock config - config.test3.upgrade = { - timelock: { - delay: 100, - roles: { - executor: nonZeroAddress, - proposer: nonZeroAddress, - }, +export function testCoreConfig( + chains: ChainName[], + owner = nonZeroAddress, +): ChainMap { + const chainConfig: CoreConfig = { + owner, + defaultIsm: { + type: ModuleType.NULL, + }, + defaultHook: { + type: HookType.MERKLE_TREE, + }, + requiredHook: { + type: HookType.PROTOCOL_FEE, + maxProtocolFee: ethers.utils.parseUnits('1', 'gwei'), // 1 gwei of native token + protocolFee: BigNumber.from(1), // 1 wei + beneficiary: nonZeroAddress, + owner, }, }; - return config; + return Object.fromEntries(chains.map((local) => [local, chainConfig])); } // A mock CoinGecko intended to be used by tests