Skip to content
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: balance <-> position disparities with crypto/crypto pairs #373

Closed
3 tasks
goodboy opened this issue Aug 5, 2022 · 3 comments
Closed
3 tasks

kraken: balance <-> position disparities with crypto/crypto pairs #373

goodboy opened this issue Aug 5, 2022 · 3 comments
Assignees
Labels
accounting prolly positioning: the accounting of "what/when (is) owned" broker-backend `brokerd`/`datad` related backend tech bug guille broke it prolly clearing auction and mm tech: EMS, OMS, algo-trading

Comments

@goodboy
Copy link
Contributor

goodboy commented Aug 5, 2022

Discovered by @iamzoltan and not surprising since i never tested any of this in the development of #349.
The plan is to land that PR despite this issue (since master has a half working / bug filled order mode for kraken right now) and resolve this in post mortem fixes.


issue descr

More or less there seems to be the ValueError raised from the pp validation logic section of trades_dialogue() but reproducing this requires a ledger with the appropriate intercrypto pair transactions.


Solutions/TODO:

  • one approach could be to have @iamzoltan submit an example ledger file / trades log + balance values that exemplifies the issue
    • hand over an example trades_kraken_spot.toml file
    • hand over the balance values from Client.get_balances() which can be hard coded manually in the code for testing
  • @iamzoltan actually solves this directy 😂

traceback

  • this would be handy to have to get idea of the issue.
@goodboy goodboy added bug guille broke it prolly clearing auction and mm tech: EMS, OMS, algo-trading broker-backend `brokerd`/`datad` related backend tech labels Aug 5, 2022
@goodboy
Copy link
Contributor Author

goodboy commented Jan 12, 2023

I got something similar due to a couple different issues, so here is a (partial) follow up TODO list:


  • for wallet transfers out of an account we get a Transaction-like entry in our pps.toml of the form:
[kraken.spot."xrpeur.kraken"]
size = 78.31125778
ppu = 0.3222278400000044
bsuid = "xrpeur"
clears = [
 { dt = "2022-12-21T07:20:16.316061+00:00", ppu = 0.3222278400000044, accum_size = 778.33125778, price = 0.3212, size = 778.33125778, cost = 0.4, tid = "TJKLRL-7UE2L-BX2UE5" },
 { dt = "2022-12-21T07:26:56+00:00", ppu = 0.3222278400000044, accum_size = 78.31125778, price = "NaN", size = -700.02, cost = 0, tid = "1503A92E24BAA1E8B8E7B78E79A78AAF3E6C3F3E35227EF7D2C13754876B37D5" },
]
  • Notice the price field has a Nan value which seemingly can break .pp.py clearing table calcs, though adjusting the clear iteration loop to sort on datetime seems to have resolved the crash?
  • we should probably put in a type field which can be filled with things like 'withdrawal' or 'split' or 'reverse_split', which i'm pretty sure is a bullet somewhere in Positioning system refinements: paper engine, file writing, toml style and perf  #345

  • we aren't tabulating nor using the widthdrawal fee schedule in our cost field in each ledger entry, obviously this should resolve any accounting discrepancies a transfer-as-sell clear should have a cost (that kraken takes) as part of the position calc.

goodboy added a commit that referenced this issue Jan 13, 2023
See more details in the GH comment:
#373 (comment)

More or less we need to pull and include the transfer fees for
withdrawals in our ledger tracking but this serves as a sloppy
workaround for the moment.
goodboy added a commit that referenced this issue Jan 13, 2023
Likely pertains to helping with stuff in issues #345 and #373 and just
generally is handy to have when processing ledgers / clearing event
tables.

Adds the following helper methods:
- `iter_by_dt()` to iter-sort an arbitrary `Transaction`-like table of
  clear entries.
- `Position.iter_clears()` as a convenience wrapper for the above.
goodboy added a commit that referenced this issue Jan 13, 2023
Likely pertains to helping with stuff in issues #345 and #373 and just
generally is handy to have when processing ledgers / clearing event
tables.

Adds the following helper methods:
- `iter_by_dt()` to iter-sort an arbitrary `Transaction`-like table of
  clear entries.
- `Position.iter_clears()` as a convenience wrapper for the above.
goodboy added a commit that referenced this issue Jan 13, 2023
See more details in the GH comment:
#373 (comment)

More or less we need to pull and include the transfer fees for
withdrawals in our ledger tracking but this serves as a sloppy
workaround for the moment.
goodboy added a commit that referenced this issue Jan 13, 2023
Likely pertains to helping with stuff in issues #345 and #373 and just
generally is handy to have when processing ledgers / clearing event
tables.

Adds the following helper methods:
- `iter_by_dt()` to iter-sort an arbitrary `Transaction`-like table of
  clear entries.
- `Position.iter_clears()` as a convenience wrapper for the above.
@goodboy
Copy link
Contributor Author

goodboy commented Jan 14, 2023

ergg,

found another after a recent xrp transfer..

Traceback (most recent call last):
  File "/home/goodboy/repos/tractor/tractor/_runtime.py", line 196, in _invoke
    res = await coro
  File "/home/goodboy/repos/piker/piker/brokers/kraken/broker.py", line 609, in trades_dialogue
    raise ValueError(
ValueError: Could not reproduce balance:
dst: xrp, 7.78e-06

goodboy added a commit that referenced this issue Jan 30, 2023
Likely pertains to helping with stuff in issues #345 and #373 and just
generally is handy to have when processing ledgers / clearing event
tables.

Adds the following helper methods:
- `iter_by_dt()` to iter-sort an arbitrary `Transaction`-like table of
  clear entries.
- `Position.iter_clears()` as a convenience wrapper for the above.
goodboy added a commit that referenced this issue Jan 31, 2023
Likely pertains to helping with stuff in issues #345 and #373 and just
generally is handy to have when processing ledgers / clearing event
tables.

Adds the following helper methods:
- `iter_by_dt()` to iter-sort an arbitrary `Transaction`-like table of
  clear entries.
- `Position.iter_clears()` as a convenience wrapper for the above.
@goodboy goodboy added the accounting prolly positioning: the accounting of "what/when (is) owned" label Mar 2, 2023
@goodboy
Copy link
Contributor Author

goodboy commented May 24, 2023

Closed in favor of #515

@goodboy goodboy closed this as completed May 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accounting prolly positioning: the accounting of "what/when (is) owned" broker-backend `brokerd`/`datad` related backend tech bug guille broke it prolly clearing auction and mm tech: EMS, OMS, algo-trading
Projects
None yet
Development

No branches or pull requests

2 participants