Comment on CNode::nLocalServices meaning #8785

pull instagibbs wants to merge 1 commits into bitcoin:master from instagibbs:nlocalservisme changing 1 files +1 −0

instagibbs commented at 12:46 AM on September 22, 2016: member

It was quite unclear what this field meant especially in places that say things like:

pfrom->nLocalServices
instagibbs commented at 12:46 AM on September 22, 2016: member

ping @theuni to make sure this is the case
in src/net.h:None in b89bd0883e outdated
```
 676 | @@ -677,6 +677,7 @@ class CNode
 677 |  
 678 |  
 679 |      uint64_t nLocalHostNonce;
 680 | +    // What local services you advertised on connection to this node
```
laanwj commented at 1:37 AM on September 22, 2016:

I think the actors in this sentence are slightly confusing, who is 'you' and who is 'this node' here?

instagibbs commented at 1:41 AM on September 22, 2016:

Suggestion to clarify? "this node" refers to "CNode" here, which is:

/** Information about a peer */

you could be rephrased as your local node?

rebroad commented at 1:42 AM on September 22, 2016:

Why do we need this variable per connection anyway? Do we advertise different services to different nodes?

instagibbs commented at 1:43 AM on September 22, 2016:

@rebroad afaik it is to keep track of what services you advertised to each peer. It could change over the lifetime of the connection.

rebroad commented at 1:46 AM on September 22, 2016:

@instagibbs Services are advertised to a peer in the version message, which can be sent only once. (future version messages are ignored, and are counted as misbehaviour)

laanwj commented at 2:04 AM on September 22, 2016:

Yes, different services may be advertised to different peers, based on, say, whitelisting.

laanwj commented at 2:14 AM on September 22, 2016:

@instagibbs yes, I'd say "the local node" versus "the peer" is unambigious

theuni commented at 6:42 AM on September 22, 2016:

Yes, the intention is to be able to (in the future) offer different services to different peers. For example, incoming vs outgoing, proxy, tor, whitelist, etc. Even if we don't do that now, it's helpful to think in terms of "what did I tell this peer I'm offering?" rather than a global value. For now each connman instance (only 1 for now) passes along the same services to all nodes.

ACK on clarifying, though I'd say something like "Services offered to this peer".
instagibbs commented at 1:47 AM on September 22, 2016: member
You are allowed to add or remove services to new peers.

On Sep 21, 2016 9:46 PM, "R E Broadley" notifications@github.com wrote:
@rebroad commented on this pull request.

In src/net.h #8785:

@@ -677,6 +677,7 @@ class CNode

uint64_t nLocalHostNonce;

// What local services you advertised on connection to this node

@instagibbs https://github.com/instagibbs Services are advertised to a peer in the version message, which can be sent only once. (future version messages are ignored, and are counted as misbehaviour)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub #8785, or mute the thread https://github.com/notifications/unsubscribe-auth/AFgC04Eie8_Jv1y_l9-x0T2Mo4WbjeXzks5qsd4NgaJpZM4KDbIF .
rebroad commented at 1:54 AM on September 22, 2016: contributor

@instagibbs Do you mean the SENDHEADERS and SENDCMPCT messages? I don't think these are counted as services according to the variable we are discussing.
fanquake added the label Docs and Output on Sep 22, 2016
Comment on CConnman::nLocalServices meaning b5ccded57f
instagibbs force-pushed on Sep 22, 2016
instagibbs commented at 12:20 PM on September 22, 2016: member

updated with @theuni 's shortened version
laanwj commented at 12:29 PM on September 22, 2016: member

Thanks, yes this seems fine.
TheBlueMatt commented at 2:19 PM on September 22, 2016: member

ACK latest copy
laanwj merged this on Sep 22, 2016
laanwj closed this on Sep 22, 2016
laanwj referenced this in commit 2b514aa2ea on Sep 22, 2016
codablock referenced this in commit f96cc9e286 on Sep 19, 2017
codablock referenced this in commit 17e110f47e on Jan 11, 2018
andvgal referenced this in commit 8cb7c7deed on Jan 6, 2019
DrahtBot locked this on Sep 8, 2021

Contributors

Labels

Comment on CNode::nLocalServices meaning #8785

@rebroad commented on this pull request.