Skip to content

Commit

Permalink
merge bitcoin#25514: Move CNode::nServices and CNode::nLocalServices …
Browse files Browse the repository at this point in the history
…to Peer

Co-authored-by: UdjinM6 <[email protected]>
  • Loading branch information
2 people authored and PastaPastaPasta committed Aug 9, 2024
1 parent c9923ca commit 0f9ece0
Show file tree
Hide file tree
Showing 15 changed files with 197 additions and 200 deletions.
4 changes: 2 additions & 2 deletions src/evo/mnauth.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ void CMNAuth::PushMNAUTH(CNode& peer, CConnman& connman, const CActiveMasternode
connman.PushMessage(&peer, CNetMsgMaker(peer.GetCommonVersion()).Make(NetMsgType::MNAUTH, mnauth));
}

PeerMsgRet CMNAuth::ProcessMessage(CNode& peer, CConnman& connman, CMasternodeMetaMan& mn_metaman, const CActiveMasternodeManager* const mn_activeman,
PeerMsgRet CMNAuth::ProcessMessage(CNode& peer, ServiceFlags node_services, CConnman& connman, CMasternodeMetaMan& mn_metaman, const CActiveMasternodeManager* const mn_activeman,
const CChain& active_chain, const CMasternodeSync& mn_sync, const CDeterministicMNList& tip_mn_list,
std::string_view msg_type, CDataStream& vRecv)
{
Expand All @@ -79,7 +79,7 @@ PeerMsgRet CMNAuth::ProcessMessage(CNode& peer, CConnman& connman, CMasternodeMe
return tl::unexpected{MisbehavingError{100, "duplicate mnauth"}};
}

if ((~peer.nServices) & (NODE_NETWORK | NODE_BLOOM)) {
if ((~node_services) & (NODE_NETWORK | NODE_BLOOM)) {
// either NODE_NETWORK or NODE_BLOOM bit is missing in node's services
return tl::unexpected{MisbehavingError{100, "mnauth from a node with invalid services"}};
}
Expand Down
4 changes: 3 additions & 1 deletion src/evo/mnauth.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ class CNode;

class UniValue;

enum ServiceFlags : uint64_t;

/**
* This class handles the p2p message MNAUTH. MNAUTH is sent directly after VERACK and authenticates the sender as a
* masternode. It is only sent when the sender is actually a masternode.
Expand Down Expand Up @@ -59,7 +61,7 @@ class CMNAuth
* @pre CMasternodeMetaMan's database must be successfully loaded before
* attempting to call this function regardless of sync state
*/
static PeerMsgRet ProcessMessage(CNode& peer, CConnman& connman, CMasternodeMetaMan& mn_metaman, const CActiveMasternodeManager* const mn_activeman,
static PeerMsgRet ProcessMessage(CNode& peer, ServiceFlags node_services, CConnman& connman, CMasternodeMetaMan& mn_metaman, const CActiveMasternodeManager* const mn_activeman,
const CChain& active_chain, const CMasternodeSync& mn_sync, const CDeterministicMNList& tip_mn_list,
std::string_view msg_type, CDataStream& vRecv);
static void NotifyMasternodeListChanged(bool undo, const CDeterministicMNList& oldMNList, const CDeterministicMNListDiff& diff, CConnman& connman);
Expand Down
42 changes: 18 additions & 24 deletions src/net.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -230,15 +230,13 @@ static std::vector<CAddress> ConvertSeeds(const std::vector<uint8_t> &vSeedsIn)
// Otherwise, return the unroutable 0.0.0.0 but filled in with
// the normal parameters, since the IP may be changed to a useful
// one by discovery.
CAddress GetLocalAddress(const CNetAddr *paddrPeer, ServiceFlags nLocalServices)
CService GetLocalAddress(const CNetAddr& addrPeer)
{
CAddress ret(CService(CNetAddr(),GetListenPort()), nLocalServices);
CService ret{CNetAddr(), GetListenPort()};
CService addr;
if (GetLocal(addr, paddrPeer))
{
ret = CAddress(addr, nLocalServices);
if (GetLocal(addr, &addrPeer)) {
ret = CService{addr};
}
ret.nTime = GetAdjustedTime();
return ret;
}

Expand All @@ -257,35 +255,35 @@ bool IsPeerAddrLocalGood(CNode *pnode)
IsReachable(addrLocal.GetNetwork());
}

std::optional<CAddress> GetLocalAddrForPeer(CNode *pnode)
std::optional<CService> GetLocalAddrForPeer(CNode& node)
{
CAddress addrLocal = GetLocalAddress(&pnode->addr, pnode->GetLocalServices());
CService addrLocal{GetLocalAddress(node.addr)};
if (gArgs.GetBoolArg("-addrmantest", false)) {
// use IPv4 loopback during addrmantest
addrLocal = CAddress(CService(LookupNumeric("127.0.0.1", GetListenPort())), pnode->GetLocalServices());
addrLocal = CService(LookupNumeric("127.0.0.1", GetListenPort()));
}
// If discovery is enabled, sometimes give our peer the address it
// tells us that it sees us as in case it has a better idea of our
// address than we do.
FastRandomContext rng;
if (IsPeerAddrLocalGood(pnode) && (!addrLocal.IsRoutable() ||
if (IsPeerAddrLocalGood(&node) && (!addrLocal.IsRoutable() ||
rng.randbits((GetnScore(addrLocal) > LOCAL_MANUAL) ? 3 : 1) == 0))
{
if (pnode->IsInboundConn()) {
if (node.IsInboundConn()) {
// For inbound connections, assume both the address and the port
// as seen from the peer.
addrLocal = CAddress{pnode->GetAddrLocal(), addrLocal.nServices, addrLocal.nTime};
addrLocal = CService{node.GetAddrLocal()};
} else {
// For outbound connections, assume just the address as seen from
// the peer and leave the port in `addrLocal` as returned by
// `GetLocalAddress()` above. The peer has no way to observe our
// listening port when we have initiated the connection.
addrLocal.SetIP(pnode->GetAddrLocal());
addrLocal.SetIP(node.GetAddrLocal());
}
}
if (addrLocal.IsRoutable() || gArgs.GetBoolArg("-addrmantest", false))
{
LogPrint(BCLog::NET, "Advertising address %s to peer=%d\n", addrLocal.ToString(), pnode->GetId());
LogPrint(BCLog::NET, "Advertising address %s to peer=%d\n", addrLocal.ToString(), node.GetId());
return addrLocal;
}
// Address is unroutable. Don't advertise.
Expand Down Expand Up @@ -610,7 +608,6 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo
addr_bind = GetBindAddress(*sock);
}
CNode* pnode = new CNode(id,
nLocalServices,
std::move(sock),
addrConnect,
CalculateKeyedNetGroup(addrConnect),
Expand Down Expand Up @@ -733,7 +730,6 @@ Network CNode::ConnectedThroughNetwork() const
void CNode::CopyStats(CNodeStats& stats)
{
stats.nodeid = this->GetId();
X(nServices);
X(addr);
X(addrBind);
stats.m_network = ConnectedThroughNetwork();
Expand Down Expand Up @@ -1247,7 +1243,7 @@ bool CConnman::AttemptToEvictConnection()

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_has_all_wanted_services,
node->m_relays_txs.load(), node->m_bloom_filter_loaded.load(),
node->nKeyedNetGroup, node->m_prefer_evict, node->addr.IsLocal(),
node->ConnectedThroughNetwork()};
Expand Down Expand Up @@ -1404,7 +1400,6 @@ void CConnman::CreateNodeFromAcceptedSocket(std::unique_ptr<Sock>&& sock,

const bool inbound_onion = std::find(m_onion_binds.begin(), m_onion_binds.end(), addr_bind) != m_onion_binds.end();
CNode* pnode = new CNode(id,
nodeServices,
std::move(sock),
addr,
CalculateKeyedNetGroup(addr),
Expand All @@ -1418,7 +1413,7 @@ void CConnman::CreateNodeFromAcceptedSocket(std::unique_ptr<Sock>&& sock,
// If this flag is present, the user probably expect that RPC and QT report it as whitelisted (backward compatibility)
pnode->m_legacyWhitelisted = legacyWhitelisted;
pnode->m_prefer_evict = discouraged;
m_msgproc->InitializeNode(pnode);
m_msgproc->InitializeNode(*pnode, nodeServices);

{
LOCK(pnode->m_sock_mutex);
Expand Down Expand Up @@ -1645,10 +1640,11 @@ void CConnman::CalculateNumConnectionsChangedStats()
for (const mapMsgTypeSize::value_type &i : pnode->mapSendBytesPerMsgType)
mapSentBytesMsgStats[i.first] += i.second;
}
if(pnode->fClient)
if (pnode->m_bloom_filter_loaded.load()) {
spvNodes++;
else
} else {
fullNodes++;
}
if(pnode->IsInboundConn())
inboundNodes++;
else
Expand Down Expand Up @@ -3139,7 +3135,7 @@ void CConnman::OpenNetworkConnection(const CAddress& addrConnect, bool fCountFai
mapSocketToNode.emplace(pnode->m_sock->Get(), pnode);
}

m_msgproc->InitializeNode(pnode);
m_msgproc->InitializeNode(*pnode, nLocalServices);
{
LOCK(m_nodes_mutex);
m_nodes.push_back(pnode);
Expand Down Expand Up @@ -4133,7 +4129,6 @@ ServiceFlags CConnman::GetLocalServices() const
unsigned int CConnman::GetReceiveFloodSize() const { return nReceiveFloodSize; }

CNode::CNode(NodeId idIn,
ServiceFlags nLocalServicesIn,
std::shared_ptr<Sock> sock,
const CAddress& addrIn,
uint64_t nKeyedNetGroupIn,
Expand All @@ -4155,7 +4150,6 @@ CNode::CNode(NodeId idIn,
id{idIn},
nLocalHostNonce{nLocalHostNonceIn},
m_conn_type{conn_type_in},
nLocalServices{nLocalServicesIn},
m_i2p_sam_session{std::move(i2p_sam_session)}
{
if (inbound_onion) assert(conn_type_in == ConnectionType::INBOUND);
Expand Down
48 changes: 11 additions & 37 deletions src/net.h
Original file line number Diff line number Diff line change
Expand Up @@ -247,8 +247,8 @@ enum
};

bool IsPeerAddrLocalGood(CNode *pnode);
/** Returns a local address that we should advertise to this peer */
std::optional<CAddress> GetLocalAddrForPeer(CNode *pnode);
/** Returns a local address that we should advertise to this peer. */
std::optional<CService> GetLocalAddrForPeer(CNode& node);

/**
* Mark a network as reachable or unreachable (no automatic connects to it)
Expand All @@ -266,7 +266,7 @@ void RemoveLocal(const CService& addr);
bool SeenLocal(const CService& addr);
bool IsLocal(const CService& addr);
bool GetLocal(CService &addr, const CNetAddr *paddrPeer = nullptr);
CAddress GetLocalAddress(const CNetAddr *paddrPeer, ServiceFlags nLocalServices);
CService GetLocalAddress(const CNetAddr& addrPeer);


extern bool fDiscover;
Expand All @@ -290,7 +290,6 @@ class CNodeStats
{
public:
NodeId nodeid;
ServiceFlags nServices;
std::chrono::seconds m_last_send;
std::chrono::seconds m_last_recv;
std::chrono::seconds m_last_tx_time;
Expand Down Expand Up @@ -456,7 +455,6 @@ class CNode
const std::unique_ptr<const TransportSerializer> m_serializer;

NetPermissionFlags m_permissionFlags{NetPermissionFlags::None}; // treated as const outside of fuzz tester
std::atomic<ServiceFlags> nServices{NODE_NONE};

/**
* Socket used for communication with the node.
Expand Down Expand Up @@ -516,8 +514,6 @@ class CNode
}
// This boolean is unusued in actual processing, only present for backward compatibility at RPC/QT level
bool m_legacyWhitelisted{false};
bool fClient{false}; // set by version message
bool m_limited_node{false}; //after BIP159, set by version message
/** fSuccessfullyConnected is set to true on receiving VERACK from the peer. */
std::atomic_bool fSuccessfullyConnected{false};
// Setting fDisconnect to true will cause the node to be disconnected the
Expand Down Expand Up @@ -617,6 +613,9 @@ class CNode
// Peer selected us as (compact blocks) high-bandwidth peer (BIP152)
std::atomic<bool> m_bip152_highbandwidth_from{false};

/** Whether this peer provides all services that we want. Used for eviction decisions */
std::atomic_bool m_has_all_wanted_services{false};

/** Whether we should relay transactions to this peer (their version
* message did not include fRelay=false and this is not a block-relay-only
* connection). This only changes from false to true. It will never change
Expand Down Expand Up @@ -658,7 +657,6 @@ class CNode
bool IsBlockRelayOnly() const;

CNode(NodeId id,
ServiceFlags nLocalServicesIn,
std::shared_ptr<Sock> sock,
const CAddress &addrIn,
uint64_t nKeyedNetGroupIn,
Expand Down Expand Up @@ -725,11 +723,6 @@ class CNode

void CopyStats(CNodeStats& stats) EXCLUSIVE_LOCKS_REQUIRED(!m_subver_mutex, !m_addr_local_mutex, !cs_vSend, !cs_vRecv);

ServiceFlags GetLocalServices() const
{
return nLocalServices;
}

std::string ConnectionTypeAsString() const { return ::ConnectionTypeAsString(m_conn_type); }

/** A ping-pong round trip has completed successfully. Update latest and minimum ping times. */
Expand Down Expand Up @@ -788,23 +781,6 @@ class CNode
const ConnectionType m_conn_type;
std::atomic<int> m_greatest_common_version{INIT_PROTO_VERSION};

//! Services offered to this peer.
//!
//! This is supplied by the parent CConnman during peer connection
//! (CConnman::ConnectNode()) from its attribute of the same name.
//!
//! This is const because there is no protocol defined for renegotiating
//! services initially offered to a peer. The set of local services we
//! offer should not change after initialization.
//!
//! An interesting example of this is NODE_NETWORK and initial block
//! download: a node which starts up from scratch doesn't have any blocks
//! to serve, but still advertises NODE_NETWORK because it will eventually
//! fulfill this role after IBD completes. P2P code is written in such a
//! way that it can gracefully handle peers who don't make good on their
//! service advertisements.
const ServiceFlags nLocalServices;

std::list<CNetMessage> vRecvMsg; // Used only by SocketHandler thread

// Our address, as reported by the peer
Expand Down Expand Up @@ -841,7 +817,7 @@ class NetEventsInterface
{
public:
/** Initialize a peer (setup state, queue any initial messages) */
virtual void InitializeNode(CNode* pnode) = 0;
virtual void InitializeNode(CNode& node, ServiceFlags our_services) = 0;

/** Handle removal of a peer (clear state) */
virtual void FinalizeNode(const CNode& node) = 0;
Expand Down Expand Up @@ -1503,16 +1479,14 @@ friend class CNode;
std::map<uint64_t, CachedAddrResponse> m_addr_response_caches;

/**
* Services this instance offers.
* Services this node offers.
*
* This data is replicated in each CNode instance we create during peer
* connection (in ConnectNode()) under a member also called
* nLocalServices.
* This data is replicated in each Peer instance we create.
*
* This data is not marked const, but after being set it should not
* change. See the note in CNode::nLocalServices documentation.
* change.
*
* \sa CNode::nLocalServices
* \sa Peer::our_services
*/
ServiceFlags nLocalServices;

Expand Down
Loading

0 comments on commit 0f9ece0

Please sign in to comment.