GlcoinCore/elements
1
0
Fork 0
mirror of https://github.com/ElementsProject/elements.git synced 2026-08-13 12:33:42 +02:00
Code Issues Projects Releases Packages Wiki Activity Actions 1

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

Browse source 
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](https://github.com/bitcoin/bitcoin/commits/1d226ae1f984c5c808f5c24c431b959cdefa692e/) 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](https://github.com/bitcoin/bitcoin/pull/29623/commits/c6be144c4b774a03a8bcab5a165768cf81e9b06b)
  achow101:
    reACK c6be144c4b
  dergoegge:
    utACK c6be144c4b

Tree-SHA512: 1063d639542e882186cdcea67d225ad1f97847f44253621a8c4b36c4d777e8f5cb0efe86bc279f01e819d33056ae4364c3300cc7400c087fb16c3f39b3e16b96
This commit is contained in:
Ava Chow 2024-04-30 18:41:45 -04:00
commit 0c3a3c9394
No known key found for this signature in database
GPG key ID: 17565732E08E5E41
25 changed files with 268 additions and 368 deletions
Download patch file Download diff file Expand all files Collapse all files

4
src/Makefile.am
View file

@ -230,6 +230,7 @@ BITCOIN_CORE_H = \
node/peerman_args.h \
node/protocol_version.h \
node/psbt.h \
node/timeoffsets.h \
node/transaction.h \
node/txreconciliation.h \
node/utxo_snapshot.h \
@ -280,7 +281,6 @@ BITCOIN_CORE_H = \
support/lockedpool.h \
sync.h \
threadsafety.h \
timedata.h \
torcontrol.h \
txdb.h \
txmempool.h \
@ -434,6 +434,7 @@ libbitcoin_node_a_SOURCES = \
node/minisketchwrapper.cpp \
node/peerman_args.cpp \
node/psbt.cpp \
node/timeoffsets.cpp \
node/transaction.cpp \
node/txreconciliation.cpp \
node/utxo_snapshot.cpp \
@ -461,7 +462,6 @@ libbitcoin_node_a_SOURCES = \
rpc/txoutproof.cpp \
script/sigcache.cpp \
signet.cpp \
timedata.cpp \
torcontrol.cpp \
txdb.cpp \
txmempool.cpp \

4
src/Makefile.test.include
View file

@ -153,7 +153,7 @@ BITCOIN_TESTS =\
test/streams_tests.cpp \
test/sync_tests.cpp \
test/system_tests.cpp \
test/timedata_tests.cpp \
test/timeoffsets_tests.cpp \
test/torcontrol_tests.cpp \
test/transaction_tests.cpp \
test/translation_tests.cpp \
@ -384,7 +384,7 @@ test_fuzz_fuzz_SOURCES = \
test/fuzz/string.cpp \
test/fuzz/strprintf.cpp \
test/fuzz/system.cpp \
test/fuzz/timedata.cpp \
test/fuzz/timeoffsets.cpp \
test/fuzz/torcontrol.cpp \
test/fuzz/transaction.cpp \
test/fuzz/tx_in.cpp \

1
src/addrman_impl.h
View file

@ -11,7 +11,6 @@
#include <protocol.h>
#include <serialize.h>
#include <sync.h>
#include <timedata.h>
#include <uint256.h>
#include <util/time.h>

2
src/init.cpp
View file

@ -67,7 +67,6 @@
#include <scheduler.h>
#include <script/sigcache.h>
#include <sync.h>
#include <timedata.h>
#include <torcontrol.h>
#include <txdb.h>
#include <txmempool.h>
@ -523,7 +522,6 @@ void SetupServerArgs(ArgsManager& argsman)
argsman.AddArg("-maxconnections=<n>", strprintf("Maintain at most <n> automatic connections to peers (default: %u). This limit does not apply to connections manually added via -addnode or the addnode RPC, which have a separate limit of %u.", DEFAULT_MAX_PEER_CONNECTIONS, MAX_ADDNODE_CONNECTIONS), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
argsman.AddArg("-maxreceivebuffer=<n>", strprintf("Maximum per-connection receive buffer, <n>*1000 bytes (default: %u)", DEFAULT_MAXRECEIVEBUFFER), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
argsman.AddArg("-maxsendbuffer=<n>", strprintf("Maximum per-connection memory usage for the send buffer, <n>*1000 bytes (default: %u)", DEFAULT_MAXSENDBUFFER), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
argsman.AddArg("-maxtimeadjustment", strprintf("Maximum allowed median peer time offset adjustment. Local perspective of time may be influenced by outbound peers forward or backward by this amount (default: %u seconds).", DEFAULT_MAX_TIME_ADJUSTMENT), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
argsman.AddArg("-maxuploadtarget=<n>", strprintf("Tries to keep outbound traffic under the given target per 24h. Limit does not apply to peers with 'download' permission or blocks created within past week. 0 = no limit (default: %s). Optional suffix units [k|K|m|M|g|G|t|T] (default: M). Lowercase is 1000 base while uppercase is 1024 base", DEFAULT_MAX_UPLOAD_TARGET), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
#if HAVE_SOCKADDR_UN
argsman.AddArg("-onion=<ip:port|path>", "Use separate SOCKS5 proxy to reach peers via Tor onion services, set -noonion to disable (default: -proxy). May be a local file path prefixed with 'unix:'.", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);

1
src/net.cpp
View file

@ -615,7 +615,6 @@ void CNode::CopyStats(CNodeStats& stats)
X(m_last_tx_time);
X(m_last_block_time);
X(m_connected);
X(nTimeOffset);
X(m_addr_name);
X(nVersion);
{

2
src/net.h
View file

@ -191,7 +191,6 @@ public:
std::chrono::seconds m_last_tx_time;
std::chrono::seconds m_last_block_time;
std::chrono::seconds m_connected;
int64_t nTimeOffset;
std::string m_addr_name;
int nVersion;
std::string cleanSubVer;
@ -703,7 +702,6 @@ public:
std::atomic<std::chrono::seconds> m_last_recv{0s};
//! Unix epoch time at peer connection
const std::chrono::seconds m_connected;
std::atomic<int64_t> nTimeOffset{0};
// Address of this peer
const CAddress addr;
// Bind address of our side of the connection

27
src/net_processing.cpp
View file

@ -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 @@ public:
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
@ -788,6 +792,8 @@ private:
/** 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;
const Options m_opts;
bool RejectIncomingTxs(const CNode& peer) const;
@ -1864,10 +1870,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)
@ -3861,12 +3876,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
// 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.

12
src/net_processing.h
View file

@ -9,6 +9,8 @@
#include <net.h>
#include <validationinterface.h>
#include <chrono>
class AddrMan;
class CChainParams;
class CTxMemPool;
@ -41,6 +43,12 @@ struct CNodeStateStats {
bool m_addr_relay_enabled{false};
ServiceFlags their_services;
int64_t presync_height{-1};
std::chrono::seconds time_offset{0};
};
struct PeerManagerInfo {
std::chrono::seconds median_outbound_time_offset{0s};
bool ignores_incoming_txs{false};
};
class PeerManager : public CValidationInterface, public NetEventsInterface
@ -83,8 +91,8 @@ public:
/** Get statistics from node state */
virtual bool GetNodeStateStats(NodeId nodeid, CNodeStateStats& stats) const = 0;
/** Whether this node ignores txs received over p2p. */
virtual bool IgnoresIncomingTxs() = 0;
/** Get peer manager info. */
virtual PeerManagerInfo GetInfo() const = 0;
/** Relay transaction to all peers. */
virtual void RelayTransaction(const uint256& txid, const uint256& wtxid) = 0;

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

4
src/qt/guiutil.cpp
View file

@ -775,9 +775,9 @@ QString formatPingTime(std::chrono::microseconds ping_time)
QObject::tr("%1 ms").arg(QString::number((int)(count_microseconds(ping_time) / 1000), 10));
}
QString formatTimeOffset(int64_t nTimeOffset)
QString formatTimeOffset(int64_t time_offset)
{
return QObject::tr("%1 s").arg(QString::number((int)nTimeOffset, 10));
return QObject::tr("%1 s").arg(QString::number((int)time_offset, 10));
}
QString formatNiceTimeOffset(qint64 secs)

4
src/qt/guiutil.h
View file

@ -240,8 +240,8 @@ namespace GUIUtil
/** Format a CNodeStats.m_last_ping_time into a user-readable string or display N/A, if 0 */
QString formatPingTime(std::chrono::microseconds ping_time);
/** Format a CNodeCombinedStats.nTimeOffset into a user-readable string */
QString formatTimeOffset(int64_t nTimeOffset);
/** Format a CNodeStateStats.time_offset into a user-readable string */
QString formatTimeOffset(int64_t time_offset);
QString formatNiceTimeOffset(qint64 secs);

3
src/qt/rpcconsole.cpp
View file

@ -23,6 +23,7 @@
#include <rpc/server.h>
#include <util/strencodings.h>
#include <util/string.h>
#include <util/time.h>
#include <util/threadnames.h>
#include <univalue.h>
@ -1196,7 +1197,6 @@ void RPCConsole::updateDetailWidget()
ui->peerBytesRecv->setText(GUIUtil::formatBytes(stats->nodeStats.nRecvBytes));
ui->peerPingTime->setText(GUIUtil::formatPingTime(stats->nodeStats.m_last_ping_time));
ui->peerMinPing->setText(GUIUtil::formatPingTime(stats->nodeStats.m_min_ping_time));
ui->timeoffset->setText(GUIUtil::formatTimeOffset(stats->nodeStats.nTimeOffset));
if (stats->nodeStats.nVersion) {
ui->peerVersion->setText(QString::number(stats->nodeStats.nVersion));
}
@ -1228,6 +1228,7 @@ void RPCConsole::updateDetailWidget()
// This check fails for example if the lock was busy and
// nodeStateStats couldn't be fetched.
if (stats->fNodeStateStatsAvailable) {
ui->timeoffset->setText(GUIUtil::formatTimeOffset(Ticks<std::chrono::seconds>(stats->nodeStateStats.time_offset)));
ui->peerServices->setText(GUIUtil::formatServicesStr(stats->nodeStateStats.their_services));
// Sync height is init to -1
if (stats->nodeStateStats.nSyncHeight > -1) {

8
src/rpc/net.cpp
View file

@ -23,7 +23,6 @@
#include <rpc/server_util.h>
#include <rpc/util.h>
#include <sync.h>
#include <timedata.h>
#include <util/chaintype.h>
#include <util/strencodings.h>
#include <util/string.h>
@ -239,7 +238,7 @@ static RPCHelpMan getpeerinfo()
obj.pushKV("bytessent", stats.nSendBytes);
obj.pushKV("bytesrecv", stats.nRecvBytes);
obj.pushKV("conntime", count_seconds(stats.m_connected));
obj.pushKV("timeoffset", stats.nTimeOffset);
obj.pushKV("timeoffset", Ticks<std::chrono::seconds>(statestats.time_offset));
if (stats.m_last_ping_time > 0us) {
obj.pushKV("pingtime", Ticks<SecondsDouble>(stats.m_last_ping_time));
}
@ -679,9 +678,10 @@ static RPCHelpMan getnetworkinfo()
obj.pushKV("localservicesnames", GetServicesNames(services));
}
if (node.peerman) {
obj.pushKV("localrelay", !node.peerman->IgnoresIncomingTxs());
auto peerman_info{node.peerman->GetInfo()};
obj.pushKV("localrelay", !peerman_info.ignores_incoming_txs);
obj.pushKV("timeoffset", Ticks<std::chrono::seconds>(peerman_info.median_outbound_time_offset));
}
obj.pushKV("timeoffset", GetTimeOffset());
if (node.connman) {
obj.pushKV("networkactive", node.connman->GetNetworkActive());
obj.pushKV("connections", node.connman->GetNodeCount(ConnectionDirection::Both));

2
src/test/denialofservice_tests.cpp
View file

@ -16,7 +16,6 @@
#include <test/util/net.h>
#include <test/util/random.h>
#include <test/util/setup_common.h>
#include <timedata.h>
#include <util/string.h>
#include <util/time.h>
#include <validation.h>
@ -72,7 +71,6 @@ BOOST_AUTO_TEST_CASE(outbound_slow_chain_eviction)
/*local_services=*/ServiceFlags(NODE_NETWORK | NODE_WITNESS),
/*version=*/PROTOCOL_VERSION,
/*relay_txs=*/true);
TestOnlyResetTimeData();
// This test requires that we have a chain with non-zero work.
{

31
src/test/fuzz/timedata.cpp
View file

@ -1,31 +0,0 @@
// Copyright (c) 2020-2021 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 <test/fuzz/FuzzedDataProvider.h>
#include <test/fuzz/fuzz.h>
#include <test/fuzz/util.h>
#include <timedata.h>
#include <cstdint>
#include <string>
#include <vector>
FUZZ_TARGET(timedata)
{
FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size());
const unsigned int max_size = fuzzed_data_provider.ConsumeIntegralInRange<unsigned int>(0, 1000);
// A max_size of 0 implies no limit, so cap the max number of insertions to avoid timeouts
auto max_to_insert = fuzzed_data_provider.ConsumeIntegralInRange<int>(0, 4000);
// Divide by 2 to avoid signed integer overflow in .median()
const int64_t initial_value = fuzzed_data_provider.ConsumeIntegral<int64_t>() / 2;
CMedianFilter<int64_t> median_filter{max_size, initial_value};
while (fuzzed_data_provider.remaining_bytes() > 0 && --max_to_insert >= 0) {
(void)median_filter.median();
assert(median_filter.size() > 0);
assert(static_cast<size_t>(median_filter.size()) == median_filter.sorted().size());
assert(static_cast<unsigned int>(median_filter.size()) <= max_size || max_size == 0);
// Divide by 2 to avoid signed integer overflow in .median()
median_filter.input(fuzzed_data_provider.ConsumeIntegral<int64_t>() / 2);
}
}

28
src/test/fuzz/timeoffsets.cpp Normal file
View file

@ -0,0 +1,28 @@
// 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 <node/timeoffsets.h>
#include <test/fuzz/FuzzedDataProvider.h>
#include <test/fuzz/fuzz.h>
#include <test/util/setup_common.h>
#include <chrono>
#include <cstdint>
#include <functional>
void initialize_timeoffsets()
{
static const auto testing_setup = MakeNoLogFileContext<>(ChainType::MAIN);
}
FUZZ_TARGET(timeoffsets, .init = initialize_timeoffsets)
{
FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size());
TimeOffsets offsets{};
LIMITED_WHILE(fuzzed_data_provider.remaining_bytes() > 0, 4'000) {
(void)offsets.Median();
offsets.Add(std::chrono::seconds{fuzzed_data_provider.ConsumeIntegral<std::chrono::seconds::rep>()});
offsets.WarnIfOutOfSync();
}
}

5
src/test/net_tests.cpp
View file

@ -19,7 +19,6 @@
#include <test/util/random.h>
#include <test/util/setup_common.h>
#include <test/util/validation.h>
#include <timedata.h>
#include <util/strencodings.h>
#include <util/string.h>
#include <validation.h>
@ -902,10 +901,6 @@ BOOST_AUTO_TEST_CASE(initial_advertise_from_version_message)
chainman.ResetIbd();
m_node.args->ForceSetArg("-capturemessages", "0");
m_node.args->ForceSetArg("-bind", "");
// PeerManager::ProcessMessage() calls AddTimeData() which changes the internal state
// in timedata.cpp and later confuses the test "timedata_tests/addtimedata". Thus reset
// that state as it was before our test was run.
TestOnlyResetTimeData();
}

105
src/test/timedata_tests.cpp
View file

@ -1,105 +0,0 @@
// Copyright (c) 2011-2021 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 <netaddress.h>
#include <noui.h>
#include <test/util/logging.h>
#include <test/util/setup_common.h>
#include <timedata.h>
#include <util/string.h>
#include <util/translation.h>
#include <warnings.h>
#include <string>
#include <boost/test/unit_test.hpp>
BOOST_FIXTURE_TEST_SUITE(timedata_tests, BasicTestingSetup)
BOOST_AUTO_TEST_CASE(util_MedianFilter)
{
CMedianFilter<int> filter(5, 15);
BOOST_CHECK_EQUAL(filter.median(), 15);
filter.input(20); // [15 20]
BOOST_CHECK_EQUAL(filter.median(), 17);
filter.input(30); // [15 20 30]
BOOST_CHECK_EQUAL(filter.median(), 20);
filter.input(3); // [3 15 20 30]
BOOST_CHECK_EQUAL(filter.median(), 17);
filter.input(7); // [3 7 15 20 30]
BOOST_CHECK_EQUAL(filter.median(), 15);
filter.input(18); // [3 7 18 20 30]
BOOST_CHECK_EQUAL(filter.median(), 18);
filter.input(0); // [0 3 7 18 30]
BOOST_CHECK_EQUAL(filter.median(), 7);
}
static void MultiAddTimeData(int n, int64_t offset)
{
static int cnt = 0;
for (int i = 0; i < n; ++i) {
CNetAddr addr;
addr.SetInternal(ToString(++cnt));
AddTimeData(addr, offset);
}
}
BOOST_AUTO_TEST_CASE(addtimedata)
{
BOOST_CHECK_EQUAL(GetTimeOffset(), 0);
//Part 1: Add large offsets to test a warning message that our clock may be wrong.
MultiAddTimeData(3, DEFAULT_MAX_TIME_ADJUSTMENT + 1);
// Filter size is 1 + 3 = 4: It is always initialized with a single element (offset 0)
{
ASSERT_DEBUG_LOG("Please check that your computer's date and time are correct!");
MultiAddTimeData(1, DEFAULT_MAX_TIME_ADJUSTMENT + 1); //filter size 5
}
BOOST_CHECK(GetWarnings(true).original.find("clock is wrong") != std::string::npos);
// nTimeOffset is not changed if the median of offsets exceeds DEFAULT_MAX_TIME_ADJUSTMENT
BOOST_CHECK_EQUAL(GetTimeOffset(), 0);
// Part 2: Test positive and negative medians by adding more offsets
MultiAddTimeData(4, 100); // filter size 9
BOOST_CHECK_EQUAL(GetTimeOffset(), 100);
MultiAddTimeData(10, -100); //filter size 19
BOOST_CHECK_EQUAL(GetTimeOffset(), -100);
// Part 3: Test behaviour when filter has reached maximum number of offsets
const int MAX_SAMPLES = 200;
int nfill = (MAX_SAMPLES - 3 - 19) / 2; //89
MultiAddTimeData(nfill, 100);
MultiAddTimeData(nfill, -100); //filter size MAX_SAMPLES - 3
BOOST_CHECK_EQUAL(GetTimeOffset(), -100);
MultiAddTimeData(2, 100);
//filter size MAX_SAMPLES -1, median is the initial 0 offset
//since we added same number of positive/negative offsets
BOOST_CHECK_EQUAL(GetTimeOffset(), 0);
// After the number of offsets has reached MAX_SAMPLES -1 (=199), nTimeOffset will never change
// because it is only updated when the number of elements in the filter becomes odd. It was decided
// not to fix this because it prevents possible attacks. See the comment in AddTimeData() or issue #4521
// for a more detailed explanation.
MultiAddTimeData(2, 100); // filter median is 100 now, but nTimeOffset will not change
// We want this test to end with nTimeOffset==0, otherwise subsequent tests of the suite will fail.
BOOST_CHECK_EQUAL(GetTimeOffset(), 0);
TestOnlyResetTimeData();
}
BOOST_AUTO_TEST_SUITE_END()

69
src/test/timeoffsets_tests.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 <node/timeoffsets.h>
#include <test/util/setup_common.h>
#include <boost/test/unit_test.hpp>
#include <chrono>
#include <vector>
using namespace std::chrono_literals;
static void AddMulti(TimeOffsets& offsets, const std::vector<std::chrono::seconds>& to_add)
{
for (auto offset : to_add) {
offsets.Add(offset);
}
}
BOOST_FIXTURE_TEST_SUITE(timeoffsets_tests, BasicTestingSetup)
BOOST_AUTO_TEST_CASE(timeoffsets)
{
TimeOffsets offsets{};
BOOST_CHECK(offsets.Median() == 0s);
AddMulti(offsets, {{0s, -1s, -2s, -3s}});
// median should be zero for < 5 offsets
BOOST_CHECK(offsets.Median() == 0s);
offsets.Add(-4s);
// we now have 5 offsets: [-4, -3, -2, -1, 0]
BOOST_CHECK(offsets.Median() == -2s);
AddMulti(offsets, {4, 5s});
// we now have 9 offsets: [-4, -3, -2, -1, 0, 5, 5, 5, 5]
BOOST_CHECK(offsets.Median() == 0s);
AddMulti(offsets, {41, 10s});
// the TimeOffsets is now at capacity with 50 offsets, oldest offsets is discarded for any additional offset
BOOST_CHECK(offsets.Median() == 10s);
AddMulti(offsets, {25, 15s});
// we now have 25 offsets of 10s followed by 25 offsets of 15s
BOOST_CHECK(offsets.Median() == 15s);
}
static bool IsWarningRaised(const std::vector<std::chrono::seconds>& check_offsets)
{
TimeOffsets offsets{};
AddMulti(offsets, check_offsets);
return offsets.WarnIfOutOfSync();
}
BOOST_AUTO_TEST_CASE(timeoffsets_warning)
{
BOOST_CHECK(IsWarningRaised({{-60min, -40min, -30min, 0min, 10min}}));
BOOST_CHECK(IsWarningRaised({5, 11min}));
BOOST_CHECK(!IsWarningRaised({4, 60min}));
BOOST_CHECK(!IsWarningRaised({100, 3min}));
}
BOOST_AUTO_TEST_SUITE_END()

116
src/timedata.cpp
View file

@ -1,116 +0,0 @@
// Copyright (c) 2014-2022 The Bitcoin Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
#if defined(HAVE_CONFIG_H)
#include <config/bitcoin-config.h>
#endif
#include <timedata.h>
#include <common/args.h>
#include <logging.h>
#include <netaddress.h>
#include <node/interface_ui.h>
#include <sync.h>
#include <tinyformat.h>
#include <util/translation.h>
#include <warnings.h>
static GlobalMutex g_timeoffset_mutex;
static int64_t nTimeOffset GUARDED_BY(g_timeoffset_mutex) = 0;
/**
* "Never go to sea with two chronometers; take one or three."
* Our three time sources are:
* - System clock
* - Median of other nodes clocks
* - The user (asking the user to fix the system clock if the first two disagree)
*/
int64_t GetTimeOffset()
{
LOCK(g_timeoffset_mutex);
return nTimeOffset;
}
#define BITCOIN_TIMEDATA_MAX_SAMPLES 200
static std::set<CNetAddr> g_sources;
static CMedianFilter<int64_t> g_time_offsets{BITCOIN_TIMEDATA_MAX_SAMPLES, 0};
static bool g_warning_emitted;
void AddTimeData(const CNetAddr& ip, int64_t nOffsetSample)
{
LOCK(g_timeoffset_mutex);
// Ignore duplicates
if (g_sources.size() == BITCOIN_TIMEDATA_MAX_SAMPLES)
return;
if (!g_sources.insert(ip).second)
return;
// Add data
g_time_offsets.input(nOffsetSample);
LogPrint(BCLog::NET, "added time data, samples %d, offset %+d (%+d minutes)\n", g_time_offsets.size(), nOffsetSample, nOffsetSample / 60);
// There is a known issue here (see issue #4521):
//
// - The structure g_time_offsets contains up to 200 elements, after which
// any new element added to it will not increase its size, replacing the
// oldest element.
//
// - The condition to update nTimeOffset includes checking whether the
// number of elements in g_time_offsets is odd, which will never happen after
// there are 200 elements.
//
// But in this case the 'bug' is protective against some attacks, and may
// actually explain why we've never seen attacks which manipulate the
// clock offset.
//
// So we should hold off on fixing this and clean it up as part of
// a timing cleanup that strengthens it in a number of other ways.
//
if (g_time_offsets.size() >= 5 && g_time_offsets.size() % 2 == 1) {
int64_t nMedian = g_time_offsets.median();
std::vector<int64_t> vSorted = g_time_offsets.sorted();
// Only let other nodes change our time by so much
int64_t max_adjustment = std::max<int64_t>(0, gArgs.GetIntArg("-maxtimeadjustment", DEFAULT_MAX_TIME_ADJUSTMENT));
if (nMedian >= -max_adjustment && nMedian <= max_adjustment) {
nTimeOffset = nMedian;
} else {
nTimeOffset = 0;
if (!g_warning_emitted) {
// If nobody has a time different than ours but within 5 minutes of ours, give a warning
bool fMatch = false;
for (const int64_t nOffset : vSorted) {
if (nOffset != 0 && nOffset > -5 * 60 && nOffset < 5 * 60) fMatch = true;
}
if (!fMatch) {
g_warning_emitted = true;
bilingual_str strMessage = strprintf(_("Please check that your computer's date and time are correct! If your clock is wrong, %s will not work properly."), PACKAGE_NAME);
SetMiscWarning(strMessage);
uiInterface.ThreadSafeMessageBox(strMessage, "", CClientUIInterface::MSG_WARNING);
}
}
}
if (LogAcceptCategory(BCLog::NET, BCLog::Level::Debug)) {
std::string log_message{"time data samples: "};
for (const int64_t n : vSorted) {
log_message += strprintf("%+d ", n);
}
log_message += strprintf("| median offset = %+d (%+d minutes)", nTimeOffset, nTimeOffset / 60);
LogPrint(BCLog::NET, "%s\n", log_message);
}
}
}
void TestOnlyResetTimeData()
{
LOCK(g_timeoffset_mutex);
nTimeOffset = 0;
g_sources.clear();
g_time_offsets = CMedianFilter<int64_t>{BITCOIN_TIMEDATA_MAX_SAMPLES, 0};
g_warning_emitted = false;
}

82
src/timedata.h
View file

@ -1,82 +0,0 @@
// Copyright (c) 2014-2022 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_TIMEDATA_H
#define BITCOIN_TIMEDATA_H
#include <algorithm>
#include <cassert>
#include <cstdint>
#include <vector>
static const int64_t DEFAULT_MAX_TIME_ADJUSTMENT = 70 * 60;
class CNetAddr;
/**
* Median filter over a stream of values.
* Returns the median of the last N numbers
*/
template <typename T>
class CMedianFilter
{
private:
std::vector<T> vValues;
std::vector<T> vSorted;
unsigned int nSize;
public:
CMedianFilter(unsigned int _size, T initial_value) : nSize(_size)
{
vValues.reserve(_size);
vValues.push_back(initial_value);
vSorted = vValues;
}
void input(T value)
{
if (vValues.size() == nSize) {
vValues.erase(vValues.begin());
}
vValues.push_back(value);
vSorted.resize(vValues.size());
std::copy(vValues.begin(), vValues.end(), vSorted.begin());
std::sort(vSorted.begin(), vSorted.end());
}
T median() const
{
int vSortedSize = vSorted.size();
assert(vSortedSize > 0);
if (vSortedSize & 1) // Odd number of elements
{
return vSorted[vSortedSize / 2];
} else // Even number of elements
{
return (vSorted[vSortedSize / 2 - 1] + vSorted[vSortedSize / 2]) / 2;
}
}
int size() const
{
return vValues.size();
}
std::vector<T> sorted() const
{
return vSorted;
}
};
/** Functions to keep track of adjusted P2P time */
int64_t GetTimeOffset();
void AddTimeData(const CNetAddr& ip, int64_t nTime);
/**
* Reset the internal state of GetTimeOffset() and AddTimeData().
*/
void TestOnlyResetTimeData();
#endif // BITCOIN_TIMEDATA_H

11
src/warnings.cpp
View file

@ -14,11 +14,13 @@
#include <util/string.h>
#include <util/translation.h>
#include <optional>
#include <vector>
static GlobalMutex g_warnings_mutex;
static bilingual_str g_misc_warnings GUARDED_BY(g_warnings_mutex);
static bool fLargeWorkInvalidChainFound GUARDED_BY(g_warnings_mutex) = false;
static std::optional<bilingual_str> g_timeoffset_warning GUARDED_BY(g_warnings_mutex){};
void SetMiscWarning(const bilingual_str& warning)
{
@ -32,6 +34,11 @@ void SetfLargeWorkInvalidChainFound(bool flag)
fLargeWorkInvalidChainFound = flag;
}
void SetMedianTimeOffsetWarning(std::optional<bilingual_str> warning)
{
LOCK(g_warnings_mutex);
g_timeoffset_warning = warning;
}
bilingual_str GetWarnings(bool verbose)
{
bilingual_str warnings_concise;
@ -56,6 +63,10 @@ bilingual_str GetWarnings(bool verbose)
warnings_verbose.emplace_back(warnings_concise);
}
if (g_timeoffset_warning) {
warnings_verbose.emplace_back(g_timeoffset_warning.value());
}
if (verbose) {
return Join(warnings_verbose, Untranslated("<hr />"));
}

3
src/warnings.h
View file

@ -6,12 +6,15 @@
#ifndef BITCOIN_WARNINGS_H
#define BITCOIN_WARNINGS_H
#include <optional>
#include <string>
struct bilingual_str;
void SetMiscWarning(const bilingual_str& warning);
void SetfLargeWorkInvalidChainFound(bool flag);
/** Pass std::nullopt to disable the warning */
void SetMedianTimeOffsetWarning(std::optional<bilingual_str> warning);
/** Format a string that describes several potential problems detected by the core.
* @param[in] verbose bool
* - if true, get all warnings separated by <hr />

4
test/functional/feature_maxtipage.py
View file

@ -43,6 +43,10 @@ class MaxTipAgeTest(BitcoinTestFramework):
self.generate(node_miner, 1)
assert_equal(node_ibd.getblockchaininfo()['initialblockdownload'], False)
# reset time to system time so we don't have a time offset with the ibd node the next
# time we connect to it, ensuring TimeOffsets::WarnIfOutOfSync() doesn't output to stderr
node_miner.setmocktime(0)
def run_test(self):
self.log.info("Test IBD with maximum tip age of 24 hours (default).")
self.test_maxtipage(DEFAULT_MAX_TIP_AGE, set_parameter=False)