Conversation
🦋 Changeset detectedLatest commit: 59a5414 The changes in this PR will be included in the next version bump. This PR includes changesets to release 20 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7544 +/- ##
============================
============================
🚀 New features to boost your workflow:
|
paulbalaji
force-pushed
the
pbio/rebalancer-package-move
branch
2 times, most recently
from
a174445 to
a4ca220
Compare
paulbalaji
requested review from
antigremlin,
ltyu,
xeno097 and
yorhodes
as code owners
|
Warning Rate limit exceeded@paulbalaji has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 24 minutes and 35 seconds before requesting another review. ⌛ 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 749c8c214dca9d68d9a354db010f52d75f2db3a5 and 59a5414. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (22)
📝 WalkthroughWalkthroughThis PR separates the Warp Route rebalancer from the CLI into a dedicated Changes
Sequence DiagramsequenceDiagram
participant Service as RebalancerService
participant Factory as RebalancerContextFactory
participant Monitor as Monitor
participant Strategy as Strategy
participant Rebalancer as Rebalancer
alt Manual Mode
Service->>Factory: initialize(config, providers, registry)
Factory-->>Service: Monitor, Strategy, Rebalancer
Service->>Service: executeManual(request)
Service->>Strategy: getRebalancingRoutes()
Strategy-->>Service: routes
Service->>Rebalancer: rebalance(routes)
Rebalancer-->>Service: success
Service->>Service: log & metrics
else Daemon Mode
Service->>Factory: initialize(config, providers, registry)
Factory-->>Service: Monitor, Strategy, Rebalancer
Service->>Monitor: start()
loop On Token Info Events
Monitor->>Service: onTokenInfo(event)
Service->>Strategy: getRebalancingRoutes()
Strategy-->>Service: routes
Service->>Rebalancer: rebalance(routes)
Rebalancer-->>Service: result
Service->>Service: update metrics
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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: 2
🧹 Nitpick comments (7)
typescript/cli/package.json (1)
18-18: Consider using workspace protocol for the rebalancer dependency.The other internal Hyperlane packages in this file use versions like
"19.11.0"or"workspace:^". Since@hyperlane-xyz/rebalanceris a new internal package in the monorepo, you might want to use"workspace:*"instead of"0.0.0"for consistency with how monorepo dependencies are typically managed. This'll make it clearer that it's a workspace package.- "@hyperlane-xyz/rebalancer": "0.0.0", + "@hyperlane-xyz/rebalancer": "workspace:*",typescript/cli/src/commands/warp.ts (1)
532-538: This validation is like putting a lock on a door that's already locked, if you catch my drift.The yargs config at lines 471-491 already uses
impliesto enforce thatorigin,destination, andamountare required whenmanualis set. This runtime check is redundant. That said, it provides a clearer error message, so it's not harmful — just a wee bit of belt-and-suspenders.typescript/rebalancer/src/service.ts (2)
61-63: Consider validating the parsed CHECK_FREQUENCY value, friend.If someone passes a non-numeric value like
CHECK_FREQUENCY=banana,parseIntwill returnNaN, and that'll cause all sorts of mischief down in the swamp. A simple check would save ye some headaches later.const checkFrequency = process.env.CHECK_FREQUENCY ? parseInt(process.env.CHECK_FREQUENCY, 10) : 60_000; + + if (isNaN(checkFrequency) || checkFrequency <= 0) { + rootLogger.error('CHECK_FREQUENCY must be a positive number'); + process.exit(1); + }
105-108: Private key validation before Wallet creation would be wise.Right now, if
HYP_KEYcontains malformed data (not a valid private key), theWalletconstructor will throw an opaque error. Might be worth catching that specifically and giving a more helpful message about what went wrong in the key formatting.// Create MultiProvider with signer const multiProvider = new MultiProvider(chainMetadata); - const signer = new Wallet(privateKey); + let signer: Wallet; + try { + signer = new Wallet(privateKey); + } catch (error) { + logger.error({ error }, 'Invalid HYP_KEY: must be a valid private key'); + process.exit(1); + } multiProvider.setSharedSigner(signer); logger.info('✅ Initialized MultiProvider with signer');typescript/rebalancer/src/core/RebalancerService.ts (2)
269-271: Careful with removeAllListeners - it's a bit heavy-handed.Using
process.removeAllListeners('SIGINT')andprocess.removeAllListeners('SIGTERM')will remove ALL listeners for these signals, not just the ones this service registered. If something else in the process also listens for these signals, they'll get removed too.Consider storing the handler references and removing only those specific listeners:
+ private sigintHandler?: () => void; + private sigtermHandler?: () => void; + async start(): Promise<void> { // ... // Set up signal handlers for graceful shutdown - process.on('SIGINT', () => this.gracefulShutdown()); - process.on('SIGTERM', () => this.gracefulShutdown()); + this.sigintHandler = () => this.gracefulShutdown(); + this.sigtermHandler = () => this.gracefulShutdown(); + process.on('SIGINT', this.sigintHandler); + process.on('SIGTERM', this.sigtermHandler); } async gracefulShutdown(): Promise<void> { // ... - process.removeAllListeners('SIGINT'); - process.removeAllListeners('SIGTERM'); + if (this.sigintHandler) process.removeListener('SIGINT', this.sigintHandler); + if (this.sigtermHandler) process.removeListener('SIGTERM', this.sigtermHandler); }
273-274: Calling process.exit(0) in gracefulShutdown limits reusability.Calling
process.exit(0)directly here means this service can only be used as the main process controller. If someone wanted to embed this in a larger application or test it, they couldn't cleanly shut it down without exiting the whole process. Might want to make this configurable or let the caller handle the exit.typescript/rebalancer/src/utils/files.ts (1)
1-16: Consider whether this duplicates the CLI's files.ts utility.Based on the imports, there's already a
typescript/cli/src/utils/files.tswith similar functionality. Duplicating utilities across packages can lead to maintenance headaches. Would it make sense to either:
- Extract shared utilities to
@hyperlane-xyz/utils, or- Import from the CLI package (if that doesn't create circular deps)?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📥 Commits
Reviewing files that changed from the base of the PR and between d5af0db and a4ca220227ddf43dbd82207d860f8249b34b8579.
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (26)
.changeset/separate-rebalancer-package.md(1 hunks)Dockerfile(1 hunks)typescript/cli/package.json(1 hunks)typescript/cli/src/commands/warp.ts(3 hunks)typescript/cli/src/context/strategies/chain/chainResolver.ts(1 hunks)typescript/cli/src/rebalancer/README.md(0 hunks)typescript/cli/src/rebalancer/config/RebalancerConfig.test.ts(0 hunks)typescript/cli/src/rebalancer/index.ts(0 hunks)typescript/cli/src/rebalancer/runner.ts(0 hunks)typescript/http-registry-server/src/routes/chain.ts(2 hunks)typescript/http-registry-server/src/routes/root.ts(1 hunks)typescript/http-registry-server/src/routes/warp.ts(2 hunks)typescript/rebalancer/.gitignore(1 hunks)typescript/rebalancer/.mocharc.json(1 hunks)typescript/rebalancer/README.md(1 hunks)typescript/rebalancer/eslint.config.mjs(1 hunks)typescript/rebalancer/package.json(1 hunks)typescript/rebalancer/src/config/RebalancerConfig.ts(1 hunks)typescript/rebalancer/src/core/RebalancerService.ts(1 hunks)typescript/rebalancer/src/factories/RebalancerContextFactory.ts(6 hunks)typescript/rebalancer/src/index.ts(1 hunks)typescript/rebalancer/src/interfaces/IMonitor.ts(1 hunks)typescript/rebalancer/src/service.ts(1 hunks)typescript/rebalancer/src/utils/errors.ts(1 hunks)typescript/rebalancer/src/utils/files.ts(1 hunks)typescript/rebalancer/tsconfig.json(1 hunks)
💤 Files with no reviewable changes (4)
- typescript/cli/src/rebalancer/index.ts
- typescript/cli/src/rebalancer/config/RebalancerConfig.test.ts
- typescript/cli/src/rebalancer/runner.ts
- typescript/cli/src/rebalancer/README.md
🧰 Additional context used
🧠 Learnings (11)
📚 Learning: 2025-11-24T17:19:38.362Z
Learnt from: CR
Repo: hyperlane-xyz/hyperlane-monorepo PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:19:38.362Z
Learning: Applies to typescript/sdk/src/**/*.ts : Use `ChainMap<T>` for type-safe per-chain configuration mapping in TypeScript SDK
Applied to files:
typescript/cli/src/context/strategies/chain/chainResolver.tstypescript/rebalancer/tsconfig.jsontypescript/http-registry-server/src/routes/chain.tstypescript/rebalancer/.mocharc.jsontypescript/rebalancer/src/factories/RebalancerContextFactory.ts
📚 Learning: 2025-10-21T14:26:22.163Z
Learnt from: paulbalaji
Repo: hyperlane-xyz/hyperlane-monorepo PR: 7223
File: typescript/infra/src/config/chain.ts:56-58
Timestamp: 2025-10-21T14:26:22.163Z
Learning: When adding chains to chainsToSkip in typescript/infra/src/config/chain.ts, it's the expected pattern to keep the chain's configurations in other files like supportedChainNames.ts, agent.ts, validators.ts, and owners.ts. The chainsToSkip array is used selectively by specific scripts (e.g., check-owner-ica.ts) to exclude chains from certain operations, not as a signal to remove all configurations.
Applied to files:
typescript/cli/src/context/strategies/chain/chainResolver.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/cli/src/context/strategies/chain/chainResolver.tstypescript/http-registry-server/src/routes/warp.tstypescript/cli/src/commands/warp.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/rebalancer/tsconfig.jsontypescript/rebalancer/src/service.tstypescript/rebalancer/eslint.config.mjstypescript/rebalancer/src/config/RebalancerConfig.tstypescript/rebalancer/.mocharc.jsontypescript/rebalancer/.gitignore
📚 Learning: 2025-11-24T17:19:38.362Z
Learnt from: CR
Repo: hyperlane-xyz/hyperlane-monorepo PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:19:38.362Z
Learning: Applies to typescript/sdk/src/**/*.ts : TypeScript SDK must provide `MultiProvider` for multi-chain provider management, `HyperlaneCore` factory for core contract interactions, and `MultiProtocolCore` for protocol-agnostic abstractions across EVM, Cosmos, Sealevel, and Starknet
Applied to files:
typescript/rebalancer/tsconfig.jsontypescript/rebalancer/src/service.tstypescript/cli/package.jsonDockerfiletypescript/rebalancer/src/index.tstypescript/rebalancer/src/factories/RebalancerContextFactory.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/http-registry-server/src/routes/warp.tstypescript/cli/src/commands/warp.ts
📚 Learning: 2025-11-26T13:28:51.658Z
Learnt from: paulbalaji
Repo: hyperlane-xyz/hyperlane-monorepo PR: 7410
File: typescript/cli/package.json:20-20
Timestamp: 2025-11-26T13:28:51.658Z
Learning: In the hyperlane-xyz/hyperlane-monorepo repository, hyperlane-xyz/registry is maintained in a separate repository and published to npm, so it should use a pinned version (e.g., "23.6.0") rather than the workspace protocol ("workspace:*") that other internal Hyperlane packages use.
Applied to files:
typescript/cli/package.jsontypescript/rebalancer/package.jsontypescript/rebalancer/src/factories/RebalancerContextFactory.ts
📚 Learning: 2025-07-01T18:44:43.514Z
Learnt from: larryob
Repo: hyperlane-xyz/hyperlane-monorepo PR: 6661
File: solidity/contracts/avs/HyperlaneServiceManager.sol:21-23
Timestamp: 2025-07-01T18:44:43.514Z
Learning: Test files and script files in Solidity projects typically don't need absolute import fixes with the same urgency as contract files, since they are not imported by other modules in the codebase. The compilation issues from absolute imports mainly affect files under the contracts directory that users import from.
Applied to files:
typescript/rebalancer/src/config/RebalancerConfig.ts
📚 Learning: 2025-11-24T17:19:38.362Z
Learnt from: CR
Repo: hyperlane-xyz/hyperlane-monorepo PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:19:38.362Z
Learning: For agent deployment, redeployment, RPC URL rotation, validator operations, manual message processing, balance management, key funding, security incident response, and Lander configuration, follow procedures detailed in the Operations Runbook at https://www.notion.so/hyperlanexyz/Runbook-AI-Agent-24a6d35200d680229b38e8501164ca66
Applied to files:
typescript/rebalancer/README.md
📚 Learning: 2025-06-16T16:22:12.017Z
Learnt from: xeno097
Repo: hyperlane-xyz/hyperlane-monorepo PR: 5883
File: typescript/cli/src/commands/fork.ts:12-39
Timestamp: 2025-06-16T16:22:12.017Z
Learning: In yargs CLI applications, hyphenated option names (kebab-case) like 'fork-config' are automatically converted to camelCase (forkConfig) when passed to handler functions. This means destructuring { forkConfig } from handler arguments when the option is defined as 'fork-config' is correct and will work as expected.
Applied to files:
typescript/cli/src/commands/warp.ts
📚 Learning: 2025-11-24T17:19:38.362Z
Learnt from: CR
Repo: hyperlane-xyz/hyperlane-monorepo PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:19:38.362Z
Learning: Applies to typescript/sdk/src/**/*.ts : Use adapter pattern for protocol-specific implementations in the TypeScript SDK for different VMs
Applied to files:
typescript/rebalancer/src/factories/RebalancerContextFactory.ts
🧬 Code graph analysis (4)
typescript/http-registry-server/src/routes/chain.ts (1)
typescript/http-registry-server/src/services/chainService.ts (1)
ChainService(9-43)
typescript/rebalancer/src/service.ts (2)
typescript/rebalancer/src/config/RebalancerConfig.ts (1)
RebalancerConfig(12-39)typescript/rebalancer/src/core/RebalancerService.ts (1)
RebalancerService(95-333)
typescript/cli/src/commands/warp.ts (5)
typescript/cli/src/logger.ts (3)
logCommandHeader(60-62)errorRed(55-55)logGreen(44-45)typescript/rebalancer/src/config/RebalancerConfig.ts (1)
RebalancerConfig(12-39)typescript/sdk/src/rebalancer/types.ts (1)
RebalancerConfig(155-155)typescript/rebalancer/src/core/RebalancerService.ts (1)
RebalancerService(95-333)typescript/utils/src/index.ts (1)
rootLogger(119-119)
typescript/rebalancer/src/factories/RebalancerContextFactory.ts (2)
typescript/rebalancer/src/config/RebalancerConfig.ts (1)
RebalancerConfig(12-39)typescript/rebalancer/src/index.ts (1)
RebalancerConfig(23-23)
🪛 markdownlint-cli2 (0.18.1)
typescript/rebalancer/README.md
115-115: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (29)
typescript/rebalancer/.gitignore (1)
1-3: Looks good, ogre or shrek. Standard ignores are all there.The entries cover the essentials—environment secrets, built artifacts, and cache directories. All makes sense for a TypeScript package.
If you feel like it down the road, you might toss in
node_modulesand.DS_Storeto keep things even tidier, but what's here is solid as a swamp. No issues blocking anything.typescript/rebalancer/.mocharc.json (1)
1-3: Configuration looks sound for the new rebalancer package.Configuring Mocha to use TSX for TypeScript imports is a solid choice—keeps things simple and gets the test harness running with minimal fuss. The setup aligns with what you'd want for a dedicated package that needs to handle TypeScript out of the box.
Just make sure the
tsxdependency is pinned in the package.json and that this config lines up with how other packages in the monorepo handle their test setups (if there's a pattern already in place, we'd want to keep things consistent).typescript/cli/src/context/strategies/chain/chainResolver.ts (1)
1-1: Import path updated correctly.The import now references the new dedicated package instead of the local path. Usage downstream remains consistent with the existing pattern.
.changeset/separate-rebalancer-package.md (1)
1-10: Changeset looks solid.Properly documents the package split and the dual-mode functionality. The versioning strategy (minor bumps for both packages) makes sense for this feature addition.
typescript/rebalancer/README.md (2)
1-178: Well-documented package introduction.The README provides clear guidance for both manual and daemon usage modes. The examples are practical and the migration notes will help existing users. Good work laying this all out.
115-137: Markdown linter false positive on directory tree.The linter flagged line 115 for missing a language specifier, but it's a directory tree structure diagram—doesn't need one. You can safely ignore that warning or add
textif you want to quiet the linter.typescript/rebalancer/eslint.config.mjs (1)
1-14: ESLint config looks appropriate.Extending the monorepo defaults and disabling import restrictions for Node.js built-ins makes sense since this is a service package. The comment explains the reasoning clearly.
typescript/rebalancer/src/config/RebalancerConfig.ts (1)
10-10: Import path adjusted for new package structure.The relative path now correctly points to the utility module within the rebalancer package hierarchy.
typescript/rebalancer/src/interfaces/IMonitor.ts (1)
3-3: Import path updated consistently.The error utility import now reflects the rebalancer package structure, matching the pattern used in other files.
typescript/rebalancer/tsconfig.json (1)
1-8: TypeScript configuration follows monorepo conventions.The setup extends the base config properly and sets up the standard build structure. All looks good here.
Dockerfile (1)
35-35: Looks good to me, donkey!The new rebalancer package manifest is placed right where it belongs in the swamp — er, I mean alphabetically between
radix-sdkandsdk. This ensures proper dependency installation during image build. Nice and tidy.typescript/http-registry-server/src/routes/chain.ts (1)
1-1: These type annotations are worth their weight in onions.Adding explicit
IRouterreturn type and using type-only import is a solid improvement. Makes the code more self-documenting and aligns nicely with the other router files in this PR. No functional changes, just cleaner typing.Also applies to: 13-13
typescript/http-registry-server/src/routes/warp.ts (1)
1-1: This is my swamp, and these types are welcome in it.Consistent with the
chain.tschanges — explicitIRouterreturn type and proper type-only import. Keeps all router factories speaking the same language.Also applies to: 13-13
typescript/rebalancer/src/utils/errors.ts (1)
1-5: A proper error wrapper, like layers on an ogre.Clean implementation using ES2022's error
causeoption. Since the package requires Node >= 18, this works perfectly fine. Handy for preserving the original error context when wrapping exceptions.typescript/http-registry-server/src/routes/root.ts (1)
1-1: All three router files now match, like a proper set of layers.Consistent
IRoutertyping acrosschain.ts,warp.ts, and nowroot.ts. Good housekeeping.Also applies to: 8-8
typescript/cli/src/commands/warp.ts (2)
5-5: The rebalancer integration looks proper and tidy.Good use of the new
@hyperlane-xyz/rebalancerpackage APIs. TheRebalancerConfig.load()andRebalancerServiceconstructor usage aligns with the package's interface. UsingrootLogger.child({ module: 'rebalancer' })for scoped logging is a nice touch.Also applies to: 19-19, 508-554
547-550: Signal handling is already implemented in RebalancerService.start().The daemon service properly registers SIGINT and SIGTERM handlers (lines 237–238 in RebalancerService.ts) before starting the monitor, and implements graceful shutdown cleanup. No additional signal handling is needed in the CLI.
typescript/rebalancer/package.json (1)
8-10: > Likely an incorrect or invalid review comment.typescript/rebalancer/src/service.ts (2)
31-44: Version loading looks good, just a wee note.The fallback to
'unknown'and the warning log when package.json can't be read is sensible. The dynamic imports forfs,path, andurlwork fine here since this is an async context. Nice and tidy.
86-137: Service initialization and startup flow looks solid.The sequence of loading config, initializing registry, setting up providers, and starting the service is well-structured. Error handling wraps the whole block and exits with a non-zero code on failure, which is what ye want for container environments.
typescript/rebalancer/src/core/RebalancerService.ts (4)
119-163: Initialization flow is clean and well-structured.The lazy initialization pattern with the early return if already initialized is a good approach. The conditional creation of monitor, metrics, strategy, and rebalancer based on mode and config options is nicely organized.
168-216: Manual rebalance execution looks proper.Good validation of the origin token lookup, amount parsing, and the assert for positive amounts. The error handling with re-throwing after logging is appropriate for letting callers handle failures.
312-325: Monitor error handling re-throws MonitorStartError.Just confirming this is intentional - throwing from an event handler will cause an unhandled rejection if the monitor emits this error asynchronously. If
MonitorStartErroronly occurs during the synchronousstart()call, this should be fine.
297-306: The fire-and-forget pattern is safe. Therebalancerinstance returned bycreateRebalancer()is wrapped withWithSemaphore, which uses anexecutingflag to prevent concurrent rebalance operations. If a rebalance call arrives while one is already executing,WithSemaphore.rebalance()returns early (line 30-34 in WithSemaphore.ts), ensuring no overlaps occur regardless ofcheckFrequencytiming.typescript/rebalancer/src/index.ts (1)
1-68: Nice and tidy barrel file for the new package.The exports are well-organized with clear section comments. Types are properly exported using
export typewhere appropriate, and the public API surface looks comprehensive. This gives consumers a clean entry point without exposing internal implementation details.typescript/rebalancer/src/utils/files.ts (1)
205-237: Interactive file selection utility looks handy.The
runFileSelectionStepfunction with inquirer prompts is well-structured. Good error messages and the fallback to manual input is a nice touch.typescript/rebalancer/src/factories/RebalancerContextFactory.ts (3)
67-72: Nice fallback for MultiProtocolProvider creation.The pattern of accepting an optional
MultiProtocolProviderand creating one fromMultiProviderif not provided gives flexibility. This allows the CLI to pass its existing provider while the daemon can just passundefined.
49-99: Factory creation cleanly decoupled from CLI context.The refactoring to accept
MultiProviderandIRegistrydirectly instead ofWriteCommandContextmakes this factory reusable across different entry points - CLI, daemon, or tests. The warp route config loading and WarpCore creation flow is straightforward.
160-188: Rebalancer creation looks proper with the new provider pattern.Using
this.multiProvider.metadatafor chain metadata and passing themultiProviderdirectly to the Rebalancer keeps things consistent. The semaphore wrapping for concurrency control is in place.
paulbalaji
force-pushed
the
pbio/rebalancer-package-move
branch
from
5a8bd28 to
7d3cf0f
Compare
paulbalaji
force-pushed
the
pbio/rebalancer-package-move
branch
from
7d3cf0f to
36aa15c
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
typescript/rebalancer/package.json (1)
33-34: Move development-only packages to devDependencies.Since this package is published (indicated by the
"files"and"bin"entries), the@inquirer/prompts,@inquirer/select, andpino-prettypackages should be indevDependenciesrather thandependencies. These are development tools - the inquirer packages are for interactive CLI prompts (not needed for the daemon service), and pino-pretty is a log formatter typically only used during local development.Move these three packages to the
devDependenciessection:"dependencies": { "@google-cloud/pino-logging-gcp-config": "^1.0.6", "@hyperlane-xyz/core": "10.0.5", "@hyperlane-xyz/provider-sdk": "0.3.0", "@hyperlane-xyz/registry": "23.7.0", "@hyperlane-xyz/sdk": "19.11.0", "@hyperlane-xyz/utils": "19.11.0", - "@inquirer/prompts": "3.3.2", - "@inquirer/select": "^1.3.3", "bignumber.js": "^9.1.1", "ethers": "^5.8.0", "express": "^5.1.0", "pino": "^8.19.0", - "pino-pretty": "^13.0.0", "prom-client": "^14.0.1", "yaml": "2.4.5", "zod": "^3.21.2", "zod-validation-error": "^3.3.0" }, "devDependencies": { + "@inquirer/prompts": "3.3.2", + "@inquirer/select": "^1.3.3", "@types/express": "^5.0.3", "@types/mocha": "^10.0.1", "@types/node": "^18.14.5", "chai": "^4.5.0", "eslint": "^9.31.0", "mocha": "^11.5.0", + "pino-pretty": "^13.0.0", "prettier": "^3.5.3", "tsx": "^4.19.1", "typescript": "5.3.3" },Also applies to: 39-39
typescript/rebalancer/README.md (1)
115-136: Add language specifier to fenced code block.The package structure diagram would benefit from a language identifier. Consider using
textorplaintextfor the tree structure to ensure consistent rendering across different Markdown viewers.Apply this change:
-``` +```text typescript/rebalancer/ ├── src/ │ ├── core/typescript/rebalancer/src/config/RebalancerConfig.test.ts (3)
4-5: Temp-path setup and cleanup look solid; consider uniqueness if you go parallel laterUsing
tmpdir()plus a sharedTEST_CONFIG_PATHand cleaning it up withrmSync(..., { force: true })keeps the repo from getting muddy and is fine for the usual single-process Mocha run. If you ever start running this suite in parallel workers or multiple processes, it might be worth salting the filename (e.g. with PID or a random suffix) so separate workers don’t stomp on each other’s config file in the same OS temp dir.Also applies to: 12-17, 47-52
54-59: Exact error-message match is a bit brittleAsserting the full error string, including the entire
TEST_CONFIG_PATH, ties this test pretty hard to the current wording ofreadYamlOrJson’s error and to that path format. If the helper ever tweaks its phrasing, behavior stays fine but this test breaks. You might consider a looser check (e.g. regex orincludes('File doesn\'t exist')) while still asserting that the thrown error mentions the path.
47-48: Tiny helper could DRY up config-file writesRight now each test hand-rolls
writeYamlOrJson(TEST_CONFIG_PATH, data)after shapingdata. That’s perfectly workable, but if you want to keep this swamp a bit tidier, a small local helper likeconst writeConfig = (cfg: RebalancerConfigFileInput) => writeYamlOrJson(TEST_CONFIG_PATH, cfg);would cut the repetition and make any future path or write behavior changes a one-liner.Also applies to: 92-93, 103-104, 138-139, 176-177, 229-232, 281-284, 321-324, 367-370
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📥 Commits
Reviewing files that changed from the base of the PR and between 5a8bd28ab3eb56ba442f38ad2f50af3dadf64e65 and 36aa15cad1d6cad3cd8e0b42c83fc0fb2c5bf2dc.
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (25)
.changeset/separate-rebalancer-package.md(1 hunks)Dockerfile(1 hunks)typescript/cli/package.json(1 hunks)typescript/cli/src/commands/warp.ts(3 hunks)typescript/cli/src/context/strategies/chain/chainResolver.ts(1 hunks)typescript/cli/src/rebalancer/README.md(0 hunks)typescript/cli/src/rebalancer/index.ts(0 hunks)typescript/cli/src/rebalancer/runner.ts(0 hunks)typescript/http-registry-server/src/routes/chain.ts(2 hunks)typescript/http-registry-server/src/routes/root.ts(1 hunks)typescript/http-registry-server/src/routes/warp.ts(2 hunks)typescript/rebalancer/.gitignore(1 hunks)typescript/rebalancer/.mocharc.json(1 hunks)typescript/rebalancer/README.md(1 hunks)typescript/rebalancer/eslint.config.mjs(1 hunks)typescript/rebalancer/package.json(1 hunks)typescript/rebalancer/src/config/RebalancerConfig.test.ts(10 hunks)typescript/rebalancer/src/config/RebalancerConfig.ts(1 hunks)typescript/rebalancer/src/core/RebalancerService.ts(1 hunks)typescript/rebalancer/src/factories/RebalancerContextFactory.ts(6 hunks)typescript/rebalancer/src/index.ts(1 hunks)typescript/rebalancer/src/interfaces/IMonitor.ts(1 hunks)typescript/rebalancer/src/service.ts(1 hunks)typescript/rebalancer/src/utils/errors.ts(1 hunks)typescript/rebalancer/tsconfig.json(1 hunks)
💤 Files with no reviewable changes (3)
- typescript/cli/src/rebalancer/index.ts
- typescript/cli/src/rebalancer/runner.ts
- typescript/cli/src/rebalancer/README.md
🚧 Files skipped from review as they are similar to previous changes (11)
- typescript/http-registry-server/src/routes/warp.ts
- typescript/rebalancer/tsconfig.json
- typescript/http-registry-server/src/routes/root.ts
- .changeset/separate-rebalancer-package.md
- typescript/http-registry-server/src/routes/chain.ts
- typescript/rebalancer/.gitignore
- typescript/cli/package.json
- typescript/rebalancer/eslint.config.mjs
- typescript/rebalancer/src/config/RebalancerConfig.ts
- typescript/rebalancer/src/core/RebalancerService.ts
- typescript/cli/src/context/strategies/chain/chainResolver.ts
🧰 Additional context used
🧠 Learnings (11)
📚 Learning: 2025-11-24T17:19:38.362Z
Learnt from: CR
Repo: hyperlane-xyz/hyperlane-monorepo PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:19:38.362Z
Learning: Applies to typescript/sdk/src/**/*.ts : TypeScript SDK must provide `MultiProvider` for multi-chain provider management, `HyperlaneCore` factory for core contract interactions, and `MultiProtocolCore` for protocol-agnostic abstractions across EVM, Cosmos, Sealevel, and Starknet
Applied to files:
Dockerfiletypescript/rebalancer/src/service.tstypescript/rebalancer/src/index.tstypescript/rebalancer/src/factories/RebalancerContextFactory.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/rebalancer/package.json
📚 Learning: 2025-11-26T13:28:51.658Z
Learnt from: paulbalaji
Repo: hyperlane-xyz/hyperlane-monorepo PR: 7410
File: typescript/cli/package.json:20-20
Timestamp: 2025-11-26T13:28:51.658Z
Learning: In the hyperlane-xyz/hyperlane-monorepo repository, hyperlane-xyz/registry is maintained in a separate repository and published to npm, so it should use a pinned version (e.g., "23.6.0") rather than the workspace protocol ("workspace:*") that other internal Hyperlane packages use.
Applied to files:
typescript/rebalancer/package.jsontypescript/rebalancer/src/factories/RebalancerContextFactory.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/rebalancer/package.jsontypescript/rebalancer/src/service.tstypescript/rebalancer/src/config/RebalancerConfig.test.tstypescript/rebalancer/.mocharc.json
📚 Learning: 2025-10-21T14:26:22.163Z
Learnt from: paulbalaji
Repo: hyperlane-xyz/hyperlane-monorepo PR: 7223
File: typescript/infra/src/config/chain.ts:56-58
Timestamp: 2025-10-21T14:26:22.163Z
Learning: When adding chains to chainsToSkip in typescript/infra/src/config/chain.ts, it's the expected pattern to keep the chain's configurations in other files like supportedChainNames.ts, agent.ts, validators.ts, and owners.ts. The chainsToSkip array is used selectively by specific scripts (e.g., check-owner-ica.ts) to exclude chains from certain operations, not as a signal to remove all configurations.
Applied to files:
typescript/rebalancer/src/config/RebalancerConfig.test.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/cli/src/commands/warp.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/cli/src/commands/warp.ts
📚 Learning: 2025-06-16T16:22:12.017Z
Learnt from: xeno097
Repo: hyperlane-xyz/hyperlane-monorepo PR: 5883
File: typescript/cli/src/commands/fork.ts:12-39
Timestamp: 2025-06-16T16:22:12.017Z
Learning: In yargs CLI applications, hyphenated option names (kebab-case) like 'fork-config' are automatically converted to camelCase (forkConfig) when passed to handler functions. This means destructuring { forkConfig } from handler arguments when the option is defined as 'fork-config' is correct and will work as expected.
Applied to files:
typescript/cli/src/commands/warp.ts
📚 Learning: 2025-11-24T17:19:38.362Z
Learnt from: CR
Repo: hyperlane-xyz/hyperlane-monorepo PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:19:38.362Z
Learning: Applies to typescript/sdk/src/**/*.ts : Use `ChainMap<T>` for type-safe per-chain configuration mapping in TypeScript SDK
Applied to files:
typescript/rebalancer/src/factories/RebalancerContextFactory.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/rebalancer/.mocharc.json
📚 Learning: 2025-11-24T17:19:38.362Z
Learnt from: CR
Repo: hyperlane-xyz/hyperlane-monorepo PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:19:38.362Z
Learning: For agent deployment, redeployment, RPC URL rotation, validator operations, manual message processing, balance management, key funding, security incident response, and Lander configuration, follow procedures detailed in the Operations Runbook at https://www.notion.so/hyperlanexyz/Runbook-AI-Agent-24a6d35200d680229b38e8501164ca66
Applied to files:
typescript/rebalancer/README.md
🧬 Code graph analysis (4)
typescript/rebalancer/src/service.ts (3)
typescript/rebalancer/src/config/RebalancerConfig.ts (1)
RebalancerConfig(11-38)typescript/rebalancer/src/index.ts (2)
RebalancerConfig(23-23)RebalancerService(11-11)typescript/rebalancer/src/core/RebalancerService.ts (1)
RebalancerService(95-333)
typescript/rebalancer/src/config/RebalancerConfig.test.ts (1)
typescript/rebalancer/src/config/RebalancerConfig.ts (1)
RebalancerConfig(11-38)
typescript/cli/src/commands/warp.ts (4)
typescript/cli/src/logger.ts (3)
logCommandHeader(60-62)errorRed(55-55)logGreen(44-45)typescript/rebalancer/src/config/RebalancerConfig.ts (1)
RebalancerConfig(11-38)typescript/sdk/src/rebalancer/types.ts (1)
RebalancerConfig(155-155)typescript/rebalancer/src/core/RebalancerService.ts (1)
RebalancerService(95-333)
typescript/rebalancer/src/factories/RebalancerContextFactory.ts (1)
typescript/rebalancer/src/config/RebalancerConfig.ts (1)
RebalancerConfig(11-38)
🪛 markdownlint-cli2 (0.18.1)
typescript/rebalancer/README.md
115-115: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ 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). (59)
- GitHub Check: cli-evm-e2e-matrix (warp-send)
- GitHub Check: cli-evm-e2e-matrix (warp-read)
- GitHub Check: cli-evm-e2e-matrix (warp-check-3)
- GitHub Check: cli-evm-e2e-matrix (warp-check-5)
- GitHub Check: cli-evm-e2e-matrix (warp-check-4)
- GitHub Check: cli-evm-e2e-matrix (warp-check-1)
- GitHub Check: cli-evm-e2e-matrix (warp-check-2)
- GitHub Check: cli-evm-e2e-matrix (warp-init)
- GitHub Check: cli-evm-e2e-matrix (warp-rebalancer)
- GitHub Check: cli-evm-e2e-matrix (warp-deploy-1)
- GitHub Check: cli-evm-e2e-matrix (warp-bridge-2)
- 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 (warp-extend-config)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-ism-updates)
- GitHub Check: cli-evm-e2e-matrix (warp-extend-basic)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-submitters)
- 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-apply-hook-updates)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-rebalancing-config)
- GitHub Check: cli-evm-e2e-matrix (relay)
- GitHub Check: cli-evm-e2e-matrix (core-check)
- GitHub Check: cli-evm-e2e-matrix (warp-extend-recovery)
- GitHub Check: cli-evm-e2e-matrix (core-deploy)
- GitHub Check: cli-evm-e2e-matrix (core-init)
- GitHub Check: cli-evm-e2e-matrix (core-read)
- GitHub Check: cli-evm-e2e-matrix (core-apply)
- GitHub Check: env-test-matrix (mainnet3, ethereum, igp)
- GitHub Check: env-test-matrix (mainnet3, optimism, igp)
- GitHub Check: env-test-matrix (mainnet3, optimism, core)
- GitHub Check: env-test-matrix (mainnet3, arbitrum, core)
- GitHub Check: env-test-matrix (mainnet3, ethereum, core)
- GitHub Check: env-test-matrix (testnet4, sepolia, core)
- GitHub Check: env-test-matrix (mainnet3, arbitrum, igp)
- GitHub Check: cli-cosmos-e2e-matrix (warp-deploy)
- GitHub Check: aleo-sdk-e2e-run
- GitHub Check: cli-cosmos-e2e-matrix (warp-read)
- GitHub Check: cli-cosmos-e2e-matrix (core-read)
- GitHub Check: cosmos-sdk-e2e-run
- GitHub Check: cli-cosmos-e2e-matrix (core-deploy)
- GitHub Check: cli-cosmos-e2e-matrix (core-apply)
- GitHub Check: cli-cosmos-e2e-matrix (core-check)
- GitHub Check: cli-radix-e2e-matrix (warp-apply-ownership-updates)
- GitHub Check: cli-radix-e2e-matrix (core-deploy)
- GitHub Check: cli-cross-chain-e2e-matrix (warp-apply)
- GitHub Check: cli-radix-e2e-matrix (warp-apply-route-extension)
- GitHub Check: cli-radix-e2e-matrix (warp-deploy)
- GitHub Check: cli-install-test-run
- GitHub Check: cli-cross-chain-e2e-matrix (warp-deploy)
- GitHub Check: yarn-test-run
- GitHub Check: infra-test
- GitHub Check: build-and-push-to-gcr
- GitHub Check: agent-configs (mainnet3)
- GitHub Check: e2e-matrix (evm)
- GitHub Check: agent-configs (testnet4)
- GitHub Check: lint-rs
- GitHub Check: lander-coverage
- GitHub Check: test-rs
🔇 Additional comments (16)
typescript/rebalancer/.mocharc.json (1)
1-3: Configuration is solid and properly aligned.TSX is correctly listed as a dependency in the rebalancer package (
"tsx": "^4.19.1"), and this minimal Mocha config matches the pattern used across most other TypeScript packages in the monorepo (deploy-sdk, http-registry-server, sdk, utils). The setup is straightforward and correct.Dockerfile (1)
36-36: Looks good!The new rebalancer package is properly included in the Docker build context, following the same pattern as the other packages. Nice and tidy.
typescript/rebalancer/src/interfaces/IMonitor.ts (1)
3-3: Import path updated correctly.The path adjustment from
../../utils/errors.jsto../utils/errors.jsis spot-on for the new package structure.typescript/rebalancer/src/utils/errors.ts (1)
1-5: Clean error wrapper implementation.The WrappedError class is well-implemented, properly leveraging the modern Error constructor's
causeoption. Simple and effective.typescript/cli/src/commands/warp.ts (2)
5-5: Imports updated correctly.The CLI now imports from the new
@hyperlane-xyz/rebalancerpackage and usesrootLoggerfrom utils. Clean separation of concerns.Also applies to: 19-19
494-554: Rebalancer handler refactored successfully.The handler logic has been nicely refactored to use the new
RebalancerService. The manual mode validation (lines 533-538) ensures required parameters are present, and both manual and daemon execution paths are handled cleanly. Error handling looks solid throughout.typescript/rebalancer/src/service.ts (2)
34-45: Version resolution handled well.The
getVersionfunction safely reads from package.json with proper error handling. The fallback to "unknown" ensures the service can start even if version detection fails.
47-148: Service initialization is solid.The
mainfunction provides thorough environment variable validation, clear error messages, and a well-structured initialization sequence. The balance check frequency validation (lines 64-73) ensures only positive values are accepted, and the service setup properly wires together all the required components (MultiProvider, MultiProtocolProvider, registry, config).typescript/rebalancer/src/index.ts (1)
1-68: Public API surface is well-organized.The index file cleanly exposes the package's public API with clear sections for each concern (core service, configuration, strategy, monitor, metrics, interfaces, utils, factory). The use of type-only exports where appropriate keeps things tidy.
typescript/rebalancer/src/factories/RebalancerContextFactory.ts (7)
3-10: Well organized imports, looking good!The imports align nicely with the refactor objectives. MultiProvider and IRegistry give you the tools needed for multi-chain operations and registry access without the CLI context dependency.
33-40: Constructor refactor is solid.Swapping out the context for multiProvider and registry makes this factory reusable outside the CLI. The parameter list is clean and well-documented.
68-72: MultiProtocolProvider setup looks proper.The nullish coalescing operator handles the optional parameter gracefully, and extending chain metadata with mailboxes ensures Sealevel warp adapters have what they need. Nice and tidy.
190-209: Private helper method looks good.The
getInitialTotalCollateral()logic is unchanged and correctly calculates the total bridged supply across eligible collateralized tokens. No concerns here.
114-118: No action needed. The migration tothis.multiProvider.metadatais correct—this property returnsChainMap<ChainMetadata>, which is exactly whatPriceGetter.create()expects as its first parameter. This is the standard pattern used throughout the codebase for accessing chain metadata.
52-52: The optionalmultiProtocolProviderparameter is not used in practice — the only caller (RebalancerService) always passes a pre-constructed instance.The factory signature accepts
multiProtocolProvideras optional and falls back to creating it frommultiProviderif undefined (lines 69–72), but RebalancerService.ts (lines 128–134) always constructs and passes an explicit MultiProtocolProvider instance. This pattern is defensive but unnecessary given current usage. Consider making the parameter required if it will always be provided, or document the optional fallback for future extensibility.
180-187: Reconsider the WithSemaphore concerns—it serves rate limiting, not concurrency control.The WithSemaphore wrapper is designed for timing control between rebalancing operations, not concurrency limits. It uses a simple
executingflag to prevent overlapping executions and awaitUntiltimestamp to enforce waiting periods based on bridge lock times from the configuration. The locking duration is determined per-route by the highestbridgeLockTimein the involved chains, which is appropriate for both daemon and manual execution modes.Only one caller exists in the codebase (RebalancerService in daemon mode), so there are no manual/CLI execution paths affected. Comprehensive tests already cover the semaphore behavior including concurrent execution prevention and waiting period enforcement.
Likely an incorrect or invalid review comment.
paulbalaji
force-pushed
the
pbio/rebalancer-package-move
branch
from
36aa15c to
594f40d
Compare
|
All alerts resolved. Learn more about Socket for GitHub. This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
typescript/rebalancer/eslint.config.mjs (2)
5-7: Remove the unused files pattern.This config object only specifies a files pattern without any rules or settings, so it doesn't actually do anything in the flat config format. Each object in the array is independent, and a standalone files pattern won't scope the rules that follow.
If you meant to apply the
no-restricted-importsoverride only to TypeScript files, combine the files pattern with the rules in a single object. Otherwise, this can be removed.Apply this diff to remove the unnecessary object:
export default [ ...MonorepoDefaults, - { - files: ['./src/**/*.ts'], - }, { rules: {
9-11: Consider using string syntax for consistency.While
['off']works fine, the more idiomatic syntax for disabling a rule is just'off'as a string rather than wrapping it in an array.Apply this diff for consistency:
rules: { // Disable restricted imports for Node.js built-ins since rebalancer is a Node.js-only service - 'no-restricted-imports': ['off'], + 'no-restricted-imports': 'off', },typescript/rebalancer/src/config/RebalancerConfig.test.ts (2)
4-5: Temp-file setup is sane; consider per-test isolation if you ever go parallelUsing
tmpdir()plusjoinand a sharedTEST_CONFIG_PATHkeeps the tests tidy and OS-agnostic, and wiring them through@hyperlane-xyz/utils/fsmirrors production nicely.Only thing to keep in the back of your mind: if these tests ever run in parallel workers, a single fixed filename in the global tmpdir can cause cross-test clobbering. If that becomes a problem later, switching to something like a per-test directory (e.g. via
mkdtempor a randomized filename) would harden things without changing the assertions.Also applies to: 12-16
47-52: Good alignment between config writer, loader, and error expectationsAll the spots where you now
writeYamlOrJson(TEST_CONFIG_PATH, data)and then callRebalancerConfig.load(TEST_CONFIG_PATH)make the tests exercise exactly the same read/write path as real usage, which is solid.You’re also asserting on the full error string including
TEST_CONFIG_PATH. Since both the throw site and the expectation interpolate the same constant, that’s stable enough, just a bit tightly coupled to the exact message shape. If you ever want looser coupling, you could assert on a substring (e.g. message containing"File doesn't exist at"), but there’s nothing functionally wrong with the current setup.Also applies to: 55-59, 63-64, 92-95, 103-106, 138-142, 176-180, 229-233, 281-284, 321-324, 367-370
typescript/rebalancer/README.md (1)
115-136: Add a language identifier to the code fence.The directory structure block at line 115 should specify a language (e.g.,
textorplaintext) for proper rendering.Apply this diff:
-``` +```text typescript/rebalancer/ ├── src/
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📥 Commits
Reviewing files that changed from the base of the PR and between 36aa15cad1d6cad3cd8e0b42c83fc0fb2c5bf2dc and 594f40d7911977d15d5907a01c9264ec4afab6f2.
📒 Files selected for processing (20)
.changeset/separate-rebalancer-package.md(1 hunks)Dockerfile(1 hunks)typescript/cli/package.json(1 hunks)typescript/cli/src/commands/warp.ts(3 hunks)typescript/cli/src/context/strategies/chain/chainResolver.ts(1 hunks)typescript/cli/src/rebalancer/README.md(0 hunks)typescript/cli/src/rebalancer/index.ts(0 hunks)typescript/cli/src/rebalancer/runner.ts(0 hunks)typescript/rebalancer/.gitignore(1 hunks)typescript/rebalancer/.mocharc.json(1 hunks)typescript/rebalancer/README.md(1 hunks)typescript/rebalancer/eslint.config.mjs(1 hunks)typescript/rebalancer/package.json(1 hunks)typescript/rebalancer/src/config/RebalancerConfig.test.ts(10 hunks)typescript/rebalancer/src/config/RebalancerConfig.ts(1 hunks)typescript/rebalancer/src/core/RebalancerService.ts(1 hunks)typescript/rebalancer/src/factories/RebalancerContextFactory.ts(6 hunks)typescript/rebalancer/src/index.ts(1 hunks)typescript/rebalancer/src/interfaces/IMonitor.ts(1 hunks)typescript/rebalancer/src/service.ts(1 hunks)
💤 Files with no reviewable changes (3)
- typescript/cli/src/rebalancer/README.md
- typescript/cli/src/rebalancer/runner.ts
- typescript/cli/src/rebalancer/index.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- typescript/rebalancer/.gitignore
- typescript/cli/package.json
- Dockerfile
- typescript/cli/src/context/strategies/chain/chainResolver.ts
- typescript/rebalancer/src/core/RebalancerService.ts
- typescript/rebalancer/src/config/RebalancerConfig.ts
- typescript/rebalancer/src/service.ts
- typescript/rebalancer/package.json
🧰 Additional context used
📓 Path-based instructions (1)
typescript/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
typescript/**/*.{ts,tsx,js,jsx}: Format TypeScript/JavaScript code using Prettier with: pnpm prettier
Lint TypeScript/JavaScript code with: pnpm lint
Files:
typescript/rebalancer/src/config/RebalancerConfig.test.tstypescript/rebalancer/src/index.tstypescript/cli/src/commands/warp.tstypescript/rebalancer/src/interfaces/IMonitor.tstypescript/rebalancer/src/factories/RebalancerContextFactory.ts
🧠 Learnings (12)
📓 Common learnings
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.
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.
📚 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/rebalancer/src/config/RebalancerConfig.test.tstypescript/rebalancer/eslint.config.mjstypescript/rebalancer/.mocharc.json
📚 Learning: 2025-10-21T14:26:22.163Z
Learnt from: paulbalaji
Repo: hyperlane-xyz/hyperlane-monorepo PR: 7223
File: typescript/infra/src/config/chain.ts:56-58
Timestamp: 2025-10-21T14:26:22.163Z
Learning: When adding chains to chainsToSkip in typescript/infra/src/config/chain.ts, it's the expected pattern to keep the chain's configurations in other files like supportedChainNames.ts, agent.ts, validators.ts, and owners.ts. The chainsToSkip array is used selectively by specific scripts (e.g., check-owner-ica.ts) to exclude chains from certain operations, not as a signal to remove all configurations.
Applied to files:
typescript/rebalancer/src/config/RebalancerConfig.test.ts
📚 Learning: 2025-12-15T15:33:46.049Z
Learnt from: CR
Repo: hyperlane-xyz/hyperlane-monorepo PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T15:33:46.049Z
Learning: Applies to typescript/sdk/src/**/*.ts : TypeScript SDK should use HyperlaneCore factory for core contract interactions and MultiProtocolCore for protocol-agnostic abstractions (EVM, Cosmos, Sealevel, Starknet)
Applied to files:
typescript/rebalancer/src/index.tstypescript/rebalancer/src/factories/RebalancerContextFactory.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/cli/src/commands/warp.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/cli/src/commands/warp.ts
📚 Learning: 2025-06-16T16:22:12.017Z
Learnt from: xeno097
Repo: hyperlane-xyz/hyperlane-monorepo PR: 5883
File: typescript/cli/src/commands/fork.ts:12-39
Timestamp: 2025-06-16T16:22:12.017Z
Learning: In yargs CLI applications, hyphenated option names (kebab-case) like 'fork-config' are automatically converted to camelCase (forkConfig) when passed to handler functions. This means destructuring { forkConfig } from handler arguments when the option is defined as 'fork-config' is correct and will work as expected.
Applied to files:
typescript/cli/src/commands/warp.ts
📚 Learning: 2025-12-15T15:33:46.049Z
Learnt from: CR
Repo: hyperlane-xyz/hyperlane-monorepo PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T15:33:46.049Z
Learning: Applies to typescript/sdk/src/**/*.ts : TypeScript SDK should use MultiProvider for multi-chain provider management with protocol adapters
Applied to files:
typescript/rebalancer/src/factories/RebalancerContextFactory.ts
📚 Learning: 2025-12-15T15:33:46.049Z
Learnt from: CR
Repo: hyperlane-xyz/hyperlane-monorepo PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T15:33:46.049Z
Learning: Applies to typescript/sdk/src/**/*.ts : TypeScript SDK should use ChainMap<T> for type-safe per-chain configuration mapping
Applied to files:
typescript/rebalancer/src/factories/RebalancerContextFactory.ts
📚 Learning: 2025-11-26T13:28:51.658Z
Learnt from: paulbalaji
Repo: hyperlane-xyz/hyperlane-monorepo PR: 7410
File: typescript/cli/package.json:20-20
Timestamp: 2025-11-26T13:28:51.658Z
Learning: In the hyperlane-xyz/hyperlane-monorepo repository, hyperlane-xyz/registry is maintained in a separate repository and published to npm, so it should use a pinned version (e.g., "23.6.0") rather than the workspace protocol ("workspace:*") that other internal Hyperlane packages use.
Applied to files:
typescript/rebalancer/src/factories/RebalancerContextFactory.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/rebalancer/.mocharc.json
📚 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/rebalancer/.mocharc.json
🧬 Code graph analysis (2)
typescript/rebalancer/src/config/RebalancerConfig.test.ts (1)
typescript/rebalancer/src/config/RebalancerConfig.ts (1)
RebalancerConfig(11-38)
typescript/rebalancer/src/factories/RebalancerContextFactory.ts (1)
typescript/rebalancer/src/config/RebalancerConfig.ts (1)
RebalancerConfig(11-38)
🪛 markdownlint-cli2 (0.18.1)
typescript/rebalancer/README.md
115-115: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ 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). (4)
- GitHub Check: e2e-matrix (evm)
- GitHub Check: test-rs
- GitHub Check: lander-coverage
- GitHub Check: lint-rs
🔇 Additional comments (10)
typescript/rebalancer/.mocharc.json (1)
1-3: LGTM! Clean and simple.The tsx import configuration lets you run TypeScript tests without compiling first. Good stuff.
typescript/rebalancer/src/interfaces/IMonitor.ts (1)
3-3: Path update looks right.The import path correctly reflects the new package structure with WrappedError now coming from the rebalancer's utils.
.changeset/separate-rebalancer-package.md (1)
1-10: Changeset looks solid.The package separation and version bumps are well documented. Both minor bumps make sense for this feature addition.
typescript/cli/src/commands/warp.ts (2)
5-5: Good import setup.The new rebalancer package integration is clean, pulling in RebalancerConfig and RebalancerService from the dedicated package, and rootLogger for proper logging infrastructure.
Also applies to: 19-19
493-555: Mode type is locked in tight - handler's all good.The mode strings are perfectly type-safe. RebalancerServiceConfig.mode accepts
'manual' | 'daemon'(literal union type), and the code generates exactly those values. The ternary operator creates the right string every time, so no runtime type mishaps lurking about. Handler refactoring is solid.typescript/rebalancer/src/index.ts (1)
1-68: Well-organized public API surface.The exports are logically grouped with clear documentation, proper separation of types and values, and comprehensive coverage of all the core components. Nice barrel export structure.
typescript/rebalancer/src/factories/RebalancerContextFactory.ts (4)
29-40: Clean refactoring to multiProvider and registry.The constructor and static create method now take explicit dependencies instead of context, which is better for testing and clearer about what's actually needed. Good separation of concerns.
Also applies to: 44-54
68-72: Smart fallback for MultiProtocolProvider.Creating the MultiProtocolProvider from the MultiProvider when not provided is a nice convenience. Keeps the API flexible.
180-187: Semaphore wrapping for concurrency control.Wrapping the rebalancer with WithSemaphore for concurrency control makes sense. Just make sure this doesn't change the interface in unexpected ways.
114-121: Migration to multiProvider.metadata and registry.getWarpDeployConfig is clean.No stray WriteCommandContext references or context usage patterns found in the rebalancer package. The migration is complete and there are no leftover context references hanging around like an ogre's layers.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
typescript/rebalancer/package.json (1)
26-44: Remove unused dependencies from package.jsonNeither
pino-prettynor the@inquirerpackages are actually used anywhere in the rebalancer codebase—not in runtime code, not in tests, nowhere. The code usespinodirectly (for type imports and test setup) and gets logger utilities from@hyperlane-xyz/utils. These belong in the dependencies section in name only, which is a bit like paying rent on rooms you never step into.Remove
"pino-pretty": "^13.0.0"and both@inquirerpackages from dependencies. If pretty-printed logging is needed, it should be an optional peer dependency (like it is in the utils package), so consuming projects opt into the extra weight.
🧹 Nitpick comments (1)
typescript/rebalancer/README.md (1)
115-115: Specify language for fenced code block.The architecture code block should declare the language for proper syntax highlighting and linting compliance.
-``` +``` typescript/rebalancer/
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📥 Commits
Reviewing files that changed from the base of the PR and between 64a6f48c0d7faac4527f6c1f6ecd5a7030ac2463 and a9c9627fff165845c1d9e29e0ce99533a56385e5.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (2)
typescript/rebalancer/README.md(1 hunks)typescript/rebalancer/package.json(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
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.
📚 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/rebalancer/package.json
📚 Learning: 2025-11-26T13:28:51.658Z
Learnt from: paulbalaji
Repo: hyperlane-xyz/hyperlane-monorepo PR: 7410
File: typescript/cli/package.json:20-20
Timestamp: 2025-11-26T13:28:51.658Z
Learning: In the hyperlane-xyz/hyperlane-monorepo repository, hyperlane-xyz/registry is maintained in a separate repository and published to npm, so it should use a pinned version (e.g., "23.6.0") rather than the workspace protocol ("workspace:*") that other internal Hyperlane packages use.
Applied to files:
typescript/rebalancer/package.json
📚 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/rebalancer/package.json
🪛 markdownlint-cli2 (0.18.1)
typescript/rebalancer/README.md
115-115: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ 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-cosmos-e2e-matrix (core-check)
- GitHub Check: cli-evm-e2e-matrix (core-init)
- GitHub Check: cli-evm-e2e-matrix (warp-check-2)
- GitHub Check: cli-evm-e2e-matrix (warp-extend-basic)
- GitHub Check: cli-evm-e2e-matrix (warp-check-3)
- GitHub Check: cli-evm-e2e-matrix (warp-check-5)
- GitHub Check: cli-cosmos-e2e-matrix (warp-deploy)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-ism-updates)
- GitHub Check: cli-evm-e2e-matrix (warp-send)
- GitHub Check: cli-evm-e2e-matrix (core-check)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-submitters)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-rebalancing-config)
- GitHub Check: cli-evm-e2e-matrix (core-read)
- GitHub Check: cli-evm-e2e-matrix (relay)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-simple-updates)
- GitHub Check: cli-evm-e2e-matrix (warp-rebalancer)
- GitHub Check: cli-evm-e2e-matrix (warp-deploy-2)
- GitHub Check: cli-evm-e2e-matrix (warp-bridge-1)
- GitHub Check: cli-evm-e2e-matrix (core-deploy)
- GitHub Check: cli-evm-e2e-matrix (warp-extend-config)
- GitHub Check: cli-evm-e2e-matrix (warp-bridge-2)
- GitHub Check: cli-evm-e2e-matrix (warp-deploy-1)
- GitHub Check: cli-evm-e2e-matrix (warp-init)
- GitHub Check: cli-evm-e2e-matrix (warp-read)
- GitHub Check: cli-evm-e2e-matrix (warp-check-4)
- GitHub Check: cli-evm-e2e-matrix (warp-extend-recovery)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-hook-updates)
- GitHub Check: cli-evm-e2e-matrix (warp-check-1)
- GitHub Check: cli-evm-e2e-matrix (core-apply)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-ownership-updates)
- GitHub Check: cli-cross-chain-e2e-matrix (warp-apply)
- GitHub Check: cli-cosmos-e2e-matrix (core-read)
- GitHub Check: cli-cosmos-e2e-matrix (core-apply)
- GitHub Check: cli-radix-e2e-matrix (warp-apply-route-extension)
- GitHub Check: cli-cosmos-e2e-matrix (warp-read)
- GitHub Check: cli-cosmos-e2e-matrix (core-deploy)
- GitHub Check: env-test-matrix (mainnet3, arbitrum, igp)
- GitHub Check: env-test-matrix (testnet4, sepolia, core)
- GitHub Check: env-test-matrix (mainnet3, ethereum, igp)
- GitHub Check: env-test-matrix (mainnet3, arbitrum, core)
- GitHub Check: pnpm-test-run
- GitHub Check: env-test-matrix (mainnet3, optimism, igp)
- GitHub Check: env-test-matrix (mainnet3, ethereum, core)
- GitHub Check: cli-radix-e2e-matrix (warp-deploy)
- 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: env-test-matrix (mainnet3, optimism, core)
- 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: infra-test
- GitHub Check: e2e-matrix (evm)
- GitHub Check: lint-prettier
- GitHub Check: agent-configs (testnet4)
- GitHub Check: agent-configs (mainnet3)
- GitHub Check: build-and-push-to-gcr
- GitHub Check: lint-rs
- GitHub Check: lander-coverage
- GitHub Check: test-rs
🔇 Additional comments (2)
typescript/rebalancer/package.json (2)
30-30: Registry dependency pinned correctly.Good catch using a pinned version (
23.7.0) for@hyperlane-xyz/registryinstead ofworkspace:*. That's the right pattern since registry is maintained separately and published to npm.
62-64: Node engine requirement is reasonable.Requiring Node >=18 aligns with modern TypeScript tooling and async/await support.
paulbalaji
force-pushed
the
pbio/rebalancer-package-move
branch
from
a9c9627 to
9f87fea
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
typescript/rebalancer/package.json (1)
26-44: Move development packages todevDependencies.Three packages have no usage in the runtime service code and should live in
devDependencies:
@inquirer/prompts(line 33)@inquirer/select(line 34)pino-pretty(line 39)All occurrences of these are zero in the service runtime. They belong in dev tooling, not in the production dependency tree.
typescript/rebalancer/src/service.ts (1)
34-45: Version getter could live insidemain, mate.There was a previous comment asking why this isn't inside the
mainfunction. MovinggetVersion()inline or calling it withinmainwould keep all initialization logic together in one place, like layers on a... well, you know.
🧹 Nitpick comments (3)
typescript/rebalancer/README.md (1)
115-136: Add language identifier to the code fence.The directory tree structure would render better with a language identifier. Toss a
textorplaintextafter those opening backticks at line 115.-``` +```text typescript/rebalancer/ ├── src/typescript/rebalancer/src/service.ts (1)
50-60: Environment validation looks good, but consider validating the private key format.The required env var checks are solid. However,
new Wallet(privateKey)on line 117 will throw a cryptic error if the key format is invalid. A quick validation before constructing the wallet would give users a clearer error message about what went wrong.const privateKey = process.env.HYP_KEY; if (!privateKey) { rootLogger.error('HYP_KEY environment variable is required'); process.exit(1); } + + // Basic format validation - Wallet constructor will do full validation + if (!privateKey.startsWith('0x') || privateKey.length !== 66) { + rootLogger.error('HYP_KEY must be a valid 64-character hex string prefixed with 0x'); + process.exit(1); + }typescript/rebalancer/src/core/RebalancerService.ts (1)
269-275: Callingprocess.exit(0)in gracefulShutdown may complicate testing.Directly calling
process.exit()makes this class harder to test in isolation. Consider making the exit behavior injectable or conditional, or returning a status instead of exiting. For production daemon use this is fine, but it's something to keep in mind if you want unit test coverage on the shutdown path.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📥 Commits
Reviewing files that changed from the base of the PR and between a9c9627fff165845c1d9e29e0ce99533a56385e5 and 9f87fea57ddf781d6523c81aa5377b890f7d3091.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (23)
.changeset/separate-rebalancer-package.md(1 hunks)Dockerfile(1 hunks)typescript/cli/package.json(2 hunks)typescript/cli/src/commands/warp.ts(3 hunks)typescript/cli/src/context/strategies/chain/chainResolver.ts(1 hunks)typescript/cli/src/rebalancer/README.md(0 hunks)typescript/cli/src/rebalancer/index.ts(0 hunks)typescript/cli/src/rebalancer/runner.ts(0 hunks)typescript/infra/package.json(1 hunks)typescript/rebalancer/.gitignore(1 hunks)typescript/rebalancer/.mocharc.json(1 hunks)typescript/rebalancer/README.md(1 hunks)typescript/rebalancer/eslint.config.mjs(1 hunks)typescript/rebalancer/package.json(1 hunks)typescript/rebalancer/src/config/RebalancerConfig.test.ts(10 hunks)typescript/rebalancer/src/config/RebalancerConfig.ts(1 hunks)typescript/rebalancer/src/core/RebalancerService.ts(1 hunks)typescript/rebalancer/src/factories/RebalancerContextFactory.ts(6 hunks)typescript/rebalancer/src/index.ts(1 hunks)typescript/rebalancer/src/interfaces/IMonitor.ts(1 hunks)typescript/rebalancer/src/service.ts(1 hunks)typescript/rebalancer/src/utils/errors.ts(1 hunks)typescript/rebalancer/tsconfig.json(1 hunks)
💤 Files with no reviewable changes (3)
- typescript/cli/src/rebalancer/runner.ts
- typescript/cli/src/rebalancer/index.ts
- typescript/cli/src/rebalancer/README.md
🚧 Files skipped from review as they are similar to previous changes (8)
- typescript/cli/src/context/strategies/chain/chainResolver.ts
- typescript/rebalancer/.gitignore
- typescript/cli/package.json
- typescript/rebalancer/tsconfig.json
- typescript/rebalancer/.mocharc.json
- typescript/infra/package.json
- typescript/rebalancer/eslint.config.mjs
- Dockerfile
🧰 Additional context used
📓 Path-based instructions (1)
typescript/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
typescript/**/*.{ts,tsx,js,jsx}: Format TypeScript/JavaScript code using Prettier with: pnpm prettier
Lint TypeScript/JavaScript code with: pnpm lint
Files:
typescript/rebalancer/src/config/RebalancerConfig.tstypescript/rebalancer/src/config/RebalancerConfig.test.tstypescript/rebalancer/src/interfaces/IMonitor.tstypescript/cli/src/commands/warp.tstypescript/rebalancer/src/factories/RebalancerContextFactory.tstypescript/rebalancer/src/core/RebalancerService.tstypescript/rebalancer/src/index.tstypescript/rebalancer/src/service.tstypescript/rebalancer/src/utils/errors.ts
🧠 Learnings (14)
📓 Common learnings
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.
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.
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-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/rebalancer/src/config/RebalancerConfig.tstypescript/rebalancer/src/config/RebalancerConfig.test.tstypescript/rebalancer/package.jsontypescript/rebalancer/src/service.tstypescript/rebalancer/README.md
📚 Learning: 2025-12-11T16:29:54.086Z
Learnt from: ltyu
Repo: hyperlane-xyz/hyperlane-monorepo PR: 7577
File: typescript/cli/src/commands/submit.ts:60-71
Timestamp: 2025-12-11T16:29:54.086Z
Learning: In the hyperlane-xyz/hyperlane-monorepo repository, the `writeYamlOrJson` utility function (from typescript/cli/src/utils/files.ts) automatically handles directory creation when writing files, so there's no need to explicitly check or create directories before calling it.
Applied to files:
typescript/rebalancer/src/config/RebalancerConfig.ts
📚 Learning: 2025-10-21T14:26:22.163Z
Learnt from: paulbalaji
Repo: hyperlane-xyz/hyperlane-monorepo PR: 7223
File: typescript/infra/src/config/chain.ts:56-58
Timestamp: 2025-10-21T14:26:22.163Z
Learning: When adding chains to chainsToSkip in typescript/infra/src/config/chain.ts, it's the expected pattern to keep the chain's configurations in other files like supportedChainNames.ts, agent.ts, validators.ts, and owners.ts. The chainsToSkip array is used selectively by specific scripts (e.g., check-owner-ica.ts) to exclude chains from certain operations, not as a signal to remove all configurations.
Applied to files:
typescript/rebalancer/src/config/RebalancerConfig.test.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/rebalancer/package.json
📚 Learning: 2025-11-26T13:28:51.658Z
Learnt from: paulbalaji
Repo: hyperlane-xyz/hyperlane-monorepo PR: 7410
File: typescript/cli/package.json:20-20
Timestamp: 2025-11-26T13:28:51.658Z
Learning: In the hyperlane-xyz/hyperlane-monorepo repository, hyperlane-xyz/registry is maintained in a separate repository and published to npm, so it should use a pinned version (e.g., "23.6.0") rather than the workspace protocol ("workspace:*") that other internal Hyperlane packages use.
Applied to files:
typescript/rebalancer/package.jsontypescript/rebalancer/src/factories/RebalancerContextFactory.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/cli/src/commands/warp.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/cli/src/commands/warp.ts
📚 Learning: 2025-06-16T16:22:12.017Z
Learnt from: xeno097
Repo: hyperlane-xyz/hyperlane-monorepo PR: 5883
File: typescript/cli/src/commands/fork.ts:12-39
Timestamp: 2025-06-16T16:22:12.017Z
Learning: In yargs CLI applications, hyphenated option names (kebab-case) like 'fork-config' are automatically converted to camelCase (forkConfig) when passed to handler functions. This means destructuring { forkConfig } from handler arguments when the option is defined as 'fork-config' is correct and will work as expected.
Applied to files:
typescript/cli/src/commands/warp.ts
📚 Learning: 2025-12-15T15:33:46.063Z
Learnt from: CR
Repo: hyperlane-xyz/hyperlane-monorepo PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T15:33:46.063Z
Learning: Applies to typescript/sdk/src/**/*.ts : TypeScript SDK should use MultiProvider for multi-chain provider management with protocol adapters
Applied to files:
typescript/rebalancer/src/factories/RebalancerContextFactory.tstypescript/rebalancer/src/service.ts
📚 Learning: 2025-12-15T15:33:46.063Z
Learnt from: CR
Repo: hyperlane-xyz/hyperlane-monorepo PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T15:33:46.063Z
Learning: Applies to typescript/sdk/src/**/*.ts : TypeScript SDK should use HyperlaneCore factory for core contract interactions and MultiProtocolCore for protocol-agnostic abstractions (EVM, Cosmos, Sealevel, Starknet)
Applied to files:
typescript/rebalancer/src/factories/RebalancerContextFactory.tstypescript/rebalancer/src/index.tstypescript/rebalancer/src/service.ts
📚 Learning: 2025-12-15T15:33:46.063Z
Learnt from: CR
Repo: hyperlane-xyz/hyperlane-monorepo PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T15:33:46.063Z
Learning: Applies to typescript/sdk/src/**/*.ts : TypeScript SDK should use ChainMap<T> for type-safe per-chain configuration mapping
Applied to files:
typescript/rebalancer/src/factories/RebalancerContextFactory.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/rebalancer/README.md
📚 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/rebalancer/README.md
🧬 Code graph analysis (2)
typescript/rebalancer/src/config/RebalancerConfig.test.ts (1)
typescript/rebalancer/src/config/RebalancerConfig.ts (1)
RebalancerConfig(11-38)
typescript/rebalancer/src/service.ts (2)
typescript/rebalancer/src/config/RebalancerConfig.ts (1)
RebalancerConfig(11-38)typescript/rebalancer/src/core/RebalancerService.ts (1)
RebalancerService(95-333)
🪛 markdownlint-cli2 (0.18.1)
typescript/rebalancer/README.md
115-115: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ 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). (62)
- GitHub Check: cli-evm-e2e-matrix (warp-check-4)
- GitHub Check: cli-evm-e2e-matrix (warp-check-3)
- GitHub Check: cli-evm-e2e-matrix (warp-bridge-2)
- GitHub Check: cli-evm-e2e-matrix (warp-read)
- GitHub Check: cli-evm-e2e-matrix (warp-rebalancer)
- GitHub Check: cli-evm-e2e-matrix (core-check)
- GitHub Check: cli-evm-e2e-matrix (warp-send)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-hook-updates)
- GitHub Check: cli-evm-e2e-matrix (warp-init)
- GitHub Check: cli-evm-e2e-matrix (warp-extend-config)
- GitHub Check: cli-evm-e2e-matrix (warp-check-2)
- GitHub Check: cli-evm-e2e-matrix (core-init)
- GitHub Check: cli-evm-e2e-matrix (core-read)
- GitHub Check: cli-evm-e2e-matrix (core-apply)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-ownership-updates)
- GitHub Check: cli-evm-e2e-matrix (warp-check-5)
- GitHub Check: cli-evm-e2e-matrix (warp-deploy-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-deploy-2)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-submitters)
- GitHub Check: cli-evm-e2e-matrix (warp-bridge-1)
- GitHub Check: cli-evm-e2e-matrix (warp-extend-recovery)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-rebalancing-config)
- GitHub Check: cli-evm-e2e-matrix (relay)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-simple-updates)
- GitHub Check: cli-evm-e2e-matrix (core-deploy)
- GitHub Check: cli-evm-e2e-matrix (warp-extend-basic)
- GitHub Check: cli-cosmos-e2e-matrix (warp-read)
- GitHub Check: cli-radix-e2e-matrix (warp-deploy)
- GitHub Check: env-test-matrix (testnet4, sepolia, core)
- GitHub Check: env-test-matrix (mainnet3, optimism, igp)
- GitHub Check: cli-cosmos-e2e-matrix (warp-deploy)
- GitHub Check: cli-radix-e2e-matrix (warp-apply-route-extension)
- GitHub Check: env-test-matrix (mainnet3, arbitrum, igp)
- GitHub Check: env-test-matrix (mainnet3, arbitrum, core)
- GitHub Check: cli-cosmos-e2e-matrix (core-check)
- GitHub Check: env-test-matrix (mainnet3, ethereum, igp)
- GitHub Check: cli-radix-e2e-matrix (core-deploy)
- GitHub Check: env-test-matrix (mainnet3, optimism, core)
- GitHub Check: cli-cosmos-e2e-matrix (core-apply)
- GitHub Check: cli-cosmos-e2e-matrix (core-deploy)
- GitHub Check: cli-radix-e2e-matrix (core-apply)
- GitHub Check: cli-cosmos-e2e-matrix (core-read)
- GitHub Check: cli-radix-e2e-matrix (warp-apply-ownership-updates)
- GitHub Check: cli-cross-chain-e2e-matrix (warp-deploy)
- GitHub Check: env-test-matrix (mainnet3, ethereum, core)
- GitHub Check: cosmos-sdk-e2e-run
- GitHub Check: aleo-sdk-e2e-run
- GitHub Check: cli-cross-chain-e2e-matrix (warp-apply)
- GitHub Check: pnpm-test-run
- GitHub Check: cli-install-test-run
- GitHub Check: infra-test
- GitHub Check: build-and-push-to-gcr
- GitHub Check: Socket Security: Pull Request Alerts
- GitHub Check: e2e-matrix (evm)
- GitHub Check: agent-configs (testnet4)
- GitHub Check: agent-configs (mainnet3)
- GitHub Check: lint-prettier
- GitHub Check: test-rs
- GitHub Check: lint-rs
- GitHub Check: lander-coverage
🔇 Additional comments (23)
.changeset/separate-rebalancer-package.md (1)
1-10: Looks good—changeset is clear and captures the essence of the package split.The version bumps make sense: minor for both packages fits the bill here. The new rebalancer package is a fresh feature, and the CLI updates maintain backward compatibility while gaining the daemon mode capability. The summary accurately reflects the PR objectives without any fluff. Shrek'd say this is as straightforward as a good swamp—layered, purposeful, and well-documented.
typescript/rebalancer/src/utils/errors.ts (1)
1-5: Good implementation of Error cause pattern.This follows the standard Error cause API nicely. The conditional options object ensures we don't pass undefined cause values.
typescript/rebalancer/src/interfaces/IMonitor.ts (1)
3-3: Import path correctly updated for new package structure.The path adjustment reflects the rebalancer logic moving into its own package. All good here.
typescript/rebalancer/src/factories/RebalancerContextFactory.ts (5)
3-10: Clean refactor removing CLI dependency.Replacing
WriteCommandContextwith directMultiProviderandIRegistryparameters makes this factory properly reusable. The constructor is well-documented with clear parameter descriptions.Also applies to: 24-40
49-99: Solid factory method with sensible defaults.The fallback to create
MultiProtocolProviderfromMultiProviderwhen not supplied (lines 69-72) is a thoughtful touch. Keeps the API flexible while ensuring all needs are met.
109-130: Registry and provider access updated correctly.The switch from
this.context.chainMetadatatothis.multiProvider.metadata(line 115) andthis.context.registrytothis.registry(line 119) properly reflects the new architecture.
160-188: Rebalancer properly wrapped for concurrency control.The
WithSemaphorewrapper at lines 181-186 provides thread-safe access to the rebalancer. The comment clarifies the intent, and the usage ofmultiProviderthroughout is consistent.
190-209: Initial collateral calculation looks solid.The method correctly sums bridged supply for eligible collateralized tokens. The filtering logic and async handling are appropriate.
typescript/rebalancer/src/config/RebalancerConfig.ts (1)
9-9: Import correctly updated to use published utils package.Switching from a relative import to
@hyperlane-xyz/utils/fsaligns with the monorepo structure and makes the dependency explicit.typescript/rebalancer/src/config/RebalancerConfig.test.ts (2)
4-5: Better test isolation with tmpdir().Using
tmpdir()for the test config path (line 16) keeps test artifacts out of the source tree. Nice cleanup. The import updates align with the package restructuring.Also applies to: 12-12, 16-16
47-387: Consistent test config path usage throughout.All test cases properly updated to use
TEST_CONFIG_PATH. The coverage remains intact, just with better test hygiene.typescript/cli/src/commands/warp.ts (3)
5-5: Clean integration with the new rebalancer package.Importing
RebalancerConfigandRebalancerServicefrom@hyperlane-xyz/rebalancerproperly separates concerns. TherootLoggerimport ensures consistent logging.Also applies to: 19-19
493-530: Well-structured handler with clear mode determination.The handler cleanly loads configuration (line 510), determines execution mode (line 513), and creates the service with all necessary context. Using a child logger with module scope (line 527) is a nice touch for log organization.
531-555: Execution logic handles both modes correctly.Manual mode properly validates required parameters (lines 533-538) before executing, and daemon mode cleanly starts the service (line 549). Error handling is appropriate with clear user feedback.
typescript/rebalancer/src/service.ts (3)
115-119: Signer setup looks proper.Creating the
MultiProviderwith chain metadata and setting a shared signer is the right approach here. Clean and straightforward - no onion-like complexity needed.
150-154: Top-level error handling is appropriate.The catch-all at the entry point ensures any uncaught errors are logged and result in a non-zero exit. This is exactly what you want for a daemon running in Kubernetes - get out of my swamp gracefully when things go wrong.
96-107: Empty registryUris array uses defaults intentionally—consider making it configurable via environment variable.The empty
registryUrisarray here is by design: when passed empty, the registry falls back to its defaults. This works fine as-is, but if you'd like operators to customize registry URIs without code changes, you might add an optionalREGISTRY_URISenvironment variable to make it configurable in the daemon setup.typescript/rebalancer/src/core/RebalancerService.ts (5)
25-46: Well-structured configuration interface.The
RebalancerServiceConfiginterface is clean with good JSDoc comments. The optional fields have sensible defaults implied by their usage. This is the kind of thing that makes other developers' lives easier - like a warm bowl of rat stew on a cold night.
119-163: Initialization is well-guarded with idempotency check.The early return when
contextFactoryalready exists prevents double initialization. The conditional component creation based on mode and config flags is clean and logical.
168-216: Manual rebalance execution looks solid.Good validation of the request parameters and proper error handling. The amount conversion using
toWeiwith the token's decimals is correct. One small thing - theNumber(amount)check on line 194 could fail for very large amounts that exceed JavaScript's safe integer range, but since you're ultimately usingBigInt, this validation is just for sanity checking positivity.
312-325: Error handling distinguishes error types appropriately.Good pattern here -
MonitorPollingErroris logged and metrics recorded (non-fatal), whileMonitorStartErroris re-thrown to bubble up (fatal). The catch-all for unexpected errors is also sensible. Layered error handling, if you will.
293-307: Fire-and-forget call is safe thanks to WithSemaphore wrapper.The fire-and-forget
rebalance()call looks bare, but the Rebalancer instance is always wrapped withWithSemaphore(see RebalancerContextFactory line 181-187). WithSemaphore has anexecutingflag that skips concurrent calls—so if another monitor event fires while a rebalance is in flight, it gets dropped gracefully. The guards are there, the pattern's intentional, and it's all covered with tests to boot.typescript/rebalancer/src/index.ts (1)
1-68: Clean and well-organized barrel file.This index file does exactly what it should - provides a clear, organized public API for the package. The section comments make it easy to find what you're looking for, and separating
export typefrom value exports is good TypeScript practice for consumers usingisolatedModules. All the.jsextensions are in place for ESM compatibility.Like a well-organized swamp - everything in its rightful place.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
typescript/rebalancer/package.json (1)
39-39: Consider movingpino-prettyto devDependencies.
pino-prettyis typically a development-only formatter for prettifying log output. Since yourstartscript uses the compiled Node binary whilestart:devuses tsx, you might only need pino-pretty when developing locally. If that's the case, moving it to devDependencies would keep your production bundle leaner.If pino-pretty is needed at runtime (e.g., for daemon mode in non-containerized environments), keep it in dependencies. But if it's only for the
start:devdevelopment workflow, this minor cleanup could help.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📥 Commits
Reviewing files that changed from the base of the PR and between 9f87fea57ddf781d6523c81aa5377b890f7d3091 and 47b88f4022b34351eed6bdb3c54db7fd76d4ba8a.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (1)
typescript/rebalancer/package.json(1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
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.
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.
📚 Learning: 2025-12-15T15:33:46.063Z
Learnt from: CR
Repo: hyperlane-xyz/hyperlane-monorepo PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T15:33:46.063Z
Learning: Applies to typescript/sdk/src/**/*.{ts,tsx} : Build TypeScript SDK with: pnpm -C typescript/sdk build
Applied to files:
typescript/rebalancer/package.json
📚 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/rebalancer/package.json
📚 Learning: 2025-11-26T13:28:51.658Z
Learnt from: paulbalaji
Repo: hyperlane-xyz/hyperlane-monorepo PR: 7410
File: typescript/cli/package.json:20-20
Timestamp: 2025-11-26T13:28:51.658Z
Learning: In the hyperlane-xyz/hyperlane-monorepo repository, hyperlane-xyz/registry is maintained in a separate repository and published to npm, so it should use a pinned version (e.g., "23.6.0") rather than the workspace protocol ("workspace:*") that other internal Hyperlane packages use.
Applied to files:
typescript/rebalancer/package.json
📚 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/rebalancer/package.json
⏰ 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-deploy-1)
- GitHub Check: cli-evm-e2e-matrix (warp-check-3)
- GitHub Check: cli-evm-e2e-matrix (warp-rebalancer)
- GitHub Check: cli-evm-e2e-matrix (warp-read)
- GitHub Check: cli-evm-e2e-matrix (warp-init)
- GitHub Check: cli-evm-e2e-matrix (warp-send)
- GitHub Check: cli-evm-e2e-matrix (warp-check-4)
- GitHub Check: cli-evm-e2e-matrix (warp-extend-config)
- GitHub Check: cli-evm-e2e-matrix (warp-check-1)
- GitHub Check: cli-evm-e2e-matrix (warp-bridge-2)
- GitHub Check: cli-evm-e2e-matrix (warp-extend-recovery)
- GitHub Check: cli-evm-e2e-matrix (warp-bridge-1)
- GitHub Check: cli-evm-e2e-matrix (core-check)
- GitHub Check: cli-evm-e2e-matrix (warp-check-5)
- GitHub Check: cli-evm-e2e-matrix (warp-deploy-2)
- GitHub Check: cli-evm-e2e-matrix (warp-check-2)
- 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-apply-ownership-updates)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-hook-updates)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-rebalancing-config)
- GitHub Check: cli-evm-e2e-matrix (core-deploy)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-submitters)
- GitHub Check: cli-evm-e2e-matrix (warp-extend-basic)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-ism-updates)
- GitHub Check: cli-evm-e2e-matrix (relay)
- GitHub Check: cli-evm-e2e-matrix (core-apply)
- GitHub Check: cli-evm-e2e-matrix (core-read)
- GitHub Check: cli-cosmos-e2e-matrix (core-check)
- GitHub Check: env-test-matrix (mainnet3, ethereum, igp)
- GitHub Check: env-test-matrix (mainnet3, ethereum, core)
- GitHub Check: env-test-matrix (mainnet3, optimism, core)
- GitHub Check: env-test-matrix (mainnet3, optimism, igp)
- GitHub Check: cli-cosmos-e2e-matrix (core-deploy)
- GitHub Check: env-test-matrix (mainnet3, arbitrum, igp)
- GitHub Check: cli-radix-e2e-matrix (warp-apply-ownership-updates)
- GitHub Check: env-test-matrix (testnet4, sepolia, core)
- GitHub Check: env-test-matrix (mainnet3, arbitrum, core)
- GitHub Check: cli-radix-e2e-matrix (core-deploy)
- GitHub Check: cli-radix-e2e-matrix (warp-apply-route-extension)
- GitHub Check: cli-cosmos-e2e-matrix (warp-deploy)
- GitHub Check: cli-cosmos-e2e-matrix (core-apply)
- GitHub Check: cli-cosmos-e2e-matrix (core-read)
- GitHub Check: cli-cosmos-e2e-matrix (warp-read)
- GitHub Check: cli-radix-e2e-matrix (warp-deploy)
- GitHub Check: cli-radix-e2e-matrix (core-apply)
- GitHub Check: cli-cross-chain-e2e-matrix (warp-deploy)
- GitHub Check: cosmos-sdk-e2e-run
- GitHub Check: aleo-sdk-e2e-run
- GitHub Check: cli-cross-chain-e2e-matrix (warp-apply)
- GitHub Check: pnpm-test-run
- GitHub Check: cli-install-test-run
- GitHub Check: infra-test
- GitHub Check: build-and-push-to-gcr
- GitHub Check: agent-configs (testnet4)
- GitHub Check: lint-prettier
- GitHub Check: e2e-matrix (evm)
- GitHub Check: agent-configs (mainnet3)
- GitHub Check: lint-rs
- GitHub Check: lander-coverage
- GitHub Check: test-rs
🔇 Additional comments (4)
typescript/rebalancer/package.json (4)
26-44: Solid dependencies section structure.Good separation of runtime dependencies from dev tools. Internal packages correctly use
workspace:*and external dependency@hyperlane-xyz/registryis properly pinned to23.7.0per the learnings. The mix of pinned and caret versions is intentional and reasonable for this context.
6-10: Entry points are well-configured.You've got both the library exports and the CLI executable wired up properly. Consumers can import the service as a library from
@hyperlane-xyz/rebalancer, and folks installing globally can runhyperlane-rebalancerfrom the command line.
15-25: Scripts are well-organized.Standard TypeScript package scripts here—nothing fancy, just what you need. The
startandstart:devsplit handles both production and development contexts properly.
2-5: Package metadata looks good.Name, description, and engine requirements are all in order. The
0.0.0version will get bumped when you're ready to publish, which is the standard pattern.Also applies to: 62-75
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
typescript/rebalancer/README.md (1)
133-154: Add language specifier to fenced code block.The directory structure block should have a language specified. You can use
textor leave it as a plain code block.Apply this diff:
-``` +```text typescript/rebalancer/ ├── src/
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📥 Commits
Reviewing files that changed from the base of the PR and between 47b88f4022b34351eed6bdb3c54db7fd76d4ba8a and bd9edbf92670e276f355c948473be25dce14abfc.
📒 Files selected for processing (2)
typescript/rebalancer/README.md(1 hunks)typescript/rebalancer/src/service.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
typescript/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
typescript/**/*.{ts,tsx,js,jsx}: Format TypeScript/JavaScript code using Prettier with: pnpm prettier
Lint TypeScript/JavaScript code with: pnpm lint
Files:
typescript/rebalancer/src/service.ts
🧠 Learnings (2)
📚 Learning: 2025-12-15T15:33:46.063Z
Learnt from: CR
Repo: hyperlane-xyz/hyperlane-monorepo PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T15:33:46.063Z
Learning: Applies to typescript/sdk/src/**/*.ts : TypeScript SDK should use HyperlaneCore factory for core contract interactions and MultiProtocolCore for protocol-agnostic abstractions (EVM, Cosmos, Sealevel, Starknet)
Applied to files:
typescript/rebalancer/src/service.ts
📚 Learning: 2025-11-26T13:28:51.658Z
Learnt from: paulbalaji
Repo: hyperlane-xyz/hyperlane-monorepo PR: 7410
File: typescript/cli/package.json:20-20
Timestamp: 2025-11-26T13:28:51.658Z
Learning: In the hyperlane-xyz/hyperlane-monorepo repository, hyperlane-xyz/registry is maintained in a separate repository and published to npm, so it should use a pinned version (e.g., "23.6.0") rather than the workspace protocol ("workspace:*") that other internal Hyperlane packages use.
Applied to files:
typescript/rebalancer/src/service.ts
🧬 Code graph analysis (1)
typescript/rebalancer/src/service.ts (2)
typescript/rebalancer/src/config/RebalancerConfig.ts (1)
RebalancerConfig(11-38)typescript/rebalancer/src/core/RebalancerService.ts (1)
RebalancerService(95-333)
🪛 markdownlint-cli2 (0.18.1)
typescript/rebalancer/README.md
133-133: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ 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-extend-basic)
- GitHub Check: cli-evm-e2e-matrix (warp-check-1)
- GitHub Check: cli-evm-e2e-matrix (warp-check-3)
- GitHub Check: cli-evm-e2e-matrix (warp-read)
- GitHub Check: cli-evm-e2e-matrix (warp-extend-config)
- GitHub Check: cli-evm-e2e-matrix (warp-init)
- GitHub Check: cli-evm-e2e-matrix (warp-send)
- GitHub Check: cli-evm-e2e-matrix (warp-check-5)
- GitHub Check: cli-evm-e2e-matrix (warp-check-4)
- GitHub Check: cli-evm-e2e-matrix (warp-bridge-2)
- GitHub Check: cli-evm-e2e-matrix (warp-deploy-1)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-ism-updates)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-rebalancing-config)
- GitHub Check: cli-evm-e2e-matrix (warp-deploy-2)
- GitHub Check: cli-evm-e2e-matrix (warp-extend-recovery)
- GitHub Check: cli-evm-e2e-matrix (warp-rebalancer)
- GitHub Check: cli-evm-e2e-matrix (warp-bridge-1)
- GitHub Check: cli-evm-e2e-matrix (core-apply)
- GitHub Check: cli-evm-e2e-matrix (warp-check-2)
- GitHub Check: cli-evm-e2e-matrix (core-deploy)
- GitHub Check: cli-evm-e2e-matrix (relay)
- GitHub Check: cli-evm-e2e-matrix (core-init)
- 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-apply-submitters)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-simple-updates)
- GitHub Check: cli-evm-e2e-matrix (core-read)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-ownership-updates)
- GitHub Check: env-test-matrix (testnet4, sepolia, core)
- GitHub Check: env-test-matrix (mainnet3, optimism, core)
- GitHub Check: env-test-matrix (mainnet3, ethereum, core)
- GitHub Check: env-test-matrix (mainnet3, optimism, igp)
- GitHub Check: env-test-matrix (mainnet3, ethereum, igp)
- GitHub Check: cli-radix-e2e-matrix (warp-apply-ownership-updates)
- GitHub Check: env-test-matrix (mainnet3, arbitrum, core)
- GitHub Check: env-test-matrix (mainnet3, arbitrum, igp)
- GitHub Check: cli-radix-e2e-matrix (warp-deploy)
- GitHub Check: cli-radix-e2e-matrix (core-apply)
- GitHub Check: cli-radix-e2e-matrix (warp-apply-route-extension)
- GitHub Check: cli-radix-e2e-matrix (core-deploy)
- GitHub Check: cli-cross-chain-e2e-matrix (warp-apply)
- GitHub Check: cli-cosmos-e2e-matrix (core-deploy)
- GitHub Check: cli-cosmos-e2e-matrix (warp-deploy)
- GitHub Check: pnpm-test-run
- GitHub Check: cli-cosmos-e2e-matrix (core-read)
- GitHub Check: cli-cosmos-e2e-matrix (core-check)
- GitHub Check: cli-cosmos-e2e-matrix (warp-read)
- GitHub Check: cli-cosmos-e2e-matrix (core-apply)
- GitHub Check: cli-cross-chain-e2e-matrix (warp-deploy)
- GitHub Check: aleo-sdk-e2e-run
- GitHub Check: cosmos-sdk-e2e-run
- GitHub Check: cli-install-test-run
- GitHub Check: infra-test
- GitHub Check: build-and-push-to-gcr
- GitHub Check: agent-configs (mainnet3)
- GitHub Check: e2e-matrix (evm)
- GitHub Check: lint-prettier
- GitHub Check: agent-configs (testnet4)
- GitHub Check: test-rs
- GitHub Check: lander-coverage
- GitHub Check: lint-rs
🔇 Additional comments (4)
typescript/rebalancer/src/service.ts (4)
36-47: Looks good, mate.The version retrieval with fallback handling is solid. Keeping it separate from
mainmakes sense for clarity.
49-82: Environment validation is spot on.Required variables are checked properly, and
CHECK_FREQUENCYvalidation ensures you don't get stuck with negative or zero milliseconds. All good here.
108-116: GithubRegistry setup addresses previous feedback.The registry initialization with optional
uriandbranchgives you the flexibility to deploy against specific registry commits or branches, which was mentioned in earlier discussions. Well done.
134-156: Service initialization and startup look solid.All the pieces are wired together properly - providers, registry, config, and runtime options all flow through cleanly. Error handling wraps it up nicely.
paulbalaji
force-pushed
the
pbio/rebalancer-package-move
branch
from
1f14ffa to
47b88f4
Compare
paulbalaji
force-pushed
the
pbio/rebalancer-package-move
branch
from
47b88f4 to
749c8c2
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
typescript/rebalancer/src/service.ts (1)
101-106: Read REGISTRY_URI and REGISTRY_COMMIT from environment variables.The code hardcodes an empty
registryUrisarray and doesn't readREGISTRY_URIorREGISTRY_COMMITfrom environment variables, despite the PR objectives mentioning this support and the Helm template settingREGISTRY_COMMIT. Similar services like ccip-server properly readREGISTRY_URIfromprocess.env. Update the code to read these environment variables and pass them togetRegistry():const registryUris = process.env.REGISTRY_URI ? [process.env.REGISTRY_URI] : []; const registry = getRegistry({ registryUris, enableProxy: false, logger: rootLogger, });Also ensure
REGISTRY_COMMITis handled appropriately if the registry client supports it.
🧹 Nitpick comments (1)
typescript/rebalancer/README.md (1)
115-136: Add language identifier to the fenced code block.The directory structure block at line 115 is missing a language identifier. Add
textor leave it empty to silence the linter.Apply this diff:
-``` +```text typescript/rebalancer/ ├── src/
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📥 Commits
Reviewing files that changed from the base of the PR and between bd9edbf92670e276f355c948473be25dce14abfc and 749c8c214dca9d68d9a354db010f52d75f2db3a5.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (22)
.changeset/separate-rebalancer-package.md(1 hunks)Dockerfile(1 hunks)typescript/cli/package.json(1 hunks)typescript/cli/src/commands/warp.ts(3 hunks)typescript/cli/src/context/strategies/chain/chainResolver.ts(1 hunks)typescript/cli/src/rebalancer/README.md(0 hunks)typescript/cli/src/rebalancer/index.ts(0 hunks)typescript/cli/src/rebalancer/runner.ts(0 hunks)typescript/rebalancer/.gitignore(1 hunks)typescript/rebalancer/.mocharc.json(1 hunks)typescript/rebalancer/README.md(1 hunks)typescript/rebalancer/eslint.config.mjs(1 hunks)typescript/rebalancer/package.json(1 hunks)typescript/rebalancer/src/config/RebalancerConfig.test.ts(10 hunks)typescript/rebalancer/src/config/RebalancerConfig.ts(1 hunks)typescript/rebalancer/src/core/RebalancerService.ts(1 hunks)typescript/rebalancer/src/factories/RebalancerContextFactory.ts(6 hunks)typescript/rebalancer/src/index.ts(1 hunks)typescript/rebalancer/src/interfaces/IMonitor.ts(1 hunks)typescript/rebalancer/src/service.ts(1 hunks)typescript/rebalancer/src/utils/errors.ts(1 hunks)typescript/rebalancer/tsconfig.json(1 hunks)
💤 Files with no reviewable changes (3)
- typescript/cli/src/rebalancer/README.md
- typescript/cli/src/rebalancer/runner.ts
- typescript/cli/src/rebalancer/index.ts
🚧 Files skipped from review as they are similar to previous changes (9)
- typescript/cli/src/context/strategies/chain/chainResolver.ts
- typescript/rebalancer/src/config/RebalancerConfig.ts
- .changeset/separate-rebalancer-package.md
- typescript/rebalancer/eslint.config.mjs
- typescript/cli/package.json
- typescript/rebalancer/src/index.ts
- typescript/rebalancer/.gitignore
- Dockerfile
- typescript/rebalancer/src/interfaces/IMonitor.ts
🧰 Additional context used
📓 Path-based instructions (1)
typescript/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
typescript/**/*.{ts,tsx,js,jsx}: Format TypeScript/JavaScript code using Prettier with: pnpm prettier
Lint TypeScript/JavaScript code with: pnpm lint
Files:
typescript/rebalancer/src/config/RebalancerConfig.test.tstypescript/cli/src/commands/warp.tstypescript/rebalancer/src/utils/errors.tstypescript/rebalancer/src/service.tstypescript/rebalancer/src/factories/RebalancerContextFactory.tstypescript/rebalancer/src/core/RebalancerService.ts
🧠 Learnings (14)
📚 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/rebalancer/src/config/RebalancerConfig.test.tstypescript/rebalancer/tsconfig.jsontypescript/rebalancer/package.jsontypescript/rebalancer/.mocharc.json
📚 Learning: 2025-08-26T13:45:44.656Z
Learnt from: paulbalaji
Repo: hyperlane-xyz/hyperlane-monorepo PR: 6943
File: rust/main/config/testnet_config.json:3380-3439
Timestamp: 2025-08-26T13:45:44.656Z
Learning: Skip reviewing mainnet_config.json and testnet_config.json files in rust/main/config/ directory as requested by paulbalaji.
Applied to files:
typescript/rebalancer/src/config/RebalancerConfig.test.ts
📚 Learning: 2025-10-21T14:26:22.163Z
Learnt from: paulbalaji
Repo: hyperlane-xyz/hyperlane-monorepo PR: 7223
File: typescript/infra/src/config/chain.ts:56-58
Timestamp: 2025-10-21T14:26:22.163Z
Learning: When adding chains to chainsToSkip in typescript/infra/src/config/chain.ts, it's the expected pattern to keep the chain's configurations in other files like supportedChainNames.ts, agent.ts, validators.ts, and owners.ts. The chainsToSkip array is used selectively by specific scripts (e.g., check-owner-ica.ts) to exclude chains from certain operations, not as a signal to remove all configurations.
Applied to files:
typescript/rebalancer/src/config/RebalancerConfig.test.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/cli/src/commands/warp.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/cli/src/commands/warp.ts
📚 Learning: 2025-06-16T16:22:12.017Z
Learnt from: xeno097
Repo: hyperlane-xyz/hyperlane-monorepo PR: 5883
File: typescript/cli/src/commands/fork.ts:12-39
Timestamp: 2025-06-16T16:22:12.017Z
Learning: In yargs CLI applications, hyphenated option names (kebab-case) like 'fork-config' are automatically converted to camelCase (forkConfig) when passed to handler functions. This means destructuring { forkConfig } from handler arguments when the option is defined as 'fork-config' is correct and will work as expected.
Applied to files:
typescript/cli/src/commands/warp.ts
📚 Learning: 2025-12-15T15:33:46.063Z
Learnt from: CR
Repo: hyperlane-xyz/hyperlane-monorepo PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T15:33:46.063Z
Learning: Applies to typescript/sdk/src/**/*.ts : TypeScript SDK should use HyperlaneCore factory for core contract interactions and MultiProtocolCore for protocol-agnostic abstractions (EVM, Cosmos, Sealevel, Starknet)
Applied to files:
typescript/rebalancer/src/service.tstypescript/rebalancer/tsconfig.jsontypescript/rebalancer/src/factories/RebalancerContextFactory.ts
📚 Learning: 2025-12-15T15:33:46.063Z
Learnt from: CR
Repo: hyperlane-xyz/hyperlane-monorepo PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T15:33:46.063Z
Learning: Applies to typescript/sdk/src/**/*.ts : TypeScript SDK should use MultiProvider for multi-chain provider management with protocol adapters
Applied to files:
typescript/rebalancer/src/service.tstypescript/rebalancer/src/factories/RebalancerContextFactory.ts
📚 Learning: 2025-11-26T13:28:51.658Z
Learnt from: paulbalaji
Repo: hyperlane-xyz/hyperlane-monorepo PR: 7410
File: typescript/cli/package.json:20-20
Timestamp: 2025-11-26T13:28:51.658Z
Learning: In the hyperlane-xyz/hyperlane-monorepo repository, hyperlane-xyz/registry is maintained in a separate repository and published to npm, so it should use a pinned version (e.g., "23.6.0") rather than the workspace protocol ("workspace:*") that other internal Hyperlane packages use.
Applied to files:
typescript/rebalancer/src/service.tstypescript/rebalancer/src/factories/RebalancerContextFactory.tstypescript/rebalancer/package.json
📚 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/rebalancer/tsconfig.jsontypescript/rebalancer/.mocharc.json
📚 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/rebalancer/tsconfig.json
📚 Learning: 2025-12-15T15:33:46.063Z
Learnt from: CR
Repo: hyperlane-xyz/hyperlane-monorepo PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T15:33:46.063Z
Learning: Applies to typescript/sdk/src/**/*.{ts,tsx} : Build TypeScript SDK with: pnpm -C typescript/sdk build
Applied to files:
typescript/rebalancer/tsconfig.json
📚 Learning: 2025-12-15T15:33:46.063Z
Learnt from: CR
Repo: hyperlane-xyz/hyperlane-monorepo PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T15:33:46.063Z
Learning: Applies to typescript/sdk/src/**/*.ts : TypeScript SDK should use ChainMap<T> for type-safe per-chain configuration mapping
Applied to files:
typescript/rebalancer/src/factories/RebalancerContextFactory.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/rebalancer/package.json
🧬 Code graph analysis (4)
typescript/rebalancer/src/config/RebalancerConfig.test.ts (1)
typescript/rebalancer/src/config/RebalancerConfig.ts (1)
RebalancerConfig(11-38)
typescript/rebalancer/src/service.ts (3)
typescript/rebalancer/src/config/RebalancerConfig.ts (1)
RebalancerConfig(11-38)typescript/rebalancer/src/index.ts (2)
RebalancerConfig(23-23)RebalancerService(11-11)typescript/rebalancer/src/core/RebalancerService.ts (1)
RebalancerService(95-333)
typescript/rebalancer/src/factories/RebalancerContextFactory.ts (1)
typescript/rebalancer/src/config/RebalancerConfig.ts (1)
RebalancerConfig(11-38)
typescript/rebalancer/src/core/RebalancerService.ts (8)
typescript/rebalancer/src/factories/RebalancerContextFactory.ts (1)
RebalancerContextFactory(24-210)typescript/rebalancer/src/monitor/Monitor.ts (1)
Monitor(18-174)typescript/rebalancer/src/interfaces/IStrategy.ts (1)
IStrategy(11-13)typescript/rebalancer/src/interfaces/IRebalancer.ts (1)
IRebalancer(18-20)typescript/rebalancer/src/metrics/Metrics.ts (1)
Metrics(44-558)typescript/rebalancer/src/config/RebalancerConfig.ts (1)
RebalancerConfig(11-38)typescript/utils/src/index.ts (1)
assert(212-212)typescript/rebalancer/src/utils/balanceUtils.ts (1)
getRawBalances(16-65)
🪛 markdownlint-cli2 (0.18.1)
typescript/rebalancer/README.md
115-115: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ 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). (7)
- GitHub Check: env-test
- GitHub Check: pnpm-test
- GitHub Check: cli-evm-e2e-matrix (warp-apply-submitters)
- GitHub Check: e2e-matrix (evm)
- GitHub Check: lander-coverage
- GitHub Check: test-rs
- GitHub Check: lint-rs
🔇 Additional comments (14)
typescript/rebalancer/tsconfig.json (1)
1-8: Configuration looks solid.This setup is clean and proper for the new package. The base tsconfig provides all the strict type checking, module resolution, and compiler settings you need—declaration maps, source maps, the whole layering cake. The rebalancer config extends it nicely, sets the output and source paths, and includes everything under
./src.No need to mess with project references—the monorepo doesn't use 'em, and pnpm handles the workspace dependencies just fine without 'em.
typescript/rebalancer/.mocharc.json (1)
1-3: Configuration's solid, but tsx could use a bump.The Mocha setup here does what it needs to - pulls in tsx so your TypeScript tests can run without a fuss. Clean and simple, which is exactly right for a new package.
Now, about that tsx version - 4.19.1's a bit behind. The latest is 4.21.0, so you might want to give that a nudge in your package.json. It's only a couple minor versions back, so nothing alarming, but fresher is generally better for staying ahead of any fixes or improvements.
⛔ Skipped due to learnings
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.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.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.typescript/rebalancer/src/utils/errors.ts (1)
1-5: LGTM! Clean error wrapper.The conditional
causehandling is proper for the Error constructor. This gives ya a nice way to chain errors throughout the rebalancer without much fuss.typescript/rebalancer/src/config/RebalancerConfig.test.ts (1)
4-16: Good move using temp directory for tests.Switching to
tmpdir()keeps test artifacts out of the repo and avoids conflicts. The import consolidation withwriteYamlOrJsonis clean.typescript/rebalancer/package.json (1)
26-44: Dependencies properly organized.The runtime dependencies are now in the right section. Consumers installing this package will get what they need.
typescript/cli/src/commands/warp.ts (2)
532-538: Manual mode validation looks solid.The guard checking for required params (origin, destination, amount) before executing is proper. Error message is clear and the early exit prevents further issues.
540-544: Clean manual execution flow.The service call is straightforward and logging is appropriate. Good to see success confirmation for manual operations.
typescript/rebalancer/src/service.ts (1)
63-73: CHECK_FREQUENCY validation is thorough.The parseInt with NaN and positive number checks prevents bad config from sneaking through. Good defensive programming.
typescript/rebalancer/src/core/RebalancerService.ts (4)
119-163: Initialization guards against double-init.The early return if
contextFactoryexists prevents duplicate initialization. Component creation order makes sense: factory, monitor (daemon only), metrics (optional), strategy, rebalancer (unless monitorOnly).
193-196: Amount validation is solid.Converting to Number, checking NaN, and ensuring positive values covers the common input issues. Good defensive checks before the bigint conversion.
260-275: Graceful shutdown prevents double-exit.The
isExitingflag prevents shutdown re-entry, and cleaning up listeners avoids surprises. Proper cleanup pattern for a daemon service.
297-306: Rebalancer fire-and-forget pattern with error handling.The
.then()/.catch()on the rebalancer call with metrics recording is appropriate for async monitoring. Errors won't crash the monitor loop.typescript/rebalancer/src/factories/RebalancerContextFactory.ts (2)
68-72: MultiProtocolProvider fallback is handy.Creating the
MultiProtocolProviderfromMultiProviderwhen not provided gives callers flexibility. The provider extension with mailboxes handles Sealevel requirements.
180-187: Semaphore wrapper added for concurrency.Wrapping the rebalancer with
WithSemaphoreadds proper concurrency control. This prevents overlapping rebalance operations from stepping on each other.
paulbalaji
force-pushed
the
pbio/rebalancer-package-move
branch
from
601d88e to
74ef96f
Compare
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Update cli and infra packages to use ^1.3.3 to match rebalancer package and satisfy syncpack version checks. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add @hyperlane-xyz/tsconfig, chai-as-promised, sinon and their type definitions to fix TypeScript compilation errors in tests. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Update typescript/rebalancer/package.json to use catalog: references for shared dependencies to align with monorepo conventions and pass lint:catalog checks. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
paulbalaji
force-pushed
the
pbio/rebalancer-package-move
branch
from
74ef96f to
59a5414
Compare
🐳 Monorepo Docker Image Built SuccessfullyImage Tags: |
3 participants
Summary
@hyperlane-xyz/rebalancerpackagehttps://www.notion.so/hyperlanexyz/Split-Rebalancer-logic-out-of-CLI-2bd6d35200d680ff8bd9f236833b5d59?source=copy_link
Test plan
Follow-Up (TBD)
#7545
🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Refactoring
✏️ Tip: You can customize this high-level summary in your review settings.