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

Fix getPortfolio function in Binance #1401

Merged
merged 13 commits into from
Dec 5, 2017
Merged

Fix getPortfolio function in Binance #1401

merged 13 commits into from
Dec 5, 2017

Conversation

cmroche
Copy link
Contributor

@cmroche cmroche commented Dec 2, 2017

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

Bugfix

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

getPortfolio always returns 0 due to an incorrect use of _.first(...)

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

Function now returns the 'free' balance, replaced _.first with _.find.

  • Other information:

Should fix issue #1356

@ms121
Copy link

ms121 commented Dec 3, 2017

Hey @cmroche, it now throws the following;

TypeError: Cannot read property 'free' of undefined
at Trader.setBalance (/root/gekko/exchanges/binance.js:127:7)
at bound (/root/gekko/node_modules/lodash/dist/lodash.js:729:21)
at Request.request [as _callback] (/root/gekko/node_modules/binance/lib/rest.js:61:21)
at Request.self.callback (/root/gekko/node_modules/request/request.js:188:22)
at emitTwo (events.js:125:13)
at Request.emit (events.js:213:7)
at Request. (/root/gekko/node_modules/request/request.js:1171:10)
at emitOne (events.js:115:13)
at Request.emit (events.js:210:7)
at IncomingMessage. (/root/gekko/node_modules/request/request.js:1091:12)

Thanks for your quick response on the last issue!

@cmroche
Copy link
Contributor Author

cmroche commented Dec 3, 2017

@ms121 Fixed bindings, try now please.

@ms121
Copy link

ms121 commented Dec 3, 2017

