feat(rebalancer): use MultiProtocolCore for message delivery checks#7991
feat(rebalancer): use MultiProtocolCore for message delivery checks#7991Mo-Hussain merged 4 commits intomainfrom
Conversation
Switched ActionTracker from HyperlaneCore (EVM-only) to MultiProtocolCore to support delivery checks across all VM types. - RebalancerContextFactory validates mailbox exists in registry addresses - ActionTracker uses adapter.isDelivered() instead of mailbox.delivered() - Updated tests to use adapter pattern Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe rebalancer now uses MultiProtocolCore (via a MultiProtocolProvider) instead of HyperlaneCore for delivery checks, validates mailbox registry addresses at startup, and centralizes per-destination blockTag resolution via a new getConfirmedBlockTag helper. Changes
Sequence Diagram(s)sequenceDiagram
participant Factory as RebalancerContextFactory
participant Provider as MultiProtocolProvider
participant Core as MultiProtocolCore
participant Adapter as CoreAdapter/Mailbox
participant Tracker as ActionTracker
Factory->>Provider: request extendedMultiProtocolProvider / addresses map
Provider-->>Factory: extendedMultiProtocolProvider, chain names
Factory->>Factory: validate mailbox registry addresses (startup)
Factory->>Core: MultiProtocolCore.fromAddressesMap(addresses, provider)
Factory-->>Tracker: construct with multiProtocolCore
Tracker->>Core: adapter(chainName).isDelivered(messageId, blockTag)
Core->>Adapter: mailbox.isDelivered(messageId, blockTag)
Adapter-->>Core: boolean delivered
Core-->>Tracker: delivered status
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@typescript/rebalancer/src/factories/RebalancerContextFactory.ts`:
- Around line 232-250: The code extends MultiProtocolProvider in create() but
only stores the base this.multiProvider, causing rebuilds (e.g., in
createActionTracker()) to lose extended mailbox metadata; modify the class to
store the extended provider (e.g., this.extendedMultiProvider) when you call
MultiProtocolProvider.fromMultiProvider and use that stored extended provider
everywhere you currently call MultiProtocolProvider.fromMultiProvider or rebuild
the provider (notably in createActionTracker()), ensuring
MultiProtocolCore.fromAddressesMap and other consumers use the preserved
extended instance rather than reconstructing from this.multiProvider.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7991 +/- ##
=======================================
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:
|
… metadata The extended MultiProtocolProvider (with mailbox metadata for Sealevel warp adapters) was being lost when createActionTracker() rebuilt it from scratch. Now stored in constructor and reused. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Review SummaryNice architectural improvement - switching to 1. Startup Reorg Safety Regression (Medium)
Old behavior had a fallback: const blockTag = providedBlockTag ?? (await this.getConfirmedBlockTag(chainName));New behavior just passes Suggestion: Either pass computed tags into 2. Registry Scope Over-Validation (Medium)
const registryAddresses = await this.registry.getAddresses(); // ALL chains
objMap(registryAddresses, (chain, addrs) => {
if (!addrs.mailbox) throw new Error(...); // Fails on unrelated chains
});Suggestion: Filter to 3. Minor: Manual Test IncompletePR checklist shows manual multi-VM test not done. Given this enables Sealevel/Cosmos support, would be good to validate before merge. Overall the implementation is solid - these are refinements rather than blockers. The third commit correctly preserves the extended |
…stry scope - Created shared getConfirmedBlockTag utility for computing reorg-safe block tags - ActionTracker.getConfirmedBlockTag now computes on-demand during initialize() when no Monitor event has provided cached tags yet, fixing startup reorg safety - Monitor uses shared utility, removing duplicate code - RebalancerContextFactory.createActionTracker now only fetches/validates registry addresses for warp route chains instead of all registry chains Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
⚙️ Node Service Docker Images Built Successfully
Full image paths |
Summary
HyperlaneCore(EVM-only) toMultiProtocolCoreto support delivery checks across all VM typesadapter.isDelivered()pattern instead of directmailbox.delivered()callTest plan
pnpm -C typescript/rebalancer buildpassespnpm -C typescript/rebalancer testpasses (172 tests)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests