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

Round the timestamp value down #15

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jakecraige
Copy link

Since in JavaScript timestamps are done in MS, this dividing by 1000
produces a number with a decimal. By flooring it, we produce
a consistent number that matches how other langauges generate
timestamps.

I encountered this issue when creating my own blockchain implementation in Ruby, and noticing the hash didn't match when I put in the same inputs. It turned out that since this was using decimals for the timestamp it didn't match. This resolves it.

Also, there are many tests failing on master right now, so I was unable to write one for this or make sure this didn't break more.

Since in JavaScript timestamps are done in MS, this dividing by 1000
produces a number with a decimal. By flooring it, we produce
a consistent number that matches how other langauges generate
timestamps.
@0xs34n
Copy link
Owner

0xs34n commented Sep 24, 2017

Won't the date object be different if we convert it back to a javascript date object?

@jakecraige
Copy link
Author

Technically yes, because we're dropping the milliseconds, but the date isn't presented anywhere showing milliseconds so I don't think it's an issue.

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.

2 participants