Skip to content

Commit

Permalink
feat: core deploy apply admin proxy ownership fixes (#4767)
Browse files Browse the repository at this point in the history
### Description

This PR updates the `hyperlane core init`, `hyperlane core deploy` and
`hyperlane core apply` commands to allow a user to change ownership of
the mailbox ProxyAdmin contract by setting a value in the config.

### Drive-by changes

- deduped `randomAddress` test util implementations across the `sdk`,
'infra' and `cli` package
- added `anvil1` to the `run-e2e-test.sh` script to test `hyperlane
core` commands in isolation
- implemented the `proxyAdminOwnershipUpdateTxs` to deduplicate proxy
admin ownership tx data generation

### Related issues

- Fixes #4728 

### Backward compatibility

- Yes

### Testing

- Manual
- e2e

### NOTE:

- Merge #4726
first

---------

Co-authored-by: Paul Balaji <10051819+paulbalaji@users.noreply.github.com>
  • Loading branch information
xeno097 and paulbalaji authored Nov 8, 2024
1 parent bf4c779 commit fa42482
Show file tree
Hide file tree
Showing 16 changed files with 438 additions and 84 deletions.
7 changes: 7 additions & 0 deletions .changeset/dry-ties-approve.md
Original file line number Diff line number Diff line change
@@ -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
27 changes: 24 additions & 3 deletions typescript/cli/src/config/core.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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,
Expand All @@ -31,16 +39,17 @@ 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
? await createAdvancedIsmConfig(context)
: await createTrustedRelayerConfig(context, advanced);

let defaultHook: HookConfig, requiredHook: HookConfig;
let proxyAdmin: OwnableConfig;
if (advanced) {
defaultHook = await createHookConfig({
context,
Expand All @@ -52,9 +61,20 @@ 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,
),
};
} else {
defaultHook = await createMerkleTreeConfig();
requiredHook = await createProtocolFeeConfig(context, advanced);
proxyAdmin = {
owner,
};
}

try {
Expand All @@ -63,6 +83,7 @@ export async function createCoreDeployConfig({
defaultIsm,
defaultHook,
requiredHook,
proxyAdmin,
});
logBlue(`Core config is valid, writing to file ${configFilePath}:\n`);
log(indentYamlOrJson(yamlStringify(coreConfig, null, 2), 4));
Expand Down
1 change: 1 addition & 0 deletions typescript/cli/src/deploy/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ interface DeployParams {
interface ApplyParams extends DeployParams {
deployedCoreAddresses: DeployedCoreAddresses;
}

/**
* Executes the core deploy command.
*/
Expand Down
43 changes: 43 additions & 0 deletions typescript/cli/src/tests/commands/core.ts
Original file line number Diff line number Diff line change
@@ -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';

/**
Expand All @@ -17,3 +21,42 @@ 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} \
--key ${ANVIL_KEY} \
--verbosity debug \
--yes`;
}

/**
* Reads the Core deployment config and outputs it to specified output path.
*/
export async function readCoreConfig(
chain: string,
coreConfigPath: string,
): Promise<CoreConfig> {
await hyperlaneCoreRead(chain, coreConfigPath);
return readYamlOrJson(coreConfigPath);
}
199 changes: 199 additions & 0 deletions typescript/cli/src/tests/core.e2e-test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,199 @@
import { expect } from 'chai';
import { Signer, Wallet, ethers } from 'ethers';

import { ProxyAdmin__factory } from '@hyperlane-xyz/core';
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, REGISTRY_PATH } from './commands/helpers.js';

const CHAIN_NAME = 'anvil2';

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}/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 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();
});

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 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(
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);
});
});
});
11 changes: 8 additions & 3 deletions typescript/cli/src/tests/warp-deploy.e2e-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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);
Expand Down Expand Up @@ -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,
},
};
Expand Down
10 changes: 6 additions & 4 deletions typescript/cli/src/utils/input.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,16 @@ import ansiEscapes from 'ansi-escapes';
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 {
ChainName,
DeployedOwnableConfig,
WarpCoreConfig,
} from '@hyperlane-xyz/sdk';
import { Address, isAddress, rootLogger } from '@hyperlane-xyz/utils';

import { readWarpCoreConfig } from '../config/warp.js';
import { CommandContext } from '../context/types.js';
import { logGray } from '../logger.js';
import { logRed } from '../logger.js';
import { logGray, logRed } from '../logger.js';

import { indentYamlOrJson } from './files.js';
import { selectRegistryWarpRoute } from './tokens.js';
Expand Down
Loading

0 comments on commit fa42482

Please sign in to comment.