feat(infra): improve RPC rotation UX#7709
Conversation
|
paulbalaji
force-pushed
the
pb/improved-rpc-rotation
branch
4 times, most recently
from
f1cff32 to
0dc9fb0
Compare
paulbalaji
changed the title
paulbalaji
changed the title
paulbalaji
changed the title
paulbalaji
changed the title
paulbalaji
force-pushed
the
pb/improved-rpc-rotation
branch
from
454ead9 to
a679662
Compare
paulbalaji
changed the title
paulbalaji
changed the title
paulbalaji
force-pushed
the
pb/improved-rpc-rotation
branch
from
c6c65af to
f0d24b3
Compare
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Additional Comments (1)
-
typescript/infra/src/warp-monitor/helm.ts, line 16 (link)syntax:
DEFAULT_REGISTRY_URIis imported but never used in this file
6 files reviewed, 1 comment
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📥 CommitsReviewing files that changed from the base of the PR and between 33fafc9878a9627d037023d751a46734613aa2fe and a89d584. 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughAdds a Check Warp Deploy Helm manager and switches the deploy script to use it; introduces Warp Route monitor discovery APIs and a status CLI that fetches pod status and parses monitor logs; refactors interactive K8s refresh flow to separate core infra, warp-monitor, and CronJob selections. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as "status.ts (CLI)"
participant Env as "Env Config"
participant K8s as "kubectl/Helm"
participant Pod as "K8s Pod"
participant Formatter as "Log Formatter"
CLI->>Env: parse env & warpRouteId, validate context
CLI->>K8s: compute helm release & pod name
K8s->>Pod: query pod status
Pod-->>CLI: pod status
CLI->>K8s: kubectl logs --tail N
K8s->>Pod: request logs
Pod-->>CLI: raw log lines
alt lines parse as JSON
CLI->>Formatter: parse, normalize timestamps, extract labels & values
Formatter-->>CLI: colored structured lines + Grafana link
else
CLI-->>CLI: print raw logs fallback
end
CLI-->>CLI: exit
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI Agents
In @typescript/infra/src/warp-monitor/helm.ts:
- Around line 200-207: The WarpRouteMonitorHelmManager is being constructed with
an empty registryCommit which yields a malformed registryUri used by
helmValues(); fix by either passing a real commit/hash (e.g., use
envConfig.registryCommit or another commit field) to the
WarpRouteMonitorHelmManager constructor instead of '' or, if commit may be
absent, update the registryUri getter inside WarpRouteMonitorHelmManager to
handle empty/undefined registryCommit gracefully (build
`${DEFAULT_GITHUB_REGISTRY}` without appending `/tree/` when registryCommit is
falsy) and ensure helmValues() uses that safe registryUri.
🧹 Nitpick comments (3)
typescript/infra/scripts/warp-routes/status.ts (3)
28-30: Donkey, you're doing the same thing twice here.
withWarpRouteIdRequiredalready wrapswithWarpRouteIdand calls.demandOption('warpRouteId')(seeagent-utils.tslines 228-230). No need to call it again.🔎 Suggested simplification
- const { warpRouteId } = await withWarpRouteIdRequired( - yargs(process.argv.slice(2)), - ).demandOption('warpRouteId').argv; + const { warpRouteId } = await withWarpRouteIdRequired( + yargs(process.argv.slice(2)), + ).argv;
43-48: The pod naming convention with "-0" suffix is a fair assumption, just worth documenting.This assumes the warp monitor pods are deployed as StatefulSets with the pod name pattern
{release-name}-0. If deployment strategy changes (e.g., to Deployments with random suffixes), this will break.Might be worth a brief comment explaining this is for StatefulSet pods.
88-90: Usingnew Date().toISOString()as fallback might be a bit misleading.If the time format is unrecognized, this shows the current time rather than the actual log time. Consider logging a placeholder like
'[unknown time]'instead, so operators know the original timestamp couldn't be parsed.🔎 Alternative approach
} else { - timestamp = new Date().toISOString(); + timestamp = '[unknown time]'; }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
typescript/infra/scripts/check/deploy-check-warp-deploy.tstypescript/infra/scripts/warp-routes/deploy-warp-monitor.tstypescript/infra/scripts/warp-routes/status.tstypescript/infra/src/check-warp-deploy/helm.tstypescript/infra/src/utils/rpcUrls.tstypescript/infra/src/warp-monitor/helm.ts
🧰 Additional context used
📓 Path-based instructions (1)
typescript/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
typescript/**/*.{ts,tsx}: UseChainMapfor per-chain configurations in TypeScript
Import types from@hyperlane-xyz/sdkwhen using TypeScript SDK types
Files:
typescript/infra/src/check-warp-deploy/helm.tstypescript/infra/scripts/warp-routes/status.tstypescript/infra/scripts/check/deploy-check-warp-deploy.tstypescript/infra/scripts/warp-routes/deploy-warp-monitor.tstypescript/infra/src/warp-monitor/helm.tstypescript/infra/src/utils/rpcUrls.ts
🧠 Learnings (4)
📓 Common learnings
Learnt from: xeno097
Repo: hyperlane-xyz/hyperlane-monorepo PR: 6486
File: typescript/cli/src/tests/warp/warp-check.e2e-test.ts:490-494
Timestamp: 2025-06-16T11:17:55.750Z
Learning: In the Hyperlane registry, the label part of warp IDs can now be any value that matches the registry's regex requirements, not just actual chain names. This means functions like getCombinedWarpRoutePath can accept descriptive filenames like WARP_DEPLOY_DEFAULT_FILE_NAME as valid label components for constructing registry paths.
📚 Learning: 2025-12-18T22:36:13.445Z
Learnt from: CR
Repo: hyperlane-xyz/hyperlane-monorepo PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-18T22:36:13.445Z
Learning: Most deployments are config-driven; check `typescript/infra/config/` for deployment configuration examples
Applied to files:
typescript/infra/src/check-warp-deploy/helm.tstypescript/infra/scripts/check/deploy-check-warp-deploy.tstypescript/infra/src/warp-monitor/helm.ts
📚 Learning: 2025-12-29T19:45:12.592Z
Learnt from: paulbalaji
Repo: hyperlane-xyz/hyperlane-monorepo PR: 7545
File: typescript/infra/src/rebalancer/helm.ts:82-83
Timestamp: 2025-12-29T19:45:12.592Z
Learning: In the hyperlane-xyz/hyperlane-monorepo repository, Docker image tags in Helm managers (e.g., typescript/infra/src/rebalancer/helm.ts) are intentionally hardcoded for reproducibility and explicit version control, rather than using dynamically generated CI tags.
Applied to files:
typescript/infra/src/check-warp-deploy/helm.tstypescript/infra/scripts/check/deploy-check-warp-deploy.ts
📚 Learning: 2025-06-16T11:17:55.750Z
Learnt from: xeno097
Repo: hyperlane-xyz/hyperlane-monorepo PR: 6486
File: typescript/cli/src/tests/warp/warp-check.e2e-test.ts:490-494
Timestamp: 2025-06-16T11:17:55.750Z
Learning: In the Hyperlane registry, the label part of warp IDs can now be any value that matches the registry's regex requirements, not just actual chain names. This means functions like getCombinedWarpRoutePath can accept descriptive filenames like WARP_DEPLOY_DEFAULT_FILE_NAME as valid label components for constructing registry paths.
Applied to files:
typescript/infra/src/warp-monitor/helm.ts
🧬 Code graph analysis (3)
typescript/infra/scripts/warp-routes/status.ts (4)
typescript/infra/scripts/agent-utils.ts (2)
withWarpRouteIdRequired(229-231)assertCorrectKubeContext(748-761)typescript/infra/scripts/core-utils.ts (1)
getEnvironmentConfig(13-15)typescript/infra/src/utils/helm.ts (1)
getHelmReleaseName(261-274)typescript/infra/src/warp-monitor/helm.ts (1)
WarpRouteMonitorHelmManager(43-262)
typescript/infra/scripts/check/deploy-check-warp-deploy.ts (3)
typescript/infra/scripts/core-utils.ts (1)
getConfigsBasedOnArgs(17-27)typescript/infra/scripts/agent-utils.ts (1)
assertCorrectKubeContext(748-761)typescript/infra/src/check-warp-deploy/helm.ts (1)
CheckWarpDeployHelmManager(12-63)
typescript/infra/src/warp-monitor/helm.ts (4)
typescript/infra/src/config/environment.ts (1)
DeployEnvironment(31-31)typescript/infra/scripts/core-utils.ts (1)
getEnvironmentConfig(13-15)typescript/infra/src/check-warp-deploy/helm.ts (1)
namespace(40-42)typescript/infra/src/utils/utils.ts (1)
execCmdAndParseJson(98-111)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (60)
- GitHub Check: pnpm-test
- GitHub Check: Greptile Review
- GitHub Check: cli-install-test-run
- GitHub Check: cli-evm-e2e-matrix (warp-extend-config)
- GitHub Check: cli-evm-e2e-matrix (warp-read)
- GitHub Check: cli-evm-e2e-matrix (core-read)
- GitHub Check: cli-evm-e2e-matrix (warp-deploy-1)
- GitHub Check: cli-evm-e2e-matrix (warp-check-4)
- GitHub Check: cli-evm-e2e-matrix (warp-extend-basic)
- GitHub Check: cli-evm-e2e-matrix (warp-init)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-submitters)
- GitHub Check: cli-evm-e2e-matrix (warp-check-1)
- GitHub Check: cli-evm-e2e-matrix (warp-bridge-1)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-ownership-updates)
- GitHub Check: cli-evm-e2e-matrix (warp-extend-recovery)
- GitHub Check: cli-evm-e2e-matrix (warp-check-5)
- GitHub Check: cli-evm-e2e-matrix (warp-rebalancer)
- GitHub Check: cli-evm-e2e-matrix (warp-send)
- GitHub Check: cli-evm-e2e-matrix (status)
- GitHub Check: cli-evm-e2e-matrix (core-deploy)
- GitHub Check: cli-evm-e2e-matrix (relay)
- GitHub Check: cli-evm-e2e-matrix (warp-check-3)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-hook-updates)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-simple-updates)
- GitHub Check: cli-evm-e2e-matrix (warp-deploy-2)
- GitHub Check: cli-evm-e2e-matrix (core-apply)
- GitHub Check: cli-evm-e2e-matrix (warp-check-2)
- GitHub Check: cli-evm-e2e-matrix (core-init)
- GitHub Check: cli-evm-e2e-matrix (core-check)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-ism-updates)
- GitHub Check: cli-evm-e2e-matrix (warp-bridge-2)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-rebalancing-config)
- GitHub Check: env-test-matrix (mainnet3, ethereum, igp)
- GitHub Check: env-test-matrix (testnet4, sepolia, core)
- GitHub Check: env-test-matrix (mainnet3, arbitrum, igp)
- GitHub Check: env-test-matrix (mainnet3, ethereum, core)
- GitHub Check: env-test-matrix (mainnet3, optimism, core)
- GitHub Check: cli-cosmos-e2e-matrix (core-read)
- GitHub Check: env-test-matrix (mainnet3, arbitrum, core)
- GitHub Check: env-test-matrix (mainnet3, optimism, igp)
- GitHub Check: cli-radix-e2e-matrix (core-deploy)
- GitHub Check: cli-cosmos-e2e-matrix (warp-deploy)
- GitHub Check: cli-cosmos-e2e-matrix (core-deploy)
- GitHub Check: cli-cosmos-e2e-matrix (core-check)
- GitHub Check: cli-radix-e2e-matrix (core-apply)
- GitHub Check: cli-cosmos-e2e-matrix (core-apply)
- GitHub Check: cli-cosmos-e2e-matrix (warp-read)
- GitHub Check: cli-radix-e2e-matrix (warp-apply-ownership-updates)
- GitHub Check: cli-radix-e2e-matrix (warp-apply-route-extension)
- GitHub Check: cli-cross-chain-e2e-matrix (warp-apply)
- GitHub Check: cli-cross-chain-e2e-matrix (warp-deploy)
- GitHub Check: cli-radix-e2e-matrix (warp-deploy)
- GitHub Check: cosmos-sdk-e2e-run
- GitHub Check: aleo-sdk-e2e-run
- GitHub Check: e2e-matrix (evm)
- GitHub Check: agent-configs (mainnet3)
- GitHub Check: lander-coverage
- GitHub Check: lint-rs
- GitHub Check: test-rs
- GitHub Check: build-and-push-to-gcr
🔇 Additional comments (11)
typescript/infra/scripts/check/deploy-check-warp-deploy.ts (1)
7-21: Lookin' good, this refactor is like getting out of the swamp and into fresh air.The switch to the
CheckWarpDeployHelmManager.forEnvironment()factory pattern simplifies this script nicely. The logic flow is clean: validate context, assert kube context, grab the manager, and run the Helm command. Nice and tidy.typescript/infra/src/warp-monitor/helm.ts (2)
269-322: This warp route ID extraction is proper onion-layering, handling both the new and legacy ways.The fix to check both
container.commandandcontainer.args(lines 299-309) addresses the pod configuration differences mentioned in the PR objectives. The fallback warning at lines 315-317 is helpful for debugging.One small observation: the namespace and prefix are interpolated directly into the kubectl command at line 274. In this codebase these come from trusted environment configs, so it's fine - just make sure this function isn't exposed to untrusted input paths.
264-267: Simple and clean, like a well-organized swamp.The interface captures exactly what's needed - the helm release name and warp route ID pairing.
typescript/infra/scripts/warp-routes/deploy-warp-monitor.ts (1)
13-13: Import path update looks right after the directory shuffle.The move from
warp/towarp-monitor/makes the module's purpose clearer. The rest of the deployment logic remains intact.typescript/infra/scripts/warp-routes/status.ts (1)
128-131: Error handling at the end is proper - logs and exits with failure code.typescript/infra/src/utils/rpcUrls.ts (4)
290-300: This two-stage collection approach is nice - gather choices first, then do all the work at once like getting it done in one swamp trip.The separation of core infrastructure selection from warp monitor selection matches the PR objectives for improved UX. Batching all refreshes together should improve parallelization.
309-312: Skipping the Neutron context makes sense per the PR objectives.Neutron agents are handled separately, so filtering them out here avoids clutter in the selection prompt.
359-415: This warp monitor selection flow is well-structured - discover, display, and let the user choose.The three-tier choice (all, select, skip) reduces prompt fatigue while still giving granular control when needed. Listing the monitors with their warp route IDs before prompting is helpful context.
353-357: Enum keeps the choice values tidy - no magic strings floating around.typescript/infra/src/check-warp-deploy/helm.ts (2)
12-62: This CheckWarpDeployHelmManager follows the same pattern as other Helm managers in the codebase - nice and consistent.The factory method
forEnvironmentgracefully handles environments without the config by returningundefined. ThehelmValues()method constructs a clean values object from config. Based on learnings, the image repo/tag coming from config is the intended pattern for reproducibility.
1-10: Imports are clean and well-organized.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
typescript/infra/src/warp-monitor/helm.ts (2)
191-197: Log the error when warp core config retrieval fails.The catch block silently continues without logging why a warp route was skipped. This makes it harder to debug configuration issues or registry problems.
🔎 Suggested enhancement
for (const { warpRouteId } of deployedMonitors) { let warpCoreConfig; try { warpCoreConfig = getWarpCoreConfig(warpRouteId); } catch (e) { + rootLogger.warn( + `Failed to retrieve warp core config for ${warpRouteId}, skipping. Error: ${e}`, + ); continue; }
273-276: Consider renaming parameter for semantic clarity.The function parameter is named
namespace: string, but at line 183-186 it's called withenvironmentwhich is aDeployEnvironmenttype. While technically compatible (sinceDeployEnvironmentis a string type), the naming suggests different semantic meanings. Given that line 122 showsnamespaceis equivalent to the environment name in this codebase, consider either:
- Renaming the parameter to
environment: DeployEnvironmentfor consistency, or- Adding a comment explaining that namespace and environment are equivalent in this context
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
typescript/infra/src/warp-monitor/helm.ts
🧰 Additional context used
📓 Path-based instructions (1)
typescript/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
typescript/**/*.{ts,tsx}: UseChainMapfor per-chain configurations in TypeScript
Import types from@hyperlane-xyz/sdkwhen using TypeScript SDK types
Files:
typescript/infra/src/warp-monitor/helm.ts
🧠 Learnings (2)
📚 Learning: 2025-06-16T11:17:55.750Z
Learnt from: xeno097
Repo: hyperlane-xyz/hyperlane-monorepo PR: 6486
File: typescript/cli/src/tests/warp/warp-check.e2e-test.ts:490-494
Timestamp: 2025-06-16T11:17:55.750Z
Learning: In the Hyperlane registry, the label part of warp IDs can now be any value that matches the registry's regex requirements, not just actual chain names. This means functions like getCombinedWarpRoutePath can accept descriptive filenames like WARP_DEPLOY_DEFAULT_FILE_NAME as valid label components for constructing registry paths.
Applied to files:
typescript/infra/src/warp-monitor/helm.ts
📚 Learning: 2025-12-18T22:36:13.445Z
Learnt from: CR
Repo: hyperlane-xyz/hyperlane-monorepo PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-18T22:36:13.445Z
Learning: Most deployments are config-driven; check `typescript/infra/config/` for deployment configuration examples
Applied to files:
typescript/infra/src/warp-monitor/helm.ts
🧬 Code graph analysis (1)
typescript/infra/src/warp-monitor/helm.ts (4)
typescript/infra/src/config/environment.ts (1)
DeployEnvironment(31-31)typescript/infra/scripts/core-utils.ts (1)
getEnvironmentConfig(13-15)typescript/infra/src/check-warp-deploy/helm.ts (1)
namespace(40-42)typescript/infra/src/utils/utils.ts (1)
execCmdAndParseJson(98-111)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (61)
- GitHub Check: cli-evm-e2e-matrix (warp-check-4)
- GitHub Check: cli-evm-e2e-matrix (warp-rebalancer)
- GitHub Check: cli-evm-e2e-matrix (core-init)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-simple-updates)
- GitHub Check: cli-evm-e2e-matrix (warp-extend-recovery)
- GitHub Check: cli-evm-e2e-matrix (warp-read)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-rebalancing-config)
- GitHub Check: cli-evm-e2e-matrix (warp-init)
- GitHub Check: cli-evm-e2e-matrix (warp-check-3)
- GitHub Check: cli-evm-e2e-matrix (warp-deploy-2)
- GitHub Check: cli-evm-e2e-matrix (warp-check-2)
- GitHub Check: cli-evm-e2e-matrix (warp-bridge-1)
- GitHub Check: cli-evm-e2e-matrix (warp-check-1)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-ism-updates)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-submitters)
- GitHub Check: cli-evm-e2e-matrix (warp-send)
- GitHub Check: cli-evm-e2e-matrix (status)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-hook-updates)
- GitHub Check: cli-evm-e2e-matrix (core-check)
- GitHub Check: cli-evm-e2e-matrix (warp-deploy-1)
- GitHub Check: cli-evm-e2e-matrix (warp-extend-config)
- GitHub Check: cli-evm-e2e-matrix (warp-check-5)
- GitHub Check: cli-evm-e2e-matrix (warp-bridge-2)
- GitHub Check: cli-evm-e2e-matrix (warp-extend-basic)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-ownership-updates)
- GitHub Check: cli-evm-e2e-matrix (core-apply)
- GitHub Check: cli-evm-e2e-matrix (core-read)
- GitHub Check: cli-evm-e2e-matrix (relay)
- GitHub Check: cli-evm-e2e-matrix (core-deploy)
- GitHub Check: cli-cosmos-e2e-matrix (warp-read)
- GitHub Check: env-test-matrix (mainnet3, optimism, core)
- GitHub Check: cli-cosmos-e2e-matrix (warp-deploy)
- GitHub Check: env-test-matrix (mainnet3, ethereum, igp)
- GitHub Check: env-test-matrix (mainnet3, optimism, igp)
- GitHub Check: env-test-matrix (mainnet3, arbitrum, igp)
- GitHub Check: env-test-matrix (testnet4, sepolia, core)
- GitHub Check: env-test-matrix (mainnet3, ethereum, core)
- GitHub Check: env-test-matrix (mainnet3, arbitrum, core)
- GitHub Check: cli-cosmos-e2e-matrix (core-deploy)
- GitHub Check: cli-radix-e2e-matrix (warp-apply-route-extension)
- GitHub Check: cli-cosmos-e2e-matrix (core-read)
- GitHub Check: cli-radix-e2e-matrix (warp-deploy)
- GitHub Check: cli-cosmos-e2e-matrix (core-apply)
- GitHub Check: cli-cosmos-e2e-matrix (core-check)
- GitHub Check: cli-radix-e2e-matrix (core-deploy)
- GitHub Check: cosmos-sdk-e2e-run
- GitHub Check: cli-radix-e2e-matrix (core-apply)
- GitHub Check: aleo-sdk-e2e-run
- GitHub Check: pnpm-test-run
- GitHub Check: cli-radix-e2e-matrix (warp-apply-ownership-updates)
- GitHub Check: cli-cross-chain-e2e-matrix (warp-deploy)
- GitHub Check: cli-install-test-run
- GitHub Check: cli-cross-chain-e2e-matrix (warp-apply)
- GitHub Check: infra-test
- GitHub Check: build-and-push-to-gcr
- GitHub Check: e2e-matrix (evm)
- GitHub Check: agent-configs (mainnet3)
- GitHub Check: agent-configs (testnet4)
- GitHub Check: lander-coverage
- GitHub Check: test-rs
- GitHub Check: lint-rs
🔇 Additional comments (1)
typescript/infra/src/warp-monitor/helm.ts (1)
293-314: Well done handling both container.command and container.args for warpRouteId extraction.The logic properly checks the
WARP_ROUTE_IDenvironment variable first, then falls back to searching for--warpRouteIdin bothcontainer.commandandcontainer.args. This aligns with the PR objective of fixing extraction from either location.
paulbalaji
force-pushed
the
pb/improved-rpc-rotation
branch
from
d886e5b to
e9d176d
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7709 +/- ##
=======================================
Coverage 77.02% 77.02%
=======================================
Files 117 117
Lines 2651 2651
Branches 244 244
=======================================
Hits 2042 2042
Misses 593 593
Partials 16 16
🚀 New features to boost your workflow:
|
|
Nice UX upgrade here — splitting selection into core infra + warp monitors and then doing a single refresh pass is much safer / less repetitive. A few suggestions:
|
|
Following up to focus on two small nits:
|
paulbalaji
force-pushed
the
pb/improved-rpc-rotation
branch
from
e9d176d to
27028c0
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
typescript/infra/src/warp-monitor/helm.ts (2)
16-18: Architecture layering concern - src importing from scripts.Now, I don't want to be all ogre about this, but importing
getEnvironmentConfigfrom../../scripts/core-utils.jsinto asrc/module creates a backwards dependency. Thesrc/layer should ideally be independent ofscripts/.Consider either:
- Passing
supportedChainNamesintogetManagersForChainas a parameter- Moving the discovery helper function into
scripts/where it can access both layersThis was also flagged in the PR comments.
♻️ Option 1: Pass supportedChainNames as parameter
static async getManagersForChain( environment: DeployEnvironment, chain: string, + supportedChainNames: string[], ): Promise<WarpRouteMonitorHelmManager[]> { const deployedMonitors = await getDeployedWarpMonitorWarpRouteIds( environment, WarpRouteMonitorHelmManager.helmReleasePrefix, ); - const envConfig = getEnvironmentConfig(environment); const helmManagers: WarpRouteMonitorHelmManager[] = []; for (const { warpRouteId } of deployedMonitors) { // ... existing logic ... helmManagers.push( new WarpRouteMonitorHelmManager( warpRouteId, environment, - envConfig.supportedChainNames, + supportedChainNames, '', ), ); } return helmManagers; }
188-194: Consider logging when warp config lookup fails.Right now when
getWarpCoreConfigthrows, we just silently move on. That's fine for the happy path, but it might make debugging a wee bit harder if something's amiss. A debug-level log would help troubleshoot without cluttering normal output.♻️ Add debug logging for failed lookups
try { warpCoreConfig = getWarpCoreConfig(warpRouteId); } catch (e) { + rootLogger.debug( + `Could not get warp core config for ${warpRouteId}, skipping: ${e}`, + ); continue; }typescript/infra/src/check-warp-deploy/helm.ts (1)
1-11: Same architecture layering concern applies here.Like in
warp-monitor/helm.ts, thissrc/module imports fromscripts/core-utils.js. If you address the layering in the other file, apply the same pattern here for consistency.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📥 Commits
Reviewing files that changed from the base of the PR and between e9d176df8d9720d1650daad1a8dcd58849173d42 and 27028c0.
📒 Files selected for processing (6)
typescript/infra/scripts/check/deploy-check-warp-deploy.tstypescript/infra/scripts/warp-routes/deploy-warp-monitor.tstypescript/infra/scripts/warp-routes/status.tstypescript/infra/src/check-warp-deploy/helm.tstypescript/infra/src/utils/rpcUrls.tstypescript/infra/src/warp-monitor/helm.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- typescript/infra/scripts/warp-routes/status.ts
- typescript/infra/scripts/warp-routes/deploy-warp-monitor.ts
🧰 Additional context used
📓 Path-based instructions (1)
typescript/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
typescript/**/*.{ts,tsx}: UseChainMapfor per-chain configurations in TypeScript
Import types from@hyperlane-xyz/sdkwhen using TypeScript SDK types
Files:
typescript/infra/src/warp-monitor/helm.tstypescript/infra/scripts/check/deploy-check-warp-deploy.tstypescript/infra/src/check-warp-deploy/helm.tstypescript/infra/src/utils/rpcUrls.ts
🧠 Learnings (11)
📓 Common learnings
Learnt from: paulbalaji
Repo: hyperlane-xyz/hyperlane-monorepo PR: 6943
File: rust/main/config/testnet_config.json:34-35
Timestamp: 2025-08-26T13:45:52.227Z
Learning: Skip reviewing mainnet_config.json and testnet_config.json configuration files in typescript/infra/config/ and rust/main/config/ directories as requested by paulbalaji to reduce review noise.
📚 Learning: 2025-06-16T11:17:55.750Z
Learnt from: xeno097
Repo: hyperlane-xyz/hyperlane-monorepo PR: 6486
File: typescript/cli/src/tests/warp/warp-check.e2e-test.ts:490-494
Timestamp: 2025-06-16T11:17:55.750Z
Learning: In the Hyperlane registry, the label part of warp IDs can now be any value that matches the registry's regex requirements, not just actual chain names. This means functions like getCombinedWarpRoutePath can accept descriptive filenames like WARP_DEPLOY_DEFAULT_FILE_NAME as valid label components for constructing registry paths.
Applied to files:
typescript/infra/src/warp-monitor/helm.ts
📚 Learning: 2025-12-29T19:45:12.592Z
Learnt from: paulbalaji
Repo: hyperlane-xyz/hyperlane-monorepo PR: 7545
File: typescript/infra/src/rebalancer/helm.ts:82-83
Timestamp: 2025-12-29T19:45:12.592Z
Learning: In the hyperlane-xyz/hyperlane-monorepo repository, Docker image tags in Helm managers (e.g., typescript/infra/src/rebalancer/helm.ts) are intentionally hardcoded for reproducibility and explicit version control, rather than using dynamically generated CI tags.
Applied to files:
typescript/infra/src/warp-monitor/helm.tstypescript/infra/src/check-warp-deploy/helm.ts
📚 Learning: 2025-11-25T17:10:33.369Z
Learnt from: paulbalaji
Repo: hyperlane-xyz/hyperlane-monorepo PR: 7410
File: solidity/foundry.toml:8-8
Timestamp: 2025-11-25T17:10:33.369Z
Learning: In the hyperlane-xyz/hyperlane-monorepo repository, when using pnpm (instead of Yarn), Foundry's `allow_paths` in solidity/foundry.toml should be set to `["./node_modules"]` rather than `["../node_modules"]` because pnpm's default node_modules structure places dependencies locally in the workspace subdirectory, not requiring access to the parent directory's node_modules.
Applied to files:
typescript/infra/src/warp-monitor/helm.ts
📚 Learning: 2025-08-26T13:46:37.695Z
Learnt from: paulbalaji
Repo: hyperlane-xyz/hyperlane-monorepo PR: 6943
File: rust/main/config/mainnet_config.json:965-965
Timestamp: 2025-08-26T13:46:37.695Z
Learning: In the repository hyperlane-xyz/hyperlane-monorepo, skip reviewing the file rust/main/config/mainnet_config.json in future code reviews as requested by paulbalaji.
Applied to files:
typescript/infra/src/warp-monitor/helm.ts
📚 Learning: 2025-08-26T13:46:37.695Z
Learnt from: paulbalaji
Repo: hyperlane-xyz/hyperlane-monorepo PR: 6943
File: rust/main/config/mainnet_config.json:965-965
Timestamp: 2025-08-26T13:46:37.695Z
Learning: In the repository hyperlane-xyz/hyperlane-monorepo, skip reviewing the file rust/main/config/testnet_config.json in future code reviews as requested by paulbalaji.
Applied to files:
typescript/infra/src/warp-monitor/helm.ts
📚 Learning: 2026-01-07T17:37:43.421Z
Learnt from: paulbalaji
Repo: hyperlane-xyz/hyperlane-monorepo PR: 7720
File: typescript/keyfunder/package.json:22-22
Timestamp: 2026-01-07T17:37:43.421Z
Learning: In the hyperlane-xyz/hyperlane-monorepo repository, hyperlane-xyz/registry should use "catalog:" as the dependency version following the monorepo pattern, not a pinned version.
Applied to files:
typescript/infra/src/warp-monitor/helm.ts
📚 Learning: 2025-08-13T16:53:55.163Z
Learnt from: paulbalaji
Repo: hyperlane-xyz/hyperlane-monorepo PR: 6891
File: typescript/infra/config/environments/mainnet3/funding.ts:22-22
Timestamp: 2025-08-13T16:53:55.163Z
Learning: In Hyperlane mainnet3 configs, funding.ts uses 'gcr.io/abacus-labs-dev/hyperlane-monorepo' docker image while agent.ts uses 'gcr.io/abacus-labs-dev/hyperlane-agent' docker image. These are different images with independent tag cycles, so tag consistency across them is not expected.
Applied to files:
typescript/infra/src/warp-monitor/helm.ts
📚 Learning: 2025-07-14T23:18:11.350Z
Learnt from: ltyu
Repo: hyperlane-xyz/hyperlane-monorepo PR: 6715
File: typescript/cli/src/deploy/warp.ts:941-948
Timestamp: 2025-07-14T23:18:11.350Z
Learning: In the Hyperlane CLI warp.ts file, casting ExtendedSubmissionStrategy to SubmissionStrategy is acceptable because getStrategy has runtime checks that throw if the strategy factory doesn't exist, providing safety even with the type cast.
Applied to files:
typescript/infra/src/warp-monitor/helm.ts
📚 Learning: 2025-12-18T22:36:13.445Z
Learnt from: CR
Repo: hyperlane-xyz/hyperlane-monorepo PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-18T22:36:13.445Z
Learning: Most deployments are config-driven; check `typescript/infra/config/` for deployment configuration examples
Applied to files:
typescript/infra/src/warp-monitor/helm.tstypescript/infra/scripts/check/deploy-check-warp-deploy.tstypescript/infra/src/check-warp-deploy/helm.ts
📚 Learning: 2025-08-26T13:45:52.227Z
Learnt from: paulbalaji
Repo: hyperlane-xyz/hyperlane-monorepo PR: 6943
File: rust/main/config/testnet_config.json:34-35
Timestamp: 2025-08-26T13:45:52.227Z
Learning: Skip reviewing mainnet_config.json and testnet_config.json configuration files in typescript/infra/config/ and rust/main/config/ directories as requested by paulbalaji to reduce review noise.
Applied to files:
typescript/infra/src/utils/rpcUrls.ts
🧬 Code graph analysis (2)
typescript/infra/src/warp-monitor/helm.ts (3)
typescript/infra/src/config/environment.ts (1)
DeployEnvironment(31-31)typescript/infra/scripts/core-utils.ts (1)
getEnvironmentConfig(13-15)typescript/infra/src/utils/utils.ts (1)
execCmdAndParseJson(98-111)
typescript/infra/scripts/check/deploy-check-warp-deploy.ts (3)
typescript/infra/scripts/core-utils.ts (1)
getConfigsBasedOnArgs(17-27)typescript/infra/scripts/agent-utils.ts (1)
assertCorrectKubeContext(741-754)typescript/infra/src/check-warp-deploy/helm.ts (1)
CheckWarpDeployHelmManager(12-63)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (60)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-ism-updates)
- GitHub Check: cli-evm-e2e-matrix (warp-check-2)
- GitHub Check: cli-evm-e2e-matrix (warp-rebalancer)
- GitHub Check: cli-evm-e2e-matrix (warp-bridge-1)
- GitHub Check: cli-evm-e2e-matrix (core-deploy)
- GitHub Check: cli-evm-e2e-matrix (warp-init)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-submitters)
- GitHub Check: cli-evm-e2e-matrix (warp-send)
- GitHub Check: cli-evm-e2e-matrix (warp-deploy-2)
- GitHub Check: cli-evm-e2e-matrix (core-read)
- GitHub Check: cli-evm-e2e-matrix (warp-check-5)
- GitHub Check: cli-evm-e2e-matrix (warp-check-1)
- GitHub Check: cli-evm-e2e-matrix (core-init)
- GitHub Check: cli-evm-e2e-matrix (warp-extend-config)
- GitHub Check: cli-evm-e2e-matrix (status)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-hook-updates)
- GitHub Check: cli-evm-e2e-matrix (warp-extend-recovery)
- GitHub Check: cli-evm-e2e-matrix (warp-read)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-simple-updates)
- GitHub Check: cli-evm-e2e-matrix (core-apply)
- GitHub Check: cli-evm-e2e-matrix (warp-check-3)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-ownership-updates)
- GitHub Check: cli-evm-e2e-matrix (relay)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-rebalancing-config)
- GitHub Check: cli-evm-e2e-matrix (warp-extend-basic)
- GitHub Check: cli-evm-e2e-matrix (warp-bridge-2)
- GitHub Check: cli-evm-e2e-matrix (warp-check-4)
- GitHub Check: cli-evm-e2e-matrix (warp-deploy-1)
- GitHub Check: cli-evm-e2e-matrix (core-check)
- GitHub Check: env-test-matrix (mainnet3, optimism, igp)
- GitHub Check: env-test-matrix (testnet4, sepolia, core)
- GitHub Check: env-test-matrix (mainnet3, arbitrum, igp)
- GitHub Check: cli-radix-e2e-matrix (core-apply)
- GitHub Check: cli-radix-e2e-matrix (warp-apply-route-extension)
- GitHub Check: env-test-matrix (mainnet3, optimism, core)
- GitHub Check: env-test-matrix (mainnet3, ethereum, igp)
- GitHub Check: cli-cosmos-e2e-matrix (core-read)
- GitHub Check: cli-cosmos-e2e-matrix (core-check)
- GitHub Check: cli-radix-e2e-matrix (warp-apply-ownership-updates)
- GitHub Check: cli-radix-e2e-matrix (warp-deploy)
- GitHub Check: env-test-matrix (mainnet3, ethereum, core)
- GitHub Check: cli-cosmos-e2e-matrix (warp-deploy)
- GitHub Check: cosmos-sdk-e2e-run
- GitHub Check: env-test-matrix (mainnet3, arbitrum, core)
- GitHub Check: cli-cosmos-e2e-matrix (core-apply)
- GitHub Check: cli-install-test-run
- GitHub Check: cli-cosmos-e2e-matrix (core-deploy)
- GitHub Check: cli-cosmos-e2e-matrix (warp-read)
- GitHub Check: cli-radix-e2e-matrix (core-deploy)
- GitHub Check: cli-cross-chain-e2e-matrix (warp-apply)
- GitHub Check: cli-cross-chain-e2e-matrix (warp-deploy)
- GitHub Check: aleo-sdk-e2e-run
- GitHub Check: pnpm-test-run
- GitHub Check: infra-test
- GitHub Check: build-and-push-to-gcr
- GitHub Check: lint-rs
- GitHub Check: test-rs
- GitHub Check: lander-coverage
- GitHub Check: e2e-matrix (evm)
- GitHub Check: agent-configs (mainnet3)
🔇 Additional comments (7)
typescript/infra/src/utils/rpcUrls.ts (3)
302-351: Solid implementation of the core infrastructure selection.The logic here is clean - iterates through agent configs, skips Neutron (as intended), and builds up the list of managers. The checkbox with all items pre-checked is a sensible default for the refresh flow.
359-415: Nice three-way selection flow for warp monitors.The discovery and selection UX is well thought out - listing monitors before prompting gives users context for their decision. The ALL/SELECT/SKIP options cover the common use cases nicely.
290-300: Good restructure with batch refresh approach.The separation of concerns between agent infrastructure and warp monitors is clean. One note: KeyFunder and Kathy were removed from the core infrastructure selection, but they still consume secret RPC endpoints (via
getSecretRpcEndpoints). The exclusion appears intentional since they're not agent-based infrastructure, but this architectural shift should be documented—either explaining why separate refresh logic is now needed for them, or confirming they no longer require K8s pod restarts when RPC secrets are updated.typescript/infra/src/warp-monitor/helm.ts (2)
57-64: Registry URI fallback looks good.The fallback to
DEFAULT_GITHUB_REGISTRYwithout the/tree/suffix when no commit is specified makes sense. This handles the case where monitors need to use the latest registry state.Note: The PR comment mentioned
DEFAULT_REGISTRY_URIbeing unused, butDEFAULT_GITHUB_REGISTRY(the actual import) is clearly used here at line 60.
270-323: Good defensive extraction logic for warp route IDs.This handles the different ways the warp route ID can be specified - environment variable for the standalone image and command-line args for the legacy monorepo image. Checking both
commandandargsarrays is the right call since pod specs can vary.The warning log when extraction fails (line 316-318) will be helpful for debugging deployment issues.
typescript/infra/src/check-warp-deploy/helm.ts (1)
12-63: Clean Helm manager implementation.This follows the established patterns nicely - factory method that handles missing config gracefully, proper namespace getter, and well-structured
helmValues(). The hardcoded release name and chart path are appropriate for this single-instance deployment.typescript/infra/scripts/check/deploy-check-warp-deploy.ts (1)
7-22: Nice refactor to the manager-based approach.Much cleaner than the previous inline Helm logic. The flow is straightforward now - get config, create manager, run command. The error handling for missing
checkWarpDeployConfigis preserved through the undefined check on the manager.This is how things should be done in my swamp... err, I mean codebase.
paulbalaji
force-pushed
the
pb/improved-rpc-rotation
branch
2 times, most recently
from
d5a1036 to
11554eb
Compare
…arp-deploy - Add CheckWarpDeployHelmManager for check-warp-deploy cron job - Add getDeployedWarpMonitorWarpRouteIds() to extract warp route IDs from k8s pods - Support both standalone image (WARP_ROUTE_ID env) and legacy monorepo (--warpRouteId arg) - Update RPC rotation to refresh warp monitors (filtered by chain) and check-warp-deploy - Refactor deploy-check-warp-deploy.ts to use CheckWarpDeployHelmManager - Rename src/warp/ to src/warp-monitor/ for clarity
Add status.ts script to show warp monitor status for single or all deployed monitors. Displays pod status, recent logs (parsed JSON), and Grafana dashboard link.
- Split refresh prompt into core infra (agents, key-funder, check-warp-deploy) and warp monitors - Add 'refresh all/select/skip' options for warp monitors to reduce prompt clutter - Remove Kathy and filter to Hyperlane+RC relayers, Hyperlane-only validators/scrapers - Fix warp route ID parsing to check both container.command and container.args
CronJobs (KeyFunder, Kathy) now get their secrets refreshed during RPC rotation, but without pod restart. They pick up new secrets on next run.
paulbalaji
force-pushed
the
pb/improved-rpc-rotation
branch
from
33fafc9 to
a89d584
Compare
🐳 Monorepo Docker Image Built SuccessfullyImage Tags: |
3 participants
Summary
Improved the RPC URL rotation script UX by splitting k8s resource refresh into multiple stages and adding better warp monitor discovery.
Changes
RPC rotation UX improvements
Warp monitor parsing fix
container.commandandcontainer.args(some pods use command, most use args)Status script
scripts/warp-routes/status.tsto show warp monitor status--warpRouteId(e.g.--warpRouteId USDC/ethereum-base)Refresh behavior
Sequence Diagram
sequenceDiagram participant User participant Script as RPC Rotation Script participant Core as selectCoreInfrastructure() participant Warp as selectWarpMonitors() participant Cron as selectCronJobs() participant K8s as refreshK8sResources() User->>Script: Rotate RPC URLs for chain Script->>Script: updateSecretAndDisablePrevious() Note over Script: Updates GCP secret with new RPC URLs Script->>User: Prompt: Refresh k8s resources? User->>Script: Confirm Script->>Core: Select core infrastructure Note over Core: Relayers, Validators, Scrapers<br/>(all contexts) Core->>User: Show checkboxes for core infra User->>Core: Select items Core->>Script: Return selected managers Script->>Warp: Select warp monitors Warp->>Warp: getManagersForChain(environment, chain) Warp->>User: Show: All / Select / Skip? User->>Warp: Choose option Warp->>Script: Return selected managers Script->>Cron: Select CronJobs Note over Cron: KeyFunder, Kathy<br/>(secrets only) Cron->>User: Show checkboxes User->>Cron: Select items Cron->>Script: Return selected managers Script->>K8s: Refresh SECRETs for ALL selected Note over K8s: Core + Warp + CronJobs Script->>K8s: Refresh PODs for services only Note over K8s: Core + Warp (not CronJobs) K8s->>User: Refresh complete