Tractor typed msg hackin #354
Draft
Conversation
This file contains 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
Move to using the websocket API for all order control ops and dropping the sync rest api approach which resulted in a bunch of buggy races. Further this gets us must faster (batch) order cancellation for free and a simpler ems request handler loop. We now heavily leverage the new py3.10 `match:` syntax for all kraken-side API msg parsing and processing and handle both the `openOrders` and `ownTrades` subscription streams. We also block "order editing" (by immediate cancellation) for now since the EMS isn't entirely yet equipped to handle brokerd side `.reqid` changes (which is how kraken implements so called order "updates" or "edits") for a given order-request dialog and we may want to even consider just implementing "updates" ourselves via independent cancel and submit requests? Definitely something to ponder. Alternatively we can "masquerade" such updates behind the count-style `.oid` remapping we had to implement anyway (kraken's limitation) and maybe everything will just work? Further details in this patch: - create 2 tables for tracking the EMS's `.oid` (uui4) value to `int`s that kraken expects (for `reqid`s): `ids` and `reqmsgs` which enable local lookup of ems uids to piker-backend-client-side request ids and received order messages. - add `openOrders` sub support which more or less directly relays to equivalent `BrokerdStatus` updates and calc the `.filled` and `.remaining` values based on cleared vlm updates. - add handler blocks for `[add/edit/cancel]OrderStatus` events including error msg cases. - don't do any order request response processing in `handle_order_requests()` since responses are always received via one (or both?) of the new ws subs: `ownTrades` and `openOrders` and thus such msgs are now handled in the response relay loop. Relates to #290 Resolves #310, #296
Turns out the EMS can support this as originally expected: you can update a `brokerd`-side `.reqid` through a `BrokerdAck` msg and the ems which update its cross-dialog (leg) tracking correctly! The issue was a bug in the `editOrderStatus` msg handling and appropriate tracking of the correct `.oid` (ems uid) on the kraken side. This unfortunately required adding a `emsflow: dict[str, list[BrokerdOrder]]` msg flow tracing table which means the broker daemon is tracking all the msg flow with the ems, though I'm wondering now if this is just good practise anyway and maybe we should offer a small primitive type from our msging utils to aid with this? I've used such constructs in event handling systems prior. There's a lot more factoring that can be done after these changes as well but the quick detailed summary is, - rework the `handle_order_requests()` loop to use `match:` syntax and update the new `emsflow` table on every new request from the ems. - fix the `editOrderStatus` case pattern to not include an error msg and thus actually be triggered to respond to the ems with a `BrokerdAck` containing the new `.reqid`, the new kraken side `txid`. - skip any `openOrders` msgs which are detected as being kraken's internal order "edits" by matching on the `cancel_reason` field. - update the `emsflow` table in all ws-stream msg handling blocks with responses sent to the ems. Relates to #290
Addressing same issue as in #350 where we need to compute position updates using the *first read* from the ledger **before** we update it to make sure `Position.lifo_update()` gets called and **not skipped** because new trades were read as clears entries but haven't actually been included in update calcs yet.. aka we call `Position.lifo_update()`. Main change here is to convert `update_ledger()` into a context mngr so that the ledger write is committed after pps updates using `pp.update_pps_conf()`.. This is basically a hotfix to #346 as well.
Not sure why I put this off for so long but the check is in now such that if the market isn't open or no rt quote comes in from the first query, we just pull from the last shm history 'close' value. Includes another fix to avoid raising when a double remove on the client side stream from the registry sometimes happens.
Syncs with goodboy/tractor#311 which is nowhere near ready and this approach didn't end up being as straight forward as hoped. We're going to need a top level `Msg`-boxing type/protocol in `tractor` first...
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.
This is a PURE WIP/POC that will probably get changed a lot.
Just putting it up so i don't forget about this branch.
Would need to sync with work on a
tractordev branch:goodboy/tractor#311