semashkinvg / Bitmex.NET

Wrapper for BitMEX.com REST & WebSocket API
MIT License
52 stars 26 forks source link

Replace NonceProvider with ExpiresTimeProvider #8

Closed tekr closed 6 years ago

tekr commented 6 years ago

Resolves issue #7 by setting expires-time to client time + 30 seconds (allowing for a reasonable offset between client & server time)

semashkinvg commented 6 years ago

hi @tekr, Wow first pull request in the repository! much appreciated! Thank you! I would like to accept it but, i'm not quite sure what's the point of adding a new provider? why don't you change the existing one?

var yearBegin = new DateTime(1970, 1, 1, 0, 0, 0, DateTimeKind.Utc); var newNonce = Convert.ToInt64((DateTime.UtcNow - yearBegin).TotalSeconds) + 30;

if you add this code into your pull request I'll accept it for sure and get rid of the new provider I will accept it.

Thanks,

tekr commented 6 years ago

Hi @semashkinvg, The main reason for renaming the provider is that it's no longer providing a nonce (i.e. an arbitrary number that is only used once); rather it is providing an expiry time for the request, which could be duplicated for multiple requests sent at approximately the same time.

semashkinvg commented 6 years ago

@tekr, I've just merged your change and tested it a little bit. Tbh, I thought that we have to send a new nonce for all the requests and if it's the same or less it fails, I'm surprised that it works. Brilliant, great job!

tekr commented 6 years ago

Yep it looks like Bitmex no longer requires or even recommends the nonce approach. I think it's a good move, because multiple requests to Bitmex sent at about the same time would occasionally be rejected as they could arrive at the exchange in a different order than they were sent in (and hence the nonce would not be increasing).

Thanks for the awesome library!