search

bmoscon / cryptofeed

Cryptocurrency Exchange Websocket Data Feed Handler
Other
2.19k stars 679 forks source link

Add L2_BOOK to list of authenticated channels for OKX #849

Closed JKOK005 closed 2 years ago

JKOK005 commented 2 years ago

Describe the bug A clear and concise description of what the bug is.

With the recent API changes introduced by Okx, users must now authenticate before accessing the order books channel. This means that the channel is no longer publicly accessible without authentication.

Current version of the code base does not support authentication of L2_BOOK channel ( see is_authenticated_channel method of https://github.com/bmoscon/cryptofeed/blob/3079107d8496f700eb1e6401bb420d5393262ad8/cryptofeed/exchange.py#L126 ). Any request for okx order books will be challenged with a please login error.

To Reproduce Steps to reproduce the behavior:

Attempt to ask for order book data from Okx using Cryptostore img and store result in Redis.

docker run \ --network=host \ -e EXCHANGE='OKX' \ -e CHANNELS='l2_book' \ -e SYMBOLS='CFX-USDT,CFX-USDT-PERP' \ -e BACKEND='REDIS' \ -e HOST='localhost' \ -e PORT=6379 \ -e SNAPSHOT_ONLY=True \ ghcr.io/bmoscon/cryptostore:latest

Expected behavior A clear and concise description of what you expected to happen.

Response: ERROR : OKX: Error: {'event': 'error', 'msg': 'Please log in', 'code': '60011'}

Screenshots If applicable, add screenshots to help explain your problem.

Operating System:

linux

Cryptofeed Version

JKOK005 commented 2 years ago

Since the Exchange class is used across multiple exchanges, one possible fix would be to overwrite the is_authenticated_channel method directly in the cryptofeed.exchanges.okx.OKX class itself, since authentication for order books seems to be a unique feature for OKX only.

I've done a test locally and this change works on my end; I can stream exchange data from Okx to Redis using Cryptostore after above changes.

Would be glad to help with a PR if necessary. May I know the proper steps to do one as well as how tests can be conducted for the code? Thanks

agijsberts commented 2 years ago

Instead of adding the l2 book to the authenticated endpoint, I replaced the authenticated books-l2-tbt channel with the publicly available books channel. This is fixed in HEAD.

Please see https://github.com/bmoscon/cryptofeed/issues/845 and the corresponding commit https://github.com/bmoscon/cryptofeed/commit/3079107d8496f700eb1e6401bb420d5393262ad8.

JKOK005 commented 2 years ago

Yes you are right. I realized that the issue was with the version of cryptofeed that I'm installing using pip. I'm running cryptostore docker image and the image is pulling version 2.2.1 of cryptofeed, whereas the latest version with the new changes is 2.2.3.

Will close this ticket and open a new one to request update of the pip repository. Thanks