core(network-request): use ms instead of seconds #14567
Conversation
connorjclark
requested review from
brendankenny
and removed request for
a team
paulirish
left a comment
paulirish
left a comment
There was a problem hiding this comment.
nice!
so happy to be so much closer to seconds-free.
one Q below that's got me pensive.
| */ | ||
| get startTime() { | ||
| return this._record.startTime * 1000 * 1000; | ||
| return this._record.startTime * 1000; |
There was a problem hiding this comment.
seems kinda awkward that the lantern depgraph and crc are still in seconds.
wdyt about changing both of those in here, too?
There was a problem hiding this comment.
I mentioned this in the other PR, but I would like to keep lantern changes in a separate PR. I expect it to change a number of things in a trivial way (snapshots, accuracy) due to the limits of floating point precision. It should be a quick change I can follow up with when this lands.
There was a problem hiding this comment.
maybe add some comments in network-node/base-node/cpu-node and crc that these times are in seconds?
and/or a TODO?
cuz right now its kinda impossible to tell from the code.
4 participants
split out from #14311
Changes the network request (aka network records) start and end time properties to be in ms, instead of seconds. This should not impact any audit values -
network-requestswas already converted to msHowever
critical-request-chaintimings and startTime/endTime in lanternnodes remain as seconds. They'll be adjusted in a followup.