-
Notifications
You must be signed in to change notification settings - Fork 38.7k
Simplify network-adjusted time warning logic #29623
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
038fd97
55361a1
ee178df
7d9c3ec
92e72b5
c6be144
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,6 +23,7 @@ | |
| #include <netbase.h> | ||
| #include <netmessagemaker.h> | ||
| #include <node/blockstorage.h> | ||
| #include <node/timeoffsets.h> | ||
| #include <node/txreconciliation.h> | ||
| #include <policy/fees.h> | ||
| #include <policy/policy.h> | ||
|
|
@@ -34,7 +35,6 @@ | |
| #include <scheduler.h> | ||
| #include <streams.h> | ||
| #include <sync.h> | ||
| #include <timedata.h> | ||
| #include <tinyformat.h> | ||
| #include <txmempool.h> | ||
| #include <txorphanage.h> | ||
|
|
@@ -390,6 +390,10 @@ struct Peer { | |
| /** Whether this peer wants invs or headers (when possible) for block announcements */ | ||
| bool m_prefers_headers GUARDED_BY(NetEventsInterface::g_msgproc_mutex){false}; | ||
|
|
||
| /** Time offset computed during the version handshake based on the | ||
| * timestamp the peer sent in the version message. */ | ||
| std::atomic<std::chrono::seconds> m_time_offset{0s}; | ||
|
|
||
| explicit Peer(NodeId id, ServiceFlags our_services) | ||
| : m_id{id} | ||
| , m_our_services{our_services} | ||
|
|
@@ -513,7 +517,7 @@ class PeerManagerImpl final : public PeerManager | |
| std::optional<std::string> FetchBlock(NodeId peer_id, const CBlockIndex& block_index) override | ||
| EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); | ||
| bool GetNodeStateStats(NodeId nodeid, CNodeStateStats& stats) const override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); | ||
| bool IgnoresIncomingTxs() override { return m_opts.ignore_incoming_txs; } | ||
| PeerManagerInfo GetInfo() const override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); | ||
| void SendPings() override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); | ||
| void RelayTransaction(const uint256& txid, const uint256& wtxid) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); | ||
| void SetBestBlock(int height, std::chrono::seconds time) override | ||
|
|
@@ -749,6 +753,8 @@ class PeerManagerImpl final : public PeerManager | |
| /** Next time to check for stale tip */ | ||
| std::chrono::seconds m_stale_tip_check_time GUARDED_BY(cs_main){0s}; | ||
|
|
||
| TimeOffsets m_outbound_time_offsets; | ||
stickies-v marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| const Options m_opts; | ||
|
|
||
| bool RejectIncomingTxs(const CNode& peer) const; | ||
|
|
@@ -1792,10 +1798,19 @@ bool PeerManagerImpl::GetNodeStateStats(NodeId nodeid, CNodeStateStats& stats) c | |
| stats.presync_height = peer->m_headers_sync->GetPresyncHeight(); | ||
| } | ||
| } | ||
| stats.time_offset = peer->m_time_offset; | ||
|
|
||
| return true; | ||
| } | ||
|
|
||
| PeerManagerInfo PeerManagerImpl::GetInfo() const | ||
| { | ||
| return PeerManagerInfo{ | ||
| .median_outbound_time_offset = m_outbound_time_offsets.Median(), | ||
| .ignores_incoming_txs = m_opts.ignore_incoming_txs, | ||
| }; | ||
| } | ||
|
|
||
| void PeerManagerImpl::AddToCompactExtraTransactions(const CTransactionRef& tx) | ||
| { | ||
| if (m_opts.max_extra_txs <= 0) | ||
|
|
@@ -3666,12 +3681,12 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, | |
| peer->m_starting_height, addrMe.ToStringAddrPort(), fRelay, pfrom.GetId(), | ||
| remoteAddr, (mapped_as ? strprintf(", mapped_as=%d", mapped_as) : "")); | ||
|
|
||
| int64_t nTimeOffset = nTime - GetTime(); | ||
| pfrom.nTimeOffset = nTimeOffset; | ||
| peer->m_time_offset = NodeSeconds{std::chrono::seconds{nTime}} - Now<NodeSeconds>(); | ||
| if (!pfrom.IsInboundConn()) { | ||
| // Don't use timedata samples from inbound peers to make it | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. After 2ef71c7,
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. updated to
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think this was covered (or it may have been regressed)
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry, I think it must have regressed with this update in the next line - will fix if I have to retouch. |
||
| // harder for others to tamper with our adjusted time. | ||
| AddTimeData(pfrom.addr, nTimeOffset); | ||
| // harder for others to create false warnings about our clock being out of sync. | ||
| m_outbound_time_offsets.Add(peer->m_time_offset); | ||
| m_outbound_time_offsets.WarnIfOutOfSync(); | ||
| } | ||
|
|
||
| // If the peer is old enough to have the old alert system, send it the final alert. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. e6d7bcf doesn't look like
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it is for
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I removed it and this seems to run fine, at least in my system
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We |
||
|
|
||
| void TimeOffsets::Add(std::chrono::seconds offset) | ||
stickies-v marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| { | ||
| 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 | ||
stickies-v marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| if (m_offsets.size() < 5) return 0s; | ||
|
|
||
| auto sorted_copy = m_offsets; | ||
stickies-v marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| 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; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 |
Uh oh!
There was an error while loading. Please reload this page.