fix(InventoryClient): clamp excess L2 withdrawal to liquid on-chain balance#3483
fix(InventoryClient): clamp excess L2 withdrawal to liquid on-chain balance#3483droplet-rl wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7ac3b32f1c
ℹ️ 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 amountToWithdraw = desiredWithdrawalAmount.lte(liquidL2Balance) | ||
| ? desiredWithdrawalAmount | ||
| : liquidL2Balance; |
There was a problem hiding this comment.
Reserve shortfall before clamping liquid withdrawals
When pending rebalance credits make desiredWithdrawalAmount larger than the liquid balance, this clamp can withdraw the entire on-chain balance even if that balance is needed for an outstanding shortfall. getCurrentAllocationPct(..., false) subtracts tokenClient.getShortfallTotalRequirement, but after a large pending credit the computed desired amount can still exceed liquid; e.g. with 100 liquid, 100 shortfall, and a large pending inbound credit, this line chooses 100 and drains the chain, leaving the shortfall unfunded until the pending transfer settles. The liquid cap should reserve the chain's current shortfall (or otherwise avoid withdrawing shortfall-needed liquidity) before submitting the bridge call.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — fixed in the follow-up commit.
getCurrentAllocationPct(..., ignoreShortfall=false) discounts shortfall in the virtual world, so the unclamped desiredWithdrawalAmount is shortfall-safe. The bug was that min(desired, liquid) could reach back into liquid that was earmarked for outstanding fills once pending credits inflated desired above liquid. Now sizing off max(0, liquid - shortfall):
- preserves the shortfall reservation already baked into the virtual-balance math
- still clamps below virtual so the bridge sim can't revert with
ERC20: transfer amount exceeds balance - skips the cycle when shortfall fully claims liquid
Added two regression tests covering the partial-shortfall (clamp to liquid - shortfall) and full-shortfall (skip entirely) cases, plus a docs update in inventory-virtual-balance-model.md spelling the rule out.
|
@codex addressed the inline feedback on |
…ess L2 withdrawal `withdrawExcessBalances` clamped the L2->L1 transfer amount to the relayer's liquid on-chain balance, but the chain's outstanding shortfall (deposits the relayer has promised to fill but hasn't yet funded) was not reserved during the clamp. `getCurrentAllocationPct(..., ignoreShortfall=false)` already discounts shortfall in the *virtual* world, so the unclamped `desiredWithdrawalAmount` is shortfall-safe. But once large pending inbound rebalance credits push `desiredWithdrawalAmount` above liquid, the previous `min(desired, liquid)` could pick liquid that's still earmarked for those outstanding fills, draining the chain and leaving shortfall unfunded until the pending credits settle. Sizing the transfer off `max(0, liquid - shortfall)` instead: - preserves the shortfall reservation that the virtual-balance math already assumed - still clamps below virtual so the bridge simulation can't revert with `ERC20: transfer amount exceeds balance` - skips the cycle when shortfall fully claims liquid (the next cycle picks it up once shortfall fills draw it down or the pending credits arrive) Adds two regression tests: - "Reserves outstanding shortfall when clamping excess withdrawal to liquid balance" — partial-shortfall case - "Skips excess withdrawal when shortfall fully claims the liquid on-chain balance" — full-shortfall case Also updates `docs/inventory-virtual-balance-model.md` so the "trigger vs sizing" section spells the rule out explicitly. Addresses Codex review feedback on PR #3483. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
Thanks for the re-review. |
3547773 to
a8c53c4
Compare
…tfall `withdrawExcessBalances` sized the L2->L1 bridge call off virtual balance (actual + pending inbound rebalance credits + pending L2->L1 withdrawal credits) and handed the result to the bridge adapter without comparing it to what's actually free to leave the chain. Two failure modes: 1. Pending inbound rebalance credits haven't settled yet — the bridge's transferFrom reverts at simulation with `ERC20: transfer amount exceeds balance`. This is the symptom that's been spamming #zbot-across-error since ~2026-05-23. 2. The chain has an outstanding shortfall (deposits the relayer promised to fill but hasn't funded yet). Pulling liquid back to L1 concurrently with promised fills either starves those fills or undoes a rebalancer that's already moving funds in to cover the shortfall. Fix: - Skip the withdrawal entirely while the chain has any outstanding shortfall. - Otherwise, cap the desired withdrawal at the relayer's liquid on-chain balance for this token. Trigger stays virtual-aware (so once the inbound settles or the shortfall is consumed by fills we re-evaluate next cycle), but the transfer is now bounded by what's actually free to send. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
a8c53c4 to
2328f72
Compare
| // Cap the desired withdrawal at the relayer's liquid on-chain balance. Sizing off | ||
| // virtual balance would revert at simulation because pending inbound rebalance | ||
| // credits haven't actually settled on-chain yet. |
There was a problem hiding this comment.
This is wrong - doing so would leave the operator with 0 tokens on this chain, whereas the intention is to rebalance down to the desired threshold.
Therefore, the withdrawal amount should reduce the liquid token balance down to the target allocation.
There was a problem hiding this comment.
Good catch — fixed in the follow-up commit.
Replaced min(desired, liquid) with min(desired, max(0, liquid - targetAmount)), so the post-withdrawal liquid balance stays at the chain's target allocation instead of going to zero. Still bounds the transfer below settled liquidity (so the bridge call doesn't revert at simulation), but no longer strips the operational reserve when the desired (virtual-derived) amount exceeds liquid.
Tests updated to assert the new clamp:
Caps the withdrawal at liquid-minus-target when pending rebalance credits inflate the virtual balance— binding clamp, expected =liquid - targetSkips the withdrawal when liquid balance is at or below target allocation— degenerate case where pending credits trigger the threshold but liquid is already inside the target reserve
Kept the skip-on-shortfall guard since that's a separate concern (don't withdraw at all while promised fills are outstanding). Happy to also fold that into the unified clamp (liquid - max(target, shortfall)) if you'd prefer — let me know.
…et, not raw liquid Per @pxrl's review, clamping the L2->L1 transfer at `liquidL2Balance` could drain the chain to zero when `desiredWithdrawalAmount` (sized off virtual balance) exceeds what's actually settled on-chain. The intent of `withdrawExcessBalances` is to bring the chain *down to* its target allocation, not to strip it of its operational reserve. Replace `min(desired, liquid)` with `min(desired, max(0, liquid - target))`: - still bounds the transfer by what's settled on-chain (so the bridge call doesn't revert at simulation with `ERC20: transfer amount exceeds balance` when pending inbound rebalance credits inflate the virtual sizing) - additionally keeps the chain's target allocation in liquid form, so the operator's structural reserve isn't drained and in-flight fills aren't starved between this withdrawal and the pending credits actually settling The existing skip-on-shortfall guard stays in place (separate concern: don't withdraw at all while promised fills are outstanding). Tests updated/added: - `Caps the withdrawal at liquid-minus-target when pending rebalance credits inflate the virtual balance` — covers the binding clamp case (liquid > target, pending pushes desired above liquid - target) - `Skips the withdrawal when liquid balance is at or below target allocation` — covers the degenerate case where pending credits trigger the threshold but liquid is already inside the target reserve Docs: re-add the "Triggering vs sizing" section to `inventory-virtual-balance-model.md` so the rule is documented at the inventory level. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
Addressed @pxrl's inline feedback. The clamp now caps the withdrawal at |
Summary
InventoryClient#withdrawExcessBalancessized the L2→L1 bridge call off virtual balance (actual + pending inbound rebalance credits + pending L2→L1 withdrawal credits) and submitted that amount without comparing it to the relayer's settled on-chain balance.pendingRebalances[chainId][symbol]inflates that chain's virtual balance, thecurrentAllocPct - targetPctsizing overshoots what's actually on-chain, and the bridge call reverts at simulation time withERC20: transfer amount exceeds balance.tokenClient.getBalance(chainId, l2Token). If liquid is zero we skip the cycle and wait for the credits to actually land on-chain.Production symptom
The
zion-across-fast-relayer-rebalancerbot has been emittingMultiCallerClient#LogSimulationFailuresevery ~5 minutes in#zbot-across-error(e.g. parent ts1781167257.515209). The reverting tx is the CCTPdepositForBurnon the HyperEVM/Arb token messenger at0x28b5a0e9C621a5BadaA536219b3a228C8168cf5d. Today's snapshot:0x07ae8551be970cb1cca11dd7a11f47ae82e70e67on HyperEVM has 134,143.96 USDC on-chain (balanceOfagainst0xb88339CB…)Executed excess L2 inventory withdrawallog proposes 602,758.70 USDC (current alloc 35.33%, target 8%, $2.2M cumulative)pendingRebalances[HYPEREVM]["USDC"]— credits from in-flight CCTP/OFT orders that haven't settledrebalanceInventoryIfNeededalready does the right thing on the L1→L2 side via theunallocatedBalancecheck (InventoryClient.ts:1109); this PR brings the L2→L1 path in line.Test plan
yarn typecheckyarn lintyarn hardhat test test/InventoryClient.InventoryRebalance.ts— 18 passing, including two new tests:Clamps excess withdrawal to liquid on-chain balance when pending rebalance credits inflate the virtual balanceSkips excess withdrawal when there is no liquid on-chain balance#zbot-across-error—fast-relayer-rebalancersimulation reverts should stop and the bot should fall back to withdrawing whatever's actually on-chain each cycle.Docs
docs/inventory-virtual-balance-model.mdgets a new "Triggering vs sizing" section documenting the virtual-trigger-but-liquid-transfer rule so future inventory-side mutations don't regress this.🤖 Generated with Claude Code