fix(python): forward outcome_id to SOR buildOrder on explicit-ID path - #2115
Open
VaggelisGian wants to merge 2 commits into
Open
fix(python): forward outcome_id to SOR buildOrder on explicit-ID path#2115VaggelisGian wants to merge 2 commits into
VaggelisGian wants to merge 2 commits into
Conversation
Python's hosted SOR escape path dropped the outcome identifier when create_order was called with explicit market_id/outcome_id: _execute_sor_order built the /api/sor/buildOrder params without ever reading kwargs["outcome_id"], and its None-filter stripped the always-None "outcome" key, so the request went out with no outcome identifier at all. TypeScript forwards the full caller input unchanged, so outcomeId always reaches the wire there. Add outcomeId to the explicit-ID branch of _execute_sor_order. The existing None-filter keeps every other call shape byte-identical. Add a dispatch test that mocks the requests calls made by the SOR path and asserts the captured buildOrder payload contains the caller-supplied outcomeId for both the explicit-ID shape (fails before the fix) and the MarketOutcome shorthand (control). Fixes pmxt-dev#2025
Add a market_id-only dispatch case asserting outcomeId and outcome are absent from the request when no outcome was supplied, so removal of the None-filter in _execute_sor_order cannot regress the wire payload to an explicit null.
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
1 participant
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.
Fixes #2025
What was broken
Python's hosted SOR escape path dropped the outcome identifier when create_order was called with explicit market_id/outcome_id. _execute_sor_order branches on kwargs.get("outcome") (client.py:3057): the MarketOutcome shorthand emits camelCase marketId/outcomeId, but the explicit-ID branch built only marketId/side/outcome/shares and never read kwargs["outcome_id"]. The None-filter then stripped the always-None "outcome" key, so the POST to /api/sor/buildOrder went out with no outcome identifier at all. TypeScript forwards the full caller input unchanged, so outcomeId always reaches the wire there.
What changed
One line: the explicit-ID branch now includes "outcomeId": kwargs.get("outcome_id"). The existing None-filter keeps every other call shape byte-identical. The dropped fee/denom/slippage_pct fields tracked in #1858 are deliberately out of scope, as is the hardcoded empty outcome_id on the returned Order (#1503).
Tests
New sdks/python/tests/test_sor_dispatch.py mocks the requests calls made by the SOR path (the path uses requests directly, unlike the httpx-based hosted flows) and asserts the captured buildOrder payload contains the caller-supplied outcomeId for both call shapes. The explicit-ID test fails before the fix (KeyError: 'outcomeId'); both pass after.
Verification