Skip to content

Kraken ledger pps#346

Merged
goodboy merged 18 commits intomasterfrom
kraken_ledger_pps
Jul 6, 2022
Merged

Kraken ledger pps#346
goodboy merged 18 commits intomasterfrom
kraken_ledger_pps

Conversation

@goodboy
Copy link
Copy Markdown
Contributor

@goodboy goodboy commented Jun 29, 2022

piker.brokers.kraken pkg reorg into multiple submods (l like #334 for ib) as well as initial ledger file pull and update support and use of our new pps.toml system from #336.

Putting this up as a draft to start until the pps stuff is written. now complete!

Probably will not be solving #290 related bugs until a follow up PR but this should give a good base to work on those.

This does solve #310's pp updates part (which was entirely missing, we were only getting pp updates on restarts of the backend..) and #311.

@goodboy goodboy requested review from guilledk and iamzoltan June 29, 2022 17:50
@goodboy goodboy force-pushed the kraken_ledger_pps branch from b5f5f0e to f93feba Compare June 29, 2022 19:32
@goodboy goodboy mentioned this pull request Jul 2, 2022
goodboy added 12 commits July 2, 2022 16:20
Just ignore them for now using new `match:` syntax B)
but we'll do incremental update sooon!

Resolves #311
No real-time update support (yet) but this is the first draft at writing
trades ledgers and `pps.toml` entries for the kraken backend.

Deatz:
- drop `pack_positions()`, no longer used.
- use `piker.pp` apis to both write a trades ledger file and update the
  `pps.toml` inside the `trades_dialogue()` endpoint startup.
- drop the weird paper engine swap over if auth can't be done, we should
  be doing something with messaging in the ems over this..
- more web API error response raising.
- pass the `pp.Transaction` set loaded from ledger into
  `process_trade_msgs()` do avoid duplicate sends of already collected
  trades msgs.
- add `norm_trade_records()` public endpoing (used by `piker.pp` api)
  and `update_ledger()` helper.
- rejig `process_trade_msgs()` to drop the weird `try:` assertion block
  and skip already-recorded-in-ledger trade msgs as well as yield *each*
  trade instead of sub-sequences.
@goodboy goodboy force-pushed the kraken_ledger_pps branch from 4c3032b to 47777e4 Compare July 2, 2022 20:20
goodboy added 4 commits July 3, 2022 11:18
Moves to using the new `piker.pp` apis to both store real-time trade
events in a ledger file as well emit position update msgs (which were
not in this backend at all prior) when new orders clear (aka fill).

In terms of outstanding issues,
- solves the pp update part of the bugs reported in #310
- starts a msg case block in prep for #293

Details of rework:
- move the `subscribe()` ws fixture to module level and `partial()` in
  the client token instead of passing it to the instance; in prep for
  removal of the `.token` attr from the `NoBsWs` wrapper.
- drop `make_auth_sub()` since it was too thin and we can just
  do it all succinctly in `subscribe()`
- filter trade update msgs to those not yet stored int the toml ledger
- much better kraken api msg unpacking using new `match:` synax B)

Resolves #311
@goodboy goodboy marked this pull request as ready for review July 3, 2022 18:54
@goodboy goodboy changed the base branch from master to pps_postmortem July 3, 2022 18:56
yield client


def normalize_symbol(
Copy link
Copy Markdown
Contributor Author

@goodboy goodboy Jul 3, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only code that's changed in this file was this routine, ow was taken verbatim from the old kraken,py.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

assume not including the logic in get trades

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

heh, I also like this much better :)


# process and relay trades events to ems
# https://docs.kraken.com/websockets/#message-ownTrades
async for msg in stream_messages(ws):
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm thinking we might want to move this into a new async func to avoid so much indentation and also just be more narrowed in the scope of the code's purpose much like how we did so in ib (though that was also just a bigger mess to organize 😂)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yea, reading through it again now is a struggle haha >_<

@@ -0,0 +1,464 @@
# piker: trading gear for hackers
Copy link
Copy Markdown
Contributor Author

