RPC : More user-friendly services field in getnetworkinfo and getpeerinfo #15511

1. 
   darosior commented at 3:24 pm on March 1, 2019: member

   The commands getpeerinfo and getnetworkinfo returns the services a node provides in the form of a flag (in the field services). This PR adds a field servicesnames (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)
2. fanquake added the label RPC/REST/ZMQ on Mar 1, 2019
3. 
   gmaxwell commented at 3:27 pm on March 1, 2019: contributor

   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.
4. 
   darosior commented at 3:38 pm on March 1, 2019: member

   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).
   Unknown values just won’t pass the conditions and would not be returned, is it an issue ?
5. 
   kristapsk commented at 3:42 pm on March 1, 2019: contributor

   NACK, there could be some software that expects output of getnetworkinfo RPC as it is now. This will break compatibility. User-friendly services field could be added, but then as a new field, not replacing existing one.
6. 
   darosior commented at 4:00 pm on March 1, 2019: member

   Indeed.
7. darosior force-pushed on Mar 1, 2019
8. Adds a field 'servicesnames' in getpeerinfo and getnetworkinfo to output services names 309102fff9
9. darosior force-pushed on Mar 1, 2019
10. 
    darosior commented at 4:11 pm on March 1, 2019: member

    Changed the behavior to add a new field instead of modifying the existent one. Edited the description.
11. 
    gmaxwell commented at 11:04 pm on March 1, 2019: contributor

    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?

    Unknown values just won’t pass the conditions and would not be returned, is it an issue ?

    Yes, field is a lot more useful for unknown values– e.g. reasoning about new or incompatible software– than it is for known ones.

12. 
    darosior commented at 11:24 am on March 2, 2019: member

    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?

    Actually, I made this after helping the guy asking about NODE_GETUTXO on the thread your responded to on bitcointalk. I thought that other node owners would wonder which services their node supports. I understand that I may be wrong and this may just be useless, I close it for now.

13. darosior closed this on Mar 2, 2019

    ---

14. 
    sdaftuar commented at 4:44 pm on March 2, 2019: member

    For what it’s worth, I would concept ACK something like this – the times I use getpeerinfo tend to be times I’m debugging something, and having the service bits be more easy to decode would be helpful to me.
15. 
    Sjors commented at 11:44 am on September 1, 2019: member

    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: NODE_NETWORK | NODE_BLOOM.

    If people still find that too much visual clutter, especially in the already quite long output of getpeerinfo, you could add a debug param. If you go that route, then I suggest returning a dictionary with all possible flags and a true / false bool.

16. 
    darosior commented at 10:01 am on September 2, 2019: member

    Reopening since @sdaftuar and @Sjors concept ACKed. Rebased and modified, as proposed in #15511 (comment), the array to a string (this introduces some temporary variables but the output is much nicer) : since it only adds one line I think it doesnt worth a command argument addition.

    Here is a truncated output of getpeerinfo:

    0"services": "000000000000040d",
    1"servicesnames": "NODE_NETWORK | NODE_BLOOM | NODE_WITNESS | NODE_NETWORK_LIMITED",
    2"relaytxes": true,
    

    And one of getnetworkinfo:

    0"protocolversion": 70015,
    1"localservices": "0000000000000409",
    2"localservicesnames": "NODE_NETWORK | NODE_WITNESS | NODE_NETWORK_LIMITED",
    3"localrelay": true,
    

17. 
    darosior commented at 10:13 am on September 2, 2019: member

    Reopened in a new PR (#16787) since I force pushed before clicking reopen….
18. laanwj referenced this in commit 33c466a642 on Sep 10, 2019
19. MarcoFalke locked this on Dec 16, 2021

Contributors
darosior gmaxwell kristapsk sdaftuar Sjors

Labels
RPC/REST/ZMQ