Skip to content

fix(infra): include all chains in keyfunder config#7957

Merged
paulbalaji merged 3 commits intomainfrom
fix/keyfunder-missing-chains
Jan 30, 2026
Merged

fix(infra): include all chains in keyfunder config#7957
paulbalaji merged 3 commits intomainfrom
fix/keyfunder-missing-chains

Conversation

@paulbalaji
Copy link
Collaborator

@paulbalaji paulbalaji commented Jan 30, 2026

Summary

  • Fixed bug where keyfunder skipped chains not in CHAINS_TO_SWEEP, even if they had balance configs
  • Mainnet: chains like incentiv with balances but no sweep were being excluded
  • Testnet: ALL chains were excluded (testnet chain names like arbitrumsepolia don't match mainnet names in CHAINS_TO_SWEEP)

Changes

  • Sweep config is now only added for chains in CHAINS_TO_SWEEP
  • All chains with valid config (balances, igp, or sweep) are included in the keyfunder output

Test plan

  • Verify keyfunder configmap includes incentiv after deployment
  • Verify testnet keyfunder still generates valid config

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Added validation to ensure sweep thresholds are finite and positive; invalid sweep configs now trigger an error to prevent improper assignments.
  • Improvements

    • Sweep handling is consolidated into a single, validated flow and now only applies to eligible chains.
    • Non-sweep settings (balances and gas-related fields) continue to apply independently; sweep address and multiplier selection is clearer and more robust.

✏️ Tip: You can customize this high-level summary in your review settings.

…ains

The keyfunder was skipping chains not in CHAINS_TO_SWEEP entirely,
even if they had balance configs. This broke:
- Mainnet: chains like incentiv with balances but no sweep
- Testnet: ALL chains (testnet names don't match CHAINS_TO_SWEEP)

Now sweep config is only added for CHAINS_TO_SWEEP chains, but all
chains with valid config (balances, igp, or sweep) are included.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@github-project-automation github-project-automation bot moved this to In Review in Hyperlane Tasks Jan 30, 2026
@paulbalaji paulbalaji marked this pull request as ready for review January 30, 2026 12:13
@paulbalaji paulbalaji enabled auto-merge January 30, 2026 12:14
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 30, 2026

📝 Walkthrough

Walkthrough

Sweep assignment is now restricted to chains in CHAINS_TO_SWEEP and only added after verifying lowUrgencyKeyFunderBalances[chain] is a finite, positive number; invalid values throw. When eligible, sweepConfig is populated using sweepOverrides or defaults; non-sweep fields continue to be applied independently.

Changes

Cohort / File(s) Summary
Sweep Configuration Validation & Population
typescript/infra/src/funding/key-funder.ts
Gated sweep creation to chains in CHAINS_TO_SWEEP; validate lowUrgencyKeyFunderBalances[chain] is finite and >0 or throw; populate sweepConfig with enabled, address (from sweepOverrides or default), threshold, and multipliers (overrides or defaults); removed previous unconditional sweep assignment.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • Mo-Hussain
  • tkporter
  • xeno097

Poem

Ah, chains now heed the keeper's call,
Balances checked, no sweeps that sprawl,
Addresses and thresholds snug and right,
Multipliers set before the night,
Clean config, no muck — that’s proper delight.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main fix: keyfunder now includes all chains in the config instead of skipping chains not in CHAINS_TO_SWEEP.
Description check ✅ Passed The PR description covers the bug, impact (mainnet and testnet), changes made, and test plan. It follows the template structure with Summary, Changes, and Test plan sections.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/keyfunder-missing-chains

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@typescript/infra/src/funding/key-funder.ts`:
- Around line 138-152: The current validation lets non-numeric strings like
"abc" or "1foo" pass because parseFloat can return a number from partial
strings; update the guard in the CHAINS_TO_SWEEP branch to strictly validate
this.config.lowUrgencyKeyFunderBalances?.[chain] by converting to a Number and
checking Number.isFinite(value) && value > 0 (or use a strict numeric regex on
the trimmed string) before assigning chainConfig.sweep; specifically change the
sweepThreshold handling around the sweepThreshold variable and the parseFloat
usage so only bona fide numeric, positive thresholds are accepted into
chainConfig.sweep.

paulbalaji and others added 2 commits January 30, 2026 12:17
Use Number() + Number.isFinite() instead of parseFloat() to reject
partial numeric strings like "1foo" that parseFloat would accept.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@paulbalaji paulbalaji added this pull request to the merge queue Jan 30, 2026
Merged via the queue into main with commit 57c4dd0 Jan 30, 2026
112 checks passed
@paulbalaji paulbalaji deleted the fix/keyfunder-missing-chains branch January 30, 2026 12:48
@github-project-automation github-project-automation bot moved this from In Review to Done in Hyperlane Tasks Jan 30, 2026
@codecov
Copy link

codecov bot commented Jan 30, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 77.02%. Comparing base (0515d2e) to head (53611ff).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #7957   +/-   ##
=======================================
  Coverage   77.02%   77.02%           
=======================================
  Files         117      117           
  Lines        2651     2651           
  Branches      244      244           
=======================================
  Hits         2042     2042           
  Misses        593      593           
  Partials       16       16           
Components Coverage Δ
core 87.80% <ø> (ø)
hooks 71.86% <ø> (ø)
isms 81.10% <ø> (ø)
token 86.67% <ø> (ø)
middlewares 84.98% <ø> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants