Skip to content

Commit

Permalink
feat(sdk): Sort ISM config arrays (#4436)
Browse files Browse the repository at this point in the history 
### Description

- Introduce sorting validator arrays and modules by type for ISM in the
`normalizeConfig` method
- This will avoid deploying a new version of the same ISM (in practice)
for updates
- Sort staticAddressSets by value before deploying
- This will enable us to use ISM recovery
- This will also avoid raising ISM mismatch violations for our checker
tooling

### Testing

<!--
What kind of testing have these changes undergone?

None/Manual/Unit Tests
-->

Unit Tests
  • Loading branch information
@Mo-Hussain
Mo-Hussain committed Sep 11, 2024
1 parent 085e7d0 commit 2a282ec
Show file tree
Hide file tree
Showing 13 changed files with 291 additions and 56 deletions.
5 changes: 5 additions & 0 deletions .changeset/chilled-coats-boil.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@hyperlane-xyz/utils': minor
---

Add sortArraysInConfig method, normalizeConfig implementation to call sortArraysInConfig after current behavior
5 changes: 5 additions & 0 deletions .changeset/tender-fishes-sniff.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@hyperlane-xyz/sdk': minor
---

Sort values in EvmModuleDeployer.deployStaticAddressSet
3 changes: 2 additions & 1 deletion typescript/sdk/src/core/EvmCoreModule.hardhat-test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,14 @@ import {
TimelockController__factory,
ValidatorAnnounce__factory,
} from '@hyperlane-xyz/core';
import { normalizeConfig, objMap } from '@hyperlane-xyz/utils';
import { objMap } from '@hyperlane-xyz/utils';

import { TestChainName } from '../consts/testChains.js';
import { IsmConfig, IsmType } from '../ism/types.js';
import { MultiProvider } from '../providers/MultiProvider.js';
import { AnnotatedEV5Transaction } from '../providers/ProviderType.js';
import { randomAddress, testCoreConfig } from '../test/testUtils.js';
import { normalizeConfig } from '../utils/ism.js';

import { EvmCoreModule } from './EvmCoreModule.js';
import { CoreConfig } from './types.js';
Expand Down
22 changes: 14 additions & 8 deletions typescript/sdk/src/deploy/EvmModuleDeployer.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -264,34 +264,40 @@ export class EvmModuleDeployer<Factories extends HyperlaneFactories> {
threshold?: number;
multiProvider: MultiProvider;
}): Promise<Address> {
const sortedValues = [...values].sort();

const address = await factory['getAddress(address[],uint8)'](
values,
sortedValues,
threshold,
);
const code = await multiProvider.getProvider(chain).getCode(address);
if (code === '0x') {
logger.debug(
`Deploying new ${threshold} of ${values.length} address set to ${chain}`,
`Deploying new ${threshold} of ${sortedValues.length} address set to ${chain}`,
);
const overrides = multiProvider.getTransactionOverrides(chain);

// estimate gas
const estimatedGas = await factory.estimateGas['deploy(address[],uint8)'](
values,
sortedValues,
threshold,
overrides,
);

// add 10% buffer
const hash = await factory['deploy(address[],uint8)'](values, threshold, {
...overrides,
gasLimit: estimatedGas.add(estimatedGas.div(10)), // 10% buffer
});
const hash = await factory['deploy(address[],uint8)'](
sortedValues,
threshold,
{
...overrides,
gasLimit: estimatedGas.add(estimatedGas.div(10)), // 10% buffer
},
);

await multiProvider.handleTx(chain, hash);
} else {
logger.debug(
`Recovered ${threshold} of ${values.length} address set on ${chain}: ${address}`,
`Recovered ${threshold} of ${sortedValues.length} address set on ${chain}: ${address}`,
);
}

Expand Down
9 changes: 2 additions & 7 deletions typescript/sdk/src/hook/EvmHookModule.hardhat-test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,7 @@ import { expect } from 'chai';
import { Signer } from 'ethers';
import hre from 'hardhat';

import {
Address,
assert,
deepEquals,
eqAddress,
normalizeConfig,
} from '@hyperlane-xyz/utils';
import { Address, assert, deepEquals, eqAddress } from '@hyperlane-xyz/utils';

import { TestChainName, testChains } from '../consts/testChains.js';
import { HyperlaneAddresses, HyperlaneContracts } from '../contracts/types.js';
Expand All @@ -20,6 +14,7 @@ import { ProxyFactoryFactories } from '../deploy/contracts.js';
import { HyperlaneIsmFactory } from '../ism/HyperlaneIsmFactory.js';
import { MultiProvider } from '../providers/MultiProvider.js';
import { randomAddress, randomInt } from '../test/testUtils.js';
import { normalizeConfig } from '../utils/ism.js';

import { EvmHookModule } from './EvmHookModule.js';
import {
Expand Down
2 changes: 1 addition & 1 deletion typescript/sdk/src/hook/EvmHookModule.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ import {
addressToBytes32,
deepEquals,
eqAddress,
normalizeConfig,
rootLogger,
} from '@hyperlane-xyz/utils';

Expand All @@ -51,6 +50,7 @@ import { ArbL2ToL1IsmConfig, IsmType, OpStackIsmConfig } from '../ism/types.js';
import { MultiProvider } from '../providers/MultiProvider.js';
import { AnnotatedEV5Transaction } from '../providers/ProviderType.js';
import { ChainNameOrId } from '../types.js';
import { normalizeConfig } from '../utils/ism.js';

import { EvmHookReader } from './EvmHookReader.js';
import { DeployedHook, HookFactories, hookFactories } from './contracts.js';
Expand Down
214 changes: 199 additions & 15 deletions typescript/sdk/src/ism/EvmIsmModule.hardhat-test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { expect } from 'chai';
import { Signer } from 'ethers';
import hre from 'hardhat';

import { Address, eqAddress, normalizeConfig } from '@hyperlane-xyz/utils';
import { Address, eqAddress } from '@hyperlane-xyz/utils';

import { TestChainName, testChains } from '../consts/testChains.js';
import { HyperlaneAddresses, HyperlaneContracts } from '../contracts/types.js';
Expand All @@ -13,6 +13,7 @@ import { HyperlaneProxyFactoryDeployer } from '../deploy/HyperlaneProxyFactoryDe
import { ProxyFactoryFactories } from '../deploy/contracts.js';
import { MultiProvider } from '../providers/MultiProvider.js';
import { randomAddress, randomInt } from '../test/testUtils.js';
import { normalizeConfig } from '../utils/ism.js';

import { EvmIsmModule } from './EvmIsmModule.js';
import { HyperlaneIsmFactory } from './HyperlaneIsmFactory.js';
Expand All @@ -36,19 +37,27 @@ const randomMultisigIsmConfig = (m: number, n: number): MultisigIsmConfig => {
};
};

const ModuleTypes = [
ModuleType.AGGREGATION,
ModuleType.MERKLE_ROOT_MULTISIG,
ModuleType.ROUTING,
ModuleType.NULL,
];

const NonNestedModuleTypes = [ModuleType.MERKLE_ROOT_MULTISIG, ModuleType.NULL];

function randomModuleType(): ModuleType {
const choices = [
ModuleType.AGGREGATION,
ModuleType.MERKLE_ROOT_MULTISIG,
ModuleType.ROUTING,
ModuleType.NULL,
];
return choices[randomInt(choices.length)];
return ModuleTypes[randomInt(ModuleTypes.length)];
}

function randomNonNestedModuleType(): ModuleType {
return NonNestedModuleTypes[randomInt(NonNestedModuleTypes.length)];
}

const randomIsmConfig = (depth = 0, maxDepth = 2): IsmConfig => {
const randomIsmConfig = (depth = 0, maxDepth = 2) => {
const moduleType =
depth == maxDepth ? ModuleType.MERKLE_ROOT_MULTISIG : randomModuleType();
depth === maxDepth ? randomNonNestedModuleType() : randomModuleType();

switch (moduleType) {
case ModuleType.MERKLE_ROOT_MULTISIG: {
const n = randomInt(5, 1);
Expand All @@ -65,10 +74,21 @@ const randomIsmConfig = (depth = 0, maxDepth = 2): IsmConfig => {
return config;
}
case ModuleType.AGGREGATION: {
const n = randomInt(5, 1);
const modules = new Array<number>(n)
.fill(0)
.map(() => randomIsmConfig(depth + 1));
const n = randomInt(2, 1);
const moduleTypes = new Set();
const modules = new Array<number>(n).fill(0).map(() => {
let moduleConfig;
let moduleType;

// Ensure that we do not add the same module type more than once per level
do {
moduleConfig = randomIsmConfig(depth + 1, maxDepth);
moduleType = moduleConfig.type;
} while (moduleTypes.has(moduleType));

moduleTypes.add(moduleType);
return moduleConfig;
});
const config: AggregationIsmConfig = {
type: IsmType.AGGREGATION,
threshold: randomInt(n, 1),
Expand Down Expand Up @@ -176,8 +196,10 @@ describe('EvmIsmModule', async () => {

// expect that the ISM matches the config after all tests
afterEach(async () => {
const normalizedDerivedConfig = normalizeConfig(await testIsm.read());
const derivedConfiig = await testIsm.read();
const normalizedDerivedConfig = normalizeConfig(derivedConfiig);
const normalizedConfig = normalizeConfig(testConfig);

assert.deepStrictEqual(normalizedDerivedConfig, normalizedConfig);
});

Expand Down Expand Up @@ -347,6 +369,74 @@ describe('EvmIsmModule', async () => {
.true;
});

it(`reordering validators in an existing ${type} should not trigger a redeployment`, async () => {
// create a new ISM
const routerConfig = {
type: IsmType.ROUTING,
owner: (await multiProvider.getSignerAddress(chain)).toLowerCase(),
domains: {
test1: {
type: IsmType.MERKLE_ROOT_MULTISIG,
validators: [
'0x5FbDB2315678afecb367f032d93F642f64180aa3',
'0xAb8483F64d9C6d1EcF9b849Ae677dD3315835cb2',
'0x4B20993Bc481177ec7E8f571ceCaE8A9e22C02db',
],
threshold: 2,
},
test2: {
type: IsmType.MERKLE_ROOT_MULTISIG,
validators: [
'0x5FbDB2315678afecb367f032d93F642f64180aa3',
'0x4B20993Bc481177ec7E8f571ceCaE8A9e22C02db',
'0xAb8483F64d9C6d1EcF9b849Ae677dD3315835cb2',
],
threshold: 2,
},
},
};

const { ism, initialIsmAddress } = await createIsm(
routerConfig as RoutingIsmConfig,
);

const updatedRouterConfig = {
type: IsmType.ROUTING,
owner: (await multiProvider.getSignerAddress(chain)).toLowerCase(),
domains: {
test1: {
type: IsmType.MERKLE_ROOT_MULTISIG,
validators: [
'0xAb8483F64d9C6d1EcF9b849Ae677dD3315835cb2',
'0x4B20993Bc481177ec7E8f571ceCaE8A9e22C02db',
'0x5FbDB2315678afecb367f032d93F642f64180aa3',
],
threshold: 2,
},
test2: {
type: IsmType.MERKLE_ROOT_MULTISIG,
validators: [
'0x4B20993Bc481177ec7E8f571ceCaE8A9e22C02db',
'0xAb8483F64d9C6d1EcF9b849Ae677dD3315835cb2',
'0x5FbDB2315678afecb367f032d93F642f64180aa3',
],
threshold: 2,
},
},
};

// expect 0 updates
await expectTxsAndUpdate(
ism,
updatedRouterConfig as RoutingIsmConfig,
0,
);

// expect the ISM address to be the same
expect(eqAddress(initialIsmAddress, ism.serialize().deployedIsm)).to.be
.true;
});

it(`update owner in an existing ${type} not owned by deployer`, async () => {
// ISM owner is not the deployer
exampleRoutingConfig.owner = randomAddress();
Expand Down Expand Up @@ -435,5 +525,99 @@ describe('EvmIsmModule', async () => {
.true;
});
}

it(`reordering modules in an existing staticAggregationIsm should not trigger a redeployment`, async () => {
// create a new ISM
const config: AggregationIsmConfig = {
type: IsmType.AGGREGATION,
modules: [
{
type: IsmType.MERKLE_ROOT_MULTISIG,
validators: [
'0x5FbDB2315678afecb367f032d93F642f64180aa3',
'0xAb8483F64d9C6d1EcF9b849Ae677dD3315835cb2',
'0x4B20993Bc481177ec7E8f571ceCaE8A9e22C02db',
],
threshold: 2,
},
{
type: IsmType.ROUTING,
owner: (await multiProvider.getSignerAddress(chain)).toLowerCase(),
domains: {
test1: {
type: IsmType.MERKLE_ROOT_MULTISIG,
validators: [
'0x5FbDB2315678afecb367f032d93F642f64180aa3',
'0xAb8483F64d9C6d1EcF9b849Ae677dD3315835cb2',
'0x4B20993Bc481177ec7E8f571ceCaE8A9e22C02db',
],
threshold: 2,
},
test2: {
type: IsmType.MERKLE_ROOT_MULTISIG,
validators: [
'0x5FbDB2315678afecb367f032d93F642f64180aa3',
'0x4B20993Bc481177ec7E8f571ceCaE8A9e22C02db',
'0xAb8483F64d9C6d1EcF9b849Ae677dD3315835cb2',
],
threshold: 2,
},
},
},
],
threshold: 2,
};

const { ism, initialIsmAddress } = await createIsm(
config as AggregationIsmConfig,
);

const updatedConfig: AggregationIsmConfig = {
type: IsmType.AGGREGATION,
modules: [
{
type: IsmType.ROUTING,
owner: (await multiProvider.getSignerAddress(chain)).toLowerCase(),
domains: {
test2: {
type: IsmType.MERKLE_ROOT_MULTISIG,
validators: [
'0x5FbDB2315678afecb367f032d93F642f64180aa3',
'0x4B20993Bc481177ec7E8f571ceCaE8A9e22C02db',
'0xAb8483F64d9C6d1EcF9b849Ae677dD3315835cb2',
],
threshold: 2,
},
test1: {
type: IsmType.MERKLE_ROOT_MULTISIG,
validators: [
'0x5FbDB2315678afecb367f032d93F642f64180aa3',
'0xAb8483F64d9C6d1EcF9b849Ae677dD3315835cb2',
'0x4B20993Bc481177ec7E8f571ceCaE8A9e22C02db',
],
threshold: 2,
},
},
},
{
type: IsmType.MERKLE_ROOT_MULTISIG,
validators: [
'0x5FbDB2315678afecb367f032d93F642f64180aa3',
'0xAb8483F64d9C6d1EcF9b849Ae677dD3315835cb2',
'0x4B20993Bc481177ec7E8f571ceCaE8A9e22C02db',
],
threshold: 2,
},
],
threshold: 2,
};

// expect 0 updates
await expectTxsAndUpdate(ism, updatedConfig, 0);

// expect the ISM address to be the same
expect(eqAddress(initialIsmAddress, ism.serialize().deployedIsm)).to.be
.true;
});
});
});
2 changes: 1 addition & 1 deletion typescript/sdk/src/ism/EvmIsmModule.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import {
assert,
deepEquals,
eqAddress,
normalizeConfig,
objFilter,
rootLogger,
} from '@hyperlane-xyz/utils';
Expand All @@ -46,6 +45,7 @@ import { ContractVerifier } from '../deploy/verify/ContractVerifier.js';
import { MultiProvider } from '../providers/MultiProvider.js';
import { AnnotatedEV5Transaction } from '../providers/ProviderType.js';
import { ChainName, ChainNameOrId } from '../types.js';
import { normalizeConfig } from '../utils/ism.js';
import { findMatchingLogEvents } from '../utils/logUtils.js';

import { EvmIsmReader } from './EvmIsmReader.js';
Expand Down
Loading

0 comments on commit 2a282ec

Please sign in to comment.