Summary
In the live pending-order fill-sync path, tracked_fill_baseline() (app/services/pending_orders/sent_order_recovery.py) trusts the row-local live_fill_sync.tracked_filled marker unconditionally (short-circuit). That marker can be stale: it is written by a sync that runs before the order fills, and the executor's subsequent fill update does not rewrite it. After a worker restart, reconciliation re-queries the exchange, computes delta = cumulative_filled − stale_marker(0), and re-books the entire fill quantity — doubling the position in the platform ledger while the exchange holds only half of it.
Observed live incident (Binance USD-M swap, market order)
| time (UTC) |
event |
| 00:00:41 |
order submitted (qty=0.102 → 0.1 after lot rounding), pre-fill sync writes live_fill_sync.tracked_filled = 0 into the row json |
| 00:01:31 |
executor fill 0.1 @ 685.48 recorded; pending_orders.filled = 0.1, row json marker still 0 |
| ~04:50 |
trading-worker container restart; reconciliation re-claims the still-sent row |
| 06:32:44 |
Exchange fill synced: ... filled=0.100000 @ 685.480000 — same exchange order booked a second time |
Result: strategy_order_fills held two rows with the same exchange_order_id (identical price/qty), pending_orders.filled = 0.2, UI position 0.2 — while Binance's API reported executedQty = 0.10 and account positionAmt = -0.10 throughout. The exchange never had the doubled position; only the platform ledger did.
Minimal reproduction (deterministic, current main)
import json
from app.services.pending_orders.sent_order_recovery import tracked_fill_baseline
# row state right before the second reconciliation:
# executor already recorded the 0.1 fill (row.filled = 0.1),
# but the row json still carries the pre-fill sync marker (tracked_filled = 0)
row = {
"filled": 0.1,
"avg_price": 685.48,
"exchange_response_json": json.dumps(
{"live_fill_sync": {"tracked_filled": 0.0, "tracked_avg_price": 0.0}}
),
}
baseline, _ = tracked_fill_baseline(
row, exchange_order_id="X",
previous_filled=0.1, previous_avg=685.48,
)
assert baseline == 0.0 # stale marker short-circuits the real fill
# in _sync_one_live_sent_order: delta = 0.1 - 0.0 = 0.1
# -> persist_strategy_fill() re-books the full quantity, aggregate 0.1 -> 0.2
Live trigger recipe: market order filled after at least one fill-sync cycle (marker written pre-fill) + worker restart (or any path that re-claims a sent row) + next reconciliation.
Root cause
tracked_fill_baseline() returns on the first present source:
if isinstance(sync_state, dict) and "tracked_filled" in sync_state:
return (...) # short-circuit: stale 0 masks row.filled and the ledger
Sources that know the truth (the row's own filled column, updated by the executor; and strategy_order_fills, the persisted ledger) are never consulted once the marker exists. The marker is only safe if it is always rewritten together with the fill — but the executor update path does not do that, and restarts make the stale window visible.
Suggested fix
Rank all known sources by taking the largest fill count (avg price travels with it), with one guard for the existing multi-leg semantics:
- executor
phases.executor.market_summary when it names this exchange order — the row then tracks legs separately (e.g. a maker/limit leg booked next to the market leg), so the row aggregate must NOT compete;
- otherwise the row aggregate (
filled/avg_price);
- the
live_fill_sync marker — candidate only, never short-circuits;
- new: a ledger baseline summed from
strategy_order_fills by exchange_order_id (authoritative whenever it is ahead of the row).
This keeps the existing restart/partial-fill tests green and adds a regression test for this incident. A PR is forthcoming.
Summary
In the live pending-order fill-sync path,
tracked_fill_baseline()(app/services/pending_orders/sent_order_recovery.py) trusts the row-locallive_fill_sync.tracked_filledmarker unconditionally (short-circuit). That marker can be stale: it is written by a sync that runs before the order fills, and the executor's subsequent fill update does not rewrite it. After a worker restart, reconciliation re-queries the exchange, computesdelta = cumulative_filled − stale_marker(0), and re-books the entire fill quantity — doubling the position in the platform ledger while the exchange holds only half of it.Observed live incident (Binance USD-M swap, market order)
qty=0.102 → 0.1after lot rounding), pre-fill sync writeslive_fill_sync.tracked_filled = 0into the row jsonpending_orders.filled = 0.1, row json marker still 0sentrowExchange fill synced: ... filled=0.100000 @ 685.480000— same exchange order booked a second timeResult:
strategy_order_fillsheld two rows with the sameexchange_order_id(identical price/qty),pending_orders.filled = 0.2, UI position 0.2 — while Binance's API reportedexecutedQty = 0.10and accountpositionAmt = -0.10throughout. The exchange never had the doubled position; only the platform ledger did.Minimal reproduction (deterministic, current
main)Live trigger recipe: market order filled after at least one fill-sync cycle (marker written pre-fill) + worker restart (or any path that re-claims a
sentrow) + next reconciliation.Root cause
tracked_fill_baseline()returns on the first present source:Sources that know the truth (the row's own
filledcolumn, updated by the executor; andstrategy_order_fills, the persisted ledger) are never consulted once the marker exists. The marker is only safe if it is always rewritten together with the fill — but the executor update path does not do that, and restarts make the stale window visible.Suggested fix
Rank all known sources by taking the largest fill count (avg price travels with it), with one guard for the existing multi-leg semantics:
phases.executor.market_summarywhen it names this exchange order — the row then tracks legs separately (e.g. a maker/limit leg booked next to the market leg), so the row aggregate must NOT compete;filled/avg_price);live_fill_syncmarker — candidate only, never short-circuits;strategy_order_fillsbyexchange_order_id(authoritative whenever it is ahead of the row).This keeps the existing restart/partial-fill tests green and adds a regression test for this incident. A PR is forthcoming.