rpc: Human readable network services #16787

pull darosior wants to merge 2 commits into bitcoin:master from darosior:services_for_humans changing 4 files +39 −2
  1. darosior commented at 10:11 AM on September 2, 2019: member

    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": "NETWORK | BLOOM | WITNESS | NETWORK_LIMITED",
"relaytxes": true,

    And one of getnetworkinfo:

    "localservices": "0000000000000409",
"localservicesnames": "NETWORK | WITNESS | NETWORK_LIMITED",
"localrelay": true,

    Fixes #16780.

  2. darosior force-pushed on Sep 2, 2019
  3. darosior cross-referenced this on Sep 2, 2019 from issue RPC : More user-friendly services field in getnetworkinfo and getpeerinfo by darosior
  4. fanquake added the label RPC/REST/ZMQ on Sep 2, 2019
  5. fanquake renamed this:
    Human readable network services
    rpc: Human readable network services
    on Sep 2, 2019
  6. laanwj commented at 11:58 AM on September 2, 2019: member

    Concept ACK, though to avoid clients having to do additional string parsing, i'd slightly prefer a list instead of one |-separated string. E.g.

    "localservicesnames": ["NODE_NETWORK", "NODE_WITNESS", "NODE_NETWORK_LIMITED"]

    or leave out the NODE_ as it's redundant,

    "localservicesnames": ["NETWORK", "WITNESS", "NETWORK_LIMITED"]
  7. sipa commented at 4:15 PM on September 2, 2019: member

    I prefer @laanwj's version as well.

  8. darosior force-pushed on Sep 2, 2019
  9. darosior commented at 7:39 PM on September 2, 2019: member

    Updated to use a list instead of a | -separated string and to remove the redundant NODE_ prefix. Will add CHANGELOG too.

  10. fanquake commented at 1:51 AM on September 3, 2019: member

    Concept ACK - looks like multiple developers have said that this would be a worthwhile inclusion. Thanks for writing release notes as well.

    master (33f9750b1b86a705d092b0e1314ed15287c45239):

        "services": "000000000000040d",
    "relaytxes": true,

    This PR (0774c03056a1722fe15b8b0926bf040c8932c7c3):

    src/bitcoin-cli getpeerinfo | grep -i 'services' -A 7
    "services": "000000000000040d",
    "servicesnames": [
      "NETWORK",
      "BLOOM",
      "WITNESS",
      "NETWORK_LIMITED"
    ],
    "relaytxes": true,
--
    "services": "000000000000000d",
    "servicesnames": [
      "NETWORK",
      "BLOOM",
      "WITNESS"
    ],
    "relaytxes": true,
    "lastsend": 1567474969,
