exchanges: Add Deribit exchange support #1082

samuael · 2022-11-11T10:52:15Z

PR Description

This pull request Included a code implementation for the Deribit exchange version 2; with REST and web-socket support. All the Websocket and Wrapper functions are tested. My code follows the style guidelines of other exchanges of GCT.
Fixed some errors with the help of golangci-lint. Endpoint methods that have no support from the GCT wrapper are also implemented for future use.

Fixes # (issue)

Type of change

Please delete options that are not relevant and add an x in [] as item is complete.

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update

How has this been tested

For testing the exchange with your own API Credentials you can use your own API key { } and passphrase fields in the unit test deribit_test.go file, or directly add credential information to the config file you will be using for running.

go test ./... -race
golangci-lint run
Test X

Checklist

My code follows the style guidelines of this project
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation and regenerated documentation via the documentation tool
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally and on Github Actions/AppVeyor with my changes
Any dependent changes have been merged and published in downstream modules

gloriousCode

Hey! While I'm awaiting you to fix the OKx comments. I did a quick review here. Its very exciting having another exchange up for review!

I think you've gone too heavy handed on the API validation. By making the checks so strict, if Derebit ever changes anything, as you can see, you would need to update well over 100 functions. Some validation is fine for API requests, like instrument != "" or "price <= 0", but looking for specific candles intervals, or currencies is too much. Let the API fail the user. I understand that I haven't really emphasised this with the OKx reviews, but the things that you check there are less likely to change than something like a currency, or a candle interval

If you do have repeating validation checks that are specific, such as the time checks, please create validation functions. That way there is only one place where the validation goes and it can be easily updated and is repeated. As a result of this, I've needed to add many comments (not exhaustive) and you've created more work for yourself to address them. As you can see with the time start and end checks, they've also be inconsistently applied, having one function handle all instances reduces the amount of work you need to do

Also looks like there are some implementation issues to work out,see the lint logs.

exchanges/deribit/deribit_wrapper.go

exchanges/deribit/deribit_websocket_eps.go

thrasher-

Thanks for adding the additional test coverage + other changes! Found these additional things:

exchanges/deribit/deribit.go

exchanges/deribit/deribit_test.go

thrasher- · 2024-05-21T02:51:21Z

exchanges/asset/asset_test.go

