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.

Sign up for GitHub

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

refactor: remove mailbox from warp config #5312

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
+93 −104
Open

refactor: remove mailbox from warp config #5312

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
64e6074
feat: Remove mailbox from warp route deploy config
mshojaei-txfusion Jan 28, 2025
e559490
refactor: Improve warp route config validation and rebase token handling
mshojaei-txfusion Jan 28, 2025
da36297
test: Update warp init e2e tests to remove mailbox references
mshojaei-txfusion Jan 28, 2025
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
32 changes: 7 additions & 25 deletions typescript/cli/src/config/warp.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,10 @@ import {
WarpCoreConfigSchema,
WarpRouteDeployConfig,
WarpRouteDeployConfigSchema,
WarpRouteDeployConfigSchemaWithoutMailbox,
WarpRouteDeployConfigWithoutMailbox,
} from '@hyperlane-xyz/sdk';
import {
Address,
assert,
isAddress,
objMap,
promiseObjAll,
} from '@hyperlane-xyz/utils';
import { Address, assert, objMap, promiseObjAll } from '@hyperlane-xyz/utils';

import { CommandContext } from '../context/types.js';
import { errorRed, log, logBlue, logGreen } from '../logger.js';
Expand Down Expand Up @@ -108,7 +104,7 @@ export async function readWarpRouteDeployConfig(
}

export function isValidWarpRouteDeployConfig(config: any) {
return WarpRouteDeployConfigSchema.safeParse(config).success;
return WarpRouteDeployConfigSchemaWithoutMailbox.safeParse(config).success;
}

