Reduce systematic error of time offset measurements #6642

pull MarcoFalke wants to merge 3 commits into bitcoin:master from MarcoFalke:MarcoFalke-2015-timeOffsetFix changing 2 files +118 −117

MarcoFalke commented at 2:12 PM on September 6, 2015: member

Assuming all nodes perfectly synced their clocks, nTimeOffset ≤ 0. This PR's commits bring it closer to zero.

[init] Min. time between socket creation and AcceptConnection()

Incoming peers can OpenNetworkConnection(), thus PushVersion()
as soon as the server is done with init step 6. Consequently,
nTimeOffset can grow arbitrarily large. Executing init step 6
in the end mitigates this issue.

0ac42c49f7

[trivial] fix comments in init.cpp 719171cdb6

[net] Reduce systematic error of time offset measurements

GetTime() can only be larger than the received time; Possibly
between 200-20000 microseconds.

e12d0ad066

gmaxwell commented at 1:45 AM on September 7, 2015: contributor

Our time system has a resolution of one second and huge inherent systemic errors vs UTC (e.g. we do not handle leap seconds). In blocks, latencies in the mining process usually make their times tens of seconds off (and misconfigurations make them minutes off).

The network also has infinite gain, e.g. there is no inherent bias to drive it back in sync with UTC except for corner case limitations in the correction procedure.

Fortunately, we do not use the time in any ways in which precision is important. (which is also fortunate because making it precise and consistent without centralization is an unsolved problem!)

Given that, I am not sure that an offset of even as much as 20000 microseconds is at all interesting.

Are you aware of a case where this matters?
gmaxwell commented at 2:08 AM on September 7, 2015: contributor

I did a cursory check for step 6 side effects having any effect before the end and did not find any. Before accepting that change a more careful examination should be done in review. I think deferring binding until later in setup is likely reasonable regardless of the timing handling.
MarcoFalke commented at 8:10 AM on September 7, 2015: member

Indeed, e12d0ad is relevant only in less than 0.5%, correcting the insignificant one second offset on my machine.

To check if 0ac42c4 works, you could pick a server with a public IP address which some other nodes actively try to connect to. Then just restart the server (or even do something costly like reindex). In the meantime you can see the other nodes PushVersion() to your server. So nTimeOffset = whatever it took you to reindex, etc.
laanwj added the label P2P on Sep 8, 2015
laanwj commented at 8:13 AM on October 6, 2015: member

As it is not clear to me what practical issue this solves - the time sync logic is a bit dodgy in the first place, and as @gmaxwell already says absolute precision is not important for our use caes - I'm not willing to take the risk of breaking the initialization sequence, sorry.
laanwj closed this on Oct 6, 2015
MarcoFalke deleted the branch on Oct 7, 2015
MarcoFalke locked this on Sep 8, 2021

Contributors

Labels