feat: add routing delta for more granular routingIsm config updates (#3017)

### Description

- add support for `routingIsmDelta` which filters out the
incompatibility between the onchain deployed config and the desired
config.
- based on the above, you either update the deployed Ism with new
routes, delete old routes, change owners, etc.
- moduleMatchesConfig uses the same

### Drive-by changes

- checking exclusion of domains in the onchain deployed config

### Related issues

- fixes https://github.com/hyperlane-xyz/issues/issues/818

### Backward compatibility

Yes

### Testing

Unit tests

---------

Co-authored-by: Yorke Rhodes <yorke@hyperlane.xyz>
pull/3058/head
Kunal Arora 11 months ago committed by GitHub
parent b832e57aef
commit 7919417ecd
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 8
      .changeset/breezy-bats-type.md
  2. 40
      solidity/test/testSendReceiver.test.ts
  3. 279
      typescript/sdk/src/ism/HyperlaneIsmFactory.hardhat-test.ts
  4. 293
      typescript/sdk/src/ism/HyperlaneIsmFactory.ts
  5. 10
      typescript/sdk/src/ism/types.ts

@ -0,0 +1,8 @@
---
'@hyperlane-xyz/sdk': patch
---
Granular control of updating predeployed routingIsms based on routing config mismatch
- Add support for routingIsmDelta which filters out the incompatibility between the onchain deployed config and the desired config.
- Based on the above, you either update the deployed Ism with new routes, delete old routes, change owners, etc.
- `moduleMatchesConfig` uses the same

@ -1,40 +0,0 @@
import { expect } from 'chai';
import { ethers } from 'hardhat';
import { addressToBytes32 } from '@hyperlane-xyz/utils';
import { TestSendReceiver__factory } from '../types';
describe('TestSendReceiver', () => {
it('randomly handles a message', async () => {
const [signer] = await ethers.getSigners();
const signerAddress = await signer.getAddress();
const recipientFactory = new TestSendReceiver__factory(signer);
const recipient = await recipientFactory.deploy();
// Didn't know how else to test the randomness
let successes = 0;
let failures = 0;
for (let i = 0; i < 100; i++) {
try {
// "Inject randomness"
await signer.sendTransaction({
from: signerAddress,
to: signerAddress,
value: 1,
});
await recipient.handle(
0,
addressToBytes32(recipient.address),
'0x1234',
);
successes += 1;
} catch (error) {
failures += 1;
}
}
expect(successes).to.be.greaterThan(5);
expect(failures).to.be.greaterThan(1);
});
});

@ -1,7 +1,7 @@
import { expect } from 'chai';
import { ethers } from 'hardhat';
import { error } from '@hyperlane-xyz/utils';
import { Address, error } from '@hyperlane-xyz/utils';
import { TestChains } from '../consts/chains';
import { TestCoreApp } from '../core/TestCoreApp';
@ -76,21 +76,37 @@ const randomIsmConfig = (depth = 0, maxDepth = 2): IsmConfig => {
describe('HyperlaneIsmFactory', async () => {
let ismFactory: HyperlaneIsmFactory;
let coreApp: TestCoreApp;
let multiProvider: MultiProvider;
let exampleRoutingConfig: RoutingIsmConfig;
let mailboxAddress: Address, newMailboxAddress: Address;
const chain = 'test1';
before(async () => {
beforeEach(async () => {
const [signer] = await ethers.getSigners();
const multiProvider = MultiProvider.createTestMultiProvider({ signer });
multiProvider = MultiProvider.createTestMultiProvider({ signer });
const ismFactoryDeployer = new HyperlaneProxyFactoryDeployer(multiProvider);
ismFactory = new HyperlaneIsmFactory(
await ismFactoryDeployer.deploy(multiProvider.mapKnownChains(() => ({}))),
multiProvider,
);
const coreDeployer = new TestCoreDeployer(multiProvider, ismFactory);
let coreDeployer = new TestCoreDeployer(multiProvider, ismFactory);
coreApp = await coreDeployer.deployApp();
mailboxAddress = coreApp.getContracts(chain).mailbox.address;
coreDeployer = new TestCoreDeployer(multiProvider, ismFactory);
coreApp = await coreDeployer.deployApp();
newMailboxAddress = coreApp.getContracts(chain).mailbox.address;
exampleRoutingConfig = {
type: IsmType.ROUTING,
owner: await multiProvider.getSignerAddress(chain),
domains: Object.fromEntries(
TestChains.filter((c) => c !== 'test1').map((c) => [
c,
randomMultisigIsmConfig(3, 5),
]),
),
};
});
it('deploys a simple ism', async () => {
@ -136,47 +152,218 @@ describe('HyperlaneIsmFactory', async () => {
});
}
it('deploys routingIsm with correct routes', async () => {
const config: RoutingIsmConfig = {
type: IsmType.ROUTING,
owner: randomAddress(),
domains: Object.fromEntries(
TestChains.map((c) => [c, randomIsmConfig()]),
),
};
const ism = await ismFactory.deploy({ destination: chain, config });
const matches = await moduleMatchesConfig(
chain,
ism.address,
config,
ismFactory.multiProvider,
ismFactory.getContracts(chain),
);
expect(matches).to.be.true;
});
for (const type of [IsmType.ROUTING, IsmType.FALLBACK_ROUTING]) {
it(`deploys ${type} routingIsm with correct routes`, async () => {
exampleRoutingConfig.type = type as
| IsmType.ROUTING
| IsmType.FALLBACK_ROUTING;
const ism = await ismFactory.deploy({
destination: chain,
config: exampleRoutingConfig,
mailbox: mailboxAddress,
});
const matches = await moduleMatchesConfig(
chain,
ism.address,
exampleRoutingConfig,
ismFactory.multiProvider,
ismFactory.getContracts(chain),
mailboxAddress,
);
expect(matches).to.be.true;
});
it('deploys defaultFallbackRoutingIsm with correct routes and fallback to mailbox', async () => {
const config: RoutingIsmConfig = {
type: IsmType.FALLBACK_ROUTING,
owner: randomAddress(),
domains: Object.fromEntries(
TestChains.map((c) => [c, randomIsmConfig()]),
),
};
const mailbox = await coreApp.getContracts(chain).mailbox;
const ism = await ismFactory.deploy({
it(`update route in an existing ${type}`, async () => {
exampleRoutingConfig.type = type as
| IsmType.ROUTING
| IsmType.FALLBACK_ROUTING;
let matches = true;
let ism = await ismFactory.deploy({
destination: chain,
config: exampleRoutingConfig,
mailbox: mailboxAddress,
});
const existingIsm = ism.address;
// changing the type of a domain should enroll the domain
(exampleRoutingConfig.domains['test2'] as MultisigIsmConfig).type =
IsmType.MESSAGE_ID_MULTISIG;
ism = await ismFactory.deploy({
destination: chain,
config: exampleRoutingConfig,
existingIsmAddress: ism.address,
mailbox: mailboxAddress,
});
matches =
matches &&
existingIsm === ism.address &&
(await moduleMatchesConfig(
chain,
ism.address,
exampleRoutingConfig,
ismFactory.multiProvider,
ismFactory.getContracts(chain),
mailboxAddress,
));
expect(matches).to.be.true;
});
it(`deletes route in an existing ${type}`, async () => {
exampleRoutingConfig.type = type as
| IsmType.ROUTING
| IsmType.FALLBACK_ROUTING;
let matches = true;
let ism = await ismFactory.deploy({
destination: chain,
config: exampleRoutingConfig,
mailbox: mailboxAddress,
});
const existingIsm = ism.address;
// deleting the domain should unenroll the domain
delete exampleRoutingConfig.domains['test3'];
ism = await ismFactory.deploy({
destination: chain,
config: exampleRoutingConfig,
existingIsmAddress: ism.address,
mailbox: mailboxAddress,
});
matches =
matches &&
existingIsm == ism.address &&
(await moduleMatchesConfig(
chain,
ism.address,
exampleRoutingConfig,
ismFactory.multiProvider,
ismFactory.getContracts(chain),
mailboxAddress,
));
expect(matches).to.be.true;
});
it(`updates owner in an existing ${type}`, async () => {
exampleRoutingConfig.type = type as
| IsmType.ROUTING
| IsmType.FALLBACK_ROUTING;
let matches = true;
let ism = await ismFactory.deploy({
destination: chain,
config: exampleRoutingConfig,
mailbox: mailboxAddress,
});
const existingIsm = ism.address;
// change the owner
exampleRoutingConfig.owner = randomAddress();
ism = await ismFactory.deploy({
destination: chain,
config: exampleRoutingConfig,
existingIsmAddress: ism.address,
mailbox: mailboxAddress,
});
matches =
matches &&
existingIsm == ism.address &&
(await moduleMatchesConfig(
chain,
ism.address,
exampleRoutingConfig,
ismFactory.multiProvider,
ismFactory.getContracts(chain),
mailboxAddress,
));
expect(matches).to.be.true;
});
it(`no changes to an existing ${type} means no redeployment or updates`, async () => {
exampleRoutingConfig.type = type as
| IsmType.ROUTING
| IsmType.FALLBACK_ROUTING;
let matches = true;
let ism = await ismFactory.deploy({
destination: chain,
config: exampleRoutingConfig,
mailbox: mailboxAddress,
});
const existingIsm = ism.address;
// using the same config should not change anything
ism = await ismFactory.deploy({
destination: chain,
config: exampleRoutingConfig,
existingIsmAddress: ism.address,
mailbox: mailboxAddress,
});
matches =
matches &&
existingIsm === ism.address &&
(await moduleMatchesConfig(
chain,
ism.address,
exampleRoutingConfig,
ismFactory.multiProvider,
ismFactory.getContracts(chain),
mailboxAddress,
));
expect(matches).to.be.true;
});
it(`redeploy same config if the deployer doesn't have ownership of ${type}`, async () => {
exampleRoutingConfig.type = type as
| IsmType.ROUTING
| IsmType.FALLBACK_ROUTING;
let matches = true;
exampleRoutingConfig.owner = randomAddress();
let ism = await ismFactory.deploy({
destination: chain,
config: exampleRoutingConfig,
mailbox: mailboxAddress,
});
const existingIsm = ism.address;
ism = await ismFactory.deploy({
destination: chain,
config: exampleRoutingConfig,
existingIsmAddress: ism.address,
mailbox: mailboxAddress,
});
matches =
matches &&
existingIsm !== ism.address &&
(await moduleMatchesConfig(
chain,
ism.address,
exampleRoutingConfig,
ismFactory.multiProvider,
ismFactory.getContracts(chain),
mailboxAddress,
));
expect(matches).to.be.true;
});
}
it(`redeploy same config if the mailbox address changes for defaultFallbackRoutingIsm`, async () => {
exampleRoutingConfig.type = IsmType.FALLBACK_ROUTING;
let matches = true;
let ism = await ismFactory.deploy({
destination: chain,
config,
mailbox: mailbox.address,
}); // not through an actual factory just for maintaining consistency in naming
const matches = await moduleMatchesConfig(
chain,
ism.address,
config,
ismFactory.multiProvider,
ismFactory.getContracts(chain),
mailbox.address,
);
config: exampleRoutingConfig,
mailbox: mailboxAddress,
});
const existingIsm = ism.address;
ism = await ismFactory.deploy({
destination: chain,
config: exampleRoutingConfig,
existingIsmAddress: ism.address,
mailbox: newMailboxAddress,
});
matches =
matches &&
existingIsm !== ism.address &&
(await moduleMatchesConfig(
chain,
ism.address,
exampleRoutingConfig,
ismFactory.multiProvider,
ismFactory.getContracts(chain),
newMailboxAddress,
));
expect(matches).to.be.true;
});
});

@ -21,7 +21,12 @@ import {
StaticThresholdAddressSetFactory,
TestIsm__factory,
} from '@hyperlane-xyz/core';
import { Address, eqAddress, formatMessage, warn } from '@hyperlane-xyz/utils';
import {
Address,
eqAddress,
formatMessage,
normalizeAddress,
} from '@hyperlane-xyz/utils';
import { HyperlaneApp } from '../app/HyperlaneApp';
import {
@ -34,6 +39,7 @@ import {
ProxyFactoryFactories,
proxyFactoryFactories,
} from '../deploy/contracts';
import { logger } from '../logger';
import { MultiProvider } from '../providers/MultiProvider';
import { ChainMap, ChainName } from '../types';
@ -47,6 +53,7 @@ import {
MultisigIsmConfig,
OpStackIsmConfig,
RoutingIsmConfig,
RoutingIsmDelta,
ismTypeToModuleType,
} from './types';
@ -89,8 +96,9 @@ export class HyperlaneIsmFactory extends HyperlaneApp<ProxyFactoryFactories> {
config: C;
origin?: ChainName;
mailbox?: Address;
existingIsmAddress?: Address;
}): Promise<DeployedIsm> {
const { destination, config, origin, mailbox } = params;
const { destination, config, origin, mailbox, existingIsmAddress } = params;
if (typeof config === 'string') {
// @ts-ignore
return IInterchainSecurityModule__factory.connect(
@ -119,6 +127,7 @@ export class HyperlaneIsmFactory extends HyperlaneApp<ProxyFactoryFactories> {
config,
origin,
mailbox,
existingIsmAddress,
});
break;
case IsmType.AGGREGATION:
@ -186,77 +195,151 @@ export class HyperlaneIsmFactory extends HyperlaneApp<ProxyFactoryFactories> {
config: RoutingIsmConfig;
origin?: ChainName;
mailbox?: Address;
existingIsmAddress?: Address;
}): Promise<IRoutingIsm> {
const { destination, config, mailbox } = params;
const routingIsmFactory = this.getContracts(destination).routingIsmFactory;
const isms: ChainMap<Address> = {};
for (const origin of Object.keys(config.domains)) {
const ism = await this.deploy({
destination,
config: config.domains[origin],
origin,
mailbox,
});
isms[origin] = ism.address;
}
const domains = Object.keys(isms).map((chain) =>
this.multiProvider.getDomainId(chain),
);
const submoduleAddresses = Object.values(isms);
const { destination, config, mailbox, existingIsmAddress } = params;
const overrides = this.multiProvider.getTransactionOverrides(destination);
let receipt: ethers.providers.TransactionReceipt;
const routingIsmFactory = this.getContracts(destination).routingIsmFactory;
let routingIsm: DomainRoutingIsm | DefaultFallbackRoutingIsm;
if (config.type === IsmType.FALLBACK_ROUTING) {
if (!mailbox) {
throw new Error(
'Mailbox address is required for deploying fallback routing ISM',
const delta: RoutingIsmDelta = existingIsmAddress
? await routingModuleDelta(
destination,
existingIsmAddress,
config,
this.multiProvider,
this.getContracts(destination),
mailbox,
)
: {
domainsToUnenroll: [],
domainsToEnroll: Object.keys(config.domains),
};
const signer = this.multiProvider.getSigner(destination);
const provider = this.multiProvider.getProvider(destination);
let isOwner = false;
if (existingIsmAddress) {
const owner = await DomainRoutingIsm__factory.connect(
existingIsmAddress,
provider,
).owner();
isOwner = eqAddress(await signer.getAddress(), owner);
}
// reconfiguring existing routing ISM
if (existingIsmAddress && isOwner && !delta.mailbox) {
const isms: ChainMap<Address> = {};
routingIsm = DomainRoutingIsm__factory.connect(
existingIsmAddress,
this.multiProvider.getSigner(destination),
);
// deploying all the ISMs which have to be updated
for (const origin of delta.domainsToEnroll) {
logger(
`Reconfiguring preexisting routing ISM at for origin ${origin}...`,
);
const ism = await this.deploy({
destination,
config: config.domains[origin],
origin,
mailbox,
});
isms[origin] = ism.address;
const tx = await routingIsm.set(
this.multiProvider.getDomainId(origin),
isms[origin],
overrides,
);
await this.multiProvider.handleTx(destination, tx);
}
debug('Deploying fallback routing ISM ...');
routingIsm = await this.multiProvider.handleDeploy(
destination,
new DefaultFallbackRoutingIsm__factory(),
[mailbox],
// unenrolling domains if needed
for (const origin of delta.domainsToUnenroll) {
logger(
`Unenrolling origin ${origin} from preexisting routing ISM at ${existingIsmAddress}...`,
);
const tx = await routingIsm.remove(
this.multiProvider.getDomainId(origin),
overrides,
);
await this.multiProvider.handleTx(destination, tx);
}
// transfer ownership if needed
if (delta.owner) {
logger(`Transferring ownership of routing ISM...`);
const tx = await routingIsm.transferOwnership(delta.owner, overrides);
await this.multiProvider.handleTx(destination, tx);
}
} else {
const isms: ChainMap<Address> = {};
for (const origin of Object.keys(config.domains)) {
const ism = await this.deploy({
destination,
config: config.domains[origin],
origin,
mailbox,
});
isms[origin] = ism.address;
}
const submoduleAddresses = Object.values(isms);
const domains = Object.keys(isms).map((chain) =>
this.multiProvider.getDomainId(chain),
);
debug('Initialising fallback routing ISM ...');
receipt = await this.multiProvider.handleTx(
destination,
routingIsm['initialize(address,uint32[],address[])'](
let receipt: ethers.providers.TransactionReceipt;
if (config.type === IsmType.FALLBACK_ROUTING) {
// deploying new fallback routing ISM
if (!mailbox) {
throw new Error(
'Mailbox address is required for deploying fallback routing ISM',
);
}
logger('Deploying fallback routing ISM ...');
routingIsm = await this.multiProvider.handleDeploy(
destination,
new DefaultFallbackRoutingIsm__factory(),
[mailbox],
);
logger('Initialising fallback routing ISM ...');
receipt = await this.multiProvider.handleTx(
destination,
routingIsm['initialize(address,uint32[],address[])'](
config.owner,
domains,
submoduleAddresses,
overrides,
),
);
} else {
// deploying new domain routing ISM
const tx = await routingIsmFactory.deploy(
config.owner,
domains,
submoduleAddresses,
overrides,
),
);
} else {
const tx = await routingIsmFactory.deploy(
config.owner,
domains,
submoduleAddresses,
overrides,
);
receipt = await this.multiProvider.handleTx(destination, tx);
// TODO: Break this out into a generalized function
const dispatchLogs = receipt.logs
.map((log) => {
try {
return routingIsmFactory.interface.parseLog(log);
} catch (e) {
return undefined;
}
})
.filter(
(log): log is ethers.utils.LogDescription =>
!!log && log.name === 'ModuleDeployed',
);
const moduleAddress = dispatchLogs[0].args['module'];
routingIsm = DomainRoutingIsm__factory.connect(
moduleAddress,
this.multiProvider.getSigner(destination),
);
receipt = await this.multiProvider.handleTx(destination, tx);
// TODO: Break this out into a generalized function
const dispatchLogs = receipt.logs
.map((log) => {
try {
return routingIsmFactory.interface.parseLog(log);
} catch (e) {
return undefined;
}
})
.filter(
(log): log is ethers.utils.LogDescription =>
!!log && log.name === 'ModuleDeployed',
);
if (dispatchLogs.length === 0) {
throw new Error('No ModuleDeployed event found');
}
const moduleAddress = dispatchLogs[0].args['module'];
routingIsm = DomainRoutingIsm__factory.connect(
moduleAddress,
this.multiProvider.getSigner(destination),
);
}
}
return routingIsm;
}
@ -410,7 +493,7 @@ export async function moduleCanCertainlyVerify(
throw new Error(`Unsupported module type: ${moduleType}`);
}
} catch (e) {
warn(`Error checking module ${destModule}: ${e}`);
logger(`Error checking module ${destModule}: ${e}`);
return false;
}
} else {
@ -523,22 +606,20 @@ export async function moduleMatchesConfig(
mailbox !== undefined &&
eqAddress(mailboxAddress, mailbox);
}
// Recursively check that the submodule for each configured
// domain matches the submodule config.
for (const [origin, subConfig] of Object.entries(config.domains)) {
const subModule = await routingIsm.module(
multiProvider.getDomainId(origin),
);
const subModuleMatches = await moduleMatchesConfig(
chain,
subModule,
subConfig,
multiProvider,
contracts,
mailbox,
);
matches = matches && subModuleMatches;
}
const delta = await routingModuleDelta(
chain,
moduleAddress,
config,
multiProvider,
contracts,
mailbox,
);
matches =
matches &&
delta.domainsToEnroll.length === 0 &&
delta.domainsToUnenroll.length === 0 &&
!delta.mailbox &&
!delta.owner;
break;
}
case IsmType.AGGREGATION: {
@ -596,6 +677,62 @@ export async function moduleMatchesConfig(
return matches;
}
export async function routingModuleDelta(
destination: ChainName,
moduleAddress: Address,
config: RoutingIsmConfig,
multiProvider: MultiProvider,
contracts: HyperlaneContracts<ProxyFactoryFactories>,
mailbox?: Address,
): Promise<RoutingIsmDelta> {
const provider = multiProvider.getProvider(destination);
const routingIsm = DomainRoutingIsm__factory.connect(moduleAddress, provider);
const owner = await routingIsm.owner();
const deployedDomains = (await routingIsm.domains()).map((domain) =>
multiProvider.getChainName(domain.toNumber()),
);
const delta: RoutingIsmDelta = {
domainsToUnenroll: [],
domainsToEnroll: [],
};
// if owners don't match, we need to transfer ownership
if (!eqAddress(owner, normalizeAddress(config.owner)))
delta.owner = config.owner;
if (config.type === IsmType.FALLBACK_ROUTING) {
const client = MailboxClient__factory.connect(moduleAddress, provider);
const mailboxAddress = await client.mailbox();
if (mailbox && !eqAddress(mailboxAddress, mailbox)) delta.mailbox = mailbox;
}
// check for exclusion of domains in the config
delta.domainsToUnenroll = deployedDomains.filter(
(domain) => !Object.keys(config.domains).includes(domain),
);
// check for inclusion of domains in the config
for (const [origin, subConfig] of Object.entries(config.domains)) {
if (!deployedDomains.includes(origin)) {
delta.domainsToEnroll.push(origin);
} else {
const subModule = await routingIsm.module(
multiProvider.getDomainId(origin),
);
// Recursively check that the submodule for each configured
// domain matches the submodule config.
const subModuleMatches = await moduleMatchesConfig(
destination,
subModule,
subConfig,
multiProvider,
contracts,
origin,
);
if (!subModuleMatches) delta.domainsToEnroll.push(origin);
}
}
return delta;
}
export function collectValidators(
origin: ChainName,
config: IsmConfig,

@ -7,7 +7,7 @@ import {
} from '@hyperlane-xyz/core';
import type { Address, ValueOf } from '@hyperlane-xyz/utils';
import { ChainMap } from '../types';
import { ChainMap, ChainName } from '../types';
// this enum should match the IInterchainSecurityModule.sol enum
// meant for the relayer
@ -103,3 +103,11 @@ export type DeployedIsmType = {
};
export type DeployedIsm = ValueOf<DeployedIsmType>;
// for finding the difference between the onchain deployment and the config provided
export type RoutingIsmDelta = {
domainsToUnenroll: ChainName[]; // new or updated isms for the domain
domainsToEnroll: ChainName[]; // isms to remove
owner?: Address; // is the owner different
mailbox?: Address; // is the mailbox different (only for fallback routing)
};

Loading…
Cancel
Save