Skip to content

Rebuild against the published DXtrade spec: SL/TP actually placed, correct endpoints, typed errors, packaging - #2

Merged
Bogzx merged 10 commits into
mainfrom
improve/2026-09-30
Sep 30, 2026
Merged

Rebuild against the published DXtrade spec: SL/TP actually placed, correct endpoints, typed errors, packaging#2
Bogzx merged 10 commits into
mainfrom
improve/2026-09-30

Conversation

@Bogzx

@Bogzx Bogzx commented Sep 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Owner

Rebuilds the client against the published DXtrade REST/Push spec (https://demo.dx.trade/developers/, OpenAPI at https://demo.dx.trade/dxsca-web/swagger/openapi.json). The README said no official documentation was available; it is public.

Why (trading-safety bugs on main)

  • Stop-loss and take-profit were always silently dropped. They were placed only if the order response contained positionCode, and per the spec it never does. Callers got "success" for an unprotected position.
  • get_balance() read equity and margin from /portfolio (they live in /metrics), and returned a fake zero balance on parse errors.
  • Positions and orders were parsed with invented field names.
  • Several calls used endpoints or methods that don't exist:
    • modify/cancel without If-Match;
    • close and SL/TP via /positions/{id};
    • history at the wrong path.
  • The Push API was invented. The token also went into the websocket URL, which was logged.
  • No session refresh, and blanket except Exception everywhere.

What changed

  • Entry + SL + TP go in one IF-THEN order group (spec example 3). Modifying any member of a working group re-sends the whole group, because a single-order PUT would strip the protections.
  • Balance, positions, orders and history follow the spec schemas, with strict parsers.
  • Modify is a full-replace PUT with If-Match; on a 412 the body is rebuilt from fresh data.
  • Cancel sends If-Match.
  • Close is a MARKET CLOSE order, correct for longs and shorts. There's also a bulk close_all().
  • Push API follows the spec: an envelope with PingRequest/Ping, a separate /md market-data socket, and subscriptions replayed after a reconnect. The token never appears in URLs, logs, reprs or exceptions.
  • Sessions: POST /ping keepalive, adopting any renewed token, and one automatic re-login on 401. Order retries keep the same orderCode; 5xx and timeouts after sending are flagged ambiguous.
  • Typed exception hierarchy (AuthenticationError, NotFoundError, ConflictError, PreconditionFailedError, RateLimitError, ServerError, OrderPlacementError).
  • Account discovery via /users/{login}.
  • Packaging: now a package (pyproject.toml, 0.2.0, py.typed) with an offline demo and CI. from dxtrade_wrapper import … still works for every public name on main.

Verification

  • Tests: 23 → 121 passed on Python 3.9/3.11/3.13/3.14; coverage 45% → 93%. ruff and mypy clean.
  • Spec-conformance tests (13): every request is validated against the official OpenAPI paths, methods and schemas, plus the prose rules.
  • An independent second review re-fetched the spec itself and found and fixed a critical bug the rebuild had missed (modifying a grouped entry stripped SL/TP), plus two more modify bugs. Verdict: ready with caveats.

⚠️ Not yet verified against a live broker

Do not trade real money with it until it has been run on a demo account. Check in particular:

  • whether a full close should omit quantity;
  • partial close;
  • how a THEN leg rejected at fill time is reported.

See the review notes in the README.

Bogdan Truta and others added 10 commits September 30, 2026 12:32
Bogdan Truta @claude
Rebuild the client against the published DXtrade REST/Push spec
c5b5c1e 
The README said official docs were not accessible, so every request shape
was inferred. They are public (demo.dx.trade/developers, OpenAPI at
/dxsca-web/swagger/openapi.json). Checked against them, most calls were
still wrong after the 2026 fixes:

- SL/TP were silently never placed: they waited for a positionCode in the
  order response, which only has orderId/updateOrderId. Now one atomic
  IF-THEN order group, so entry and protections succeed or fail together.
- get_balance read equity/margin from /portfolio (they live in /metrics)
  and returned Balance(0,0,0,0) on any parse problem. Now /metrics, strict.
- Positions/orders parsed invented names (id, entryPrice, stopLoss) from a
  bare list; the API wraps {"positions": [...]} with positionCode, symbol,
  openPrice, stopLossPrice, legs[0].price... Parsers are now strict.
- History hit /accounts/{a}/history; it is /accounts/{a}/orders/history.
- modify_order PATCHed /orders/{id} with price/qty; the spec is PUT
  /accounts/{a}/orders with the complete order and If-Match. cancel_order
  lacked If-Match (403 per spec). Both now read the ETag and retry on 412.
- close_position / modify_position_sl_tp called /positions/{id}, which does
  not exist. Now a linked CLOSE market order, and protection orders
  modified via PUT or placed via POST. New close_all() (bulk close).
- ping was GET (spec: POST); a renewed sessionToken is adopted.
- Push API used made-up 'subscribe' messages, Atmosphere headers and the
  token in the URL (logged at INFO). Now the spec envelope (type/session/
  requestId), PingRequest->Ping replies, /md channel for quotes, portfolio
  and metrics subscriptions, reconnect that really replays subscriptions.

Also: one _request() path with typed exceptions (status, errorCode,
description), a single re-login on 401, account discovery via /users when
none is configured (never invented), URL-encoded account/order codes,
OrderPlacementError.ambiguous for timeouts after sending, no logging
config in the library, credentials never logged. Module moved into a
package (dxtrade_wrapper/); `from dxtrade_wrapper import ...` is unchanged.
The old file is in git history at dxtrade_wrapper.py.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Bogdan Truta @claude
Package it: pyproject.toml, version 0.2.0, py.typed
fca891c 
pip install . (or git+https://...) now works and ships type hints. Version
lives in dxtrade_wrapper.__version__. Not published to PyPI; that stays the
owner's call. Dev requirements gain jsonschema/websockets/ruff/mypy for the
new conformance and live-socket tests.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Bogdan Truta @claude
Test against the official OpenAPI document and a real local websocket
56e2143 
test_spec_conformance.py (opt-in: DXTRADE_SPEC_TESTS=1, downloads the public
spec rather than vendoring it) drives every REST method and asserts each
request hits a documented path/method/query param, sends only declared
fields and validates against the schema, and that every fixture matches its
response schema. Mutation-checked: renaming quantity->qty or reverting the
history path makes it fail.

test_push_live_socket.py runs the Push channels against a fake DXtrade
server over real sockets: PingRequest/Ping on both channels, quote
subscription on /md, and resubscription after the TCP connection drops.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Bogdan Truta @claude
Reuse the fetched account list instead of calling /users twice
15ab55b 
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Bogdan Truta @claude
README, example, offline demo, CHANGELOG and CI
ba0921f 
- README: status is now 'spec-conformant, not live-verified' with links
  to the public docs. The quick start runs with no broker account
  (verified from a fresh clone), plus an API table mapping each method
  to its endpoint, and sections on errors, sessions and security.
- Drops the 'Use this instead: pip install dxtrade-sdk' advice: no such
  package exists on PyPI (404).
- example.py: read-only tour of a real account; orders only with
  --demo-order. examples/offline_demo.py prints every request it sends.
- CI: ruff + pytest on 3.9/3.11/3.13/3.14, mypy + spec-conformance job,
  wheel build and install.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Bogdan Truta @claude
Position P/L on request; retry rate-limited reads once
e55565d 
- get_positions(include_pnl=True) fills Position.pnl from the per-position
  fpl in /metrics?include-positions=true (one extra request).
- A GET that gets 429 waits out Retry-After (up to 5s, 1s if absent) and
  retries once; the spec's default read limit is 10/s, easy to hit from a
  dashboard poll loop. Orders are never retried on 429.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Bogdan Truta @claude
Document include_pnl and the 429 retry
92c5978 
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Bogdan Truta @claude
review: keep SL/TP on group modify; rebuild 412 retries; flag 5xx as …
6d22b6b 
…ambiguous

Independent review against the prose REST spec (demo.dx.trade/developers):

- modify_order on the entry of an IF-THEN group sent a single-order PUT.
  Modify Order example 1 shows that converts the group to a single order,
  so the stop loss and take profit disappear. Members of a working group
  are now modified as the whole group (example 2b); OCO members and groups
  with a missing child are refused with nothing sent.
- The Open Orders listing gives OPEN legs positionCode == orderId; the
  replace request copied it, which the spec says must be omitted unless
  positionEffect is CLOSE. The fixture now has it, as in the spec.
- The 412 retry resent the body built from the stale read, silently
  undoing the change that caused the 412. It is rebuilt from the re-read.
- 5xx on placement, and a group acknowledged with fewer OrderResponses
  than orders sent, are now OrderPlacementError(ambiguous=True).
- Concurrent 401s on one stale token log in once; _relogin no longer
  flips is_authenticated off while logging in.
- Conformance test also checks the prose rules (positionCode only on
  CLOSE, no type in PUT) and a group modify.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Bogdan Truta @claude
review: require setuptools>=77 for the SPDX license string
6e1f32d 
license = "MIT" (PEP 639) fails validation on setuptools 76 and older
("project.license must be ... {text: ...}"), so the declared >=69 floor
did not build.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Bogdan Truta @claude
review: example.py honours DXTRADE_LOGIN_PATH / DXTRADE_WEBSOCKET_PAT…
a2743a5 
…H again

Both were in main's .env.example; the rewritten example silently ignored
them, so an existing .env for a broker with non-default paths would hit
the wrong URLs.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Bogzx
Bogzx merged commit f735e59 into main Sep 30, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@Bogzx