From f83b492deb4fdd13e3494b6867c1ae3c5e06b4d1 Mon Sep 17 00:00:00 2001 From: Paul Balaji <10051819+paulbalaji@users.noreply.github.com> Date: Mon, 15 Jul 2024 10:47:29 +0100 Subject: [PATCH] feat: implement `update()` for `EvmHookModule` (#3888) resolves https://github.com/hyperlane-xyz/hyperlane-monorepo/issues/3577 - enables updates of hooks - includes functionality for updating IGP hook and gas oracles - drive-by fix to ism module TODO: - [x] tests (non-igp) - [x] tests (igp) --------- Signed-off-by: Paul Balaji Signed-off-by: Paul Balaji <10051819+paulbalaji@users.noreply.github.com> Signed-off-by: pbio <10051819+paulbalaji@users.noreply.github.com> --- .changeset/hip-suits-smoke.md | 6 + .registryrc | 2 +- rust/config/testnet_config.json | 36 ++ .../src/hook/EvmHookModule.hardhat-test.ts | 587 +++++++++++++----- typescript/sdk/src/hook/EvmHookModule.ts | 519 +++++++++++++--- typescript/sdk/src/hook/types.ts | 9 + .../sdk/src/ism/EvmIsmModule.hardhat-test.ts | 3 +- typescript/sdk/src/ism/EvmIsmModule.ts | 116 ++-- typescript/sdk/src/test/testUtils.ts | 2 +- 9 files changed, 975 insertions(+), 305 deletions(-) create mode 100644 .changeset/hip-suits-smoke.md diff --git a/.changeset/hip-suits-smoke.md b/.changeset/hip-suits-smoke.md new file mode 100644 index 000000000..3d2f2cb64 --- /dev/null +++ b/.changeset/hip-suits-smoke.md @@ -0,0 +1,6 @@ +--- +'@hyperlane-xyz/sdk': minor +--- + +- Enable updating of hooks through the `EvmHookModule`, including IGP and gas oracles. +- Drive-by fixes to ISM module and tests. diff --git a/.registryrc b/.registryrc index 7fe52d367..b1d18bc43 100644 --- a/.registryrc +++ b/.registryrc @@ -1 +1 @@ -v2.2.1 +v2.3.0 diff --git a/rust/config/testnet_config.json b/rust/config/testnet_config.json index 3680b07c9..70361f92a 100644 --- a/rust/config/testnet_config.json +++ b/rust/config/testnet_config.json @@ -22,6 +22,10 @@ "reorgPeriod": 0 }, "chainId": 44787, + "deployer": { + "name": "Abacus Works", + "url": "https://www.hyperlane.xyz" + }, "displayName": "Alfajores", "domainId": 44787, "domainRoutingIsmFactory": "0x30d9A03762431F8A917a0C469E7A62Bf55092Ca6", @@ -75,6 +79,10 @@ "reorgPeriod": 9 }, "chainId": 97, + "deployer": { + "name": "Abacus Works", + "url": "https://www.hyperlane.xyz" + }, "displayName": "BSC Testnet", "domainId": 97, "fallbackRoutingHook": "0x2670ED2EC08cAd135307556685a96bD4c16b007b", @@ -125,6 +133,10 @@ "reorgPeriod": 0 }, "chainId": 239092742, + "deployer": { + "name": "Abacus Works", + "url": "https://www.hyperlane.xyz" + }, "displayName": "Eclipse Testnet", "domainId": 239092742, "index": { @@ -165,6 +177,10 @@ "reorgPeriod": 3 }, "chainId": 43113, + "deployer": { + "name": "Abacus Works", + "url": "https://www.hyperlane.xyz" + }, "displayName": "Fuji", "domainId": 43113, "domainRoutingIsmFactory": "0x683a81E0e1a238dcA7341e04c08d3bba6f0Cb74f", @@ -222,6 +238,10 @@ "reorgPeriod": 2 }, "chainId": 17000, + "deployer": { + "name": "Abacus Works", + "url": "https://www.hyperlane.xyz" + }, "displayName": "Holesky", "domainId": 17000, "domainRoutingIsmFactory": "0xDDcFEcF17586D08A5740B7D91735fcCE3dfe3eeD", @@ -273,6 +293,10 @@ "reorgPeriod": 0 }, "chainId": 161221135, + "deployer": { + "name": "Abacus Works", + "url": "https://www.hyperlane.xyz" + }, "displayName": "Plume Testnet", "domainId": 161221135, "domainRoutingIsmFactory": "0x54148470292C24345fb828B003461a9444414517", @@ -332,6 +356,10 @@ "reorgPeriod": 1 }, "chainId": 534351, + "deployer": { + "name": "Abacus Works", + "url": "https://www.hyperlane.xyz" + }, "displayName": "Scroll Sepolia", "domainId": 534351, "domainRoutingIsmFactory": "0x17866ebE0e503784a9461d3e753dEeD0d3F61153", @@ -385,6 +413,10 @@ "reorgPeriod": 2 }, "chainId": 11155111, + "deployer": { + "name": "Abacus Works", + "url": "https://www.hyperlane.xyz" + }, "displayName": "Sepolia", "domainId": 11155111, "domainRoutingIsmFactory": "0x3F100cBBE5FD5466BdB4B3a15Ac226957e7965Ad", @@ -445,6 +477,10 @@ "reorgPeriod": 0 }, "chainId": 1399811150, + "deployer": { + "name": "Abacus Works", + "url": "https://www.hyperlane.xyz" + }, "displayName": "Solana Testnet", "displayNameShort": "Sol Testnet", "domainId": 1399811150, diff --git a/typescript/sdk/src/hook/EvmHookModule.hardhat-test.ts b/typescript/sdk/src/hook/EvmHookModule.hardhat-test.ts index 3a8094eb6..dede95caf 100644 --- a/typescript/sdk/src/hook/EvmHookModule.hardhat-test.ts +++ b/typescript/sdk/src/hook/EvmHookModule.hardhat-test.ts @@ -1,13 +1,10 @@ /* eslint-disable no-console */ +import assert from 'assert'; import { expect } from 'chai'; +import { Signer } from 'ethers'; import hre from 'hardhat'; -import { - Address, - configDeepEquals, - normalizeConfig, - stringifyObject, -} from '@hyperlane-xyz/utils'; +import { Address, eqAddress, normalizeConfig } from '@hyperlane-xyz/utils'; import { TestChainName, testChains } from '../consts/testChains.js'; import { HyperlaneAddresses, HyperlaneContracts } from '../contracts/types.js'; @@ -27,7 +24,7 @@ import { HookConfig, HookType, IgpHookConfig, - MerkleTreeHookConfig, + MUTABLE_HOOK_TYPE, PausableHookConfig, ProtocolFeeHookConfig, } from './types.js'; @@ -156,15 +153,18 @@ function randomHookConfig( } describe('EvmHookModule', async () => { + const chain = TestChainName.test4; + let multiProvider: MultiProvider; let coreAddresses: CoreAddresses; - - const chain = TestChainName.test4; + let signer: Signer; + let funder: Signer; let proxyFactoryAddresses: HyperlaneAddresses; let factoryContracts: HyperlaneContracts; + let exampleRoutingConfig: DomainRoutingHookConfig | FallbackRoutingHookConfig; beforeEach(async () => { - const [signer] = await hre.ethers.getSigners(); + [signer, funder] = await hre.ethers.getSigners(); multiProvider = MultiProvider.createTestMultiProvider({ signer }); const ismFactoryDeployer = new HyperlaneProxyFactoryDeployer(multiProvider); @@ -202,27 +202,47 @@ describe('EvmHookModule', async () => { proxyAdmin: proxyAdmin.address, validatorAnnounce: validatorAnnounce.address, }; + + // reusable for routing/fallback routing specific tests + exampleRoutingConfig = { + owner: (await multiProvider.getSignerAddress(chain)).toLowerCase(), + domains: Object.fromEntries( + testChains.map((c) => [ + c, + { + type: HookType.MERKLE_TREE, + }, + ]), + ), + type: HookType.FALLBACK_ROUTING, + fallback: { type: HookType.MERKLE_TREE }, + }; }); - // Helper method for checking whether Hook module matches a given config - async function hookModuleMatchesConfig({ - hook, - config, - }: { - hook: EvmHookModule; - config: HookConfig; - }): Promise { - const normalizedDerivedConfig = normalizeConfig(await hook.read()); - const normalizedConfig = normalizeConfig(config); - const matches = configDeepEquals(normalizedDerivedConfig, normalizedConfig); - if (!matches) { - console.error( - 'Derived config:\n', - stringifyObject(normalizedDerivedConfig), - ); - console.error('Expected config:\n', stringifyObject(normalizedConfig)); + // Helper method for create a new multiprovider with an impersonated account + async function impersonateAccount(account: Address): Promise { + await hre.ethers.provider.send('hardhat_impersonateAccount', [account]); + await funder.sendTransaction({ + to: account, + value: hre.ethers.utils.parseEther('1.0'), + }); + return MultiProvider.createTestMultiProvider({ + signer: hre.ethers.provider.getSigner(account), + }); + } + + // Helper method to expect exactly N updates to be applied + async function expectTxsAndUpdate( + hook: EvmHookModule, + config: HookConfig, + n: number, + ) { + const txs = await hook.update(config); + expect(txs.length).to.equal(n); + + for (const tx of txs) { + await multiProvider.sendTransaction(chain, tx); } - return matches; } // hook module and config for testing @@ -231,9 +251,9 @@ describe('EvmHookModule', async () => { // expect that the hook matches the config after all tests afterEach(async () => { - expect( - await hookModuleMatchesConfig({ hook: testHook, config: testConfig }), - ).to.be.true; + const normalizedDerivedConfig = normalizeConfig(await testHook.read()); + const normalizedConfig = normalizeConfig(testConfig); + assert.deepStrictEqual(normalizedDerivedConfig, normalizedConfig); }); // create a new Hook and verify that it matches the config @@ -247,124 +267,45 @@ describe('EvmHookModule', async () => { coreAddresses, multiProvider, }); - testConfig = config; testHook = hook; + testConfig = config; return { hook, initialHookAddress: hook.serialize().deployedHook }; } describe('create', async () => { + // generate a random config for each hook type + const exampleHookConfigs: HookConfig[] = [ + // include an address config + randomAddress(), + ...hookTypes + // need to setup deploying/mocking IL1CrossDomainMessenger before this test can be enabled + .filter( + (hookType) => + hookType !== HookType.OP_STACK && hookType !== HookType.CUSTOM, + ) + // generate a random config for each hook type + .map((hookType) => { + return randomHookConfig(0, 1, hookType); + }), + ]; + + // test deployment of each hookType, except OP_STACK and CUSTOM + // minimum depth only + for (const config of exampleHookConfigs) { + it(`deploys a hook of type ${ + typeof config === 'string' ? 'address' : config.type + }`, async () => { + await createHook(config); + }); + } + + // manually include test for CUSTOM hook type it('deploys a hook of type CUSTOM', async () => { const config: HookConfig = randomAddress(); await createHook(config); }); - it('deploys a hook of type MERKLE_TREE', async () => { - const config: MerkleTreeHookConfig = { - type: HookType.MERKLE_TREE, - }; - await createHook(config); - }); - - it('deploys a hook of type INTERCHAIN_GAS_PAYMASTER', async () => { - const owner = randomAddress(); - const config: IgpHookConfig = { - owner, - type: HookType.INTERCHAIN_GAS_PAYMASTER, - beneficiary: randomAddress(), - oracleKey: owner, - overhead: Object.fromEntries( - testChains.map((c) => [c, Math.floor(Math.random() * 100)]), - ), - oracleConfig: Object.fromEntries( - testChains.map((c) => [ - c, - { - tokenExchangeRate: randomInt(1234567891234).toString(), - gasPrice: randomInt(1234567891234).toString(), - }, - ]), - ), - }; - await createHook(config); - }); - - it('deploys a hook of type PROTOCOL_FEE', async () => { - const { maxProtocolFee, protocolFee } = randomProtocolFee(); - const config: ProtocolFeeHookConfig = { - owner: randomAddress(), - type: HookType.PROTOCOL_FEE, - maxProtocolFee, - protocolFee, - beneficiary: randomAddress(), - }; - await createHook(config); - }); - - it('deploys a hook of type ROUTING', async () => { - const config: DomainRoutingHookConfig = { - owner: randomAddress(), - type: HookType.ROUTING, - domains: Object.fromEntries( - testChains - .filter((c) => c !== TestChainName.test4) - .map((c) => [ - c, - { - type: HookType.MERKLE_TREE, - }, - ]), - ), - }; - await createHook(config); - }); - - it('deploys a hook of type FALLBACK_ROUTING', async () => { - const config: FallbackRoutingHookConfig = { - owner: randomAddress(), - type: HookType.FALLBACK_ROUTING, - fallback: { type: HookType.MERKLE_TREE }, - domains: Object.fromEntries( - testChains - .filter((c) => c !== TestChainName.test4) - .map((c) => [ - c, - { - type: HookType.MERKLE_TREE, - }, - ]), - ), - }; - await createHook(config); - }); - - it('deploys a hook of type AGGREGATION', async () => { - const config: AggregationHookConfig = { - type: HookType.AGGREGATION, - hooks: [{ type: HookType.MERKLE_TREE }, { type: HookType.MERKLE_TREE }], - }; - await createHook(config); - }); - - it('deploys a hook of type PAUSABLE', async () => { - const config: PausableHookConfig = { - owner: randomAddress(), - type: HookType.PAUSABLE, - paused: false, - }; - await createHook(config); - }); - - // it('deploys a hook of type OP_STACK', async () => { - // need to setup deploying/mocking IL1CrossDomainMessenger before this test can be enabled - // const config: OpStackHookConfig = { - // owner: randomAddress(), - // type: HookType.OP_STACK, - // nativeBridge: randomAddress(), - // destinationChain: 'testChain', - // }; - // await createHook(config); - // }); - + // random configs upto depth 2 for (let i = 0; i < 16; i++) { it(`deploys a random hook config #${i}`, async () => { // random config with depth 0-2 @@ -373,6 +314,7 @@ describe('EvmHookModule', async () => { }); } + // manual test to catch regressions on a complex config type it('regression test #1', async () => { const config: HookConfig = { type: HookType.AGGREGATION, @@ -484,4 +426,371 @@ describe('EvmHookModule', async () => { await createHook(config); }); }); + + describe('update', async () => { + it('should update by deploying a new aggregation hook', async () => { + const config: AggregationHookConfig = { + type: HookType.AGGREGATION, + hooks: [randomHookConfig(0, 2), randomHookConfig(0, 2)], + }; + + // create a new hook + const { hook, initialHookAddress } = await createHook(config); + + // change the hooks + config.hooks = [randomHookConfig(0, 2), randomHookConfig(0, 2)]; + + // expect 0 tx to be returned, as it should deploy a new aggregation hook + await expectTxsAndUpdate(hook, config, 0); + + // expect the hook address to be different + expect(eqAddress(initialHookAddress, hook.serialize().deployedHook)).to.be + .false; + }); + + const createDeployerOwnedIgpHookConfig = + async (): Promise => { + const owner = await multiProvider.getSignerAddress(chain); + return { + owner, + type: HookType.INTERCHAIN_GAS_PAYMASTER, + beneficiary: randomAddress(), + oracleKey: owner, + overhead: Object.fromEntries( + testChains.map((c) => [c, Math.floor(Math.random() * 100)]), + ), + oracleConfig: Object.fromEntries( + testChains.map((c) => [ + c, + { + tokenExchangeRate: randomInt(1234567891234).toString(), + gasPrice: randomInt(1234567891234).toString(), + }, + ]), + ), + }; + }; + + it('should update beneficiary in IGP', async () => { + const config = await createDeployerOwnedIgpHookConfig(); + + // create a new hook + const { hook } = await createHook(config); + + // change the beneficiary + config.beneficiary = randomAddress(); + + // expect 1 tx to update the beneficiary + await expectTxsAndUpdate(hook, config, 1); + }); + + it('should update the overheads in IGP', async () => { + const config = await createDeployerOwnedIgpHookConfig(); + + // create a new hook + const { hook } = await createHook(config); + + // change the overheads + config.overhead = Object.fromEntries( + testChains.map((c) => [c, Math.floor(Math.random() * 100)]), + ); + + // expect 1 tx to update the overheads + await expectTxsAndUpdate(hook, config, 1); + }); + + it('should update the oracle config in IGP', async () => { + const config = await createDeployerOwnedIgpHookConfig(); + + // create a new hook + const { hook } = await createHook(config); + + // change the oracle config + config.oracleConfig = Object.fromEntries( + testChains.map((c) => [ + c, + { + tokenExchangeRate: randomInt(987654321).toString(), + gasPrice: randomInt(987654321).toString(), + }, + ]), + ); + + // expect 1 tx to update the oracle config + await expectTxsAndUpdate(hook, config, 1); + }); + + it('should update protocol fee in protocol fee hook', async () => { + const config: ProtocolFeeHookConfig = { + owner: await multiProvider.getSignerAddress(chain), + type: HookType.PROTOCOL_FEE, + maxProtocolFee: '1000', + protocolFee: '100', + beneficiary: randomAddress(), + }; + + // create a new hook + const { hook } = await createHook(config); + + // change the protocol fee + config.protocolFee = '200'; + + // expect 1 tx to update the protocol fee + await expectTxsAndUpdate(hook, config, 1); + }); + + it('should update max fee in protocol fee hook', async () => { + const config: ProtocolFeeHookConfig = { + owner: await multiProvider.getSignerAddress(chain), + type: HookType.PROTOCOL_FEE, + maxProtocolFee: '1000', + protocolFee: '100', + beneficiary: randomAddress(), + }; + + // create a new hook + const { hook, initialHookAddress } = await createHook(config); + + // change the protocol fee + config.maxProtocolFee = '2000'; + + // expect 0 tx to update the max protocol fee as it has to deploy a new hook + await expectTxsAndUpdate(hook, config, 0); + + // expect the hook address to be different + expect(eqAddress(initialHookAddress, hook.serialize().deployedHook)).to.be + .false; + }); + + it('should update paused state of pausable hook', async () => { + const config: PausableHookConfig = { + owner: randomAddress(), + type: HookType.PAUSABLE, + paused: false, + }; + + // create a new hook + const { hook } = await createHook(config); + + // change the paused state + config.paused = true; + + // impersonate the hook owner + multiProvider = await impersonateAccount(config.owner); + + // expect 1 tx to update the paused state + await expectTxsAndUpdate(hook, config, 1); + }); + + for (const type of [HookType.ROUTING, HookType.FALLBACK_ROUTING]) { + beforeEach(() => { + exampleRoutingConfig.type = type as + | HookType.ROUTING + | HookType.FALLBACK_ROUTING; + }); + + it(`should skip deployment with warning if no chain metadata configured ${type}`, async () => { + // create a new hook + const { hook } = await createHook(exampleRoutingConfig); + + // add config for a domain the multiprovider doesn't have + const updatedConfig: HookConfig = { + ...exampleRoutingConfig, + domains: { + ...exampleRoutingConfig.domains, + test5: { type: HookType.MERKLE_TREE }, + }, + }; + + // expect 0 txs, as adding test5 domain is no-op + await expectTxsAndUpdate(hook, updatedConfig, 0); + }); + + it(`no changes to an existing ${type} means no redeployment or updates`, async () => { + // create a new hook + const { hook, initialHookAddress } = await createHook( + exampleRoutingConfig, + ); + + // expect 0 updates + await expectTxsAndUpdate(hook, exampleRoutingConfig, 0); + + // expect the hook address to be the same + expect(eqAddress(initialHookAddress, hook.serialize().deployedHook)).to + .be.true; + }); + + it(`updates an existing ${type} with new domains`, async () => { + exampleRoutingConfig = { + owner: (await multiProvider.getSignerAddress(chain)).toLowerCase(), + domains: { + test1: { + type: HookType.MERKLE_TREE, + }, + }, + type: HookType.FALLBACK_ROUTING, + fallback: { type: HookType.MERKLE_TREE }, + }; + + // create a new hook + const { hook, initialHookAddress } = await createHook( + exampleRoutingConfig, + ); + + // add a new domain + exampleRoutingConfig.domains[TestChainName.test2] = { + type: HookType.MERKLE_TREE, + }; + + // expect 1 tx to update the domains + await expectTxsAndUpdate(hook, exampleRoutingConfig, 1); + + // expect the hook address to be the same + expect(eqAddress(initialHookAddress, hook.serialize().deployedHook)).to + .be.true; + }); + + it(`updates an existing ${type} with new domains`, async () => { + exampleRoutingConfig = { + owner: (await multiProvider.getSignerAddress(chain)).toLowerCase(), + domains: { + test1: { + type: HookType.MERKLE_TREE, + }, + }, + type: HookType.FALLBACK_ROUTING, + fallback: { type: HookType.MERKLE_TREE }, + }; + + // create a new hook + const { hook, initialHookAddress } = await createHook( + exampleRoutingConfig, + ); + + // add multiple new domains + exampleRoutingConfig.domains[TestChainName.test2] = { + type: HookType.MERKLE_TREE, + }; + exampleRoutingConfig.domains[TestChainName.test3] = { + type: HookType.MERKLE_TREE, + }; + exampleRoutingConfig.domains[TestChainName.test4] = { + type: HookType.MERKLE_TREE, + }; + + // expect 1 tx to update the domains + await expectTxsAndUpdate(hook, exampleRoutingConfig, 1); + + // expect the hook address to be the same + expect(eqAddress(initialHookAddress, hook.serialize().deployedHook)).to + .be.true; + }); + } + + it(`update fallback in an existing fallback routing hook`, async () => { + // create a new hook + const config = exampleRoutingConfig as FallbackRoutingHookConfig; + const { hook, initialHookAddress } = await createHook(config); + + // change the fallback + config.fallback = { + type: HookType.PROTOCOL_FEE, + owner: randomAddress(), + maxProtocolFee: '9000', + protocolFee: '350', + beneficiary: randomAddress(), + }; + + // expect 0 tx as it will have to deploy a new fallback routing hook + await expectTxsAndUpdate(hook, config, 0); + + // expect the hook address to be different + expect(eqAddress(initialHookAddress, hook.serialize().deployedHook)).to.be + .false; + }); + + it(`update fallback in an existing fallback routing hook with no change`, async () => { + // create a new hook + const config = exampleRoutingConfig as FallbackRoutingHookConfig; + const { hook } = await createHook(config); + + // expect 0 updates + await expectTxsAndUpdate(hook, config, 0); + }); + + // generate a random config for each ownable hook type + const ownableHooks = hookTypes + .filter((hookType) => MUTABLE_HOOK_TYPE.includes(hookType)) + .map((hookType) => { + return randomHookConfig(0, 1, hookType); + }); + + for (const config of ownableHooks) { + assert( + typeof config !== 'string', + 'Address is not an ownable hook config', + ); + assert( + 'owner' in config, + 'Ownable hook config must have an owner property', + ); + + it(`updates owner in an existing ${config.type}`, async () => { + // hook owned by the deployer + config.owner = await multiProvider.getSignerAddress(chain); + + // create a new hook + const { hook, initialHookAddress } = await createHook(config); + + // change the config owner + config.owner = randomAddress(); + + // expect 1 tx to transfer ownership + await expectTxsAndUpdate(hook, config, 1); + + // expect the hook address to be the same + expect(eqAddress(initialHookAddress, hook.serialize().deployedHook)).to + .be.true; + }); + + it(`update owner in an existing ${config.type} not owned by deployer`, async () => { + // hook owner is not the deployer + config.owner = randomAddress(); + const originalOwner = config.owner; + + // create a new hook + const { hook, initialHookAddress } = await createHook(config); + + // update the config owner and impersonate the original owner + config.owner = randomAddress(); + multiProvider = await impersonateAccount(originalOwner); + + // expect 1 tx to transfer ownership + await expectTxsAndUpdate(hook, config, 1); + + // expect the hook address to be unchanged + expect(eqAddress(initialHookAddress, hook.serialize().deployedHook)).to + .be.true; + }); + + it(`update owner in an existing ${config.type} not owned by deployer and no change`, async () => { + // hook owner is not the deployer + config.owner = randomAddress(); + const originalOwner = config.owner; + + // create a new hook + const { hook, initialHookAddress } = await createHook(config); + + // impersonate the original owner + multiProvider = await impersonateAccount(originalOwner); + + // expect 0 updates + await expectTxsAndUpdate(hook, config, 0); + + // expect the hook address to be unchanged + expect(eqAddress(initialHookAddress, hook.serialize().deployedHook)).to + .be.true; + }); + } + }); }); diff --git a/typescript/sdk/src/hook/EvmHookModule.ts b/typescript/sdk/src/hook/EvmHookModule.ts index 29ea95534..19333932c 100644 --- a/typescript/sdk/src/hook/EvmHookModule.ts +++ b/typescript/sdk/src/hook/EvmHookModule.ts @@ -7,20 +7,27 @@ import { IL1CrossDomainMessenger__factory, IPostDispatchHook__factory, InterchainGasPaymaster, + InterchainGasPaymaster__factory, OPStackHook, OPStackIsm__factory, + Ownable__factory, PausableHook, + PausableHook__factory, ProtocolFee, + ProtocolFee__factory, StaticAggregationHook, StaticAggregationHookFactory__factory, StaticAggregationHook__factory, StorageGasOracle, + StorageGasOracle__factory, } from '@hyperlane-xyz/core'; import { Address, ProtocolType, addressToBytes32, configDeepEquals, + eqAddress, + normalizeConfig, rootLogger, } from '@hyperlane-xyz/utils'; @@ -44,6 +51,7 @@ import { ChainNameOrId } from '../types.js'; import { EvmHookReader } from './EvmHookReader.js'; import { DeployedHook, HookFactories, hookFactories } from './contracts.js'; +import { HookConfigSchema } from './schemas.js'; import { AggregationHookConfig, DomainRoutingHookConfig, @@ -51,6 +59,7 @@ import { HookConfig, HookType, IgpHookConfig, + MUTABLE_HOOK_TYPE, OpStackHookConfig, PausableHookConfig, ProtocolFeeHookConfig, @@ -82,13 +91,14 @@ export class EvmHookModule extends HyperlaneModule< protected constructor( protected readonly multiProvider: MultiProvider, - args: HyperlaneModuleParams< + params: HyperlaneModuleParams< HookConfig, HyperlaneAddresses & HookModuleAddresses >, contractVerifier?: ContractVerifier, ) { - super(args); + params.config = HookConfigSchema.parse(params.config); + super(params); this.reader = new EvmHookReader(multiProvider, this.args.chain); this.deployer = new EvmModuleDeployer( @@ -113,8 +123,100 @@ export class EvmHookModule extends HyperlaneModule< : this.reader.deriveHookConfig(this.args.addresses.deployedHook); } - public async update(_config: HookConfig): Promise { - throw new Error('Method not implemented.'); + public async update( + targetConfig: HookConfig, + ): Promise { + targetConfig = HookConfigSchema.parse(targetConfig); + + // Do not support updating to a custom Hook address + if (typeof targetConfig === 'string') { + throw new Error( + 'Invalid targetConfig: Updating to a custom Hook address is not supported. Please provide a valid Hook configuration.', + ); + } + + const unnormalizedCurrentConfig = await this.read(); + const currentConfig = normalizeConfig(unnormalizedCurrentConfig); + + // Update the config + this.args.config = targetConfig; + + // If configs match, no updates needed + if (configDeepEquals(currentConfig, targetConfig)) { + return []; + } + + if (this.shouldDeployNewHook(currentConfig, targetConfig)) { + const contract = await this.deploy({ + config: targetConfig, + }); + + this.args.addresses.deployedHook = contract.address; + return []; + } + + const updateTxs: AnnotatedEV5Transaction[] = []; + + // obtain the update txs for each hook type + switch (targetConfig.type) { + case HookType.INTERCHAIN_GAS_PAYMASTER: + updateTxs.push( + ...(await this.updateIgpHook({ + currentConfig, + targetConfig, + })), + ); + break; + case HookType.PROTOCOL_FEE: + updateTxs.push( + ...(await this.updateProtocolFeeHook({ + currentConfig, + targetConfig, + })), + ); + break; + case HookType.PAUSABLE: + updateTxs.push( + ...(await this.updatePausableHook({ + currentConfig, + targetConfig, + })), + ); + break; + case HookType.ROUTING: + case HookType.FALLBACK_ROUTING: + updateTxs.push( + ...(await this.updateRoutingHook({ + currentConfig, + targetConfig, + })), + ); + break; + default: + // MERKLE_TREE, AGGREGATION and OP_STACK hooks should already be handled before the switch + throw new Error(`Unsupported hook type: ${targetConfig.type}`); + } + + // Lastly, check if the resolved owner is different from the current owner + const owner = await Ownable__factory.connect( + this.args.addresses.deployedHook, + this.multiProvider.getProvider(this.chain), + ).owner(); + + // Return an ownership transfer transaction if required + if (!eqAddress(targetConfig.owner, owner)) { + updateTxs.push({ + annotation: 'Transferring ownership of ownable Hook...', + chainId: this.domainId, + to: this.args.addresses.deployedHook, + data: Ownable__factory.createInterface().encodeFunctionData( + 'transferOwnership(address)', + [targetConfig.owner], + ), + }); + } + + return updateTxs; } // manually write static create function @@ -184,29 +286,296 @@ export class EvmHookModule extends HyperlaneModule< return routingHookUpdates; } + protected async updatePausableHook({ + currentConfig, + targetConfig, + }: { + currentConfig: PausableHookConfig; + targetConfig: PausableHookConfig; + }): Promise { + const updateTxs: AnnotatedEV5Transaction[] = []; + + if (currentConfig.paused !== targetConfig.paused) { + // Have to encode separately otherwise tsc will complain + // about being unable to infer types correctly + const pausableInterface = PausableHook__factory.createInterface(); + const data = targetConfig.paused + ? pausableInterface.encodeFunctionData('pause') + : pausableInterface.encodeFunctionData('unpause'); + + updateTxs.push({ + annotation: `Updating paused state to ${targetConfig.paused}`, + chainId: this.domainId, + to: this.args.addresses.deployedHook, + data, + }); + } + + return updateTxs; + } + + protected async updateIgpHook({ + currentConfig, + targetConfig, + }: { + currentConfig: IgpHookConfig; + targetConfig: IgpHookConfig; + }): Promise { + const updateTxs: AnnotatedEV5Transaction[] = []; + const igpInterface = InterchainGasPaymaster__factory.createInterface(); + + // Update beneficiary if changed + if (!eqAddress(currentConfig.beneficiary, targetConfig.beneficiary)) { + updateTxs.push({ + annotation: `Updating beneficiary from ${currentConfig.beneficiary} to ${targetConfig.beneficiary}`, + chainId: this.domainId, + to: this.args.addresses.deployedHook, + data: igpInterface.encodeFunctionData('setBeneficiary(address)', [ + targetConfig.beneficiary, + ]), + }); + } + + // get gasOracleAddress using any remote domain in the current config + let gasOracle; + const domainKeys = Object.keys(currentConfig.oracleConfig); + + // If possible, reuse and reconfigure the gas oracle from the first remote we know. + // Otherwise if there are no remotes in current config, deploy a new gas oracle with our target config. + // We should be reusing the same oracle for all remotes, but if not, the updateIgpRemoteGasParams step will rectify this + if (domainKeys.length > 0) { + const domainId = this.multiProvider.getDomainId(domainKeys[0]); + ({ gasOracle } = await InterchainGasPaymaster__factory.connect( + this.args.addresses.deployedHook, + this.multiProvider.getSignerOrProvider(this.chain), + )['destinationGasConfigs(uint32)'](domainId)); + + // update storage gas oracle + // Note: this will only update the gas oracle for remotes that are in the target config + updateTxs.push( + ...(await this.updateStorageGasOracle({ + gasOracle, + currentOracleConfig: currentConfig.oracleConfig, + targetOracleConfig: targetConfig.oracleConfig, + targetOverhead: targetConfig.overhead, // used to log example remote gas costs + })), + ); + } else { + const newGasOracle = await this.deployStorageGasOracle({ + config: targetConfig, + }); + gasOracle = newGasOracle.address; + } + + // update igp remote gas params + // Note: this will only update the gas params for remotes that are in the target config + updateTxs.push( + ...(await this.updateIgpRemoteGasParams({ + interchainGasPaymaster: this.args.addresses.deployedHook, + gasOracle, + currentOverheads: currentConfig.overhead, + targetOverheads: targetConfig.overhead, + })), + ); + + return updateTxs; + } + + protected async updateIgpRemoteGasParams({ + interchainGasPaymaster, + gasOracle, + currentOverheads, + targetOverheads, + }: { + interchainGasPaymaster: Address; + gasOracle: Address; + currentOverheads?: IgpConfig['overhead']; + targetOverheads: IgpConfig['overhead']; + }): Promise { + const gasParamsToSet: InterchainGasPaymaster.GasParamStruct[] = []; + for (const [remote, gasOverhead] of Object.entries(targetOverheads)) { + // Note: non-EVM remotes actually *are* supported, provided that the remote domain is in the MultiProvider. + // Previously would check core metadata for non EVMs and fallback to multiprovider for custom EVMs + const remoteDomain = this.multiProvider.tryGetDomainId(remote); + + if (!remoteDomain) { + this.logger.warn( + `Skipping overhead ${this.chain} -> ${remote}. Expected if the remote domain is not in the MultiProvider.`, + ); + continue; + } + + // only update if the gas overhead has changed + if (currentOverheads?.[remote] !== gasOverhead) { + this.logger.debug( + `Setting gas params for ${this.chain} -> ${remote}: gasOverhead = ${gasOverhead} gasOracle = ${gasOracle}`, + ); + gasParamsToSet.push({ + remoteDomain, + config: { + gasOverhead, + gasOracle, + }, + }); + } + } + + if (gasParamsToSet.length === 0) { + return []; + } + + return [ + { + annotation: `Updating overhead for domains ${Object.keys( + targetOverheads, + ).join(', ')}...`, + chainId: this.domainId, + to: interchainGasPaymaster, + data: InterchainGasPaymaster__factory.createInterface().encodeFunctionData( + 'setDestinationGasConfigs((uint32,(address,uint96))[])', + [gasParamsToSet], + ), + }, + ]; + } + + protected async updateStorageGasOracle({ + gasOracle, + currentOracleConfig, + targetOracleConfig, + targetOverhead, + }: { + gasOracle: Address; + currentOracleConfig?: IgpConfig['oracleConfig']; + targetOracleConfig: IgpConfig['oracleConfig']; + targetOverhead: IgpConfig['overhead']; + }): Promise { + this.logger.info(`Updating gas oracle configuration from ${this.chain}...`); + const configsToSet: Array = []; + + for (const [remote, target] of Object.entries(targetOracleConfig)) { + // Note: non-EVM remotes actually *are* supported, provided that the remote domain is in the MultiProvider. + // Previously would check core metadata for non EVMs and fallback to multiprovider for custom EVMs + const current = currentOracleConfig?.[remote]; + const remoteDomain = this.multiProvider.tryGetDomainId(remote); + + if (!remoteDomain) { + this.logger.warn( + `Skipping gas oracle update ${this.chain} -> ${remote}. Expected if the remote domain is not in the MultiProvider.`, + ); + continue; + } + + // only update if the oracle config has changed + if (!current || !configDeepEquals(current, target)) { + configsToSet.push({ remoteDomain, ...target }); + + // Log an example remote gas cost + const exampleRemoteGas = (targetOverhead[remote] ?? 200_000) + 50_000; + const exampleRemoteGasCost = BigNumber.from(target.tokenExchangeRate) + .mul(target.gasPrice) + .mul(exampleRemoteGas) + .div(TOKEN_EXCHANGE_RATE_SCALE); + this.logger.info( + `${ + this.chain + } -> ${remote}: ${exampleRemoteGas} remote gas cost: ${ethers.utils.formatEther( + exampleRemoteGasCost, + )}`, + ); + } + } + + if (configsToSet.length === 0) { + return []; + } + + return [ + { + annotation: `Updating gas oracle config for domains ${Object.keys( + targetOracleConfig, + ).join(', ')}...`, + chainId: this.domainId, + to: gasOracle, + data: StorageGasOracle__factory.createInterface().encodeFunctionData( + 'setRemoteGasDataConfigs((uint32,uint128,uint128)[])', + [configsToSet], + ), + }, + ]; + } + + protected async updateProtocolFeeHook({ + currentConfig, + targetConfig, + }: { + currentConfig: ProtocolFeeHookConfig; + targetConfig: ProtocolFeeHookConfig; + }): Promise { + const updateTxs: AnnotatedEV5Transaction[] = []; + const protocolFeeInterface = ProtocolFee__factory.createInterface(); + + // if maxProtocolFee has changed, deploy a new hook + if (currentConfig.maxProtocolFee !== targetConfig.maxProtocolFee) { + const hook = await this.deployProtocolFeeHook({ config: targetConfig }); + this.args.addresses.deployedHook = hook.address; + return []; + } + + // Update protocol fee if changed + if (currentConfig.protocolFee !== targetConfig.protocolFee) { + updateTxs.push({ + annotation: `Updating protocol fee from ${currentConfig.protocolFee} to ${targetConfig.protocolFee}`, + chainId: this.domainId, + to: this.args.addresses.deployedHook, + data: protocolFeeInterface.encodeFunctionData( + 'setProtocolFee(uint256)', + [targetConfig.protocolFee], + ), + }); + } + + // Update beneficiary if changed + if (currentConfig.beneficiary !== targetConfig.beneficiary) { + updateTxs.push({ + annotation: `Updating beneficiary from ${currentConfig.beneficiary} to ${targetConfig.beneficiary}`, + chainId: this.domainId, + to: this.args.addresses.deployedHook, + data: protocolFeeInterface.encodeFunctionData( + 'setBeneficiary(address)', + [targetConfig.beneficiary], + ), + }); + } + + // Return the transactions to update the protocol fee hook + return updateTxs; + } + // Updates a routing hook protected async updateRoutingHook({ - current, - target, + currentConfig, + targetConfig, }: { - current: DomainRoutingHookConfig | FallbackRoutingHookConfig; - target: DomainRoutingHookConfig | FallbackRoutingHookConfig; + currentConfig: DomainRoutingHookConfig | FallbackRoutingHookConfig; + targetConfig: DomainRoutingHookConfig | FallbackRoutingHookConfig; }): Promise { // Deploy a new fallback hook if the fallback config has changed if ( - target.type === HookType.FALLBACK_ROUTING && + targetConfig.type === HookType.FALLBACK_ROUTING && !configDeepEquals( - target.fallback, - (current as FallbackRoutingHookConfig).fallback, + targetConfig.fallback, + (currentConfig as FallbackRoutingHookConfig).fallback, ) ) { - const hook = await this.deploy({ config: target }); + const hook = await this.deploy({ config: targetConfig }); this.args.addresses.deployedHook = hook.address; + return []; } const routingUpdates = await this.computeRoutingHooksToSet({ - currentDomains: current.domains, - targetDomains: target.domains, + currentDomains: currentConfig.domains, + targetDomains: targetConfig.domains, }); // Return if no updates are required @@ -221,7 +590,7 @@ export class EvmHookModule extends HyperlaneModule< chainId: this.domainId, to: this.args.addresses.deployedHook, data: DomainRoutingHook__factory.createInterface().encodeFunctionData( - 'setHooks', + 'setHooks((uint32,address)[])', [routingUpdates], ), }, @@ -233,6 +602,8 @@ export class EvmHookModule extends HyperlaneModule< }: { config: HookConfig; }): Promise { + config = HookConfigSchema.parse(config); + // If it's an address, just return a base Hook if (typeof config === 'string') { // TODO: https://github.com/hyperlane-xyz/hyperlane-monorepo/issues/3773 @@ -317,7 +688,7 @@ export class EvmHookModule extends HyperlaneModule< this.logger.debug('Deploying AggregationHook...'); // deploy subhooks - const aggregatedHooks = []; + const aggregatedHooks: string[] = []; for (const hookConfig of config.hooks) { const { address } = await this.deploy({ config: hookConfig }); aggregatedHooks.push(address); @@ -522,10 +893,12 @@ export class EvmHookModule extends HyperlaneModule< storageGasOracle: StorageGasOracle; config: IgpConfig; }): Promise { + // Set the deployer as the owner of the IGP for configuration purposes const deployerAddress = await this.multiProvider.getSignerAddress( this.chain, ); + // Deploy the InterchainGasPaymaster const igp = await this.deployer.deployProxiedContract({ chain: this.chain, contractKey: HookType.INTERCHAIN_GAS_PAYMASTER, @@ -535,35 +908,16 @@ export class EvmHookModule extends HyperlaneModule< initializeArgs: [deployerAddress, config.beneficiary], }); - const gasParamsToSet: InterchainGasPaymaster.GasParamStruct[] = []; - for (const [remote, gasOverhead] of Object.entries(config.overhead)) { - // Note: non-EVM remotes actually *are* supported, provided that the remote domain is in the MultiProvider. - // Previously would check core metadata for non EVMs and fallback to multiprovider for custom EVMs - const remoteDomain = this.multiProvider.tryGetDomainId(remote); - if (!remoteDomain) { - this.logger.warn( - `Skipping overhead ${this.chain} -> ${remote}. Expected if the remote is a non-EVM chain.`, - ); - continue; - } - - this.logger.debug( - `Setting gas params for ${this.chain} -> ${remote}: gasOverhead = ${gasOverhead} gasOracle = ${storageGasOracle.address}`, - ); - gasParamsToSet.push({ - remoteDomain, - config: { - gasOverhead, - gasOracle: storageGasOracle.address, - }, - }); - } + // Obtain the transactions to set the gas params for each remote + const configureTxs = await this.updateIgpRemoteGasParams({ + interchainGasPaymaster: igp.address, + gasOracle: storageGasOracle.address, + targetOverheads: config.overhead, + }); - if (gasParamsToSet.length > 0) { - await this.multiProvider.handleTx( - this.chain, - igp.setDestinationGasConfigs(gasParamsToSet, this.txOverrides), - ); + // Set the gas params for each remote + for (const tx of configureTxs) { + await this.multiProvider.sendTransaction(this.chain, tx); } // Transfer igp to the configured owner @@ -580,57 +934,23 @@ export class EvmHookModule extends HyperlaneModule< }: { config: IgpConfig; }): Promise { + // Deploy the StorageGasOracle, by default msg.sender is the owner const gasOracle = await this.deployer.deployContract({ chain: this.chain, contractKey: 'storageGasOracle', constructorArgs: [], }); - if (!config.oracleConfig) { - this.logger.debug('No oracle config provided, skipping...'); - return gasOracle; - } - - this.logger.info(`Configuring gas oracle from ${this.chain}...`); - const configsToSet: Array = []; - - for (const [remote, desired] of Object.entries(config.oracleConfig)) { - // Note: non-EVM remotes actually *are* supported, provided that the remote domain is in the MultiProvider. - // Previously would check core metadata for non EVMs and fallback to multiprovider for custom EVMs - const remoteDomain = this.multiProvider.tryGetDomainId(remote); - if (!remoteDomain) { - this.logger.warn( - `Skipping gas oracle ${this.chain} -> ${remote}.` + - ' Expected if the remote is a non-EVM chain or the remote domain is not the in the MultiProvider.', - ); - continue; - } - - configsToSet.push({ - remoteDomain, - ...desired, - }); - - // Log an example remote gas cost - const exampleRemoteGas = (config.overhead[remote] ?? 200_000) + 50_000; - const exampleRemoteGasCost = BigNumber.from(desired.tokenExchangeRate) - .mul(desired.gasPrice) - .mul(exampleRemoteGas) - .div(TOKEN_EXCHANGE_RATE_SCALE); - this.logger.info( - `${ - this.chain - } -> ${remote}: ${exampleRemoteGas} remote gas cost: ${ethers.utils.formatEther( - exampleRemoteGasCost, - )}`, - ); - } + // Obtain the transactions to set the gas params for each remote + const configureTxs = await this.updateStorageGasOracle({ + gasOracle: gasOracle.address, + targetOracleConfig: config.oracleConfig, + targetOverhead: config.overhead, + }); - if (configsToSet.length > 0) { - await this.multiProvider.handleTx( - this.chain, - gasOracle.setRemoteGasDataConfigs(configsToSet, this.txOverrides), - ); + // Set the gas params for each remote + for (const tx of configureTxs) { + await this.multiProvider.sendTransaction(this.chain, tx); } // Transfer gas oracle to the configured owner @@ -641,4 +961,27 @@ export class EvmHookModule extends HyperlaneModule< return gasOracle; } + + /** + * Determines if a new hook should be deployed based on the current and target configurations. + * + * @param currentConfig - The current hook configuration. + * @param targetConfig - The target hook configuration. Must not be a string. + * @returns {boolean} - Returns true if a new hook should be deployed, otherwise false. + * + * Conditions for deploying a new hook: + * - If updating from an address/custom config to a proper hook config. + * - If updating a proper hook config whose types are different. + * - If it is not a mutable Hook. + */ + private shouldDeployNewHook( + currentConfig: HookConfig, + targetConfig: Exclude, + ): boolean { + return ( + typeof currentConfig === 'string' || + currentConfig.type !== targetConfig.type || + !MUTABLE_HOOK_TYPE.includes(targetConfig.type) + ); + } } diff --git a/typescript/sdk/src/hook/types.ts b/typescript/sdk/src/hook/types.ts index c0edaafce..907d00ddf 100644 --- a/typescript/sdk/src/hook/types.ts +++ b/typescript/sdk/src/hook/types.ts @@ -61,3 +61,12 @@ export type FallbackRoutingHookConfig = RoutingHookConfig & { }; export type HookConfig = z.infer; + +// Hook types that can be updated in-place +export const MUTABLE_HOOK_TYPE = [ + HookType.INTERCHAIN_GAS_PAYMASTER, + HookType.PROTOCOL_FEE, + HookType.ROUTING, + HookType.FALLBACK_ROUTING, + HookType.PAUSABLE, +]; diff --git a/typescript/sdk/src/ism/EvmIsmModule.hardhat-test.ts b/typescript/sdk/src/ism/EvmIsmModule.hardhat-test.ts index 54ccbf040..3c079d138 100644 --- a/typescript/sdk/src/ism/EvmIsmModule.hardhat-test.ts +++ b/typescript/sdk/src/ism/EvmIsmModule.hardhat-test.ts @@ -129,7 +129,7 @@ describe('EvmIsmModule', async () => { // example routing config exampleRoutingConfig = { type: IsmType.ROUTING, - owner: await multiProvider.getSignerAddress(chain), + owner: (await multiProvider.getSignerAddress(chain)).toLowerCase(), domains: Object.fromEntries( testChains .filter((c) => c !== TestChainName.test4) @@ -307,7 +307,6 @@ describe('EvmIsmModule', async () => { const numDomainsAfter = Object.keys( ((await ism.read()) as RoutingIsmConfig).domains, ).length; - console.log(numDomainsBefore, numDomainsAfter); expect(numDomainsBefore - 1).to.equal(numDomainsAfter); }); diff --git a/typescript/sdk/src/ism/EvmIsmModule.ts b/typescript/sdk/src/ism/EvmIsmModule.ts index ab492fd68..6be0b16b3 100644 --- a/typescript/sdk/src/ism/EvmIsmModule.ts +++ b/typescript/sdk/src/ism/EvmIsmModule.ts @@ -12,7 +12,6 @@ import { IMultisigIsm, IMultisigIsm__factory, IRoutingIsm, - MailboxClient__factory, OPStackIsm__factory, Ownable__factory, PausableIsm__factory, @@ -49,6 +48,7 @@ import { ChainName, ChainNameOrId } from '../types.js'; import { findMatchingLogEvents } from '../utils/logUtils.js'; import { EvmIsmReader } from './EvmIsmReader.js'; +import { IsmConfigSchema } from './schemas.js'; import { AggregationIsmConfig, DeployedIsm, @@ -89,6 +89,7 @@ export class EvmIsmModule extends HyperlaneModule< >, contractVerifier?: ContractVerifier, ) { + params.config = IsmConfigSchema.parse(params.config); super(params); this.reader = new EvmIsmReader(multiProvider, params.chain); @@ -129,6 +130,15 @@ export class EvmIsmModule extends HyperlaneModule< public async update( targetConfig: IsmConfig, ): Promise { + targetConfig = IsmConfigSchema.parse(targetConfig); + + // Do not support updating to a custom ISM address + if (typeof targetConfig === 'string') { + throw new Error( + 'Invalid targetConfig: Updating to a custom ISM address is not supported. Please provide a valid ISM configuration.', + ); + } + // save current config for comparison // normalize the config to ensure it's in a consistent format for comparison const currentConfig = normalizeConfig(await this.read()); @@ -136,18 +146,12 @@ export class EvmIsmModule extends HyperlaneModule< this.args.config = targetConfig; targetConfig = normalizeConfig(targetConfig); - // moduleMatchesConfig expects any domain filtering to have been done already - if ( - typeof targetConfig !== 'string' && - (targetConfig.type === IsmType.ROUTING || - targetConfig.type === IsmType.FALLBACK_ROUTING) - ) { - // filter for known domains - const { availableDomains } = this.filterRoutingIsmDomains({ - config: targetConfig, - }); - targetConfig.domains = availableDomains; - } + assert( + typeof targetConfig === 'object', + 'normalized targetConfig should be an object', + ); + + // if it's a fallback routing ISM, do a mailbox diff check // If configs match, no updates needed if (configDeepEquals(currentConfig, targetConfig)) { @@ -155,21 +159,11 @@ export class EvmIsmModule extends HyperlaneModule< } // Else, we have to figure out what an update for this ISM entails - - // If target config is an address ISM, just update the address - // if config -> address ISM, update address - // if address ISM -> address ISM, update address - if (typeof targetConfig === 'string') { - // TODO: https://github.com/hyperlane-xyz/hyperlane-monorepo/issues/3773 - this.args.addresses.deployedIsm = targetConfig; - return []; - } - // Check if we need to deploy a new ISM if ( - // if address ISM -> config, do a new deploy + // if updating from an address/custom config to a proper ISM config, do a new deploy typeof currentConfig === 'string' || - // if config -> config, AND types are different, do a new deploy + // if updating a proper ISM config whose types are different, do a new deploy currentConfig.type !== targetConfig.type || // if it is not a mutable ISM, do a new deploy !MUTABLE_ISM_TYPE.includes(targetConfig.type) @@ -195,33 +189,8 @@ export class EvmIsmModule extends HyperlaneModule< destination: this.chain, ismType: targetConfig.type, }); - const provider = this.multiProvider.getProvider(this.chain); - logger.debug(`Updating ${targetConfig.type} on ${this.chain}`); - // if it's a fallback routing ISM, do a mailbox diff check and deploy a new ISM if needed - if (targetConfig.type === IsmType.FALLBACK_ROUTING) { - // can only retrieve mailbox address if current ISM type is also Fallback Routing - const mailboxAddress = - currentConfig.type === IsmType.FALLBACK_ROUTING - ? await MailboxClient__factory.connect( - this.args.addresses.deployedIsm, - provider, - ).mailbox() - : ''; // empty string to force a mailbox diff - - // if mailbox delta, deploy new routing ISM before updating - // this will always be the case if the current ISM is not a fallback routing ISM - if (!eqAddress(mailboxAddress, this.args.addresses.mailbox)) { - const newIsm = await this.deployRoutingIsm({ - config: targetConfig, - logger, - }); - - this.args.addresses.deployedIsm = newIsm.address; - } - } - // if it's either of the routing ISMs, update their submodules let updateTxs: AnnotatedEV5Transaction[] = []; if ( @@ -236,6 +205,7 @@ export class EvmIsmModule extends HyperlaneModule< } // Lastly, check if the resolved owner is different from the current owner + const provider = this.multiProvider.getProvider(this.chain); const owner = await Ownable__factory.connect( this.args.addresses.deployedIsm, provider, @@ -301,6 +271,20 @@ export class EvmIsmModule extends HyperlaneModule< const routingIsmInterface = DomainRoutingIsm__factory.createInterface(); const updateTxs = []; + // filter out domains which are not part of the multiprovider + current = { + ...current, + domains: this.filterRoutingIsmDomains({ + config: current, + }).availableDomains, + }; + target = { + ...target, + domains: this.filterRoutingIsmDomains({ + config: target, + }).availableDomains, + }; + const { domainsToEnroll, domainsToUnenroll } = calculateDomainRoutingDelta( current, target, @@ -343,11 +327,13 @@ export class EvmIsmModule extends HyperlaneModule< return updateTxs; } - protected async deploy({ + protected async deploy({ config, }: { - config: C; + config: IsmConfig; }): Promise { + config = IsmConfigSchema.parse(config); + // If it's an address ISM, just return a base ISM if (typeof config === 'string') { // TODO: https://github.com/hyperlane-xyz/hyperlane-monorepo/issues/3773 @@ -463,11 +449,13 @@ export class EvmIsmModule extends HyperlaneModule< this.filterRoutingIsmDomains({ config, }); - config.domains = availableDomains; + config = { + ...config, + domains: availableDomains, + }; // deploy the submodules first const submoduleAddresses: Address[] = []; - for (const origin of Object.keys(config.domains)) { const { address } = await this.deploy({ config: config.domains[origin], @@ -587,26 +575,6 @@ export class EvmIsmModule extends HyperlaneModule< return IAggregationIsm__factory.connect(address, signer); } - // Updates the mailbox address if it is different from the current one. - // Logs changes and updates the internal state of the module. - public setNewMailbox(newMailboxAddress: Address): void { - const currentMailboxAddress = this.args.addresses.mailbox; - - if (currentMailboxAddress === newMailboxAddress) { - this.logger.debug( - `Mailbox address is already set to ${newMailboxAddress}`, - ); - return; - } - - this.logger.debug( - `Setting new mailbox address from ${currentMailboxAddress} to ${newMailboxAddress}`, - ); - - // Update the mailbox address in the arguments - this.args.addresses.mailbox = newMailboxAddress; - } - // filtering out domains which are not part of the multiprovider private filterRoutingIsmDomains({ config }: { config: RoutingIsmConfig }) { const availableDomainIds: number[] = []; diff --git a/typescript/sdk/src/test/testUtils.ts b/typescript/sdk/src/test/testUtils.ts index 35b711823..5a2d4f2ba 100644 --- a/typescript/sdk/src/test/testUtils.ts +++ b/typescript/sdk/src/test/testUtils.ts @@ -17,7 +17,7 @@ export function randomInt(max: number, min = 0): number { } export function randomAddress(): Address { - return ethers.utils.hexlify(ethers.utils.randomBytes(20)); + return ethers.utils.hexlify(ethers.utils.randomBytes(20)).toLowerCase(); } export function createRouterConfigMap(