Skip to content

improve(InventoryClient): skip rebalances with no L1 -> L2 bridge route#3566

Open
droplet-rl wants to merge 2 commits into
masterfrom
droplet/C073ELHSDQ8-1783702598-119989
Open

improve(InventoryClient): skip rebalances with no L1 -> L2 bridge route#3566
droplet-rl wants to merge 2 commits into
masterfrom
droplet/C073ELHSDQ8-1783702598-119989

Conversation

@droplet-rl

Copy link
Copy Markdown
Contributor

Motivation

Since #3564 enabled USDT on Avalanche in SUPPORTED_TOKENS, the rebalancer errors on every run:

AssertionError [ERR_ASSERTION]: Token 0xdAC17F958D2ee523a2206206994597C13D831ec7 is not supported
    at BaseChainAdapter.sendTokenToTargetChain

Avalanche USDT is L2 -> L1 withdrawal-only (via Binance) — there is deliberately no entry in CUSTOM_BRIDGE[AVALANCHE] and Avalanche has no canonical bridge. But InventoryClient.rebalanceInventoryIfNeeded selects rebalances purely from the inventory config, so whenever Avalanche USDT sits below its target allocation it attempts the L1 -> L2 send and trips the assert. The assert message is also misleading: it fires identically for "missing from SUPPORTED_TOKENS" and "no L1 -> L2 bridge configured", which makes it look like the SUPPORTED_TOKENS change didn't take effect.

Slack context: https://riskmarkets.slack.com/archives/C073ELHSDQ8/p1783702598119989

Changes

  • InventoryClient.rebalanceInventoryIfNeeded skips rebalances for (token, chain) pairs with no L1 -> L2 bridge and logs a warning pointing at the config mismatch, instead of throwing at transfer time. Same skip pattern chore: Enable withdrawing USDT via Binance from AVAX #3564 applied to getOutstandingCrossChainTransfers.
  • New AdapterManager.canSendTokenCrossChain / BaseChainAdapter.isSupportedL1Bridge helpers (mirroring the existing isSupportedL2Bridge).
  • Split the sendTokenToTargetChain assertion so the two failure modes produce distinct messages.
  • Doc note in src/clients/README.md on L2 -> L1-only tokens.

Note this does not change which rebalances are possible — sends that previously threw are now skipped with a warn. If we later want L1 -> AVAX USDT rebalancing, that's a separate bridge-route addition (e.g. an L1-side Binance route).

Testing

  • New test: InventoryClient.InventoryRebalance.ts — "Skips rebalances when the token has no L1 -> L2 bridge on the destination chain" (skip + warn, then executes normally once a route exists).
  • yarn typecheck, eslint + prettier on changed files, and the InventoryClient.InventoryRebalance, InventoryClient.RefundChain, Monitor suites all pass locally (64 passing).

🤖 Generated with Claude Code

The rebalancer errors on every run with a generic "Token ... is not
supported" assertion when the inventory config targets a token on a
chain that has no L1 -> L2 bridge (e.g. Avalanche USDT, which is
L2 -> L1 withdrawal-only via Binance).

- InventoryClient now skips such rebalances and logs a warning naming
  the config mismatch instead of throwing at transfer time.
- Split the sendTokenToTargetChain assertion so a missing L1 -> L2
  bridge is distinguishable from a token missing from SUPPORTED_TOKENS.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 317a61d5a3

ℹ️ 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".

Comment thread src/clients/InventoryClient.ts Outdated
Comment on lines +973 to +975
if (!this.adapterManager.canSendTokenCrossChain(chainId, l1Token)) {
unbridgeableRebalances.push(rebalance);
continue;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Preserve unbridgeable rebalances for same-asset routes

When runSameAssetRebalancer is used for the supported Avalanche USDT route, it gets its work items from inventoryClient.rebalanceInventoryIfNeeded(true) and then routes them through Binance (src/rebalancer/clients/SameAssetRebalancerClient.ts:49-54, src/rebalancer/buildSameAssetRebalanceRoutes.ts:7-10). This new guard filters out exactly those no-canonical-bridge imbalances before returnRebalancesOnly can hand them to the same-asset rebalancer, so Avalanche USDT deficits are only warned about and never initialized through the configured Binance route. The skip should apply only to direct InventoryClient sends, or otherwise preserve these rebalances for callers that execute alternate routes.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch — confirmed: SameAssetRebalancerClient collects work items via rebalanceInventoryIfNeeded(true) and executes them through Binance, so the guard was filtering out exactly the rebalances that client exists to handle. Fixed in a096ac6: the skip now only applies when the InventoryClient executes the send itself (returnRebalancesOnly === false); collect-only callers still receive unbridgeable rebalances. Added a test covering the returnRebalancesOnly path and softened the warning to note the route may be handled by a rebalancer bot.

…lancesOnly callers

The SameAssetRebalancerClient collects rebalances via
rebalanceInventoryIfNeeded(true) and executes them through alternate
routes (e.g. Avalanche USDT via Binance). The unbridgeable-rebalance
skip was filtering those out before they could be returned, so such
deficits would only ever be warned about and never rebalanced.

Only apply the skip when the InventoryClient executes the send itself
(returnRebalancesOnly === false), and soften the warning to note the
route may be handled by a rebalancer bot.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@droplet-rl

Copy link
Copy Markdown
Contributor Author

Addressed the Codex review finding in a096ac6. The P1 was valid: the new unbridgeable-rebalance guard also filtered rebalances out of the rebalanceInventoryIfNeeded(true) return path, which is how SameAssetRebalancerClient collects the work items it executes via Binance (e.g. Avalanche USDT). The skip now only applies when the InventoryClient executes the send itself; collect-only callers still receive these rebalances. Added a regression test for the returnRebalancesOnly path and updated the README note and warning message accordingly.

@codex review

@droplet-rl

Copy link
Copy Markdown
Contributor Author

🔎 View trace

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 🚀

Reviewed commit: a096ac6c41

ℹ️ 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".

@droplet-rl

Copy link
Copy Markdown
Contributor Author

Codex re-review of a096ac6 came back clean. The PR now fixes the original "Token ... is not supported" crash while preserving the returnRebalancesOnly path that the SameAssetRebalancerClient relies on for Binance-routed rebalances. Ready for human review.

@droplet-rl

Copy link
Copy Markdown
Contributor Author

🔎 View trace

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant