Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nonce value is not reliable #4

Closed
yeldarby opened this issue Apr 12, 2014 · 11 comments
Closed

Nonce value is not reliable #4

yeldarby opened this issue Apr 12, 2014 · 11 comments

Comments

@yeldarby
Copy link

In trying to test both locally and on a server I ran into an issue where coinbase would return 401 unauthrorized. I pinned this down to the requirement that nonce values are strictly higher than any nonce value submitted before.

Looks like this library is using process.hrtime() to generate the nonce. While this works Ok on a single machine it is not consistent from machine to machine.

@yeldarby
Copy link
Author

Suggested solution: use Date.now() instead; it's still a bit susceptible to servertime drift but should be Ok for most situations. I patched the library and it seems to be working Ok now both in my local and remote environments.

If you're using Ubuntu on EC2 make sure to install NTP

@mateodelnorte
Copy link
Owner

Care to submit a pull request and a test case?

@mateodelnorte
Copy link
Owner

So, the reason we moved to an hrtime() based approach was because Date.now() breaks down on a single machine where multiple requests happen faster than the precision of Date.now(): 6ee61e4

Have another suggestion that would work for both single machine and multi-machine cases?

@yeldarby
Copy link
Author

Possibly a multi-machine flag when you initialize it? For our use case we are doing low-frequency ops so millisecond time limits aren't a large issue.

You could also use an internal counter that increments after each api call. So something like Date.now()*1000+(counter++)%1000

I'm not sure why but date.now() was randomly failing with a 401 sometimes as well. Not sure if my change maybe didn't get one of the hrtime()s changed. 

We ended up giving each machine its own api key which works for now but will be a headache to automate if we want to, for instance, do autoscaling of our infrastructure in the future. 


Sent from Mailbox for iPhone

On Mon, Apr 14, 2014 at 8:51 AM, Matt Walters notifications@github.com
wrote:

So, the reason we moved to an hrtime() based approach was because Date.now() breaks down on a single machine where multiple requests happen faster than the precision of Date.now(): 6ee61e4

Have another suggestion that would work for both single machine and multi-machine cases?

Reply to this email directly or view it on GitHub:
#4 (comment)

@mateodelnorte
Copy link
Owner

Are you sure you won't still have the same problem, essentially that you have two machines using the same algorithm to provide increasingly incremented values over time - but one machine thinks the current time is earlier than another?

The point of the new method was to give higher resolution, making it harder to have multiple calls between ticks. It sounds to me like your second machine is just sending an outdated nonce because the first is ahead of it.

Perhaps the solution is to ensure your servers are time synced: https://help.ubuntu.com/10.04/serverguide/NTP.html

@yeldarby
Copy link
Author

Yeah that was a suggestion for combining the new strategy with your need for more than one call per millisecond. 

Date.now() works if the servers' time is synced to within less than the precision of the time between calls (for us that is multiple seconds since it is triggered by a hit on a we page)

Sent from Mailbox for iPhone

On Mon, Apr 14, 2014 at 9:04 AM, Matt Walters notifications@github.com
wrote:

Are you sure you won't still have the same problem, essentially that you have two machines using the same algorithm to provide increasingly incremented values over time - but one machine thinks the current time is earlier than another?
The point of the new method was to give higher resolution, making it harder to have multiple calls between ticks. It sounds to me like your second machine is just sending an outdated nonce because the first is ahead of it.

Perhaps the solution is to ensure your servers are time synced: https://help.ubuntu.com/10.04/serverguide/NTP.html

Reply to this email directly or view it on GitHub:
#4 (comment)

@mateodelnorte
Copy link
Owner

If you've got a branch you'd like me to look at, shoot me a link or make a PR.

@pwlmaciejewski
Copy link

@mateodelnorte I've stumbled upon an issue where two synced servers gave me two totally different values of process.hrtime. In docs you can read that it returns timestamp which is "relative to an arbitrary time in the past". My question is: are you sure this is the right usage of process.hrtime? For me it sounds like it should be used for benchmarking only.

@pwlmaciejewski
Copy link

BTW I've tested the @yeldarby's solution and it works fine for me. #5 👍

@mateodelnorte
Copy link
Owner

@fragphace, @yeldarby take a comments in #5. If the changes in https://github.com/mateodelnorte/coinbase/tree/feature/enhanced-nonce work for you, I'll bump version, push to master, and publish asap.

Thx.

@mateodelnorte
Copy link
Owner

Closing. Fixed by #7

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

No branches or pull requests

3 participants