feat: mark-to-market P&L with multi-provider quote feeds and ranked failover - #16
Merged
Conversation
added 8 commits
July 17, 2026 08:42
MarkStore has been write-only since it was added: opra_stream was the only
writer and no get/get_all caller existed anywhere in src/, so every quote we
decoded was discarded. The module doc claimed "/positions valuation and
market-order pre-trade risk read" this store; neither consumer was ever built,
and 0011's comment ("unrealized P&L is intentionally absent — it needs a
market-data mark") has been waiting for exactly this.
Join the mark in at request time: /portfolios/:id/positions now returns mark,
market_value, unrealized_pnl and mark_ts. The query gains contract_size from
the master (LEFT JOIN, defaulting to 1) so options scale by 100 while spot
does not.
The mark-dependent fields are Option. A position we can hold but not value —
no provider covers it, or the feed is down — renders as null rather than 0,
because a zero would read as "flat P&L" instead of "unknown". This is not
hypothetical: the three Binance crypto instruments are routable but have no
PROVIDER xref row, so they are structurally unmarkable today.
The valuation itself is extracted to positions::value() rather than left inline
in the handler, because sign errors on shorts are the classic bug here and they
belong under test. Both figures are signed through net_qty: a short carries a
negative market value and gains when the mark falls below avg_cost.
Marks are snapshotted once per request so every row is valued against the same
instant rather than racing the feed mid-loop.
All three stream tasks had grown the same outer loop, verbatim: set the health badge connecting, run a session, set it down, sleep, double the backoff, retry. Only the session body differed. Extract the loop into stream_supervisor and give each stream a Session impl holding what its session needs. This fixes a bug all three shared. backoff_secs was initialized outside the loop and never reset, so it only ever grew for the life of the process: a stream that stayed up for hours and then dropped once would wait the last capped 30s before retrying, as though it were flapping. A session that lasted STABLE_AFTER_SECS is now treated as healthy and its next failure restarts the schedule from 1s. The backoff rule is unit-tested, including that reset. The streams are deliberately not unified further. Alpaca and Binance emit execution reports; OPRA emits quotes. They share mechanics, not purpose, so a single trait over both would be the wrong abstraction — Session is only the reconnect seam. Also drops binance_stream::run's _api_key/_api_secret parameters. They were dead: the adapter already holds the credentials and signs with an Ed25519 key loaded at construction, so main.rs was passing two String::new() to satisfy a signature that existed only to mirror Alpaca's. Streams now name themselves to the supervisor (ALPACA/PAPER, BINANCE/PAPER, DATABENTO/OPRA), so reconnect logs say which stream without reading the module path. Verified: all three connect and reach live/connecting as before; 66 tests pass.
crates/dataprovider has advertised a "quote source" capability since it was
written — provider.rs names it in the doc comment beside UniverseSource and
Enricher — but it never existed, so the OPRA feed grew in the app as one function
that connected a socket, queried the database, decoded DBN, and wrote MarkStore.
Adding Binance or Bybit meant copying all four concerns.
Split them along the seams that actually differ per vendor:
LiveQuoteFeed (dataprovider) wire protocol -> Quote. Nothing else.
quote_feed::QuoteFeedSession what to subscribe to; watches the doorbell and
pushes newly-held instruments mid-session.
stream_supervisor reconnect, backoff, health.
mark_router the sole writer to MarkStore.
A new vendor is now one impl LiveQuoteFeed: no reconnect loop, no health wiring,
no SQL, no subscription bookkeeping. Vendor concepts stay contained — Databento's
session-scoped numeric ids never reach the trait, because they mean nothing to
Binance.
Quote carries ts_recv and source_code. ts_recv is deliberately our clock, so a
vendor with a skewed or missing timestamp cannot make a stale quote look fresh;
source_code is provenance, without which failover cannot arbitrate between sources
it cannot name and a mark cannot be audited.
Feeds emit rather than write. That makes a feed testable with nothing but a
channel, and lets marks/Kafka/cockpit fan out later without reopening any feed.
mark_router drops one-sided books: a mid computed from a missing side is silently
wrong, and a stale real mark beats a fabricated one.
Ranked failover is not built. No instrument currently has more than one provider,
so it would be machinery with nothing to arbitrate; mark_router is where it goes
when a second feed lands.
load_subscribable still reads instrument.symbol and filters to options — it works
only because Databento's raw symbol is byte-identical to ours. P3 replaces the body
with the instrument_xref PROVIDER join, which is what makes the driver genuinely
vendor-neutral.
Verified: 66 tests; router starts, feed idles correctly with nothing held, all
three streams reach live/connecting as before.
Self-review of the P0-P2 refactor caught two regressions it introduced, plus three smaller defects. Health reporting was lost. The pre-refactor opra_stream called health.set_live() after client.start() and health.record_event() per record; the new path called neither, because the feed no longer held a StreamHandle and the supervisor only knows set_connecting/set_down. DATABENTO/OPRA could therefore never leave "connecting" even while streaming, and last_event_at stayed null forever — which is precisely the freshness signal the planned staleness-based failover reads, so the source could never have been selected as active. Fixed with a FeedHealth port in dataprovider (on_connected/on_event) that StreamHandle implements, keeping the crate ignorant of our registry while restoring the exact prior behaviour. The mark router rejected zero-bid quotes. `!(bid > 0.0 && ask > 0.0)` treats a 0.00 x 0.05 book as one-sided, but a zero bid is a real price — that is what a deep-OTM option looks like near expiry, exactly where today's SPY 750 call is headed. Those contracts would report mark=null, indistinguishable from having no data source at all, when in fact the market is quoting them. The missing-side sentinel is UNDEF_PRICE, which feeds already filter. Now rejects only what cannot produce a mid: no offer, negative price, or a crossed book. is_usable() is extracted and tested, since this is the second time a plausible-looking guard here has been wrong. Also: - quote_feed no longer returns Ok(()) from run_once when nothing is held. The supervisor read that as a closed stream, so an idle feed logged a reconnect warning and flapped its badge down->connecting every 60s while behaving exactly as designed. It now waits on the doorbell (with a timer backstop), which also picks up a fill immediately instead of up to a minute later. - binance_stream passed a hardcoded "BINANCE/PAPER" to supervise despite taking an `environment` parameter, so a LIVE stream's reconnects would be logged as paper. - get_portfolio_positions cloned the entire MarkStore per request to read two or three entries; it now looks up per row. Cost was O(all marks), and the clone held the read lock against the router's writes. Verified: 72 tests; no flapping over 20s (was 1 warning/min); positions endpoint unchanged; all three streams reach live/connecting.
….symbol The quote driver read instrument.symbol and filtered to instrument_class='OPTION' in SQL. That worked only by coincidence: Databento's raw_symbol happens to be byte-identical to our master symbol for OSI options, and for nothing else. Binance calls it SOLUSDT where we call it SOL, so the coincidence ends at the first non-Databento feed — the driver was vendor-neutral in shape only. Subscriptions now come from oms.instrument_xref PROVIDER rows, so the provider's own external_symbol drives the subscription and a feed is free to use whatever strings its venue uses. Verified parity over the full option universe: old and new queries return identical (symbol, id) sets — 27,252 rows, 0 mismatches — so this changes the mechanism without changing behaviour. Coverage needed a second half, which the data made obvious. source_code alone is not coverage: the held SPY *equity* carries a PROVIDER/DATABENTO xref row (external_symbol 'SPY' @ ARCX), so a source_code-only query hands it to the OPRA.PILLAR session — an options-only dataset that cannot quote it. LiveQuoteFeed therefore declares covers() -> &[instrument_class], and DatabentoOpraFeed declares OPTION. Confirmed against the live DB: without the filter the held set is [SPY/SPOT]; with it, correctly empty. covers() is the minimum honest scope, not the final answer. One vendor running several feeds (Databento equities + OPRA) wants a per-feed dataset/schema config; that is what the planned provider_feed_cfg table is for. Until then, a feed declaring its classes beats a class hardcoded in shared SQL. An instrument with no xref row for a source is simply absent from that feed's subscribe set — the "tradable but unmarkable" state, which is a fact about coverage, not an error. Verified: 72 tests; all three streams connect; no errors.
The three Binance crypto instruments were routable but had no PROVIDER xref row, so nothing in the identity graph claimed to price them: /positions reported a null mark for positions we can freely trade. This closes that gap and, in doing so, is the first real test of the LiveQuoteFeed abstraction — the whole point of the previous three commits was that a second vendor should cost one trait impl. It did. BinanceFeed is protocol only: connect the public bookTicker stream, decode, emit Quote. No reconnect loop, no health wiring, no SQL, no subscription bookkeeping — quote_feed, stream_supervisor and mark_router already owned those, unchanged. Marks come from PRODUCTION Binance even though orders route to testnet. Testnet runs its own synthetic book, so marking against it would value positions at prices nobody can trade; P&L against a testnet fill is meaningless either way, so the choice is between a useful number and a useless one. The stream is public — no key, no signing — and shares nothing with the authenticated WS-API the order stream uses. The doorbell had to fan out. An mpsc has exactly one consumer, so with two feeds the fill path's single signal now goes to a relay task that try_sends to each feed's own doorbell. Keeping one sender on the fill path means execution.rs never learns how many feeds exist. Verified end-to-end against the live market — the first time any quote has flowed through this chain: feed -> Quote -> mark_router -> MarkStore -> /positions SOL: mark 74.605 vs avg_cost 77.82, qty 0.1 -> unrealized -0.3215 BTC: mark 62772.90 vs avg_cost 64100.26 -> unrealized -3.9821 Hand-checked: (74.605 - 77.82) * 0.1 = -0.3215. Marks tick live (74.595 -> 74.615) with mark_ts advancing. 75 tests pass.
…to symbols
Three interlocking changes: Bybit gives crypto a second source, which is the first
case the failover design has ever had to arbitrate, and both depend on crypto
identity being right first.
Symbol collision (0015_CRYPTO_SYMBOL_IS_THE_PAIR)
The seed modelled crypto as symbol = base asset (SOL), currency = quote (USDT).
That cannot represent SOL/USDT and SOL/BTC at once: both key to (SOL, BINANCE)
under instrument_symbol_venue_key, so the model silently permitted exactly one
quote currency per base per venue — not a property of the market. The symbol now
names the pair (SOLUSDT), which is what every venue calls it and what our own
xref already stored as external_symbol; instrument.symbol was the only place
saying SOL. Matches Nautilus's raw_symbol convention and CCXT's rule that
contracts which differ must have distinct symbols. currency stays USDT so the
pair remains decomposable, and recon is untouched (it matches the base asset via
external_native_id).
Bybit feed
orderbook.1 on the v5 public spot stream. Confirmed against the live endpoint
before writing the decoder — Bybit and Binance quote SOL within 0.15% of each
other, which is what makes one a usable stand-in for the other. Unlike Binance's
bookTicker, orderbook.1 is stateful: a snapshot replaces the book, a delta
carries only the side that moved, and size "0" deletes a level. The feed tracks
the last known top per pair and emits only once both sides are known, so a delta
can never publish a half-quote. Needs an application-level {"op":"ping"} every
20s — a protocol ping frame does not reset Bybit's timer.
Ranked failover (0018_CREATE_PROVIDER_FEED_POLICY + mark_router)
Rank lives in policy keyed by (source_code, instrument_class), not per instrument
— that would be ~42k copies of the same integer for a decision that is a property
of the source. Binance outranks Bybit for SPOT because we route crypto orders to
Binance: its book is the one our fills print against, and Bybit prices the same
pair on a different book.
The router now owns arbitration: best-ranked live source holds the mark, a better
source preempts immediately, a worse one may take over only once the incumbent
has been quiet for STALE_AFTER_SECS. Staleness rather than connection state is
the trigger because it catches the failure a socket cannot report — up,
subscribed, and silently not delivering. Every ownership change logs; a silent
failover is one nobody can diagnose later.
Deliberately not A/B line arbitration, which is per-message sequence
reconciliation between redundant lines of the same feed at microsecond
granularity. This is cross-vendor, at seconds, between different books.
Verified live, not just in tests:
- both feeds up: Bybit connected first and took the mark, Binance preempted it
0.5s later ("promoting better-ranked source")
- primary dead (BINANCE_FEED_WS_URL pointed at an unreachable host): BINANCE/SPOT
down, BYBIT/SPOT live, SOL still marked at 74.825 with a fresh timestamp
- primary recovered: promoted straight back to Binance, mark 74.835
84 tests, including 5 that pin the arbitration state machine.
The Bybit market-data xref rows were created by a manual INSERT during failover testing and never landed in a seed file, so a fresh install had a BINANCE provider but no BYBIT one — failover could not be reproduced. Add them next to the Binance PROVIDER rows, keyed the same way (same pair string, external_exchange = BYBIT).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Turns
MarkStorefrom a write-only cache into live valuation, unifies the threehand-written stream tasks behind one feed abstraction, and adds a second crypto
data source with ranked failover.
What changed (phased; each commit ships standalone)
/positionsreturnsmark,market_value,unrealized_pnl,mark_ts.MarkStorehad no reader before this; unrealized P&L was the missing input all along.stream_supervisorfor reconnect/backoff/health across all three streams; fixes a backoff-never-resets bug that was in all three.LiveQuoteFeedcapability trait indataprovider; OPRA ported to it. Feeds emitQuote, never touchMarkStore.instrument_xrefPROVIDER rows, notinstrument.symbol. Parity proven: 27,252 = 27,252, zero mismatches.provider_feed_policy) + crypto symbol = the pair (fixes a(symbol, venue)collision).Design
A new vendor is one
impl LiveQuoteFeed— proven twice (Binance, Bybit).Quotecarriests_recv(our clock, so a vendor can't fake freshness) andsource_code(provenance for failover + audit).Verified against live markets
BINANCE/SPOT down,BYBIT/SPOT live, SOL still marked; primary recovered → promoted straight back.Schema
0017(oms) — xref identity dropsexternal_native_idfrom the unique key; deduped 26,411 rows.0015(public) — cryptosymbol= the pair (SOLUSDT).0018(oms) — newprovider_feed_policytable.Known gaps (documented, not fixed here)
valid_from/valid_to); renames still rewrite history.symbology_resolveromitssource_typefrom its xref lookup.STALE_AFTER = 10sis safe only because crypto ticks constantly.84 tests.