Deribit: Add subscription configuration #1636

gbjk · 2024-08-30T01:20:03Z

Deribit

Add subscription configuration
Consolidate subscription requests
Fix error on authenticated subscription for orderbook

Misc

Fix kline.Raw marshalling

Stacked Dependencies

Subscriptions: Add List.Enabled, List.Public and List.Private #1629 ( But can merge in this too )

Type of change

New feature (non-breaking change which adds functionality)

codecov · 2024-08-30T01:27:57Z

Codecov Report

Attention: Patch coverage is 54.25532% with 43 lines in your changes missing coverage. Please review.

Project coverage is 37.40%. Comparing base (b209b0e) to head (a427d9a).
Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
exchanges/deribit/deribit_websocket.go	45.56%	43 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1636      +/-   ##
==========================================
+ Coverage   37.35%   37.40%   +0.05%     
==========================================
  Files         415      415              
  Lines      178627   178351     -276     
==========================================
- Hits        66731    66719      -12     
+ Misses     104112   103857     -255     
+ Partials     7784     7775       -9

Files with missing lines	Coverage Δ
exchanges/deribit/deribit.go	`46.17% <100.00%> (+0.11%)`	⬆️
exchanges/deribit/deribit_wrapper.go	`42.15% <100.00%> (+0.81%)`	⬆️
exchanges/deribit/deribit_ws_endpoints.go	`45.38% <ø> (ø)`
exchanges/kline/kline.go	`90.96% <100.00%> (+0.12%)`	⬆️
exchanges/deribit/deribit_websocket.go	`46.74% <45.56%> (+7.93%)`	⬆️

... and 12 files with indirect coverage changes

gloriousCode

tACK! That's a ~~paddlin'~~ good culling

shazbert

Thanks just one nit. 🚀

exchanges/deribit/deribit_test.go

shazbert

Pretty much tACK. Just one suggestion.

exchanges/deribit/deribit.go

shazbert

Changes ACK. Thanks!

thrasher-

One minor nit

thrasher- · 2025-01-02T01:21:56Z

exchanges/deribit/deribit.go

+// optionPairToString formats an options pair as: SYMBOL-EXPIRE-STRIKE-TYPE
+// SYMBOL may be a currency (BTC) or a pair (XRP_USDC)
+// EXPIRE is DDMMMYY
+// STRIKE make include a d for decimal point in linear options


Suggested change

// STRIKE make include a d for decimal point in linear options

// STRIKE may include a d for decimal point in linear options

Fixed gbjk@adad84b6b

Moved to exchange base by thrasher-corp#1472

Since I had to ask what this abbreviation meant, I think we should abandon it

Calling Setup twice would race on the assignment to this package var. There was an option to just move the assignment to the package var declaration, but this change improves the performance and allocations: ``` BenchmarkOptionPairToString-8 1000000 1239 ns/op 485 B/op 10 allocs/op BenchmarkOptionPairToString2-8 3473804 656.2 ns/op 348 B/op 7 allocs/op ``` I've also removed the t.Run because even success the -v output from tests would be very noisy, and I don't think we were getting any benefit from it at all: ``` === RUN TestOptionPairToString === PAUSE TestOptionPairToString === CONT TestOptionPairToString === RUN TestOptionPairToString/BTC-30MAY24-61000-C === PAUSE TestOptionPairToString/BTC-30MAY24-61000-C === RUN TestOptionPairToString/ETH-1JUN24-3200-P === PAUSE TestOptionPairToString/ETH-1JUN24-3200-P === RUN TestOptionPairToString/SOL_USDC-31MAY24-162-P === PAUSE TestOptionPairToString/SOL_USDC-31MAY24-162-P === RUN TestOptionPairToString/MATIC_USDC-6APR24-0d98-P === PAUSE TestOptionPairToString/MATIC_USDC-6APR24-0d98-P === CONT TestOptionPairToString/BTC-30MAY24-61000-C === CONT TestOptionPairToString/SOL_USDC-31MAY24-162-P === CONT TestOptionPairToString/ETH-1JUN24-3200-P === CONT TestOptionPairToString/MATIC_USDC-6APR24-0d98-P --- PASS: TestOptionPairToString (0.00s) --- PASS: TestOptionPairToString/BTC-30MAY24-61000-C (0.00s) --- PASS: TestOptionPairToString/ETH-1JUN24-3200-P (0.00s) --- PASS: TestOptionPairToString/SOL_USDC-31MAY24-162-P (0.00s) --- PASS: TestOptionPairToString/MATIC_USDC-6APR24-0d98-P (0.00s) ``` ( And that got worse with me adding more tests )

