Skip to content

fix(rebalancer): tie Binance swap-deposit tag lifecycle to order state#3560

Merged
nicholaspai merged 9 commits into
masterfrom
droplet/C0AFB0BP3SB-1783490172-528569-fix2
Jul 8, 2026
Merged

fix(rebalancer): tie Binance swap-deposit tag lifecycle to order state#3560
nicholaspai merged 9 commits into
masterfrom
droplet/C0AFB0BP3SB-1783490172-528569-fix2

Conversation

@droplet-rl

Copy link
Copy Markdown
Contributor

Problem

_depositToBinance tags each swap deposit SWAP in Redis so the Binance finalizer excludes it from its amountToFinalize ledger — but the tag expired on a 30-minute wall clock, while the finalizer's deposit-history lookback is far longer. Once the tag lapsed, getBinanceDepositType returned UNKNOWN and the deposit was re-counted as a bridge deposit awaiting withdrawal — even when its swap had completed and the funds were long gone. This phantom debt was then paid out of the next order's just-credited deposit, starving that order until REBALANCER_PENDING_ORDER_TTL pruned it.

Observed 7 times over 2026-07-01 → 07-08 on USDT→USDC (HyperEVM) routes. Traced example (cloid 0xe42d273cb1ce29c53aad23e9241606cd, 2026-07-08): its 82,385.72 USDT deposit (tx) was credited at ~04:58 and withdrawn back to BSC by the finalizer at 04:59:27 against a 158k phantom backlog created by the previous, successfully completed order (0x32e9b903, swap filled 04:16, finalized 04:21, tag expired ~04:31). The starved order was pruned at 05:56:12 — the PagerDuty alert that kicked this off.

Change

