fix: sort-agnostic orderbook walk for market price calculation#23
Closed
deipeypaanchal wants to merge 1 commit into
Closed
fix: sort-agnostic orderbook walk for market price calculation#23deipeypaanchal wants to merge 1 commit into
deipeypaanchal wants to merge 1 commit into
Conversation
`calculate_buy_market_price` and `calculate_sell_market_price` iterated `reversed(positions)` under the assumption that the server returns asks in descending-price order and bids in ascending-price order. That's an undocumented contract — if the server ever changes ordering (or a caller passes a pre-sorted snapshot), the walk returns the *worst* price instead of the marginal price required to fill the requested amount. Sort locally by price instead: * buys walk asks ascending (cheapest first) * sells walk bids descending (highest first) Also consolidate the dict/dataclass position parsing into a single `_normalize_positions` helper. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
scutuatua-crypto
approved these changes
Apr 29, 2026
scutuatua-crypto
approved these changes
Apr 29, 2026
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.
Summary
calculate_buy_market_price/calculate_sell_market_priceiteratedreversed(positions), assuming the server returns asks descending and bids ascending. That's an undocumented contract. If the server ever changes ordering (or a caller passes a pre-sorted snapshot), the walk returns the worst price rather than the marginal price required to fill the amount._normalize_positionshelper to consolidate the dict-vs-dataclass access.Test plan
🤖 Generated with Claude Code
Note
Medium Risk
Changes the core market-price calculation used for building market orders, which can alter execution price selection and edge-case behavior when books are partially fillable.
Overview
Fixes market price estimation to be independent of server/orderbook input ordering.
calculate_buy_market_priceandcalculate_sell_market_pricenow normalize position entries (dict vs object), locally sort by price (asks ascending, bids descending), and return the marginal price needed to fillamount_to_match; for non-FOKorders with insufficient depth, they return the worst price reached during the walk instead of relying on the first entry.Reviewed by Cursor Bugbot for commit 8c81a12. Bugbot is set up for automated code reviews on this repo. Configure here.