--
    "services": "000000000000040d",
    "servicesnames": [
      "NETWORK",
      "BLOOM",
      "WITNESS",
      "NETWORK_LIMITED"
    ],
    "relaytxes": true,
  11. fanquake requested review from sdaftuar on Sep 3, 2019
  12. fanquake requested review from Sjors on Sep 3, 2019
  13. practicalswift commented at 12:02 PM on September 3, 2019: contributor

    Concept ACK -- nice usability improvement!

  14. in src/rpc/net.cpp:85 in 0774c03056 outdated 
      81 | @@ -82,6 +82,10 @@ static UniValue getpeerinfo(const JSONRPCRequest& request)
  82 |              "    \"addrbind\":\"ip:port\",    (string) Bind address of the connection to the peer\n"
  83 |              "    \"addrlocal\":\"ip:port\",   (string) Local address as reported by the peer\n"
  84 |              "    \"services\":\"xxxxxxxxxxxxxxxx\",   (string) The services offered\n"
  85 | +            "    \"servicesnames\":[              (array) the services offered, in human-readable form\n"
    MarcoFalke commented at 12:59 PM on September 3, 2019:

    Should mention that unrecognised services are not shown

    darosior commented at 1:36 PM on September 3, 2019:

    Fixed

  15. in src/rpc/net.cpp:165 in 0774c03056 outdated 
     160 | +            servicesNames.push_back("BLOOM");
 161 | +        if (stats.nServices & NODE_WITNESS)
 162 | +            servicesNames.push_back("WITNESS");
 163 | +        if (stats.nServices & NODE_NETWORK_LIMITED)
 164 | +            servicesNames.push_back("NETWORK_LIMITED");
 165 | +        obj.pushKV("servicesnames", servicesNames);
    MarcoFalke commented at 1:01 PM on September 3, 2019:

    Couldn't this be in a function, so that only one place has to be updated in the future?

            obj.pushKV("servicesnames", GetServicesNames(stats.nServices));
    darosior commented at 1:36 PM on September 3, 2019:

    Fixed

  16. MarcoFalke commented at 1:01 PM on September 3, 2019: member

    a documentation nit

  17. darosior force-pushed on Sep 3, 2019
  18. darosior commented at 1:38 PM on September 3, 2019: member

    Rebased to point in the help that only known services are decoded, and to group the decoding into one function as suggested by @MarcoFalke

  19. DrahtBot commented at 1:57 PM on September 3, 2019: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #16839 (Replace Connman and BanMan globals with Node local by ryanofsky)
    • #16411 (Signet support by kallewoof)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

  20. laanwj commented at 8:08 PM on September 3, 2019: member

    Code review ACK 1e29f044dd3580f53d44b8293893c287100aa48d

  21. in src/rpc/net.cpp:454 in 1e29f044dd outdated 
     450 | @@ -446,6 +451,10 @@ static UniValue getnetworkinfo(const JSONRPCRequest& request)
 451 |              "  \"subversion\": \"/Satoshi:x.x.x/\",     (string) the server subversion string\n"
 452 |              "  \"protocolversion\": xxxxx,              (numeric) the protocol version\n"
 453 |              "  \"localservices\": \"xxxxxxxxxxxxxxxx\", (string) the services we offer to the network\n"
 454 | +            "  \"localservicesnames\": [              (array) the known services we offer to the network, in human-readable form\n"
    MarcoFalke commented at 11:36 PM on September 3, 2019:

    unknown services can not be offered, so mentioning it here will cause confusion

  22. in src/rpc/net.cpp:496 in 1e29f044dd outdated 
     492 | @@ -484,8 +493,11 @@ static UniValue getnetworkinfo(const JSONRPCRequest& request)
 493 |      obj.pushKV("version",       CLIENT_VERSION);
 494 |      obj.pushKV("subversion",    strSubVersion);
 495 |      obj.pushKV("protocolversion",PROTOCOL_VERSION);
 496 | -    if(g_connman)
 497 | -        obj.pushKV("localservices", strprintf("%016x", g_connman->GetLocalServices()));
 498 | +    if(g_connman) {
    MarcoFalke commented at 11:37 PM on September 3, 2019: 
        if (g_connman) {
  23. darosior force-pushed on Sep 4, 2019
  24. darosior commented at 2:58 PM on September 4, 2019: member

    (Travis is failing for mysterious reasons but the build is passing on https://bitcoinbuilds.org/?build=1484 fwiw :-) )

  25. in doc/release-notes-16787.md:3 in 10adfa5a76 outdated 
       0 | @@ -0,0 +1,3 @@
   1 | +RPC changes
   2 | +-----------
   3 | +The `getnetworkinfo` and `getpeerinfo` commands now contains a new field with decoded network service flags.
    mzumsande commented at 11:03 PM on September 4, 2019:

    nit: contain

  26. in src/rpc/net.cpp:455 in 2443d32650 outdated 
     450 | @@ -446,6 +451,10 @@ static UniValue getnetworkinfo(const JSONRPCRequest& request)
 451 |              "  \"subversion\": \"/Satoshi:x.x.x/\",     (string) the server subversion string\n"
 452 |              "  \"protocolversion\": xxxxx,              (numeric) the protocol version\n"
 453 |              "  \"localservices\": \"xxxxxxxxxxxxxxxx\", (string) the services we offer to the network\n"
 454 | +            "  \"localservicesnames\": [              (array) the services we offer to the network, in human-readable form\n"
 455 | +            "      \"SERVICE_NAME\",         (string) the service name if it is recognised\n"
    mzumsande commented at 11:07 PM on September 4, 2019:

    nit: could put some more whitespace before "(string)" for optics

    MarcoFalke commented at 11:50 PM on September 4, 2019:

    If you need to change this for other reasons, might as well remove the "if it is recognised". If we offer a service, we better recognize it.

                "      \"SERVICE_NAME\",         (string) the service name\n"
    darosior commented at 11:16 AM on September 5, 2019:

    Ah, I copied it from getpeerinfo ...

    darosior commented at 1:12 PM on September 5, 2019:

    fixed

    darosior commented at 1:13 PM on September 5, 2019:

    Fixed

  27. mzumsande commented at 11:33 PM on September 4, 2019: contributor

    ACK 10adfa5a769b0f47858bc77f2641c691e9b80c52. Reviewed code, ran tests and tried it out on mainnet - works as expected. Feel free to ignore nits if you don't change something else.

  28. laanwj commented at 11:42 AM on September 5, 2019: member

    (Travis is failing for mysterious reasons but the build is passing on https://bitcoinbuilds.org/?build=1484 fwiw :-) )

    yeahh Travis is having infrastructure issues again (I think), restarted the failing jobs …

  29. rpc/net: decode the services flags in a new entry 6564f58c87
  30. doc: add a release note for the new field in 'getpeerinfo' and 'getnetworkinfo' 66740f460a
  31. darosior force-pushed on Sep 5, 2019
  32. darosior commented at 1:26 PM on September 5, 2019: member

    Rebased to fix the last doc nits (and hopefully make Travis happy).

  33. MarcoFalke commented at 5:48 PM on September 5, 2019: member

    unsigned ACK 66740f460af5f9d8c61eb5b154863bffb20d94b5

  34. in src/rpc/util.cpp:737 in 66740f460a 
     732 | @@ -733,3 +733,21 @@ std::vector<CScript> EvalDescriptorStringOrObject(const UniValue& scanobject, Fl
 733 |      }
 734 |      return ret;
 735 |  }
 736 | +
 737 | +UniValue GetServicesNames(ServiceFlags services)
    promag commented at 11:23 PM on September 5, 2019:

    Should also #include <protocol.h>

    darosior commented at 8:07 AM on September 6, 2019:

    It is included. https://github.com/bitcoin/bitcoin/pull/16787/files

  35. in src/rpc/util.cpp:739 in 66740f460a 
     732 | @@ -733,3 +733,21 @@ std::vector<CScript> EvalDescriptorStringOrObject(const UniValue& scanobject, Fl
 733 |      }
 734 |      return ret;
 735 |  }
 736 | +
 737 | +UniValue GetServicesNames(ServiceFlags services)
 738 | +{
 739 | +    UniValue servicesNames(UniValue::VARR);
    promag commented at 11:24 PM on September 5, 2019:

    nit, s/servicesNames/names or s/servicesNames/res.

  36. in src/rpc/util.cpp:742 in 66740f460a 
     737 | +UniValue GetServicesNames(ServiceFlags services)
 738 | +{
 739 | +    UniValue servicesNames(UniValue::VARR);
 740 | +
 741 | +    if (services & NODE_NETWORK)
 742 | +        servicesNames.push_back("NETWORK");
    promag commented at 11:26 PM on September 5, 2019:

    nit, same line as if?

  37. in src/rpc/net.cpp:455 in 66740f460a 
     450 | @@ -446,6 +451,10 @@ static UniValue getnetworkinfo(const JSONRPCRequest& request)
 451 |              "  \"subversion\": \"/Satoshi:x.x.x/\",     (string) the server subversion string\n"
 452 |              "  \"protocolversion\": xxxxx,              (numeric) the protocol version\n"
 453 |              "  \"localservices\": \"xxxxxxxxxxxxxxxx\", (string) the services we offer to the network\n"
 454 | +            "  \"localservicesnames\": [                (array) the services we offer to the network, in human-readable form\n"
 455 | +            "      \"SERVICE_NAME\",                    (string) the service name\n"
    promag commented at 11:27 PM on September 5, 2019:

    if it is recognised?

    darosior commented at 8:08 AM on September 6, 2019:

    As mentioned above, getnetworkinfo indicates service we offer : we'd better recognise it

  38. promag commented at 11:28 PM on September 5, 2019: member

    Should have tests for these new fields?

  39. laanwj commented at 7:02 AM on September 10, 2019: member

    Works for me ACK 66740f460af5f9d8c61eb5b154863bffb20d94b5 Please add a (functional) tests for this feature in a next PR

  40. laanwj referenced this in commit 33c466a642 on Sep 10, 2019
  41. laanwj merged this on Sep 10, 2019
  42. laanwj closed this on Sep 10, 2019
  43. MarcoFalke cross-referenced this on Sep 10, 2019 from issue test: Add smoke test for getpeerinfo['servicesnames'] by MarcoFalke
  44. darosior deleted the branch on Sep 10, 2019
  45. darosior commented at 7:13 PM on September 10, 2019: member

    Please add a (functional) tests for this feature in a next PR

    Will do

  46. darosior cross-referenced this on Sep 10, 2019 from issue test: `servicesnames` field in `getpeerinfo` and `getnetworkinfo` by darosior
  47. laanwj referenced this in commit 04d9939f46 on Sep 12, 2019
  48. tynes cross-referenced this on Sep 20, 2019 from issue RPC: Expose service names by tynes
  49. luke-jr referenced this in commit 8971be7dc8 on Sep 21, 2019
  50. tynes cross-referenced this on Sep 25, 2019 from issue http: expose human readable service names by tynes
  51. sickpig referenced this in commit 922cab0e99 on Jan 20, 2020
  52. sickpig cross-referenced this on Jan 20, 2020 from issue [port][rpc] Add getchaintxstats and uptime RPC calls by sickpig
  53. sickpig referenced this in commit 91d9bf998d on Jan 22, 2020
  54. sickpig referenced this in commit 716432bba5 on Jan 31, 2020
  55. sickpig referenced this in commit ff34445b4f on Feb 6, 2020
  56. sickpig referenced this in commit 38ade11f9a on Feb 7, 2020
  57. sickpig referenced this in commit c787d89dec on Feb 10, 2020
  58. sickpig referenced this in commit e7f15db5fc on Feb 11, 2020
  59. gandrewstone referenced this in commit 6403cd77c4 on Feb 12, 2020
  60. jasonbcox referenced this in commit e5b0e658a2 on Oct 16, 2020
  61. kwvg referenced this in commit 1dbf0257e7 on Aug 22, 2021
  62. kwvg referenced this in commit 3c2b35368d on Aug 22, 2021
  63. kwvg referenced this in commit c3552eb57e on Sep 16, 2021
  64. kwvg referenced this in commit 58340cef63 on Sep 18, 2021
  65. UdjinM6 referenced this in commit 4ffd42de63 on Sep 21, 2021
  66. thelazier referenced this in commit 948a9e12a1 on Sep 25, 2021
  67. kwvg referenced this in commit 7c0326161a on Oct 12, 2021
  68. bitcoin locked this on Dec 16, 2021
Contributors
darosiorlaanwjsipafanquakepracticalswiftMarcoFalkeDrahtBotmzumsandepromag


Sjorssdaftuar

Labels
RPC/REST/ZMQ
Linked (view graph)
#15511 RPC : More user-friendly services field in getnetworkinfo and getpeerinfo#16780 Human readable localservices for getnetworkinfo#16844 test: Add smoke test for getpeerinfo['servicesnames']#16850 test: `servicesnames` field in `getpeerinfo` and `getnetworkinfo`
github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-05-20 06:54 UTC