-
Notifications
You must be signed in to change notification settings - Fork 3.9k
[Binance] Reimplement binance exchange with better error and retry handling #1508
Conversation
Thanks as always @cmroche. I will test it now. |
Currently getting the following; [binance.js] undefined |
@ms121 Thanks, can you tell me what action it was trying to complete? Enable debug logging please for more details. FYI, the "undefined" error should be fixed once the binance PR is merged. With debug logging we should still be able to see the nature of the error leading to this. Edit: I was able to figure out which call was causing it, fix will come soon. |
@ms121 Should be fixed now. |
Bonus fix for sqlite3 issues
@askmike Note that I've updated sqlite3 to 3.1.13 since the older versions we were relying on were no longer available for download and were failing to build. This will resolve likely incoming issues of people unable to install that module. |
@cmroche how do I install the fixed binance |
@clownfish44 Easiest way to get this PR is to checkout the branch I used to generate it, or download the branch. Details here https://github.com/cmroche/gekko/tree/binance_retry once this is merged to develop and release, you'll probably want to switch back here. |
Although it isn't in order (a-z) (hence why I haven't created a pull request until I fix it up) here is every Currency / Asset for anyone that wants to use it; |
I've been trying to use your branch, and find that it still crashes every so often. I get an error like [binance.js] undefined Or something along those lines. |
@TedChenNZ sharing the actual error message would help a lot... |
@TedChenNZ You need to update your binance module. |
@greenbigfrog The error message was literally Error: @cmroche I updated it to 1.1.1 and still got an error. This time it's [binance.js] Response code 400 |
@TedChenNZ 400 is a data error, start by checking that your API keys are correct, and if so then please open a new support issue with details from the debug log and indicate if you were paper trading, live trading, and which asset pair. You can enable debug logging by changing config.debug to true in your configuration files, if you are using the UI you will find this in baseConfig.js. Thank you. |
I figured out where the logs were and got this: I was live trading ETH/OMG. It's not my API key because it does still make some trades sometimes before crashing. Should I be adding the support issue in this repo or the binance one. |
@TedChenNZ This one please. Try changing this:
to
and let me know if that works please. It seems some asset have a different precision and minimal order. |
@cmroche the MIN_NOTIONAL error can occur when the rounding may be over the minimum order through the actual buy price/amount isn't. To stop a majority of those errors we could check the "cost" which would simply be price * amount (which would have to be above the minimum order to make a trade).
|
I am getting the MIN_NOTIONAL error as well, I made some adjustments to the minOrder and Precision but it still hits it every now and then. The last time it did:
The API documentation seems to imply that you can pull these values for traded pairs dynamically (although I am not sure I'm reading it right). So maybe the hard coded values are not needed. In any case, I am not sure exactly what the issue is but I'd love to help figure it out. |
@ms121 Thanks for the information. I'll look at re-doing the way rounding and lot sizes are done to better handle the minimums after the holidays. |
@werkkrew Yes, you can, but this would require changes to gekko to make calls to getTradeCapabilities asynchronous. I've looked at this since it was something I want to do still, it isn't a trivial change and so updating hard coded values is the way to go for now. There are still bugs to be fixed with lot size minimums. |
@cmroche So for example, if you try to sell 0.01 ZEC @ 0.75 ETH the TOTAL is < 0.01 which results in an error. I'm not sure if its as simple as changing what the minimalOrder logic looks at or if the bug is more difficult to solve. It looks like it would be somewhere in the addOrder function but I don't know if this computation is done upstream of the binance api definition or not because I don't see where the logic of deciding if an order can be added or not exists before it gets to the addOrder stage. For now I adjusted the minimalOrder value for ETH/ZEC to be a bit higher to hopefully prevent it from placing orders below that value. Edit: Looking at: plugins/trader/portfolioManager.js it appears that the code calculates the minimum correctly if the minimumOrder type is 'currency' - although it does not seem to try to calculate the total order on both sides (buy/sell).
|
@cmroche looks like you are missing |
This bring the binance exchange inline with the error and retry handling improvements made for the Kraken exchange.
And additional change was pushed to tiagosiebler/binance#9 to improve error reporting on the binance API module (but we can safely merge this without that change)
Do not merge #1507 once this change is accepted
Should fix issues #1481 #1487 #1491 #1517 #1524