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

Warp Route extensions should be idempotent #5317

Open
nambrot opened this issue Jan 28, 2025 · 1 comment
Open

Warp Route extensions should be idempotent #5317

nambrot opened this issue Jan 28, 2025 · 1 comment
Assignees

Comments

@nambrot
Copy link
Contributor

nambrot commented Jan 28, 2025

Warp route extensions are meant to be done idempotently. That means that if you run the same warp apply command with the same deploy config and artifact references (WarpCore config), the outcome should be the same, whether or not whether the user actually submitted the transactions of the first apply run. Unfortunately, that is currently no true.

Specifically, only upon the first invocation, a warp apply command makes it past this if statement

let extendedConfigs: WarpRouteDeployConfig = objFilter(
warpDeployConfig,
(chain, _config): _config is any => !warpCoreChains.includes(chain),
);
const extendedChains = Object.keys(extendedConfigs);
if (extendedChains.length === 0) return [];

After that, executeDeploy will actually deploy the contracts on the new chains, which will make extendedChains to be an empty array.

Only on the first run, we do (rather oddly) a mutation of the remoteRouters table. On the second run, we hit the code path only in [updateExistingWarpRoute](https://github.com/hyperlane-xyz/hyperlane-monorepo/blob/228f7c3d16098f217a8cd9663df27265a2ea8561/typescript/cli/src/deploy/warp.ts#L612). Since the original config does not include the router table, it will not create the enrollment transactions in

if (!expectedConfig.remoteRouters) {
return [];
}

The objective of this ticket is to streamline the warp apply router enrollment handling to
a) Allow warp apply to be idempontent. I.e. no matter what pending transactions are, when warp apply is run
b) Explicitly test these scenarios of (all without having to specify the router table manually in the warp deploy config
i. extending a warp route in one shot
ii. extending a warp route later
iii. extending a warp route after a router was manually unenrolled

For that we probably need to
a) Reduce the number of places where we "infer" the intended remoteRouter table to only 1 place. I.e. remove this extra enrollRemoteRouters function (but maybe there is another reason for why its in there?)

Questions:

  1. Why do we do this mutation of the warp route config remote router table instead of deriving it from the routers passed in [deployedContractsMap](https://github.com/hyperlane-xyz/hyperlane-monorepo/blob/228f7c3d16098f217a8cd9663df27265a2ea8561/typescript/cli/src/deploy/warp.ts#L745)
@nambrot
Copy link
Contributor Author

nambrot commented Jan 29, 2025

The PR for this should absolutely include tests for these scenarios of not finishing the enrollments of the previous invocation and it still working

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Sprint
Development

No branches or pull requests

2 participants