Skip to content

Commit 82e53f3

Browse files
committed
doc: add comments clarifying how local services are advertised
Recent questions have come up regarding dynamic service registration (see bitcoin/bitcoin#16442 (comment) and the assumeutxo project, which needs to dynamically flip NODE_NETWORK). While investigating how dynamic service registration might work, I was confused about how we convey local services to peers. This adds some documentation that hopefully clarifies this process.
1 parent 1985c4e commit 82e53f3

File tree

2 files changed

+38
-2
lines changed

2 files changed

+38
-2
lines changed

src/net.h

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -282,6 +282,12 @@ class CConnman
282282
bool DisconnectNode(const CNetAddr& addr);
283283
bool DisconnectNode(NodeId id);
284284

285+
//! Used to convey which local services we are offering peers during node
286+
//! connection.
287+
//!
288+
//! The data returned by this is used in CNode construction,
289+
//! which is used to advertise which services we are offering
290+
//! that peer during `net_processing.cpp:PushNodeVersion()`.
285291
ServiceFlags GetLocalServices() const;
286292

287293
//!set the max outbound target in bytes
@@ -413,7 +419,18 @@ class CConnman
413419
std::atomic<NodeId> nLastNodeId{0};
414420
unsigned int nPrevNodeCount{0};
415421

416-
/** Services this instance offers */
422+
/**
423+
* Services this instance offers.
424+
*
425+
* This data is replicated in each CNode instance we create during peer
426+
* connection (in ConnectNode()) under a member also called
427+
* nLocalServices.
428+
*
429+
* This data is not marked const, but after being set it should not
430+
* change. See the note in CNode::nLocalServices documentation.
431+
*
432+
* \sa CNode::nLocalServices
433+
*/
417434
ServiceFlags nLocalServices;
418435

419436
std::unique_ptr<CSemaphore> semOutbound;
@@ -786,8 +803,24 @@ class CNode
786803
private:
787804
const NodeId id;
788805
const uint64_t nLocalHostNonce;
789-
// Services offered to this peer
806+
807+
//! Services offered to this peer.
808+
//!
809+
//! This is supplied by the parent CConnman during peer connection
810+
//! (CConnman::ConnectNode()) from its attribute of the same name.
811+
//!
812+
//! This is const because there is no protocol defined for renegotiating
813+
//! services initially offered to a peer. The set of local services we
814+
//! offer should not change after initialization.
815+
//!
816+
//! An interesting example of this is NODE_NETWORK and initial block
817+
//! download: a node which starts up from scratch doesn't have any blocks
818+
//! to serve, but still advertises NODE_NETWORK because it will eventually
819+
//! fulfill this role after IBD completes. P2P code is written in such a
820+
//! way that it can gracefully handle peers who don't make good on their
821+
//! service advertisements.
790822
const ServiceFlags nLocalServices;
823+
791824
const int nMyStartingHeight;
792825
int nSendVersion{0};
793826
NetPermissionFlags m_permissionFlags{ PF_NONE };

src/net_processing.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -415,6 +415,9 @@ static void UpdatePreferredDownload(CNode* node, CNodeState* state) EXCLUSIVE_LO
415415

416416
static void PushNodeVersion(CNode *pnode, CConnman* connman, int64_t nTime)
417417
{
418+
// Note that pnode->GetLocalServices() is a reflection of the local
419+
// services we were offering when the CNode object was created for this
420+
// peer.
418421
ServiceFlags nLocalNodeServices = pnode->GetLocalServices();
419422
uint64_t nonce = pnode->GetLocalNonce();
420423
int nNodeStartingHeight = pnode->GetMyStartingHeight();

0 commit comments

Comments
 (0)