-
Notifications
You must be signed in to change notification settings - Fork 17
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
kraken
: use userref
field AND reqid
, utilize openOrders
sub for most msging
#368
Merged
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
goodboy
added
broker-backend
`brokerd`/`datad` related backend tech
brokers-can-smbz
features that suits should provide
labels
Jul 29, 2022
goodboy
changed the title
WIP: use
Jul 29, 2022
userref
field over reqid
...kraken
WIP: use userref
field over reqid
...
goodboy
force-pushed
the
kraken_ws_orders
branch
from
July 30, 2022 21:34
c0617a5
to
e6a3e8b
Compare
goodboy
force-pushed
the
kraken_userref_hackzin
branch
from
July 30, 2022 21:35
c5bad85
to
aa3a10c
Compare
Turns out you can pass both thus making mapping an ems `oid` to a brokerd-side `reqid` much more simple. This allows us to avoid keeping as much local dialog state but with still the following caveats: - ok `editOrder` msgs must update the reqid<->txid map - only pop `reqids2txids` entries inside the `cancelOrderStatus` handler
Why we need so many fields to accomplish passing through a dialog key to orders is beyond me but this is how they do it with edits.. Allows not having to handle `editOrderStatus` msgs to update the dialog key table and instead just do it in the `openOrders` sub by checking the canceled msg for a 'cancel_reason' of 'Order replaced', in which case we just pop the txid and wait for the new order the kraken backend engine will submit automatically, which will now have the correct 'userref' value we passed in via the `newuserref`, and then we add that new `txid` to our table.
goodboy
force-pushed
the
kraken_userref_hackzin
branch
from
July 31, 2022 18:36
aa3a10c
to
1cbf45b
Compare
Since we figured out how to pass through ems dialog ids to the `openOrders` sub we don't really need to do much with status updates other then error handling. This drops `process_status()` and moves the error handling logic into a status handler sub-block; we now just info-log status updates for troubleshooting purposes.
The (partial) fills from this sub are most indicative of clears (also says support) whereas the msgs in the `ownTrades` sub are only emitted after the entire order request has completed - there is no size-vlm remaining. Further enhancements: - this also includes proper subscription-syncing inside `subscribe()` with a small pre-msg-loop which waits on ack-msgs for each sub and raises any errors. This approach should probably be implemented for the data feed streams as well. - configure the `ownTrades` sub to not bother sending historical data on startup. - make the `openOrders` sub include rate limit counters. - handle the rare case where the ems is trying to cancel an order which was just edited and hasn't yet had it's new `txid` registered.
goodboy
changed the title
Aug 2, 2022
kraken
WIP: use userref
field over reqid
...kraken
WIP: use userref
field AND reqid
, utilize openOrders
sub for most msging
goodboy
changed the title
Aug 2, 2022
kraken
WIP: use userref
field AND reqid
, utilize openOrders
sub for most msgingkraken
: use userref
field AND reqid
, utilize openOrders
sub for most msging
guilledk
approved these changes
Aug 2, 2022
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Havent tested but looks good, hehe also more pydantic removals eh?
🤫 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
broker-backend
`brokerd`/`datad` related backend tech
brokers-can-smbz
features that suits should provide
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.
Bleh, this is some mucking aboutA proper set of changes to use theuserref
field instead ofreqid
in submissions over the ws api.Turns out you
can'tcan use both!currently though I'm in the process of trying to show their API support why it's that a weird design that should be changed in order to make client implementations less complex...This moves fill handling, order edits, and cancellation event relaying to the
openOrders
sub block since after much discussion with support, this is more or less the big boi sub that indicates the most real-time state of their backend clearing engine.