gbjk · 2025-01-02T05:18:04Z

Rebase squashed :)

thrasher-

tACK pub subs. Great stuff @gbjk !

* Kline: Fix Raw Short, Marshal and Unmarshal * Deribit: Rename GenerateDefaultSubs * Deribit: Remove custom GetDefaultConfig Moved to exchange base by #1472 * Deribit: Straight Rename of eps to endpoints Since I had to ask what this abbreviation meant, I think we should abandon it * Deribit: Add Subscription configuration * Deribit: Fix race on Setup with optionsRegex Calling Setup twice would race on the assignment to this package var. There was an option to just move the assignment to the package var declaration, but this change improves the performance and allocations: ``` BenchmarkOptionPairToString-8 1000000 1239 ns/op 485 B/op 10 allocs/op BenchmarkOptionPairToString2-8 3473804 656.2 ns/op 348 B/op 7 allocs/op ``` I've also removed the t.Run because even success the -v output from tests would be very noisy, and I don't think we were getting any benefit from it at all: ``` === RUN TestOptionPairToString === PAUSE TestOptionPairToString === CONT TestOptionPairToString === RUN TestOptionPairToString/BTC-30MAY24-61000-C === PAUSE TestOptionPairToString/BTC-30MAY24-61000-C === RUN TestOptionPairToString/ETH-1JUN24-3200-P === PAUSE TestOptionPairToString/ETH-1JUN24-3200-P === RUN TestOptionPairToString/SOL_USDC-31MAY24-162-P === PAUSE TestOptionPairToString/SOL_USDC-31MAY24-162-P === RUN TestOptionPairToString/MATIC_USDC-6APR24-0d98-P === PAUSE TestOptionPairToString/MATIC_USDC-6APR24-0d98-P === CONT TestOptionPairToString/BTC-30MAY24-61000-C === CONT TestOptionPairToString/SOL_USDC-31MAY24-162-P === CONT TestOptionPairToString/ETH-1JUN24-3200-P === CONT TestOptionPairToString/MATIC_USDC-6APR24-0d98-P --- PASS: TestOptionPairToString (0.00s) --- PASS: TestOptionPairToString/BTC-30MAY24-61000-C (0.00s) --- PASS: TestOptionPairToString/ETH-1JUN24-3200-P (0.00s) --- PASS: TestOptionPairToString/SOL_USDC-31MAY24-162-P (0.00s) --- PASS: TestOptionPairToString/MATIC_USDC-6APR24-0d98-P (0.00s) ``` ( And that got worse with me adding more tests )

gbjk self-assigned this Aug 30, 2024

gbjk mentioned this pull request Aug 30, 2024

Deribit: bug fixes, test fixes and implement GetCurrencyTradeURL #1558

Merged

14 tasks

gbjk force-pushed the feature/deribit_sub_conf branch 4 times, most recently from 7a6cee6 to 3bb37fe Compare August 30, 2024 07:19

gbjk requested review from gloriousCode, shazbert and thrasher- August 30, 2024 07:38

gbjk added the review me This pull request is ready for review label Aug 30, 2024

gbjk force-pushed the feature/deribit_sub_conf branch from 04871d1 to fa3520b Compare September 18, 2024 06:46

