Skip to content

fix(finalizer): count in-flight Binance withdrawals when reconciling deposits - #3753

Open
droplet-rl wants to merge 1 commit into
masterfrom
droplet/T90K0AL22-C0BHMM63D9Q-1787756777-113829
Open

fix(finalizer): count in-flight Binance withdrawals when reconciling deposits#3753
droplet-rl wants to merge 1 commit into
masterfrom
droplet/T90K0AL22-C0BHMM63D9Q-1787756777-113829

Conversation

@droplet-rl

Copy link
Copy Markdown
Contributor

Problem

The Binance finalizer computes how much is left to finalize as attributed deposits - withdrawals, but only counts withdrawals in the COMPLETED state:

isCompletedBinanceWithdrawal(withdrawal.status) && ...

A withdrawal Binance has accepted but not yet settled (EMAIL_SENT, AWAITING_APPROVAL, PROCESSING) is therefore treated as if it never happened. Two consequences:

  1. amountToFinalize stays inflated while a withdrawal is in flight, so the finalizer reports an outstanding balance that is already on its way out. That figure feeds the inventory rebalancer's view of in-flight transfers.
  2. Once the account balance allows it, a later run can reissue a withdrawal that is already pending. Today the only thing preventing that is the balance check — the amount is capped at the free balance, which usually drops when the first withdrawal debits. That is timing-dependent, not a guarantee.

Change

Count any withdrawal Binance has not terminally failed, reusing the existing isFailedBinanceWithdrawal helper. CANCELLED, REJECTED and FAILURE release the funds, so those stay re-withdrawable; everything else holds balance and is netted off.

isCompletedBinanceWithdrawal is deliberately left alone. The two bridge adapters resolve an on-chain receipt from withdrawal.txId to build a BridgeEvent, and only a settled withdrawal has a usable txId there.

Also

getAccountCoins mapped balance: coin.free, discarding the rest of the Binance response. When the free balance is lower than the deposit history implies, there is no way from the logs to tell funds that are reserved (in-flight withdrawal, exchange-side lock) from funds that were spent.

This adds optional locked / withdrawing to the parsed Coin and includes them in the finalizer's two balance-shortfall debug logs. balance still means the spendable balance, so no behaviour changes.

Testing

yarn typecheck, yarn lint, and the Binance suite (110 tests) pass.

New coverage in test/BinanceFinalizer.ts asserts that a prior withdrawal in each in-flight status suppresses reissue, and that each terminal-failure status still reissues. Verified the first case fails against the previous COMPLETED-only predicate.

🤖 Generated with Claude Code

…deposits

The Binance finalizer derived the amount left to finalize by subtracting only
completed withdrawals from attributed deposits. A withdrawal that Binance had
accepted but not yet settled was therefore treated as if it had never happened,
leaving the outstanding amount overstated and allowing a later run to reissue
the same withdrawal once the balance allowed it.

Count any withdrawal Binance has not terminally failed instead. Cancelled,
rejected and failed withdrawals release the funds, so they remain
re-withdrawable.

Also surface the locked and withdrawing balances from accountCoins in the
finalizer's balance diagnostics. The parsed Coin previously carried only the
free balance, which makes a shortfall caused by reserved funds indistinguishable
from one caused by a spend.

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

Copy link
Copy Markdown
Contributor Author

Thanks for the approval — no inline comments on the review, so nothing to change. HEAD is still 9674259 (the commit you approved) and all six checks are green. Leaving the merge to you.

@droplet-rl

Copy link
Copy Markdown
Contributor Author

🔎 View trace

Comment on lines -243 to +245
isCompletedBinanceWithdrawal(withdrawal.status) &&
!isFailedBinanceWithdrawal(withdrawal.status) &&

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Does this change our accounting somehow?

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.

Yes — one place beyond the intended drop in amountToFinalize, and it's worth spelling out.

amountToFinalize also accumulates into remainingAttributedBalances, which is the attributedDepositAmount argument to getSweepableOrphanBinanceBalance. So netting off an in-flight withdrawal raises the sweepable orphan balance by that same amount (100 → sweepable goes 400 → 500 in a worked example).

That's a correction rather than a regression. coinBalance in that formula is coin.free, and Binance debits free when it accepts a withdrawal — the amount moves into withdrawing. Previously we subtracted the in-flight amount from a balance that had already lost it, i.e. a double-count that under-swept by exactly the in-flight size. Now the obligation and the funds clear together. Terminal failures stay excluded from the filter, so CANCELLED/REJECTED/FAILURE add the amount back to attributed as Binance returns it to free — symmetric.

The assumption this rests on is that free is debited at acceptance (status 0) rather than at settlement. That's what withdrawing tracks, which is part of why this PR surfaces it in the shortfall logs.

Nothing outside this file moves: remainingAttributedBalances is file-local, and both bridge adapters still gate BridgeEvent construction on isCompletedBinanceWithdrawal, so the txId-derived accounting is untouched.

@droplet-rl

Copy link
Copy Markdown
Contributor Author

Answered inline. Short version: yes, it moves one figure beyond amountToFinalize — netting off in-flight withdrawals shrinks remainingAttributedBalances, which raises the sweepable orphan balance by the same amount. That's a double-count being removed rather than a regression, since coin.free is already debited at withdrawal acceptance. No code change; nothing outside the finalizer is affected.

@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.

2 participants