Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: core deploy apply admin proxy ownership fixes #4767

Open
wants to merge 14 commits into
base: xeno/warp-deploy-apply-ownership-fixes
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
6 changes: 5 additions & 1 deletion typescript/cli/scripts/run-e2e-test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,20 @@
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
}

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 &

Expand Down
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
42 changes: 42 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,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<CoreConfig> {
await hyperlaneCoreRead(chain, coreConfigPath);
return readYamlOrJson(coreConfigPath);
}
156 changes: 156 additions & 0 deletions typescript/cli/src/tests/core.e2e-test.ts
Original file line number Diff line number Diff line change
@@ -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);
});
});
});
6 changes: 2 additions & 4 deletions typescript/infra/test/govern.hardhat-test.ts
Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -27,6 +27,7 @@ import {
TestChainName,
TestCoreApp,
TestCoreDeployer,
randomAddress,
} from '@hyperlane-xyz/sdk';
import { Address, CallData, eqAddress } from '@hyperlane-xyz/utils';

Expand All @@ -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<TestApp, OwnableConfig> {
Expand Down
Loading