Merge bitcoin/bitcoin#29623: Simplify network-adjusted time warning logic

c6be144c4b Remove timedata (stickies-v)
92e72b5d0d [net processing] Move IgnoresIncomingTxs to PeerManagerInfo (dergoegge)
7d9c3ec622 [net processing] Introduce PeerManagerInfo (dergoegge)
ee178dfcc1 Add TimeOffsets helper class (stickies-v)
55361a15d1 [net processing] Use std::chrono for type-safe time offsets (stickies-v)
038fd979ef [net processing] Move nTimeOffset to net_processing (dergoegge)

Pull request description:

  [An earlier approach](1d226ae1f9/) in #28956 involved simplifying and refactoring the network-adjusted time calculation logic, but this was eventually [left out](https://github.com/bitcoin/bitcoin/pull/28956#issuecomment-1904214370) of the PR to make it easier for reviewers to focus on consensus logic changes.

  Since network-adjusted time is now only used for warning/informational purposes, cleaning up the logic (building on @dergoegge's approach in #28956) should be quite straightforward and uncontroversial. The main changes are:

  - Previously, we would only calculate the time offset from the first 199 outbound peers that we connected to. This limitation is now removed, and we have a proper rolling calculation. I've reduced the set to 50 outbound peers, which seems plenty.
  - Previously, we would automatically use the network-adjusted time if the difference was < 70 mins, and warn the user if the difference was larger than that. Since there is no longer any automated time adjustment, I've changed the warning threshold to ~~20~~ 10 minutes (which is an arbitrary number).
  - Previously, a warning would only be raised once, and then never again until node restart. This behaviour is now updated to  1) warn to log for every new outbound peer for as long as we appear out of sync, 2) have the RPC warning toggled on/off whenever we go in/out of sync, and 3) have the GUI warn whenever we are out of sync (again), but limited to 1 messagebox per 60 minutes
  - no more globals
  - remove the `-maxtimeadjustment` startup arg

  Closes #4521

ACKs for top commit:
  sr-gi:
    Re-ACK [c6be144](c6be144c4b)
  achow101:
    reACK c6be144c4b
  dergoegge:
    utACK c6be144c4b

Tree-SHA512: 1063d639542e882186cdcea67d225ad1f97847f44253621a8c4b36c4d777e8f5cb0efe86bc279f01e819d33056ae4364c3300cc7400c087fb16c3f39b3e16b96
This commit is contained in:
Ava Chow
2024-04-30 18:41:45 -04:00
25 changed files with 268 additions and 368 deletions

69
src/node/timeoffsets.cpp Normal file
View File

@@ -0,0 +1,69 @@
// Copyright (c) 2024-present The Bitcoin Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
#include <logging.h>
#include <node/interface_ui.h>
#include <node/timeoffsets.h>
#include <sync.h>
#include <tinyformat.h>
#include <util/time.h>
#include <util/translation.h>
#include <warnings.h>
#include <algorithm>
#include <chrono>
#include <cstdint>
#include <deque>
#include <limits>
#include <optional>
using namespace std::chrono_literals;
void TimeOffsets::Add(std::chrono::seconds offset)
{
LOCK(m_mutex);
if (m_offsets.size() >= MAX_SIZE) {
m_offsets.pop_front();
}
m_offsets.push_back(offset);
LogDebug(BCLog::NET, "Added time offset %+ds, total samples %d\n",
Ticks<std::chrono::seconds>(offset), m_offsets.size());
}
std::chrono::seconds TimeOffsets::Median() const
{
LOCK(m_mutex);
// Only calculate the median if we have 5 or more offsets
if (m_offsets.size() < 5) return 0s;
auto sorted_copy = m_offsets;
std::sort(sorted_copy.begin(), sorted_copy.end());
return sorted_copy[sorted_copy.size() / 2]; // approximate median is good enough, keep it simple
}
bool TimeOffsets::WarnIfOutOfSync() const
{
// when median == std::numeric_limits<int64_t>::min(), calling std::chrono::abs is UB
auto median{std::max(Median(), std::chrono::seconds(std::numeric_limits<int64_t>::min() + 1))};
if (std::chrono::abs(median) <= WARN_THRESHOLD) {
SetMedianTimeOffsetWarning(std::nullopt);
uiInterface.NotifyAlertChanged();
return false;
}
bilingual_str msg{strprintf(_(
"Your computer's date and time appear to be more than %d minutes out of sync with the network, "
"this may lead to consensus failure. After you've confirmed your computer's clock, this message "
"should no longer appear when you restart your node. Without a restart, it should stop showing "
"automatically after you've connected to a sufficient number of new outbound peers, which may "
"take some time. You can inspect the `timeoffset` field of the `getpeerinfo` and `getnetworkinfo` "
"RPC methods to get more info."
), Ticks<std::chrono::minutes>(WARN_THRESHOLD))};
LogWarning("%s\n", msg.original);
SetMedianTimeOffsetWarning(msg);
uiInterface.NotifyAlertChanged();
return true;
}

39
src/node/timeoffsets.h Normal file
View File

@@ -0,0 +1,39 @@
// Copyright (c) 2024-present The Bitcoin Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
#ifndef BITCOIN_NODE_TIMEOFFSETS_H
#define BITCOIN_NODE_TIMEOFFSETS_H
#include <sync.h>
#include <chrono>
#include <cstddef>
#include <deque>
class TimeOffsets
{
//! Maximum number of timeoffsets stored.
static constexpr size_t MAX_SIZE{50};
//! Minimum difference between system and network time for a warning to be raised.
static constexpr std::chrono::minutes WARN_THRESHOLD{10};
mutable Mutex m_mutex;
/** The observed time differences between our local clock and those of our outbound peers. A
* positive offset means our peer's clock is ahead of our local clock. */
std::deque<std::chrono::seconds> m_offsets GUARDED_BY(m_mutex){};
public:
/** Add a new time offset sample. */
void Add(std::chrono::seconds offset) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex);
/** Compute and return the median of the collected time offset samples. The median is returned
* as 0 when there are less than 5 samples. */
std::chrono::seconds Median() const EXCLUSIVE_LOCKS_REQUIRED(!m_mutex);
/** Raise warnings if the median time offset exceeds the warnings threshold. Returns true if
* warnings were raised. */
bool WarnIfOutOfSync() const EXCLUSIVE_LOCKS_REQUIRED(!m_mutex);
};
#endif // BITCOIN_NODE_TIMEOFFSETS_H