Tag lifecycle now follows order state instead of wall clock (the withdrawal-side SWAP tag already worked this way — set with the Redis default 1-week TTL):

  • DepositsetBinanceDepositType(..., SWAP) without explicit TTL → Redis default (1 week), which outlives the finalizer lookback. A cloid → {chainId, transactionHash} mapping is stored alongside the order state so later transitions can find the tag.
  • Order abandoned in PENDING_DEPOSIT (TTL prune) — new BaseAdapter._onExpiredOrderPruned hook (no-op by default, wrapped so it can't break the shared read path); the Binance adapter deletes the deposit's tag, handing the unconsumed funds to the finalizer for reclaim on its next run.
  • Order progressed past PENDING_DEPOSIT — the deposit was consumed by the spot order; the tag stays, so the finalizer's ledger never re-counts consumed deposits.

Behavior change: stuck-funds recovery starts when the order dies (REBALANCER_PENDING_ORDER_TTL, default 1h) rather than 30 minutes after deposit. This also removes the race where the finalizer could reclaim a live order's deposit that was legitimately waiting out Binance's deposit-unlock confirmations (RW00441).

Not touched: scripts/swapOnBinance.ts (interactive operator flow manages its own tag TTL); orders created before this change have no txn mapping — the prune hook logs and skips, falling back to today's behavior.

Replaces #3559, which protected the balance from the phantom debt but didn't stop the phantom debt from forming.

Testing

  • yarn tsc --noEmit clean
  • hardhat test test/BinanceAdapter.helpers.ts test/BinanceAdapter.withdrawals.ts test/BinanceUtils.ts test/BinanceFinalizer.ts — 48 passing
  • eslint + prettier clean on touched files

🤖 Generated with Claude Code

The SWAP tag set by _depositToBinance expired on a 30-minute wall clock
while the Binance finalizer's deposit-history lookback is far longer, so
every swap deposit — including successfully swapped ones — was later
re-counted into the finalizer's amountToFinalize ledger as if it were a
bridge deposit awaiting withdrawal. This phantom debt was then paid out
of the next order's just-credited deposit, starving that order until its
REBALANCER_PENDING_ORDER_TTL pruned it (7 occurrences 2026-07-01..08 on
USDT->USDC HyperEVM routes; e.g. cloid
0xe42d273cb1ce29c53aad23e9241606cd).

Make the tag lifecycle follow order state instead of wall clock:

- Deposit: tag SWAP with the Redis default TTL (1 week, matching the
  withdrawal-side tag) so it outlives the finalizer lookback, and store
  a cloid -> deposit txn mapping alongside the order state.
- Order abandoned in PENDING_DEPOSIT (TTL prune): a new
  BaseAdapter._onExpiredOrderPruned hook lets the Binance adapter delete
  the tag, handing the unconsumed deposit to the finalizer for reclaim.
- Order progressed past PENDING_DEPOSIT: the deposit was consumed by the
  spot order, so the tag stays and the ledger remains accurate.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment thread src/rebalancer/adapters/baseAdapter.ts Outdated
Per review feedback, let hook errors propagate instead of swallowing
them with a warn log.

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

Copy link
Copy Markdown
Contributor Author

Addressed the review in bdf7ede: removed the try-catch (and its comment) around the _onExpiredOrderPruned hook call in baseAdapter.ts, so hook errors now propagate instead of being swallowed with a warn log. Replied on the inline thread as well.

@droplet-rl

Copy link
Copy Markdown
Contributor Author

🔎 View trace

Comment thread src/rebalancer/adapters/binance.ts Outdated
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@droplet-rl

Copy link
Copy Markdown
Contributor Author

Applied the suggestion in 3236701: the "Released SWAP deposit tag" log in _onExpiredOrderPruned now emits at debug level instead of info, matching the other log in that method. Replied on the inline thread as well.

@droplet-rl

Copy link
Copy Markdown
Contributor Author

🔎 View trace

@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: 93c1416cef

ℹ️ 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/rebalancer/adapters/binance.ts Outdated
The SWAP tag previously relied on the Redis default TTL (1 week), which
is shorter than the Binance finalizer's default 14-day deposit-history
lookback — a consumed swap deposit aged 7-14 days would lose its tag and
be re-counted as finalizable. Tag both the deposit type key and the
cloid -> deposit txn mapping with 2x FINALIZER_TOKENBRIDGE_LOOKBACK.

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

Copy link
Copy Markdown
Contributor Author

Addressed Codex's P1 in 4843d2a. Verified the finding was real: the SWAP tag relied on the Redis default TTL (1 week, DEFAULT_CACHING_TTL in the SDK) while the Binance finalizer's deposit-history lookback defaults to 14 days (FINALIZER_TOKENBRIDGE_LOOKBACK), so a consumed swap deposit aged 7–14 days would have lost its tag and been re-counted as finalizable — exactly the phantom amountToFinalize bug this PR targets.

Fix: _depositToBinance now tags the deposit (and the cloid → deposit-txn mapping used by the prune path) with an explicit TTL of 2 * FINALIZER_TOKENBRIDGE_LOOKBACK (28 days), leaving margin for FINALIZER_MAX_TOKENBRIDGE_LOOKBACK overrides. README updated to match; typecheck passes.

@codex review

@droplet-rl

Copy link
Copy Markdown
Contributor Author

🔎 View trace

@droplet-rl droplet-rl force-pushed the droplet/C0AFB0BP3SB-1783490172-528569-fix2 branch from c06f4d7 to 4843d2a Compare July 8, 2026 20:06
nicholaspai
nicholaspai previously approved these changes Jul 8, 2026
@droplet-rl

Copy link
Copy Markdown
Contributor Author

Thanks for the review and approval! All feedback rounds are addressed on the branch: hook errors now propagate (bdf7ede), the released-tag log is at debug level (3236701), and the SWAP tag TTL now outlives the finalizer lookback (4843d2a). Leaving the merge to you.

@droplet-rl

Copy link
Copy Markdown
Contributor Author

🔎 View trace

Comment thread src/rebalancer/adapters/binance.ts
@droplet-rl

Copy link
Copy Markdown
Contributor Author

Held off on applying the suggestion and replied inline with why: removing the explicit TTL from setBinanceDepositType falls back to the SDK's 1-week DEFAULT_CACHING_TTL, which is shorter than the finalizer's default 14-day deposit lookback — the exact 7–14-day re-count gap Codex flagged earlier. It would also leave the code inconsistent (the mapping set below still uses 28d, and the comment/README document the longer TTL).

If the intent is that prod finalizer lookback is always under a week and the default is fine, say the word and I'll apply it plus trim the comment, mapping TTL, and README to match in one commit.

@droplet-rl

Copy link
Copy Markdown
Contributor Author

🔎 View trace

@nicholaspai nicholaspai merged commit 351627c into master Jul 8, 2026
5 checks passed
@nicholaspai nicholaspai deleted the droplet/C0AFB0BP3SB-1783490172-528569-fix2 branch July 8, 2026 20:48
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.

2 participants