whittlem / pycryptobot

Python Crypto Bot (PyCryptoBot)
Apache License 2.0
1.97k stars 738 forks source link

Fix binance zero fees #724

Closed Nicolas469 closed 1 year ago

Nicolas469 commented 1 year ago

Description

Binance has introduce zero fees (maker and taker) for some pairs. Refs: https://www.binance.com/en/support/announcement/binance-launches-zero-fee-bitcoin-trading-10435147c55d4a40b64fcbf43cb46329 && https://www.binance.com/en/blog/markets/zero-btc-trading-fees-they-do-what-they-say-on-the-tin-6168074889174657450

Fixes # (issue)

This change will check the market and returns 0 if the bot market is one of those. I've also updated the simulation.

Please make your selection.

How Has This Been Tested?

I used the provided script-get_fees.py with different market to check that fees are 0.1% for all pairs and 0% for those.

Checklist:

loomsen commented 1 year ago

Hi,

thank you. Looks ok to me, but could you look into the unit tests and why they are failing, please? That'd be great :)

Nicolas469 commented 1 year ago

Done. It sas some typos in readme and in a test file. Don't know how they've pass the codespell test before...

loomsen commented 1 year ago

Thank you, the code LGTM. I didn't check if it makes sense though. The only issue I could see is the implicit type conversion from float to int when you're setting takerfee = -1 which was 0.00 before.

So you may want to explicitly typecast it back to float, just for safety.

>>> type(-1)
<class 'int'>
>>> type(0.00)
<class 'float'>
>>> type(float(-1))
<class 'float'>
>>> 
whittlem commented 1 year ago

Hi @Nicolas469, thanks for the PR. This is a great update. We don't raise PR's directly into the "main" branch. It needs to be go into "beta" first. In addition to this for the last few weeks/months I've been working on a major update. It will replace the "beta" branch and become main. Can you please check out "dev/pycryptobot7", install the deps, and make your change there in a new PR. Please note that there has been a major code update so please confirm to the new "rules". I've explained here: https://trading-data-analysis.pro/pycryptobot-v7-is-almost-here-b68bdb87d33a

Nicolas469 commented 1 year ago

Thx @loomsen for the feedback, I've updated the type of "takerfee". @whittlem: I've changed to "beta" and I'll check the v7 later :)

whittlem commented 1 year ago

Thanks, not "beta"... "dev/pycryptobot7". That is the new major release which will replace "beta". There is a huge code update so you may need to update your code to work on the new version.

Nicolas469 commented 1 year ago

Sorry I thought that while waiting for v7 to be live we could merge this update in v6. But yes I'll check v7 and also make this change there.