thrasher- added the medium priority label Oct 21, 2024

shazbert added the rebase/merge of master required label Nov 25, 2024

gbjk force-pushed the feature/deribit_sub_conf branch from fa3520b to a6e200f Compare December 4, 2024 07:31

gbjk removed the rebase/merge of master required label Dec 4, 2024

gloriousCode approved these changes Dec 5, 2024

View reviewed changes

gloriousCode added the gcrc GloriousCode Review Complete label Dec 5, 2024

gbjk force-pushed the feature/deribit_sub_conf branch from a196533 to 4fa7bc3 Compare December 12, 2024 02:48

shazbert requested changes Dec 12, 2024

View reviewed changes

exchanges/deribit/deribit_test.go Show resolved Hide resolved

gbjk requested a review from shazbert December 13, 2024 08:15

shazbert requested changes Dec 15, 2024

View reviewed changes

exchanges/deribit/deribit.go Outdated Show resolved Hide resolved

gbjk requested a review from shazbert December 17, 2024 01:57

shazbert approved these changes Dec 19, 2024

View reviewed changes

shazbert added the szrc shazbert review complete label Dec 19, 2024

thrasher- requested changes Jan 2, 2025

View reviewed changes

gbjk added 4 commits January 2, 2025 12:17

Kline: Fix Raw Short, Marshal and Unmarshal

29ee990

Deribit: Rename GenerateDefaultSubs

8a7123d

Deribit: Remove custom GetDefaultConfig

7b9a6ec

Moved to exchange base by thrasher-corp#1472

Deribit: Straight Rename of eps to endpoints

2d742c7

Since I had to ask what this abbreviation meant, I think we should abandon it

gbjk added 2 commits January 2, 2025 12:17

Deribit: Add Subscription configuration

2387d8b

gbjk force-pushed the feature/deribit_sub_conf branch from adad84b to a427d9a Compare January 2, 2025 05:17

gbjk requested a review from thrasher- January 2, 2025 05:18

thrasher- approved these changes Jan 2, 2025

View reviewed changes

thrasher- changed the title ~~Deribit: Add Subscription Configuration~~ Deribit: Add subscription configuration Jan 2, 2025

thrasher- merged commit 4fcee84 into thrasher-corp:master Jan 2, 2025
10 of 13 checks passed

gbjk deleted the feature/deribit_sub_conf branch January 3, 2025 00:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Deribit: Add subscription configuration #1636

Deribit: Add subscription configuration #1636

gbjk commented Aug 30, 2024 •

edited

Loading

codecov bot commented Aug 30, 2024 •

edited

Loading

gloriousCode left a comment

shazbert left a comment

shazbert left a comment

shazbert left a comment

thrasher- left a comment

thrasher- Jan 2, 2025

gbjk Jan 2, 2025

gbjk commented Jan 2, 2025

thrasher- left a comment

	// STRIKE make include a d for decimal point in linear options
	// STRIKE may include a d for decimal point in linear options

Deribit: Add subscription configuration #1636

Deribit: Add subscription configuration #1636

Conversation

gbjk commented Aug 30, 2024 • edited Loading

Deribit

Misc

Stacked Dependencies

Type of change

codecov bot commented Aug 30, 2024 • edited Loading

Codecov Report

gloriousCode left a comment

Choose a reason for hiding this comment

shazbert left a comment

Choose a reason for hiding this comment

shazbert left a comment

Choose a reason for hiding this comment

shazbert left a comment

Choose a reason for hiding this comment

thrasher- left a comment

Choose a reason for hiding this comment

thrasher- Jan 2, 2025

Choose a reason for hiding this comment

gbjk Jan 2, 2025

Choose a reason for hiding this comment

gbjk commented Jan 2, 2025

thrasher- left a comment

Choose a reason for hiding this comment

gbjk commented Aug 30, 2024 •

edited

Loading

codecov bot commented Aug 30, 2024 •

edited

Loading