Skip to content
This repository has been archived by the owner on Feb 16, 2020. It is now read-only.

Use script to generate market data for binance, karken and bitfinex and exchange function for lot sizing #1550

Merged
merged 29 commits into from
Jan 21, 2018
Merged

Conversation

cmroche
Copy link
Contributor

@cmroche cmroche commented Dec 24, 2017

  • What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

Feature

  • What is the current behavior? (You can also link to an open issue here)

Product entries are done by hand, for exchanges with many assets this is tedious and prone to human error.

getMinimumSize on portfolioManager doesn't handle Binance's lot size requirements correctly result in occasional errors when posting small orders.

  • What is the new behavior (if this is a feature change)?

Binance market data is now moved to a second .js file, which is updateable with a script. The script parses the product definitions for all available pairs and adds them to the product list with all necessary information.

Added the function getLotSize to the exchange and updated portfolioManager to check for this function and use it is available for computing the correct lot size of an order. General rules are that if we are blow the minimum, we don't trade. This will allow any exchange to define necessary behaviour for lot sizing to that specific exchange.

  • Other information:

If we are happy with this approach for managing market data I'll apply the change to Kraken and Bitfinex as well.

@cmroche
Copy link
Contributor Author

cmroche commented Dec 24, 2017

This change is built on top of #1508, changes specific to this request start at 580fafd

@cmroche
Copy link
Contributor Author

cmroche commented Dec 25, 2017

I submitted the same change for Kraken, as a bonus I was finally able to remove all the special handling code for newer assets. While not clearly documented in their AI, the returned API object keys actually give us the correct order book name for an asset pair queries.

@ms121
Copy link

ms121 commented Dec 26, 2017

@cmroche
Tested although got the following error:

2017-12-26 16:07:17 (DEBUG): [binance.js] (addOrder) BUY 1704.8631578947368 POE @0.00000189 BTC
2017-12-26 16:07:18 (ERROR): [binance.js] (addOrder) returned an irrecoverable error: undefined
2017-12-26 16:07:18 (DEBUG): [binance.js] entering "setOrder" callback after api call, err: [binance.js] undefined data: {"code":-1013,"msg":"Filter failure: LOT_SIZE"}

@cmroche
Copy link
Contributor Author

cmroche commented Dec 26, 2017

@ms121 The undefined error suggst that you might be using a binance older than 1.1.0, could you check and confirm that please?

That aside, I'll look into why it tried to allow that order to go through, it clearly should have been prevented due to the lot size.

@cmroche
Copy link
Contributor Author

cmroche commented Dec 26, 2017

@ms121 Should be fixed now.

@jessrbird
Copy link

@cmroche Sorry for another newb question, but what is the easiest way to download all the changed files in the "files changed" section, or otherwise update my gekko installation with these latest code changes? I've been copying and pasting the text, but I'm thinking there has to be a better way. Thanks!

@cmroche
Copy link
Contributor Author

cmroche commented Dec 28, 2017

@jessrbird You cannot really download just the changed files, the easiest way to test a PR is to checkout or download the branch it is based on. In this case that would be here: https://github.com/cmroche/gekko/tree/origin/product_updates

If you are using GitKraken as your GIT client, it does have the ability to grab and apply PRs in the client. You would need to handle conflicts yourself, but should be minimal if you are based on the 'develop' branch. I am not an expert with GitKraken though, so I cannot offer much help with this.

@werkkrew
Copy link
Contributor

I pulled this commit and ran the updateBinance.js script, the markets file looks correct. When running it on actual trading I get the error:

2017-12-28 10:10:54 (DEBUG): [binance.js] entering "setBalance" callback after api call, err: null, data: [object Object]
2017-12-28 10:10:54 (ERROR): Wanted to buy ZEC but the amount is too small (0.000000000000) at binance

@cmroche
Copy link
Contributor Author

cmroche commented Dec 28, 2017

@werkkrew Please update your binance module with npm i [email protected] to fix the error reporting. I'll look at why it tried to buy with nothing.

@cmroche
Copy link
Contributor Author

cmroche commented Dec 28, 2017

@werkkrew The error that you are seeing, that the amount was too small, is correct if the amount it is requesting is too small for the market LOT size. I've made some small changes though, changing the error to a warning and printing the original requested amount and price (not the adjusted) just in case the lot size calculation was wrong. If you update try again and let me know if the requested amount was actually larger than the minimum lot sizes.

@jessrbird
Copy link

@cmroche I downloaded the zip using the link you provided above to get the latest PR. Getting an error running tradebot at binance USDT/BCC pair. The bot tried to sell BCC, but BCC balance was already 0. Here is the error:

