Motivation
This PR builds on top of PR #28956 to cleanup the network-adjusted time leftovers.
The main achievements of this PR are:
- Fix a historical bug 🐞 when computing the time offset based on the time reported when establishing P2P connections where only the first (ever) 199 outbound connections where taking into account.
- The limitation is removed in favor of having a rolling calculation.
- Warn the user if the difference between the local time and the median time reported by the peers is longer than 10 min (previously 70)
- Make the warning recurrent (instead of once) while the node keeps having a potentially wrong time (according to the median of the time reported by their peers). The warning is displayed as follows:
- Every time a new outbound connection is made (logs)
- Toggle on/off in the RPC when we go off/on sync
- Show in the GUI every time we go off-sync (but limited to once every 60 secs)
Thoughts/Questions
The PR has already gone through a few rounds of review, so most of the design decisions have already been discussed and reshaped when needed.
I think the PR makes sense, some of the commits are cherry-picked from #28956, before it was reshaped to exclude the warning part, so I already reviewed them there.
The main differences now are, as stated by the PR description, how times are sampled and when the warning is displayed:
- The 199 hard limit bug is removed, so now the median is computed taking into account any new outbound connections (with a rolling set of the last 50 connections)
- The warning is now displayed if we are off by more than 10 minutes, while before it was after the offset was bigger than 70 min
Furthermore, the way the warning is shown has also been improved. Before this PR, the warning would have been shown only once, but now it would be shown in the logs every time we go offsync or create a new outbound connection while offsync, plus in the GUI (at most once every 60 min), so the user is well aware of the issue.
Review
Simplify network-adjusted time warning logic by stickies-v · Pull Request #29623 · bitcoin/bitcoin