Skip to content

Commit

Permalink
merge bitcoin#23758: Use type-safe mockable time for peer connection …
Browse files Browse the repository at this point in the history 
…time
  • Loading branch information
@kwvg
kwvg committed Jun 12, 2024
1 parent 7beeae7 commit 6e6c944
Show file tree
Hide file tree
Showing 11 changed files with 115 additions and 108 deletions.
12 changes: 6 additions & 6 deletions src/bench/peer_eviction.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ static void EvictionProtection0Networks250Candidates(benchmark::Bench& bench)
bench,
250 /* num_candidates */,
[](NodeEvictionCandidate& c) {
c.nTimeConnected = c.id;
c.m_connected = std::chrono::seconds{c.id};
c.m_network = NET_IPV4;
});
}
Expand All @@ -54,7 +54,7 @@ static void EvictionProtection1Networks250Candidates(benchmark::Bench& bench)
bench,
250 /* num_candidates */,
[](NodeEvictionCandidate& c) {
c.nTimeConnected = c.id;
c.m_connected = std::chrono::seconds{c.id};
c.m_is_local = false;
if (c.id >= 130 && c.id < 240) { // 110 Tor
c.m_network = NET_ONION;
Expand All @@ -70,7 +70,7 @@ static void EvictionProtection2Networks250Candidates(benchmark::Bench& bench)
bench,
250 /* num_candidates */,
[](NodeEvictionCandidate& c) {
c.nTimeConnected = c.id;
c.m_connected = std::chrono::seconds{c.id};
c.m_is_local = false;
if (c.id >= 90 && c.id < 160) { // 70 Tor
c.m_network = NET_ONION;
Expand All @@ -88,7 +88,7 @@ static void EvictionProtection3Networks050Candidates(benchmark::Bench& bench)
bench,
50 /* num_candidates */,
[](NodeEvictionCandidate& c) {
c.nTimeConnected = c.id;
c.m_connected = std::chrono::seconds{c.id};
c.m_is_local = (c.id == 28 || c.id == 47); // 2 localhost
if (c.id >= 30 && c.id < 47) { // 17 I2P
c.m_network = NET_I2P;
Expand All @@ -106,7 +106,7 @@ static void EvictionProtection3Networks100Candidates(benchmark::Bench& bench)
bench,
100 /* num_candidates */,
[](NodeEvictionCandidate& c) {
c.nTimeConnected = c.id;
c.m_connected = std::chrono::seconds{c.id};
c.m_is_local = (c.id >= 55 && c.id < 60); // 5 localhost
if (c.id >= 70 && c.id < 80) { // 10 I2P
c.m_network = NET_I2P;
Expand All @@ -124,7 +124,7 @@ static void EvictionProtection3Networks250Candidates(benchmark::Bench& bench)
bench,
250 /* num_candidates */,
[](NodeEvictionCandidate& c) {
c.nTimeConnected = c.id;
c.m_connected = std::chrono::seconds{c.id};
c.m_is_local = (c.id >= 140 && c.id < 160); // 20 localhost
if (c.id >= 170 && c.id < 180) { // 10 I2P
c.m_network = NET_I2P;
Expand Down
4 changes: 2 additions & 2 deletions src/masternode/utils.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ void CMasternodeUtils::DoMaintenance(CConnman& connman, CDeterministicMNManager&
connman.ForEachNode(CConnman::AllNodes, [&](CNode* pnode) {
if (pnode->m_masternode_probe_connection) {
// we're not disconnecting masternode probes for at least PROBE_WAIT_INTERVAL seconds
if (GetTimeSeconds() - pnode->nTimeConnected < PROBE_WAIT_INTERVAL) return;
if (GetTime<std::chrono::seconds>() - pnode->m_connected < PROBE_WAIT_INTERVAL) return;
} else {
// we're only disconnecting m_masternode_connection connections
if (!pnode->m_masternode_connection) return;
Expand All @@ -67,7 +67,7 @@ void CMasternodeUtils::DoMaintenance(CConnman& connman, CDeterministicMNManager&
if (pnode->IsInboundConn()) {
return;
}
} else if (GetTimeSeconds() - pnode->nTimeConnected < 5) {
} else if (GetTime<std::chrono::seconds>() - pnode->m_connected < 5s) {
// non-verified, give it some time to verify itself
return;
} else if (pnode->qwatch) {
Expand Down
36 changes: 18 additions & 18 deletions src/net.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -704,9 +704,9 @@ void CNode::CopyStats(CNodeStats& stats)
stats.m_network = ConnectedThroughNetwork();
X(m_last_send);
X(m_last_recv);
X(nLastTXTime);
X(nLastBlockTime);
X(nTimeConnected);
X(m_last_tx_time);
X(m_last_block_time);
X(m_connected);
X(nTimeOffset);
X(m_addr_name);
X(nVersion);
Expand Down Expand Up @@ -972,7 +972,7 @@ static bool ReverseCompareNodeMinPingTime(const NodeEvictionCandidate& a, const

static bool ReverseCompareNodeTimeConnected(const NodeEvictionCandidate& a, const NodeEvictionCandidate& b)
{
return a.nTimeConnected > b.nTimeConnected;
return a.m_connected > b.m_connected;
}

static bool CompareNetGroupKeyed(const NodeEvictionCandidate &a, const NodeEvictionCandidate &b) {
Expand All @@ -982,27 +982,27 @@ static bool CompareNetGroupKeyed(const NodeEvictionCandidate &a, const NodeEvict
static bool CompareNodeBlockTime(const NodeEvictionCandidate &a, const NodeEvictionCandidate &b)
{
// There is a fall-through here because it is common for a node to have many peers which have not yet relayed a block.
if (a.nLastBlockTime != b.nLastBlockTime) return a.nLastBlockTime < b.nLastBlockTime;
if (a.m_last_block_time != b.m_last_block_time) return a.m_last_block_time < b.m_last_block_time;
if (a.fRelevantServices != b.fRelevantServices) return b.fRelevantServices;
return a.nTimeConnected > b.nTimeConnected;
return a.m_connected > b.m_connected;
}

static bool CompareNodeTXTime(const NodeEvictionCandidate &a, const NodeEvictionCandidate &b)
{
// There is a fall-through here because it is common for a node to have more than a few peers that have not yet relayed txn.
if (a.nLastTXTime != b.nLastTXTime) return a.nLastTXTime < b.nLastTXTime;
if (a.m_last_tx_time != b.m_last_tx_time) return a.m_last_tx_time < b.m_last_tx_time;
if (a.m_relay_txs != b.m_relay_txs) return b.m_relay_txs;
if (a.fBloomFilter != b.fBloomFilter) return a.fBloomFilter;
return a.nTimeConnected > b.nTimeConnected;
return a.m_connected > b.m_connected;
}

// Pick out the potential block-relay only peers, and sort them by last block time.
static bool CompareNodeBlockRelayOnlyTime(const NodeEvictionCandidate &a, const NodeEvictionCandidate &b)
{
if (a.m_relay_txs != b.m_relay_txs) return a.m_relay_txs;
if (a.nLastBlockTime != b.nLastBlockTime) return a.nLastBlockTime < b.nLastBlockTime;
if (a.m_last_block_time != b.m_last_block_time) return a.m_last_block_time < b.m_last_block_time;
if (a.fRelevantServices != b.fRelevantServices) return b.fRelevantServices;
return a.nTimeConnected > b.nTimeConnected;
return a.m_connected > b.m_connected;
}

/**
Expand All @@ -1021,7 +1021,7 @@ struct CompareNodeNetworkTime {
{
if (m_is_local && a.m_is_local != b.m_is_local) return b.m_is_local;
if ((a.m_network == m_network) != (b.m_network == m_network)) return b.m_network == m_network;
return a.nTimeConnected > b.nTimeConnected;
return a.m_connected > b.m_connected;
};
};

Expand Down Expand Up @@ -1148,12 +1148,12 @@ void ProtectEvictionCandidatesByRatio(std::vector<NodeEvictionCandidate>& evicti
// (vEvictionCandidates is already sorted by reverse connect time)
uint64_t naMostConnections;
unsigned int nMostConnections = 0;
int64_t nMostConnectionsTime = 0;
std::chrono::seconds nMostConnectionsTime{0};
std::map<uint64_t, std::vector<NodeEvictionCandidate> > mapNetGroupNodes;
for (const NodeEvictionCandidate &node : vEvictionCandidates) {
std::vector<NodeEvictionCandidate> &group = mapNetGroupNodes[node.nKeyedNetGroup];
group.push_back(node);
const int64_t grouptime = group[0].nTimeConnected;
const auto grouptime{group[0].m_connected};

if (group.size() > nMostConnections || (group.size() == nMostConnections && grouptime > nMostConnectionsTime)) {
nMostConnections = group.size();
Expand Down Expand Up @@ -1196,7 +1196,7 @@ bool CConnman::AttemptToEvictConnection()
// was accepted. This short time is meant for the VERSION/VERACK exchange and the possible MNAUTH that might
// follow when the incoming connection is from another masternode. When a message other than MNAUTH
// is received after VERSION/VERACK, the protection is lifted immediately.
bool isProtected = GetTimeSeconds() - node->nTimeConnected < INBOUND_EVICTION_PROTECTION_TIME;
bool isProtected = GetTime<std::chrono::seconds>() - node->m_connected < INBOUND_EVICTION_PROTECTION_TIME;
if (node->nTimeFirstMessageReceived != 0 && !node->fFirstMessageIsMNAUTH) {
isProtected = false;
}
Expand All @@ -1210,8 +1210,8 @@ bool CConnman::AttemptToEvictConnection()
}
}

NodeEvictionCandidate candidate = {node->GetId(), node->nTimeConnected, node->m_min_ping_time,
node->nLastBlockTime, node->nLastTXTime,
NodeEvictionCandidate candidate = {node->GetId(), node->m_connected, node->m_min_ping_time,
node->m_last_block_time, node->m_last_tx_time,
HasAllDesirableServiceFlags(node->nServices),
node->m_relays_txs.load(), node->m_bloom_filter_loaded.load(),
node->nKeyedNetGroup, node->m_prefer_evict, node->addr.IsLocal(),
Expand Down Expand Up @@ -1644,7 +1644,7 @@ void CConnman::CalculateNumConnectionsChangedStats()

bool CConnman::ShouldRunInactivityChecks(const CNode& node, std::chrono::seconds now) const
{
return std::chrono::seconds{node.nTimeConnected} + m_peer_connect_timeout < now;
return node.m_connected + m_peer_connect_timeout < now;
}

bool CConnman::InactivityCheck(const CNode& node) const
Expand Down Expand Up @@ -4092,7 +4092,7 @@ unsigned int CConnman::GetReceiveFloodSize() const { return nReceiveFloodSize; }

CNode::CNode(NodeId idIn, ServiceFlags nLocalServicesIn, std::shared_ptr<Sock> sock, const CAddress& addrIn, uint64_t nKeyedNetGroupIn, uint64_t nLocalHostNonceIn, const CAddress& addrBindIn, const std::string& addrNameIn, ConnectionType conn_type_in, bool inbound_onion, std::unique_ptr<i2p::sam::Session>&& i2p_sam_session)
: m_sock{sock},
nTimeConnected{GetTimeSeconds()},
m_connected{GetTime<std::chrono::seconds>()},
addr{addrIn},
addrBind{addrBindIn},
m_addr_name{addrNameIn.empty() ? addr.ToStringIPPort() : addrNameIn},
Expand Down
28 changes: 14 additions & 14 deletions src/net.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,8 @@ static const bool DEFAULT_WHITELISTFORCERELAY = false;

/** Time after which to disconnect, after waiting for a ping response (or inactivity). */
static constexpr std::chrono::minutes TIMEOUT_INTERVAL{20};
/** Time to wait since nTimeConnected before disconnecting a probe node. **/
static const int PROBE_WAIT_INTERVAL = 5;
/** Time to wait since m_connected before disconnecting a probe node. */
static const auto PROBE_WAIT_INTERVAL{5s};
/** Minimum time between warnings printed to log. */
static const int WARNING_INTERVAL = 10 * 60;
/** Run the feeler connection loop once every 2 minutes. **/
Expand All @@ -82,7 +82,7 @@ static const int MAX_OUTBOUND_FULL_RELAY_CONNECTIONS = 8;
/** Maximum number of addnode outgoing nodes */
static const int MAX_ADDNODE_CONNECTIONS = 8;
/** Eviction protection time for incoming connections */
static const int INBOUND_EVICTION_PROTECTION_TIME = 1;
static const auto INBOUND_EVICTION_PROTECTION_TIME{1s};
/** Maximum number of block-relay-only outgoing connections */
static const int MAX_BLOCK_RELAY_ONLY_CONNECTIONS = 2;
/** Maximum number of feeler connections */
Expand Down Expand Up @@ -276,9 +276,9 @@ class CNodeStats
ServiceFlags nServices;
std::chrono::seconds m_last_send;
std::chrono::seconds m_last_recv;
int64_t nLastTXTime;
int64_t nLastBlockTime;
int64_t nTimeConnected;
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;
Expand Down Expand Up @@ -472,8 +472,8 @@ class CNode

std::atomic<std::chrono::seconds> m_last_send{0s};
std::atomic<std::chrono::seconds> m_last_recv{0s};
//! Unix epoch time at peer connection, in seconds.
const int64_t nTimeConnected;
//! Unix epoch time at peer connection
const std::chrono::seconds m_connected;
std::atomic<int64_t> nTimeOffset{0};
std::atomic<int64_t> nLastWarningTime{0};
std::atomic<int64_t> nTimeFirstMessageReceived{0};
Expand Down Expand Up @@ -513,7 +513,7 @@ class CNode
std::atomic<bool> m_masternode_connection{false};
/**
* If 'true' this node will be disconnected after MNAUTH (outbound only) or
* after PROBE_WAIT_INTERVAL seconds since nTimeConnected
* after PROBE_WAIT_INTERVAL seconds since m_connected
*/
std::atomic<bool> m_masternode_probe_connection{false};
// If 'true', we identified it as an intra-quorum relay connection
Expand Down Expand Up @@ -615,13 +615,13 @@ class CNode
* preliminary validity checks and was saved to disk, even if we don't
* connect the block or it eventually fails connection. Used as an inbound
* peer eviction criterium in CConnman::AttemptToEvictConnection. */
std::atomic<int64_t> nLastBlockTime{0};
std::atomic<std::chrono::seconds> m_last_block_time{0s};

/** UNIX epoch time of the last transaction received from this peer that we
* had not yet seen (e.g. not already received from another peer) and that
* was accepted into our mempool. Used as an inbound peer eviction criterium
* in CConnman::AttemptToEvictConnection. */
std::atomic<int64_t> nLastTXTime{0};
std::atomic<std::chrono::seconds> m_last_tx_time{0s};

/** Last measured round-trip time. Used only for RPC/GUI stats/debugging.*/
std::atomic<std::chrono::microseconds> m_last_ping_time{0us};
Expand Down Expand Up @@ -1628,10 +1628,10 @@ extern std::function<void(const CAddress& addr,
struct NodeEvictionCandidate
{
NodeId id;
int64_t nTimeConnected;
std::chrono::seconds m_connected;
std::chrono::microseconds m_min_ping_time;
int64_t nLastBlockTime;
int64_t nLastTXTime;
std::chrono::seconds m_last_block_time;
std::chrono::seconds m_last_tx_time;
bool fRelevantServices;
bool m_relay_txs;
bool fBloomFilter;
Expand Down
Loading

0 comments on commit 6e6c944

Please sign in to comment.