Skip to content

Conversation

Kangarused
Copy link

Implemented fix for timestamps. Ensures that timestamps are generated in UTC format, this prevents issues where the new Date() command generates a timestamp based on the system timezone which can cause Binance to reject any and all calls.

balthazar and others added 3 commits November 19, 2017 13:15
… in UTC format, this prevents issues where the new Date() command generates a timestamp based on the system timezone which can cause Binance to reject any and all calls.
@coveralls
Copy link

coveralls commented Nov 23, 2017

Coverage Status

Coverage decreased (-22.9%) to 72.193% when pulling 760fd92 on Kangarused:master into 40c03ad on HyperCubeProject:master.

@Kangarused
Copy link
Author

Test coverage went down a bit because I didn't have time to work on the test cases.

I'm using this plugin in my electron app, the electron app runs on my desktop which causes an issue as the current implementation uses "new Date()" which gets the date/time based on the timezone of the current system.

This is causing a difference between the timestamp generated on my end and the timestamp generated on Binance server greater than 1000ms. The Binance server rejects any requests where the difference between timestamps is greater than 1000ms.

The fix I have implement will ensure that the UTC time is used regardless of the system timezone settings.

Repository owner deleted a comment from coveralls Nov 23, 2017
@balthazar
Copy link
Owner

Heyo! First off I'm using Date.now().getTime(), which is returning a date in UTC format and not new Date(). Are you saying that electron is not following this and returns a non-UTC timestamp? It seems very unlikely to me and I can't find anything about that, are you sure it's just that your system clock was behind?

I wouldn't call -27% a little, but I think that's due to Travis not exposing the secrets to PRs, so most likely a false-positive.

@Kangarused
Copy link
Author

Hey man, apologies, you are using Date.now() which is essentially calling new Date().getTime().

This issue is very strange, I know that the documentation says that getTime() always returns the time in UTC format and yet after 11pm (+9:30 GMT) my local time, my app starts to return the following error: {"code":-1021,"msg":"Timestamp for this request is not valid."}.

I ended up creating a small function last night to determine the difference between local time and time of the server. Last night I was getting consistently getting differences of around 1200ms, which was causing the request to be rejected by the binance server.

The first thing I checked last night was my system time, however it is syncing from the internet. I forced a resync from the internet anyway with no change in behaviour.

I've run the same script this morning 8:38am (+9:30GMT) and now I'm getting a difference of around 600ms which under the 1000ms threshold so its all working fine... So I am kind of at a loss here.

I'll be checking again after 11pm tonight to see if I'm going crazy or not. Otherwise I apologise for wasting your time haha.

@balthazar
Copy link
Owner

balthazar commented Nov 23, 2017

Haha that's okay, yeah I think that's mostly due to a time difference, and not actually the format of the timestamp being sent. I got this error too at some point, reloaded my timezone and it seemed to fix the issue.

It might also come from them, you never know. But if you're experiencing too many of these issues, you should use the recvWindow parameter. By default Binance rejects request that differs from more than 5000 ms, but you can change it using this special param.

@Kangarused
Copy link
Author

I did set the recvWindow to 20,000, but it made no difference, I wasn't having any network issues so the response was returning immediately, it was solely to do with the difference in timestamps.

It is strange that the change I implemented fixed my issue last night, when really it shouldn't be any different from calling Date.now()

I'll test it again tonight anyway because I feel like I'm going crazy now.

@balthazar
Copy link
Owner

Hahaha, just to confirm you should look at the payload that is sent and make sure the param is sent, maybe I'm not setting it correctly? I haven't done any check on this to be honest. Otherwise the issue relies elsewhere.

@Kangarused
Copy link
Author

Ok, so I have done some more testing. This is definitely an issue for me.

  1. I tried running the app untouched from this morning and it was unusable, every request was rejected with error -1021 even though it was working fine this morning.

  2. I decided to run a quick test to see what the difference between my local timestamp and the binance server timestamp was, it was over 1000ms difference every time. (see pic)
    5

  3. I decided to implement my code to see if it made a difference again, and everything started to work again. See pic, the difference in the timestamp generated by our two calls, mine is less accurate because it cuts out the ms factor.
    6

I should mention that I was still getting the error even with my 'fix'. However, with the 'fix' in place it went from a 0% success rate to about 75% success rate on requests.
7

So then I tried switching back to Date.now() and changing my system time to UTC timezone. This had no effect, which was expected seen as we are converting to UTC time anyway, but I wanted to rule this out.

So it almost seems like Binance is not generating timestamps accurate to the ms?

I'm really not sure how to overcome this issue, do you have any ideas?

@balthazar
Copy link
Owner

You tried the recvWindow thing and made sure it was correctly passed to the xhr call right? If you did I would say the best thing to try would be to ask in their telegram channel where they are pretty responsive. It looks like you did some advanced analysis on the problem, so that might be something they will look at.

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

Successfully merging this pull request may close these issues.

3 participants