/home/money/gekko/plugins/trader/portfolioManager.js:236
return log.warning(
^

TypeError: log.warning is not a function
at process (/home/money/gekko/plugins/trader/portfolioManager.js:236:18)
at bound (/home/money/gekko/node_modules/lodash/dist/lodash.js:729:21)
at Trader.getLotSize (/home/money/gekko/exchanges/binance.js:215:12)
at Trader.bound [as getLotSize] (/home/money/gekko/node_modules/lodash/dist/lodash.js:729:21)
at Manager.sell (/home/money/gekko/plugins/trader/portfolioManager.js:261:19)
at Manager.bound [as sell] (/home/money/gekko/node_modules/lodash/dist/lodash.js:729:21)
at Manager.act (/home/money/gekko/plugins/trader/portfolioManager.js:169:16)
at bound (/home/money/gekko/node_modules/lodash/dist/lodash.js:729:21)
at /home/money/gekko/node_modules/async/dist/async.js:3853:9
at /home/money/gekko/node_modules/async/dist/async.js:484:16
RECEIVED ERROR IN GEKKO 352640273213704

Let me know what you think.

@cmroche cmroche changed the title Use script to generate market data for binance, and exchange function for lot sizing Use script to generate market data for binance, karken and bitfinex and exchange function for lot sizing Dec 29, 2017
@cmroche
Copy link
Contributor Author

cmroche commented Dec 29, 2017

@jessrbird Bah, should be log.warn sorry.. will send a fix in a few minutes

@askmike
Copy link
Owner

askmike commented Jan 21, 2018

@cmroche apologies for the delays on this PR, it escaped my attention completely.

First note: I love this approach! I think it is important that Gekko only supports currencies whatever we release, and this way we stay in total control between release cycles.

Some quick questions/notes:

Added the function getLotSize to the exchange and updated portfolioManager

Is this function required? Eg. will merging this break every other exchange (that does not have this implemented)? If so: I can work on making this optional.

exchanges/data/bitfinexMarkets.js

For me personally it would be a lot more clear to call this file exchanges/bitfinex-markets.json: when looking at available exchanges only .js files are considered, on top of that JSON for generated static data makes more sense to me.

exchanges/data/update-binance.js

What do you think about moving this to a util folder that can generate whatever is necessary (which for now means only market files).

@cmroche
Copy link
Contributor Author

cmroche commented Jan 21, 2018

@askmike

getLotSize is sort of necessary for Binance. They have a lot of rules about lot sizing, they could be expressed generically and add more properties to control in the portfolioManager, but I think the code would actually be more complicated in the end, and likely wouldn't be used anywhere else. By providing this function we can take more complicated cases of lot sizing techniques used by exchanges and allow them to be defined in the exchanges themselves which is potentially beneficial.

The function is option though. portfolioManager checks is the function exists, if it does then it uses the provided function, if not it falls back to the old behaviour. Documentation is updated to indicate that it is optional as well.

Changing to JSON is not a problem, I considered this but went with JS to be consistent with the config files. It makes sense to use JSON though, and putting them in another folder is no problem at all. util is sort of a busy folder right now and very code related, perhaps a scripts folder, or a data folder, or something else?

@askmike
Copy link
Owner

askmike commented Jan 21, 2018

The function is option though. portfolioManager checks is the function exists, if it does then it uses the provided function, if not it falls back to the old behaviour.

perfect!


testing this now, if it all seems to work and I will manually fix what I like to see different :)

@askmike
Copy link
Owner

askmike commented Jan 21, 2018

works great! If you can merge this: https://github.com/cmroche/gekko/pull/3 I'll merge this PR :)

@cmroche
Copy link
Contributor Author

cmroche commented Jan 21, 2018

@askmike Thanks :) Done.

@askmike askmike merged commit c9ec544 into askmike:develop Jan 21, 2018
@askmike
Copy link
Owner

askmike commented Jan 21, 2018

Seems I didn't commit everything, note that this is commit is needed now: 8bf4670

@talvasconcelos
Copy link

@cmroche or anyone that can shed some light on this...
I need, for a scanner/trader bot i'm building for Binance, to place some orders. Can't seem to figure out how to make it work with binance's requirements. Been lurking gekko's code, especially the exchange file for binance, but i can't seem to figure out how the roundAmount and the lotSize work with the place order method!! Math is not my strong point! Also i can't seem to find the file where the placeOrder is called to see how gekko is preparing the amount to pass to the method.

What i want to achieve is place a buy order, at market price would be nice, with the BTC amount available. Then when conditions are meet, place a sell order with the bought currency amount available.

Can someone help out?

@zzmike76
Copy link

hello

has the fix been merged? i am still getting the MIN_NOTIONAL error even after updating the dev branch. should I open a new issue?

thanks

@cmroche cmroche deleted the origin/product_updates branch February 19, 2018 23:10
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.