From c675dbb0979ef8a56dda2f420b212357a4c9498c Mon Sep 17 00:00:00 2001 From: xeno097 Date: Mon, 21 Oct 2024 16:22:21 -0400 Subject: [PATCH 01/25] feat(sdk): updated the RouterConfigSchema to have an optional proxyAdmin field to allow deploy overrides and checks --- .../sdk/src/router/ProxiedRouterDeployer.ts | 29 +++++++++++++++---- typescript/sdk/src/router/schemas.ts | 1 + 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/typescript/sdk/src/router/ProxiedRouterDeployer.ts b/typescript/sdk/src/router/ProxiedRouterDeployer.ts index e938c096c4..f7d50aae09 100644 --- a/typescript/sdk/src/router/ProxiedRouterDeployer.ts +++ b/typescript/sdk/src/router/ProxiedRouterDeployer.ts @@ -1,6 +1,8 @@ import { constants } from 'ethers'; import { + ProxyAdmin, + ProxyAdmin__factory, Router, TimelockController, TimelockController__factory, @@ -82,12 +84,27 @@ export abstract class ProxiedRouterDeployer< chain: ChainName, config: Config, ): Promise> { - const proxyAdmin = await this.deployContractFromFactory( - chain, - this.factories.proxyAdmin, - 'proxyAdmin', - [], - ); + let proxyAdmin: ProxyAdmin; + if (config.proxyAdmin) { + this.logger.debug( + `Reusing existing ProxyAdmin at ${config.proxyAdmin.address} for chain ${chain}`, + ); + proxyAdmin = ProxyAdmin__factory.connect( + config.proxyAdmin.address, + this.multiProvider.getSigner(chain), + ); + } else { + this.logger.debug( + `A ProxyAdmin config has not been supplied for chain ${chain}, deploying a new contract`, + ); + + proxyAdmin = await this.deployContractFromFactory( + chain, + this.factories.proxyAdmin, + 'proxyAdmin', + [], + ); + } let timelockController: TimelockController; let adminOwner: string; diff --git a/typescript/sdk/src/router/schemas.ts b/typescript/sdk/src/router/schemas.ts index abccd91610..f13d5d1d4f 100644 --- a/typescript/sdk/src/router/schemas.ts +++ b/typescript/sdk/src/router/schemas.ts @@ -29,6 +29,7 @@ export const RouterConfigSchema = MailboxClientConfigSchema.merge( ).merge( z.object({ remoteRouters: RemoteRoutersSchema.optional(), + proxyAdmin: OwnableSchema.extend({ address: ZHash }).optional(), }), ); From b9785914ec0fbeaff88df50ab03beae6f8bbbdc6 Mon Sep 17 00:00:00 2001 From: xeno097 Date: Mon, 21 Oct 2024 16:31:49 -0400 Subject: [PATCH 02/25] refactor(sdk): defined the DeployedOwnableConfig and DeployedOwnableSchema --- typescript/sdk/src/deploy/types.ts | 4 +++- typescript/sdk/src/router/schemas.ts | 4 ++-- typescript/sdk/src/schemas.ts | 2 ++ 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/typescript/sdk/src/deploy/types.ts b/typescript/sdk/src/deploy/types.ts index e8afe8f7ad..c1612d10d2 100644 --- a/typescript/sdk/src/deploy/types.ts +++ b/typescript/sdk/src/deploy/types.ts @@ -8,11 +8,13 @@ import type { } from '@hyperlane-xyz/core'; import { Address } from '@hyperlane-xyz/utils'; -import { OwnableSchema } from '../schemas.js'; +import { DeployedOwnableSchema, OwnableSchema } from '../schemas.js'; import type { ChainName } from '../types.js'; export type OwnableConfig = z.infer; +export type DeployedOwnableConfig = z.infer; + export interface CheckerViolation { chain: ChainName; type: string; diff --git a/typescript/sdk/src/router/schemas.ts b/typescript/sdk/src/router/schemas.ts index f13d5d1d4f..4392d3cbf9 100644 --- a/typescript/sdk/src/router/schemas.ts +++ b/typescript/sdk/src/router/schemas.ts @@ -4,7 +4,7 @@ import { ProxyFactoryFactoriesSchema } from '../deploy/schemas.js'; import { HookConfigSchema } from '../hook/schemas.js'; import { IsmConfigSchema } from '../ism/schemas.js'; import { ZHash } from '../metadata/customZodTypes.js'; -import { OwnableSchema } from '../schemas.js'; +import { DeployedOwnableSchema, OwnableSchema } from '../schemas.js'; export const MailboxClientConfigSchema = OwnableSchema.extend({ mailbox: ZHash, @@ -29,7 +29,7 @@ export const RouterConfigSchema = MailboxClientConfigSchema.merge( ).merge( z.object({ remoteRouters: RemoteRoutersSchema.optional(), - proxyAdmin: OwnableSchema.extend({ address: ZHash }).optional(), + proxyAdmin: DeployedOwnableSchema.optional(), }), ); diff --git a/typescript/sdk/src/schemas.ts b/typescript/sdk/src/schemas.ts index 9e9b0ee1ca..b0c200d896 100644 --- a/typescript/sdk/src/schemas.ts +++ b/typescript/sdk/src/schemas.ts @@ -7,6 +7,8 @@ export const OwnableSchema = z.object({ ownerOverrides: z.record(ZHash).optional(), }); +export const DeployedOwnableSchema = OwnableSchema.extend({ address: ZHash }); + export const PausableSchema = OwnableSchema.extend({ paused: z.boolean(), }); From 747b00830ea69103b2c92022acd9bee7103b69c0 Mon Sep 17 00:00:00 2001 From: xeno097 Date: Mon, 21 Oct 2024 16:47:16 -0400 Subject: [PATCH 03/25] feat(sdk): updated the EvmWarpReader to add a fetchAdminProxyData method to retrive proxy admin data --- .../sdk/src/token/EvmERC20WarpRouteReader.ts | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/typescript/sdk/src/token/EvmERC20WarpRouteReader.ts b/typescript/sdk/src/token/EvmERC20WarpRouteReader.ts index 8ffbe82cc1..cfc174dc12 100644 --- a/typescript/sdk/src/token/EvmERC20WarpRouteReader.ts +++ b/typescript/sdk/src/token/EvmERC20WarpRouteReader.ts @@ -6,6 +6,7 @@ import { HypERC4626Collateral__factory, HypERC4626OwnerCollateral__factory, HypERC4626__factory, + ProxyAdmin__factory, TokenRouter__factory, } from '@hyperlane-xyz/core'; import { @@ -22,6 +23,7 @@ import { } from '@hyperlane-xyz/utils'; import { DEFAULT_CONTRACT_READ_CONCURRENCY } from '../consts/concurrency.js'; +import { DeployedOwnableConfig } from '../deploy/types.js'; import { EvmHookReader } from '../hook/EvmHookReader.js'; import { EvmIsmReader } from '../ism/EvmIsmReader.js'; import { MultiProvider } from '../providers/MultiProvider.js'; @@ -29,6 +31,7 @@ import { RemoteRouters } from '../router/types.js'; import { ChainNameOrId } from '../types.js'; import { HyperlaneReader } from '../utils/HyperlaneReader.js'; +import { proxyAdmin } from './../deploy/proxy.js'; import { CollateralExtensions } from './config.js'; import { TokenMetadata } from './types.js'; @@ -64,11 +67,13 @@ export class EvmERC20WarpRouteReader extends HyperlaneReader { const baseMetadata = await this.fetchMailboxClientConfig(warpRouteAddress); const tokenMetadata = await this.fetchTokenMetadata(type, warpRouteAddress); const remoteRouters = await this.fetchRemoteRouters(warpRouteAddress); + const proxyAdmin = await this.fetchAdminProxyData(warpRouteAddress); return { ...baseMetadata, ...tokenMetadata, remoteRouters, + proxyAdmin, type, } as TokenRouterConfig; } @@ -245,4 +250,19 @@ export class EvmERC20WarpRouteReader extends HyperlaneReader { ), ); } + + async fetchAdminProxyData( + tokenAddress: Address, + ): Promise { + const proxyAdminAddress = await proxyAdmin(this.provider, tokenAddress); + const proxyAdminInstance = ProxyAdmin__factory.connect( + proxyAdminAddress, + this.provider, + ); + + return { + address: proxyAdminAddress, + owner: await proxyAdminInstance.owner(), + }; + } } From 85843dad69888f057a34397676d0caa5b3640b2c Mon Sep 17 00:00:00 2001 From: xeno097 Date: Mon, 21 Oct 2024 16:53:55 -0400 Subject: [PATCH 04/25] feat(sdk): added the updateProxyAdminOwnershipTxs method to the EvmERC20WarpModule --- .../sdk/src/token/EvmERC20WarpModule.ts | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/typescript/sdk/src/token/EvmERC20WarpModule.ts b/typescript/sdk/src/token/EvmERC20WarpModule.ts index 19ea854899..3f57e80ab4 100644 --- a/typescript/sdk/src/token/EvmERC20WarpModule.ts +++ b/typescript/sdk/src/token/EvmERC20WarpModule.ts @@ -97,6 +97,7 @@ export class EvmERC20WarpModule extends HyperlaneModule< ...(await this.createIsmUpdateTxs(actualConfig, expectedConfig)), ...this.createRemoteRoutersUpdateTxs(actualConfig, expectedConfig), ...this.createOwnershipUpdateTxs(actualConfig, expectedConfig), + ...this.updateProxyAdminOwnershipTxs(actualConfig, expectedConfig), ); return transactions; @@ -223,6 +224,38 @@ export class EvmERC20WarpModule extends HyperlaneModule< }); } + updateProxyAdminOwnershipTxs( + actualConfig: Readonly, + expectedConfig: Readonly, + ): AnnotatedEV5Transaction[] { + const transactions: AnnotatedEV5Transaction[] = []; + + // Return early because old warp config files did not have the + // proxyAdmin property + if (!expectedConfig.proxyAdmin) { + return transactions; + } + + const actualProxyAdmin = actualConfig.proxyAdmin!; + assert( + actualProxyAdmin.address === expectedConfig.proxyAdmin.address, + `ProxyAdmin contract addresses do not match. Expected ${expectedConfig.proxyAdmin.address}, got ${actualProxyAdmin.address}`, + ); + + transactions.push( + // Internally the createTransferOwnershipTx method already checks if the + // two owner values are the same and produces an empty tx batch if they are + ...EvmModuleDeployer.createTransferOwnershipTx({ + actualOwner: actualProxyAdmin.owner, + expectedOwner: expectedConfig.proxyAdmin.owner, + deployedAddress: actualProxyAdmin.address, + chainId: this.domainId, + }), + ); + + return transactions; + } + /** * Updates or deploys the ISM using the provided configuration. * From a55dc016ba1d0a009d10917625c727992fd68fd3 Mon Sep 17 00:00:00 2001 From: xeno097 Date: Mon, 21 Oct 2024 17:36:36 -0400 Subject: [PATCH 05/25] feat(cli): implemented the setExistingProxyAdmin to promt users to use an already existing proxy admi ncontract for deployments --- typescript/cli/src/config/warp.ts | 21 ++++++++++++++++-- typescript/cli/src/utils/input.ts | 36 +++++++++++++++++++++++++++++++ typescript/sdk/src/index.ts | 1 + 3 files changed, 56 insertions(+), 2 deletions(-) diff --git a/typescript/cli/src/config/warp.ts b/typescript/cli/src/config/warp.ts index dd3a237139..63335865f5 100644 --- a/typescript/cli/src/config/warp.ts +++ b/typescript/cli/src/config/warp.ts @@ -3,6 +3,7 @@ import { stringify as yamlStringify } from 'yaml'; import { ChainMap, + DeployedOwnableConfig, IsmConfig, IsmType, MailboxClientConfig, @@ -28,7 +29,10 @@ import { readYamlOrJson, writeYamlOrJson, } from '../utils/files.js'; -import { detectAndConfirmOrPrompt } from '../utils/input.js'; +import { + detectAndConfirmOrPrompt, + setExistingProxyAdmin, +} from '../utils/input.js'; import { createAdvancedIsmConfig } from './ism.js'; @@ -129,7 +133,9 @@ export async function createWarpRouteDeployConfig({ chainMetadata: context.chainMetadata, message: 'Select chains to connect', requireNumber: 1, - requiresConfirmation: true, + // If the user supplied the --yes flag we skip asking selection + // confirmation + requiresConfirmation: !context.skipConfirmation, }); const result: WarpRouteDeployConfig = {}; @@ -147,6 +153,12 @@ export async function createWarpRouteDeployConfig({ message: `Could not retrieve mailbox address from the registry for chain "${chain}". Please enter a valid mailbox address:`, })); + const proxyAdmin: DeployedOwnableConfig | undefined = + // default to deploying a new ProxyAdmin if the user supplied the --yes flag + !context.skipConfirmation + ? await setExistingProxyAdmin(context, chain) + : undefined; + /** * The logic from the cli is as follows: * --advanced flag is provided: the user will have to build their own configuration using the available ISM types @@ -190,6 +202,7 @@ export async function createWarpRouteDeployConfig({ mailbox, type, owner, + proxyAdmin, isNft, interchainSecurityModule, token: await input({ @@ -203,6 +216,7 @@ export async function createWarpRouteDeployConfig({ type, owner, isNft, + proxyAdmin, collateralChainName: '', // This will be derived correctly by zod.parse() below interchainSecurityModule, }; @@ -216,6 +230,7 @@ export async function createWarpRouteDeployConfig({ mailbox, type, owner, + proxyAdmin, isNft, interchainSecurityModule, token: await input({ @@ -230,6 +245,7 @@ export async function createWarpRouteDeployConfig({ mailbox, type, owner, + proxyAdmin, isNft, interchainSecurityModule, token: await input({ @@ -242,6 +258,7 @@ export async function createWarpRouteDeployConfig({ mailbox, type, owner, + proxyAdmin, isNft, interchainSecurityModule, }; diff --git a/typescript/cli/src/utils/input.ts b/typescript/cli/src/utils/input.ts index 4b54c4f3e0..8e3d9dd5a8 100644 --- a/typescript/cli/src/utils/input.ts +++ b/typescript/cli/src/utils/input.ts @@ -18,6 +18,11 @@ import type { PartialDeep } from '@inquirer/type'; import ansiEscapes from 'ansi-escapes'; import chalk from 'chalk'; +import { ProxyAdmin__factory } from '@hyperlane-xyz/core'; +import { ChainName, DeployedOwnableConfig } from '@hyperlane-xyz/sdk'; +import { isAddress } from '@hyperlane-xyz/utils'; + +import { CommandContext } from '../context/types.js'; import { logGray } from '../logger.js'; import { indentYamlOrJson } from './files.js'; @@ -72,6 +77,37 @@ export async function inputWithInfo({ return answer; } +/** + * Prompts the user to optionally set an existing ProxyAdmin contract address to be used in a WarpToken deployment. + */ +export async function setExistingProxyAdmin( + context: CommandContext, + chain: ChainName, +): Promise { + const useExistingProxy = await confirm({ + message: `Use an existing Proxy Admin contract for the warp route deployment on chain "${chain}"?`, + }); + + if (!useExistingProxy) { + return; + } + + const proxyAdminAddress = await input({ + message: `Please enter the address of the Proxy Admin contract to be used on chain "${chain}":`, + validate: isAddress, + }); + + const proxy = ProxyAdmin__factory.connect( + proxyAdminAddress, + context.multiProvider.getProvider(chain), + ); + const ownerAddress = await proxy.owner(); + return { + address: proxyAdminAddress, + owner: ownerAddress, + }; +} + /** * Searchable checkbox code * diff --git a/typescript/sdk/src/index.ts b/typescript/sdk/src/index.ts index e80ad1f3ed..fc084d783e 100644 --- a/typescript/sdk/src/index.ts +++ b/typescript/sdk/src/index.ts @@ -96,6 +96,7 @@ export { HyperlaneProxyFactoryDeployer } from './deploy/HyperlaneProxyFactoryDep export { CheckerViolation, OwnableConfig, + DeployedOwnableConfig, OwnerViolation, ProxyAdminViolation, ViolationType, From 50d2047676aef00fe73e2a2f6b210c0d62b2623a Mon Sep 17 00:00:00 2001 From: xeno097 Date: Tue, 22 Oct 2024 10:21:11 -0400 Subject: [PATCH 06/25] docs(changeset): Add optional proxy admin reuse in warp route deployments and admin proxy ownership transfer in warp apply --- .changeset/pink-bats-mix.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changeset/pink-bats-mix.md diff --git a/.changeset/pink-bats-mix.md b/.changeset/pink-bats-mix.md new file mode 100644 index 0000000000..dd3ae396ac --- /dev/null +++ b/.changeset/pink-bats-mix.md @@ -0,0 +1,6 @@ +--- +'@hyperlane-xyz/cli': minor +'@hyperlane-xyz/sdk': minor +--- + +Add optional proxy admin reuse in warp route deployments and admin proxy ownership transfer in warp apply From 552bbb5939603a14b03575ef90032a426c2aae65 Mon Sep 17 00:00:00 2001 From: xeno097 Date: Tue, 22 Oct 2024 12:36:45 -0400 Subject: [PATCH 07/25] chore: paul pr review suggestion Co-authored-by: Paul Balaji <10051819+paulbalaji@users.noreply.github.com> --- typescript/sdk/src/token/EvmERC20WarpRouteReader.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/typescript/sdk/src/token/EvmERC20WarpRouteReader.ts b/typescript/sdk/src/token/EvmERC20WarpRouteReader.ts index cfc174dc12..0fe9a19e27 100644 --- a/typescript/sdk/src/token/EvmERC20WarpRouteReader.ts +++ b/typescript/sdk/src/token/EvmERC20WarpRouteReader.ts @@ -251,7 +251,7 @@ export class EvmERC20WarpRouteReader extends HyperlaneReader { ); } - async fetchAdminProxyData( + async fetchProxyAdminConfig( tokenAddress: Address, ): Promise { const proxyAdminAddress = await proxyAdmin(this.provider, tokenAddress); From 8082d178bac7f86d185908905a4f70fb0f6cb5d7 Mon Sep 17 00:00:00 2001 From: xeno097 Date: Tue, 22 Oct 2024 12:38:57 -0400 Subject: [PATCH 08/25] fix(sdk): fixed methdo cann for fetchProxyAdminConfig --- typescript/sdk/src/token/EvmERC20WarpRouteReader.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/typescript/sdk/src/token/EvmERC20WarpRouteReader.ts b/typescript/sdk/src/token/EvmERC20WarpRouteReader.ts index 0fe9a19e27..df80298bd9 100644 --- a/typescript/sdk/src/token/EvmERC20WarpRouteReader.ts +++ b/typescript/sdk/src/token/EvmERC20WarpRouteReader.ts @@ -67,7 +67,7 @@ export class EvmERC20WarpRouteReader extends HyperlaneReader { const baseMetadata = await this.fetchMailboxClientConfig(warpRouteAddress); const tokenMetadata = await this.fetchTokenMetadata(type, warpRouteAddress); const remoteRouters = await this.fetchRemoteRouters(warpRouteAddress); - const proxyAdmin = await this.fetchAdminProxyData(warpRouteAddress); + const proxyAdmin = await this.fetchProxyAdminConfig(warpRouteAddress); return { ...baseMetadata, From cf5f0aa71ec2b44bed194ca2e94048d551094c0d Mon Sep 17 00:00:00 2001 From: xeno097 Date: Wed, 23 Oct 2024 12:57:20 -0400 Subject: [PATCH 09/25] feat(cli): added proxy admin and ica router ownership configurability to core init command --- typescript/cli/src/config/core.ts | 39 +++++++++++++++++++++++++++--- typescript/cli/src/deploy/core.ts | 1 + typescript/sdk/src/core/schemas.ts | 9 ++++++- 3 files changed, 45 insertions(+), 4 deletions(-) diff --git a/typescript/cli/src/config/core.ts b/typescript/cli/src/config/core.ts index 8be42113d7..73f2649c4d 100644 --- a/typescript/cli/src/config/core.ts +++ b/typescript/cli/src/config/core.ts @@ -1,6 +1,11 @@ import { stringify as yamlStringify } from 'yaml'; -import { CoreConfigSchema, HookConfig, IsmConfig } from '@hyperlane-xyz/sdk'; +import { + CoreConfigSchema, + HookConfig, + IsmConfig, + OwnableConfig, +} from '@hyperlane-xyz/sdk'; import { CommandContext } from '../context/types.js'; import { errorRed, log, logBlue, logGreen } from '../logger.js'; @@ -18,6 +23,9 @@ import { } from './hooks.js'; import { createAdvancedIsmConfig, createTrustedRelayerConfig } from './ism.js'; +const ENTER_DESIRED_VALUE_MSG = 'Enter the desired'; +const SIGNER_PROMPT_LABEL = 'signer'; + export async function createCoreDeployConfig({ context, configFilePath, @@ -31,9 +39,9 @@ export async function createCoreDeployConfig({ const owner = await detectAndConfirmOrPrompt( async () => context.signer?.getAddress(), - 'Enter the desired', + ENTER_DESIRED_VALUE_MSG, 'owner address', - 'signer', + SIGNER_PROMPT_LABEL, ); const defaultIsm: IsmConfig = advanced @@ -41,6 +49,7 @@ export async function createCoreDeployConfig({ : await createTrustedRelayerConfig(context, advanced); let defaultHook: HookConfig, requiredHook: HookConfig; + let proxyAdmin: OwnableConfig, interchainAccountRouter: OwnableConfig; if (advanced) { defaultHook = await createHookConfig({ context, @@ -52,9 +61,31 @@ export async function createCoreDeployConfig({ selectMessage: 'Select required hook type', advanced, }); + proxyAdmin = { + owner: await detectAndConfirmOrPrompt( + async () => context.signer?.getAddress(), + ENTER_DESIRED_VALUE_MSG, + 'ProxyAdmin owner address', + SIGNER_PROMPT_LABEL, + ), + }; + interchainAccountRouter = { + owner: await detectAndConfirmOrPrompt( + async () => context.signer?.getAddress(), + ENTER_DESIRED_VALUE_MSG, + 'ICA Router owner address', + SIGNER_PROMPT_LABEL, + ), + }; } else { defaultHook = await createMerkleTreeConfig(); requiredHook = await createProtocolFeeConfig(context, advanced); + proxyAdmin = { + owner, + }; + interchainAccountRouter = { + owner, + }; } try { @@ -63,6 +94,8 @@ export async function createCoreDeployConfig({ defaultIsm, defaultHook, requiredHook, + proxyAdmin, + interchainAccountRouter, }); logBlue(`Core config is valid, writing to file ${configFilePath}:\n`); log(indentYamlOrJson(yamlStringify(coreConfig, null, 2), 4)); diff --git a/typescript/cli/src/deploy/core.ts b/typescript/cli/src/deploy/core.ts index f0848458fc..a05cef5d40 100644 --- a/typescript/cli/src/deploy/core.ts +++ b/typescript/cli/src/deploy/core.ts @@ -34,6 +34,7 @@ interface DeployParams { interface ApplyParams extends DeployParams { deployedCoreAddresses: DeployedCoreAddresses; } + /** * Executes the core deploy command. */ diff --git a/typescript/sdk/src/core/schemas.ts b/typescript/sdk/src/core/schemas.ts index 569bb2ee08..e25a4ee30d 100644 --- a/typescript/sdk/src/core/schemas.ts +++ b/typescript/sdk/src/core/schemas.ts @@ -1,6 +1,9 @@ import { z } from 'zod'; -import { ProxyFactoryFactoriesSchema } from '../deploy/schemas.js'; +import { + OwnableConfigSchema, + ProxyFactoryFactoriesSchema, +} from '../deploy/schemas.js'; import { HookConfigSchema } from '../hook/schemas.js'; import { IsmConfigSchema } from '../ism/schemas.js'; import { OwnableSchema } from '../schemas.js'; @@ -9,6 +12,10 @@ export const CoreConfigSchema = OwnableSchema.extend({ defaultIsm: IsmConfigSchema, defaultHook: HookConfigSchema, requiredHook: HookConfigSchema, + // These fields are set as optional because the old core config + // did not have them and we want to maintain backward compatibility + proxyAdmin: OwnableConfigSchema.optional(), + interchainAccountRouter: OwnableConfigSchema.optional(), }); export const DeployedCoreAddressesSchema = ProxyFactoryFactoriesSchema.extend({ From 9440e501b6ec5deef52b898a04d49a0ca9572894 Mon Sep 17 00:00:00 2001 From: xeno097 Date: Wed, 23 Oct 2024 13:20:59 -0400 Subject: [PATCH 10/25] feat(sdk): updated the EvmCoreModule.deploy method to check if the config defines owners fro the mailbox proxy admin adn ica router and use those values intead of the signer --- typescript/sdk/src/core/EvmCoreModule.ts | 38 +++++++++++++++++++----- 1 file changed, 31 insertions(+), 7 deletions(-) diff --git a/typescript/sdk/src/core/EvmCoreModule.ts b/typescript/sdk/src/core/EvmCoreModule.ts index fd4e96018b..33788a91cd 100644 --- a/typescript/sdk/src/core/EvmCoreModule.ts +++ b/typescript/sdk/src/core/EvmCoreModule.ts @@ -1,4 +1,8 @@ -import { Mailbox, Mailbox__factory } from '@hyperlane-xyz/core'; +import { + Mailbox, + Mailbox__factory, + Ownable__factory, +} from '@hyperlane-xyz/core'; import { Address, Domain, @@ -274,15 +278,17 @@ export class EvmCoreModule extends HyperlaneModule< ); // Deploy proxyAdmin - const proxyAdmin = ( - await coreDeployer.deployContract(chainName, 'proxyAdmin', []) - ).address; + const proxyAdmin = await coreDeployer.deployContract( + chainName, + 'proxyAdmin', + [], + ); // Deploy Mailbox const mailbox = await this.deployMailbox({ config, coreDeployer, - proxyAdmin, + proxyAdmin: proxyAdmin.address, multiProvider, chain, }); @@ -294,7 +300,9 @@ export class EvmCoreModule extends HyperlaneModule< multiProvider: multiProvider, config: { mailbox: mailbox.address, - owner: await multiProvider.getSigner(chain).getAddress(), + owner: + config.interchainAccountRouter?.owner ?? + (await multiProvider.getSigner(chain).getAddress()), }, contractVerifier, }) @@ -331,11 +339,27 @@ export class EvmCoreModule extends HyperlaneModule< const { merkleTreeHook, interchainGasPaymaster } = serializedContracts[chainName]; + // Update the ProxyAdmin owner of the Mailbox if the config defines a different owner from the current signer + const currentProxyOwner = await proxyAdmin.owner(); + if ( + config?.proxyAdmin?.owner && + config.proxyAdmin.owner !== currentProxyOwner + ) { + await multiProvider.sendTransaction(chainName, { + annotation: `Transferring ownership of ProxyAdmin to the configured address ${config.proxyAdmin.owner}`, + to: proxyAdmin.address, + data: Ownable__factory.createInterface().encodeFunctionData( + 'transferOwnership(address)', + [config.proxyAdmin.owner], + ), + }); + } + // Set Core & extra addresses return { ...ismFactoryFactories, - proxyAdmin, + proxyAdmin: proxyAdmin.address, mailbox: mailbox.address, interchainAccountRouter, interchainAccountIsm, From 62c2067a267c4be128dc520df9ece6e2dde39405 Mon Sep 17 00:00:00 2001 From: xeno097 Date: Wed, 23 Oct 2024 22:24:03 -0400 Subject: [PATCH 11/25] chore: lee pr review changes + e2e test fix for warp route apply --- typescript/cli/src/config/warp.ts | 12 ++--- typescript/cli/src/utils/input.ts | 36 +++++++++---- .../sdk/src/router/ProxiedRouterDeployer.ts | 2 +- typescript/sdk/src/schemas.ts | 4 +- .../token/EvmERC20WarpModule.hardhat-test.ts | 50 ++++++++++++++++++- .../sdk/src/token/EvmERC20WarpModule.ts | 2 +- 6 files changed, 86 insertions(+), 20 deletions(-) diff --git a/typescript/cli/src/config/warp.ts b/typescript/cli/src/config/warp.ts index 63335865f5..1174d0156b 100644 --- a/typescript/cli/src/config/warp.ts +++ b/typescript/cli/src/config/warp.ts @@ -31,7 +31,7 @@ import { } from '../utils/files.js'; import { detectAndConfirmOrPrompt, - setExistingProxyAdmin, + setProxyAdminConfig, } from '../utils/input.js'; import { createAdvancedIsmConfig } from './ism.js'; @@ -153,11 +153,11 @@ export async function createWarpRouteDeployConfig({ message: `Could not retrieve mailbox address from the registry for chain "${chain}". Please enter a valid mailbox address:`, })); - const proxyAdmin: DeployedOwnableConfig | undefined = - // default to deploying a new ProxyAdmin if the user supplied the --yes flag - !context.skipConfirmation - ? await setExistingProxyAdmin(context, chain) - : undefined; + const proxyAdmin: DeployedOwnableConfig = await setProxyAdminConfig( + context, + chain, + owner, + ); /** * The logic from the cli is as follows: diff --git a/typescript/cli/src/utils/input.ts b/typescript/cli/src/utils/input.ts index 7ccc90827c..61dac53443 100644 --- a/typescript/cli/src/utils/input.ts +++ b/typescript/cli/src/utils/input.ts @@ -21,7 +21,7 @@ import chalk from 'chalk'; import { ProxyAdmin__factory } from '@hyperlane-xyz/core'; import { ChainName, DeployedOwnableConfig } from '@hyperlane-xyz/sdk'; import { WarpCoreConfig } from '@hyperlane-xyz/sdk'; -import { isAddress } from '@hyperlane-xyz/utils'; +import { Address, isAddress } from '@hyperlane-xyz/utils'; import { readWarpCoreConfig } from '../config/warp.js'; import { CommandContext } from '../context/types.js'; @@ -84,16 +84,27 @@ export async function inputWithInfo({ /** * Prompts the user to optionally set an existing ProxyAdmin contract address to be used in a WarpToken deployment. */ -export async function setExistingProxyAdmin( +export async function setProxyAdminConfig( context: CommandContext, chain: ChainName, -): Promise { + warpRouteOwner: Address, +): Promise { + const defaultAdminConfig: DeployedOwnableConfig = { + owner: warpRouteOwner, + }; + + // default to deploying a new ProxyAdmin with `warpRouteOwner` as the owner + // if the user supplied the --yes flag + if (context.skipConfirmation) { + return defaultAdminConfig; + } + const useExistingProxy = await confirm({ message: `Use an existing Proxy Admin contract for the warp route deployment on chain "${chain}"?`, }); if (!useExistingProxy) { - return; + return defaultAdminConfig; } const proxyAdminAddress = await input({ @@ -105,11 +116,18 @@ export async function setExistingProxyAdmin( proxyAdminAddress, context.multiProvider.getProvider(chain), ); - const ownerAddress = await proxy.owner(); - return { - address: proxyAdminAddress, - owner: ownerAddress, - }; + + try { + const ownerAddress = await proxy.owner(); + return { + address: proxyAdminAddress, + owner: ownerAddress, + }; + } catch (error) { + throw new Error( + `Failed to read owner address from ProxyAdmin contract at ${proxy.address}. Are you sure this is a ProxyAdmin contract?`, + ); + } } /** diff --git a/typescript/sdk/src/router/ProxiedRouterDeployer.ts b/typescript/sdk/src/router/ProxiedRouterDeployer.ts index f7d50aae09..9edcad76c6 100644 --- a/typescript/sdk/src/router/ProxiedRouterDeployer.ts +++ b/typescript/sdk/src/router/ProxiedRouterDeployer.ts @@ -85,7 +85,7 @@ export abstract class ProxiedRouterDeployer< config: Config, ): Promise> { let proxyAdmin: ProxyAdmin; - if (config.proxyAdmin) { + if (config.proxyAdmin?.address) { this.logger.debug( `Reusing existing ProxyAdmin at ${config.proxyAdmin.address} for chain ${chain}`, ); diff --git a/typescript/sdk/src/schemas.ts b/typescript/sdk/src/schemas.ts index b0c200d896..ef37e92fb9 100644 --- a/typescript/sdk/src/schemas.ts +++ b/typescript/sdk/src/schemas.ts @@ -7,7 +7,9 @@ export const OwnableSchema = z.object({ ownerOverrides: z.record(ZHash).optional(), }); -export const DeployedOwnableSchema = OwnableSchema.extend({ address: ZHash }); +export const DeployedOwnableSchema = OwnableSchema.extend({ + address: ZHash.optional(), +}); export const PausableSchema = OwnableSchema.extend({ paused: z.boolean(), diff --git a/typescript/sdk/src/token/EvmERC20WarpModule.hardhat-test.ts b/typescript/sdk/src/token/EvmERC20WarpModule.hardhat-test.ts index 467a150ad5..0a8ddb8765 100644 --- a/typescript/sdk/src/token/EvmERC20WarpModule.hardhat-test.ts +++ b/typescript/sdk/src/token/EvmERC20WarpModule.hardhat-test.ts @@ -271,6 +271,7 @@ describe('EvmERC20WarpHyperlaneModule', async () => { paused: false, }, ]; + it('should deploy and set a new Ism', async () => { const config = { ...baseConfig, @@ -489,7 +490,7 @@ describe('EvmERC20WarpHyperlaneModule', async () => { ismFactoryAddresses, } as TokenRouterConfig; - const owner = randomAddress(); + const owner = signer.address.toLowerCase(); const evmERC20WarpModule = await EvmERC20WarpModule.create({ chain, config: { @@ -498,7 +499,9 @@ describe('EvmERC20WarpHyperlaneModule', async () => { }, multiProvider, }); - expect(owner).to.equal(owner); + + const currentConfig = await evmERC20WarpModule.read(); + expect(currentConfig.owner.toLowerCase()).to.equal(owner); const newOwner = randomAddress(); await sendTxs( @@ -518,5 +521,48 @@ describe('EvmERC20WarpHyperlaneModule', async () => { }); expect(txs.length).to.equal(0); }); + + it('should update the ProxyAdmin owner only if they are different', async () => { + const config: TokenRouterConfig = { + ...baseConfig, + type: TokenType.native, + hook: hookAddress, + ismFactoryAddresses, + }; + + const owner = signer.address.toLowerCase(); + const evmERC20WarpModule = await EvmERC20WarpModule.create({ + chain, + config: { + ...config, + interchainSecurityModule: ismAddress, + }, + multiProvider, + }); + + const currentConfig = await evmERC20WarpModule.read(); + expect(currentConfig.proxyAdmin?.owner.toLowerCase()).to.equal(owner); + + const newOwner = randomAddress(); + const updatedWarpCoreConfig: TokenRouterConfig = { + ...config, + proxyAdmin: { + address: currentConfig.proxyAdmin!.address, + owner: newOwner, + }, + }; + await sendTxs(await evmERC20WarpModule.update(updatedWarpCoreConfig)); + + const latestConfig: TokenRouterConfig = normalizeConfig( + await evmERC20WarpModule.read(), + ); + expect(latestConfig.proxyAdmin?.owner).to.equal(newOwner); + // Sanity check to be sure that the owner of the warp route token has not been updated if not changed + expect(latestConfig.owner).to.equal(owner); + + // No op if the same owner + const txs = await evmERC20WarpModule.update(updatedWarpCoreConfig); + expect(txs.length).to.equal(0); + }); }); }); diff --git a/typescript/sdk/src/token/EvmERC20WarpModule.ts b/typescript/sdk/src/token/EvmERC20WarpModule.ts index 3f57e80ab4..4c9f6b1f60 100644 --- a/typescript/sdk/src/token/EvmERC20WarpModule.ts +++ b/typescript/sdk/src/token/EvmERC20WarpModule.ts @@ -248,7 +248,7 @@ export class EvmERC20WarpModule extends HyperlaneModule< ...EvmModuleDeployer.createTransferOwnershipTx({ actualOwner: actualProxyAdmin.owner, expectedOwner: expectedConfig.proxyAdmin.owner, - deployedAddress: actualProxyAdmin.address, + deployedAddress: actualProxyAdmin.address!, chainId: this.domainId, }), ); From c59ad8b09069a5cd71d9acaa6d79b18c4b65292c Mon Sep 17 00:00:00 2001 From: xeno097 Date: Thu, 24 Oct 2024 12:57:22 -0400 Subject: [PATCH 12/25] fix(sdk): fixed deriveTokenMetadata return value to exlcude fields not defined on the type --- typescript/sdk/src/token/deploy.ts | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/typescript/sdk/src/token/deploy.ts b/typescript/sdk/src/token/deploy.ts index ee2a9a399b..67d04e3331 100644 --- a/typescript/sdk/src/token/deploy.ts +++ b/typescript/sdk/src/token/deploy.ts @@ -29,6 +29,7 @@ import { hypERC721factories, } from './contracts.js'; import { + TokenMetadataSchema, TokenRouterConfig, isCollateralConfig, isNativeConfig, @@ -104,13 +105,16 @@ abstract class TokenDeployer< for (const [chain, config] of Object.entries(configMap)) { if (isTokenMetadata(config)) { - return config; + return TokenMetadataSchema.parse(config); } if (isNativeConfig(config)) { const nativeToken = multiProvider.getChainMetadata(chain).nativeToken; if (nativeToken) { - return { totalSupply: DERIVED_TOKEN_SUPPLY, ...nativeToken }; + return TokenMetadataSchema.parse({ + totalSupply: DERIVED_TOKEN_SUPPLY, + ...nativeToken, + }); } } @@ -126,11 +130,11 @@ abstract class TokenDeployer< erc721.name(), erc721.symbol(), ]); - return { + return TokenMetadataSchema.parse({ name, symbol, totalSupply: DERIVED_TOKEN_SUPPLY, - }; + }); } let token: string; @@ -159,7 +163,12 @@ abstract class TokenDeployer< erc20.decimals(), ]); - return { name, symbol, decimals, totalSupply: DERIVED_TOKEN_SUPPLY }; + return TokenMetadataSchema.parse({ + name, + symbol, + decimals, + totalSupply: DERIVED_TOKEN_SUPPLY, + }); } } From e396abc237eaa9a47a2538899dbfd5ef725c0175 Mon Sep 17 00:00:00 2001 From: xeno097 Date: Thu, 24 Oct 2024 19:06:30 -0400 Subject: [PATCH 13/25] feat(sdk): updated the EvmCoreReader to fetch the admin proxy data for the mailbox contract --- typescript/sdk/src/core/EvmCoreReader.ts | 32 +++++++++++++++++++----- typescript/sdk/src/core/schemas.ts | 11 +++----- 2 files changed, 30 insertions(+), 13 deletions(-) diff --git a/typescript/sdk/src/core/EvmCoreReader.ts b/typescript/sdk/src/core/EvmCoreReader.ts index 6c945bcf5d..392e4f1823 100644 --- a/typescript/sdk/src/core/EvmCoreReader.ts +++ b/typescript/sdk/src/core/EvmCoreReader.ts @@ -1,6 +1,6 @@ import { providers } from 'ethers'; -import { Mailbox__factory } from '@hyperlane-xyz/core'; +import { Mailbox__factory, ProxyAdmin__factory } from '@hyperlane-xyz/core'; import { Address, objMap, @@ -9,6 +9,8 @@ import { } from '@hyperlane-xyz/utils'; import { DEFAULT_CONTRACT_READ_CONCURRENCY } from '../consts/concurrency.js'; +import { proxyAdmin } from '../deploy/proxy.js'; +import { DeployedOwnableConfig } from '../deploy/types.js'; import { EvmHookReader } from '../hook/EvmHookReader.js'; import { EvmIsmReader } from '../ism/EvmIsmReader.js'; import { MultiProvider } from '../providers/MultiProvider.js'; @@ -46,11 +48,13 @@ export class EvmCoreReader implements CoreReader { */ async deriveCoreConfig(address: Address): Promise { const mailbox = Mailbox__factory.connect(address, this.provider); - const [defaultIsm, defaultHook, requiredHook] = await Promise.all([ - mailbox.defaultIsm(), - mailbox.defaultHook(), - mailbox.requiredHook(), - ]); + const [defaultIsm, defaultHook, requiredHook, mailboxProxyAdmin] = + await Promise.all([ + mailbox.defaultIsm(), + mailbox.defaultHook(), + mailbox.requiredHook(), + proxyAdmin(this.provider, mailbox.address), + ]); // Parallelize each configuration request const results = await promiseObjAll( @@ -60,6 +64,7 @@ export class EvmCoreReader implements CoreReader { defaultIsm: this.evmIsmReader.deriveIsmConfig(defaultIsm), defaultHook: this.evmHookReader.deriveHookConfig(defaultHook), requiredHook: this.evmHookReader.deriveHookConfig(requiredHook), + proxyAdmin: this._getProxyAdminConfig(mailboxProxyAdmin), }, async (_, readerCall) => { try { @@ -77,4 +82,19 @@ export class EvmCoreReader implements CoreReader { return results as CoreConfig; } + + private async _getProxyAdminConfig( + proxyAdminAddress: Address, + ): Promise { + const instance = ProxyAdmin__factory.connect( + proxyAdminAddress, + this.provider, + ); + + const owner = await instance.owner(); + return { + owner, + address: proxyAdminAddress, + }; + } } diff --git a/typescript/sdk/src/core/schemas.ts b/typescript/sdk/src/core/schemas.ts index e25a4ee30d..b196b5d84c 100644 --- a/typescript/sdk/src/core/schemas.ts +++ b/typescript/sdk/src/core/schemas.ts @@ -1,12 +1,9 @@ import { z } from 'zod'; -import { - OwnableConfigSchema, - ProxyFactoryFactoriesSchema, -} from '../deploy/schemas.js'; +import { ProxyFactoryFactoriesSchema } from '../deploy/schemas.js'; import { HookConfigSchema } from '../hook/schemas.js'; import { IsmConfigSchema } from '../ism/schemas.js'; -import { OwnableSchema } from '../schemas.js'; +import { DeployedOwnableSchema, OwnableSchema } from '../schemas.js'; export const CoreConfigSchema = OwnableSchema.extend({ defaultIsm: IsmConfigSchema, @@ -14,8 +11,8 @@ export const CoreConfigSchema = OwnableSchema.extend({ requiredHook: HookConfigSchema, // These fields are set as optional because the old core config // did not have them and we want to maintain backward compatibility - proxyAdmin: OwnableConfigSchema.optional(), - interchainAccountRouter: OwnableConfigSchema.optional(), + proxyAdmin: DeployedOwnableSchema.optional(), + interchainAccountRouter: DeployedOwnableSchema.optional(), }); export const DeployedCoreAddressesSchema = ProxyFactoryFactoriesSchema.extend({ From cca3416549e93c909f7d54d378188cc2885f2e78 Mon Sep 17 00:00:00 2001 From: xeno097 Date: Fri, 25 Oct 2024 16:02:22 -0400 Subject: [PATCH 14/25] refactor(sdk): implemented the proxyAdminOwnershipUpdateTxs to dedup admin proxy ownership code --- typescript/sdk/src/core/EvmCoreModule.ts | 6 +++ typescript/sdk/src/deploy/proxy.ts | 40 ++++++++++++++++++- .../sdk/src/token/EvmERC20WarpModule.ts | 39 +++--------------- 3 files changed, 51 insertions(+), 34 deletions(-) diff --git a/typescript/sdk/src/core/EvmCoreModule.ts b/typescript/sdk/src/core/EvmCoreModule.ts index 33788a91cd..d86548d83f 100644 --- a/typescript/sdk/src/core/EvmCoreModule.ts +++ b/typescript/sdk/src/core/EvmCoreModule.ts @@ -27,6 +27,7 @@ import { ProxyFactoryFactories, proxyFactoryFactories, } from '../deploy/contracts.js'; +import { proxyAdminOwnershipUpdateTxs } from '../deploy/proxy.js'; import { ContractVerifier } from '../deploy/verify/ContractVerifier.js'; import { HookFactories } from '../hook/contracts.js'; import { EvmIsmModule } from '../ism/EvmIsmModule.js'; @@ -95,6 +96,11 @@ export class EvmCoreModule extends HyperlaneModule< transactions.push( ...(await this.createDefaultIsmUpdateTxs(actualConfig, expectedConfig)), ...this.createMailboxOwnerUpdateTxs(actualConfig, expectedConfig), + ...proxyAdminOwnershipUpdateTxs( + actualConfig, + expectedConfig, + this.domainId, + ), ); return transactions; diff --git a/typescript/sdk/src/deploy/proxy.ts b/typescript/sdk/src/deploy/proxy.ts index 041d27cac2..29ec36d683 100644 --- a/typescript/sdk/src/deploy/proxy.ts +++ b/typescript/sdk/src/deploy/proxy.ts @@ -1,6 +1,11 @@ import { ethers } from 'ethers'; -import { Address, eqAddress } from '@hyperlane-xyz/utils'; +import { Address, assert, eqAddress } from '@hyperlane-xyz/utils'; + +import { AnnotatedEV5Transaction } from '../providers/ProviderType.js'; + +import { EvmModuleDeployer } from './EvmModuleDeployer.js'; +import { DeployedOwnableConfig } from './types.js'; export type UpgradeConfig = { timelock: { @@ -67,3 +72,36 @@ export async function isProxy( const admin = await proxyAdmin(provider, proxy); return !eqAddress(admin, ethers.constants.AddressZero); } + +export function proxyAdminOwnershipUpdateTxs( + actualConfig: Readonly<{ proxyAdmin?: DeployedOwnableConfig }>, + expectedConfig: Readonly<{ proxyAdmin?: DeployedOwnableConfig }>, + chainId: number, +): AnnotatedEV5Transaction[] { + const transactions: AnnotatedEV5Transaction[] = []; + + // Return early because old config files did not have the + // proxyAdmin property + if (!expectedConfig.proxyAdmin?.address) { + return transactions; + } + + const actualProxyAdmin = actualConfig.proxyAdmin!; + assert( + actualProxyAdmin.address === expectedConfig.proxyAdmin.address, + `ProxyAdmin contract addresses do not match. Expected ${expectedConfig.proxyAdmin.address}, got ${actualProxyAdmin.address}`, + ); + + transactions.push( + // Internally the createTransferOwnershipTx method already checks if the + // two owner values are the same and produces an empty tx batch if they are + ...EvmModuleDeployer.createTransferOwnershipTx({ + actualOwner: actualProxyAdmin.owner, + expectedOwner: expectedConfig.proxyAdmin.owner, + deployedAddress: actualProxyAdmin.address!, + chainId: chainId, + }), + ); + + return transactions; +} diff --git a/typescript/sdk/src/token/EvmERC20WarpModule.ts b/typescript/sdk/src/token/EvmERC20WarpModule.ts index 4c9f6b1f60..eafbf25363 100644 --- a/typescript/sdk/src/token/EvmERC20WarpModule.ts +++ b/typescript/sdk/src/token/EvmERC20WarpModule.ts @@ -20,6 +20,7 @@ import { HyperlaneModuleParams, } from '../core/AbstractHyperlaneModule.js'; import { EvmModuleDeployer } from '../deploy/EvmModuleDeployer.js'; +import { proxyAdminOwnershipUpdateTxs } from '../deploy/proxy.js'; import { EvmIsmModule } from '../ism/EvmIsmModule.js'; import { DerivedIsmConfig } from '../ism/EvmIsmReader.js'; import { MultiProvider } from '../providers/MultiProvider.js'; @@ -97,7 +98,11 @@ export class EvmERC20WarpModule extends HyperlaneModule< ...(await this.createIsmUpdateTxs(actualConfig, expectedConfig)), ...this.createRemoteRoutersUpdateTxs(actualConfig, expectedConfig), ...this.createOwnershipUpdateTxs(actualConfig, expectedConfig), - ...this.updateProxyAdminOwnershipTxs(actualConfig, expectedConfig), + ...proxyAdminOwnershipUpdateTxs( + actualConfig, + expectedConfig, + this.domainId, + ), ); return transactions; @@ -224,38 +229,6 @@ export class EvmERC20WarpModule extends HyperlaneModule< }); } - updateProxyAdminOwnershipTxs( - actualConfig: Readonly, - expectedConfig: Readonly, - ): AnnotatedEV5Transaction[] { - const transactions: AnnotatedEV5Transaction[] = []; - - // Return early because old warp config files did not have the - // proxyAdmin property - if (!expectedConfig.proxyAdmin) { - return transactions; - } - - const actualProxyAdmin = actualConfig.proxyAdmin!; - assert( - actualProxyAdmin.address === expectedConfig.proxyAdmin.address, - `ProxyAdmin contract addresses do not match. Expected ${expectedConfig.proxyAdmin.address}, got ${actualProxyAdmin.address}`, - ); - - transactions.push( - // Internally the createTransferOwnershipTx method already checks if the - // two owner values are the same and produces an empty tx batch if they are - ...EvmModuleDeployer.createTransferOwnershipTx({ - actualOwner: actualProxyAdmin.owner, - expectedOwner: expectedConfig.proxyAdmin.owner, - deployedAddress: actualProxyAdmin.address!, - chainId: this.domainId, - }), - ); - - return transactions; - } - /** * Updates or deploys the ISM using the provided configuration. * From 1c8c6824aabab04d22cae3da56de2796244dff1e Mon Sep 17 00:00:00 2001 From: xeno097 Date: Fri, 25 Oct 2024 16:22:39 -0400 Subject: [PATCH 15/25] test(cli): implemented testing utils for core commands testing --- typescript/cli/src/tests/commands/core.ts | 42 +++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/typescript/cli/src/tests/commands/core.ts b/typescript/cli/src/tests/commands/core.ts index b28dbafd2c..a6d95b5ed3 100644 --- a/typescript/cli/src/tests/commands/core.ts +++ b/typescript/cli/src/tests/commands/core.ts @@ -1,5 +1,9 @@ import { $ } from 'zx'; +import { CoreConfig } from '@hyperlane-xyz/sdk'; + +import { readYamlOrJson } from '../../utils/files.js'; + import { ANVIL_KEY, REGISTRY_PATH } from './helpers.js'; /** @@ -17,3 +21,41 @@ export async function hyperlaneCoreDeploy( --verbosity debug \ --yes`; } + +/** + * Reads a Hyperlane core deployment on the specified chain using the provided config. + */ +export async function hyperlaneCoreRead(chain: string, coreOutputPath: string) { + return $`yarn workspace @hyperlane-xyz/cli run hyperlane core read \ + --registry ${REGISTRY_PATH} \ + --config ${coreOutputPath} \ + --chain ${chain} \ + --verbosity debug \ + --yes`; +} + +/** + * Updates a Hyperlane core deployment on the specified chain using the provided config. + */ +export async function hyperlaneCoreApply( + chain: string, + coreOutputPath: string, +) { + return $`yarn workspace @hyperlane-xyz/cli run hyperlane core apply \ + --registry ${REGISTRY_PATH} \ + --config ${coreOutputPath} \ + --chain ${chain} \ + --verbosity debug \ + --yes`; +} + +/** + * Reads the Core deployment config and outputs it to specified output path. + */ +export async function readCoreConfig( + chain: string, + coreConfigPath: string, +): Promise { + await hyperlaneCoreRead(chain, coreConfigPath); + return readYamlOrJson(coreConfigPath); +} From 3623d06ceca5bbd723d43fed401071c62d15e747 Mon Sep 17 00:00:00 2001 From: xeno097 Date: Fri, 25 Oct 2024 16:30:46 -0400 Subject: [PATCH 16/25] refactor(sdk,infra): remvoed duplicated impls of randomAddress --- typescript/infra/test/govern.hardhat-test.ts | 6 ++--- typescript/sdk/src/hook/EvmHookReader.test.ts | 27 +++++++++---------- typescript/sdk/src/index.ts | 1 + typescript/sdk/src/ism/EvmIsmReader.test.ts | 14 +++++----- 4 files changed, 22 insertions(+), 26 deletions(-) diff --git a/typescript/infra/test/govern.hardhat-test.ts b/typescript/infra/test/govern.hardhat-test.ts index 6de17b11f6..90bf2d11ba 100644 --- a/typescript/infra/test/govern.hardhat-test.ts +++ b/typescript/infra/test/govern.hardhat-test.ts @@ -1,6 +1,6 @@ import { SignerWithAddress } from '@nomiclabs/hardhat-ethers/signers.js'; import { expect } from 'chai'; -import { BigNumber, utils } from 'ethers'; +import { BigNumber } from 'ethers'; import hre from 'hardhat'; import { @@ -27,6 +27,7 @@ import { TestChainName, TestCoreApp, TestCoreDeployer, + randomAddress, } from '@hyperlane-xyz/sdk'; import { Address, CallData, eqAddress } from '@hyperlane-xyz/utils'; @@ -35,9 +36,6 @@ import { HyperlaneAppGovernor, } from '../src/govern/HyperlaneAppGovernor.js'; -// TODO de-dupe with test-utils after migrating this file to the SDK -const randomAddress = () => utils.hexlify(utils.randomBytes(20)); - export class TestApp extends HyperlaneApp<{}> {} export class TestChecker extends HyperlaneAppChecker { diff --git a/typescript/sdk/src/hook/EvmHookReader.test.ts b/typescript/sdk/src/hook/EvmHookReader.test.ts index 6bab47a920..befd73a431 100644 --- a/typescript/sdk/src/hook/EvmHookReader.test.ts +++ b/typescript/sdk/src/hook/EvmHookReader.test.ts @@ -19,6 +19,7 @@ import { WithAddress } from '@hyperlane-xyz/utils'; import { TestChainName, test1 } from '../consts/testChains.js'; import { MultiProvider } from '../providers/MultiProvider.js'; +import { randomAddress } from '../test/testUtils.js'; import { EvmHookReader } from './EvmHookReader.js'; import { @@ -35,8 +36,6 @@ describe('EvmHookReader', () => { let multiProvider: MultiProvider; let sandbox: sinon.SinonSandbox; - const generateRandomAddress = () => ethers.Wallet.createRandom().address; - beforeEach(() => { sandbox = sinon.createSandbox(); multiProvider = MultiProvider.createTestMultiProvider(); @@ -48,8 +47,8 @@ describe('EvmHookReader', () => { }); it('should derive merkle tree config correctly', async () => { - const mockAddress = generateRandomAddress(); - const mockOwner = generateRandomAddress(); + const mockAddress = randomAddress(); + const mockOwner = randomAddress(); // Mocking the connect method + returned what we need from contract object const mockContract = { @@ -78,9 +77,9 @@ describe('EvmHookReader', () => { }); it('should derive protocol fee hook correctly', async () => { - const mockAddress = generateRandomAddress(); - const mockOwner = generateRandomAddress(); - const mockBeneficiary = generateRandomAddress(); + const mockAddress = randomAddress(); + const mockOwner = randomAddress(); + const mockBeneficiary = randomAddress(); // Mocking the connect method + returned what we need from contract object const mockContract = { @@ -116,8 +115,8 @@ describe('EvmHookReader', () => { }); it('should derive pausable config correctly', async () => { - const mockAddress = generateRandomAddress(); - const mockOwner = generateRandomAddress(); + const mockAddress = randomAddress(); + const mockOwner = randomAddress(); const mockPaused = randomBytes(1)[0] % 2 === 0; // Mocking the connect method + returned what we need from contract object @@ -151,9 +150,9 @@ describe('EvmHookReader', () => { // eslint-disable-next-line @typescript-eslint/no-empty-function it('should derive op stack config correctly', async () => { - const mockAddress = generateRandomAddress(); - const mockOwner = generateRandomAddress(); - const l1Messenger = generateRandomAddress(); + const mockAddress = randomAddress(); + const mockOwner = randomAddress(); + const l1Messenger = randomAddress(); // Mocking the connect method + returned what we need from contract object const mockContract = { @@ -187,8 +186,8 @@ describe('EvmHookReader', () => { }); it('should throw if derivation fails', async () => { - const mockAddress = generateRandomAddress(); - const mockOwner = generateRandomAddress(); + const mockAddress = randomAddress(); + const mockOwner = randomAddress(); // Mocking the connect method + returned what we need from contract object const mockContract = { diff --git a/typescript/sdk/src/index.ts b/typescript/sdk/src/index.ts index 23ba745788..f3f3d78969 100644 --- a/typescript/sdk/src/index.ts +++ b/typescript/sdk/src/index.ts @@ -26,6 +26,7 @@ export { testCosmosChain, testSealevelChain, } from './consts/testChains.js'; +export { randomAddress } from './test/testUtils.js'; export { attachAndConnectContracts, attachContracts, diff --git a/typescript/sdk/src/ism/EvmIsmReader.test.ts b/typescript/sdk/src/ism/EvmIsmReader.test.ts index 95368949b3..4da13a80a9 100644 --- a/typescript/sdk/src/ism/EvmIsmReader.test.ts +++ b/typescript/sdk/src/ism/EvmIsmReader.test.ts @@ -1,5 +1,4 @@ import { expect } from 'chai'; -import { ethers } from 'ethers'; import sinon from 'sinon'; import { @@ -20,6 +19,7 @@ import { WithAddress } from '@hyperlane-xyz/utils'; import { TestChainName } from '../consts/testChains.js'; import { MultiProvider } from '../providers/MultiProvider.js'; +import { randomAddress } from '../test/testUtils.js'; import { EvmIsmReader } from './EvmIsmReader.js'; import { @@ -35,8 +35,6 @@ describe('EvmIsmReader', () => { let multiProvider: MultiProvider; let sandbox: sinon.SinonSandbox; - const generateRandomAddress = () => ethers.Wallet.createRandom().address; - beforeEach(() => { sandbox = sinon.createSandbox(); multiProvider = MultiProvider.createTestMultiProvider(); @@ -48,8 +46,8 @@ describe('EvmIsmReader', () => { }); it('should derive multisig config correctly', async () => { - const mockAddress = generateRandomAddress(); - const mockValidators = [generateRandomAddress(), generateRandomAddress()]; + const mockAddress = randomAddress(); + const mockValidators = [randomAddress(), randomAddress()]; const mockThreshold = 2; // Mocking the connect method + returned what we need from contract object @@ -83,8 +81,8 @@ describe('EvmIsmReader', () => { }); it('should derive pausable config correctly', async () => { - const mockAddress = generateRandomAddress(); - const mockOwner = generateRandomAddress(); + const mockAddress = randomAddress(); + const mockOwner = randomAddress(); const mockPaused = true; // Mocking the connect method + returned what we need from contract object @@ -120,7 +118,7 @@ describe('EvmIsmReader', () => { }); it('should derive test ISM config correctly', async () => { - const mockAddress = generateRandomAddress(); + const mockAddress = randomAddress(); // Mocking the connect method + returned what we need from contract object const mockContract = { From 86178c9413f75ead725bc5b6cd416a8d1e160ab1 Mon Sep 17 00:00:00 2001 From: xeno097 Date: Fri, 25 Oct 2024 16:39:24 -0400 Subject: [PATCH 17/25] test(cli): updated the test e2e script to also run a anvil 1 chain only for core command testing --- typescript/cli/scripts/run-e2e-test.sh | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/typescript/cli/scripts/run-e2e-test.sh b/typescript/cli/scripts/run-e2e-test.sh index e398253b2e..44b04902b1 100755 --- a/typescript/cli/scripts/run-e2e-test.sh +++ b/typescript/cli/scripts/run-e2e-test.sh @@ -3,8 +3,10 @@ function cleanup() { set +e pkill -f anvil + rm -rf /tmp/anvil1 rm -rf /tmp/anvil2 rm -rf /tmp/anvil3 + rm -f ./test-configs/anvil/chains/anvil1/addresses.yaml rm -f ./test-configs/anvil/chains/anvil2/addresses.yaml rm -f ./test-configs/anvil/chains/anvil3/addresses.yaml set -e @@ -12,7 +14,9 @@ function cleanup() { cleanup -echo "Starting anvil2 and anvil3 chain for E2E tests" +echo "Starting anvil1, anvil2 and anvil3 chain for E2E tests" +# Anvil1 should be used only for core commands e2e testing to avoid interfierence with other e2e tests +anvil --chain-id 31337 -p 8545 --state /tmp/anvil1/state --gas-price 1 > /dev/null & anvil --chain-id 31338 -p 8555 --state /tmp/anvil2/state --gas-price 1 > /dev/null & anvil --chain-id 31347 -p 8600 --state /tmp/anvil3/state --gas-price 1 > /dev/null & From cdadfe44a71410911b0758a0b3fa41070d557c95 Mon Sep 17 00:00:00 2001 From: xeno097 Date: Fri, 25 Oct 2024 16:48:21 -0400 Subject: [PATCH 18/25] test(cli): added e2e tests for the hyperlane core commands --- typescript/cli/src/tests/core.e2e-test.ts | 156 ++++++++++++++++++++++ 1 file changed, 156 insertions(+) create mode 100644 typescript/cli/src/tests/core.e2e-test.ts diff --git a/typescript/cli/src/tests/core.e2e-test.ts b/typescript/cli/src/tests/core.e2e-test.ts new file mode 100644 index 0000000000..1fe6853589 --- /dev/null +++ b/typescript/cli/src/tests/core.e2e-test.ts @@ -0,0 +1,156 @@ +import { expect } from 'chai'; +import { Signer, Wallet } from 'ethers'; + +import { + CoreConfig, + ProtocolFeeHookConfig, + randomAddress, +} from '@hyperlane-xyz/sdk'; +import { Address } from '@hyperlane-xyz/utils'; + +import { readYamlOrJson, writeYamlOrJson } from '../utils/files.js'; + +import { + hyperlaneCoreApply, + hyperlaneCoreDeploy, + readCoreConfig, +} from './commands/core.js'; +import { ANVIL_KEY } from './commands/helpers.js'; + +const CHAIN_NAME = 'anvil1'; + +const EXAMPLES_PATH = './examples'; +const CORE_CONFIG_PATH = `${EXAMPLES_PATH}/core-config.yaml`; + +const TEMP_PATH = '/tmp'; // /temp gets removed at the end of all-test.sh +const CORE_READ_CONFIG_PATH = `${TEMP_PATH}/anvil1/core-config-read.yaml`; + +const TEST_TIMEOUT = 100_000; // Long timeout since these tests can take a while +describe('hyperlane core e2e tests', async function () { + this.timeout(TEST_TIMEOUT); + + let initialOwnerAddress: Address; + before(async () => { + const signer: Signer = new Wallet(ANVIL_KEY); + + initialOwnerAddress = await signer.getAddress(); + }); + + describe('core.deploy', () => { + it('should create a core deployment with the signer as the mailbox owner', async () => { + await hyperlaneCoreDeploy(CHAIN_NAME, CORE_CONFIG_PATH); + + const coreConfig: CoreConfig = await readCoreConfig( + CHAIN_NAME, + CORE_READ_CONFIG_PATH, + ); + + expect(coreConfig.owner).to.equal(initialOwnerAddress); + expect(coreConfig.proxyAdmin?.owner).to.equal(initialOwnerAddress); + // Assuming that the ProtocolFeeHook is used for deployment + expect((coreConfig.requiredHook as ProtocolFeeHookConfig).owner).to.equal( + initialOwnerAddress, + ); + }); + + it('should create a core deployment with the mailbox owner set to the address in the config', async () => { + const coreConfig: CoreConfig = await readYamlOrJson(CORE_CONFIG_PATH); + + const newOwner = randomAddress().toLowerCase(); + + coreConfig.owner = newOwner; + writeYamlOrJson(CORE_READ_CONFIG_PATH, coreConfig); + + // Deploy the core contracts with the updated mailbox owner + await hyperlaneCoreDeploy(CHAIN_NAME, CORE_READ_CONFIG_PATH); + + // Verify that the owner has been set correctly without modifying any other owner values + const updatedConfig: CoreConfig = await readCoreConfig( + CHAIN_NAME, + CORE_READ_CONFIG_PATH, + ); + + expect(updatedConfig.owner.toLowerCase()).to.equal(newOwner); + expect(updatedConfig.proxyAdmin?.owner).to.equal(initialOwnerAddress); + // Assuming that the ProtocolFeeHook is used for deployment + expect( + (updatedConfig.requiredHook as ProtocolFeeHookConfig).owner, + ).to.equal(initialOwnerAddress); + }); + + it('should create a core deployment with ProxyAdmin owner of the mailbox set to the address in the config', async () => { + const coreConfig: CoreConfig = await readYamlOrJson(CORE_CONFIG_PATH); + + const newOwner = randomAddress().toLowerCase(); + + coreConfig.proxyAdmin = { owner: newOwner }; + writeYamlOrJson(CORE_READ_CONFIG_PATH, coreConfig); + + // Deploy the core contracts with the updated mailbox owner + await hyperlaneCoreDeploy(CHAIN_NAME, CORE_READ_CONFIG_PATH); + + // Verify that the owner has been set correctly without modifying any other owner values + const updatedConfig: CoreConfig = await readCoreConfig( + CHAIN_NAME, + CORE_READ_CONFIG_PATH, + ); + + expect(updatedConfig.owner).to.equal(initialOwnerAddress); + expect(updatedConfig.proxyAdmin?.owner.toLowerCase()).to.equal(newOwner); + // Assuming that the ProtocolFeeHook is used for deployment + expect( + (updatedConfig.requiredHook as ProtocolFeeHookConfig).owner, + ).to.equal(initialOwnerAddress); + }); + }); + + describe('core.apply', () => { + it('should update the mailbox owner', async () => { + await hyperlaneCoreDeploy(CHAIN_NAME, CORE_CONFIG_PATH); + const coreConfig: CoreConfig = await readCoreConfig( + CHAIN_NAME, + CORE_READ_CONFIG_PATH, + ); + expect(coreConfig.owner).to.equal(initialOwnerAddress); + const newOwner = randomAddress().toLowerCase(); + coreConfig.owner = newOwner; + writeYamlOrJson(CORE_READ_CONFIG_PATH, coreConfig); + await hyperlaneCoreApply(CHAIN_NAME, CORE_READ_CONFIG_PATH); + // Verify that the owner has been set correctly without modifying any other owner values + const updatedConfig: CoreConfig = await readCoreConfig( + CHAIN_NAME, + CORE_READ_CONFIG_PATH, + ); + expect(updatedConfig.owner.toLowerCase()).to.equal(newOwner); + expect(updatedConfig.proxyAdmin?.owner).to.equal(initialOwnerAddress); + // Assuming that the ProtocolFeeHook is used for deployment + expect( + (updatedConfig.requiredHook as ProtocolFeeHookConfig).owner, + ).to.equal(initialOwnerAddress); + }); + + it('should update the ProxyAdmin owner for the mailbox', async () => { + await hyperlaneCoreDeploy(CHAIN_NAME, CORE_CONFIG_PATH); + const coreConfig: CoreConfig = await readCoreConfig( + CHAIN_NAME, + CORE_READ_CONFIG_PATH, + ); + expect(coreConfig.owner).to.equal(initialOwnerAddress); + const newOwner = randomAddress().toLowerCase(); + coreConfig.proxyAdmin!.owner = newOwner; + writeYamlOrJson(CORE_READ_CONFIG_PATH, coreConfig); + await hyperlaneCoreApply(CHAIN_NAME, CORE_READ_CONFIG_PATH); + // Verify that the owner has been set correctly without modifying any other owner values + const updatedConfig: CoreConfig = await readCoreConfig( + CHAIN_NAME, + CORE_READ_CONFIG_PATH, + ); + expect(updatedConfig.owner).to.equal(initialOwnerAddress); + expect(updatedConfig.proxyAdmin?.owner.toLowerCase()).to.equal(newOwner); + // Assuming that the ProtocolFeeHook is used for deployment + expect( + (updatedConfig.requiredHook as ProtocolFeeHookConfig).owner, + ).to.equal(initialOwnerAddress); + }); + }); +}); From 46c1dfb6c6d7ca8834e0186d980a0fa9f3afefa3 Mon Sep 17 00:00:00 2001 From: xeno097 Date: Fri, 25 Oct 2024 17:10:34 -0400 Subject: [PATCH 19/25] refactor(cli): removed ica router proxy admin setting from core init command --- typescript/cli/src/config/core.ts | 14 +------------- typescript/sdk/src/core/EvmCoreModule.ts | 4 +--- typescript/sdk/src/core/schemas.ts | 5 ++--- 3 files changed, 4 insertions(+), 19 deletions(-) diff --git a/typescript/cli/src/config/core.ts b/typescript/cli/src/config/core.ts index 73f2649c4d..92e6fc6882 100644 --- a/typescript/cli/src/config/core.ts +++ b/typescript/cli/src/config/core.ts @@ -49,7 +49,7 @@ export async function createCoreDeployConfig({ : await createTrustedRelayerConfig(context, advanced); let defaultHook: HookConfig, requiredHook: HookConfig; - let proxyAdmin: OwnableConfig, interchainAccountRouter: OwnableConfig; + let proxyAdmin: OwnableConfig; if (advanced) { defaultHook = await createHookConfig({ context, @@ -69,23 +69,12 @@ export async function createCoreDeployConfig({ SIGNER_PROMPT_LABEL, ), }; - interchainAccountRouter = { - owner: await detectAndConfirmOrPrompt( - async () => context.signer?.getAddress(), - ENTER_DESIRED_VALUE_MSG, - 'ICA Router owner address', - SIGNER_PROMPT_LABEL, - ), - }; } else { defaultHook = await createMerkleTreeConfig(); requiredHook = await createProtocolFeeConfig(context, advanced); proxyAdmin = { owner, }; - interchainAccountRouter = { - owner, - }; } try { @@ -95,7 +84,6 @@ export async function createCoreDeployConfig({ defaultHook, requiredHook, proxyAdmin, - interchainAccountRouter, }); logBlue(`Core config is valid, writing to file ${configFilePath}:\n`); log(indentYamlOrJson(yamlStringify(coreConfig, null, 2), 4)); diff --git a/typescript/sdk/src/core/EvmCoreModule.ts b/typescript/sdk/src/core/EvmCoreModule.ts index d86548d83f..96d1088b55 100644 --- a/typescript/sdk/src/core/EvmCoreModule.ts +++ b/typescript/sdk/src/core/EvmCoreModule.ts @@ -306,9 +306,7 @@ export class EvmCoreModule extends HyperlaneModule< multiProvider: multiProvider, config: { mailbox: mailbox.address, - owner: - config.interchainAccountRouter?.owner ?? - (await multiProvider.getSigner(chain).getAddress()), + owner: await multiProvider.getSigner(chain).getAddress(), }, contractVerifier, }) diff --git a/typescript/sdk/src/core/schemas.ts b/typescript/sdk/src/core/schemas.ts index b196b5d84c..470df95ab0 100644 --- a/typescript/sdk/src/core/schemas.ts +++ b/typescript/sdk/src/core/schemas.ts @@ -9,10 +9,9 @@ export const CoreConfigSchema = OwnableSchema.extend({ defaultIsm: IsmConfigSchema, defaultHook: HookConfigSchema, requiredHook: HookConfigSchema, - // These fields are set as optional because the old core config - // did not have them and we want to maintain backward compatibility + // This field is set as optional because the old core config + // did not have it and we want to maintain backward compatibility proxyAdmin: DeployedOwnableSchema.optional(), - interchainAccountRouter: DeployedOwnableSchema.optional(), }); export const DeployedCoreAddressesSchema = ProxyFactoryFactoriesSchema.extend({ From d58f6a719a39ec80d5a96a6c02e900ecc59a9c4c Mon Sep 17 00:00:00 2001 From: xeno097 Date: Fri, 25 Oct 2024 17:43:56 -0400 Subject: [PATCH 20/25] docs(changeset): Add support for updating the mailbox proxy admin owner --- .changeset/dry-ties-approve.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .changeset/dry-ties-approve.md diff --git a/.changeset/dry-ties-approve.md b/.changeset/dry-ties-approve.md new file mode 100644 index 0000000000..88334d522f --- /dev/null +++ b/.changeset/dry-ties-approve.md @@ -0,0 +1,7 @@ +--- +'@hyperlane-xyz/infra': minor +'@hyperlane-xyz/cli': minor +'@hyperlane-xyz/sdk': minor +--- + +Add support for updating the mailbox proxy admin owner From 1e7c3045e30d29c1cdd61ee6d300335bf8d8f70a Mon Sep 17 00:00:00 2001 From: xeno097 Date: Fri, 1 Nov 2024 12:25:36 -0400 Subject: [PATCH 21/25] chore: fix main merge dup code --- .../sdk/src/token/EvmERC20WarpModule.ts | 34 ------------------- 1 file changed, 34 deletions(-) diff --git a/typescript/sdk/src/token/EvmERC20WarpModule.ts b/typescript/sdk/src/token/EvmERC20WarpModule.ts index fe912d0122..cb7e3965d7 100644 --- a/typescript/sdk/src/token/EvmERC20WarpModule.ts +++ b/typescript/sdk/src/token/EvmERC20WarpModule.ts @@ -14,7 +14,6 @@ import { addressToBytes32, assert, deepEquals, - eqAddress, isObjEmpty, objMap, rootLogger, @@ -115,7 +114,6 @@ export class EvmERC20WarpModule extends HyperlaneModule< expectedConfig, this.domainId, ), - ...this.updateProxyAdminOwnershipTxs(actualConfig, expectedConfig), ); return transactions; @@ -294,38 +292,6 @@ export class EvmERC20WarpModule extends HyperlaneModule< ); } - updateProxyAdminOwnershipTxs( - actualConfig: Readonly, - expectedConfig: Readonly, - ): AnnotatedEV5Transaction[] { - const transactions: AnnotatedEV5Transaction[] = []; - - // Return early because old warp config files did not have the - // proxyAdmin property - if (!expectedConfig.proxyAdmin) { - return transactions; - } - - const actualProxyAdmin = actualConfig.proxyAdmin!; - assert( - eqAddress(actualProxyAdmin.address!, expectedConfig.proxyAdmin.address!), - `ProxyAdmin contract addresses do not match. Expected ${expectedConfig.proxyAdmin.address}, got ${actualProxyAdmin.address}`, - ); - - transactions.push( - // Internally the createTransferOwnershipTx method already checks if the - // two owner values are the same and produces an empty tx batch if they are - ...transferOwnershipTransactions( - this.domainId, - actualProxyAdmin.address!, - actualProxyAdmin, - expectedConfig.proxyAdmin, - ), - ); - - return transactions; - } - /** * Updates or deploys the ISM using the provided configuration. * From 872cb395fbc5d30170c52776a6d2949e122b95a9 Mon Sep 17 00:00:00 2001 From: xeno097 Date: Fri, 1 Nov 2024 16:37:48 -0400 Subject: [PATCH 22/25] chore: lee pr changes --- typescript/cli/src/tests/core.e2e-test.ts | 2 +- typescript/sdk/src/core/EvmCoreModule.ts | 2 +- typescript/sdk/src/core/EvmCoreReader.ts | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/typescript/cli/src/tests/core.e2e-test.ts b/typescript/cli/src/tests/core.e2e-test.ts index 1fe6853589..47668700b7 100644 --- a/typescript/cli/src/tests/core.e2e-test.ts +++ b/typescript/cli/src/tests/core.e2e-test.ts @@ -17,7 +17,7 @@ import { } from './commands/core.js'; import { ANVIL_KEY } from './commands/helpers.js'; -const CHAIN_NAME = 'anvil1'; +const CHAIN_NAME = 'anvil2'; const EXAMPLES_PATH = './examples'; const CORE_CONFIG_PATH = `${EXAMPLES_PATH}/core-config.yaml`; diff --git a/typescript/sdk/src/core/EvmCoreModule.ts b/typescript/sdk/src/core/EvmCoreModule.ts index dd970e18f2..13fcb588d9 100644 --- a/typescript/sdk/src/core/EvmCoreModule.ts +++ b/typescript/sdk/src/core/EvmCoreModule.ts @@ -348,7 +348,7 @@ export class EvmCoreModule extends HyperlaneModule< const currentProxyOwner = await proxyAdmin.owner(); if ( config?.proxyAdmin?.owner && - config.proxyAdmin.owner !== currentProxyOwner + !eqAddress(config.proxyAdmin.owner, currentProxyOwner) ) { await multiProvider.sendTransaction(chainName, { annotation: `Transferring ownership of ProxyAdmin to the configured address ${config.proxyAdmin.owner}`, diff --git a/typescript/sdk/src/core/EvmCoreReader.ts b/typescript/sdk/src/core/EvmCoreReader.ts index 392e4f1823..0c0a7887a1 100644 --- a/typescript/sdk/src/core/EvmCoreReader.ts +++ b/typescript/sdk/src/core/EvmCoreReader.ts @@ -64,7 +64,7 @@ export class EvmCoreReader implements CoreReader { defaultIsm: this.evmIsmReader.deriveIsmConfig(defaultIsm), defaultHook: this.evmHookReader.deriveHookConfig(defaultHook), requiredHook: this.evmHookReader.deriveHookConfig(requiredHook), - proxyAdmin: this._getProxyAdminConfig(mailboxProxyAdmin), + proxyAdmin: this.getProxyAdminConfig(mailboxProxyAdmin), }, async (_, readerCall) => { try { @@ -83,7 +83,7 @@ export class EvmCoreReader implements CoreReader { return results as CoreConfig; } - private async _getProxyAdminConfig( + private async getProxyAdminConfig( proxyAdminAddress: Address, ): Promise { const instance = ProxyAdmin__factory.connect( From f204163074faeab635b0401097f5e26370587d28 Mon Sep 17 00:00:00 2001 From: xeno097 Date: Fri, 1 Nov 2024 22:14:08 -0400 Subject: [PATCH 23/25] test: maybe fix e2e test issues --- typescript/cli/scripts/run-e2e-test.sh | 6 +----- typescript/cli/src/tests/warp-deploy.e2e-test.ts | 11 ++++++++--- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/typescript/cli/scripts/run-e2e-test.sh b/typescript/cli/scripts/run-e2e-test.sh index 44b04902b1..e398253b2e 100755 --- a/typescript/cli/scripts/run-e2e-test.sh +++ b/typescript/cli/scripts/run-e2e-test.sh @@ -3,10 +3,8 @@ function cleanup() { set +e pkill -f anvil - rm -rf /tmp/anvil1 rm -rf /tmp/anvil2 rm -rf /tmp/anvil3 - rm -f ./test-configs/anvil/chains/anvil1/addresses.yaml rm -f ./test-configs/anvil/chains/anvil2/addresses.yaml rm -f ./test-configs/anvil/chains/anvil3/addresses.yaml set -e @@ -14,9 +12,7 @@ function cleanup() { cleanup -echo "Starting anvil1, anvil2 and anvil3 chain for E2E tests" -# Anvil1 should be used only for core commands e2e testing to avoid interfierence with other e2e tests -anvil --chain-id 31337 -p 8545 --state /tmp/anvil1/state --gas-price 1 > /dev/null & +echo "Starting anvil2 and anvil3 chain for E2E tests" anvil --chain-id 31338 -p 8555 --state /tmp/anvil2/state --gas-price 1 > /dev/null & anvil --chain-id 31347 -p 8600 --state /tmp/anvil3/state --gas-price 1 > /dev/null & diff --git a/typescript/cli/src/tests/warp-deploy.e2e-test.ts b/typescript/cli/src/tests/warp-deploy.e2e-test.ts index 6263f70cb7..9971878eb0 100644 --- a/typescript/cli/src/tests/warp-deploy.e2e-test.ts +++ b/typescript/cli/src/tests/warp-deploy.e2e-test.ts @@ -34,6 +34,7 @@ const WARP_CORE_CONFIG_PATH_2_3 = `${REGISTRY_PATH}/deployments/warp_routes/VAUL const TEST_TIMEOUT = 60_000; // Long timeout since these tests can take a while describe('WarpDeploy e2e tests', async function () { let chain2Addresses: ChainAddresses = {}; + let chain3Addresses: ChainAddresses = {}; let token: any; let vault: any; @@ -46,7 +47,11 @@ describe('WarpDeploy e2e tests', async function () { ANVIL_KEY, ); - await deployOrUseExistingCore(CHAIN_NAME_3, CORE_CONFIG_PATH, ANVIL_KEY); + chain3Addresses = await deployOrUseExistingCore( + CHAIN_NAME_3, + CORE_CONFIG_PATH, + ANVIL_KEY, + ); token = await deployToken(ANVIL_KEY, CHAIN_NAME_2); vault = await deploy4626Vault(ANVIL_KEY, CHAIN_NAME_2, token.address); @@ -81,8 +86,8 @@ describe('WarpDeploy e2e tests', async function () { }, [CHAIN_NAME_3]: { type: TokenType.syntheticRebase, - mailbox: chain2Addresses.mailbox, - owner: chain2Addresses.mailbox, + mailbox: chain3Addresses.mailbox, + owner: chain3Addresses.mailbox, collateralChainName: CHAIN_NAME_2, }, }; From 30cd02ef8c6d762074327641152bea5c2ea0bc4b Mon Sep 17 00:00:00 2001 From: xeno097 Date: Mon, 4 Nov 2024 18:36:49 -0400 Subject: [PATCH 24/25] feat(sdk,cli): added logic to update proxy admin contract + test --- typescript/cli/src/tests/core.e2e-test.ts | 51 +++++++++++++++++-- typescript/sdk/src/core/EvmCoreModule.ts | 11 ++-- typescript/sdk/src/deploy/proxy.ts | 49 +++++++++++------- .../sdk/src/token/EvmERC20WarpModule.ts | 11 ++-- 4 files changed, 95 insertions(+), 27 deletions(-) diff --git a/typescript/cli/src/tests/core.e2e-test.ts b/typescript/cli/src/tests/core.e2e-test.ts index 47668700b7..9d43244792 100644 --- a/typescript/cli/src/tests/core.e2e-test.ts +++ b/typescript/cli/src/tests/core.e2e-test.ts @@ -1,6 +1,7 @@ import { expect } from 'chai'; -import { Signer, Wallet } from 'ethers'; +import { Signer, Wallet, ethers } from 'ethers'; +import { ProxyAdmin__factory } from '@hyperlane-xyz/core'; import { CoreConfig, ProtocolFeeHookConfig, @@ -15,7 +16,7 @@ import { hyperlaneCoreDeploy, readCoreConfig, } from './commands/core.js'; -import { ANVIL_KEY } from './commands/helpers.js'; +import { ANVIL_KEY, REGISTRY_PATH } from './commands/helpers.js'; const CHAIN_NAME = 'anvil2'; @@ -23,15 +24,26 @@ const EXAMPLES_PATH = './examples'; const CORE_CONFIG_PATH = `${EXAMPLES_PATH}/core-config.yaml`; const TEMP_PATH = '/tmp'; // /temp gets removed at the end of all-test.sh -const CORE_READ_CONFIG_PATH = `${TEMP_PATH}/anvil1/core-config-read.yaml`; +const CORE_READ_CONFIG_PATH = `${TEMP_PATH}/anvil2/core-config-read.yaml`; const TEST_TIMEOUT = 100_000; // Long timeout since these tests can take a while describe('hyperlane core e2e tests', async function () { this.timeout(TEST_TIMEOUT); + let signer: Signer; let initialOwnerAddress: Address; + before(async () => { - const signer: Signer = new Wallet(ANVIL_KEY); + const chainMetadata: any = readYamlOrJson( + `${REGISTRY_PATH}/chains/${CHAIN_NAME}/metadata.yaml`, + ); + + const provider = new ethers.providers.JsonRpcProvider( + chainMetadata.rpcUrls[0].http, + ); + + const wallet = new Wallet(ANVIL_KEY); + signer = wallet.connect(provider); initialOwnerAddress = await signer.getAddress(); }); @@ -129,6 +141,35 @@ describe('hyperlane core e2e tests', async function () { ).to.equal(initialOwnerAddress); }); + it('should update the ProxyAdmin to a new one for the mailbox', async () => { + await hyperlaneCoreDeploy(CHAIN_NAME, CORE_CONFIG_PATH); + const coreConfig: CoreConfig = await readCoreConfig( + CHAIN_NAME, + CORE_READ_CONFIG_PATH, + ); + expect(coreConfig.owner).to.equal(initialOwnerAddress); + + const proxyFactory = new ProxyAdmin__factory().connect(signer); + const deployTx = await proxyFactory.deploy(); + const newProxyAdmin = await deployTx.deployed(); + coreConfig.proxyAdmin!.address = newProxyAdmin.address; + + writeYamlOrJson(CORE_READ_CONFIG_PATH, coreConfig); + await hyperlaneCoreApply(CHAIN_NAME, CORE_READ_CONFIG_PATH); + + // Verify that the owner has been set correctly without modifying any other owner values + const updatedConfig: CoreConfig = await readCoreConfig( + CHAIN_NAME, + CORE_READ_CONFIG_PATH, + ); + expect(updatedConfig.owner).to.equal(initialOwnerAddress); + expect(updatedConfig.proxyAdmin?.address).to.equal(newProxyAdmin.address); + // Assuming that the ProtocolFeeHook is used for deployment + expect( + (updatedConfig.requiredHook as ProtocolFeeHookConfig).owner, + ).to.equal(initialOwnerAddress); + }); + it('should update the ProxyAdmin owner for the mailbox', async () => { await hyperlaneCoreDeploy(CHAIN_NAME, CORE_CONFIG_PATH); const coreConfig: CoreConfig = await readCoreConfig( @@ -136,10 +177,12 @@ describe('hyperlane core e2e tests', async function () { CORE_READ_CONFIG_PATH, ); expect(coreConfig.owner).to.equal(initialOwnerAddress); + const newOwner = randomAddress().toLowerCase(); coreConfig.proxyAdmin!.owner = newOwner; writeYamlOrJson(CORE_READ_CONFIG_PATH, coreConfig); await hyperlaneCoreApply(CHAIN_NAME, CORE_READ_CONFIG_PATH); + // Verify that the owner has been set correctly without modifying any other owner values const updatedConfig: CoreConfig = await readCoreConfig( CHAIN_NAME, diff --git a/typescript/sdk/src/core/EvmCoreModule.ts b/typescript/sdk/src/core/EvmCoreModule.ts index 13fcb588d9..309f9956b9 100644 --- a/typescript/sdk/src/core/EvmCoreModule.ts +++ b/typescript/sdk/src/core/EvmCoreModule.ts @@ -5,6 +5,7 @@ import { } from '@hyperlane-xyz/core'; import { Address, + ChainId, Domain, ProtocolType, eqAddress, @@ -27,7 +28,7 @@ import { ProxyFactoryFactories, proxyFactoryFactories, } from '../deploy/contracts.js'; -import { proxyAdminOwnershipUpdateTxs } from '../deploy/proxy.js'; +import { proxyAdminUpdateTxs } from '../deploy/proxy.js'; import { ContractVerifier } from '../deploy/verify/ContractVerifier.js'; import { HookFactories } from '../hook/contracts.js'; import { EvmIsmModule } from '../ism/EvmIsmModule.js'; @@ -61,6 +62,8 @@ export class EvmCoreModule extends HyperlaneModule< // return a number, and EVM the domainId and chainId are the same. public readonly domainId: Domain; + public readonly chainId: ChainId; + constructor( protected readonly multiProvider: MultiProvider, args: HyperlaneModuleParams, @@ -69,6 +72,7 @@ export class EvmCoreModule extends HyperlaneModule< this.coreReader = new EvmCoreReader(multiProvider, this.args.chain); this.chainName = this.multiProvider.getChainName(this.args.chain); this.domainId = multiProvider.getDomainId(args.chain); + this.chainId = multiProvider.getChainId(args.chain); } /** @@ -96,10 +100,11 @@ export class EvmCoreModule extends HyperlaneModule< transactions.push( ...(await this.createDefaultIsmUpdateTxs(actualConfig, expectedConfig)), ...this.createMailboxOwnerUpdateTxs(actualConfig, expectedConfig), - ...proxyAdminOwnershipUpdateTxs( + ...proxyAdminUpdateTxs( + this.chainId, + this.args.addresses.mailbox, actualConfig, expectedConfig, - this.domainId, ), ); diff --git a/typescript/sdk/src/deploy/proxy.ts b/typescript/sdk/src/deploy/proxy.ts index 204475b7cd..ed9767225e 100644 --- a/typescript/sdk/src/deploy/proxy.ts +++ b/typescript/sdk/src/deploy/proxy.ts @@ -1,6 +1,7 @@ import { ethers } from 'ethers'; -import { Address, assert, eqAddress } from '@hyperlane-xyz/utils'; +import { ProxyAdmin__factory } from '@hyperlane-xyz/core'; +import { Address, ChainId, eqAddress } from '@hyperlane-xyz/utils'; import { transferOwnershipTransactions } from '../contracts/contracts.js'; import { AnnotatedEV5Transaction } from '../providers/ProviderType.js'; @@ -73,10 +74,11 @@ export async function isProxy( return !eqAddress(admin, ethers.constants.AddressZero); } -export function proxyAdminOwnershipUpdateTxs( +export function proxyAdminUpdateTxs( + chainId: ChainId, + proxyAddress: Address, actualConfig: Readonly<{ proxyAdmin?: DeployedOwnableConfig }>, expectedConfig: Readonly<{ proxyAdmin?: DeployedOwnableConfig }>, - chainId: number, ): AnnotatedEV5Transaction[] { const transactions: AnnotatedEV5Transaction[] = []; @@ -87,21 +89,34 @@ export function proxyAdminOwnershipUpdateTxs( } const actualProxyAdmin = actualConfig.proxyAdmin!; - assert( - eqAddress(actualProxyAdmin.address!, expectedConfig.proxyAdmin.address), - `ProxyAdmin contract addresses do not match. Expected ${expectedConfig.proxyAdmin.address}, got ${actualProxyAdmin.address}`, - ); + const parsedChainId = + typeof chainId === 'string' ? parseInt(chainId) : chainId; - transactions.push( - // Internally the createTransferOwnershipTx method already checks if the - // two owner values are the same and produces an empty tx batch if they are - ...transferOwnershipTransactions( - chainId, - actualProxyAdmin.address!, - actualProxyAdmin, - expectedConfig.proxyAdmin, - ), - ); + if ( + actualProxyAdmin.address && + actualProxyAdmin.address !== expectedConfig.proxyAdmin.address + ) { + transactions.push({ + chainId: parsedChainId, + annotation: `Updating ProxyAdmin for proxy at "${proxyAddress}" from "${actualProxyAdmin.address}" to "${expectedConfig.proxyAdmin.address}"`, + to: actualProxyAdmin.address, + data: ProxyAdmin__factory.createInterface().encodeFunctionData( + 'changeProxyAdmin(address,address)', + [proxyAddress, expectedConfig.proxyAdmin.address], + ), + }); + } else { + transactions.push( + // Internally the createTransferOwnershipTx method already checks if the + // two owner values are the same and produces an empty tx batch if they are + ...transferOwnershipTransactions( + parsedChainId, + actualProxyAdmin.address!, + actualProxyAdmin, + expectedConfig.proxyAdmin, + ), + ); + } return transactions; } diff --git a/typescript/sdk/src/token/EvmERC20WarpModule.ts b/typescript/sdk/src/token/EvmERC20WarpModule.ts index cb7e3965d7..317a26c716 100644 --- a/typescript/sdk/src/token/EvmERC20WarpModule.ts +++ b/typescript/sdk/src/token/EvmERC20WarpModule.ts @@ -9,6 +9,7 @@ import { buildArtifact as coreBuildArtifact } from '@hyperlane-xyz/core/buildArt import { ContractVerifier, ExplorerLicenseType } from '@hyperlane-xyz/sdk'; import { Address, + ChainId, Domain, ProtocolType, addressToBytes32, @@ -24,7 +25,7 @@ import { HyperlaneModule, HyperlaneModuleParams, } from '../core/AbstractHyperlaneModule.js'; -import { proxyAdminOwnershipUpdateTxs } from '../deploy/proxy.js'; +import { proxyAdminUpdateTxs } from '../deploy/proxy.js'; import { EvmIsmModule } from '../ism/EvmIsmModule.js'; import { DerivedIsmConfig } from '../ism/EvmIsmReader.js'; import { MultiProvider } from '../providers/MultiProvider.js'; @@ -51,6 +52,8 @@ export class EvmERC20WarpModule extends HyperlaneModule< // return a number, and EVM the domainId and chainId are the same. public readonly domainId: Domain; + public readonly chainId: ChainId; + constructor( protected readonly multiProvider: MultiProvider, args: HyperlaneModuleParams< @@ -64,6 +67,7 @@ export class EvmERC20WarpModule extends HyperlaneModule< super(args); this.reader = new EvmERC20WarpRouteReader(multiProvider, args.chain); this.domainId = multiProvider.getDomainId(args.chain); + this.chainId = multiProvider.getChainId(args.chain); this.contractVerifier ??= new ContractVerifier( multiProvider, {}, @@ -109,10 +113,11 @@ export class EvmERC20WarpModule extends HyperlaneModule< ...this.createRemoteRoutersUpdateTxs(actualConfig, expectedConfig), ...this.createSetDestinationGasUpdateTxs(actualConfig, expectedConfig), ...this.createOwnershipUpdateTxs(actualConfig, expectedConfig), - ...proxyAdminOwnershipUpdateTxs( + ...proxyAdminUpdateTxs( + this.chainId, + this.args.addresses.deployedTokenRoute, actualConfig, expectedConfig, - this.domainId, ), ); From 7798c00712c4e8ff958e8bb118078f336ee023fe Mon Sep 17 00:00:00 2001 From: xeno097 Date: Mon, 4 Nov 2024 19:07:57 -0400 Subject: [PATCH 25/25] fix: maybe fix e2e test part 3 --- typescript/cli/src/tests/commands/core.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/typescript/cli/src/tests/commands/core.ts b/typescript/cli/src/tests/commands/core.ts index a6d95b5ed3..24c9109744 100644 --- a/typescript/cli/src/tests/commands/core.ts +++ b/typescript/cli/src/tests/commands/core.ts @@ -45,6 +45,7 @@ export async function hyperlaneCoreApply( --registry ${REGISTRY_PATH} \ --config ${coreOutputPath} \ --chain ${chain} \ + --key ${ANVIL_KEY} \ --verbosity debug \ --yes`; }