foster/bitcoin-bitcoin-core
0
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-02-08 10:31:50 -05:00
Code Issues Activity

Merge bitcoin/bitcoin#30233: refactor: move m_is_inbound out of CNodeState

Browse source 
07f4cebe57 refactor: move m_is_inbound out of CNodeState (Sergi Delgado Segura)

Pull request description:

  `m_is_inbound` cannot be changed throughout the life of a `Peer`. However, we are currently storing it in `CNodeState`, which requires locking `cs_main` in order to access it. This can be moved to the outside scope and only require `m_peer_mutex`.

  This is a refactor in preparation for Erlay reworks.

ACKs for top commit:
  maflcko:
    ACK 07f4cebe57 🗞
  achow101:
    ACK 07f4cebe57
  marcofleon:
    ACK 07f4cebe57
  naumenkogs:
    ACK 07f4cebe57

Tree-SHA512: bcc77135646c697204a4605971774cb3ccdf11b3e363a7339bfb4d1678de1681d6ca642dc467f7d71834a94dd56e05367e7fd32a3e6dc6be30c89342f39bf695
This commit is contained in:
Ava Chow 2024-09-13 15:40:43 -04:00
commit 71af7435ef
No known key found for this signature in database
GPG key ID: 17565732E08E5E41
1 changed files with 18 additions and 24 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

42
src/net_processing.cpp
View file

