From c02fa47baa517e17b5c43bde3902b1e410c1b93f Mon Sep 17 00:00:00 2001 From: John Newbery Date: Wed, 3 Mar 2021 08:46:55 +0000 Subject: [net processing] Only call GetTime() once in SendMessages() We currently call GetTime() 4 times in SendMessages(). Consolidate this to once GetTime() call. --- src/net_processing.cpp | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) (limited to 'src/net_processing.cpp') diff --git a/src/net_processing.cpp b/src/net_processing.cpp index e561f02c4a..9f9af0aa33 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -317,8 +317,10 @@ private: void PushNodeVersion(CNode& pnode, int64_t nTime); /** Send a ping message every PING_INTERVAL or if requested via RPC. May - * mark the peer to be disconnected if a ping has timed out. */ - void MaybeSendPing(CNode& node_to, Peer& peer); + * mark the peer to be disconnected if a ping has timed out. + * We use mockable time for ping timeouts, so setmocktime may cause pings + * to time out. */ + void MaybeSendPing(CNode& node_to, Peer& peer, std::chrono::microseconds now); const CChainParams& m_chainparams; CConnman& m_connman; @@ -4096,12 +4098,8 @@ void PeerManagerImpl::CheckForStaleTipAndEvictPeers() } } -void PeerManagerImpl::MaybeSendPing(CNode& node_to, Peer& peer) +void PeerManagerImpl::MaybeSendPing(CNode& node_to, Peer& peer, std::chrono::microseconds now) { - // Use mockable time for ping timeouts. - // This means that setmocktime may cause pings to time out. - auto now = GetTime(); - if (m_connman.RunInactivityChecks(node_to) && peer.m_ping_nonce_sent && now > peer.m_ping_start.load() + std::chrono::seconds{TIMEOUT_INTERVAL}) { LogPrint(BCLog::NET, "ping timeout: %fs peer=%d\n", 0.000001 * count_microseconds(now - peer.m_ping_start.load()), peer.m_id); @@ -4178,7 +4176,9 @@ bool PeerManagerImpl::SendMessages(CNode* pto) // If we get here, the outgoing message serialization version is set and can't change. const CNetMsgMaker msgMaker(pto->GetCommonVersion()); - MaybeSendPing(*pto, *peer); + const auto current_time = GetTime(); + + MaybeSendPing(*pto, *peer, current_time); // MaybeSendPing may have marked peer for disconnection if (pto->fDisconnect) return true; @@ -4189,7 +4189,6 @@ bool PeerManagerImpl::SendMessages(CNode* pto) CNodeState &state = *State(pto->GetId()); // Address refresh broadcast - auto current_time = GetTime(); if (fListen && pto->RelayAddrsWithConn() && !m_chainman.ActiveChainstate().IsInitialBlockDownload() && @@ -4485,7 +4484,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto) vInv.clear(); } } - pto->m_tx_relay->m_last_mempool_req = GetTime(); + pto->m_tx_relay->m_last_mempool_req = std::chrono::duration_cast(current_time); } // Determine transactions to relay @@ -4573,7 +4572,6 @@ bool PeerManagerImpl::SendMessages(CNode* pto) m_connman.PushMessage(pto, msgMaker.Make(NetMsgType::INV, vInv)); // Detect whether we're stalling - current_time = GetTime(); if (state.m_stalling_since.count() && state.m_stalling_since < current_time - BLOCK_STALLING_TIMEOUT) { // Stalling only triggers when the block download window cannot move. During normal steady state, // the download window should be much larger than the to-be-downloaded set of blocks, so disconnection -- cgit v1.2.3 From 4ad4abcf07efefafd439b28679dff8d6bbf62943 Mon Sep 17 00:00:00 2001 From: John Newbery Date: Mon, 29 Mar 2021 11:36:19 +0100 Subject: [net] Change addr send times fields to be guarded by new mutex --- src/net_processing.cpp | 3 +++ 1 file changed, 3 insertions(+) (limited to 'src/net_processing.cpp') diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 9f9af0aa33..359b7d9843 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -4189,6 +4189,8 @@ bool PeerManagerImpl::SendMessages(CNode* pto) CNodeState &state = *State(pto->GetId()); // Address refresh broadcast + { + LOCK(pto->m_addr_send_times_mutex); if (fListen && pto->RelayAddrsWithConn() && !m_chainman.ActiveChainstate().IsInitialBlockDownload() && @@ -4249,6 +4251,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto) if (pto->vAddrToSend.capacity() > 40) pto->vAddrToSend.shrink_to_fit(); } + } // pto->m_addr_send_times_mutex // Start block sync if (pindexBestHeader == nullptr) -- cgit v1.2.3 From ad719297f2ecdd2394eff668b3be7070bc9cb3e2 Mon Sep 17 00:00:00 2001 From: John Newbery Date: Thu, 9 Jul 2020 10:51:20 +0100 Subject: [net processing] Extract `addr` send functionality into MaybeSendAddr() Reviewer hint: review with `git diff --color-moved=dimmed-zebra --ignore-all-space` --- src/net_processing.cpp | 136 ++++++++++++++++++++++++++----------------------- 1 file changed, 71 insertions(+), 65 deletions(-) (limited to 'src/net_processing.cpp') diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 359b7d9843..b79da072de 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -322,6 +322,9 @@ private: * to time out. */ void MaybeSendPing(CNode& node_to, Peer& peer, std::chrono::microseconds now); + /** Send `addr` messages on a regular schedule. */ + void MaybeSendAddr(CNode* pto, std::chrono::microseconds current_time); + const CChainParams& m_chainparams; CConnman& m_connman; /** Pointer to this node's banman. May be nullptr - check existence before dereferencing. */ @@ -4138,6 +4141,72 @@ void PeerManagerImpl::MaybeSendPing(CNode& node_to, Peer& peer, std::chrono::mic } } +void PeerManagerImpl::MaybeSendAddr(CNode* pto, std::chrono::microseconds current_time) +{ + LOCK(pto->m_addr_send_times_mutex); + const CNetMsgMaker msgMaker(pto->GetCommonVersion()); + + if (fListen && pto->RelayAddrsWithConn() && + !m_chainman.ActiveChainstate().IsInitialBlockDownload() && + pto->m_next_local_addr_send < current_time) { + // If we've sent before, clear the bloom filter for the peer, so that our + // self-announcement will actually go out. + // This might be unnecessary if the bloom filter has already rolled + // over since our last self-announcement, but there is only a small + // bandwidth cost that we can incur by doing this (which happens + // once a day on average). + if (pto->m_next_local_addr_send != 0us) { + pto->m_addr_known->reset(); + } + if (std::optional local_addr = GetLocalAddrForPeer(pto)) { + FastRandomContext insecure_rand; + pto->PushAddress(*local_addr, insecure_rand); + } + pto->m_next_local_addr_send = PoissonNextSend(current_time, AVG_LOCAL_ADDRESS_BROADCAST_INTERVAL); + } + + // + // Message: addr + // + if (pto->RelayAddrsWithConn() && pto->m_next_addr_send < current_time) { + pto->m_next_addr_send = PoissonNextSend(current_time, AVG_ADDRESS_BROADCAST_INTERVAL); + std::vector vAddr; + vAddr.reserve(pto->vAddrToSend.size()); + assert(pto->m_addr_known); + + const char* msg_type; + int make_flags; + if (pto->m_wants_addrv2) { + msg_type = NetMsgType::ADDRV2; + make_flags = ADDRV2_FORMAT; + } else { + msg_type = NetMsgType::ADDR; + make_flags = 0; + } + + for (const CAddress& addr : pto->vAddrToSend) + { + if (!pto->m_addr_known->contains(addr.GetKey())) + { + pto->m_addr_known->insert(addr.GetKey()); + vAddr.push_back(addr); + // receiver rejects addr messages larger than MAX_ADDR_TO_SEND + if (vAddr.size() >= MAX_ADDR_TO_SEND) + { + m_connman.PushMessage(pto, msgMaker.Make(make_flags, msg_type, vAddr)); + vAddr.clear(); + } + } + } + pto->vAddrToSend.clear(); + if (!vAddr.empty()) + m_connman.PushMessage(pto, msgMaker.Make(make_flags, msg_type, vAddr)); + // we only send the big addr message once + if (pto->vAddrToSend.capacity() > 40) + pto->vAddrToSend.shrink_to_fit(); + } +} + namespace { class CompareInvMempoolOrder { @@ -4183,76 +4252,13 @@ bool PeerManagerImpl::SendMessages(CNode* pto) // MaybeSendPing may have marked peer for disconnection if (pto->fDisconnect) return true; + MaybeSendAddr(pto, current_time); + { LOCK(cs_main); CNodeState &state = *State(pto->GetId()); - // Address refresh broadcast - { - LOCK(pto->m_addr_send_times_mutex); - - if (fListen && pto->RelayAddrsWithConn() && - !m_chainman.ActiveChainstate().IsInitialBlockDownload() && - pto->m_next_local_addr_send < current_time) { - // If we've sent before, clear the bloom filter for the peer, so that our - // self-announcement will actually go out. - // This might be unnecessary if the bloom filter has already rolled - // over since our last self-announcement, but there is only a small - // bandwidth cost that we can incur by doing this (which happens - // once a day on average). - if (pto->m_next_local_addr_send != 0us) { - pto->m_addr_known->reset(); - } - if (std::optional local_addr = GetLocalAddrForPeer(pto)) { - FastRandomContext insecure_rand; - pto->PushAddress(*local_addr, insecure_rand); - } - pto->m_next_local_addr_send = PoissonNextSend(current_time, AVG_LOCAL_ADDRESS_BROADCAST_INTERVAL); - } - - // - // Message: addr - // - if (pto->RelayAddrsWithConn() && pto->m_next_addr_send < current_time) { - pto->m_next_addr_send = PoissonNextSend(current_time, AVG_ADDRESS_BROADCAST_INTERVAL); - std::vector vAddr; - vAddr.reserve(pto->vAddrToSend.size()); - assert(pto->m_addr_known); - - const char* msg_type; - int make_flags; - if (pto->m_wants_addrv2) { - msg_type = NetMsgType::ADDRV2; - make_flags = ADDRV2_FORMAT; - } else { - msg_type = NetMsgType::ADDR; - make_flags = 0; - } - - for (const CAddress& addr : pto->vAddrToSend) - { - if (!pto->m_addr_known->contains(addr.GetKey())) - { - pto->m_addr_known->insert(addr.GetKey()); - vAddr.push_back(addr); - // receiver rejects addr messages larger than MAX_ADDR_TO_SEND - if (vAddr.size() >= MAX_ADDR_TO_SEND) - { - m_connman.PushMessage(pto, msgMaker.Make(make_flags, msg_type, vAddr)); - vAddr.clear(); - } - } - } - pto->vAddrToSend.clear(); - if (!vAddr.empty()) - m_connman.PushMessage(pto, msgMaker.Make(make_flags, msg_type, vAddr)); - // we only send the big addr message once - if (pto->vAddrToSend.capacity() > 40) - pto->vAddrToSend.shrink_to_fit(); - } - } // pto->m_addr_send_times_mutex - // Start block sync if (pindexBestHeader == nullptr) pindexBestHeader = m_chainman.ActiveChain().Tip(); -- cgit v1.2.3 From c87423c58b5165de835a49bebd566538a70c07ab Mon Sep 17 00:00:00 2001 From: John Newbery Date: Wed, 23 Dec 2020 10:01:18 +0000 Subject: [net processing] Change MaybeSendAddr() to take a reference Change name of CNode parameter to node now that it's no longer a pointer. --- src/net_processing.cpp | 50 +++++++++++++++++++++++++------------------------- 1 file changed, 25 insertions(+), 25 deletions(-) (limited to 'src/net_processing.cpp') diff --git a/src/net_processing.cpp b/src/net_processing.cpp index b79da072de..cc0231afac 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -323,7 +323,7 @@ private: void MaybeSendPing(CNode& node_to, Peer& peer, std::chrono::microseconds now); /** Send `addr` messages on a regular schedule. */ - void MaybeSendAddr(CNode* pto, std::chrono::microseconds current_time); + void MaybeSendAddr(CNode& node, std::chrono::microseconds current_time); const CChainParams& m_chainparams; CConnman& m_connman; @@ -4141,42 +4141,42 @@ void PeerManagerImpl::MaybeSendPing(CNode& node_to, Peer& peer, std::chrono::mic } } -void PeerManagerImpl::MaybeSendAddr(CNode* pto, std::chrono::microseconds current_time) +void PeerManagerImpl::MaybeSendAddr(CNode& node, std::chrono::microseconds current_time) { - LOCK(pto->m_addr_send_times_mutex); - const CNetMsgMaker msgMaker(pto->GetCommonVersion()); + LOCK(node.m_addr_send_times_mutex); + const CNetMsgMaker msgMaker(node.GetCommonVersion()); - if (fListen && pto->RelayAddrsWithConn() && + if (fListen && node.RelayAddrsWithConn() && !m_chainman.ActiveChainstate().IsInitialBlockDownload() && - pto->m_next_local_addr_send < current_time) { + node.m_next_local_addr_send < current_time) { // If we've sent before, clear the bloom filter for the peer, so that our // self-announcement will actually go out. // This might be unnecessary if the bloom filter has already rolled // over since our last self-announcement, but there is only a small // bandwidth cost that we can incur by doing this (which happens // once a day on average). - if (pto->m_next_local_addr_send != 0us) { - pto->m_addr_known->reset(); + if (node.m_next_local_addr_send != 0us) { + node.m_addr_known->reset(); } - if (std::optional local_addr = GetLocalAddrForPeer(pto)) { + if (std::optional local_addr = GetLocalAddrForPeer(&node)) { FastRandomContext insecure_rand; - pto->PushAddress(*local_addr, insecure_rand); + node.PushAddress(*local_addr, insecure_rand); } - pto->m_next_local_addr_send = PoissonNextSend(current_time, AVG_LOCAL_ADDRESS_BROADCAST_INTERVAL); + node.m_next_local_addr_send = PoissonNextSend(current_time, AVG_LOCAL_ADDRESS_BROADCAST_INTERVAL); } // // Message: addr // - if (pto->RelayAddrsWithConn() && pto->m_next_addr_send < current_time) { - pto->m_next_addr_send = PoissonNextSend(current_time, AVG_ADDRESS_BROADCAST_INTERVAL); + if (node.RelayAddrsWithConn() && node.m_next_addr_send < current_time) { + node.m_next_addr_send = PoissonNextSend(current_time, AVG_ADDRESS_BROADCAST_INTERVAL); std::vector vAddr; - vAddr.reserve(pto->vAddrToSend.size()); - assert(pto->m_addr_known); + vAddr.reserve(node.vAddrToSend.size()); + assert(node.m_addr_known); const char* msg_type; int make_flags; - if (pto->m_wants_addrv2) { + if (node.m_wants_addrv2) { msg_type = NetMsgType::ADDRV2; make_flags = ADDRV2_FORMAT; } else { @@ -4184,26 +4184,26 @@ void PeerManagerImpl::MaybeSendAddr(CNode* pto, std::chrono::microseconds curren make_flags = 0; } - for (const CAddress& addr : pto->vAddrToSend) + for (const CAddress& addr : node.vAddrToSend) { - if (!pto->m_addr_known->contains(addr.GetKey())) + if (!node.m_addr_known->contains(addr.GetKey())) { - pto->m_addr_known->insert(addr.GetKey()); + node.m_addr_known->insert(addr.GetKey()); vAddr.push_back(addr); // receiver rejects addr messages larger than MAX_ADDR_TO_SEND if (vAddr.size() >= MAX_ADDR_TO_SEND) { - m_connman.PushMessage(pto, msgMaker.Make(make_flags, msg_type, vAddr)); + m_connman.PushMessage(&node, msgMaker.Make(make_flags, msg_type, vAddr)); vAddr.clear(); } } } - pto->vAddrToSend.clear(); + node.vAddrToSend.clear(); if (!vAddr.empty()) - m_connman.PushMessage(pto, msgMaker.Make(make_flags, msg_type, vAddr)); + m_connman.PushMessage(&node, msgMaker.Make(make_flags, msg_type, vAddr)); // we only send the big addr message once - if (pto->vAddrToSend.capacity() > 40) - pto->vAddrToSend.shrink_to_fit(); + if (node.vAddrToSend.capacity() > 40) + node.vAddrToSend.shrink_to_fit(); } } @@ -4252,7 +4252,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto) // MaybeSendPing may have marked peer for disconnection if (pto->fDisconnect) return true; - MaybeSendAddr(pto, current_time); + MaybeSendAddr(*pto, current_time); { LOCK(cs_main); -- cgit v1.2.3 From 38c0be5da3af17208b165e73cee7612d3670b038 Mon Sep 17 00:00:00 2001 From: John Newbery Date: Sun, 28 Feb 2021 11:16:52 +0000 Subject: [net processing] Refactor MaybeSendAddr() - early exits Add early exit guard clauses if node.RelayAddrsWithConn() is false or if current_time < node.m_next_addr_send. Add comments. This commit leaves some lines over-indented. Those will be fixed in a subsequent whitespace-only commit. --- src/net_processing.cpp | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) (limited to 'src/net_processing.cpp') diff --git a/src/net_processing.cpp b/src/net_processing.cpp index cc0231afac..fd8c653837 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -4143,11 +4143,16 @@ void PeerManagerImpl::MaybeSendPing(CNode& node_to, Peer& peer, std::chrono::mic void PeerManagerImpl::MaybeSendAddr(CNode& node, std::chrono::microseconds current_time) { - LOCK(node.m_addr_send_times_mutex); + // Nothing to do for non-address-relay peers + if (!node.RelayAddrsWithConn()) return; + + assert(node.m_addr_known); + const CNetMsgMaker msgMaker(node.GetCommonVersion()); - if (fListen && node.RelayAddrsWithConn() && - !m_chainman.ActiveChainstate().IsInitialBlockDownload() && + LOCK(node.m_addr_send_times_mutex); + // Periodically advertise our local address to the peer. + if (fListen && !m_chainman.ActiveChainstate().IsInitialBlockDownload() && node.m_next_local_addr_send < current_time) { // If we've sent before, clear the bloom filter for the peer, so that our // self-announcement will actually go out. @@ -4165,14 +4170,12 @@ void PeerManagerImpl::MaybeSendAddr(CNode& node, std::chrono::microseconds curre node.m_next_local_addr_send = PoissonNextSend(current_time, AVG_LOCAL_ADDRESS_BROADCAST_INTERVAL); } - // - // Message: addr - // - if (node.RelayAddrsWithConn() && node.m_next_addr_send < current_time) { + // We sent an `addr` message to this peer recently. Nothing more to do. + if (current_time <= node.m_next_addr_send) return; + { node.m_next_addr_send = PoissonNextSend(current_time, AVG_ADDRESS_BROADCAST_INTERVAL); std::vector vAddr; vAddr.reserve(node.vAddrToSend.size()); - assert(node.m_addr_known); const char* msg_type; int make_flags; -- cgit v1.2.3 From 01a79ff924b11f91796d4aa63c571897b047ac7d Mon Sep 17 00:00:00 2001 From: John Newbery Date: Sun, 28 Feb 2021 11:17:29 +0000 Subject: [net processing] Fix overindentation in MaybeSendAddr() Reviewer hint: review with `git diff --ignore-all-space`. --- src/net_processing.cpp | 57 +++++++++++++++++++++++++------------------------- 1 file changed, 28 insertions(+), 29 deletions(-) (limited to 'src/net_processing.cpp') diff --git a/src/net_processing.cpp b/src/net_processing.cpp index fd8c653837..1e58225158 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -4172,42 +4172,41 @@ void PeerManagerImpl::MaybeSendAddr(CNode& node, std::chrono::microseconds curre // We sent an `addr` message to this peer recently. Nothing more to do. if (current_time <= node.m_next_addr_send) return; - { - node.m_next_addr_send = PoissonNextSend(current_time, AVG_ADDRESS_BROADCAST_INTERVAL); - std::vector vAddr; - vAddr.reserve(node.vAddrToSend.size()); - const char* msg_type; - int make_flags; - if (node.m_wants_addrv2) { - msg_type = NetMsgType::ADDRV2; - make_flags = ADDRV2_FORMAT; - } else { - msg_type = NetMsgType::ADDR; - make_flags = 0; - } + node.m_next_addr_send = PoissonNextSend(current_time, AVG_ADDRESS_BROADCAST_INTERVAL); + std::vector vAddr; + vAddr.reserve(node.vAddrToSend.size()); + + const char* msg_type; + int make_flags; + if (node.m_wants_addrv2) { + msg_type = NetMsgType::ADDRV2; + make_flags = ADDRV2_FORMAT; + } else { + msg_type = NetMsgType::ADDR; + make_flags = 0; + } - for (const CAddress& addr : node.vAddrToSend) + for (const CAddress& addr : node.vAddrToSend) + { + if (!node.m_addr_known->contains(addr.GetKey())) { - if (!node.m_addr_known->contains(addr.GetKey())) + node.m_addr_known->insert(addr.GetKey()); + vAddr.push_back(addr); + // receiver rejects addr messages larger than MAX_ADDR_TO_SEND + if (vAddr.size() >= MAX_ADDR_TO_SEND) { - node.m_addr_known->insert(addr.GetKey()); - vAddr.push_back(addr); - // receiver rejects addr messages larger than MAX_ADDR_TO_SEND - if (vAddr.size() >= MAX_ADDR_TO_SEND) - { - m_connman.PushMessage(&node, msgMaker.Make(make_flags, msg_type, vAddr)); - vAddr.clear(); - } + m_connman.PushMessage(&node, msgMaker.Make(make_flags, msg_type, vAddr)); + vAddr.clear(); } } - node.vAddrToSend.clear(); - if (!vAddr.empty()) - m_connman.PushMessage(&node, msgMaker.Make(make_flags, msg_type, vAddr)); - // we only send the big addr message once - if (node.vAddrToSend.capacity() > 40) - node.vAddrToSend.shrink_to_fit(); } + node.vAddrToSend.clear(); + if (!vAddr.empty()) + m_connman.PushMessage(&node, msgMaker.Make(make_flags, msg_type, vAddr)); + // we only send the big addr message once + if (node.vAddrToSend.capacity() > 40) + node.vAddrToSend.shrink_to_fit(); } namespace { -- cgit v1.2.3 From 935d4889228e7e361c8b0020761fa0e08a55fb48 Mon Sep 17 00:00:00 2001 From: John Newbery Date: Thu, 9 Jul 2020 11:32:15 +0100 Subject: [net processing] Refactor MaybeSendAddr() Changes to make MaybeSendAddr simpler and easier to maintain/update: - assert invariant that node.vAddrToSend.size() can never exceed MAX_ADDR_TO_SEND - erase known addresses from vAddrToSend in one pass - no check for (vAddr.size() >= MAX_ADDR_TO_SEND) during iteration, since vAddr can never exceed MAX_ADDR_TO_SEND. --- src/net_processing.cpp | 46 ++++++++++++++++++++++++---------------------- 1 file changed, 24 insertions(+), 22 deletions(-) (limited to 'src/net_processing.cpp') diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 1e58225158..4fd37d3f76 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -33,6 +33,7 @@ #include #include +#include #include #include #include @@ -4148,8 +4149,6 @@ void PeerManagerImpl::MaybeSendAddr(CNode& node, std::chrono::microseconds curre assert(node.m_addr_known); - const CNetMsgMaker msgMaker(node.GetCommonVersion()); - LOCK(node.m_addr_send_times_mutex); // Periodically advertise our local address to the peer. if (fListen && !m_chainman.ActiveChainstate().IsInitialBlockDownload() && @@ -4174,8 +4173,25 @@ void PeerManagerImpl::MaybeSendAddr(CNode& node, std::chrono::microseconds curre if (current_time <= node.m_next_addr_send) return; node.m_next_addr_send = PoissonNextSend(current_time, AVG_ADDRESS_BROADCAST_INTERVAL); - std::vector vAddr; - vAddr.reserve(node.vAddrToSend.size()); + + if (!Assume(node.vAddrToSend.size() <= MAX_ADDR_TO_SEND)) { + // Should be impossible since we always check size before adding to + // vAddrToSend. Recover by trimming the vector. + node.vAddrToSend.resize(MAX_ADDR_TO_SEND); + } + + // Remove addr records that the peer already knows about, and add new + // addrs to the m_addr_known filter on the same pass. + auto addr_already_known = [&node](const CAddress& addr) { + bool ret = node.m_addr_known->contains(addr.GetKey()); + if (!ret) node.m_addr_known->insert(addr.GetKey()); + return ret; + }; + node.vAddrToSend.erase(std::remove_if(node.vAddrToSend.begin(), node.vAddrToSend.end(), addr_already_known), + node.vAddrToSend.end()); + + // No addr messages to send + if (node.vAddrToSend.empty()) return; const char* msg_type; int make_flags; @@ -4186,27 +4202,13 @@ void PeerManagerImpl::MaybeSendAddr(CNode& node, std::chrono::microseconds curre msg_type = NetMsgType::ADDR; make_flags = 0; } - - for (const CAddress& addr : node.vAddrToSend) - { - if (!node.m_addr_known->contains(addr.GetKey())) - { - node.m_addr_known->insert(addr.GetKey()); - vAddr.push_back(addr); - // receiver rejects addr messages larger than MAX_ADDR_TO_SEND - if (vAddr.size() >= MAX_ADDR_TO_SEND) - { - m_connman.PushMessage(&node, msgMaker.Make(make_flags, msg_type, vAddr)); - vAddr.clear(); - } - } - } + m_connman.PushMessage(&node, CNetMsgMaker(node.GetCommonVersion()).Make(make_flags, msg_type, node.vAddrToSend)); node.vAddrToSend.clear(); - if (!vAddr.empty()) - m_connman.PushMessage(&node, msgMaker.Make(make_flags, msg_type, vAddr)); + // we only send the big addr message once - if (node.vAddrToSend.capacity() > 40) + if (node.vAddrToSend.capacity() > 40) { node.vAddrToSend.shrink_to_fit(); + } } namespace { -- cgit v1.2.3