@goodboy goodboy Jul 3, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything here was again moved over verbatim from the old god module..

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lol what a module name

piker/pp.py Outdated
if not pps:
log.warning(
f'No trade history could be loaded for {brokername}:{acctid}'
f'No `pps.toml` positions cold be loaded for {brokername}:{acctid}'
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤦🏼

guilledk
guilledk previously approved these changes Jul 3, 2022
Base automatically changed from pps_postmortem to master July 4, 2022 19:28
@goodboy goodboy added broker-backend `brokerd`/`datad` related backend tech brokers-can-smbz features that suits should provide ledger trade, accounts and other user focal event history tracking, management and storage labels Jul 4, 2022
@goodboy
Copy link
Copy Markdown
Contributor Author

goodboy commented Jul 4, 2022

Just waiting on a stamp from @iamzoltan here and we can probably land dis stuff.

@goodboy
Copy link
Copy Markdown
Contributor Author

goodboy commented Jul 5, 2022

Note, found something broke that is the same issue that is being addressed in #350 but the fix is coming in #349 so don't worry y'all 😂

goodboy added a commit that referenced this pull request Jul 5, 2022
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.
Copy link
Copy Markdown
Contributor

@iamzoltan iamzoltan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great @goodboy.

I really dig the new sub-module structure and it should make it easier for new eyes (and old ones) to grok this over :)

Merge Merge Merge!

- ``broker.py`` part for orders / trading endpoints
- ``feed.py`` for real-time data feed endpoints
- ``api.py`` for the core API machinery which is ``trio``-ized
wrapping around ``ib_insync``.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am a big fan of the sub modules!

  • why does the comment for api.py reference ib_insync?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lol yeah probably a mistake, i copy/pasta from the ib version of this..

by_id = resp['result']['trades']
trades_by_id.update(by_id)

# we can get up to 50 results per query
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

big fan of how clean this is. The logic is easier to follow

yield client


def normalize_symbol(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

assume not including the logic in get trades

yield client


def normalize_symbol(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

heh, I also like this much better :)


# process and relay trades events to ems
# https://docs.kraken.com/websockets/#message-ownTrades
async for msg in stream_messages(ws):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yea, reading through it again now is a struggle haha >_<


return records


Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fan of the ledger abstraction :)

@@ -0,0 +1,464 @@
# piker: trading gear for hackers
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lol what a module name


) -> tuple[Callable, int]:

# TODO implement history getter for the new storage layer.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this would be slick!!!

@goodboy goodboy merged commit 419ebeb into master Jul 6, 2022
@goodboy goodboy deleted the kraken_ledger_pps branch July 6, 2022 00:56
goodboy added a commit that referenced this pull request Jul 6, 2022
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.
goodboy added a commit that referenced this pull request Jul 8, 2022
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.
goodboy added a commit that referenced this pull request Jul 9, 2022
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.
goodboy added a commit that referenced this pull request Jul 11, 2022
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.
goodboy added a commit that referenced this pull request Jul 14, 2022
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.
goodboy added a commit that referenced this pull request Jul 19, 2022
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.
goodboy added a commit that referenced this pull request Jul 21, 2022
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.
goodboy added a commit that referenced this pull request Jul 21, 2022
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.
goodboy added a commit that referenced this pull request Jul 21, 2022
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.
goodboy added a commit that referenced this pull request Jul 21, 2022
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.
goodboy added a commit that referenced this pull request Jul 24, 2022
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.
goodboy added a commit that referenced this pull request Jul 25, 2022
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.
goodboy added a commit that referenced this pull request Jul 26, 2022
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.
goodboy added a commit that referenced this pull request Jul 27, 2022
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.
goodboy added a commit that referenced this pull request Jul 27, 2022
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.
goodboy added a commit that referenced this pull request Jul 27, 2022
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.
goodboy added a commit that referenced this pull request Jul 27, 2022
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.
goodboy added a commit that referenced this pull request Jul 30, 2022
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.
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 clearing auction and mm tech: EMS, OMS, algo-trading factoring less codez dawg ledger trade, accounts and other user focal event history tracking, management and storage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants