fix(InventoryClient): exclude pendingRebalances when ignoreL1ToL2PendingAmount is set#3492
fix(InventoryClient): exclude pendingRebalances when ignoreL1ToL2PendingAmount is set#3492droplet-rl wants to merge 2 commits into
Conversation
…ingAmount is set The July-2025 guardrail in `getBalanceOnChain` excludes canonical L1→L2 pending transfers from the virtual chain balance when the caller asks for a liquidity-faithful view (notably `withdrawExcessBalances` via `getCurrentAllocationPct`). The Feb-2026 `pendingRebalances` integration introduced a second source of virtual inbound — rebalancer-service pending bridges to a chain — which the same guardrail did not cover. As the swap rebalancer became active in production, that second channel began inflating `currentAllocPct` for chains receiving in-flight CCTP/OFT inbound, triggering `withdrawExcessBalances` to size a withdraw sized off the virtual balance against a smaller liquid balance, reverting the resulting `depositForBurn` with `ERC20: transfer amount exceeds balance`. Extend the same `ignoreL1ToL2PendingAmount` flag to also skip the `pendingRebalances[chainId][token]` contribution so both virtual-inbound sources are filtered together. Callers that intentionally want to count inbound (e.g. `_getPossibleInventoryRebalances` to avoid duplicate L1→L2 refills) keep the flag at its default `false` and see the same behavior as before. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
droplet-rl
requested review from
bmzig,
dijanin-brat,
mrice32,
nicholaspai and
pxrl
as code owners
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6fbbb327b7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const pendingRebalancesForChain = this.pendingRebalances[chainId]; | ||
| const canonicalL2Token = getRemoteTokenForL1Token(l1Token, chainId, this.hubPoolClient.chainId); | ||
| if ( | ||
| !ignoreL1ToL2PendingAmount && |
There was a problem hiding this comment.
When ignoreL1ToL2PendingAmount is true this now skips every pendingRebalances entry, but some adapters intentionally return negative amounts rather than inbound credits. In HyperliquidStablecoinSwapAdapter.getPendingRebalances, finalized Hypercore withdrawals are subtracted from HyperEVM (src/rebalancer/adapters/hyperliquid.ts:780-787) so intermediate funds that have physically landed are not treated as free inventory while the pending order still needs them bridged onward. withdrawExcessBalances calls this path with true, so on HyperEVM those reserved balances can now be counted as excess and withdrawn away from the pending rebalance; only positive virtual inbound amounts should be suppressed here.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — fixed in ada8e07. The new logic only suppresses positive pendingRebalances entries when ignoreL1ToL2PendingAmount=true; negative entries (the kind that HyperliquidStablecoinSwapAdapter writes at hyperliquid.ts:780-787 to reserve a finalized intermediate withdrawal for the still-pending onward bridge step) are kept so withdrawExcessBalances cannot count them as excess. Added a case 5 to InventoryClient.InventoryRebalance.ts that sets a negative pendingRebalances entry and asserts the balance still reflects it with the flag set.
| * @param ignoreL1ToL2PendingAmount If true, exclude virtual inbound balance that hasn't physically landed | ||
| * on the target chain yet. Currently two sources contribute virtual inbound: canonical L1→L2 transfers | ||
| * from `crossChainTransferClient`, and rebalancer-service pending rebalances from `pendingRebalances`. |
There was a problem hiding this comment.
This changes the documented behavior of the core virtual-balance primitive, but only the inline JSDoc was updated. AGENTS.md requires relevant README/AGENTS docs to be updated when behavior changes, and src/clients/README.md:28-30 still tells operators that incomplete rebalancer-client rebalances are part of InventoryClient virtual balances without mentioning that callers can now suppress them via this flag; that leaves the module docs misleading for contributors debugging withdrawExcessBalances and allocation calculations.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Updated src/clients/README.md (the "Setting and Getting Virtual Balances" section) and docs/inventory-virtual-balance-model.md (the getBalanceOnChain primitive section) in ada8e07 to call out both that ignoreL1ToL2PendingAmount now suppresses positive pendingRebalances entries alongside outstanding CrossChainTransferClient transfers, and that negative pendingRebalances entries are kept by design so adapter-driven reservations (e.g. the Hyperliquid stablecoin swap subtraction on HyperEVM) survive the filter.
…L1ToL2PendingAmount is set Codex review on #3492 flagged that suppressing every pendingRebalances entry under `ignoreL1ToL2PendingAmount` would also drop the negative reservations that adapters like `HyperliquidStablecoinSwapAdapter` push to the destination chain when an intermediate withdrawal finalizes but the onward bridge step is still pending. Treating that reserved balance as excess would let `withdrawExcessBalances` size a withdraw against funds earmarked for the in-flight rebalance. Only suppress positive pendingRebalances entries; keep negative entries in the virtual balance regardless of the flag. Document the new semantics in the JSDoc, the clients README, and the virtual-balance model deep dive, and add a regression test asserting that a negative pending rebalance is preserved with `ignoreL1ToL2PendingAmount=true`. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
@codex addressed both review notes in ada8e07.
Lint and |
Summary
No additional code changes were needed, so I did not create a new commit or PR. Testing
|
|
Thanks for re-verifying. For the record on the blocked tooling step: |
1 participant
Summary
Extend the existing
ignoreL1ToL2PendingAmountguardrail ingetBalanceOnChainso it also excludes the rebalancer-servicependingRebalancescontribution — not justcrossChainTransferClientcanonical L1→L2 pending. Both inflate a chain's perceived balance with in-flight inbound that hasn't physically arrived; the existing guardrail only filtered the canonical-bridge side, leaving the rebalancer-service side as an unfixed second channel.Background
The July-2025 PR #2398 added
ignoreL1ToL2PendingAmount=truetogetCurrentAllocationPctinwithdrawExcessBalances, so that pending canonical L1→L2 transfers don't trigger spurious L2→L1 withdraws ("on the margin might cause a withdrawal of L2 balances to be delayed but it also means we are less likely to cancel out pending L1 to L2 transfers by immediately withdrawing back to L1").The Feb-2026 PR #2944 integrated
RebalancerClient.getPendingRebalancesintoInventoryClient.getBalanceOnChain(InventoryClient.ts:306-318), introducing a second source of virtual inbound. As the swap rebalancer became active in production in spring 2026, this second source began inflatingcurrentAllocPctfor chains receiving CCTP/OFT inbound from the rebalancer service. ThewithdrawExcessBalancespath then sized a withdraw off the inflated cumulative balance, and the resultingdepositForBurnreverted withERC20: transfer amount exceeds balancebecause the on-chain ERC20 balance hadn't grown to match.First occurrence in
bots-across-3839GCP logs is 2026-06-02T16:46:10Z on Base (483,023 USDC); it has since recurred on HyperEVM (578,094 USDCtoday) every ~5 minutes.Change
getBalanceOnChainnow treatspendingRebalances[chainId][token]the same way it treatscrossChainTransferClient.getOutstandingCrossChainTransferAmount: skip the addition whenignoreL1ToL2PendingAmount=true. Callers that intentionally want to count inbound (notably_getPossibleInventoryRebalancesat:1017, which passesfalseto avoid duplicate L1→L2 refills) are unchanged.The JSDoc on
getBalanceOnChainis updated to spell out both sources of virtual inbound that the flag controls.What this fix does and doesn't cover
withdrawExcessBalancestrigger on chains that are over-target only because rebalancer-service pending inbound is being counted. This is the dominant production symptom._getPossibleInventoryRebalancespath (separate option E in the design discussion), nor the L1→L2 "succeeded simulation but failed to submit" race (separate "single-loop submit + per-submitbalanceOf" follow-up), nor themaxL2WithdrawalVolumerate-limit overshoot (separate follow-up).Test plan
yarn typecheckpassesyarn lintpassesyarn hardhat test test/InventoryClient.InventoryRebalance.tspasses (16/16, including extended "cross chain swap rebalances" test with new Case 4 asserting the flag now excludespendingRebalances)bots-across-3839for🚧 Token balance on mainnet changedand thedepositForBurn ... exceeds balancecluster on chains 56/999/8453/42161 once deployed; both should fall sharply as the trigger no longer fires on rebalancer-service-induced over-allocation.🤖 Generated with Claude Code