feat: read a market's whole book in one consistent call - #1413
Conversation
Time Submission Status
Submit or update total time with: Add time on top of previous submission with: See available commands to help comply with our Guidelines. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe migration adds ChangesFull market depth query
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@holdex pr submit-time 3h |
`get_full_market_depth(query_id)` returns aggregated volume per price level for **both** outcomes from one read, each level tagged with the outcome it rests on. Related, no closing keyword: - <truflation/website#4445> - <truflation/website#4387> #4445 is the Problem this action was written for and is already closed, by the sdk-go and sdk-js PRs that consumed it. Python's consolidated book was reading through it from the moment the preceding PR landed the sdk-go pin, so the job story is satisfied without this. What is left is the direct accessor, which is API parity rather than the problem. ## Why `get_market_depth` answers for one outcome, so comparing the two sides of a market takes two calls at two independent points in time. An order landing between them appears on one side and not the other, and the pair then describes a market state that never existed. One statement is one snapshot, and that whole class of artifact goes away. The node has carried `get_full_market_depth` since trufnetwork/node#1413 and it is live on mainnet. sdk-js and sdk-go both expose it. Python was the last SDK reading a market two calls at a time. ## What is in it - `get_full_market_depth(query_id)` on `TNClient`, returning `FullDepthLevel` rows — `outcome`, `price`, `buy_volume`, `sell_volume`. Same aggregation as `get_market_depth`, for the whole market instead of one outcome. Rows arrive YES first then NO, price ascending within each. - `GetFullMarketDepth` in `bindings/bindings.go`, delegating to sdk-go. - Unit tests pinning the wrapper: forwarding, decoding, and that the outcome tag survives. The tag is the only thing separating a YES level from the NO level at the same price, so the fixture puts a sell at 60 on each side — lose the tag and the two rows are indistinguishable. - A live check that the whole-market read and two per-outcome reads describe the same book, plus the live discovery scan switched over to it: "is this market two-sided?" is exactly the question this action exists to answer, so it now costs one read per market rather than two. ## Testing `go build ./...` and `go vet ./bindings/` clean, bindings rebuilt with `make gopy_build`. - 105 offline tests pass. - `tests/test_order_book.py` passes against a local node (44 tests). - `TN_LIVE_NODE_URL=https://gateway.mainnet.truf.network pytest tests/test_order_book_live.py` — 5 passed against mainnet, including the new equivalence check. The equivalence check is the one that matters. If the two reads ever disagreed on anything but timing, every consolidated ladder built on the new one would be wrong while the per-outcome reads stayed right, and nothing offline would notice. It re-reads to confirm the market held still before asserting, so a moving book skips rather than fails. ## What is not in it `get_market_depth` is untouched and stays the right call when you want one outcome — a depth chart, or a bot quoting a single side. `get_consolidated_order_book` needs no change: it already reads through this action inside sdk-go, from the pin #132 brought in. This adds the direct accessor for callers who want the raw both-outcome depth, which is what sdk-js and sdk-go each expose alongside their consolidated method. ## Note on the base branch Was stacked on #132 while that was open. #132 has since merged, so this now targets `main` and carries a single commit. - <#132>
Adds
get_full_market_depth($query_id), which returns the aggregated depth for both outcomes of amarket in one read, tagged with the outcome each price level belongs to.
Why
A market's YES and NO shares are two views of one position, so a resting NO sell at 93 is a standing
bid for YES at 7 — a trader hits it by selling YES and the pair burns. Anything that folds the two
books into one ladder therefore needs both sides.
get_market_depthserves one outcome at a time and the call carries no block height, so today aconsolidated ladder is stitched from two independent reads. An order landing between them appears on
one side and not the other, and the result can show a bid above an ask that never existed at any
single moment. Both SDKs currently ship the consolidated read documented as best-effort because of
this, and the sdk-go live test has to read the raw depth either side of the sequence and skip when
the market moved.
One statement over one snapshot removes that.
What is in it
get_market_depthin038-order-book-queries.sql. Same aggregation,grouped by
(outcome, abs(price))instead of filtered to one outcome; holdings at price 0 stayexcluded. Rows come back YES first then NO, price ascending within each.
tests/streams/order_book/queries_test.go: an empty market, a market quoted on bothoutcomes including a resting NO sell (the row a single-outcome read can never show), holdings
excluded, and an equivalence test pinning the new action's rows to two
get_market_depthreads sothe two cannot drift apart.
What is not in it
get_market_depthis untouched. The market-maker bot, the LP bot and depth charts read one outcomeand should keep using it.
get_best_priceshas the same two-read gap. It is left alone here: a caller holding full depth canderive the touch from it, and the market list reads best prices for every market at once, where full
depth would be much heavier.
A block-height parameter on the existing read actions would also close the gap, but one call is
cheaper for every caller and a height parameter puts the burden of picking a consistent height on
each of them.
Testing
go test -tags kwiltest ./tests/streams/order_book/ -run TestQueries, andkwil-cli utils parse -i internal/migrations/038-order-book-queries.sql.Regenerating the mainnet overrides produces no diff — the action touches no bridge, so it ships in
the base migration set.
Follow-ups
The Problem this closes also covers two SDK changes that are not written yet: sdk-js and sdk-go point
getConsolidatedOrderBookat the new action once a node carrying it is released, and drop thebest-effort caveat and the re-read guard. Reopen the Problem after merging this, or track those two
separately.
resolves: https://github.com/truflation/website/issues/4445
Summary by CodeRabbit
New Features
Bug Fixes