@ -224,6 +224,9 @@ struct Peer {
/** Services this peer offered to us. */ /** Services this peer offered to us. */
std::atomic<ServiceFlags> m_their_services{NODE_NONE}; std::atomic<ServiceFlags> m_their_services{NODE_NONE};
//! Whether this peer is an inbound connection
const bool m_is_inbound;
/** Protects misbehavior data members */ /** Protects misbehavior data members */
Mutex m_misbehavior_mutex; Mutex m_misbehavior_mutex;
/** Whether this peer should be disconnected and marked as discouraged (unless it has NetPermissionFlags::NoBan permission). */ /** Whether this peer should be disconnected and marked as discouraged (unless it has NetPermissionFlags::NoBan permission). */
@ -394,9 +397,10 @@ struct Peer {
* timestamp the peer sent in the version message. */ * timestamp the peer sent in the version message. */
std::atomic<std::chrono::seconds> m_time_offset{0s}; std::atomic<std::chrono::seconds> m_time_offset{0s};
explicit Peer(NodeId id, ServiceFlags our_services) explicit Peer(NodeId id, ServiceFlags our_services, bool is_inbound)
: m_id{id} : m_id{id}
, m_our_services{our_services} , m_our_services{our_services}
, m_is_inbound{is_inbound}
{} {}
private: private:
@ -476,11 +480,6 @@ struct CNodeState {
//! Time of last new block announcement //! Time of last new block announcement
int64_t m_last_block_announcement{0}; int64_t m_last_block_announcement{0};
//! Whether this peer is an inbound connection
const bool m_is_inbound;
CNodeState(bool is_inbound) : m_is_inbound(is_inbound) {}
}; };
class PeerManagerImpl final : public PeerManager class PeerManagerImpl final : public PeerManager
@ -1015,7 +1014,7 @@ private:
bool IsBlockRequested(const uint256& hash) EXCLUSIVE_LOCKS_REQUIRED(cs_main); bool IsBlockRequested(const uint256& hash) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
/** Have we requested this block from an outbound peer */ /** Have we requested this block from an outbound peer */
bool IsBlockRequestedFromOutbound(const uint256& hash) EXCLUSIVE_LOCKS_REQUIRED(cs_main); bool IsBlockRequestedFromOutbound(const uint256& hash) EXCLUSIVE_LOCKS_REQUIRED(cs_main, !m_peer_mutex);
/** Remove this block from our tracked requested blocks. Called if: /** Remove this block from our tracked requested blocks. Called if:
* - the block has been received from a peer * - the block has been received from a peer
@ -1099,7 +1098,7 @@ private:
* lNodesAnnouncingHeaderAndIDs, and keeping that list under a certain size by * lNodesAnnouncingHeaderAndIDs, and keeping that list under a certain size by
* removing the first element if necessary. * removing the first element if necessary.
*/ */
void MaybeSetPeerAsAnnouncingHeaderAndIDs(NodeId nodeid) EXCLUSIVE_LOCKS_REQUIRED(cs_main); void MaybeSetPeerAsAnnouncingHeaderAndIDs(NodeId nodeid) EXCLUSIVE_LOCKS_REQUIRED(cs_main, !m_peer_mutex);
/** Stack of nodes which we have set to announce using compact blocks */ /** Stack of nodes which we have set to announce using compact blocks */
std::list<NodeId> lNodesAnnouncingHeaderAndIDs GUARDED_BY(cs_main); std::list<NodeId> lNodesAnnouncingHeaderAndIDs GUARDED_BY(cs_main);
@ -1302,8 +1301,8 @@ bool PeerManagerImpl::IsBlockRequestedFromOutbound(const uint256& hash)
{ {
for (auto range = mapBlocksInFlight.equal_range(hash); range.first != range.second; range.first++) { for (auto range = mapBlocksInFlight.equal_range(hash); range.first != range.second; range.first++) {
auto [nodeid, block_it] = range.first->second; auto [nodeid, block_it] = range.first->second;
CNodeState& nodestate = *Assert(State(nodeid)); PeerRef peer{GetPeerRef(nodeid)};
if (!nodestate.m_is_inbound) return true; if (peer && !peer->m_is_inbound) return true;
} }
return false; return false;
@ -1392,6 +1391,7 @@ void PeerManagerImpl::MaybeSetPeerAsAnnouncingHeaderAndIDs(NodeId nodeid)
if (m_opts.ignore_incoming_txs) return; if (m_opts.ignore_incoming_txs) return;
CNodeState* nodestate = State(nodeid); CNodeState* nodestate = State(nodeid);
PeerRef peer{GetPeerRef(nodeid)};
if (!nodestate || !nodestate->m_provides_cmpctblocks) { if (!nodestate || !nodestate->m_provides_cmpctblocks) {
// Don't request compact blocks if the peer has not signalled support // Don't request compact blocks if the peer has not signalled support
return; return;
@ -1404,15 +1404,15 @@ void PeerManagerImpl::MaybeSetPeerAsAnnouncingHeaderAndIDs(NodeId nodeid)
lNodesAnnouncingHeaderAndIDs.push_back(nodeid); lNodesAnnouncingHeaderAndIDs.push_back(nodeid);
return; return;
} }
CNodeState *state = State(*it); PeerRef peer_ref{GetPeerRef(*it)};
if (state != nullptr && !state->m_is_inbound) ++num_outbound_hb_peers; if (peer_ref && !peer_ref->m_is_inbound) ++num_outbound_hb_peers;
} }
if (nodestate->m_is_inbound) { if (peer && peer->m_is_inbound) {
// If we're adding an inbound HB peer, make sure we're not removing // If we're adding an inbound HB peer, make sure we're not removing
// our last outbound HB peer in the process. // our last outbound HB peer in the process.
if (lNodesAnnouncingHeaderAndIDs.size() >= 3 && num_outbound_hb_peers == 1) { if (lNodesAnnouncingHeaderAndIDs.size() >= 3 && num_outbound_hb_peers == 1) {
CNodeState *remove_node = State(lNodesAnnouncingHeaderAndIDs.front()); PeerRef remove_peer{GetPeerRef(lNodesAnnouncingHeaderAndIDs.front())};
if (remove_node != nullptr && !remove_node->m_is_inbound) { if (remove_peer && !remove_peer->m_is_inbound) {
// Put the HB outbound peer in the second slot, so that it // Put the HB outbound peer in the second slot, so that it
// doesn't get removed. // doesn't get removed.
std::swap(lNodesAnnouncingHeaderAndIDs.front(), *std::next(lNodesAnnouncingHeaderAndIDs.begin())); std::swap(lNodesAnnouncingHeaderAndIDs.front(), *std::next(lNodesAnnouncingHeaderAndIDs.begin()));
@ -1720,7 +1720,7 @@ void PeerManagerImpl::InitializeNode(const CNode& node, ServiceFlags our_service
NodeId nodeid = node.GetId(); NodeId nodeid = node.GetId();
{ {
LOCK(cs_main); // For m_node_states LOCK(cs_main); // For m_node_states
m_node_states.emplace_hint(m_node_states.end(), std::piecewise_construct, std::forward_as_tuple(nodeid), std::forward_as_tuple(node.IsInboundConn())); m_node_states.try_emplace(m_node_states.end(), nodeid);
} }
{ {
LOCK(m_tx_download_mutex); LOCK(m_tx_download_mutex);
@ -1731,7 +1731,7 @@ void PeerManagerImpl::InitializeNode(const CNode& node, ServiceFlags our_service
our_services = static_cast<ServiceFlags>(our_services | NODE_BLOOM); our_services = static_cast<ServiceFlags>(our_services | NODE_BLOOM);
} }
PeerRef peer = std::make_shared<Peer>(nodeid, our_services); PeerRef peer = std::make_shared<Peer>(nodeid, our_services, node.IsInboundConn());
{ {
LOCK(m_peer_mutex); LOCK(m_peer_mutex);
m_peer_map.emplace_hint(m_peer_map.end(), nodeid, peer); m_peer_map.emplace_hint(m_peer_map.end(), nodeid, peer);
@ -1968,15 +1968,9 @@ void PeerManagerImpl::MaybePunishNodeForBlock(NodeId nodeid, const BlockValidati
break; break;
case BlockValidationResult::BLOCK_CACHED_INVALID: case BlockValidationResult::BLOCK_CACHED_INVALID:
{ {
LOCK(cs_main);
CNodeState *node_state = State(nodeid);
if (node_state == nullptr) {
break;
}
// Discourage outbound (but not inbound) peers if on an invalid chain. // Discourage outbound (but not inbound) peers if on an invalid chain.
// Exempt HB compact block peers. Manual connections are always protected from discouragement. // Exempt HB compact block peers. Manual connections are always protected from discouragement.
if (!via_compact_block && !node_state->m_is_inbound) { if (peer && !via_compact_block && !peer->m_is_inbound) {
if (peer) Misbehaving(*peer, message); if (peer) Misbehaving(*peer, message);
return; return;
} }