thrasher-corp / gocryptotrader

A cryptocurrency trading bot and framework supporting multiple exchanges written in Golang.
MIT License
2.96k stars 792 forks source link

Kraken: Fix wsCancelOrders not erroring with order id #1505

Closed gbjk closed 3 months ago

gbjk commented 4 months ago

We were using the "cancel many" facility of the Kraken api. However since that doesn't actually report errors individually, it seems saner to just multiplex over it. We were going to get N+ responses anyway. Might as well send N+ requests

One thing I'm unhappy about is the change of Key/Secret for this to work. I really don't like that there's special values for the auth values which get silently ignored, such as Test, Mock, Key, Secret, etc. So I've ended up using something adhoc, and putting a require in the test to ensure it's obvious if the auth-conn isn't connected.

Type of change

How has this been tested

gbjk commented 4 months ago

I'm not liking those failures in kraken's TestWsOpenOrders; Sniffing it now

gbjk commented 3 months ago

TestWsOpenOrders errors were unrelated. This is good to review.

gbjk commented 3 months ago

Already in the next upcoming branch.

I've got significant testing of Subscribe in there without mocking, so I'm on the fence about adding a mock for it if we don't have to ( perversely ).

Anyway, Let's touch on this when that PR lands.

codecov[bot] commented 3 months ago

Codecov Report

Attention: Patch coverage is 83.01887% with 9 lines in your changes are missing coverage. Please review.

Project coverage is 36.02%. Comparing base (b909e15) to head (e7e5a5e). Report is 2 commits behind head on master.

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/thrasher-corp/gocryptotrader/pull/1505/graphs/tree.svg?width=650&height=150&src=pr&token=41784B23TS&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=thrasher-corp)](https://app.codecov.io/gh/thrasher-corp/gocryptotrader/pull/1505?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=thrasher-corp) ```diff @@ Coverage Diff @@ ## master #1505 +/- ## ========================================== - Coverage 37.83% 36.02% -1.82% ========================================== Files 411 411 Lines 147204 176906 +29702 ========================================== + Hits 55699 63728 +8029 - Misses 83733 105391 +21658 - Partials 7772 7787 +15 ``` | [Files](https://app.codecov.io/gh/thrasher-corp/gocryptotrader/pull/1505?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=thrasher-corp) | Coverage Δ | | |---|---|---| | [common/common.go](https://app.codecov.io/gh/thrasher-corp/gocryptotrader/pull/1505?src=pr&el=tree&filepath=common%2Fcommon.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=thrasher-corp#diff-Y29tbW9uL2NvbW1vbi5nbw==) | `94.79% <100.00%> (+0.03%)` | :arrow_up: | | [internal/testing/exchange/exchange.go](https://app.codecov.io/gh/thrasher-corp/gocryptotrader/pull/1505?src=pr&el=tree&filepath=internal%2Ftesting%2Fexchange%2Fexchange.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=thrasher-corp#diff-aW50ZXJuYWwvdGVzdGluZy9leGNoYW5nZS9leGNoYW5nZS5nbw==) | `63.02% <78.57%> (-0.62%)` | :arrow_down: | | [exchanges/kraken/kraken\_websocket.go](https://app.codecov.io/gh/thrasher-corp/gocryptotrader/pull/1505?src=pr&el=tree&filepath=exchanges%2Fkraken%2Fkraken_websocket.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=thrasher-corp#diff-ZXhjaGFuZ2VzL2tyYWtlbi9rcmFrZW5fd2Vic29ja2V0Lmdv) | `39.36% <76.00%> (+5.67%)` | :arrow_up: | ... and [385 files with indirect coverage changes](https://app.codecov.io/gh/thrasher-corp/gocryptotrader/pull/1505/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=thrasher-corp)
gbjk commented 3 months ago

Rebased and ready to roll 😄