Seems to be working ill let it run and see how it goes!
I changed line 181 to; var result = _.first(data, function(ticker) {
and err.message was returning
'TypeError: Cannot read property 'message' of null
at Trader.setTicker (/root/gekko/exchanges/binance.js:192:18)
at bound (/root/gekko/node_modules/lodash/dist/lodash.js:729:21)
at Request.request [as _callback] (/root/gekko/node_modules/binance/lib/rest.js:61:21)
at Request.self.callback (/root/gekko/node_modules/request/request.js:188:22)
at emitTwo (events.js:125:13)
at Request.emit (events.js:213:7)
at Request. (/root/gekko/node_modules/request/request.js:1171:10)
at emitOne (events.js:115:13)
at Request.emit (events.js:210:7)
at IncomingMessage. (/root/gekko/node_modules/request/request.js:1091:12)'
Fixed it by changing it to just err.

@cmroche
Copy link
Contributor Author

cmroche commented Dec 3, 2017

@ms121 Thanks for your help :) Same problem with getPortfolio, I used the wrong function... the update I just pushed should fix it. Thanks again.

@ms121
Copy link

ms121 commented Dec 3, 2017

@cmroche
As the bot tried to buy the following happened;

2017-12-03` 03:24:38 (INFO): Attempting to BUY 1.1992578439197714 NEO at binance price: 0.077
2017-12-03 03:24:38 (DEBUG): [binance.js] (addOrder) BUY 1.1992578439197714 NEO @ 0.077 ETH
2017-12-03 03:24:38 (DEBUG): [binance.js] entering "getTicker" callback after api call, err: { code: -1102,
msg: 'Mandatory parameter 'timestamp' was not sent, was empty/null, or malformed.' } data: { code: -1102,
msg: 'Mandatory parameter 'timestamp' was not sent, was empty/null, or malformed.' }
2017-12-03 03:24:38 (ERROR): [binance.js] unable to buy { code: -1102,
msg: 'Mandatory parameter 'timestamp' was not sent, was empty/null, or malformed.' }

/root/gekko/exchanges/binance.js:35
if (!error || !error.message.match(recoverableErrors)) {
^

TypeError: Cannot read property 'match' of undefined
at Trader.retry (/root/gekko/exchanges/binance.js:35:32)
at Trader.bound [as retry] (/root/gekko/node_modules/lodash/dist/lodash.js:729:21)
at Trader.setOrder (/root/gekko/exchanges/binance.js:232:19)
at bound (/root/gekko/node_modules/lodash/dist/lodash.js:729:21)
at Request.request [as _callback] (/root/gekko/node_modules/binance/lib/rest.js:52:21)
at Request.self.callback (/root/gekko/node_modules/request/request.js:188:22)
at emitTwo (events.js:125:13)
at Request.emit (events.js:213:7)
at Request. (/root/gekko/node_modules/request/request.js:1171:10)
at emitOne (events.js:115:13)

I am currently doing tests on the server time + my client time to see if that is the issue.

Normally the binance api module adds this automatically for requests that require it, but it is not on newOrder. Probably an oversight, but an easy workaround on our side.
@cmroche
Copy link
Contributor Author

cmroche commented Dec 3, 2017

@ms121 The binance node module normally automatically adds the timestamp to any authenticated queries, but look at the source it doesn't in the case of addOrder. Probably a bug on their side, but the fix is as simple as adding it to the query parameters so I've pushed a fix for it.

Also the comments said "getTicker" but actually that was a copy and paste error on my side, it was actually "addOrder" that was triggering it :)

Thank again, and let me know how it goes. I think we're pretty close now thanks to your help.

@ms121
Copy link

ms121 commented Dec 4, 2017

Thanks again @cmroche next error is the following; Going to double check it isn't because my trade is too small, although it might be to do with how many decimal places we are sending to binance.

2017-12-04 01:00:08 (DEBUG): [binance.js] entering "setOrder" callback after api call, err: { code: -1013, msg: 'Filter failure: LOT_SIZE' } data: { code: -1013, msg: 'Filter failure: LOT_SIZE' }
2017-12-04 01:00:08 (ERROR): [binance.js] unable to buy { code: -1013, msg: 'Filter failure: LOT_SIZE' }

I will try setting it to a precision of 6 and see if it makes the trade.

@cmroche
Copy link
Contributor Author

cmroche commented Dec 4, 2017

@ms121 It's because the min order was wrong, the info wasn't readily available at the time I put this together but found it now. THe markets have been cleaned up, and the min orders fixed.

@ms121
Copy link

ms121 commented Dec 4, 2017

Tested still throwing the same error;
2017-12-04 01:55:41 (INFO): Attempting to BUY 59.90620811174061 POWR at binance price: 0.00154501
2017-12-04 01:55:41 (DEBUG): [binance.js] (addOrder) BUY 59.90620811174061 POWR @ 0.00154501 ETH
2017-12-04 01:55:42 (DEBUG): [binance.js] entering "setOrder" callback after api call, err: { code: -1013, msg: 'Filter failure: LOT_SIZE' } data: { code: -1013, msg: 'Filter failure: LOT_SIZE' }
2017-12-04 01:55:42 (ERROR): [binance.js] unable to buy { code: -1013, msg: 'Filter failure: LOT_SIZE' }

@askmike
Copy link
Owner

askmike commented Dec 4, 2017

@cmroche @ms121

[binance.js] entering "setOrder" callback after api call, err: { code: -1013, msg: 'Filter failure: LOT_SIZE' } data: { code: -1013, msg: 'Filter failure: LOT_SIZE' }
2017-12-04 01:55:42 (ERROR): [binance.js] unable to buy { code: -1013, msg: 'Filter failure: LOT_SIZE' }

LOT_SIZE errors in binance have to do with the rounding of either the amount of the volume, which is different per market. In binance there are 2 checks for minimal orders:

  • is the amount above the minimum (for some small coins it needs be 1+)?
  • is the total amount (rate * amount) above another minimum (I think: 0.001 for BTC pairs and 0.01 for ETH pairs)?

After that the amount and the rate are checked for rounding: the amount and the rate both need to be rounded according to some rounding, else you get this error. I am pretty sure this is not handled by the lib we are using.

@cmroche
Copy link
Contributor Author

cmroche commented Dec 4, 2017

@askmike All of the currency pairs specific use 8 decimals places for input, are you saying that Gekko will attempt to use more decimal places than that? If so I can push a fix tonight.

@cmroche
Copy link
Contributor Author

cmroche commented Dec 4, 2017

@ms121 @askmike After a bit of research on this issue, it looks like Binance uses minOrder as the precision for the asset, and tickSize as the precision for the currency. I've added a rounding function for both of these now, hopefully that addresses the issue.

@ms121
Copy link

ms121 commented Dec 5, 2017

Thanks again @cmroche, problem now is the Price is being calculated the same as the amount by the looks of it;
2017-12-05 01:09:47 (DEBUG): [binance.js] (addOrder) BUY 60 POWR @ 60 ETH
2017-12-05 01:09:48 (DEBUG): [binance.js] entering "setOrder" callback after api call, err: { code: -2010,
msg: 'Account has insufficient balance for requested action.' } data: { code: -2010,
msg: 'Account has insufficient balance for requested action.' }
2017-12-05 01:09:48 (ERROR): [binance.js] unable to buy { code: -2010,
msg: 'Account has insufficient balance for requested action.' }

@cmroche
Copy link
Contributor Author

cmroche commented Dec 5, 2017

@ms121 Ooof :S That was a nasty copy and paste error, I'm glad it didn't work.

Fixed now.

@ms121
Copy link

ms121 commented Dec 5, 2017

HAHA I thought so as well! I am testing it now.
@cmroche We now have an open order I will see how it goes!
Thanks Again!

@cmroche
Copy link
Contributor Author

cmroche commented Dec 5, 2017

@askmike The build pipeline errors are related to using an older npm version, I don't think I can fix this on my side.

@askmike
Copy link
Owner

askmike commented Dec 5, 2017

No problem, I'll work on upgrading dependencies. I'll merge this for now and if more problems arise we can address them in new PRs. Thanks a lot @cmroche!

@askmike askmike merged commit 30f7e32 into askmike:develop Dec 5, 2017
@cmroche cmroche deleted the binance_fixes_2 branch December 5, 2017 06:16
@ms121
Copy link

ms121 commented Dec 6, 2017

@cmroche after 10 hours of running (bought a sold 5 times no problems) it decided to throw this out;

2017-12-06 02:02:12 (DEBUG): [binance.js] entering "setOrder" callback after api call, err: { code: -1013, msg: 'Filter failure: MIN_NOTIONAL' } data: { code: -1013, msg: 'Filter failure: MIN_NOTIONAL' }
It was trying to use under 1 BNB to ETH.

Looking at line 492 It looks to be correctly set to 1 not sure why it decided to try and buy. I would also note that it did throw the;

/root/gekko/exchanges/binance.js:35
if (!error || !error.message.match(recoverableErrors)) {

@cmroche
Copy link
Contributor Author

cmroche commented Dec 6, 2017

@ms121 Did it terminate the process, or continue?

Either way, if it continued to buy, then there is probably a bug in the Binance API... unless the buy you saw was not related, and perhaps there are rounding errors bringing it just slightly under the minimal amount? I could throw a Math.max in there and see if it fixes it, would be a bit of a guess though unless you could catch the error and print out the amount it is trying to request.

@ms121
Copy link

ms121 commented Dec 6, 2017

Ended up terminating the process. It might have actually been a rounding issue. I believe it might have round up thinking it was over 1 but was in fact just under.

Oh and I've found when it does round up at times it misses the buy because it is like 0.003 higher than the order price (not a big issue though).

@cmroche
Copy link
Contributor Author

cmroche commented Dec 6, 2017

Refer to PR #1427 now please.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants