Fixed issue #4521: AddTimeData will never update nTimeOffset past 199 sa… #4526

pull jprupp wants to merge 1 commits into bitcoin:master from jprupp:master changing 1 files +1 −1

jprupp commented at 2:08 pm on July 14, 2014: none

…mples
Fixed issue #4521: AddTimeData will never update nTimeOffset past 199 samples eeeaffbdae
BitcoinPullTester commented at 2:31 pm on July 14, 2014: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/p4526_eeeaffbdae9b60864a51f1622c9afcd5828301cf/ for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.
laanwj commented at 2:42 pm on July 14, 2014: member

Untested ACK.
laanwj commented at 2:56 pm on July 14, 2014: member

As this bug has remained undetected for so long I’m wondering whether we need the adjusted time functionality at all.

It makes sense to detect problems with the system time and warn, sure. But fixing them may be beyond the scope of bitcoin.

Also this median filter that is updated when a node connects is the same idea as the starting height, which has proven to be unreliable.
gmaxwell commented at 2:01 am on July 16, 2014: contributor

I am in favor of keeping the functionality, the limits on the range prevent some nonsense… but in this case the ‘bug’ is protective against some attacks (and may actually explain why we’ve never seen the one I’m thinking of exploited, which had surprised me…).

So I think we should hold off on fixing this and just clean it up as a result of some timing cleanup the strengthens it in a number of other ways. I should have some time to work on this soon.
laanwj commented at 8:47 am on July 16, 2014: member

OK - let’s add a comment to the code for now that this behaviour is intended, to prevent that more people think they found and fixed an issue.
laanwj closed this on Jul 24, 2014
laanwj commented at 5:50 pm on July 24, 2014: member

See 9365937
MarcoFalke locked this on Sep 8, 2021

Contributors
jprupp BitcoinPullTester laanwj gmaxwell