fix: include solana rpc indexing#8901
Conversation
troykessler
requested review from
Mo-Hussain,
paulbalaji and
tkporter
as code owners
📝 WalkthroughWalkthrough
ChangesRebalancer chain name merge
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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
🧹 Nitpick comments (1)
typescript/infra/src/rebalancer/helm.ts (1)
56-61: ⚡ Quick winConsider asserting that warp tokens exist.
You've got a check for the config itself, but if
warpCoreConfig.tokensis empty, this rebalancer's got nothing to rebalance. Per the coding guidelines, useassert()liberally for preconditions like this — better to fail fast than proceed with an invalid setup.Suggested assertion
const warpCoreConfig = getWarpCoreConfig(this.warpRouteId); if (!warpCoreConfig) { throw new Error( `Warp Route ID not found in registry: ${this.warpRouteId}`, ); } +assert( + warpCoreConfig.tokens.length > 0, + `Warp route ${this.warpRouteId} has no tokens`, +);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@typescript/infra/src/rebalancer/helm.ts` around lines 56 - 61, After the existing check that warpCoreConfig is not null in the block where getWarpCoreConfig(this.warpRouteId) is called, add an assert() statement to verify that warpCoreConfig.tokens exists and is not empty. This precondition check ensures the rebalancer has tokens to work with and follows the coding guideline to fail fast with invalid setups rather than proceeding with incomplete data.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@typescript/infra/src/rebalancer/helm.ts`:
- Around line 77-86: The static method getManagersForChain (line 186) only
checks warpChains from warpCoreConfig, but now that rebalancerChains includes
strategy-only chains (which may not be warp tokens), the method needs to account
for those additional chains. Update getManagersForChain to fetch the deployed
rebalancer config from the configmap (following the pattern in lines 271-287),
parse it with RebalancerConfigSchema, extract strategy chains using
getStrategyChainNames, and then check if the provided chain parameter exists in
either the warpChains set or the strategy chains set before returning matching
managers.
---
Nitpick comments:
In `@typescript/infra/src/rebalancer/helm.ts`:
- Around line 56-61: After the existing check that warpCoreConfig is not null in
the block where getWarpCoreConfig(this.warpRouteId) is called, add an assert()
statement to verify that warpCoreConfig.tokens exists and is not empty. This
precondition check ensures the rebalancer has tokens to work with and follows
the coding guideline to fail fast with invalid setups rather than proceeding
with incomplete data.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2ed9c70c-2660-41f8-afaa-cc1b152b2ddd
📒 Files selected for processing (1)
typescript/infra/src/rebalancer/helm.ts
| // Store chains for helm values (used for private RPC secrets). | ||
| // Merge warp token chains and strategy chains — some strategy chains | ||
| // (e.g. solanamainnet used as a LiFi bridge target) are not warp tokens | ||
| // and would otherwise fall back to the public registry RPC. | ||
| this.rebalancerChains = [ | ||
| ...new Set(warpCoreConfig.tokens.map((t) => t.chainName)), | ||
| ...new Set([ | ||
| ...warpCoreConfig.tokens.map((t) => t.chainName), | ||
| ...chainNames, | ||
| ]), | ||
| ]; |
There was a problem hiding this comment.
Cross-file impact: getManagersForChain won't find strategy-only chains.
The merge here is sound — you're rightly including strategy chains in rebalancerChains for RPC secrets. But look at the static method getManagersForChain (line 186): it only checks warpChains (warp token chains), not the full rebalancerChains that now includes strategy chains.
When a strategy-only chain's RPC rotates, getManagersForChain won't return this rebalancer, so the pods won't refresh. To fix it, getManagersForChain needs to read the deployed rebalancer config (similar to lines 271-287), extract strategy chains with getStrategyChainNames, and check both warp and strategy chains.
Suggested approach
In getManagersForChain, after retrieving warpCoreConfig:
- Fetch the deployed rebalancer config from the configmap
- Parse it with
RebalancerConfigSchema - Extract strategy chains via
getStrategyChainNames - Check if
chainis in eitherwarpChainsor strategy chains
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@typescript/infra/src/rebalancer/helm.ts` around lines 77 - 86, The static
method getManagersForChain (line 186) only checks warpChains from
warpCoreConfig, but now that rebalancerChains includes strategy-only chains
(which may not be warp tokens), the method needs to account for those additional
chains. Update getManagersForChain to fetch the deployed rebalancer config from
the configmap (following the pattern in lines 271-287), parse it with
RebalancerConfigSchema, extract strategy chains using getStrategyChainNames, and
then check if the provided chain parameter exists in either the warpChains set
or the strategy chains set before returning matching managers.
1 participant
Description
rebalancerChains(used to populateRPC_URL_*env vars via ExternalSecret) was built solely fromwarpCoreConfig.tokens— the warp token chains. Strategy-only chains likesolanamainnet(used as a LiFi bridge target in USDC/eclipsemainnet) were never included, soRPC_URL_SOLANAMAINNETwas never injected and the rebalancer fell back to the public registry RPC (https://api.mainnet-beta.solana.com).Under load this causes 429s that wedge the rebalancer's Solana balance monitor, preventing it from detecting and rebalancing a collateral deficit — which in turn blocks arbitrum→solanamainnet transfers from being relayed (gas estimation fails with SPL InsufficientFunds).
Fix: merge strategy chain names into
rebalancerChainsso all chains the rebalancer interacts with (not just warp token chains) get private RPC URLs injected.Drive-by changes
None.
Related issues
Backward compatibility
Yes. Additive only — existing rebalancers already have
mainnet3-rpc-endpoints-<chain>GCP secrets for their warp token chains; this only adds secrets lookups for chains that were previously missing.Testing
Manual — redeployed
hyperlane-rebalancer-usdc-eclipsemainnetand confirmedRPC_URL_SOLANAMAINNETis now set to the private Alchemy endpoint in the pod.