+		t.Run(testScenario.item.String(), func(t *testing.T) {
+			t.Parallel()
+			if testScenario.item.IsOptions() != testScenario.isOptions {
+				t.Errorf("expected %v isFutures to be %v", testScenario.item, testScenario.isOptions)


Suggested change

t.Errorf("expected %v isFutures to be %v", testScenario.item, testScenario.isOptions)

t.Errorf("expected %v isOptions to be %v", testScenario.item, testScenario.isOptions)

thrasher- · 2024-05-21T02:54:03Z

CONTRIBUTORS

 TaltaM | https://github.com/TaltaM
 dackroyd | https://github.com/dackroyd
 cranktakular | https://github.com/cranktakular
 khcchiu | https://github.com/khcchiu
 yangrq1018 | https://github.com/yangrq1018
 woshidama323 | https://github.com/woshidama323
 crackcomm | https://github.com/crackcomm
-azhang | https://github.com/azhang
+mshogin | https://github.com/mshogin


mshogin, herenow, soxipy, azhang are duplicated

… into gderibit

shazbert

Basic things on new changes

exchanges/deribit/deribit.go

shazbert · 2024-05-22T01:03:10Z

exchanges/deribit/deribit.go

@@ -2393,6 +2393,9 @@ func (d *Deribit) ExecuteBlockTrade(ctx context.Context, timestampMS time.Time,
 	if len(trades) == 0 {
 		return nil, errNoArgumentPassed
 	}
+	if timestampMS.IsZero() {


VerifiyBlockTrade is called below so there are a double up on these checks, can probably remove them and also target coverage for trade data.

shazbert · 2024-05-22T01:06:02Z

exchanges/deribit/deribit_websocket_eps.go

@@ -2192,7 +2192,7 @@ func (d *Deribit) WSExecuteBlockTrade(timestampMS time.Time, nonce, role, symbol
 	if timestampMS.IsZero() {


Same doubling up affect as the other function as this calls WSVerifyBlockTrade

gloriousCode

Thanks for all those changes!

exchanges/deribit/deribit_websocket.go

exchanges/deribit/deribit_test.go

shazbert

Changes ACK. Thanks sir.

thrasher-

Thanks for making those changes!

gloriousCode

tACK!

thrasher-

Last batch of nits, I swear 😆

CONTRIBUTORS

thrasher- · 2024-05-24T04:20:37Z

exchanges/deribit/deribit_types.go

+	errInvalidAPIKeyID                     = errors.New("invalid api key id")
+	errMaxScopeIsRequired                  = errors.New("max scope is required")
+	errTradeModeIsRequired                 = errors.New("self trading mode is required")
+	errInvalidSubaccountPassword           = errors.New("subaccount password can not be empty")


This is unused

exchanges/deribit/deribit_types.go

thrasher- · 2024-05-24T04:24:13Z

exchanges/deribit/deribit_wrapper.go

+	if err != nil {
+		log.Errorln(log.ExchangeSys, err)
+	}
+	err = d.DisableAssetWebsocketSupport(asset.Options)


Suggested change

err = d.DisableAssetWebsocketSupport(asset.Options)

for _, assetType := range []asset.Item{asset.Options, asset.OptionCombo, asset.FutureCombo} {

if err = d.DisableAssetWebsocketSupport(assetType); err != nil {

log.Errorln(log.ExchangeSys, err)

}

}

thrasher- · 2024-05-24T04:25:20Z

exchanges/deribit/deribit_wrapper.go

+	if err != nil {
+		log.Errorln(log.ExchangeSys, err)
+	}
+	if err = d.StoreAssetPairFormat(asset.Futures, currency.PairStore{RequestFormat: requestFmt, ConfigFormat: configFmt}); err != nil {


Suggested change

if err = d.StoreAssetPairFormat(asset.Futures, currency.PairStore{RequestFormat: requestFmt, ConfigFormat: configFmt}); err != nil {

for _, assetType := range []asset.Item{asset.Futures, asset.Options, asset.OptionCombo, asset.FutureCombo} {

if err = d.StoreAssetPairFormat(assetType, currency.PairStore{RequestFormat: requestFmt, ConfigFormat: configFmt}); err != nil {

log.Errorln(log.ExchangeSys, err)

}

}

thrasher- · 2024-05-24T04:29:17Z

exchanges/deribit/deribit_test.go

+)
+
+func TestMain(m *testing.M) {
+	d.SetDefaults()


This can take advtange of the internal test exchange Setup helper, like:

d = new(Deribit) if err := testexch.Setup(d); err != nil { log.Fatal(err) }

To rid a lot of the TestMain boilerplate

thrasher- · 2024-05-24T04:31:46Z

exchanges/deribit/deribit_test.go

+
+var (
+	d                                                                                                            = &Deribit{}
+	futuresTradablePair, optionsTradablePair, optionComboTradablePair, futureComboTradablePair, spotTradablePair currency.Pair


You can hardcode spotTradablePair to currency.NewPairWithDelimiter("BTC", "USDC", "_") and futuresTradablePair to currency.NewPairWithDelimiter("BTC", "PERPETUAL", "-") thus eliminating the need to use NewPairFromString in instantiateTradablePairs

exchanges/deribit/deribit_test.go

…ints

shazbert

Nice work on the cleanup! One thing I found on recent changes that could be a problem.

exchanges/deribit/deribit.go

shazbert · 2024-05-27T00:14:25Z

also can you add these lines to fix codespell reference here. Your PR will be merged first.

shazbert

Changes ACK. Thanks.

gloriousCode

tACK! Thanks for the test updates

… into gderibit

thrasher-

Thanks for making those changes, some further ones I've found

thrasher- · 2024-05-28T05:54:47Z

exchanges/deribit/deribit_websocket.go

+				}
+			}
+		case instrumentStateChannel,
+			rawUsersOrdersKindCurrencyChannel:


This case statement contains string formatting directives

thrasher- · 2024-05-28T05:55:22Z

exchanges/deribit/deribit_websocket.go

+			}
+		case incrementalTickerChannel,
+			quoteChannel,
+			rawUserOrdersChannel:


This string contains string format directives

These strings differentiate channels selected for subscription and are not used in handling the incoming stream data messages.

geseq and others added 21 commits September 14, 2022 15:15

deribit implementation

317542f

add ws impll

12cad77

cleanup

cdfc09b

Update deribit_wrapper.go

326d6e6

Add missing endpoints

5386edb

Fix config file

ab3b1f9

Merge branch 'master' into gderibit

5407c0b

asset type update

ff8bdc8

Update code structure

0521182

Update authenticated private endpoints unit tests

9035b5a

Updating websocket

af3d152

Updating websocket connection and subscription handling

319826e

Finishing up adding subscription push data

86dce65

Adding websocket public endpoint

217df42

Adding WS endpoints

0a4e1c3

Adding websocket unit tests

c52a9c3

Minor clean-up

968d484

Integrating websocket endpoints into the wrapper funcs

6145614

Updating exchange documentations

309807d

Fixing test issues

8862072

Code cleaning-up

b937687

samuael force-pushed the gderibit branch from dc5d2f0 to b937687 Compare November 11, 2022 18:22

samuael added 2 commits November 12, 2022 08:36

Merge branch 'master' into gderibit

0ac5d68

fix test issues

ccbe121

gloriousCode requested a review from a team November 13, 2022 20:25

gloriousCode assigned samuael Nov 13, 2022

gloriousCode added the review me This pull request is ready for review label Nov 13, 2022

thrasher- mentioned this pull request Nov 13, 2022

deribit implementation #1032

Closed

gloriousCode requested changes Nov 13, 2022

View reviewed changes

shazbert removed the review me This pull request is ready for review label Nov 13, 2022

thrasher- requested changes May 21, 2024

View reviewed changes

samuael added 2 commits May 21, 2024 09:03

Updated unit tests based on review comment

ac69c16

Merge branch 'master' of http://github.com/thrasher-corp/gocryptotrader…

86f891f

… into gderibit

shazbert requested changes May 22, 2024

View reviewed changes

gloriousCode requested changes May 22, 2024

View reviewed changes

samuael added 2 commits May 22, 2024 09:25

Update unit tests and removed password change endpoint

9100bba

Fix race condition

b9e008d

shazbert approved these changes May 22, 2024

View reviewed changes

thrasher- approved these changes May 23, 2024

View reviewed changes

shazbert removed the gcrc GloriousCode Review Complete label May 23, 2024

gloriousCode approved these changes May 23, 2024

View reviewed changes

thrasher- requested changes May 24, 2024

View reviewed changes

samuael added 4 commits May 24, 2024 08:53

Update on tests, test pairs, and wrapper config

6ebb729

Update test tradable pairs loading

1f2d5f2

Update unit tests, fix linter issues, and update wrapper functions

724ad17

remove credentials

5891a8b

samuael force-pushed the gderibit branch from dc9f702 to 5891a8b Compare May 26, 2024 09:28

Update test and fix authentication method and few authenticated endpo…

ce6fa40

…ints

shazbert requested changes May 27, 2024

View reviewed changes

exchanges/deribit/deribit.go Show resolved Hide resolved

fix codespell issue

33c5a99

shazbert approved these changes May 27, 2024

View reviewed changes

gloriousCode approved these changes May 28, 2024

View reviewed changes

samuael added 3 commits May 28, 2024 06:59

Merge branch 'master' of http://github.com/thrasher-corp/gocryptotrader…

6fe8076

… into gderibit

group the repeated currency code check to a func

626c485

added unit test for repeated pair check func

e1497ba

thrasher- requested changes May 28, 2024

View reviewed changes

Added a base coin and related updates

21fa83e

thrasher- merged commit aeb4a87 into thrasher-corp:master May 29, 2024
5 of 11 checks passed

samuael deleted the gderibit branch June 22, 2024 19:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

exchanges: Add Deribit exchange support #1082

exchanges: Add Deribit exchange support #1082

samuael commented Nov 11, 2022

gloriousCode left a comment •

edited

Loading

thrasher- left a comment

thrasher- May 21, 2024

thrasher- May 21, 2024

shazbert left a comment

shazbert May 22, 2024

shazbert May 22, 2024

gloriousCode left a comment

shazbert left a comment

thrasher- left a comment

gloriousCode left a comment

thrasher- left a comment

thrasher- May 24, 2024

thrasher- May 24, 2024

thrasher- May 24, 2024

thrasher- May 24, 2024

thrasher- May 24, 2024 •

edited

Loading

shazbert left a comment

shazbert commented May 27, 2024

shazbert left a comment

gloriousCode left a comment

thrasher- left a comment

thrasher- May 28, 2024

thrasher- May 28, 2024

samuael May 28, 2024

	t.Errorf("expected %v isFutures to be %v", testScenario.item, testScenario.isOptions)
	t.Errorf("expected %v isOptions to be %v", testScenario.item, testScenario.isOptions)

		@@ -2192,7 +2192,7 @@ func (d *Deribit) WSExecuteBlockTrade(timestampMS time.Time, nonce, role, symbol
		if timestampMS.IsZero() {

exchanges: Add Deribit exchange support #1082

exchanges: Add Deribit exchange support #1082

Conversation

samuael commented Nov 11, 2022

PR Description

Type of change

How has this been tested

Checklist

gloriousCode left a comment • edited Loading

Choose a reason for hiding this comment

thrasher- left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shazbert left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gloriousCode 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

gloriousCode left a comment

Choose a reason for hiding this comment

thrasher- left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thrasher- May 24, 2024 • edited Loading

Choose a reason for hiding this comment

shazbert left a comment

Choose a reason for hiding this comment

shazbert commented May 27, 2024

shazbert left a comment

Choose a reason for hiding this comment

gloriousCode left a comment

Choose a reason for hiding this comment

thrasher- left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gloriousCode left a comment •

edited

Loading

thrasher- May 24, 2024 •

edited

Loading