-
Notifications
You must be signed in to change notification settings - Fork 38.7k
RPC : More user-friendly services field in getnetworkinfo and getpeerinfo #15511
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
What's the motivation for this? Generally we've regarded the flags as rather esoteric troubleshooting stuff. The example output doesn't make it clear how unknown values would be handled. |
|
The motivation is that it is more readable for an user which services the node provides, thus making it more convenient to use : no need to parse the flags himself (which needs to read the code to check what is the meaning of each flag). |
|
NACK, there could be some software that expects output of |
|
Indeed. |
59ca4df to
c6ec631
Compare
…put services names
c6ec631 to
309102f
Compare
|
Changed the behavior to add a new field instead of modifying the existent one. Edited the description. |
|
I'm sorry, I don't see how "more readable" is actually helpful in and of itself. Why does anyone need to be reading these things?
Yes, field is a lot more useful for unknown values-- e.g. reasoning about new or incompatible software-- than it is for known ones. |
Actually, I made this after helping the guy asking about |
|
For what it's worth, I would concept ACK something like this -- the times I use |
|
Concept ACK. I didn't know about this PR when I opened #16780. Don't override the existing field though. You could make the output more compact by presenting it as a string: If people still find that too much visual clutter, especially in the already quite long output of |
|
Reopening since @sdaftuar and @Sjors concept ACKed. Here is a truncated output of And one of |
|
Reopened in a new PR (#16787) since I force pushed before clicking reopen.... |
66740f4 doc: add a release note for the new field in 'getpeerinfo' and 'getnetworkinfo' (darosior) 6564f58 rpc/net: decode the services flags in a new entry (darosior) Pull request description: This is a reopen of #15511 (comment) since there have been concept ACKs from sdaftuar and Sjors. This adds a new entry to `getpeerinfo` and `getnetworkinfo` which decodes the network services flags. Here is a truncated output of `getpeerinfo`: ``` "services": "000000000000040d", "servicesnames": "NODE_NETWORK | NODE_BLOOM | NODE_WITNESS | NODE_NETWORK_LIMITED", "relaytxes": true, ``` And one of `getnetworkinfo`: ``` "localservices": "0000000000000409", "localservicesnames": "NODE_NETWORK | NODE_WITNESS | NODE_NETWORK_LIMITED", "localrelay": true, ``` Fixes #16780. ACKs for top commit: MarcoFalke: unsigned ACK 66740f4 laanwj: ACK 66740f4 Tree-SHA512: 0acc37134b283f56004a41243903d7790cb01591ddf0342489bd05f3a2c780563075373ba5fd55180fa15632e8968ffa11a979b8afece75a6a2e891342601440
The commandsgetpeerinfoandgetnetworkinforeturns the services a node provides in the form of a flag (in the fieldservices). This PR adds a fieldservicesnames(resp.localservicesnames) which contains the names of these services so that a user does not have to parse the flag to know which services his node provides.EDIT: see #15511 (comment)