-
Notifications
You must be signed in to change notification settings - Fork 996
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
make timestamp numeric #3902
make timestamp numeric #3902
Conversation
07a292d
to
2c27873
Compare
2c27873
to
08a743b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but since this is a Scalar value change, perhaps there's something missing, I would be more confident if @lutter takes a look too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good - two comments that should be addressed before merging, but other than that ok.
store/postgres/src/chain_store.rs
Outdated
|
||
let radix = if ts.starts_with("0x") { 16 } else { 10 }; | ||
|
||
u64::from_str_radix(&ts[2..], radix) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't right when the string is base 10 - there we don't want to skip the first two chars
"Timestamp of the block if available, format depends on the chain" | ||
timestamp: String | ||
"Timestamp of the block if available" | ||
timestamp: Int |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment should also say something about how to interpret that number - maybe add Integer representation of the timestamp stored in blocks for the chain
to tell people that we don't do any other processing
6ee6a71
to
beaaf61
Compare
beaaf61
to
ea7fb2b
Compare
* make timestamp numeric
No description provided.