Skip to content

Commit

Permalink
partial bitcoin#20196: fix GetListenPort() to derive the proper port
Browse files Browse the repository at this point in the history
excludes:
- 0cfc0cd
- 7d64ea4
  • Loading branch information
kwvg committed Apr 26, 2024
1 parent 7f72009 commit 24205d9
Show file tree
Hide file tree
Showing 6 changed files with 55 additions and 18 deletions.
12 changes: 11 additions & 1 deletion src/net.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4268,7 +4268,11 @@ void CConnman::UnregisterEvents(CNode *pnode)
}
#endif
}
void CaptureMessage(const CAddress& addr, const std::string& msg_type, const Span<const unsigned char>& data, bool is_incoming)

void CaptureMessageToFile(const CAddress& addr,
const std::string& msg_type,
Span<const unsigned char> data,
bool is_incoming)
{
// Note: This function captures the message at the time of processing,
// not at socket receive/send time.
Expand All @@ -4295,3 +4299,9 @@ void CaptureMessage(const CAddress& addr, const std::string& msg_type, const Spa
ser_writedata32(f, size);
f.write(AsBytes(data));
}

std::function<void(const CAddress& addr,
const std::string& msg_type,
Span<const unsigned char> data,
bool is_incoming)>
CaptureMessage = CaptureMessageToFile;
13 changes: 12 additions & 1 deletion src/net.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
#include <condition_variable>
#include <cstdint>
#include <deque>
#include <functional>
#include <map>
#include <memory>
#include <thread>
Expand Down Expand Up @@ -1528,7 +1529,17 @@ friend class CNode;
std::chrono::microseconds PoissonNextSend(std::chrono::microseconds now, std::chrono::seconds average_interval);

/** Dump binary message to file, with timestamp */
void CaptureMessage(const CAddress& addr, const std::string& msg_type, const Span<const unsigned char>& data, bool is_incoming);
void CaptureMessageToFile(const CAddress& addr,
const std::string& msg_type,
Span<const unsigned char> data,
bool is_incoming);

/** Defaults to `CaptureMessageToFile()`, but can be overridden by unit tests. */
extern std::function<void(const CAddress& addr,
const std::string& msg_type,
Span<const unsigned char> data,
bool is_incoming)>
CaptureMessage;

struct NodeEvictionCandidate
{
Expand Down
3 changes: 2 additions & 1 deletion src/test/timedata_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -96,9 +96,10 @@ BOOST_AUTO_TEST_CASE(addtimedata)
// 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);

// We want this test to end with nTimeOffset==0, otherwise subsequent tests of the suite will fail.
TestOnlyResetTimeData();
}

BOOST_AUTO_TEST_SUITE_END()
38 changes: 24 additions & 14 deletions src/timedata.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,29 +39,31 @@ int64_t GetAdjustedTime()

#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
static std::set<CNetAddr> setKnown;
if (setKnown.size() == BITCOIN_TIMEDATA_MAX_SAMPLES)
if (g_sources.size() == BITCOIN_TIMEDATA_MAX_SAMPLES)
return;
if (!setKnown.insert(ip).second)
if (!g_sources.insert(ip).second)
return;

// Add data
static CMedianFilter<int64_t> vTimeOffsets(BITCOIN_TIMEDATA_MAX_SAMPLES, 0);
vTimeOffsets.input(nOffsetSample);
LogPrint(BCLog::NET, "added time data, samples %d, offset %+d (%+d minutes)\n", vTimeOffsets.size(), nOffsetSample, nOffsetSample / 60);
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 vTimeOffsets contains up to 200 elements, after which
// - 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 vTimeOffsets is odd, which will never happen after
// 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
Expand All @@ -71,26 +73,25 @@ void AddTimeData(const CNetAddr& ip, int64_t nOffsetSample)
// 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 (vTimeOffsets.size() >= 5 && vTimeOffsets.size() % 2 == 1) {
int64_t nMedian = vTimeOffsets.median();
std::vector<int64_t> vSorted = vTimeOffsets.sorted();
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.GetArg("-maxtimeadjustment", DEFAULT_MAX_TIME_ADJUSTMENT));
if (nMedian >= -max_adjustment && nMedian <= max_adjustment) {
nTimeOffset = nMedian;
} else {
nTimeOffset = 0;

static bool fDone;
if (!fDone) {
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) {
fDone = true;
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);
Expand All @@ -108,3 +109,12 @@ void AddTimeData(const CNetAddr& ip, int64_t nOffsetSample)
}
}
}

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;
}
5 changes: 5 additions & 0 deletions src/timedata.h
Original file line number Diff line number Diff line change
Expand Up @@ -75,4 +75,9 @@ int64_t GetTimeOffset();
int64_t GetAdjustedTime();
void AddTimeData(const CNetAddr& ip, int64_t nTime);

/**
* Reset the internal state of GetTimeOffset(), GetAdjustedTime() and AddTimeData().
*/
void TestOnlyResetTimeData();

#endif // BITCOIN_TIMEDATA_H
2 changes: 1 addition & 1 deletion test/functional/p2p_message_capture.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
MSGTYPE_SIZE = 12

def mini_parser(dat_file):
"""Parse a data file created by CaptureMessage.
"""Parse a data file created by CaptureMessageToFile.
From the data file we'll only check the structure.
Expand Down

0 comments on commit 24205d9

Please sign in to comment.