export async function createWarpRouteDeployConfig({
Expand All @@ -131,7 +127,7 @@ export async function createWarpRouteDeployConfig({
requiresConfirmation: !context.skipConfirmation,
});

const result: WarpRouteDeployConfig = {};
const result: WarpRouteDeployConfigWithoutMailbox = {};
let typeChoices = TYPE_CHOICES;
for (const chain of warpChains) {
logBlue(`${chain}: Configuring warp route...`);
Expand All @@ -142,16 +138,6 @@ export async function createWarpRouteDeployConfig({
'signer',
);

// default to the mailbox from the registry and if not found ask to the user to submit one
const chainAddresses = await context.registry.getChainAddresses(chain);

const mailbox =
chainAddresses?.mailbox ??
(await input({
validate: isAddress,
message: `Could not retrieve mailbox address from the registry for chain "${chain}". Please enter a valid mailbox address:`,
}));

const proxyAdmin: DeployedOwnableConfig = await setProxyAdminConfig(
context,
chain,
Expand Down Expand Up @@ -198,7 +184,6 @@ export async function createWarpRouteDeployConfig({
case TokenType.collateralUri:
case TokenType.fastCollateral:
result[chain] = {
mailbox,
type,
owner,
proxyAdmin,
Expand All @@ -211,7 +196,6 @@ export async function createWarpRouteDeployConfig({
break;
case TokenType.syntheticRebase:
result[chain] = {
mailbox,
type,
owner,
isNft,
Expand All @@ -226,7 +210,6 @@ export async function createWarpRouteDeployConfig({
break;
case TokenType.collateralVaultRebase:
result[chain] = {
mailbox,
type,
owner,
proxyAdmin,
Expand All @@ -241,7 +224,6 @@ export async function createWarpRouteDeployConfig({
break;
case TokenType.collateralVault:
result[chain] = {
mailbox,
type,
owner,
proxyAdmin,
Expand All @@ -254,7 +236,6 @@ export async function createWarpRouteDeployConfig({
break;
default:
result[chain] = {
mailbox,
type,
owner,
proxyAdmin,
Expand All @@ -265,7 +246,8 @@ export async function createWarpRouteDeployConfig({
}

try {
const warpRouteDeployConfig = WarpRouteDeployConfigSchema.parse(result);
const warpRouteDeployConfig =
WarpRouteDeployConfigSchemaWithoutMailbox.parse(result);
logBlue(`Warp Route config is valid, writing to file ${outPath}:\n`);
log(indentYamlOrJson(yamlStringify(warpRouteDeployConfig, null, 2), 4));
writeYamlOrJson(outPath, warpRouteDeployConfig, 'yaml');
Expand Down
37 changes: 7 additions & 30 deletions typescript/cli/src/tests/warp/warp-init.e2e-test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
import { expect } from 'chai';
import { Wallet } from 'ethers';

import { ChainAddresses } from '@hyperlane-xyz/registry';
import {
ChainMap,
ChainName,
TokenType,
WarpRouteDeployConfig,
WarpRouteDeployConfigWithoutMailbox,
} from '@hyperlane-xyz/sdk';
import { Address } from '@hyperlane-xyz/utils';

Expand All @@ -16,15 +14,13 @@ import {
CHAIN_NAME_2,
CHAIN_NAME_3,
CONFIRM_DETECTED_OWNER_STEP,
CORE_CONFIG_PATH,
DEFAULT_E2E_TEST_TIMEOUT,
KeyBoardKeys,
SELECT_ANVIL_2_AND_ANVIL_3_STEPS,
SELECT_ANVIL_2_FROM_MULTICHAIN_PICKER,
SELECT_MAINNET_CHAIN_TYPE_STEP,
TestPromptAction,
WARP_CONFIG_PATH_2,
deployOrUseExistingCore,
deployToken,
handlePrompts,
} from '../commands/helpers.js';
Expand All @@ -33,38 +29,21 @@ import { hyperlaneWarpInit } from '../commands/warp.js';
describe('hyperlane warp init e2e tests', async function () {
this.timeout(2 * DEFAULT_E2E_TEST_TIMEOUT);

let chain2Addresses: ChainAddresses = {};
let chain3Addresses: ChainAddresses = {};
let initialOwnerAddress: Address;
let chainMapAddresses: ChainMap<ChainAddresses> = {};

before(async function () {
[chain2Addresses, chain3Addresses] = await Promise.all([
deployOrUseExistingCore(CHAIN_NAME_2, CORE_CONFIG_PATH, ANVIL_KEY),
deployOrUseExistingCore(CHAIN_NAME_3, CORE_CONFIG_PATH, ANVIL_KEY),
]);

chainMapAddresses = {
[CHAIN_NAME_2]: chain2Addresses,
[CHAIN_NAME_3]: chain3Addresses,
};

const wallet = new Wallet(ANVIL_KEY);
initialOwnerAddress = wallet.address;
});

describe('hyperlane warp init --yes', () => {
function assertWarpConfig(
warpConfig: WarpRouteDeployConfig,
chainMapAddresses: ChainMap<ChainAddresses>,
warpConfig: WarpRouteDeployConfigWithoutMailbox,
chainName: ChainName,
) {
expect(warpConfig[chainName]).not.to.be.undefined;

const chain2TokenConfig = warpConfig[chainName];
expect(chain2TokenConfig.mailbox).equal(
chainMapAddresses[chainName].mailbox,
);
expect(chain2TokenConfig.owner).equal(initialOwnerAddress);
expect(chain2TokenConfig.type).equal(TokenType.native);
}
Expand All @@ -91,10 +70,10 @@ describe('hyperlane warp init e2e tests', async function () {

await handlePrompts(output, steps);

const warpConfig: WarpRouteDeployConfig =
const warpConfig: WarpRouteDeployConfigWithoutMailbox =
readYamlOrJson(WARP_CONFIG_PATH_2);

assertWarpConfig(warpConfig, chainMapAddresses, CHAIN_NAME_2);
assertWarpConfig(warpConfig, CHAIN_NAME_2);
});

it('it should generate a warp deploy config with a 2 chains warp route (native->native)', async function () {
Expand All @@ -119,11 +98,11 @@ describe('hyperlane warp init e2e tests', async function () {

await handlePrompts(output, steps);

const warpConfig: WarpRouteDeployConfig =
const warpConfig: WarpRouteDeployConfigWithoutMailbox =
readYamlOrJson(WARP_CONFIG_PATH_2);

[CHAIN_NAME_2, CHAIN_NAME_3].map((chainName) =>
assertWarpConfig(warpConfig, chainMapAddresses, chainName),
assertWarpConfig(warpConfig, chainName),
);
});

Expand Down Expand Up @@ -159,21 +138,19 @@ describe('hyperlane warp init e2e tests', async function () {

await handlePrompts(output, steps);

const warpConfig: WarpRouteDeployConfig =
const warpConfig: WarpRouteDeployConfigWithoutMailbox =
readYamlOrJson(WARP_CONFIG_PATH_2);

expect(warpConfig[CHAIN_NAME_2]).not.to.be.undefined;

const chain2TokenConfig = warpConfig[CHAIN_NAME_2];
expect(chain2TokenConfig.mailbox).equal(chain2Addresses.mailbox);
expect(chain2TokenConfig.owner).equal(initialOwnerAddress);
expect(chain2TokenConfig.type).equal(TokenType.collateral);
expect((chain2TokenConfig as any).token).equal(erc20Token.address);

expect(warpConfig[CHAIN_NAME_3]).not.to.be.undefined;

const chain3TokenConfig = warpConfig[CHAIN_NAME_3];
expect(chain3TokenConfig.mailbox).equal(chain3Addresses.mailbox);
expect(chain3TokenConfig.owner).equal(initialOwnerAddress);
expect(chain3TokenConfig.type).equal(TokenType.synthetic);
});
Expand Down
2 changes: 2 additions & 0 deletions typescript/sdk/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -587,6 +587,8 @@ export {
WarpRouteDeployConfig,
WarpRouteDeployConfigSchema,
WarpRouteDeployConfigSchemaErrors,
WarpRouteDeployConfigWithoutMailbox,
WarpRouteDeployConfigSchemaWithoutMailbox,
} from './token/types.js';
export {
ChainMap,
Expand Down
126 changes: 77 additions & 49 deletions typescript/sdk/src/token/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,58 +101,86 @@ export const HypTokenRouterConfigSchema = HypTokenConfigSchema.and(
);
export type HypTokenRouterConfig = z.infer<typeof HypTokenRouterConfigSchema>;

export const WarpRouteDeployConfigSchema = z
.record(HypTokenRouterConfigSchema)
.refine((configMap) => {
const entries = Object.entries(configMap);
return (
entries.some(
([_, config]) =>
isCollateralTokenConfig(config) ||
isCollateralRebaseTokenConfig(config) ||
isNativeTokenConfig(config),
) || entries.every(([_, config]) => isTokenMetadata(config))
);
}, WarpRouteDeployConfigSchemaErrors.NO_SYNTHETIC_ONLY)
.transform((warpRouteDeployConfig, ctx) => {
const collateralRebaseEntry = Object.entries(warpRouteDeployConfig).find(
([_, config]) => isCollateralRebaseTokenConfig(config),
);

const syntheticRebaseEntry = Object.entries(warpRouteDeployConfig).find(
([_, config]) => isSyntheticRebaseTokenConfig(config),
);

// Require both collateral rebase and synthetic rebase to be present in the config
if (!collateralRebaseEntry && !syntheticRebaseEntry) {
// Pass through for other token types
return warpRouteDeployConfig;
}

if (
collateralRebaseEntry &&
isCollateralRebasePairedCorrectly(warpRouteDeployConfig)
) {
const collateralChainName = collateralRebaseEntry[0];
return objMap(warpRouteDeployConfig, (_, config) => {
if (config.type === TokenType.syntheticRebase)
config.collateralChainName = collateralChainName;
return config;
}) as Record<string, HypTokenRouterConfig>;
}

ctx.addIssue({
code: z.ZodIssueCode.custom,
message: WarpRouteDeployConfigSchemaErrors.ONLY_SYNTHETIC_REBASE,
export const HypTokenRouterConfigWithoutMailboxSchema =
HypTokenConfigSchema.and(GasRouterConfigSchema.omit({ mailbox: true }));
Comment on lines +104 to +105
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think zod will not error on extraneous fields so wondering why not just remove the mailbox field from the existing schema?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure I understood correctly but it is not possible do remove mailbox from MailBoxClienSchema as it is required for other parts of code, also omit is not supported for HypTokenRouterConfigMailboxSchema


export type HypTokenRouterConfigWithoutMailbox = z.infer<
typeof HypTokenRouterConfigWithoutMailboxSchema
>;

export const WarpRouteDeployConfigSchemaWithoutMailbox = z
.record(HypTokenRouterConfigWithoutMailboxSchema)
.refine(refineTokens, WarpRouteDeployConfigSchemaErrors.NO_SYNTHETIC_ONLY)
.transform((config, ctx) => transformRebaseConfig(config, ctx));

export type WarpRouteDeployConfigWithoutMailbox = z.infer<
typeof WarpRouteDeployConfigSchemaWithoutMailbox
>;

function refineTokens<
T extends HypTokenRouterConfig | HypTokenRouterConfigWithoutMailbox,
>(configMap: Record<string, T>): boolean {
const entries = Object.entries(configMap);
return (
entries.some(
([_, config]) =>
isCollateralTokenConfig(config) ||
isCollateralRebaseTokenConfig(config) ||
isNativeTokenConfig(config),
) || entries.every(([_, config]) => isTokenMetadata(config))
);
}

function transformRebaseConfig<
T extends HypTokenRouterConfig | HypTokenRouterConfigWithoutMailbox,
>(
warpRouteDeployConfig: Record<string, T>,
ctx: z.RefinementCtx,
): Record<string, T> | typeof z.NEVER {
const collateralRebaseEntry = Object.entries(warpRouteDeployConfig).find(
([_, config]) => isCollateralRebaseTokenConfig(config),
);

const syntheticRebaseEntry = Object.entries(warpRouteDeployConfig).find(
([_, config]) => isSyntheticRebaseTokenConfig(config),
);

// Require both collateral rebase and synthetic rebase to be present in the config
if (!collateralRebaseEntry && !syntheticRebaseEntry) {
// Pass through for other token types
return warpRouteDeployConfig;
}

if (
collateralRebaseEntry &&
isCollateralRebasePairedCorrectly(warpRouteDeployConfig)
) {
const collateralChainName = collateralRebaseEntry[0];
return objMap(warpRouteDeployConfig, (_, config) => {
if (config.type === TokenType.syntheticRebase)
config.collateralChainName = collateralChainName;
return config;
});
}

return z.NEVER; // Causes schema validation to throw with above issue
ctx.addIssue({
code: z.ZodIssueCode.custom,
message: WarpRouteDeployConfigSchemaErrors.ONLY_SYNTHETIC_REBASE,
});

return z.NEVER; // Causes schema validation to throw with above issue
}

export const WarpRouteDeployConfigSchema = z
.record(HypTokenRouterConfigSchema)
.refine(refineTokens, WarpRouteDeployConfigSchemaErrors.NO_SYNTHETIC_ONLY)
.transform(transformRebaseConfig);
Comment on lines +174 to +177
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

generally prefer not to include drive-bys like this except because it makes what would otherwise be a very easy to review PR much harder to review
IIUC we have better test coverage on this now so relying on that exercising this

Copy link
Collaborator Author

@mshojaei-txfusion mshojaei-txfusion Jan 29, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not really sure what exactly you mean with drive-bys,
I refactored this section since both WarpRouteDeployConfigSchema and WarpRouteDeployConfigSchemaWithoutMailbox needed to share the same refine and transform functions.


export type WarpRouteDeployConfig = z.infer<typeof WarpRouteDeployConfigSchema>;

function isCollateralRebasePairedCorrectly(
warpRouteDeployConfig: Record<string, HypTokenRouterConfig>,
): boolean {
function isCollateralRebasePairedCorrectly<
T extends HypTokenRouterConfig | HypTokenRouterConfigWithoutMailbox,
>(warpRouteDeployConfig: Record<string, T>): boolean {
// Filter out all the non-collateral rebase configs to check if they are only synthetic rebase tokens
const otherConfigs = Object.entries(warpRouteDeployConfig).filter(
([_, config]) => !isCollateralRebaseTokenConfig(config),
Expand All @@ -161,8 +189,8 @@ function isCollateralRebasePairedCorrectly(
if (otherConfigs.length === 0) return false;

// The other configs MUST be synthetic rebase
const allOthersSynthetic: boolean = otherConfigs.every(
([_, config], _index) => isSyntheticRebaseTokenConfig(config),
const allOthersSynthetic: boolean = otherConfigs.every(([_, config]) =>
isSyntheticRebaseTokenConfig(config),
);
return allOthersSynthetic;
}
Loading