feat(keyfunder): add standalone keyfunder package#7720
Conversation
|
📝 WalkthroughWalkthroughAdds a new standalone KeyFunder TypeScript service: config schemas and loader, core funding orchestration with per-chain timeouts, IGP interactions, Prometheus metrics, tests, project tooling, and Docker/CI integration. Changes
Sequence Diagram(s)sequenceDiagram
participant Service as service.ts
participant Loader as KeyFunderConfigLoader
participant Provider as MultiProvider/Wallet
participant IGP as HyperlaneIgp
participant KeyFunder as KeyFunder
participant Metrics as KeyFunderMetrics
Service->>Loader: load(KEYFUNDER_CONFIG_FILE)
Loader-->>Service: validated config
Service->>Provider: init MultiProvider & signer
Service->>Service: apply RPC overrides, discover IGP addresses
Service->>IGP: init per-chain IGP (when present)
Service->>Metrics: init metrics
Service->>KeyFunder: new(KeyFunderOptions)
Service->>KeyFunder: fundAllChains()
KeyFunder->>KeyFunder: determine chains to process
par Per-chain funding (parallel)
KeyFunder->>IGP: claimFromIgp (if configured)
IGP-->>KeyFunder: claim result
KeyFunder->>KeyFunder: resolve keys & desired balances
KeyFunder->>Provider: send funding txs
KeyFunder->>KeyFunder: sweepExcessFunds (if configured)
KeyFunder->>Metrics: record balances, amounts, durations
end
KeyFunder-->>Service: aggregated results / errors
Service->>Metrics: push()
Metrics-->>Service: push result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @typescript/keyfunder/package.json:
- Line 22: Update the dependency entry for "@hyperlane-xyz/registry" in
package.json to pin it to a specific released version instead of using the
workspace/catalog protocol; replace the value "catalog:" with the intended
semver string (e.g., "23.6.0") so npm installs the published registry package
rather than attempting to resolve it as a workspace package.
🧹 Nitpick comments (6)
typescript/keyfunder/src/core/KeyFunder.ts (1)
291-297: Potential precision loss in multiplier calculations.The math here:
const targetBalance = threshold .mul(Math.floor(targetMultiplier * 100)) .div(100);Using
Math.floor(targetMultiplier * 100)works fine for multipliers like 1.5 (becomes 150) or 2.0 (becomes 200), but for values like 1.55 it would floor to 155, which is correct. However, for something like 1.555, you'd get 155 instead of 155.5, losing that fractional cent.Given the schema limits multipliers to reasonable ranges and defaults (1.5 and 2.0), this is probably fine in practice. Just something to keep in mind if ye ever need finer precision - might want to use a larger scaling factor like 10000.
typescript/keyfunder/src/service.ts (3)
13-30: Looks good, though ye might want to handle that logger creation differently.The service version fallback and config file validation are proper. However,
createServiceLoggeris awaited before entering the try block, so if it throws, ye'll get a generic error instead of your nice formatted one.🧅 Consider wrapping logger creation in error handling
async function main(): Promise<void> { const VERSION = process.env.SERVICE_VERSION || 'dev'; const configFile = process.env.KEYFUNDER_CONFIG_FILE; if (!configFile) { rootLogger.error('KEYFUNDER_CONFIG_FILE environment variable is required'); process.exit(1); } + let logger; + try { + logger = await createServiceLogger({ + service: 'keyfunder', + version: VERSION, + }); + } catch (error) { + rootLogger.error({ error }, 'Failed to initialize logger'); + process.exit(1); + } - const logger = await createServiceLogger({ - service: 'keyfunder', - version: VERSION, - });
62-76: IGP initialization could use a wee bit of tightening.The filtering logic works, but there's a potential issue - if a chain has IGP config but no addresses in the registry, it silently gets skipped. Might want to log a warning when this happens so folks aren't scratching their heads wondering why IGP claims aren't happening.
🧅 Add warning for chains with IGP config but missing registry addresses
if (chainsWithIgp.length > 0) { const addresses = await registry.getAddresses(); + const missingChains = chainsWithIgp.filter((chain) => !addresses[chain]); + if (missingChains.length > 0) { + logger.warn( + { chains: missingChains }, + 'Chains with IGP config missing from registry addresses', + ); + } const igpAddresses = Object.fromEntries( chainsWithIgp .filter((chain) => addresses[chain]) .map((chain) => [chain, addresses[chain]]), ); igp = HyperlaneIgp.fromAddressesMap(igpAddresses, multiProvider); logger.info({ chains: chainsWithIgp }, 'Initialized IGP contracts'); }
90-96: Metrics push after funding - good pattern for CronJobs.This follows the Prometheus push-gateway pattern nicely. However, if
fundAllChains()throws (which it does on partial failures), metrics won't be pushed at all. For observability, ye might want those failure metrics to still make it out.🧅 Push metrics even on partial failure for better observability
- await funder.fundAllChains(); - - await metrics.push(); - logger.info('Metrics pushed to gateway'); + let fundingError: Error | undefined; + try { + await funder.fundAllChains(); + } catch (error) { + fundingError = error as Error; + } + + await metrics.push(); + logger.info('Metrics pushed to gateway'); + + if (fundingError) { + throw fundingError; + } logger.info('KeyFunder completed successfully');typescript/keyfunder/src/metrics/Metrics.ts (1)
106-113: Push method silently returns on no gateway - consider logging.When there's no Pushgateway configured, this just returns silently. That's fine for optional metrics, but the caller in service.ts logs "Metrics pushed to gateway" regardless. Either log here that there's no gateway, or have the caller check.
🧅 Return a boolean or log when push is skipped
- async push(): Promise<void> { + async push(): Promise<boolean> { if (!this.pushGateway) { - return; + return false; } const jobName = this.config?.jobName ?? 'keyfunder'; await this.pushGateway.push({ jobName }); + return true; }Then in service.ts:
- await metrics.push(); - logger.info('Metrics pushed to gateway'); + const pushed = await metrics.push(); + if (pushed) { + logger.info('Metrics pushed to gateway'); + } else { + logger.debug('No metrics gateway configured, skipping push'); + }typescript/keyfunder/src/config/types.ts (1)
77-103: Consider using ChainMap type for stronger typing.Per the coding guidelines, per-chain configurations should use
ChainMapfrom@hyperlane-xyz/sdk. Currentlychainsis typed asz.record(z.string(), ChainConfigSchema)which works at runtime but loses the SDK's chain typing benefits.This is optional since Zod schemas don't directly integrate with
ChainMap, but ye could at least type the inferredKeyFunderConfig.chainsasChainMap<ChainConfig>downstream where it's used. Based on coding guidelines, usingChainMapfor per-chain configurations is preferred.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📥 Commits
Reviewing files that changed from the base of the PR and between 5e3f9b3 and 24e271b2f70a349dd9cc3f33cd23f1d0c3d4a8d6.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (21)
.github/workflows/keyfunder-docker.yml.gitignoreDockerfiletypescript/keyfunder/.gitignoretypescript/keyfunder/.mocharc.jsontypescript/keyfunder/Dockerfiletypescript/keyfunder/README.mdtypescript/keyfunder/eslint.config.mjstypescript/keyfunder/package.jsontypescript/keyfunder/scripts/ncc.post-bundle.mjstypescript/keyfunder/src/config/KeyFunderConfig.test.tstypescript/keyfunder/src/config/KeyFunderConfig.tstypescript/keyfunder/src/config/types.test.tstypescript/keyfunder/src/config/types.tstypescript/keyfunder/src/core/KeyFunder.tstypescript/keyfunder/src/index.tstypescript/keyfunder/src/metrics/Metrics.test.tstypescript/keyfunder/src/metrics/Metrics.tstypescript/keyfunder/src/service.tstypescript/keyfunder/tsconfig.jsontypescript/keyfunder/turbo.json
🧰 Additional context used
📓 Path-based instructions (1)
typescript/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
typescript/**/*.{ts,tsx}: UseChainMapfor per-chain configurations in TypeScript
Import types from@hyperlane-xyz/sdkwhen using TypeScript SDK types
Files:
typescript/keyfunder/src/config/KeyFunderConfig.tstypescript/keyfunder/src/core/KeyFunder.tstypescript/keyfunder/src/config/KeyFunderConfig.test.tstypescript/keyfunder/src/config/types.tstypescript/keyfunder/src/service.tstypescript/keyfunder/src/config/types.test.tstypescript/keyfunder/src/index.tstypescript/keyfunder/src/metrics/Metrics.test.tstypescript/keyfunder/src/metrics/Metrics.ts
🧠 Learnings (12)
📓 Common learnings
Learnt from: paulbalaji
Repo: hyperlane-xyz/hyperlane-monorepo PR: 6891
File: typescript/infra/config/environments/mainnet3/funding.ts:22-22
Timestamp: 2025-08-13T16:53:55.163Z
Learning: In Hyperlane mainnet3 configs, funding.ts uses 'gcr.io/abacus-labs-dev/hyperlane-monorepo' docker image while agent.ts uses 'gcr.io/abacus-labs-dev/hyperlane-agent' docker image. These are different images with independent tag cycles, so tag consistency across them is not expected.
📚 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/keyfunder/.gitignoretypescript/keyfunder/src/config/KeyFunderConfig.test.tstypescript/keyfunder/eslint.config.mjstypescript/keyfunder/tsconfig.jsontypescript/keyfunder/.mocharc.jsontypescript/keyfunder/src/config/types.test.ts
📚 Learning: 2025-08-13T16:53:55.163Z
Learnt from: paulbalaji
Repo: hyperlane-xyz/hyperlane-monorepo PR: 6891
File: typescript/infra/config/environments/mainnet3/funding.ts:22-22
Timestamp: 2025-08-13T16:53:55.163Z
Learning: In Hyperlane mainnet3 configs, funding.ts uses 'gcr.io/abacus-labs-dev/hyperlane-monorepo' docker image while agent.ts uses 'gcr.io/abacus-labs-dev/hyperlane-agent' docker image. These are different images with independent tag cycles, so tag consistency across them is not expected.
Applied to files:
.github/workflows/keyfunder-docker.ymltypescript/keyfunder/Dockerfiletypescript/keyfunder/README.mdtypescript/keyfunder/package.json
📚 Learning: 2025-12-18T22:36:13.445Z
Learnt from: CR
Repo: hyperlane-xyz/hyperlane-monorepo PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-18T22:36:13.445Z
Learning: Applies to typescript/**/*.{ts,tsx} : Use `ChainMap` for per-chain configurations in TypeScript
Applied to files:
typescript/keyfunder/src/config/KeyFunderConfig.tstypescript/keyfunder/src/config/KeyFunderConfig.test.tstypescript/keyfunder/eslint.config.mjstypescript/keyfunder/src/config/types.tstypescript/keyfunder/tsconfig.jsontypescript/keyfunder/.mocharc.jsontypescript/keyfunder/src/config/types.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/keyfunder/src/config/KeyFunderConfig.tstypescript/keyfunder/src/config/KeyFunderConfig.test.tstypescript/keyfunder/src/config/types.tstypescript/keyfunder/src/config/types.test.ts
📚 Learning: 2025-12-29T19:45:12.592Z
Learnt from: paulbalaji
Repo: hyperlane-xyz/hyperlane-monorepo PR: 7545
File: typescript/infra/src/rebalancer/helm.ts:82-83
Timestamp: 2025-12-29T19:45:12.592Z
Learning: In the hyperlane-xyz/hyperlane-monorepo repository, Docker image tags in Helm managers (e.g., typescript/infra/src/rebalancer/helm.ts) are intentionally hardcoded for reproducibility and explicit version control, rather than using dynamically generated CI tags.
Applied to files:
typescript/keyfunder/Dockerfile
📚 Learning: 2025-12-22T16:50:19.890Z
Learnt from: paulbalaji
Repo: hyperlane-xyz/hyperlane-monorepo PR: 7565
File: typescript/ccip-server/Dockerfile:10-11
Timestamp: 2025-12-22T16:50:19.890Z
Learning: Standardize Foundry installation in Dockerfiles by using the official curl -L https://foundry.paradigm.xyz | bash followed by foundryup. This pattern is already used across multiple Dockerfiles in the repo and should be retained for consistency and reliability wherever Foundry is installed.
Applied to files:
typescript/keyfunder/DockerfileDockerfile
📚 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/keyfunder/Dockerfiletypescript/keyfunder/package.json.gitignore
📚 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/keyfunder/package.json
📚 Learning: 2025-12-18T22:36:13.445Z
Learnt from: CR
Repo: hyperlane-xyz/hyperlane-monorepo PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-18T22:36:13.445Z
Learning: Applies to typescript/**/*.{ts,tsx} : Import types from `hyperlane-xyz/sdk` when using TypeScript SDK types
Applied to files:
typescript/keyfunder/tsconfig.jsontypescript/keyfunder/.mocharc.json
📚 Learning: 2025-12-18T22:36:13.445Z
Learnt from: CR
Repo: hyperlane-xyz/hyperlane-monorepo PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-18T22:36:13.445Z
Learning: Most deployments are config-driven; check `typescript/infra/config/` for deployment configuration examples
Applied to files:
Dockerfile.gitignore
📚 Learning: 2025-08-26T13:45:29.848Z
Learnt from: paulbalaji
Repo: hyperlane-xyz/hyperlane-monorepo PR: 6943
File: typescript/infra/config/environments/testnet4/ism/verification.json:3835-3939
Timestamp: 2025-08-26T13:45:29.848Z
Learning: Skip reviewing verification.json files in typescript/infra/config/environments/testnet4/ism/verification.json and similar verification.json files as requested by paulbalaji.
Applied to files:
typescript/keyfunder/src/config/types.test.ts
🧬 Code graph analysis (5)
typescript/keyfunder/src/core/KeyFunder.ts (2)
typescript/keyfunder/src/metrics/Metrics.ts (1)
KeyFunderMetrics(5-118)typescript/keyfunder/src/config/types.ts (3)
KeyFunderConfig(111-111)ChainConfig(108-108)ResolvedKeyConfig(114-118)
typescript/keyfunder/src/config/KeyFunderConfig.test.ts (1)
typescript/keyfunder/src/config/KeyFunderConfig.ts (1)
KeyFunderConfigLoader(11-55)
typescript/keyfunder/scripts/ncc.post-bundle.mjs (1)
typescript/rebalancer/scripts/ncc.post-bundle.mjs (1)
prepend(17-36)
typescript/keyfunder/src/service.ts (7)
typescript/infra/src/infrastructure/monitoring/grafana.ts (1)
logger(14-14)typescript/keyfunder/src/config/KeyFunderConfig.ts (1)
KeyFunderConfigLoader(11-55)typescript/keyfunder/src/index.ts (3)
KeyFunderConfigLoader(1-1)KeyFunderMetrics(24-24)KeyFunder(23-23)typescript/infra/src/warp/helm.ts (1)
registryUri(59-62)typescript/keyfunder/src/metrics/Metrics.ts (2)
getRegistry(115-117)KeyFunderMetrics(5-118)typescript/sdk/src/index.ts (1)
HyperlaneIgp(164-164)typescript/keyfunder/src/core/KeyFunder.ts (1)
KeyFunder(29-346)
typescript/keyfunder/src/metrics/Metrics.test.ts (2)
typescript/keyfunder/src/index.ts (1)
KeyFunderMetrics(24-24)typescript/keyfunder/src/metrics/Metrics.ts (1)
KeyFunderMetrics(5-118)
⏰ 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-rebalancer)
- GitHub Check: cli-evm-e2e-matrix (core-apply)
- GitHub Check: cli-evm-e2e-matrix (warp-extend-basic)
- 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-init)
- GitHub Check: cli-evm-e2e-matrix (warp-check-1)
- GitHub Check: cli-evm-e2e-matrix (warp-extend-config)
- GitHub Check: cli-evm-e2e-matrix (warp-check-3)
- GitHub Check: cli-evm-e2e-matrix (warp-extend-recovery)
- GitHub Check: cli-evm-e2e-matrix (warp-deploy-2)
- GitHub Check: cli-evm-e2e-matrix (warp-check-2)
- GitHub Check: cli-evm-e2e-matrix (warp-send)
- GitHub Check: cli-evm-e2e-matrix (warp-read)
- GitHub Check: cli-evm-e2e-matrix (warp-check-4)
- GitHub Check: cli-evm-e2e-matrix (warp-bridge-1)
- GitHub Check: cli-evm-e2e-matrix (core-read)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-ownership-updates)
- GitHub Check: cli-evm-e2e-matrix (core-check)
- GitHub Check: cli-evm-e2e-matrix (status)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-ism-updates)
- GitHub Check: cli-evm-e2e-matrix (warp-bridge-2)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-rebalancing-config)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-submitters)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-hook-updates)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-simple-updates)
- GitHub Check: cli-evm-e2e-matrix (core-init)
- GitHub Check: cli-evm-e2e-matrix (core-deploy)
- GitHub Check: cli-evm-e2e-matrix (relay)
- GitHub Check: env-test-matrix (mainnet3, arbitrum, core)
- GitHub Check: env-test-matrix (mainnet3, optimism, igp)
- GitHub Check: env-test-matrix (mainnet3, ethereum, core)
- GitHub Check: env-test-matrix (testnet4, sepolia, core)
- GitHub Check: env-test-matrix (mainnet3, ethereum, igp)
- GitHub Check: env-test-matrix (mainnet3, arbitrum, igp)
- GitHub Check: env-test-matrix (mainnet3, optimism, core)
- GitHub Check: cli-cosmos-e2e-matrix (core-check)
- GitHub Check: cli-radix-e2e-matrix (warp-apply-route-extension)
- GitHub Check: cli-radix-e2e-matrix (core-deploy)
- GitHub Check: cli-cosmos-e2e-matrix (core-deploy)
- GitHub Check: cli-radix-e2e-matrix (warp-deploy)
- GitHub Check: cli-cosmos-e2e-matrix (warp-deploy)
- GitHub Check: cli-radix-e2e-matrix (warp-apply-ownership-updates)
- GitHub Check: cli-radix-e2e-matrix (core-apply)
- GitHub Check: cosmos-sdk-e2e-run
- GitHub Check: cli-cosmos-e2e-matrix (core-read)
- GitHub Check: cli-cosmos-e2e-matrix (core-apply)
- GitHub Check: cli-cosmos-e2e-matrix (warp-read)
- GitHub Check: aleo-sdk-e2e-run
- GitHub Check: pnpm-test-run
- GitHub Check: cli-cross-chain-e2e-matrix (warp-apply)
- GitHub Check: cli-cross-chain-e2e-matrix (warp-deploy)
- GitHub Check: cli-install-test-run
- GitHub Check: infra-test
- GitHub Check: build-and-push-to-gcr
- GitHub Check: build-and-push-to-gcr
- GitHub Check: e2e-matrix (evm)
- GitHub Check: agent-configs (mainnet3)
- GitHub Check: lander-coverage
- GitHub Check: test-rs
- GitHub Check: lint-rs
🔇 Additional comments (34)
typescript/keyfunder/eslint.config.mjs (1)
5-12: Clarify the scope of the no-restricted-imports override.Right, so here's the thing—these two config objects are doin' their own thing separately. The first one (lines 5-7) says "hey, look at these TypeScript files" but doesn't actually do anything with 'em. The second one (lines 8-12) turns off that import rule, but since it's got no
filespattern, it's applyin' to the whole shebang—every file in the package.If you're wantin' the rule disabled just for your src files, you'll need to bundle 'em together like this:
♻️ Combine files and rules if scoped override intended
export default [ ...MonorepoDefaults, { + files: ['./src/**/*.ts'], + rules: { + 'no-restricted-imports': ['off'], + }, + }, - files: ['./src/**/*.ts'], - }, - { - rules: { - 'no-restricted-imports': ['off'], - }, - }, ];Now, if you actually meant to turn off the rule for everything—tests included—then the current setup works, and you can just drop that empty files config. Let me know which way you're goin' with this.
typescript/keyfunder/.gitignore (1)
1-7: Good setup, keeps secrets out and examples in.Standard ignore patterns fer the package—no issues here. The .env* catch-all paired with the !.env.example negation is the way to go fer environment config templates.
.gitignore (1)
46-49: Tidy addition to keep generated bits outta version control.The .opencode/ pattern fits right in, no conflicts with existing patterns. Just what's needed to keep the repo clean of opencode artifacts.
typescript/keyfunder/tsconfig.json (1)
1-8: All layers in place—the base config is there and compatible.The base tsconfig at
@hyperlane-xyz/tsconfig/tsconfig.jsonexists and has all the right compiler options (strict mode, declaration maps, ES2023 target). Your keyfunder config properly extends it and adds the build roots—outDir and rootDir sit on top of that foundation without stepping on anything. The include pattern matches your rootDir, so everything's aligned. No conflicts, no surprises. This setup'll compile clean.typescript/keyfunder/turbo.json (1)
1-9: Configuration looks solid.The turbo setup properly chains build → bundle with correct output paths. This'll work well with the Docker workflow.
typescript/keyfunder/.mocharc.json (1)
1-3: Test config looks good.The tsx import setup will handle TypeScript tests nicely.
Dockerfile (1)
34-34: Properly integrated into the build.The keyfunder package.json copy follows the same pattern as other packages and fits right in with the monorepo structure.
typescript/keyfunder/README.md (1)
161-161: Path is already in place and properly configured.The Helm chart directory
typescript/infra/helm/key-funder/exists with the expected structure (Chart.yaml, values.yaml, and templates). The README reference is accurate and needs no changes.Likely an incorrect or invalid review comment.
typescript/keyfunder/src/metrics/Metrics.test.ts (1)
1-125: Solid test coverage for the metrics class, looks good to me.The test suite covers all the key recording methods and validates that metrics appear in the registry output with the right names and labels. The base labels propagation test at lines 106-123 is particularly nice - ensures labels like
environmentandregionmake it through to all metrics.One small thing to consider for the future: ye might want to add tests for edge cases like empty strings or special characters in label values, but that's more of a "someday" improvement than something blocking the road to Far Far Away.
typescript/keyfunder/scripts/ncc.post-bundle.mjs (1)
1-37: This follows the same pattern as the rebalancer script - nice and consistent.The script properly handles the ESM bundling quirk where
__dirnameisn't available. The approach of splitting on the shebang and prepending the dirname definition is the same proven pattern fromtypescript/rebalancer/scripts/ncc.post-bundle.mjs.Get out of me swamp... I mean, this is ready to merge.
typescript/keyfunder/src/config/KeyFunderConfig.test.ts (1)
1-188: Well-structured test suite covering the config loader's key paths.The test setup with sinon stubs for
fs.existsSyncandfs.readFileSyncis clean, and theafterEachcleanup ensures no test pollution. Good coverage of:
- File-based loading (lines 21-44)
- Error handling for missing files and invalid configs (lines 46-66)
- Object-based creation via
fromObject(lines 69-104)- Chain filtering with
chainsToSkip(lines 125-160)- Default value handling for
privateKeyEnvVar(lines 177-186)Like layers in an onion, these tests peel back each piece of functionality. Well done.
typescript/keyfunder/src/config/types.test.ts (1)
1-291: Thorough Zod schema validation tests - covers the important validation rules.The test cases exercise the schema refinements well:
- Sweep multiplier difference validation (lines 48-58)
- Skipping validation when sweep is disabled (lines 73-81)
- Cross-reference validation for role definitions (lines 225-244, 246-273)
- Default value application (lines 275-289)
These tests ensure the config validation will catch misconfigurations before they cause problems at runtime. That's what I call doing the donkey work upfront.
.github/workflows/keyfunder-docker.yml (1)
1-125: Workflow follows the established patterns from other service workflows.The structure is solid:
- Proper concurrency grouping (lines 18-20) to avoid redundant builds
- Secret validation via
check-envjob before attempting GCR operations- Multi-platform build support with opt-in arm64 (lines 85-92)
- Dynamic tagging with branch/PR refs and SHA-date combo for traceability
The image name
gcr.io/abacus-labs-dev/hyperlane-keyfunderaligns with the separate image convention used in this repo. Based on learnings, different services like keyfunder, monorepo, and agent use independent images with their own tag cycles.Some workflows might look the same but they're each beautiful in their own way. This one's ready.
typescript/keyfunder/src/config/KeyFunderConfig.ts (1)
11-55: Clean config loader with proper validation - the private constructor pattern is a nice touch.The factory methods
loadandfromObjectensure all configs go through Zod validation before creating instances. ThefromZodErrorwrapper makes validation errors readable for humans.One thing worth mentioning: per coding guidelines,
ChainMapfrom@hyperlane-xyz/sdkis preferred for per-chain configurations. The current implementation usesz.record(z.string(), ChainConfigSchema)which is structurally similar but doesn't leverage the SDK type. For a config that's YAML-driven and Zod-validated, this approach works fine, but if you later integrate more tightly with SDK utilities, consider aligning withChainMap.That said, what we've got here works perfectly well in this swamp.
typescript/keyfunder/src/core/KeyFunder.ts (4)
29-56: Solid orchestration with Promise.allSettled - handles partial failures gracefully.The
fundAllChainsmethod properly usesPromise.allSettledto run all chains in parallel without short-circuiting on first failure. The failure count is logged and an aggregate error is thrown, which gives good visibility into what went wrong.The timeout wrapper at lines 58-69 with proper cleanup is a nice touch to prevent hung chains from blocking the entire run.
259-272: Funding threshold logic uses a 60% minimum delta guard.The calculation at lines 268-271 ensures you don't send tiny top-ups - the delta must exceed 60% of the desired balance. This is a sensible optimization to avoid transaction costs for marginal funding.
Just making sure this is intentional: if
desiredBalanceis 1 ETH andcurrentBalanceis 0.5 ETH, the delta is 0.5 ETH butminDeltais 0.6 ETH, so no funding happens. The key would need to drop below 0.4 ETH before funding kicks in. That might be the intended behavior for hysteresis, but worth confirming.
139-176: IGP claim flow looks good, but consider the claim destination.The
igpContract.populateTransaction.claim()call at line 172 will claim IGP funds to the caller (funder wallet). This aligns with the flow where you first claim IGP funds to the funder, then distribute to keys. Just making sure this is the intended behavior.Also, the IGP balance check uses
provider.getBalance(igpContract.address)to get the contract's native token balance. This works for IGP contracts that hold native tokens, but if the IGP ever moves to an ERC20 model, this would need updating.
348-360: Clean timeout helper with proper cleanup.The
createTimeoutPromisefunction returns both the promise and a cleanup function, which is the right pattern to avoid timer leaks. The caller properly callscleanup()in afinallyblock at line 67.This is the kind of helper that belongs in a utility module if it gets reused elsewhere, but for now it's fine here in the swamp.
typescript/keyfunder/src/service.ts (4)
1-11: LGTM! Imports look solid, like layers of an onion.The imports are well-organized with external dependencies first, then internal modules. Good use of
@hyperlane-xyz/sdktypes as per the coding guidelines.
37-41: Private key handling looks right.Reading the private key from a configurable environment variable is the way to go - keeps secrets out of config files and supports flexibility. No logging of sensitive data here, which is what we want.
104-115: RPC override helper is clean and simple.The env var naming convention
RPC_URL_{CHAIN}with hyphen-to-underscore replacement is sensible. The in-place mutation ofchainMetadatais a bit swampy but works fine since it's called right before passing to MultiProvider.
117-121: Top-level error handler as a backup - fair enough.This catches anything that escapes the main try/catch. Since
main()already has comprehensive error handling, this is just a safety net. Solid defensive coding.typescript/keyfunder/Dockerfile (4)
9-16: Foundry installation looks proper.Using the direct release download with architecture detection matches the pattern from the learnings. The
set -o pipefailensures the curl|tar chain fails properly if curl errors out. Good stuff.
18-39: Layer caching strategy is on point.Copying package.jsons and lockfile before the full source allows Docker to cache the
pnpm installlayer. This'll save ye time on rebuilds when only source changes.
68-73: Runtime config looks good.Setting
NODE_ENV=production, defaultLOG_LEVEL=info, and passing throughSERVICE_VERSIONare sensible defaults. TheCMDpointing to the bundle entrypoint is clean.
58-67: This dependency is already in package.json, so the suggested fix won't solve the real problem.The
@google-cloud/pino-logging-gcp-configis already declared intypescript/keyfunder/package.jsonand installed at build time viapnpm install --frozen-lockfile. However, the bundler explicitly excludes it with the-e @google-cloud/pino-logging-gcp-configflag in the ncc build command, which is why the runtimenpm installis necessary.The concern about bypassing the lockfile is valid—the runtime install doesn't respect the pnpm lock—but just adding the dependency to package.json (where it already exists) won't help. To fix this properly, either:
- Remove the bundler exclusion flag and include the dependency in the bundle, or
- Use
npm ciinstead ofnpm installat runtime to respect the lockfile from build timetypescript/keyfunder/src/metrics/Metrics.ts (2)
15-61: Constructor sets up gauges nicely, but there's a label mismatch.The
walletBalanceGaugeandfundingAmountGaugeare defined with labelNames including['chain', 'address', 'role', ...baseLabels], which is correct. However,igpBalanceGaugeandsweepAmountGaugeonly have['chain', ...baseLabels]- this is intentional since those don't track per-address metrics. That's fine.One thing though - ye're spreading
Object.keys(baseLabels)into the label arrays, but if someone passes a baseLabel with the same name as an existing label (likechain), things could get confusing. Might be worth documenting or validating.
63-104: Recording methods are straightforward and consistent.Each method properly spreads the baseLabels alongside the specific labels for that gauge. The numeric values are passed directly to
set(). No issues here.typescript/keyfunder/src/index.ts (1)
1-24: Public API exports look well-organized.Ye've got a clean separation between value exports (schemas, classes) and type exports. The
export typesyntax for types-only exports helps with tree-shaking. This makes for a nice, layered public surface - like an onion, it's got layers.typescript/keyfunder/src/config/types.ts (5)
3-8: Address validation is solid but EVM-only.The regex
^0x[a-fA-F0-9]{40}$correctly validates Ethereum addresses. If this package ever needs to support non-EVM chains (Cosmos, Radix from the imports I see in the Dockerfile), this schema would need updating. For now though, it's fit for purpose.
10-15: BalanceStringSchema could be tighter.The current validation allows strings like
"1e18"(scientific notation) or".5"(no leading zero) sinceparseFloathandles those. If ye want stricter decimal format validation, ye could use a regex. But for ETH balances passed toethers.utils.parseEther, this works fine.
32-59: SweepConfig refinement is clever.The cross-field validation ensuring
triggerMultiplier >= targetMultiplier + 0.05only whenenabledis smart - prevents configs where ye'd sweep and immediately trigger again. The min/max bounds are reasonable.
86-103: Role reference validation is a nice touch.The refinement that checks all role names in chain balances exist in the
rolessection catches config errors early. Good user experience - tells 'em exactly what's wrong.
114-118: ResolvedKeyConfig interface is clean.Simple interface for the resolved key data after merging role configs with chain balance requirements. This is used in the core KeyFunder logic.
paulbalaji
force-pushed
the
pbio/keyfunder-package
branch
from
1b8c30e to
86722d9
Compare
paulbalaji
changed the title
paulbalaji
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
typescript/keyfunder/src/config/types.ts (2)
3-8: Address validation works, but could be stronger.The regex correctly validates the basic Ethereum address format. However, you might want to consider using
ethers.utils.getAddress()for validation, which also handles checksum validation and normalization.🔧 Consider checksum validation
-const AddressSchema = z - .string() - .regex( - /^0x[a-fA-F0-9]{40}$/, - 'Must be a valid Ethereum address (0x-prefixed, 40 hex characters)', - ); +const AddressSchema = z + .string() + .refine( + (val) => { + try { + ethers.utils.getAddress(val); + return true; + } catch { + return false; + } + }, + 'Must be a valid Ethereum address with correct checksum', + );Note: This would require importing
ethersin this file.
10-15: Optimization opportunity here.The validation calls
parseFloat(val)twice – once for the NaN check and once for the >= 0 check. You could parse it once and reuse the result.♻️ Optimize parseFloat calls
const BalanceStringSchema = z .string() .refine( - (val) => !isNaN(parseFloat(val)) && parseFloat(val) >= 0, + (val) => { + const parsed = parseFloat(val); + return !isNaN(parsed) && parsed >= 0; + }, 'Must be a valid non-negative number string', );
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📥 Commits
Reviewing files that changed from the base of the PR and between 24e271b2f70a349dd9cc3f33cd23f1d0c3d4a8d6 and 86722d9.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (21)
.github/workflows/keyfunder-docker.yml.gitignoreDockerfiletypescript/keyfunder/.gitignoretypescript/keyfunder/.mocharc.jsontypescript/keyfunder/Dockerfiletypescript/keyfunder/README.mdtypescript/keyfunder/eslint.config.mjstypescript/keyfunder/package.jsontypescript/keyfunder/scripts/ncc.post-bundle.mjstypescript/keyfunder/src/config/KeyFunderConfig.test.tstypescript/keyfunder/src/config/KeyFunderConfig.tstypescript/keyfunder/src/config/types.test.tstypescript/keyfunder/src/config/types.tstypescript/keyfunder/src/core/KeyFunder.tstypescript/keyfunder/src/index.tstypescript/keyfunder/src/metrics/Metrics.test.tstypescript/keyfunder/src/metrics/Metrics.tstypescript/keyfunder/src/service.tstypescript/keyfunder/tsconfig.jsontypescript/keyfunder/turbo.json
🚧 Files skipped from review as they are similar to previous changes (13)
- typescript/keyfunder/src/config/types.test.ts
- typescript/keyfunder/turbo.json
- typescript/keyfunder/.mocharc.json
- typescript/keyfunder/eslint.config.mjs
- typescript/keyfunder/.gitignore
- .gitignore
- Dockerfile
- typescript/keyfunder/src/metrics/Metrics.test.ts
- typescript/keyfunder/package.json
- typescript/keyfunder/src/service.ts
- typescript/keyfunder/src/index.ts
- typescript/keyfunder/scripts/ncc.post-bundle.mjs
- typescript/keyfunder/tsconfig.json
🧰 Additional context used
📓 Path-based instructions (1)
typescript/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
typescript/**/*.{ts,tsx}: UseChainMapfor per-chain configurations in TypeScript
Import types from@hyperlane-xyz/sdkwhen using TypeScript SDK types
Files:
typescript/keyfunder/src/config/KeyFunderConfig.test.tstypescript/keyfunder/src/core/KeyFunder.tstypescript/keyfunder/src/metrics/Metrics.tstypescript/keyfunder/src/config/types.tstypescript/keyfunder/src/config/KeyFunderConfig.ts
🧠 Learnings (7)
📚 Learning: 2025-08-13T16:53:55.163Z
Learnt from: paulbalaji
Repo: hyperlane-xyz/hyperlane-monorepo PR: 6891
File: typescript/infra/config/environments/mainnet3/funding.ts:22-22
Timestamp: 2025-08-13T16:53:55.163Z
Learning: In Hyperlane mainnet3 configs, funding.ts uses 'gcr.io/abacus-labs-dev/hyperlane-monorepo' docker image while agent.ts uses 'gcr.io/abacus-labs-dev/hyperlane-agent' docker image. These are different images with independent tag cycles, so tag consistency across them is not expected.
Applied to files:
typescript/keyfunder/Dockerfile.github/workflows/keyfunder-docker.ymltypescript/keyfunder/README.md
📚 Learning: 2025-12-29T19:45:12.592Z
Learnt from: paulbalaji
Repo: hyperlane-xyz/hyperlane-monorepo PR: 7545
File: typescript/infra/src/rebalancer/helm.ts:82-83
Timestamp: 2025-12-29T19:45:12.592Z
Learning: In the hyperlane-xyz/hyperlane-monorepo repository, Docker image tags in Helm managers (e.g., typescript/infra/src/rebalancer/helm.ts) are intentionally hardcoded for reproducibility and explicit version control, rather than using dynamically generated CI tags.
Applied to files:
typescript/keyfunder/Dockerfile
📚 Learning: 2025-12-22T16:50:19.890Z
Learnt from: paulbalaji
Repo: hyperlane-xyz/hyperlane-monorepo PR: 7565
File: typescript/ccip-server/Dockerfile:10-11
Timestamp: 2025-12-22T16:50:19.890Z
Learning: Standardize Foundry installation in Dockerfiles by using the official curl -L https://foundry.paradigm.xyz | bash followed by foundryup. This pattern is already used across multiple Dockerfiles in the repo and should be retained for consistency and reliability wherever Foundry is installed.
Applied to files:
typescript/keyfunder/Dockerfile
📚 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/keyfunder/Dockerfile
📚 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/keyfunder/src/config/KeyFunderConfig.test.tstypescript/keyfunder/src/config/types.ts
📚 Learning: 2025-12-18T22:36:13.445Z
Learnt from: CR
Repo: hyperlane-xyz/hyperlane-monorepo PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-18T22:36:13.445Z
Learning: Applies to typescript/**/*.{ts,tsx} : Use `ChainMap` for per-chain configurations in TypeScript
Applied to files:
typescript/keyfunder/src/config/KeyFunderConfig.test.tstypescript/keyfunder/src/core/KeyFunder.tstypescript/keyfunder/src/config/types.tstypescript/keyfunder/src/config/KeyFunderConfig.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/keyfunder/src/config/KeyFunderConfig.test.tstypescript/keyfunder/src/config/types.tstypescript/keyfunder/src/config/KeyFunderConfig.ts
🧬 Code graph analysis (2)
typescript/keyfunder/src/config/KeyFunderConfig.test.ts (1)
typescript/keyfunder/src/config/KeyFunderConfig.ts (1)
KeyFunderConfigLoader(11-51)
typescript/keyfunder/src/metrics/Metrics.ts (2)
typescript/keyfunder/src/index.ts (2)
KeyFunderMetrics(22-22)MetricsConfig(17-17)typescript/keyfunder/src/config/types.ts (1)
MetricsConfig(104-104)
🪛 LanguageTool
typescript/keyfunder/README.md
[uncategorized] ~92-~92: The official name of this software platform is spelled with a capital “H”.
Context: ...gistry). Supports commit pinning (e.g., `github://hyperlane-xyz/hyperlane-registry/comm...
(GITHUB)
⏰ 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). (1)
- GitHub Check: Greptile Review
🔇 Additional comments (24)
typescript/keyfunder/README.md (1)
1-174: Well-organized documentation for the new service.The README provides clear guidance on configuration, environment setup, usage patterns, and operational behavior. The examples are practical and the funding logic explanation helps users understand when actions trigger.
typescript/keyfunder/src/config/KeyFunderConfig.test.ts (1)
1-161: Solid test coverage for the config loader.The test suite covers all critical paths: file loading with validation, object construction, chain retrieval, and skip filtering. The use of stubs for file operations keeps tests isolated and fast.
.github/workflows/keyfunder-docker.yml (1)
1-125: Workflow handles multi-platform builds properly.The workflow correctly orchestrates Docker builds with dynamic platform selection, proper tagging strategy (branch/PR/SHA-Date), and PR commenting for visibility. The Foundry version extraction and build args are wired up as expected.
typescript/keyfunder/Dockerfile (1)
1-73: Multi-stage build follows established patterns.The Dockerfile properly integrates the new keyfunder package into the monorepo build process. Foundry installation follows the learned pattern, workspace dependencies are correctly wired, and the runner stage is appropriately minimal.
typescript/keyfunder/src/config/KeyFunderConfig.ts (1)
42-50: No action needed—the config schema correctly usesz.record().The coding guideline about
ChainMapapplies to runtime chain configurations in the SDK, not to config file schemas. The KeyFunderConfig correctly usesz.record(z.string(), ChainConfigSchema)to define and validate the chains structure from YAML, which is the established pattern across the codebase. Once validated, the plain object interface is appropriate for config loaders. ChainMap gets used downstream when the application needs runtime per-chain data structures.typescript/keyfunder/src/metrics/Metrics.ts (4)
1-17: Looking good here!The imports and class structure are spot-on. Using readonly for the gauge properties ensures they can't be accidentally reassigned after initialization.
15-61: Nice and tidy constructor!The label differentiation is handled well – wallet and funding metrics track individual keys (chain/address/role), while IGP and sweep metrics are chain-level only. BaseLabels are consistently propagated across all metrics.
63-104: These recording methods are solid!Each method correctly provides the labels that match its corresponding gauge definition from the constructor. The label spreading pattern is consistent throughout.
106-118: Push gateway handling looks proper!The early return when pushGateway isn't configured is a clean pattern, and the jobName fallback to 'keyfunder' provides sensible defaults.
typescript/keyfunder/src/config/types.ts (4)
17-24: Simple and effective schemas!RoleConfig and IgpConfig are cleanly defined with appropriate field types.
26-59: Sweep configuration is well thought out!The multiplier bounds and the refinement ensuring trigger > target + 0.05 make good sense. Defaults of 1.5x/2.0x provide reasonable sweep behavior, and the validation only applies when sweeping is enabled.
61-71: Config schemas look good!ChainConfig and MetricsConfig provide the right level of optionality and sensible defaults (jobName='keyfunder').
73-112: Excellent cross-validation refinement!The refinement that ensures all roles referenced in chain balances actually exist in the roles section prevents a whole class of configuration errors. Clear error message too.
typescript/keyfunder/src/core/KeyFunder.ts (11)
16-16: Verify this hardcoded sweep address.The default sweep address
0x478be6076f31E9666123B9721D0B6631baD944AFis hardcoded. Make sure this is an address your team controls and that it's intentional to have this fallback. If different environments need different addresses, consider making this configurable.
29-56: Parallel funding with proper error aggregation!Using
Promise.allSettledis the right choice here – it ensures all chains get a chance to fund even if some fail, and the error reporting gives clear counts of failures vs. total chains.
58-69: Timeout handling is nicely done!The try/finally pattern ensures the timeout timer gets cleaned up whether the operation succeeds, fails, or times out. This prevents timer leaks.
71-106: Clean orchestration logic!The funding flow is well-organized: IGP claim → fund keys → sweep excess. The duration tracking and contextual logging are good operational practices.
108-137: Role resolution with good defensive checks!The warning when a role isn't found is helpful for debugging config issues, and skipping the key rather than throwing allows the rest of the chain to be funded.
139-176: IGP claim logic looks solid!The threshold check prevents unnecessary claims, and recording the balance metric before the check gives good visibility into IGP state even when no claim happens.
178-257: Key funding implementation is thorough!Sequential processing of keys is the right choice for transaction nonce management, and the comprehensive logging (including funder balance and explorer URLs) makes this easy to monitor and debug.
259-272: Smart funding threshold!The 60% minimum delta (meaning fund only when below 40% of desired) prevents frequent small top-ups and saves on gas costs. Good balance between keeping keys funded and transaction efficiency.
285-297: Multiplier math uses integer percentage conversion.Lines 293-294 and 295-297 convert floating-point multipliers to integer percentages (e.g., 1.5 → 150) using
Math.floor(multiplier * 100), then apply them to BigNumber math. This avoids floating-point precision issues in BigNumber arithmetic.Note: This truncates values to 2 decimal places (e.g., 1.567 becomes 1.56), which should be fine given the schema constraints, but be aware the conversion happens.
298-345: Sweep execution looks proper!The trigger check prevents unnecessary sweeps, and the amount calculation (funderBalance - targetBalance) correctly leaves the funder with the target amount. Logging and metrics are consistent with the rest of the class.
348-360: Timeout helper is well-designed!Returning both the promise and cleanup function is a clean pattern, and using
Promise<never>as the type correctly reflects that this promise only rejects.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7720 +/- ##
=======================================
Coverage 77.02% 77.02%
=======================================
Files 117 117
Lines 2651 2651
Branches 244 244
=======================================
Hits 2042 2042
Misses 593 593
Partials 16 16
🚀 New features to boost your workflow:
|
Review notesConfig/docs/behavior mismatches
Validation / safety
Docker reproducibility
FYI local checks
|
Follow-up (updated suggestions)
(Please ignore my earlier Dockerfile-related note.) |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @typescript/keyfunder/src/core/KeyFunder.ts:
- Around line 287-293: The multiplier math in KeyFunder.ts can lose precision
due to floating-point errors; update the calculations that compute targetBalance
and triggerThreshold to avoid using Math.floor on floating multiplications —
replace Math.floor(sweepConfig.targetMultiplier * 100) and
Math.floor(sweepConfig.triggerMultiplier * 100) with Math.round(...) (or
alternatively compute the multiplier as integer cents via parsing the config
string and using BigNumber math) so threshold.mul(...) uses the correct rounded
multiplier; adjust the expressions for targetBalance and triggerThreshold
accordingly.
🧹 Nitpick comments (4)
typescript/keyfunder/README.md (1)
92-92: Minor capitalization nit, if ye care about such things.The static analysis tool noticed "github" should be "GitHub" with a capital H. Not the end of the world, but might as well keep things lookin' proper in the docs.
📝 Suggested fix
-| `REGISTRY_URI` | Hyperlane registry URI (default: GitHub registry). Supports commit pinning (e.g., `github://hyperlane-xyz/hyperlane-registry/commit/abc123`) | No | +| `REGISTRY_URI` | Hyperlane registry URI (default: GitHub registry). Supports commit pinning (e.g., `GitHub://hyperlane-xyz/hyperlane-registry/commit/abc123`) | No |typescript/keyfunder/src/config/types.test.ts (1)
92-100: Consider adding a test for edge case with multipliers.When sweep is disabled, the test shows that validation is skipped even when
triggerMultiplierequalstargetMultiplier. That's fine. But ye might want to add a test that verifies whenenabled: trueand both multipliers are provided but equal (should fail), just to be extra thorough on that boundary.it('should reject equal multipliers when enabled', () => { const config = { enabled: true, address: '0x478be6076f31E9666123B9721D0B6631baD944AF', threshold: '0.5', targetMultiplier: 1.5, triggerMultiplier: 1.5, }; const result = SweepConfigSchema.safeParse(config); expect(result.success).to.be.false; });typescript/keyfunder/src/core/KeyFunder.ts (1)
190-206: Minor inefficiency: balance fetched twice.
calculateFundingAmountfetches the balance at line 260-262, thenfundKeyfetches it again at lines 197-199 for metrics recording. Not a big deal for correctness, but ye could refactor to pass the balance around and save an RPC call per key.♻️ Optional refactor to avoid double fetch
private async fundKey(chain: string, key: ResolvedKeyConfig): Promise<void> { const logger = this.options.logger.child({ chain, address: key.address, role: key.role, }); + const currentBalance = await this.multiProvider + .getProvider(chain) + .getBalance(key.address); + const desiredBalance = ethers.utils.parseEther(key.desiredBalance); - const fundingAmount = await this.calculateFundingAmount( - chain, - key.address, - desiredBalance, - ); - - const currentBalance = await this.multiProvider - .getProvider(chain) - .getBalance(key.address); + const fundingAmount = this.calculateFundingAmount( + currentBalance, + desiredBalance, + );And update
calculateFundingAmountto takecurrentBalanceas a parameter instead of fetching it.typescript/keyfunder/src/config/types.ts (1)
69-73: Consider usingChainMaptype for stronger typing.Per the coding guidelines,
ChainMapfrom@hyperlane-xyz/sdkis preferred for per-chain configurations in TypeScript. Currently usingz.record(z.string(), ChainConfigSchema)which works but loses the semantic meaning. Since this is a Zod schema for runtime validation, the record approach is reasonable, but the inferred type could be cast or aliased toChainMap<ChainConfig>for better integration with the SDK.This is a minor suggestion - the current approach works fine, especially since Zod doesn't have native
ChainMapsupport.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📥 Commits
Reviewing files that changed from the base of the PR and between 86722d9 and d7cf85d3ff1d0b1f3a4445e63e432551cd9a0a0e.
📒 Files selected for processing (5)
typescript/keyfunder/README.mdtypescript/keyfunder/src/config/types.test.tstypescript/keyfunder/src/config/types.tstypescript/keyfunder/src/core/KeyFunder.tstypescript/keyfunder/src/service.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- typescript/keyfunder/src/service.ts
🧰 Additional context used
📓 Path-based instructions (1)
typescript/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
typescript/**/*.{ts,tsx}: UseChainMapfor per-chain configurations in TypeScript
Import types from@hyperlane-xyz/sdkwhen using TypeScript SDK types
Files:
typescript/keyfunder/src/config/types.tstypescript/keyfunder/src/core/KeyFunder.tstypescript/keyfunder/src/config/types.test.ts
🧠 Learnings (6)
📓 Common learnings
Learnt from: paulbalaji
Repo: hyperlane-xyz/hyperlane-monorepo PR: 6891
File: typescript/infra/config/environments/mainnet3/funding.ts:22-22
Timestamp: 2025-08-13T16:53:55.163Z
Learning: In Hyperlane mainnet3 configs, funding.ts uses 'gcr.io/abacus-labs-dev/hyperlane-monorepo' docker image while agent.ts uses 'gcr.io/abacus-labs-dev/hyperlane-agent' docker image. These are different images with independent tag cycles, so tag consistency across them is not expected.
Learnt from: xeno097
Repo: hyperlane-xyz/hyperlane-monorepo PR: 7253
File: typescript/infra/scripts/funding/fund-wallet-from-deployer-key.ts:285-289
Timestamp: 2025-10-24T18:58:44.557Z
Learning: In the Hyperlane funding script (typescript/infra/scripts/funding/fund-wallet-from-deployer-key.ts), tokens with 0 decimals are not valid and should be rejected. The truthy check on decimals is intentional and correct for this use case.
📚 Learning: 2025-12-18T22:36:13.445Z
Learnt from: CR
Repo: hyperlane-xyz/hyperlane-monorepo PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-18T22:36:13.445Z
Learning: Applies to typescript/**/*.{ts,tsx} : Use `ChainMap` for per-chain configurations in TypeScript
Applied to files:
typescript/keyfunder/src/config/types.tstypescript/keyfunder/src/core/KeyFunder.tstypescript/keyfunder/src/config/types.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/keyfunder/src/config/types.tstypescript/keyfunder/src/config/types.test.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/keyfunder/src/config/types.tstypescript/keyfunder/src/config/types.test.ts
📚 Learning: 2025-10-24T18:58:44.557Z
Learnt from: xeno097
Repo: hyperlane-xyz/hyperlane-monorepo PR: 7253
File: typescript/infra/scripts/funding/fund-wallet-from-deployer-key.ts:285-289
Timestamp: 2025-10-24T18:58:44.557Z
Learning: In the Hyperlane funding script (typescript/infra/scripts/funding/fund-wallet-from-deployer-key.ts), tokens with 0 decimals are not valid and should be rejected. The truthy check on decimals is intentional and correct for this use case.
Applied to files:
typescript/keyfunder/src/config/types.test.ts
📚 Learning: 2025-08-13T16:53:55.163Z
Learnt from: paulbalaji
Repo: hyperlane-xyz/hyperlane-monorepo PR: 6891
File: typescript/infra/config/environments/mainnet3/funding.ts:22-22
Timestamp: 2025-08-13T16:53:55.163Z
Learning: In Hyperlane mainnet3 configs, funding.ts uses 'gcr.io/abacus-labs-dev/hyperlane-monorepo' docker image while agent.ts uses 'gcr.io/abacus-labs-dev/hyperlane-agent' docker image. These are different images with independent tag cycles, so tag consistency across them is not expected.
Applied to files:
typescript/keyfunder/README.md
🧬 Code graph analysis (1)
typescript/keyfunder/src/config/types.ts (1)
typescript/keyfunder/src/index.ts (14)
RoleConfigSchema(4-4)IgpConfigSchema(5-5)SweepConfigSchema(6-6)ChainConfigSchema(7-7)MetricsConfigSchema(8-8)KeyFunderConfigSchema(3-3)RoleConfig(13-13)IgpConfig(14-14)SweepConfig(15-15)ChainConfig(16-16)MetricsConfig(17-17)KeyFunderConfig(11-11)KeyFunderConfigInput(12-12)ResolvedKeyConfig(18-18)
🪛 LanguageTool
typescript/keyfunder/README.md
[uncategorized] ~92-~92: The official name of this software platform is spelled with a capital “H”.
Context: ...gistry). Supports commit pinning (e.g., `github://hyperlane-xyz/hyperlane-registry/comm...
(GITHUB)
⏰ 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-send)
- GitHub Check: cli-evm-e2e-matrix (warp-init)
- GitHub Check: cli-evm-e2e-matrix (warp-bridge-1)
- GitHub Check: cli-evm-e2e-matrix (warp-deploy-2)
- GitHub Check: cli-evm-e2e-matrix (warp-check-1)
- GitHub Check: cli-evm-e2e-matrix (core-deploy)
- GitHub Check: cli-evm-e2e-matrix (warp-rebalancer)
- 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-apply-ownership-updates)
- GitHub Check: cli-evm-e2e-matrix (warp-read)
- GitHub Check: cli-evm-e2e-matrix (warp-check-5)
- GitHub Check: cli-evm-e2e-matrix (core-init)
- 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-check-2)
- GitHub Check: cli-evm-e2e-matrix (status)
- GitHub Check: cli-evm-e2e-matrix (warp-extend-config)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-submitters)
- GitHub Check: cli-evm-e2e-matrix (core-apply)
- GitHub Check: cli-evm-e2e-matrix (core-read)
- GitHub Check: cli-evm-e2e-matrix (warp-extend-recovery)
- GitHub Check: cli-evm-e2e-matrix (relay)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-rebalancing-config)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-simple-updates)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-hook-updates)
- GitHub Check: cli-evm-e2e-matrix (warp-extend-basic)
- GitHub Check: cli-evm-e2e-matrix (core-check)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-ism-updates)
- GitHub Check: env-test-matrix (mainnet3, ethereum, igp)
- GitHub Check: env-test-matrix (mainnet3, optimism, core)
- GitHub Check: env-test-matrix (mainnet3, arbitrum, core)
- GitHub Check: cli-cosmos-e2e-matrix (core-check)
- GitHub Check: cli-radix-e2e-matrix (core-deploy)
- GitHub Check: env-test-matrix (mainnet3, optimism, igp)
- GitHub Check: cli-cosmos-e2e-matrix (warp-deploy)
- GitHub Check: env-test-matrix (testnet4, sepolia, core)
- GitHub Check: env-test-matrix (mainnet3, arbitrum, igp)
- GitHub Check: env-test-matrix (mainnet3, ethereum, core)
- GitHub Check: cli-radix-e2e-matrix (core-apply)
- GitHub Check: cli-radix-e2e-matrix (warp-deploy)
- GitHub Check: cli-cosmos-e2e-matrix (core-apply)
- GitHub Check: cosmos-sdk-e2e-run
- GitHub Check: cli-cross-chain-e2e-matrix (warp-apply)
- GitHub Check: cli-cosmos-e2e-matrix (core-read)
- GitHub Check: cli-cosmos-e2e-matrix (warp-read)
- GitHub Check: cli-cosmos-e2e-matrix (core-deploy)
- GitHub Check: cli-cross-chain-e2e-matrix (warp-deploy)
- GitHub Check: cli-install-test-run
- GitHub Check: cli-radix-e2e-matrix (warp-apply-ownership-updates)
- GitHub Check: cli-radix-e2e-matrix (warp-apply-route-extension)
- GitHub Check: pnpm-test-run
- GitHub Check: aleo-sdk-e2e-run
- GitHub Check: infra-test
- GitHub Check: build-and-push-to-gcr
- GitHub Check: build-and-push-to-gcr
- GitHub Check: e2e-matrix (evm)
- GitHub Check: agent-configs (mainnet3)
- GitHub Check: test-rs
- GitHub Check: lander-coverage
- GitHub Check: lint-rs
🔇 Additional comments (13)
typescript/keyfunder/README.md (1)
131-150: Documentation looks solid here.The funding logic explanation at line 135 correctly states that keys get funded when balance drops below 40% of desired - this matches the implementation in
KeyFunder.tswhereMIN_DELTA_NUMERATOR=6andMIN_DELTA_DENOMINATOR=10means funding triggers whendelta > 60%of desired (current balance < 40%). The sweep logic explanation with the example is also clear and accurate.typescript/keyfunder/src/config/types.test.ts (3)
155-183: Nice one on the balance validation tests.These tests cover the important edge cases - scientific notation rejection, decimal precision limits, and negative values. This addresses the concern from the PR comments about
BalanceStringSchemaneeding to reject formats thatparseEthercan't handle. The regex-based validation will catch these before they cause runtime failures.
48-76: Sweep config validation tests look proper.Good coverage here - ye've got tests for enabled sweep without address, without threshold, and the trigger/target multiplier relationship. This directly addresses the PR comment about sweep config safety. The schema now properly requires both
addressandthresholdwhenenabled: true.
262-310: Role reference validation is well tested.The cross-reference validation between chains and roles is covered nicely - both the rejection of undefined roles and acceptance of properly defined ones across multiple chains. This prevents configuration mistakes where someone might typo a role name in their chain balances.
typescript/keyfunder/src/core/KeyFunder.ts (5)
13-16: Constants look good, nice and clear.The
MIN_DELTA_NUMERATOR=6andMIN_DELTA_DENOMINATOR=10make the 60% threshold explicit. The 60-second timeout per chain is reasonable for blockchain operations. These align with what's documented in the README.
54-65: Clean timeout handling with proper cleanup.The
createTimeoutPromisehelper with cleanup in thefinallyblock prevents timer leaks. This is the right pattern for timeout races - ye don't want dangling timers muckin' things up.
270-285: Good defensive validation for sweep config.The runtime check at lines 281-285 throws a clear error if
addressorthresholdis missing when sweep is enabled. This is a nice safety net complementing the Zod schema validation - belt and suspenders, as they say in the swamp.
344-356: Timeout helper is clean and reusable.The
createTimeoutPromisefunction properly returns both the promise and a cleanup function. The cleanup prevents the timeout from firing after the race is won by the actual operation. Solid pattern.
135-172: The IGP address source already aligns with the intended design.The config
igp.addressis already being used correctly inservice.ts(line 82) where it's prioritized with the registry as fallback:igpConfig.address ?? registryChainAddresses.interchainGasPaymaster. TheHyperlaneIgpis initialized with these addresses, so whenKeyFundercallsthis.options.igp.getContracts(chain).interchainGasPaymaster, it's receiving the address that already prefers config. The implementation matches the PR's intended behavior.Likely an incorrect or invalid review comment.
typescript/keyfunder/src/config/types.ts (4)
10-15: Balance validation regex addresses the PR concern nicely.The regex
^(?:\d+)(?:\.\d{1,18})?$properly rejects scientific notation like "1e3" while allowing valid decimal strings thatparseEthercan handle. This directly addresses the reviewer concern about runtime parse failures from scientific notation slipping through. The 18 decimal limit matches ETH's precision.
32-67: SweepConfigSchema refinements are well structured.The three refinements ensure:
- Address is required when enabled
- Threshold is required when enabled
- Trigger multiplier maintains minimum gap from target multiplier
This addresses the PR comment about sweep config safety - no more silent fallback to a hard-coded default address. The validation is clear about what's needed.
81-106: Cross-schema validation for role references is a nice touch.The refinement ensures that any role referenced in chain balances actually exists in the
rolessection. This prevents typos and configuration drift from causing silent failures at runtime. The error message is clear about what went wrong.
116-120: ResolvedKeyConfig interface is clean and straightforward.Clear interface for the resolved key data that gets passed to funding operations. Having
desiredBalanceas a string here means it gets parsed to BigNumber at the point of use, which is fine.
paulbalaji
force-pushed
the
pbio/keyfunder-package
branch
from
d7cf85d to
affd9b6
Compare
paulbalaji
force-pushed
the
pbio/keyfunder-package
branch
2 times, most recently
from
c18278d to
d5cc001
Compare
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Fix all issues with AI agents
In @.gitignore:
- Around line 45-49: Update the .gitignore entry for Yarn to explicitly target
the directory by replacing or adding the directory-form entry (change ".yarn" to
".yarn/") so it clearly ignores the .yarn folder rather than ambiguously
matching files named ".yarn"; keep the existing ".opencode/" entry as-is.
In @typescript/keyfunder/Dockerfile:
- Around line 58-67: The Alpine runner (FROM node:20-alpine) will break when
installing @google-cloud/pino-logging-gcp-config because it pulls glibc gRPC
binaries; either switch the runner to a glibc-based image (e.g., node:20-slim)
or keep Alpine and install build deps so the native modules can be rebuilt: add
apk packages (python3, g++, make, ca-certificates) in the runner stage before
the npm install of @google-cloud/pino-logging-gcp-config (refer to the runner
stage lines with FROM node:20-alpine, WORKDIR /app, and the subsequent RUN/npm
install step) so the package builds correctly on Alpine.
In @typescript/keyfunder/src/core/KeyFunder.ts:
- Around line 255-268: In calculateFundingAmount, avoid BigNumber underflow by
computing delta only when currentBalance is less than desiredBalance: first
compute minDelta as before, then set delta = currentBalance.lt(desiredBalance) ?
desiredBalance.sub(currentBalance) : BigNumber.from(0), and finally return
delta.gt(minDelta) ? delta : BigNumber.from(0); update references to
calculateFundingAmount, currentBalance, desiredBalance, MIN_DELTA_NUMERATOR,
MIN_DELTA_DENOMINATOR and multiProvider.getProvider.getBalance accordingly.
- Around line 54-65: The current fundChainWithTimeout uses Promise.race so
fundChain(chain) keeps running after a timeout; change it to perform a hard stop
by creating an AbortController here and either pass its signal into fundChain
(update fundChain(chain, signal?: AbortSignal)) and all provider/tx calls to
respect the signal, or implement per-call timeouts inside fundChain that check
an injected AbortSignal; on timeout call controller.abort() (instead of only
relying on the timeout promise) and ensure createTimeoutPromise cleanup is wired
to abort the controller so background txs are canceled when
CHAIN_FUNDING_TIMEOUT_MS elapses.
In @typescript/keyfunder/src/metrics/Metrics.ts:
- Around line 1-113: The Pushgateway is constructed with an incorrect second
argument (an empty array) in the KeyFunderMetrics constructor; replace the empty
array with undefined (or an empty object) when calling new
Pushgateway(config.pushGateway, ..., this.registry) so it matches prom-client's
expected signature — update the constructor where Pushgateway is assigned (the
Pushgateway instantiation that checks config?.pushGateway) to call new
Pushgateway(config.pushGateway, undefined, this.registry) (or new
Pushgateway(config.pushGateway, {}, this.registry)).
In @typescript/keyfunder/src/service.ts:
- Around line 124-145: When calling funder.fundAllChains() and then
metrics.push(), do not let errors from metrics.push() overwrite an existing
fundingError and do not log "Metrics pushed to gateway" when push() is a no-op:
capture the funding error from funder.fundAllChains() into fundingError as you
do now, then call metrics.push() inside its own try/catch storing any error into
a separate metricsError variable and a boolean pushed flag (e.g., pushed = true
only if push completed and actually did work); only call logger.info('Metrics
pushed to gateway') when pushed is true; after both attempts, if fundingError
exists throw fundingError, else if metricsError exists throw metricsError; keep
references to funder.fundAllChains(), metrics.push(), fundingError,
metricsError, pushed, and logger.info/logger.error to locate the changes.
🧹 Nitpick comments (7)
typescript/keyfunder/src/config/types.ts (3)
3-8: Address validation is format-only (no checksum).If you want “valid address” semantics (and not just “40 hex chars”), consider refining with an
isAddresshelper (EIP-55 aware) rather than regex-only.
70-89: PreferChainMaptyping for per-chain config (keeps things consistent).Right now
chains/chainsToSkipare plainrecord<string, …>/string[]. If the rest of the repo standardizes onChainMap, consider exportingKeyFunderConfig(or a wrapper type) withChainMap<ChainConfig>to avoid “random string” chain names creeping in. As per coding guidelines, useChainMapfor per-chain configurations in TypeScript.Also applies to: 82-107
90-107: Refine error could be more actionable.The cross-role refine returns a generic message; if you can, include which chain + role was invalid (even just in
message) to speed up config debugging.typescript/keyfunder/Dockerfile (1)
9-16: Foundry install method differs from the repo’s usual pattern—confirm this is intentional.If you want consistency with other images, consider the
foundryupflow; if you want strict reproducibility, the pinned tarball approach is understandable—just make sure it’s the chosen standard here. Based on learnings, standardize Foundry installation via the official installer where Foundry is installed.typescript/keyfunder/src/index.ts (1)
1-26: Consider whethercalculateMultipliedBalanceshould be public API.Everything else here reads like a clean package surface; exporting the balance-multiplier helper could lock you into supporting it long-term. If it’s meant to stay internal, keep it unexported (or tuck it under a
utilsexport).typescript/keyfunder/src/service.ts (1)
148-165: PreferChainMapfor per-chain maps + consider returning a new map instead of mutating.Repo guideline says
ChainMapfor per-chain configs;applyRpcOverridescurrently takes aRecord<string, Partial<ChainMetadata>>and mutates it in place. Not broken, just easier to reason about if it’s typed asChainMap<Partial<ChainMetadata>>(orChainMap<ChainMetadata>) and/or returns the updated object. As per coding guidelines, useChainMapfor per-chain configurations in TypeScript.typescript/keyfunder/src/core/KeyFunder.ts (1)
183-207: Avoid the double balance read (and the tiny TOCTOU wobble).
calculateFundingAmount()already fetchescurrentBalance, thenfundKey()fetches it again. Consider returning{ currentBalance, fundingAmount }fromcalculateFundingAmount(or passcurrentBalanceinto it) so you only hit the RPC once.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📥 Commits
Reviewing files that changed from the base of the PR and between d7cf85d3ff1d0b1f3a4445e63e432551cd9a0a0e and e835fac.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (21)
.github/workflows/keyfunder-docker.yml.gitignoreDockerfiletypescript/keyfunder/.gitignoretypescript/keyfunder/.mocharc.jsontypescript/keyfunder/Dockerfiletypescript/keyfunder/README.mdtypescript/keyfunder/eslint.config.mjstypescript/keyfunder/package.jsontypescript/keyfunder/scripts/ncc.post-bundle.mjstypescript/keyfunder/src/config/KeyFunderConfig.test.tstypescript/keyfunder/src/config/KeyFunderConfig.tstypescript/keyfunder/src/config/types.test.tstypescript/keyfunder/src/config/types.tstypescript/keyfunder/src/core/KeyFunder.tstypescript/keyfunder/src/index.tstypescript/keyfunder/src/metrics/Metrics.test.tstypescript/keyfunder/src/metrics/Metrics.tstypescript/keyfunder/src/service.tstypescript/keyfunder/tsconfig.jsontypescript/keyfunder/turbo.json
✅ Files skipped from review due to trivial changes (1)
- typescript/keyfunder/tsconfig.json
🚧 Files skipped from review as they are similar to previous changes (10)
- typescript/keyfunder/.gitignore
- typescript/keyfunder/turbo.json
- typescript/keyfunder/package.json
- typescript/keyfunder/eslint.config.mjs
- typescript/keyfunder/src/metrics/Metrics.test.ts
- .github/workflows/keyfunder-docker.yml
- typescript/keyfunder/src/config/KeyFunderConfig.test.ts
- typescript/keyfunder/scripts/ncc.post-bundle.mjs
- typescript/keyfunder/src/config/types.test.ts
- typescript/keyfunder/src/config/KeyFunderConfig.ts
🧰 Additional context used
📓 Path-based instructions (1)
typescript/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
typescript/**/*.{ts,tsx}: UseChainMapfor per-chain configurations in TypeScript
Import types from@hyperlane-xyz/sdkwhen using TypeScript SDK types
Files:
typescript/keyfunder/src/index.tstypescript/keyfunder/src/service.tstypescript/keyfunder/src/core/KeyFunder.tstypescript/keyfunder/src/metrics/Metrics.tstypescript/keyfunder/src/config/types.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.
Learnt from: xeno097
Repo: hyperlane-xyz/hyperlane-monorepo PR: 7253
File: typescript/infra/scripts/funding/fund-wallet-from-deployer-key.ts:285-289
Timestamp: 2025-10-24T18:58:44.557Z
Learning: In the Hyperlane funding script (typescript/infra/scripts/funding/fund-wallet-from-deployer-key.ts), tokens with 0 decimals are not valid and should be rejected. The truthy check on decimals is intentional and correct for this use case.
Learnt from: paulbalaji
Repo: hyperlane-xyz/hyperlane-monorepo PR: 6891
File: typescript/infra/config/environments/mainnet3/funding.ts:22-22
Timestamp: 2025-08-13T16:53:55.163Z
Learning: In Hyperlane mainnet3 configs, funding.ts uses 'gcr.io/abacus-labs-dev/hyperlane-monorepo' docker image while agent.ts uses 'gcr.io/abacus-labs-dev/hyperlane-agent' docker image. These are different images with independent tag cycles, so tag consistency across them is not expected.
📚 Learning: 2025-12-18T22:36:13.445Z
Learnt from: CR
Repo: hyperlane-xyz/hyperlane-monorepo PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-18T22:36:13.445Z
Learning: Applies to typescript/**/*.{ts,tsx} : Use `ChainMap` for per-chain configurations in TypeScript
Applied to files:
typescript/keyfunder/.mocharc.jsontypescript/keyfunder/src/core/KeyFunder.tstypescript/keyfunder/src/config/types.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/keyfunder/.mocharc.jsontypescript/keyfunder/src/config/types.ts
📚 Learning: 2025-12-18T22:36:13.445Z
Learnt from: CR
Repo: hyperlane-xyz/hyperlane-monorepo PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-18T22:36:13.445Z
Learning: Applies to typescript/**/*.{ts,tsx} : Import types from `hyperlane-xyz/sdk` when using TypeScript SDK types
Applied to files:
typescript/keyfunder/.mocharc.json
📚 Learning: 2025-08-13T16:53:55.163Z
Learnt from: paulbalaji
Repo: hyperlane-xyz/hyperlane-monorepo PR: 6891
File: typescript/infra/config/environments/mainnet3/funding.ts:22-22
Timestamp: 2025-08-13T16:53:55.163Z
Learning: In Hyperlane mainnet3 configs, funding.ts uses 'gcr.io/abacus-labs-dev/hyperlane-monorepo' docker image while agent.ts uses 'gcr.io/abacus-labs-dev/hyperlane-agent' docker image. These are different images with independent tag cycles, so tag consistency across them is not expected.
Applied to files:
typescript/keyfunder/Dockerfiletypescript/keyfunder/README.md
📚 Learning: 2025-12-29T19:45:12.592Z
Learnt from: paulbalaji
Repo: hyperlane-xyz/hyperlane-monorepo PR: 7545
File: typescript/infra/src/rebalancer/helm.ts:82-83
Timestamp: 2025-12-29T19:45:12.592Z
Learning: In the hyperlane-xyz/hyperlane-monorepo repository, Docker image tags in Helm managers (e.g., typescript/infra/src/rebalancer/helm.ts) are intentionally hardcoded for reproducibility and explicit version control, rather than using dynamically generated CI tags.
Applied to files:
typescript/keyfunder/Dockerfile
📚 Learning: 2025-12-22T16:50:19.890Z
Learnt from: paulbalaji
Repo: hyperlane-xyz/hyperlane-monorepo PR: 7565
File: typescript/ccip-server/Dockerfile:10-11
Timestamp: 2025-12-22T16:50:19.890Z
Learning: Standardize Foundry installation in Dockerfiles by using the official curl -L https://foundry.paradigm.xyz | bash followed by foundryup. This pattern is already used across multiple Dockerfiles in the repo and should be retained for consistency and reliability wherever Foundry is installed.
Applied to files:
typescript/keyfunder/DockerfileDockerfile
📚 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/keyfunder/Dockerfile.gitignore
📚 Learning: 2025-10-24T18:58:44.557Z
Learnt from: xeno097
Repo: hyperlane-xyz/hyperlane-monorepo PR: 7253
File: typescript/infra/scripts/funding/fund-wallet-from-deployer-key.ts:285-289
Timestamp: 2025-10-24T18:58:44.557Z
Learning: In the Hyperlane funding script (typescript/infra/scripts/funding/fund-wallet-from-deployer-key.ts), tokens with 0 decimals are not valid and should be rejected. The truthy check on decimals is intentional and correct for this use case.
Applied to files:
typescript/keyfunder/src/service.tstypescript/keyfunder/src/core/KeyFunder.ts
📚 Learning: 2025-12-16T17:23:10.420Z
Learnt from: paulbalaji
Repo: hyperlane-xyz/hyperlane-monorepo PR: 7627
File: typescript/infra/config/environments/mainnet3/gasPrices.json:10-13
Timestamp: 2025-12-16T17:23:10.420Z
Learning: In typescript/infra/config/environments/mainnet3/gasPrices.json, the "decimals" field represents operational precision for gas price calculations, which may differ from the standard token decimal precision defined in token registries. These are different types of precisions serving different purposes.
Applied to files:
typescript/keyfunder/src/core/KeyFunder.ts
📚 Learning: 2025-12-18T22:36:13.445Z
Learnt from: CR
Repo: hyperlane-xyz/hyperlane-monorepo PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-18T22:36:13.445Z
Learning: Most deployments are config-driven; check `typescript/infra/config/` for deployment configuration examples
Applied to files:
Dockerfile.gitignore
📚 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/keyfunder/src/config/types.ts
🪛 LanguageTool
typescript/keyfunder/README.md
[uncategorized] ~97-~97: The official name of this software platform is spelled with a capital “H”.
Context: ...gistry). Supports commit pinning (e.g., `github://hyperlane-xyz/hyperlane-registry/comm...
(GITHUB)
⏰ 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-rebalancer)
- GitHub Check: cli-evm-e2e-matrix (warp-extend-basic)
- GitHub Check: cli-evm-e2e-matrix (warp-init)
- GitHub Check: cli-evm-e2e-matrix (status)
- GitHub Check: cli-evm-e2e-matrix (core-read)
- GitHub Check: cli-evm-e2e-matrix (warp-deploy-2)
- GitHub Check: cli-evm-e2e-matrix (warp-bridge-1)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-rebalancing-config)
- GitHub Check: cli-evm-e2e-matrix (relay)
- GitHub Check: cli-evm-e2e-matrix (warp-check-4)
- 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-check-2)
- GitHub Check: cli-evm-e2e-matrix (warp-deploy-1)
- GitHub Check: cli-evm-e2e-matrix (warp-check-1)
- GitHub Check: cli-evm-e2e-matrix (core-check)
- GitHub Check: cli-evm-e2e-matrix (core-init)
- GitHub Check: cli-evm-e2e-matrix (warp-read)
- GitHub Check: cli-evm-e2e-matrix (warp-check-5)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-ism-updates)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-ownership-updates)
- GitHub Check: cli-evm-e2e-matrix (warp-check-3)
- GitHub Check: cli-evm-e2e-matrix (warp-extend-recovery)
- 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-apply-submitters)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-simple-updates)
- GitHub Check: cli-evm-e2e-matrix (core-apply)
- GitHub Check: cli-evm-e2e-matrix (core-deploy)
- GitHub Check: env-test-matrix (testnet4, sepolia, core)
- GitHub Check: env-test-matrix (mainnet3, arbitrum, core)
- GitHub Check: env-test-matrix (mainnet3, ethereum, igp)
- GitHub Check: cli-cosmos-e2e-matrix (core-deploy)
- GitHub Check: env-test-matrix (mainnet3, optimism, igp)
- GitHub Check: env-test-matrix (mainnet3, ethereum, core)
- GitHub Check: env-test-matrix (mainnet3, arbitrum, igp)
- GitHub Check: env-test-matrix (mainnet3, optimism, core)
- GitHub Check: cli-radix-e2e-matrix (warp-apply-ownership-updates)
- GitHub Check: cli-cosmos-e2e-matrix (warp-read)
- GitHub Check: cli-cosmos-e2e-matrix (warp-deploy)
- GitHub Check: cli-radix-e2e-matrix (warp-deploy)
- GitHub Check: cli-cosmos-e2e-matrix (core-apply)
- GitHub Check: cli-cosmos-e2e-matrix (core-read)
- GitHub Check: cli-radix-e2e-matrix (warp-apply-route-extension)
- GitHub Check: cli-install-test-run
- GitHub Check: cli-cosmos-e2e-matrix (core-check)
- GitHub Check: cli-radix-e2e-matrix (core-deploy)
- GitHub Check: cosmos-sdk-e2e-run
- GitHub Check: cli-radix-e2e-matrix (core-apply)
- GitHub Check: aleo-sdk-e2e-run
- GitHub Check: pnpm-test-run
- GitHub Check: cli-cross-chain-e2e-matrix (warp-deploy)
- GitHub Check: cli-cross-chain-e2e-matrix (warp-apply)
- GitHub Check: infra-test
- GitHub Check: build-and-push-to-gcr
- GitHub Check: build-and-push-to-gcr
- GitHub Check: e2e-matrix (evm)
- GitHub Check: agent-configs (mainnet3)
- GitHub Check: lander-coverage
- GitHub Check: lint-rs
- GitHub Check: test-rs
🔇 Additional comments (12)
typescript/keyfunder/.mocharc.json (1)
1-3: Nice and simple, just the way I like it.This minimal Mocha config does exactly what it needs to — loads TSX so your TypeScript tests can run without any extra compilation fuss. Like a good onion, it's got just the layer it needs right now. If you ever need more (timeouts, file patterns, reporter settings), you can always add 'em later.
typescript/keyfunder/src/config/types.ts (3)
10-16: BalanceStringSchema regex is parseEther-safe (nice).This blocks scientific notation like
1e3, which avoids runtime surprises when parsing. Might be worth calling out explicitly in docs, too.
27-68: Sweep config validation is strict in the right places.Requiring
addressandthresholdwhenenabledis true is exactly the kind of guard that keeps the swamp from flooding later.
22-26: IGP address wiring is sound—schema and runtime are in sync.The code properly consumes
chains.<chain>.igp.addressfrom config inservice.ts(line 82), with the registry serving as intended fallback. The preference ordering is explicitly documented: config address takes precedence via the nullish coalescing operator, then falls back to registry if needed. KeyFunder then uses the wired-up IGP contract for balance checks and claims. No issues here.Dockerfile (1)
32-36: Good: keeps pnpm install cache-friendly for the new package.Adding
typescript/keyfunder/package.jsonhere fits the existing “copy manifests first” pattern.typescript/keyfunder/README.md (5)
61-84: Docs match schema shape; please confirm runtime uses the same shape.Especially
chains.<chain>.igp.addressandsweep.*requirements—if runtime falls back to registry or defaults, call that out here so operators aren’t guessing.
136-143: Funding threshold section is clear and consistent.The 40% trigger + “top up to desired” example is easy to reason about.
157-160: Timeout value: verify it matches the implementation.If the code uses a different timeout (or makes it configurable), this line will age fast.
97-97: Minor doc nit: “GitHub” capitalization.
GitHub registryreads nicer thangithub registry.⛔ Skipped due to learnings
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.Learnt from: paulbalaji Repo: hyperlane-xyz/hyperlane-monorepo PR: 7720 File: typescript/keyfunder/package.json:22-22 Timestamp: 2026-01-07T17:37:43.421Z Learning: In the hyperlane-xyz/hyperlane-monorepo repository, hyperlane-xyz/registry should use "catalog:" as the dependency version following the monorepo pattern, not a pinned version.
85-89: No action needed—multiplier precision is properly enforced.The README accurately describes the actual behavior. The
calculateMultipliedBalance()function atKeyFunder.ts:350-355implements exactly what's documented:Math.floor(multiplier * 100).div(100)floors to 2 decimal places. Tests confirm this (e.g.,1.555becomes1.55, not1.56).The schema doesn't restrict input to 2 decimals, but that's fine—the calculation itself enforces the floor. The documentation is precise and truthful.
typescript/keyfunder/src/service.ts (1)
63-110: IGP address resolution looks solid (config first, registry fallback).This matches the intended behavior and avoids silently ignoring
chains.<chain>.igp.address.typescript/keyfunder/src/core/KeyFunder.ts (1)
13-15: Funding threshold is “fund when current < 40% of desired” — make sure docs match.With
MIN_DELTA = 0.6 * desiredandfund if delta > minDelta, this funds only when the wallet is below ~40% of desired. That’s fine—just make sure README/config docs state the same threshold so nobody gets surprised later.Also applies to: 255-268
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
typescript/keyfunder/src/core/KeyFunder.ts (1)
183-214: Two balance fetches where one would do, donkey.The
calculateFundingAmountmethod fetches the balance (line 260-262), and thenfundKeyfetches it again (lines 197-199). This is redundant and could lead to inconsistent state if the balance changes between calls.Consider refactoring to fetch once and pass the balance as a parameter.
♻️ Suggested refactor to fetch balance once
private async fundKey(chain: string, key: ResolvedKeyConfig): Promise<void> { const logger = this.options.logger.child({ chain, address: key.address, role: key.role, }); const desiredBalance = ethers.utils.parseEther(key.desiredBalance); - const fundingAmount = await this.calculateFundingAmount( - chain, - key.address, - desiredBalance, - ); - - const currentBalance = await this.multiProvider + const currentBalance = await this.multiProvider .getProvider(chain) .getBalance(key.address); + const fundingAmount = this.calculateFundingAmount( + currentBalance, + desiredBalance, + );And update
calculateFundingAmountto be synchronous:- private async calculateFundingAmount( - chain: string, - address: string, + private calculateFundingAmount( + currentBalance: BigNumber, desiredBalance: BigNumber, - ): Promise<BigNumber> { - const currentBalance = await this.multiProvider - .getProvider(chain) - .getBalance(address); + ): BigNumber { if (currentBalance.gte(desiredBalance)) { return BigNumber.from(0); } // ... rest unchanged }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
typescript/keyfunder/src/core/KeyFunder.ts
🧰 Additional context used
📓 Path-based instructions (1)
typescript/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
typescript/**/*.{ts,tsx}: UseChainMapfor per-chain configurations in TypeScript
Import types from@hyperlane-xyz/sdkwhen using TypeScript SDK types
Files:
typescript/keyfunder/src/core/KeyFunder.ts
🧠 Learnings (8)
📓 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: xeno097
Repo: hyperlane-xyz/hyperlane-monorepo PR: 7253
File: typescript/infra/scripts/funding/fund-wallet-from-deployer-key.ts:285-289
Timestamp: 2025-10-24T18:58:44.557Z
Learning: In the Hyperlane funding script (typescript/infra/scripts/funding/fund-wallet-from-deployer-key.ts), tokens with 0 decimals are not valid and should be rejected. The truthy check on decimals is intentional and correct for this use case.
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-18T22:36:13.445Z
Learnt from: CR
Repo: hyperlane-xyz/hyperlane-monorepo PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-18T22:36:13.445Z
Learning: Applies to typescript/**/*.{ts,tsx} : Use `ChainMap` for per-chain configurations in TypeScript
Applied to files:
typescript/keyfunder/src/core/KeyFunder.ts
📚 Learning: 2025-10-24T18:58:44.557Z
Learnt from: xeno097
Repo: hyperlane-xyz/hyperlane-monorepo PR: 7253
File: typescript/infra/scripts/funding/fund-wallet-from-deployer-key.ts:285-289
Timestamp: 2025-10-24T18:58:44.557Z
Learning: In the Hyperlane funding script (typescript/infra/scripts/funding/fund-wallet-from-deployer-key.ts), tokens with 0 decimals are not valid and should be rejected. The truthy check on decimals is intentional and correct for this use case.
Applied to files:
typescript/keyfunder/src/core/KeyFunder.ts
📚 Learning: 2025-12-16T17:23:10.420Z
Learnt from: paulbalaji
Repo: hyperlane-xyz/hyperlane-monorepo PR: 7627
File: typescript/infra/config/environments/mainnet3/gasPrices.json:10-13
Timestamp: 2025-12-16T17:23:10.420Z
Learning: In typescript/infra/config/environments/mainnet3/gasPrices.json, the "decimals" field represents operational precision for gas price calculations, which may differ from the standard token decimal precision defined in token registries. These are different types of precisions serving different purposes.
Applied to files:
typescript/keyfunder/src/core/KeyFunder.ts
📚 Learning: 2025-07-22T22:21:54.918Z
Learnt from: xeno097
Repo: hyperlane-xyz/hyperlane-monorepo PR: 6746
File: typescript/sdk/src/timelock/evm/EvmTimelockReader.ts:155-166
Timestamp: 2025-07-22T22:21:54.918Z
Learning: When reviewing RPC call optimizations in timelock-related code, be mindful that parallel requests using Promise.all can cause rate limiting issues with public RPC endpoints. Sequential processing, while slower, is often preferred for reliability and to avoid overwhelming RPC providers.
Applied to files:
typescript/keyfunder/src/core/KeyFunder.ts
📚 Learning: 2025-10-08T11:26:26.590Z
Learnt from: paulbalaji
Repo: hyperlane-xyz/hyperlane-monorepo PR: 7035
File: typescript/infra/src/utils/squads.ts:140-144
Timestamp: 2025-10-08T11:26:26.590Z
Learning: In the sqds/multisig SDK for Solana squads, proposals start with transactionIndex 1 (not 0). When iterating through proposal indices, the lower bound should be Math.max(1, ...) to avoid scanning invalid index 0.
Applied to files:
typescript/keyfunder/src/core/KeyFunder.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/keyfunder/src/core/KeyFunder.ts
📚 Learning: 2025-08-26T13:46:37.695Z
Learnt from: paulbalaji
Repo: hyperlane-xyz/hyperlane-monorepo PR: 6943
File: rust/main/config/mainnet_config.json:965-965
Timestamp: 2025-08-26T13:46:37.695Z
Learning: In the repository hyperlane-xyz/hyperlane-monorepo, skip reviewing the file rust/main/config/mainnet_config.json in future code reviews as requested by paulbalaji.
Applied to files:
typescript/keyfunder/src/core/KeyFunder.ts
⏰ 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-read)
- GitHub Check: cli-evm-e2e-matrix (warp-check-4)
- GitHub Check: cli-evm-e2e-matrix (warp-check-5)
- GitHub Check: cli-evm-e2e-matrix (warp-init)
- GitHub Check: cli-evm-e2e-matrix (warp-rebalancer)
- GitHub Check: cli-evm-e2e-matrix (warp-send)
- GitHub Check: cli-evm-e2e-matrix (warp-check-3)
- GitHub Check: cli-evm-e2e-matrix (warp-deploy-2)
- GitHub Check: cli-evm-e2e-matrix (warp-check-2)
- GitHub Check: cli-evm-e2e-matrix (core-apply)
- 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-extend-config)
- GitHub Check: cli-evm-e2e-matrix (warp-deploy-1)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-ownership-updates)
- GitHub Check: cli-evm-e2e-matrix (warp-extend-recovery)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-submitters)
- GitHub Check: cli-evm-e2e-matrix (status)
- GitHub Check: cli-evm-e2e-matrix (core-read)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-simple-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-deploy)
- GitHub Check: cli-evm-e2e-matrix (warp-bridge-1)
- GitHub Check: cli-evm-e2e-matrix (warp-bridge-2)
- GitHub Check: cli-evm-e2e-matrix (core-init)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-ism-updates)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-hook-updates)
- GitHub Check: cli-evm-e2e-matrix (core-check)
- GitHub Check: cli-cosmos-e2e-matrix (core-apply)
- GitHub Check: env-test-matrix (testnet4, sepolia, core)
- GitHub Check: env-test-matrix (mainnet3, arbitrum, core)
- GitHub Check: env-test-matrix (mainnet3, ethereum, igp)
- GitHub Check: env-test-matrix (mainnet3, optimism, core)
- GitHub Check: env-test-matrix (mainnet3, optimism, igp)
- GitHub Check: env-test-matrix (mainnet3, arbitrum, igp)
- GitHub Check: env-test-matrix (mainnet3, ethereum, core)
- GitHub Check: cli-cosmos-e2e-matrix (core-deploy)
- GitHub Check: cli-cosmos-e2e-matrix (warp-deploy)
- GitHub Check: cli-radix-e2e-matrix (warp-deploy)
- GitHub Check: cli-cosmos-e2e-matrix (warp-read)
- GitHub Check: radix-sdk-e2e-run
- GitHub Check: cli-cosmos-e2e-matrix (core-check)
- GitHub Check: cli-radix-e2e-matrix (warp-apply-ownership-updates)
- GitHub Check: cli-cosmos-e2e-matrix (core-read)
- GitHub Check: cli-radix-e2e-matrix (warp-apply-route-extension)
- GitHub Check: cli-radix-e2e-matrix (core-apply)
- GitHub Check: cli-radix-e2e-matrix (core-deploy)
- GitHub Check: aleo-sdk-e2e-run
- GitHub Check: cli-cross-chain-e2e-matrix (warp-deploy)
- GitHub Check: cli-cross-chain-e2e-matrix (warp-apply)
- GitHub Check: cosmos-sdk-e2e-run
- GitHub Check: pnpm-test-run
- GitHub Check: cli-install-test-run
- GitHub Check: infra-test
- GitHub Check: agent-configs (testnet4)
- GitHub Check: e2e-matrix (evm)
- GitHub Check: agent-configs (mainnet3)
- GitHub Check: test-rs
- GitHub Check: lander-coverage
- GitHub Check: lint-rs
🔇 Additional comments (9)
typescript/keyfunder/src/core/KeyFunder.ts (9)
1-16: Lookin' good, these imports and constants are tidy as me swamp.The constants for the delta threshold (6/10 = 60%) and timeout are well-defined. Clear and straightforward setup.
18-30: Simple and clean, just the way I like it.The options interface gives good flexibility with optional metrics and IGP handling. Constructor's straightforward.
32-52: Nice use ofPromise.allSettledhere.This lets all chains attempt funding even if some fail, and the error aggregation gives good visibility into partial failures. Solid approach.
54-102: The timeout handling is proper, not like some half-baked onion ring.Good use of
Promise.racewith proper cleanup infinallyto avoid timer leaks. ThefundChainmethod orchestrates the flow nicely with appropriate logging.
135-172: The IGP contract address comes from registry, not config.Now, look here - the code fetches the IGP contract from
this.options.igp.getContracts(chain)(line 147), but the config schema requireschains.<chain>.igp.address. That config value gets ignored completely.Per the PR discussion, consider either:
- Using
chainConfig.igp.addressto initialize per-chain IGP with registry as fallback, or- Removing
igp.addressfrom the config schema if it's truly not needed
255-271: The funding threshold math checks out.Just so we're all on the same page in this swamp: with
minDelta = desired * 6/10, funding only happens whendelta > 60%of desired, meaning balance must be below 40% of desired. Per PR discussion, the README should be updated to reflect this "<40%" behavior rather than the current "below 60%" wording.
273-346: Good validation on the sweep config, no more silent defaults.The explicit error when
addressorthresholdis missing (lines 284-288) addresses the concern about silently falling back to defaults. That's proper.One small thing to ponder: if
targetMultiplier > triggerMultiplier(unusual config),sweepAmounton line 314 could go negative sincetargetBalance > funderBalanceis possible even whenfunderBalance > triggerThreshold. Might want to add a guard or document thattriggerMultiplier >= targetMultiplieris expected.
349-358: Neat utility with proper documentation.The comment explaining the floor behavior is helpful. This handles typical multiplier values well.
360-372: Proper timeout pattern with cleanup, no leftover timers lurking about.Returning the cleanup function and using it in a
finallyblock is the right way to handle this. Prevents memory leaks and stray rejections.
paulbalaji
force-pushed
the
pbio/keyfunder-package
branch
from
f275ee5 to
5e465d7
Compare
paulbalaji
force-pushed
the
pbio/unified-docker-workflows
branch
from
0815fbe to
edefef1
Compare
paulbalaji
force-pushed
the
pbio/keyfunder-package
branch
3 times, most recently
from
17a7a86 to
a23aa33
Compare
paulbalaji
force-pushed
the
pbio/keyfunder-package
branch
2 times, most recently
from
ce42b5e to
38cbb55
Compare
paulbalaji
force-pushed
the
pbio/keyfunder-package
branch
from
38cbb55 to
d3e450c
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@typescript/keyfunder/README.md`:
- Around line 108-114: Update the Docker image name in the example docker run
snippet to match the build tag: replace any occurrence of
"gcr.io/abacus-labs-dev/hyperlane-keyfunder:latest" with the correct image name
"gcr.io/abacus-labs-dev/hyperlane-key-funder:latest" so the example's docker run
command and KEYFUNDER_CONFIG_FILE usage reference the same built image.
🧹 Nitpick comments (2)
typescript/keyfunder/src/service.ts (1)
148-163: Drop the type assertion onrpcUrls.You can keep this typed without
asby assigning a typed local and spreading back into the partial.As per coding guidelines, avoid unnecessary type casts (`as` assertions), especially in TypeScript.♻️ Proposed tweak
- chainMetadata[chain].rpcUrls = [ - { http: rpcUrl }, - ] as ChainMetadata['rpcUrls']; + const rpcUrls: ChainMetadata['rpcUrls'] = [{ http: rpcUrl }]; + chainMetadata[chain] = { + ...chainMetadata[chain], + rpcUrls, + };typescript/keyfunder/src/core/KeyFunder.ts (1)
32-40: Consider limiting chain funding concurrency to avoid RPC rate limits.Running all chains in parallel can overload public RPCs; a simple concurrency cap (or sequential option) would make this more reliable in the wild.
Based on learnings, when reviewing RPC call optimizations in timelock-related code, be mindful that parallel requests using Promise.all can cause rate limiting issues with public RPC endpoints. Sequential processing, while slower, is often preferred for reliability and to avoid overwhelming RPC providers.
The schema already required `igp.address`. README and service code implied it was optional with registry fallback. Simplified IGP initialization from ~40 lines to ~10 by removing unnecessary registry.getAddresses() call and fallback logic. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
paulbalaji
force-pushed
the
pbio/keyfunder-package
branch
from
54e71b7 to
7641761
Compare
🐳 Monorepo Docker Image Built Successfully
Full image paths |
⚙️ Node Service Docker Images Built Successfully
Full image paths |
3 participants
Summary
Extracts the key funding functionality from the monolithic
typescript/infrainto a standalone@hyperlane-xyz/keyfundernpm package that can be deployed independently as a Docker container.Motivation
The existing keyfunder script (
fund-keys-from-deployer.ts, 1,207 lines) was tightly coupled to the infra package, requiring the full monorepo image and GCP SDK for secret resolution. This refactor:Changes
New Package (
typescript/keyfunder/)src/core/KeyFunder.tssrc/config/types.tssrc/service.tssrc/metrics/Metrics.ts@hyperlane-xyz/metricspackageDockerfileFunding Operations
Configuration Example
Environment Variables
KEYFUNDER_CONFIG_FILEHYP_KEYRPC_URL_<CHAIN>REGISTRY_URITesting
pnpm -C typescript/keyfunder test pnpm -C typescript/keyfunder buildRelated
Summary by CodeRabbit
New Features
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.