feat(rebalancer-sim): Add simulation harness for warp route rebalancer testing#7903
Conversation
Fast real-time simulation framework for testing rebalancers with: - Single-anvil multi-domain deployments - Mock bridges with configurable delays (500ms) and failures - Scenario generation (explicit, random, historic) - KPI tracking (completion rate, latency, costs) - Rebalancer-agnostic observation (JSON-RPC only) Implementation phases: 1. Foundation - Multi-domain deployment 2. Bridge mocking - Async delivery with setTimeout 3. Rebalancer integration - Daemon mode with fast polling 4. Scenario generation - Predefined + historic fetcher tool 5. Simulation engine - Real-time event orchestration 6. KPI collection - Metrics and reporting 7. Harness API - runSimulation, compareRebalancers 8. Advanced - Failure testing, explorer mock, visualization Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…r testing New package for fast real-time simulation testing of rebalancers: - Multi-domain deployment on single anvil instance - BridgeMockController with configurable delays, failures, fees - HyperlaneRunner rebalancer implementation - Scenario generation (unidirectional, random, imbalance, surge) - KPI collection (latencies, completion rates, rebalance volumes) - SimulationEngine orchestrating transfers, bridge delivery, rebalancer - RebalancerSimulationHarness top-level API Uses separate anvil accounts for deployer, rebalancer, and bridge controller to prevent nonce collisions during concurrent operations. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds a complete TypeScript rebalancer simulation package: tooling/config, scenario catalog and generator, Anvil multi‑domain deployment and mocks, bridge and message simulators, KPI collection, multiple rebalancer runners, simulation engine and harness, visualizer, tests, CI workflow, and supporting scripts — all new files and no production API regressions. Changes
Sequence Diagram(s)sequenceDiagram
participant Test as Test Suite
participant Deploy as SimulationDeployment
participant Engine as SimulationEngine
participant Bridge as BridgeMockController
participant Tracker as MessageTracker
participant KPI as KPICollector
participant Rebalancer as RebalancerRunner
Test->>Deploy: deployMultiDomainSimulation()
Deploy-->>Test: MultiDomainDeploymentResult
Test->>Engine: new SimulationEngine(deployment)
Test->>Engine: runSimulation(scenario, rebalancer, config)
Engine->>Bridge: start()
Engine->>KPI: initialize()
Engine->>Tracker: initialize()
Engine->>Rebalancer: initialize()/start()
Engine->>Engine: executeTransfers(scenario)
Engine->>Bridge: transferRemote() per transfer
Bridge->>Bridge: schedule delivery (delay/jitter/failure)
Bridge-->>Tracker: notify/tracked message
Engine->>KPI: recordTransferStart()
par Rebalancer flow
Rebalancer->>Rebalancer: poll & compute rebalances
Rebalancer-->>Engine: submit rebalance txs
Engine->>KPI: recordRebalanceStart/Complete()
and Tracker flow
Tracker->>Tracker: processReadyMessages()
Tracker-->>Engine: deliver inbound messages
Engine->>KPI: recordTransferComplete()
end
Engine->>KPI: generateKPIs()
Engine-->>Test: SimulationResult
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Important Action Needed: IP Allowlist UpdateIf your organization protects your Git platform with IP whitelisting, please add the new CodeRabbit IP address to your allowlist:
Failure to add the new IP will result in interrupted reviews. 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: 10
🤖 Fix all issues with AI agents
In @.claude/rebalancer-simulation-plan.md:
- Around line 9-17: The markdown has several fenced code blocks (the ASCII
diagram block containing "TestHarness → ScenarioEngine … KPICollector" and the
other blocks between lines 39–70 and 74–77) that lack language identifiers and
trigger markdownlint MD040; update each triple-backtick fence to include an
appropriate language tag (e.g., mark the diagram/ASCII blocks as text and any
code snippets as typescript or the actual language) so the fences become ```text
or ```typescript etc., preserving the block contents and ensuring MD040 is
satisfied.
In `@typescript/rebalancer-sim/eslint.config.mjs`:
- Around line 5-15: Merge the standalone files pattern object and the rules
object into a single ESLint flat-config object so the rules are scoped to
'./src/**/*.ts'; specifically, combine the existing { files: ['./src/**/*.ts'] }
and the { rules: { 'no-restricted-imports': ['off'], 'no-console': ['off'] } }
into one object so the 'no-restricted-imports' and 'no-console' overrides only
apply to the specified files.
In `@typescript/rebalancer-sim/src/bridges/BridgeMockController.ts`:
- Around line 180-185: BridgeMockController currently computes only token fees
(tokenFeeBps) and ignores config.nativeFeeBps; update the fee logic to compute a
native fee when config.nativeFeeBps is present (e.g., const nativeFee = (amount
* BigInt(config.nativeFeeBps)) / BigInt(10000)), subtract or separately track it
as appropriate, and include the nativeFee in the method’s returned result/event
payload (or in the emitted transfer/delivery log) so consumers can see both
token and native fees; alternatively, if native fees are not needed, remove the
config.nativeFeeBps field and associated references to avoid confusion.
In `@typescript/rebalancer-sim/src/engine/SimulationEngine.ts`:
- Around line 196-199: The executeTransfers method currently takes a _timing:
SimulationTiming parameter but never uses it and instead uses a hardcoded 500ms
mailbox processing interval; either remove the unused _timing parameter from
executeTransfers or replace the hardcoded 500ms with a value derived from the
SimulationTiming object (e.g., timing.mailboxInterval or an appropriately named
field) so the mailbox loop uses the provided timing configuration; update
callers of executeTransfers if you remove the parameter and ensure
SimulationTiming is consulted where intervals are needed.
- Around line 283-305: Change the return type of buildWarpConfig from any to
WarpCoreConfig and add an import for WarpCoreConfig from '@hyperlane-xyz/sdk';
update the method signature to private buildWarpConfig(): WarpCoreConfig and
ensure the constructed object (tokens) conforms to the WarpCoreConfig shape
returned by that function.
In `@typescript/rebalancer-sim/src/kpi/KPICollector.ts`:
- Around line 46-52: startSnapshotCollection's async callback can cause
unhandled promise rejections when takeSnapshot() throws; wrap the await
this.takeSnapshot() call inside a try/catch in the setInterval callback, use
this.processLogger/error or throw a handled error (or call a configured error
handler) to record the failure, and ensure snapshotInterval is still
cleared/handled appropriately if needed; reference startSnapshotCollection,
takeSnapshot, snapshotInterval, and snapshotFrequencyMs when making the change.
In `@typescript/rebalancer-sim/src/rebalancer/HyperlaneRunner.ts`:
- Around line 207-215: The nested loops in HyperlaneRunner (inside the minAmount
strategy) iterate over belowMin and aboveTarget but don't decrement or track the
remaining deficit/excess, causing over-rebalancing; modify the loop to update
the remaining deficit and excess after each await
this.executeRebalance(fromChain, toChain, amount, domains) call (e.g., subtract
amount from the current deficit and excess variables or update the objects in
belowMin/aboveTarget), stop or continue inner/outer loops when a side reaches
zero (BigInt(0)), and ensure amount is computed from the updated values so each
executeRebalance is for the correct residual amount.
- Around line 164-178: The nested loops in HyperlaneRunner.ts iterate over
excess and deficit arrays without updating remaining amounts, causing
over-rebalancing; modify the logic to track and update remaining amounts (e.g.,
create mutable copies like remainingExcess and remainingDeficit maps or arrays
keyed by chain) and after each executeRebalance(fromChain, toChain,
rebalanceAmount, domains) subtract rebalanceAmount from both the fromChain's
remainingExcess and the toChain's remainingDeficit, skip pairs when remaining
amount is zero, and stop inner/outer loops when a chain’s remaining amount is
depleted so total moved matches original excess/deficit.
In `@typescript/rebalancer-sim/src/rebalancer/types.ts`:
- Around line 30-42: The ChainStrategyConfig has inconsistent numeric types:
weighted.weight and weighted.tolerance are strings while minAmount.min and
minAmount.target are numbers; update ChainStrategyConfig to make minAmount.min
and minAmount.target strings (like weighted) to avoid precision loss and keep
JSON/serialization consistency, and then update HyperlaneRunner parsing logic
(where parseFloat() and BigInt() are used) to explicitly convert those string
fields to the required numeric types (e.g., BigInt for on-chain amounts,
parseFloat/Decimal for ratios) and add a brief doc comment on
ChainStrategyConfig explaining that large token amounts are represented as
strings to preserve precision and require explicit conversion in
HyperlaneRunner.
In `@typescript/rebalancer-sim/src/scenario/ScenarioGenerator.ts`:
- Around line 14-21: The function randomBigIntInRange converts a bigint range to
Number which can overflow for ranges > Number.MAX_SAFE_INTEGER; replace the
Number-based Math.random approach with secure/rejection-sampling using random
bytes: compute range = (max - min + 1) as a bigint, determine the minimum number
of bytes/bits required to represent that range, generate a random bigint from
crypto.randomBytes (or Web Crypto getRandomValues) of that byte length, if the
random value >= range reject and repeat, otherwise return min + randomValue;
update the function randomBigIntInRange to use this byte-based rejection
sampling to avoid precision loss for large bigints.
🧹 Nitpick comments (21)
typescript/rebalancer-sim/.gitignore (1)
1-3: Solid choices here.These three patterns cover the essentials—keeping secrets out, build artifacts clean, and cached stuff where it belongs. Works just fine for a new package.
If you're feeling thorough, you might toss in a few more common ones like
coverage/(for test reports),*.log(for log files), or*.tsbuildinfo(TypeScript incremental build info), but those are more nice-to-haves than must-haves.📝 Optional additions
.env dist cache +coverage +*.log +*.tsbuildinfotypescript/rebalancer-sim/package.json (1)
6-7: Consider adding anexportsfield for better ESM resolution.Since this is an ES module, adding an explicit
exportsfield would give consumers clearer entry points and help with modern bundlers. Not urgent though - what you've got works fine for now, like a good mud bath.♻️ Optional enhancement
"main": "./dist/index.js", "types": "./dist/index.d.ts", + "exports": { + ".": { + "types": "./dist/index.d.ts", + "import": "./dist/index.js" + } + }, "files": [typescript/rebalancer-sim/src/bridges/BridgeMockController.ts (2)
187-190: Avoid theas Addresscast for recipient.
Safer to normalize/validate the address and let the type flow naturally. As per coding guidelines, avoid type assertions.✅ Suggested fix
- const recipient = ethers.utils.hexDataSlice( - recipientBytes32, - 12, - ) as Address; + const recipient: Address = ethers.utils.getAddress( + ethers.utils.hexDataSlice(recipientBytes32, 12), + );
120-129: Looks like the domain IDs are already coming through as numbers in ethers v5.The SentTransferRemote event emits
uint32 indexed origin, uint32 indexed destination, and ethers.js v5 returns uint32 values as native JavaScript numbers—not BigNumbers. The comparison at line 168 (d.domainId === destinationDomainId) works fine as-is since both sides are numeric. The suggested conversion would actually break the code because calling.toNumber()on a number throws an error.That said, there are two actual concerns worth addressing:
- nativeFee field in BridgeRouteConfig is defined but never used—either implement it or remove it.
- recipient conversion uses a type assertion (
as Address). While it's safe here, consider using a helper function likebytes32ToAddress()(which already exists in the test file) for consistency and to avoid relying on the cast.typescript/rebalancer-sim/src/deployment/SimulationDeployment.ts (3)
249-283: Replaceanywithunknownand narrow the error type safely.Using
unknowninstead ofanykeeps proper type safety. JavaScript can throw literally anything—strings, objects, whatever—so you need to check what you're dealing with before accessing properties. The type guardinstanceof Errorhandles the common case, whileString(error)catches everything else.♻️ Suggested change
- } catch (error: any) { + } catch (error: unknown) { + const reason = + error instanceof Error ? error.message : String(error); console.error( ` ${domain.chainName}: Failed to process message:`, - error.reason || error.message, + reason, ); break; }
220-243: ReplaceRecord<string, any>with proper SDK types for better type safety.Import
ChainMapandChainMetadatafrom@hyperlane-xyz/sdkand use them as the return type. The objects you're creating already match the schema, so this'll just give the caller proper typing without any runtime changes.♻️ Example refactor
-import type { Address } from '@hyperlane-xyz/utils'; +import type { Address } from '@hyperlane-xyz/utils'; +import type { ChainMap, ChainMetadata } from '@hyperlane-xyz/sdk'; import type { DeployedDomain, MultiDomainDeploymentOptions, MultiDomainDeploymentResult, SimulatedChainConfig, } from './types.js'; export function createSimulationChainMetadata( anvilRpc: string, chains: SimulatedChainConfig[], -): Record<string, any> { - const metadata: Record<string, any> = {}; +): ChainMap<ChainMetadata> { + const metadata: ChainMap<ChainMetadata> = {};
192-213: Use type guard function to eliminate inlineas Addresscasts.These casts can be narrowed with a single validation check. The repo already has
isValidAddressEvmfrom@hyperlane-xyz/utilswhich validates addresses and handles edge cases, so consider using that rather than calling ethers directly—keeps things swampy in one place. Create an assertion helper that calls it once at the start, then use the narrowed variables throughout.♻️ Suggested refactor using repo utilities
+import { isValidAddressEvm } from '@hyperlane-xyz/utils'; + +function assertAddress(value: string): asserts value is Address { + if (!isValidAddressEvm(value)) { + throw new Error(`Invalid address: ${value}`); + } +} + // Build result const domains: Record<string, DeployedDomain> = {}; for (const chain of chains) { + const mailboxAddress = mailboxes[chain.domainId].address; + const warpTokenAddress = warpTokens[chain.domainId].address; + const collateralAddress = collateralTokens[chain.domainId].address; + const bridgeAddress = bridges[chain.domainId].address; + assertAddress(mailboxAddress); + assertAddress(warpTokenAddress); + assertAddress(collateralAddress); + assertAddress(bridgeAddress); domains[chain.chainName] = { chainName: chain.chainName, domainId: chain.domainId, - mailbox: mailboxes[chain.domainId].address as Address, - warpToken: warpTokens[chain.domainId].address as Address, - collateralToken: collateralTokens[chain.domainId].address as Address, - bridge: bridges[chain.domainId].address as Address, + mailbox: mailboxAddress, + warpToken: warpTokenAddress, + collateralToken: collateralAddress, + bridge: bridgeAddress, }; } - return { + assertAddress(deployerAddress); + assertAddress(rebalancerAddress); + assertAddress(bridgeControllerAddress); + return { anvilRpc, - deployer: deployerAddress as Address, + deployer: deployerAddress, deployerKey, - rebalancer: rebalancerAddress as Address, + rebalancer: rebalancerAddress, rebalancerKey, - bridgeController: bridgeControllerAddress as Address, + bridgeController: bridgeControllerAddress, bridgeControllerKey, domains, snapshotId, };typescript/rebalancer-sim/scenarios/extreme-drain-chain1.json (1)
1-171: Good extreme imbalance test!This scenario will really put the rebalancer through its paces — 95% of transfers piling onto chain1 like villagers at my swamp. The 500ms intervals make it more aggressive than the other scenarios.
Minor observation: the filename is
extreme-drain-chain1.jsonbut the scenario name inside is"imbalance-chain1-95pct". Not a blocker, but keeping these aligned might help when debugging test failures later.typescript/rebalancer-sim/test/scenarios/unidirectional.test.ts (1)
85-104: Random-traffic chain coverage can still flake.
The “all chains used” expectation is probabilistic; tiny chance, but still non‑zero. If you want zero‑flake tests, consider injecting a seeded RNG or asserting a minimum per‑chain count instead of strict inclusion.typescript/rebalancer-sim/test/integration/deployment.test.ts (1)
21-45: Consider sharing the anvil boot helper across integration tests.
startAnvilis duplicated in the full‑simulation test; extracting a tiny helper keeps fixes in one spot.typescript/rebalancer-sim/scripts/generate-scenarios.ts (1)
20-27: Swap out theanyfor properTransferScenariotyping.Line 20 just uses
anyhere, which is like leaving the door open for bugs. TheTransferScenariotype already exists and has thetransfersanddurationproperties this function needs. Makes the code more honest about what it's working with.♻️ Proposed fix
-import { ScenarioGenerator } from '../src/scenario/ScenarioGenerator.js'; +import { ScenarioGenerator } from '../src/scenario/ScenarioGenerator.js'; +import type { TransferScenario } from '../src/scenario/types.js'; @@ -function saveScenario(name: string, scenario: any) { +function saveScenario(name: string, scenario: TransferScenario) {typescript/rebalancer-sim/src/scenario/ScenarioLoader.ts (2)
14-27: Consider validating the parsed JSON structure before deserialization.Right now, the
JSON.parseresult is cast directly toSerializedScenario. If someone drops a malformed file in there, you'll get cryptic errors downstream indeserialize()rather than a clear validation failure. Might be worth a quick schema check, but not a showstopper for a test utility.
46-67: Duplicate file-reading and JSON parsing logic across functions.
loadScenarioandgetScenarioMetadataboth do the same file-existence check andJSON.parsedance. Might be worth extracting a shared helper if this grows, but for now it's fine - just a wee bit of repetition.typescript/rebalancer-sim/src/rebalancer/HyperlaneRunner.ts (1)
57-64: Unhandled promise rejection in polling loop.The
setTimeoutcallback is async but errors aren't caught at the scheduling level. IfpollAndRebalancethrows after the try-catch, it could cause issues. Though the internal try-catch should handle it, it's worth being defensive here.🔧 Defensive error handling in scheduler
private scheduleNextPoll(): void { if (!this.running || !this.config) return; this.pollingTimer = setTimeout(async () => { - await this.pollAndRebalance(); - this.scheduleNextPoll(); + try { + await this.pollAndRebalance(); + } finally { + this.scheduleNextPoll(); + } }, this.config.pollingFrequency); }typescript/rebalancer-sim/src/engine/SimulationEngine.ts (2)
90-97: Non-null assertions on kpiCollector in event handlers.These event handlers use
this.kpiCollector!which could theoretically fail if the handler fires after cleanup or during teardown. Since this is a simulation utility, it's probably fine in practice, but worth noting.🔧 Defensive null check
this.bridgeController.on('transfer_delivered', (event) => { - this.kpiCollector!.recordTransferComplete(event.transfer.id); + this.kpiCollector?.recordTransferComplete(event.transfer.id); }); this.bridgeController.on('transfer_failed', (event) => { - this.kpiCollector!.recordTransferFailed(event.transfer.id); + this.kpiCollector?.recordTransferFailed(event.transfer.id); });
273-279: Error handling logs but doesn't fail the transfer count.When a transfer fails (line 277), it's recorded as failed but execution continues silently. The console.error is good, but for simulation accuracy you might want to track the error reason for KPI analysis.
typescript/rebalancer-sim/src/harness/RebalancerSimulationHarness.ts (2)
168-179: Mutatingthis.deployment.snapshotIdduring comparison loop.This modifies shared state while iterating. If
runSimulationfails mid-loop or something accessesdeploymentconcurrently, the snapshot ID could be in an inconsistent state. Consider using a local variable for the comparison-specific snapshot.♻️ Avoid mutating shared state
for (const rebalancer of rebalancers) { // Reset state before each run await restoreSnapshot(provider, this.deployment.snapshotId); // Create fresh snapshot for this run const newSnapshotId = await provider.send('evm_snapshot', []); - this.deployment.snapshotId = newSnapshotId; + const runDeployment = { ...this.deployment, snapshotId: newSnapshotId }; // Run simulation - const result = await this.runSimulation(scenario, rebalancer, options); + // Pass deployment context explicitly or use the fresh snapshot + const result = await this.engine!.runSimulation( + scenario, + rebalancer, + options.bridgeConfig ?? createSymmetricBridgeConfig(this.config.chains.map((c) => c.chainName)), + options.timing ?? DEFAULT_TIMING, + options.strategyConfig, + ); results.push(result); }
200-203: Large bigint literal for initial minGasCost.That's a proper big number there. Using
BigInt('0xffffffffffffffffffffffffffffffff')works butBigInt(2) ** BigInt(128) - BigInt(1)or importing a constant might be clearer. Not a bug though, just a style thing.typescript/rebalancer-sim/src/scenario/ScenarioGenerator.ts (1)
72-74: Theas Addressassertion is unnecessary and should be removed.Since
usercomes from destructuring with a default value ofrandomAddress()(which explicitly returnsAddress), TypeScript should correctly inferuserasAddresstype. The optionaluser?: Addressin the interface gets narrowed by the default parameter—if it's not provided, the default gives you anAddressanyway.No need to force the type with a cast here; let the types fall into place naturally.
typescript/rebalancer-sim/src/kpi/KPICollector.ts (2)
126-144: Silent no-ops when transfer ID not found.These methods quietly do nothing if the transfer record doesn't exist. In a simulation harness this might be fine, but if IDs get mismatched somewhere, you'd never know about it.
Might want to at least log a warning when the record isn't found - makes debugging easier when things go sideways in the swamp.
♻️ Optional: Add warning logs
recordTransferComplete(id: string): void { const record = this.transferRecords.get(id); - if (record) { + if (!record) { + console.warn(`Transfer record not found for completion: ${id}`); + return; + } - record.endTime = Date.now(); - record.latency = record.endTime - record.startTime; - record.status = 'completed'; - } + record.endTime = Date.now(); + record.latency = record.endTime - record.startTime; + record.status = 'completed'; }
198-207: Non-null assertion could be avoided with a type guard.The
!assertion on line 201 works because of the filter, but per the coding guidelines about preferring type guards over assertions, there's a cleaner way to do this.♻️ Suggested refactor using type guard
// Calculate latencies - const latencies = completed - .filter((t) => t.latency !== undefined) - .map((t) => t.latency!) - .sort((a, b) => a - b); + const latencies = completed + .map((t) => t.latency) + .filter((latency): latency is number => latency !== undefined) + .sort((a, b) => a - b);Using a type predicate in the filter lets TypeScript infer the correct type without needing the assertion. As per the coding guidelines, type guards are preferred over type assertions.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7903 +/- ##
=======================================
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:
|
…ssing - Add mailboxProcessorKey as 4th signer to avoid nonce conflicts - Separate concerns: deployer for transfers, bridgeController for bridges, mailboxProcessor for mailbox message processing - Refactor processAllPendingMessages to fire transactions in parallel - Return per-chain processed counts for accurate completion tracking - Increase deployer token mint from 2x to 100x for longer scenarios Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- New MessageTracker class tracks messages off-chain with per-message status - Supports 'pending', 'inflight', 'delivered', 'failed' states - Uses static calls to pre-check which messages can be processed - Fires processable transactions in parallel without blocking on receipts - Emits events for delivery tracking (message_delivered, message_failed) - Refactor SimulationEngine to use MessageTracker instead of inline tracking - Wire MessageTracker events to KPICollector for accurate latency measurement This enables more granular control over message processing and accurate per-transfer latency tracking, similar to how the Hyperlane relayer batches. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Added new scenario generator method that creates truly balanced traffic patterns by pairing each A→B transfer with a corresponding B→A transfer of the same amount. This ensures net flow per chain is zero, isolating message delivery latency from rebalancer behavior. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…ctations - Added ScenarioFile type with embedded default configurations - Scenarios now include defaultTiming, defaultBridgeConfig, defaultStrategyConfig - Added expectations field for automated assertions (minCompletionRate, etc.) - Updated ScenarioLoader to parse new format and wire up bridge addresses - Added serialized config types for JSON persistence Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…ced traffic - Updated all scenarios with embedded default configs and expectations - balanced-bidirectional now uses balancedTraffic generator for true balance - Added random-with-headroom scenario: random traffic with sufficient collateral Scenarios now self-contained with timing, bridge config, strategy config, and expected KPI assertions. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…tations - full-simulation tests now load defaults from scenario JSON files - Added dynamic strategy config wiring with deployed bridge addresses - Tests assert expectations from scenario files (minCompletionRate, etc.) - random-with-headroom test checks p50 latency < 500ms for headroom validation - Results saved to results/ directory for post-hoc analysis Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Added test/utils/anvil.ts with setupAnvilTestSuite helper for mocha - Added inflight-guard test demonstrating future inflight tracking integration - Added README.md with package documentation and usage examples - Updated .gitignore to exclude results/ directory Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add RealRebalancerRunner wrapping actual @hyperlane-xyz/rebalancer - Add SimulationRegistry implementing IRegistry for simulation env - Add cleanup functions to prevent state leakage between tests - Track rebalance events via bridge contract listeners - Support multi-rebalancer comparison via REBALANCERS env var - Default to HyperlaneRunner only; opt-in to real service comparison Usage: REBALANCERS=hyperlane pnpm test # Default, all tests pass REBALANCERS=hyperlane,real pnpm test --grep "scenario-name" # Compare Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
RebalancerService.start() registers SIGINT/SIGTERM handlers but stop() doesn't remove them. Track handlers added by the service and clean them up explicitly to prevent accumulation across test runs. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@typescript/rebalancer-sim/scenarios/moderate-imbalance-chain1.json`:
- Around line 3-4: The description string "70% of transfers go TO chain1" in the
JSON should be made accurate: either update the "description" value to state
"80% of transfers go TO chain1" to match the current transfers array, or
regenerate/adjust the transfer entries in the "transfers" array so that 70%
(e.g., 11 out of 15) go to chain1; modify the "description" field and/or the
"transfers" array accordingly so the documented percentage matches the actual
distribution.
In `@typescript/rebalancer-sim/src/mailbox/MessageTracker.ts`:
- Around line 117-124: The JSDoc for MessageTracker.getPendingMessages says
"including not yet ready, excluding inflight" but the predicate currently
includes inflight; pick one and make them consistent: either update the comment
to say it includes inflight, or change the filter to only include statuses that
represent pending/not-yet-ready (e.g., remove the 'inflight' check so it only
returns m.status === 'pending' or the specific waiting status used in
TrackedMessage). Locate the getPendingMessages method and update either the
docstring or the predicate (referring to TrackedMessage.status and
this.messages.values()) so the behavior and documentation match.
- Around line 167-169: The early return in MessageTracker (when
processable.length === 0) incorrectly reports failed = ready.length even though
those messages remain pending; update the branch in the method that checks
processable to return { delivered: 0, failed: 0 } instead (or, if the intent is
to mark them as failures, explicitly move items from ready to a failed state
before returning) so KPI counts are accurate; reference the variables
processable and ready and the containing class/function MessageTracker to locate
and change this behavior.
In `@typescript/rebalancer-sim/src/rebalancer/RealRebalancerRunner.ts`:
- Around line 241-257: The stubbed override of process.exit in
RealRebalancerRunner (where originalExit is saved and process.exit is replaced
before calling this.service.start()) is being restored immediately in the
finally block, exposing the background daemon to later exit calls; instead keep
the stub active for the runner's lifetime by removing the finally restoration
and moving the restoration logic into the runner's stop() method (restore
process.exit to originalExit there), ensuring start() still launches the
background this.service.start() and stop() restores process.exit and performs
any cleanup.
In `@typescript/rebalancer-sim/src/scenario/types.ts`:
- Around line 25-26: The comment for defaultInitialCollateral is ambiguous:
decide whether it is truly a per-chain map or a single global default; if it's
meant to be per-chain, change the type of defaultInitialCollateral (and its
documentation) to a map type (e.g., Record<string,string> or
Record<number,string>) and update all usages of defaultInitialCollateral to
index by chain id; if it is meant to be a single global default, update the
comment/text to remove “per chain” (e.g., “Default initial collateral balance
(in wei, as string for JSON)”) and optionally rename the property to
defaultInitialCollateralGlobal or defaultInitialCollateralDefault to avoid
future confusion. Ensure you update any consuming code/tests to match the chosen
shape.
🧹 Nitpick comments (13)
typescript/rebalancer-sim/scenarios/extreme-drain-chain1.json (1)
217-242: Consider weight precision for the strategy config.The weights here add up to 0.999 (0.333 × 3) rather than a clean 1.0. Now, this might not matter a lick if the consuming code normalizes weights or handles floating-point tolerance gracefully. But if anything down the line expects weights to sum exactly to 1.0, you could get some unexpected behavior.
Could bump these to something like
"0.3334","0.3333","0.3333"to hit 1.0, or use a higher precision string. Up to you whether it's worth the bother."chain1": { "weighted": { - "weight": "0.333", + "weight": "0.3334", "tolerance": "0.15" }, "bridgeLockTime": 500 }, "chain2": { "weighted": { - "weight": "0.333", + "weight": "0.3333", "tolerance": "0.15" }, "bridgeLockTime": 500 }, "chain3": { "weighted": { - "weight": "0.333", + "weight": "0.3333", "tolerance": "0.15" }, "bridgeLockTime": 500 }typescript/rebalancer-sim/README.md (1)
193-195: Vary the repeated bullet openings.
Three bullets starting with “Does” in a row reads a bit clunky—easy polish.✏️ Suggested rewording
- - Does `unidirectionalFlow()` create correct transfer patterns? - - Does `randomTraffic()` distribute across all chains? - - Does serialization preserve BigInt amounts? + - Does `unidirectionalFlow()` create correct transfer patterns? + - Is `randomTraffic()` distributing across all chains? + - Is BigInt serialization preserved?typescript/rebalancer-sim/src/mailbox/MessageTracker.ts (1)
153-163: Preferunknownin catch blocks.
Usinganyhere drops type-safety; a tiny helper keeps it clean. Apply the same pattern to the later catch in this method.As per coding guidelines: Use proper typing over `any` or type assertions in TypeScript.♻️ Example cleanup
- } catch (error: any) { - // Would revert - mark attempt but keep pending for retry - message.attempts++; - message.lastError = error.reason || error.message; - // Don't emit failed event - it will retry - } + } catch (error: unknown) { + // Would revert - mark attempt but keep pending for retry + message.attempts++; + message.lastError = + error instanceof Error ? error.message : String(error); + // Don't emit failed event - it will retry + }typescript/rebalancer-sim/src/rebalancer/SimulationRegistry.ts (1)
25-28: Avoid theas RegistryTypecast.
The literal should satisfy the union; if not, prefersatisfiesor adjust the type instead of casting.As per coding guidelines: Adjust interface definitions to be compatible rather than using type casts in TypeScript.♻️ Suggested tweak
- readonly type: RegistryType = 'partial' as RegistryType; + readonly type: RegistryType = 'partial';typescript/rebalancer-sim/src/rebalancer/RealRebalancerRunner.ts (2)
101-124: Type the chain metadata instead ofany.
UsingChainMetadata(or a typed map) keeps the MultiProvider config honest and avoids silent shape drift.♻️ Suggested tweak
- const chainMetadata: Record<string, any> = {}; + const chainMetadata: Record<string, ChainMetadata> = {};import { MultiProvider, type ChainMetadata } from '@hyperlane-xyz/sdk';As per coding guidelines: Use proper typing over
anyor type assertions in TypeScript; Import types from@hyperlane-xyz/sdkwhen available in TypeScript code.
169-223: Avoidas StrategyConfigandanyin strategy mapping.
Better to shape the object toStrategyConfigdirectly (typedchainsmap) so mismatches don’t get masked.As per coding guidelines: Adjust interface definitions to be compatible rather than using type casts in TypeScript; Use proper typing over
anyor type assertions in TypeScript.typescript/rebalancer-sim/test/integration/deployment.test.ts (1)
144-154: Consider awaiting transaction confirmation explicitly.The
mintTocall doesn't explicitly wait for the transaction to be mined before checking the balance. With Anvil's default automine mode this works fine, but for robustness (especially if someone ever changes the mining config), it's better to be explicit.🔧 Optional: Explicit transaction confirmation
// Modify state - mint more tokens to warp contract const token = ERC20Test__factory.connect(domain.collateralToken, deployer); - await token.mintTo(domain.warpToken, toWei(100)); + const tx = await token.mintTo(domain.warpToken, toWei(100)); + await tx.wait();typescript/rebalancer-sim/test/integration/inflight-guard.test.ts (1)
274-276: Consider using a more robust wait mechanism, friend.This hardcoded 4000ms sleep after the simulation could be flaky if the bridge configuration changes. The engine already calls
waitForAllDeliveries, but if you need additional wait time afterrunSimulationreturns, consider making it relative tobridgeConfig.deliveryDelayrather than a magic number.- // Wait for any remaining bridge deliveries - await new Promise((resolve) => setTimeout(resolve, 4000)); + // Wait for any remaining bridge deliveries (bridge delay + buffer) + const bridgeDelay = bridgeConfig['heavy']['light'].deliveryDelay; + await new Promise((resolve) => setTimeout(resolve, bridgeDelay + 1000));typescript/rebalancer-sim/src/scenario/ScenarioLoader.ts (1)
38-52: Theas Addresscast is acceptable here, but validation could help.The type assertion on line 49 is reasonable since you're deserializing from a known schema. However, if someone creates a malformed scenario file, this could silently pass invalid addresses. Consider adding validation (e.g., check
ethers.utils.isAddress(t.user)) or using thevalidatemethod fromScenarioGeneratorafter loading.Per coding guidelines: "Avoid unnecessary type casts (
asassertions)... If types don't match, fix the underlying types rather than casting." This cast is borderline acceptable since JSON deserialization inherently requires some type bridging.typescript/rebalancer-sim/test/integration/full-simulation.test.ts (2)
168-171: Avoidas Record<string, any>type cast.This cast could be cleaned up by defining a proper type for the strategy config. The coding guidelines say to adjust interface definitions rather than using type casts.
🔧 Suggested fix
+interface RuntimeStrategyConfig { + type: string; + chains: Record<string, { + weighted?: { weight: string; tolerance: string }; + bridge: string; + bridgeLockTime?: number; + }>; +} - const strategyConfig = { + const strategyConfig: RuntimeStrategyConfig = { type: file.defaultStrategyConfig.type, - chains: {} as Record<string, any>, + chains: {}, };As per coding guidelines: "Use proper typing over
anyor type assertions in TypeScript."
315-353: Theoutput: anytype should be properly typed.This object has a well-defined structure - consider creating a
SimulationResultOutputinterface. It would improve maintainability and catch errors if the structure changes.🔧 Suggested fix
+interface SimulationResultOutput { + scenario: string; + timestamp: string; + description: string; + expectedBehavior: string; + expectations: ScenarioExpectations; + results: Array<{ + rebalancerName: string; + kpis: { + totalTransfers: number; + completedTransfers: number; + completionRate: number; + averageLatency: number; + p50Latency: number; + p95Latency: number; + p99Latency: number; + totalRebalances: number; + rebalanceVolume: string; + }; + }>; + config: { + timing: SimulationTiming; + initialCollateral: string; + }; + comparison?: { bestCompletionRate: string; bestLatency: string }; +} - const output: any = { + const output: SimulationResultOutput = {typescript/rebalancer-sim/src/engine/SimulationEngine.ts (1)
290-297: Unused_deliveryDelayparameters could be removed or documented.Both
startMailboxProcessingandwaitForUserTransferDeliverieshave_deliveryDelayparameters that aren't used. If they're reserved for future use, a comment explaining why would help. Otherwise, consider removing them to reduce confusion.🔧 Option: Remove unused parameters
- private startMailboxProcessing(_deliveryDelay: number): void { + private startMailboxProcessing(): void { // Process mailbox every 100ms to check for deliveries due const PROCESS_INTERVAL = 100;And update the caller on line 137:
- this.startMailboxProcessing(timing.userTransferDeliveryDelay); + this.startMailboxProcessing();Also applies to: 321-324
typescript/rebalancer-sim/src/scenario/types.ts (1)
165-179: TightenRandomTrafficOptionswith a discriminated union.If
distribution: 'poisson',poissonMeanIntervalis effectively required; otherwise it’s optional. A discriminated union would prevent callers from forgetting it and causing runtime surprises.♻️ Possible shape
-export interface RandomTrafficOptions { - chains: string[]; - transferCount: number; - duration: number; - amountRange: [bigint, bigint]; - users?: Address[]; - distribution?: 'uniform' | 'poisson'; - poissonMeanInterval?: number; -} +export type RandomTrafficOptions = + | { + chains: string[]; + transferCount: number; + duration: number; + amountRange: [bigint, bigint]; + users?: Address[]; + distribution?: 'uniform'; + } + | { + chains: string[]; + transferCount: number; + duration: number; + amountRange: [bigint, bigint]; + users?: Address[]; + distribution: 'poisson'; + poissonMeanInterval: number; + };
- Clean up deployment provider after contracts are deployed (was polling forever) - Handle "already delivered" messages in MessageTracker (was retrying forever) - Stop provider polling in test cleanup - Relax latency assertion for comparison testing These fixes enable reliable comparison testing with both rebalancers on 1-3 scenarios. Full suite (10 scenarios × 2 rebalancers) still has issues due to deeper ethers.js/Node.js state accumulation. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
nambrot
force-pushed
the
feat/rebalancer-simulation-harness
branch
from
b7eb5a7 to
bc1589c
Compare
Change default from 'hyperlane' only to 'hyperlane,real' so comparison testing is the default behavior. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@typescript/rebalancer-sim/test/integration/full-simulation.test.ts`:
- Around line 9-12: The test file's header and inline comment are out of sync
with the code: update the comment that currently states "Default: runs
HyperlaneRunner only" (and the similar inline comment later) to reflect the
actual default `REBALANCERS` value used by the test (i.e., `hyperlane,real`), or
alternatively change the test's default `REBALANCERS` setting to match the
comment; locate the REBALANCERS usage and the header/inline comment in
full-simulation.test.ts and make the text consistent (either change the comment
to "Default: runs hyperlane,real" or change the code to default to just
"hyperlane").
- Around line 65-68: The current parsing of REBALANCERS into ENABLED_REBALANCERS
can produce an empty array which makes tests silently no-op; update the code
handling REBALANCER_ENV/ENABLED_REBALANCERS so that after building
ENABLED_REBALANCERS (from REBALANCER_ENV.split(...).filter(...)), you
immediately guard: if ENABLED_REBALANCERS.length === 0 then either throw a clear
Error (e.g. "No rebalancers selected via REBALANCERS; expected 'hyperlane'
and/or 'real'") to fail fast in tests or fall back to a safe default (e.g.
['hyperlane','real']), ensuring the test runner does not silently skip
scenarios; reference ENABLED_REBALANCERS and REBALANCER_ENV when implementing
the guard.
🧹 Nitpick comments (2)
typescript/rebalancer-sim/test/integration/full-simulation.test.ts (2)
169-172: Avoidas Record<string, any>; strongly type the strategy config.
Theany+ type assertion sidesteps type safety. It’s better to derive types fromfile.defaultStrategyConfigand the deployment shape so the compiler keeps us honest.♻️ Typed alternative (no `any` cast)
- const strategyConfig = { - type: file.defaultStrategyConfig.type, - chains: {} as Record<string, any>, - }; + type BaseChainConfig = + (typeof file.defaultStrategyConfig.chains)[string]; + type Bridge = + (typeof deployment.domains)[string]['bridge']; + type StrategyConfig = { + type: typeof file.defaultStrategyConfig.type; + chains: Record<string, BaseChainConfig & { bridge: Bridge }>; + }; + + const strategyConfig: StrategyConfig = { + type: file.defaultStrategyConfig.type, + chains: {}, + };
323-341: ReplaceanyinsaveResultsoutput with a typed shape.
const output: anyweakens test correctness and makes refactors risky. A typed result (orsatisfies) keeps structure honest without much extra work.✅ Example using `satisfies`
- const output: any = { + type SavedResult = { + scenario: string; + timestamp: string; + description: ScenarioFile['description']; + expectedBehavior: ScenarioFile['expectedBehavior']; + expectations: ScenarioFile['expectations']; + results: Array<{ + rebalancerName: SimulationResult['rebalancerName']; + kpis: { + totalTransfers: number; + completedTransfers: number; + completionRate: number; + averageLatency: number; + p50Latency: number; + p95Latency: number; + p99Latency: number; + totalRebalances: number; + rebalanceVolume: string; + }; + }>; + config: { + timing: ScenarioFile['defaultTiming']; + initialCollateral: ScenarioFile['defaultInitialCollateral']; + }; + comparison?: { bestCompletionRate: string; bestLatency: string }; + }; + + const output = { scenario: scenarioName, timestamp: new Date().toISOString(), description: file.description, expectedBehavior: file.expectedBehavior, expectations: file.expectations, results: results.map((r) => ({ rebalancerName: r.rebalancerName, kpis: { totalTransfers: r.kpis.totalTransfers, completedTransfers: r.kpis.completedTransfers, completionRate: r.kpis.completionRate, averageLatency: r.kpis.averageLatency, p50Latency: r.kpis.p50Latency, p95Latency: r.kpis.p95Latency, p99Latency: r.kpis.p99Latency, totalRebalances: r.kpis.totalRebalances, rebalanceVolume: r.kpis.rebalanceVolume.toString(), }, })), config: { timing: file.defaultTiming, initialCollateral: file.defaultInitialCollateral, }, - }; + } satisfies SavedResult;
…lancer testing - Restart anvil before/after each test for complete isolation - Mark timed-out transfers as failed instead of blocking - Run both HyperlaneRunner and RealRebalancerRunner by default - All 21 tests pass with both rebalancers Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…ults - Generate interactive HTML timelines showing transfers, rebalances, balances - KPI cards show completion rate, latency metrics, rebalance counts - Transfer bars show duration (start to delivery) per chain - Rebalance markers show direction with arrows - Balance curves overlay liquidity over time - Tooltips and click-to-detail panel for drill-down - Auto-generates HTML alongside JSON after each test Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@typescript/rebalancer-sim/src/bridges/BridgeMockController.ts`:
- Around line 378-383: The emitted timeout event from BridgeMockController
currently sends { transfer, error } which doesn't match the BridgeEvent shape;
update the emit call that iterates over this.pendingTransfers (the loop
containing this.emit('transfer_failed', ...)) to emit an object matching
BridgeEvent by including a type field (e.g., type: 'transfer_failed'), the
transfer, and a timestamp (e.g., Date.now() or new Date().toISOString()), and if
you need to surface the error keep it in an additional property like reason or
errorDetail while preserving the BridgeEvent required fields.
🧹 Nitpick comments (4)
typescript/rebalancer-sim/test/utils/anvil.ts (1)
7-13: Platform compatibility note - 'which' is a Unix thing.This uses
which anvilto check availability, which works great on Unix-like systems but won't fly on Windows. For local dev test infrastructure this is probably fine, but if Windows support ever becomes a thing, you'd needwhere anvilor a cross-platform approach.🔧 Cross-platform alternative (if needed later)
+import { execSync } from 'child_process'; + export async function isAnvilAvailable(): Promise<boolean> { - return new Promise((resolve) => { - const check = spawn('which', ['anvil']); - check.on('close', (code) => resolve(code === 0)); - check.on('error', () => resolve(false)); - }); + try { + const cmd = process.platform === 'win32' ? 'where anvil' : 'which anvil'; + execSync(cmd, { stdio: 'ignore' }); + return true; + } catch { + return false; + } }typescript/rebalancer-sim/test/integration/full-simulation.test.ts (2)
156-167: Type cast withas Record<string, any>- not the cleanest approach.Per the coding guidelines, we should avoid
anyand type assertions. This could use proper typing for the strategy config structure.♻️ Suggested improvement
Define a proper interface for the chains config rather than casting:
+interface ChainStrategyConfig { + targetBalance?: string; + minBalance?: string; + bridge: string; +} + const strategyConfig = { type: file.defaultStrategyConfig.type, - chains: {} as Record<string, any>, + chains: {} as Record<string, ChainStrategyConfig>, };Or if
RebalancerSimConfig['strategyConfig']already has the right shape, use that type directly.As per coding guidelines: "Use proper typing over
anyor type assertions in TypeScript."
304-342:saveResultsusesanyfor output - could use a proper type.The output object has a clear structure, so defining an interface would make this more maintainable and catch shape errors at compile time.
♻️ Optional type definition
interface ScenarioResultOutput { scenario: string; timestamp: string; description: string; expectedBehavior: string; expectations: ScenarioFile['expectations']; results: Array<{ rebalancerName: string; kpis: { /* ... */ }; }>; config: { timing: SimulationTiming; initialCollateral: string; }; comparison?: { bestCompletionRate: string; bestLatency: string }; }typescript/rebalancer-sim/src/engine/SimulationEngine.ts (1)
290-297: Unused_deliveryDelayparameter instartMailboxProcessing.The parameter is accepted but not used - the interval is hardcoded to 100ms. Either use the delay to configure behavior or remove the parameter.
♻️ Options
Option A: Remove unused parameter
- private startMailboxProcessing(_deliveryDelay: number): void { + private startMailboxProcessing(): void {Option B: Use the parameter (if configurable interval is desired)
private startMailboxProcessing(_deliveryDelay: number): void { - // Process mailbox every 100ms to check for deliveries due - const PROCESS_INTERVAL = 100; + // Process mailbox at a fraction of the delivery delay + const PROCESS_INTERVAL = Math.min(100, _deliveryDelay / 10 || 100);
…ebalance bars - Compute balance curves from transfer/rebalance events instead of on-chain snapshots - Fixes incorrect collateral flow display (destination now correctly decreases on payout) - Shows step-function curves for clearer discrete balance changes - Display rebalances as duration bars with latency labels (like transfers) - Add rebalance_start/complete/failed event types for timeline - Update tooltips and details panel for new rebalance structure with latency Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Update RebalanceRecord with startTime, endTime, latency, status fields - Change KPICollector to use Map-based tracking with bridge transfer correlation - Add recordRebalanceStart/Complete/Failed methods for lifecycle tracking - Wire SimulationEngine to use bridgeController events for rebalance tracking - Remove duplicate SentTransferRemote listeners (bridgeController handles this) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
ethers v5 providers automatically poll for new blocks by default. With multiple provider instances pointing to the same anvil RPC, this caused significant contention and 3-4 second delays for some transfers. Changes: - Disable polling on all JsonRpcProvider instances - Create and pass pre-configured MultiProtocolProvider with polling disabled to RebalancerService - Add cleanup for MultiProtocolProvider in RealRebalancerRunner Result: p95 latency dropped from ~4000ms to ~400-1000ms depending on scenario. Both rebalancers now show comparable performance. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The RealRebalancerService does heavy async initialization: - RebalancerContextFactory.create() creates WarpCore with RPC calls - createStrategy() queries initial token balances via RPC - monitor.start() begins the polling loop This takes 2-3 seconds. The previous 50ms wait was insufficient, causing the rebalancer to not be polling when transfers started. This led to RPC contention between initialization and message processing, causing some transfers to have 4+ second delays. Increased wait time to 3000ms to ensure full initialization. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Added logging to help diagnose slow message processing: - Logs slow static calls (>100ms) - Logs failed processing attempts with high retry counts (>10) - Logs total static call check time when >200ms Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
…ion pattern - Extract shared test helpers to simulation-helpers.ts - Add initialImbalance support to ScenarioFile for creating imbalanced states - Create inflight-guard.json scenario using chain1/chain2 naming - Refactor inflight-guard.test.ts to use runScenarioWithRebalancers() - Refactor full-simulation.test.ts to use shared helpers - Both tests now generate consistent HTML timelines and comparison tables Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The HTML timeline generator expected balanceTimeline for rendering balance curves, but it wasn't being populated. Added the field to VisualizationData interface and populate it with initial balances from config in toVisualizationData(). Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@typescript/rebalancer-sim/test/utils/simulation-helpers.ts`:
- Around line 124-144: The JsonRpcProvider instance created as "provider" via
new ethers.providers.JsonRpcProvider(options.anvilRpc) is never cleaned up;
ensure you destroy/remove listeners for that provider after the imbalance loop
(or in the same test cleanup used for balanceProvider) to avoid leaking
connections. Locate the provider creation (variable provider) and add a cleanup
call (prefer provider.destroy() if available, otherwise
provider.removeAllListeners() and any disconnect/close) in the existing
teardown/cleanup routine that already handles balanceProvider so the
ANVIL_DEPLOYER_KEY Wallet and ERC20Test__factory usage don't leave open RPC
connections.
🧹 Nitpick comments (2)
typescript/rebalancer-sim/test/integration/inflight-guard.test.ts (1)
83-93: The test runs but doesn't assert - that's intentional, right?The implementation correctly exercises the inflight-guard scenario and generates reports, but there's no programmatic assertion. The comment on line 91-92 acknowledges this. When you're ready to formalize the expected behavior, considerin' adding assertions like:
expect(results.results.find(r => r.rebalancerName.includes('Production'))?.kpis.totalRebalances) .to.be.lessThan(results.results.find(r => r.rebalancerName.includes('Simple'))?.kpis.totalRebalances ?? Infinity);This would verify the ProductionRunner sends fewer rebalances than SimpleRunner, which is the core behavior bein' demonstrated.
typescript/rebalancer-sim/test/utils/simulation-helpers.ts (1)
1-1: This import's just sittin' there unused.The
expectfrom chai is imported but never used in this file. If it's not needed, might as well clear it out.🧹 Remove unused import
-import { expect } from 'chai'; import { ethers } from 'ethers';
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@typescript/rebalancer-sim/src/types.ts`:
- Around line 848-857: The current heuristic in the initial balance conversion
(using numValue > 1e15, parseFloat, and BigInt(Math.floor(numValue))) is unsafe
and loses precision; instead require an explicit unit on
SimulationConfig.initialCollateral (e.g., unit: 'wei' | 'ether') or mandate that
inputs are already wei strings, and remove the magnitude-guessing logic in the
code that sets initialBalances[chain] for the variable/value named value. If you
support both units, branch on the new unit field: when unit==='wei' accept the
string as-is and validate with BigInt (no parseFloat), when unit==='ether'
convert safely from a decimal string to wei using precise string-based
arithmetic (or a decimal library) to multiply by 1e18 and produce a BigInt
string; update parsing/validation to avoid Number/MAX_SAFE_INTEGER and eliminate
the numValue > 1e15 check.
🧹 Nitpick comments (2)
typescript/rebalancer-sim/src/types.ts (2)
186-232: Consider adding a test-only warning comment for the Anvil keys.These are the well-known Anvil test account keys, which is fine for local testing - everybody knows 'em, they're not exactly secrets hidden away in any swamp. But some security scanners might get spooked seeing private keys in source. A brief comment block noting these are test-only, publicly-known Anvil keys could save someone a wee bit of confusion later.
📝 Suggested comment addition
+/** + * ⚠️ TEST KEYS ONLY - DO NOT USE IN PRODUCTION + * These are the standard Anvil/Hardhat test account keys. + * They are publicly known and funded only on local test networks. + */ + /** * Default anvil deployer key (first account) */ export const ANVIL_DEPLOYER_KEY = '0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80';
368-380: Consider making the strategy config mutually exclusive.Right now
ChainStrategyConfighas bothweighted?andminAmount?as optional. Someone could accidentally provide both or neither. Now, I know validation happens elsewhere, but a discriminated union could make the types self-documenting and catch misuse at compile time.That said, this is just a thought for the road - current approach works fine if runtime validation handles it.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…rrectly MockValueTransferBridge now actually pulls tokens, so the old approach of using hyperlaneWarpSendRelay to "complete" the rebalance was incorrect - it sent tokens FROM dest TO origin, causing origin to unlock tokens to a recipient. Instead, simulate bridge delivery by minting tokens directly to the destination warp token, which is what a real bridge would do. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Ensures JSON files are formatted with prettier on commit. Also includes formatting fixes for rebalancer-sim scenario files. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Code Review: Refined Analysis (OpenCode + Codex)This review consolidates findings from manual code review and Codex static analysis. Overall AssessmentThe PR is well-architected with clean separation of concerns, comprehensive testing, and excellent documentation. However, there are a few API-level issues that should be addressed or explicitly accepted before merge. Issues to Address1. HIGH:
|
| Issue | File | Notes |
|---|---|---|
| Unused provider created but never disposed | RebalancerSimulationHarness.ts:175-179 |
Easy leak in long-running use |
buildWarpConfig uses 'HypCollateral' string |
SimulationEngine.ts:397 |
Should use TokenStandard.EvmHypCollateral for registry compatibility |
Summary
| Severity | Issue | Status |
|---|---|---|
| HIGH | compareRebalancers state reuse |
Tests work around it, but API is buggy |
| MEDIUM | Duplicate processing race | Partial mitigation via inflight status, but race window exists |
| MEDIUM | Startup errors swallowed | Can produce misleading KPIs |
| LOW | Unused provider | Minor leak |
| LOW | Wrong standard string | Future footgun |
Recommendation: Address HIGH + MEDIUM issues before merge, or explicitly document them as known limitations in the README's "Current Limitations" section.
|
I had claude code review the pr in his extremely unfriendly/sassy mode: Rebalancer Simulation Test Suite - Code ReviewPR Goal: Introduce test suite for validating rebalancer behavior against double rebalances, over-rebalancing, and stress scenarios Review Date: 2026-02-02 Overall Assessment: ✅ Architecture is sound and appropriate for testing timing-related bugs. Needs critical fixes to assertions and validation tests before merge. Executive SummaryThe simulation framework architecture is correct for the stated goals - you cannot unit test timing bugs like double rebalancing and over-rebalancing, so the simulation approach is justified. The implementation shows strong engineering with clean separation of concerns. Critical Issues: Missing assertions in key tests and need for validation tests to prove the simulation itself is correct. Timeline: Estimated 11 hours of work to address blocking issues. Feasible for a one-week merge timeline. Blocking Issues (Must Fix Before Merge)🚨 Issue #1: Inflight Guard Test Has No AssertionsLocation: Problem: This is your PRIMARY test for catching double rebalances and over-rebalancing, but it has no pass/fail criteria. A test without assertions always passes. Current Code: it('inflight-guard: demonstrates slow bridge + fast polling behavior', async function () {
await runScenarioWithRebalancers('inflight-guard', {
anvilRpc: anvil.rpc,
});
// No assertions for now - just generating reports
// The HTML timeline and comparison table show the behavioral difference
});Required Fix: it('inflight-guard: demonstrates slow bridge + fast polling behavior', async function () {
this.timeout(60000);
const { results } = await runScenarioWithRebalancers('inflight-guard', {
anvilRpc: anvil.rpc,
});
const simpleResult = results.find(r => r.rebalancerName === 'SimpleRebalancer');
const prodResult = results.find(r => r.rebalancerName.includes('Production'));
// Scenario: chain1=150, chain2=100, target is 125 each (need to move 25 tokens)
// With 3000ms bridge delay and 200ms polling (15 polls before delivery),
// a rebalancer without inflight guard will send redundant transfers
if (simpleResult) {
console.log(`SimpleRebalancer: ${simpleResult.kpis.totalRebalances} rebalances`);
// SimpleRunner lacks inflight guard - should over-rebalance
// Expect at least 5 redundant transfers (conservative threshold)
expect(simpleResult.kpis.totalRebalances).to.be.greaterThanOrEqual(
5,
'SimpleRunner without inflight guard should send many redundant transfers'
);
}
if (prodResult) {
console.log(`ProductionRebalancer: ${prodResult.kpis.totalRebalances} rebalances`);
// Production rebalancer has ActionTracker (inflight guard)
// Should send 1-2 transfers max (1 initial + maybe 1 adjustment)
expect(prodResult.kpis.totalRebalances).to.be.lessThanOrEqual(
3,
'Production rebalancer with inflight guard should not over-rebalance'
);
}
// If comparing both, the difference should be dramatic
if (simpleResult && prodResult) {
const ratio = simpleResult.kpis.totalRebalances / prodResult.kpis.totalRebalances;
expect(ratio).to.be.greaterThan(
2,
'SimpleRunner should send significantly more transfers than Production'
);
}
});Effort: 2 hours Validation: After implementing, run the test and check console output. If both rebalancers show similar rebalance counts, the production rebalancer's ActionTracker may not be working in simulation (see Issue #2). 🚨 Issue #2: Verify Production Rebalancer's Inflight Guard WorksLocation: Problem: The production Investigation Steps:
If ActionTracker Doesn't Work in Simulation: Option A: Implement a mock Explorer API that BridgeMockController updates // pseudocode
export class MockExplorerApi {
// Called by BridgeMockController when rebalance initiated
registerPendingTransfer(origin, dest, amount): void;
// Called by BridgeMockController when delivered
markDelivered(transferId): void;
// Called by ActionTracker
async getInflightTransfers(origin, dest): Promise<Transfer[]>;
}Option B: Document as a known limitation and skip production rebalancer in inflight-guard test Effort: 2-4 hours (2 for investigation, 2 more if mock needed) 🚨 Issue #3: Add Balance Conservation Validation TestLocation: Create Problem: You're building a simulator to test the rebalancer, but who tests the simulator? Balance conservation is the fundamental invariant - total token supply must remain constant. Required Test: import { expect } from 'chai';
import { ethers } from 'ethers';
import { ERC20Test__factory } from '@hyperlane-xyz/core';
import {
RebalancerSimulationHarness,
ScenarioLoader,
SimpleRunner,
} from '../../src/index.js';
import { setupAnvilTestSuite } from '../utils/anvil.js';
describe('Simulation Validation', function () {
const anvilPort = 8548;
const anvil = setupAnvilTestSuite(this, anvilPort);
it('should conserve total token supply across all transfers and rebalances', async function () {
this.timeout(60000);
const scenario = ScenarioLoader.loadScenario('balanced-bidirectional');
const harness = new RebalancerSimulationHarness({
anvilRpc: anvil.rpc,
initialCollateralBalance: BigInt(scenario.defaultInitialCollateral),
});
await harness.initialize();
// Record initial total supply
const initialTotal = await getTotalSupplyAcrossChains(harness, anvil.rpc);
// Run simulation
const result = await harness.runSimulation(
scenario,
new SimpleRunner(),
{
bridgeConfig: scenario.defaultBridgeConfig,
timing: scenario.defaultTiming,
strategyConfig: scenario.defaultStrategyConfig,
}
);
// Record final total supply
const finalTotal = await getTotalSupplyAcrossChains(harness, anvil.rpc);
console.log(`Initial supply: ${initialTotal}`);
console.log(`Final supply: ${finalTotal}`);
console.log(`Transfers completed: ${result.kpis.completedTransfers}/${result.kpis.totalTransfers}`);
console.log(`Rebalances: ${result.kpis.totalRebalances}`);
// Total supply must be exactly conserved
expect(finalTotal).to.equal(
initialTotal,
'Total token supply must remain constant across all operations'
);
await harness.cleanup();
});
});
async function getTotalSupplyAcrossChains(
harness: RebalancerSimulationHarness,
rpc: string
): Promise<bigint> {
let total = 0n;
const provider = new ethers.providers.JsonRpcProvider(rpc);
for (const domain of Object.values(harness.getDeployment().domains)) {
const token = ERC20Test__factory.connect(domain.collateralToken, provider);
const balance = await token.balanceOf(domain.warpToken);
total += balance.toBigInt();
}
provider.removeAllListeners();
return total;
}Note: You may need to add a Effort: 1 hour 🚨 Issue #4: Test Isolation FailuresLocation: Problem: The comment about timeouts when running full suite indicates test state leakage: // KNOWN LIMITATION:
// When running the full test suite with REBALANCERS=simple,production, some tests
// may timeout due to cumulative state from the ProductionRebalancerRunner.Root Cause: Likely ethers provider or rebalancer instance accumulation despite cleanup attempts. Investigation:
Potential Fixes: Option A: Fresh Anvil per test (most reliable) // In test/utils/simulation-helpers.ts
export async function withFreshAnvil<T>(
testName: string,
fn: (rpc: string) => Promise<T>
): Promise<T> {
// Use a different port for each test
const port = 8545 + Math.floor(Math.random() * 1000);
const anvil = await startAnvil(port);
try {
return await fn(`http://127.0.0.1:${port}`);
} finally {
await anvil.stop();
// Wait for port to be released
await new Promise(resolve => setTimeout(resolve, 100));
}
}Option B: More aggressive cleanup // In cleanupRebalancers()
export async function cleanupRebalancers(): Promise<void> {
await cleanupSimpleRunner();
await cleanupProductionRebalancer();
// Force garbage collection if possible (requires --expose-gc flag)
if (global.gc) {
global.gc();
}
// Longer delay to ensure cleanup completes
await new Promise(resolve => setTimeout(resolve, 500));
}Option C: Skip production rebalancer in full suite, test separately // In package.json scripts
{
"test": "mocha --exit",
"test:simple": "REBALANCERS=simple mocha --exit",
"test:production": "REBALANCERS=production mocha --grep 'inflight-guard'",
"test:all": "pnpm test:simple && pnpm test:production"
}Effort: 3 hours High Priority Issues (Should Fix)
|
| Issue | Priority | Estimated Time |
|---|---|---|
| #1: Inflight guard assertions | BLOCKING | 2 hours |
| #2: Verify ActionTracker works | BLOCKING | 2-4 hours |
| #3: Balance conservation test | BLOCKING | 1 hour |
| #4: Test isolation | BLOCKING | 3 hours |
| #5: Stress test assertions | HIGH | 1 hour |
| #6: No-rebalancer baseline | HIGH | 1 hour |
| #7: Magic number constants | MEDIUM | 30 min |
| #8: Mailbox race condition | MEDIUM | 1 hour |
| #9: Scenario file size | MEDIUM | 1 hour |
Total Blocking: 8-10 hours
Total with High Priority: 10-12 hours
Total with Medium Priority: 12.5-14.5 hours
Recommendation for 1-week timeline: Focus on blocking issues (8-10 hours), add high-priority stress assertions, defer medium priority items to follow-up PR.
Conclusion
The simulation framework is the correct architectural choice for testing timing-related rebalancer bugs. The implementation shows strong engineering fundamentals.
Key Strengths:
- Appropriate tool for the problem (can't unit test timing bugs)
- Clean architecture with good separation of concerns
- Thoughtful scenario coverage
Key Gaps:
- Missing assertions in critical tests
- No validation tests for simulator correctness
- Test isolation issues need debugging
Recommendation: ✅ Approve with required changes
Fix the blocking issues (especially assertions and validation tests) and this becomes a strong test suite that catches the exact bugs you're concerned about: double rebalances, over-rebalancing, and stress scenarios.
Questions for Developer
-
ActionTracker: Does the production
RebalancerServiceActionTracker work in simulation, or does it need external dependencies (Explorer API, database)? -
Test Timeouts: Have you profiled what's causing timeouts when running the full suite? Is it provider accumulation, memory leaks, or something else?
-
Scenario Files: Are the large JSON scenario files intentionally committed, or should they be generated on-demand?
-
Coverage Goals: What specific production incidents are you trying to prevent? This should drive what additional assertions are needed.
-
CI Strategy: Will this run in CI on every PR, or only manually/nightly? Runtime might influence test count.
Reviewed By: AI Code Reviewer (Claude)
Review Date: 2026-02-02
PR Status: Needs revision before merge
Follow-up: Developer to address blocking issues and respond to questions
- Remove misleading snapshot restore comment (no longer using snapshots) - Fix race condition by removing duplicate processReadyMailboxDeliveries call - Surface ProductionRebalancerRunner startup errors instead of swallowing - Remove outdated KNOWN LIMITATION comment from test file - Document ActionTracker limitation in README - Add NoOpRebalancer baseline test to show behavior without rebalancing Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Addressed all review comments in eb05905: @paulbalaji's comments:
@xeno097's comments: Deferred items (follow-up PRs):
|
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
typescript/cli/src/tests/ethereum/warp/warp-rebalancer.e2e-test.ts (1)
1117-1117:⚠️ Potential issue | 🟡 MinorComment says one thing, code says another.
The comment claims "10s" but the timeout's actually 30000ms, which is 30 seconds. Small thing, but might confuse someone later who's wonderin' why the test seems to wait forever.
📝 Suggested fix
- const timeout = 30000; // 10s + const timeout = 30000; // 30s
🤖 Fix all issues with AI agents
In `@typescript/rebalancer-sim/scenarios/moderate-imbalance-chain1.json`:
- Around line 3-4: The scenario's "description" claims 70% of transfers go TO
chain1 but the transfers list (imb-000000 through imb-000014) actually sends 13
of 15 (~87%) to chain1; either update the "description" (and "expectedBehavior"
text) to reflect ~87% to chain1 or modify the transfers (e.g., change two of the
transfers currently targeting chain1 such as one of imb-00000x entries) so that
11 of 15 (~70%) go to chain1; ensure the "description" and "expectedBehavior"
remain consistent with the final transfer distribution.
In `@typescript/rebalancer-sim/src/RebalancerSimulationHarness.ts`:
- Around line 175-180: The JsonRpcProvider instance named provider is created
with custom polling settings but never used or cleaned up; either remove the
unused provider creation (the lines creating provider and setting
pollingInterval/polling) or ensure the code uses that provider when recreating
this.engine (replace new ethers.providers.JsonRpcProvider(this.config.anvilRpc)
with the provider) and stop constructing a second provider; if you keep the
shared provider, also ensure you remove any listeners or properly dispose it
when torn down (use provider.removeAllListeners()/appropriate cleanup) so it
doesn't hang around.
🧹 Nitpick comments (7)
typescript/rebalancer-sim/scenarios/balanced-bidirectional.json (1)
213-238: Weights don't quite add up to one - might want to double-check that.The weights for the three chains sum to
0.333 + 0.333 + 0.333 = 0.999, not exactly1.0. Now, this is probably fine for most weighted-balance implementations since they'll normalize internally, but it's worth knowin' about in case some algorithm expects weights to sum precisely to unity.Consider whether
0.334for one chain (or using higher precision like0.3333333) would be cleaner for documentation purposes, or verify the rebalancer normalizes weights internally.typescript/rebalancer-sim/scenarios/surge-to-chain1.json (1)
7-288: Consider generating scenario data on-demand rather than committing static JSON.This is a 360+ line static data file with 35 hardcoded transfers. Per earlier reviewer feedback, committing large scenario files can bloat the repository over time, especially if more scenarios are added. Since the
ScenarioGeneratoralready exists in this package, it might make more sense to generate these deterministically at test runtime (using a fixed seed for reproducibility) rather than storing them verbatim.That said, having a committed reference scenario isn't the worst thing in the swamp - it does make the test data reviewable and the scenario behavior explicit. Just something to keep in mind as more scenarios get added.
typescript/rebalancer-sim/src/runners/ProductionRebalancerRunner.ts (1)
48-100: Consider stricter typing for the strategy config builders.The
chains: Record<string, any>patterns here work, but using explicit types would make the code more self-documenting and catch mismatches earlier. Not blocking since this is internal implementation, just something to consider when you've got time.typescript/rebalancer-sim/src/SimulationEngine.ts (1)
185-197: Consider adding debug logging to these cleanup catches.The other silent catches in the codebase got debug logging added, but these cleanup catches for
rebalancer.stop()andbridgeController.stop()are still silent. Not critical since cleanup errors are usually ignorable, but consistent debug logging would help when tracking down mysterious test failures.Optional improvement
try { await rebalancer.stop(); - } catch { - // Ignore stop errors + } catch (error) { + logger.debug({ error }, 'rebalancer.stop() failed during cleanup'); } if (this.bridgeController) { try { await this.bridgeController.stop(); - } catch { - // Ignore stop errors + } catch (error) { + logger.debug({ error }, 'bridgeController.stop() failed during cleanup'); } }typescript/rebalancer-sim/src/RebalancerSimulationHarness.ts (1)
214-216: Consider using a named constant for the max bigint sentinel.The magic hex value works but isn't immediately obvious what it represents. A named constant would be clearer.
Optional improvement
+const MAX_GAS_COST_SENTINEL = BigInt('0xffffffffffffffffffffffffffffffff'); + private generateComparison( results: SimulationResult[], ): ComparisonReport['comparison'] { let bestCompletionRate = ''; let bestLatency = ''; let lowestGasCost = ''; let maxCompletionRate = -1; let minLatency = Infinity; - let minGasCost = BigInt('0xffffffffffffffffffffffffffffffff'); + let minGasCost = MAX_GAS_COST_SENTINEL;typescript/rebalancer-sim/test/utils/simulation-helpers.ts (1)
152-155: Consider adding proper types instead ofanyfor better swamp... er, type safety.The
Record<string, any>here andanytypes later (lines 311, 356) lose compile-time checking. Since this is test utility code, it's not critical, but proper typing would catch shape mismatches earlier.♻️ Optional type improvement for strategyConfig
+import type { ChainStrategyConfig } from '../../src/types.js'; + const strategyConfig = { type: file.defaultStrategyConfig.type, - chains: {} as Record<string, any>, + chains: {} as Record<string, ChainStrategyConfig>, };typescript/rebalancer-sim/test/integration/full-simulation.test.ts (1)
170-180: Consider extractin' that magic number to a constant.The
0.1threshold for completion rate difference is clear in context, but a named constant likeCOMPLETION_RATE_TOLERANCE = 0.1would make the intent even clearer and easier to adjust if needed.♻️ Optional: Extract threshold constant
+const COMPLETION_RATE_TOLERANCE = 0.1; // 10% tolerance for cross-rebalancer comparison + // When comparing, completion rates should be similar if (activeResults.length > 1) { const completionDiff = Math.abs( activeResults[0].kpis.completionRate - activeResults[1].kpis.completionRate, ); expect(completionDiff).to.be.lessThan( - 0.1, + COMPLETION_RATE_TOLERANCE, 'Completion rates should be within 10% of each other', ); }
…8003) Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@typescript/rebalancer-sim/src/SimulationEngine.ts`:
- Around line 318-326: The interval callback starting the mailbox tick may yield
an unhandled rejection if processReadyMailboxDeliveries() throws; modify the
async callback that uses mailboxProcessingInterval and mailboxProcessingInFlight
to catch errors from processReadyMailboxDeliveries(), logging the error (e.g.,
via console.error or the existing logger) inside a catch block so the finally
block still clears mailboxProcessingInFlight; keep the existing overlapping-tick
guard and ensure the catch prevents unhandled promise rejections.
|
I have some refactoring of the |
Co-authored-by: Nam Chu Hoai <nam@hyperlane.xyz>
⚙️ Node Service Docker Images Built Successfully
Full image paths |
5 participants
Description
New
typescript/rebalancer-simpackage providing a fast real-time simulation framework for testing warp route rebalancers against synthetic transfer scenarios.Key components:
Architecture highlights:
Drive-by changes
None
Related issues
None
Backward compatibility
Yes - this is a new package with no changes to existing code.
Testing
Unit Tests + Integration Tests
RUN_ANVIL_TESTS=1):Run tests with:
Summary by CodeRabbit