cli: return local services in -netinfo #31886

pull jonatack wants to merge 2 commits into bitcoin:master from jonatack:2025-02-netinfo-services changing 1 files +15 −1
  1. jonatack commented at 6:31 pm on February 16, 2025: member

    Add local services info to -netinfo dashboard that already provides this info for each of the peer connections.

    • bitcoin-cli -netinfo with no args passed provides a nice easy-to-understand services list:
    0Bitcoin Core client v28.99.0 - server 70016/Satoshi:28.99.0/
    1
    2         ipv4    ipv6   onion     i2p   cjdns   total   block  manual
    3in          0       0      12       8       0      20
    4out         6       0       4       3       2      15       3       4
    5total       6       0      16      11       2      35
    6
    7Local services: network, bloom, witness, compact filters, network limited, p2p v2
    8
    9Local addresses
    
    • With a details level passed, e.g. -netinfo 3, print the services in the versions header instead (to avoid adding a line for more static information), in the same format as the peers list (see -netinfo help for info on the output of the serv column):
     0Bitcoin Core client v28.99.0 - server 70016/Satoshi:28.99.0/ - services nbwcl2
     1
     2<->   type   net   serv  v  mping   ping send recv  txn  blk  hb addrp addrl  age  asmap  id version
     3 in        onion         1    283    498   48   48    *              .         77        388 70016
     4 in        onion   nwl2  2    318    485    5  111                             79        372 70016/Satoshi:28.0.0/
     5 in        onion    nwl  1    342    344    4    1   53             96         84        344 70016/Satoshi:26.0.0/
     6 in        onion    nwl  1    411    601    4    1   35            124         85        339 70016/Satoshi:26.0.0/
     7 in        onion  nwcl2  2    436   4330    2    2    2             31         13        623 70016/Satoshi:28.0.0/
     8 in        onion    wl2  2    445    503    4    4    6            138         81        363 70016/Satoshi:28.0.0/
     9 in        onion    nwl  1    462    726    4    1   56             92         81        365 70016/Satoshi:23.0.0/
    10 in        onion    nwl  1    500    765    4    1   34             94         83        351 70016/Satoshi:25.0.0/
    11 in        onion   nwl2  2    578    684    4    0    1            134         87        327 70016/Satoshi:28.0.0/
    12 in          i2p   nwl2  2    712   1322    4    2   35            204     1   93        308 70016/Satoshi:27.2.0/
    13 in        onion   nwl2  2    727    873    5    5   56            162         85        342 70016/Satoshi:27.1.0/
    14 in          i2p   nwl2  2    749    976    4    2   25            120         72        408 70016/Satoshi:27.1.0/
    15 in          i2p   nwl2  2    776    954    4    1    0             72         68        426 70016/Satoshi:28.0.0/
    16 in          i2p   nbwl  1    883   1735    4    4                  53         34        551 70016/Satoshi:26.0.0/
    17 in          i2p  nwcl2  2    920   1044    2    0    0            131         83        350 70016/Satoshi:28.0.0/
    18 in        onion     wl  1   1021  20832   29   67                   3         49        501 70016/Satoshi:23.0.0/
    19 in          i2p  nwcl2  2   1830   1830    5    0                   3          3        668 70016/Satoshi:27.1.0/
    20 in        onion    nwl  1  41155  41155   87  204                              4        658 70016/Satoshi:25.0.0/
    21out   full  ipv4   nwl2  2     74     93    0    0    0           1028         85   1221 338 70016/Satoshi:27.1.0/
    22out   full  ipv4    nwl  1     82    104    0    2    0    5  .   1076         95  13536 301 70016/Satoshi:26.0.0/
    23out   full  ipv4    nwl  1    147    178    2    2    0   28  .   1104         95 395570 300 70016/Satoshi:25.0.0/
    24out  block  ipv4   nwl2  2    166    513    2    2    *              .         88  38001 324 70016/Satoshi:27.2.0/
    25out   full  ipv4     wl  1    193    201    0    4    0           1035         94  31376 307 70016/Satoshi:25.99.0/
    26out   full  ipv4   nwl2  2    199    796    1    1    0           1027         94   9723 304 70016/Satoshi:27.2.0/
    27out manual cjdns   nwl2  2    213    235    1    9    0           1109         83        353 70016/Satoshi:28.99.0/
    28out   full onion   nbwl  1    282    457    3    3    1           1130         73        404 70016/Satoshi:25.0.0/
    29out  block onion   nbwl  1    324    353   23   23    *              .         85        341 70016/Satoshi:26.0.0/
    30out manual cjdns   nwl2  2    340    445    1    1    7           1059         82        361 70016/Satoshi:27.0.0/
    31out manual onion    wl2  2    386    386    1    1    1           1048         84        345 70016/Satoshi:28.99.0/
    32out manual   i2p  nwcl2  2    697   1084    1    1    8           1113     3   93        310 70016/Satoshi:27.0.0/
    33out   full   i2p  nwcl2  2    730   1254    1    9    0           1128         89        318 70016/Satoshi:28.0.0/
    34out   full   i2p  nwcl2  2    765   1804    1    1    1           1132         72        409 70016/Satoshi:28.0.0/
    35                               ms     ms  sec  sec  min  min                  min
    36
    37         ipv4    ipv6   onion     i2p   cjdns   total   block  manual
    38in          0       0      12       6       0      18
    39out         6       0       3       3       2      14       2       4
    40total       6       0      15       9       2      32
    41
    42Local addresses
    
  2. netinfo: return local services in the default report 3851960576
  3. netinfo: return shortened services, if peers list requested
    When the detailed peers list is requested, return the shortened services in the
    -netinfo header in the same format as the "serv" column, instead of the full names
    list in the report.
    724546e28a
  4. DrahtBot commented at 6:31 pm on February 16, 2025: contributor

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

    Code Coverage & Benchmarks

    For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/31886.

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    ACK zaidmstrr, danielabrozzoni
    Concept ACK l0rinc

    If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

  5. DrahtBot added the label Scripts and tools on Feb 16, 2025
  6. zaidmstrr commented at 6:18 pm on February 20, 2025: contributor
    Code ACK 724546e I tested the changes on Ubuntu 24.04.2, and it’s working fine. I also manually reviewed the changed code for any logical errors. Although it’s very beneficial to check the node advertising services in a quick way.
  7. luke-jr referenced this in commit 4ffdf3acae on Feb 26, 2025
  8. luke-jr referenced this in commit b8356b8502 on Feb 26, 2025
  9. laanwj commented at 2:18 pm on March 3, 2025: member
    Seems fine to add this, though it also takes up a line for information that is always the same for the same node, dynamic status information is the most interesting to show in -*info calls.
  10. jonatack commented at 8:26 pm on March 3, 2025: member

    Thanks for taking a look!

    Seems fine to add this, though it also takes up a line for information

    Right, when calling -netinfo without a details level (without the peers list).

    I began by adding the services in the versions line, in the same format as in the peers list:

    0Bitcoin Core client v28.99.0 - server 70016/Satoshi:28.99.0/ - services nbwcl2
    1
    2<->   type   net   serv  v  mping   ping send recv  txn  blk  hb addrp addrl  age  asmap  id version
    3 in        onion   nwl2  2    318    485    5  111                             79        372 70016/Satoshi:28.0.0/
    4 in        onion    nwl  1    342    344    4    1   53             96         84        344 70016/Satoshi:26.0.0/
    5 in        onion    nwl  1    411    601    4    1   35            124         85        339 70016/Satoshi:26.0.0
    

    Then, when calling -netinfo without a details level, I thought it lacked context or looked a bit cryptic, so for that case I replaced it with the extra line that prints the services in an easy-to-read manner, which seems nice. Nevertheless, I could revert that change if you prefer it remain appended to the versions header, WDYT?

  11. laanwj commented at 9:11 pm on March 3, 2025: member

    Nevertheless, I could revert that change if you prefer it remain appended to the versions header, WDYT?

    i agree an alphabet soup like “nbwcl2” does not say anything without a legend what every character means.

    To be clear, i’m not against adding it, it’s only one line, it was more of a general concern about the direction.

  12. jonatack commented at 9:15 pm on March 3, 2025: member
    I’ve updated the pull description to be clearer about it.
  13. danielabrozzoni commented at 10:25 am on April 7, 2025: contributor

    Code Review ACK 724546e28a5778e1c3f5100406ed7237f76aab55

    This PR adds the “Local services” line to -netinfo, when called without details:

    When -netinfo is called with details, services are print in the header line:

    Adding service info to -netinfo improves usability, and the code looks good to me.

  14. in src/bitcoin-cli.cpp:484 in 724546e28a
    480@@ -481,6 +481,16 @@ class NetinfoRequestHandler : public BaseRequestHandler
    481         }
    482         return str;
    483     }
    484+    std::string ServicesList(const UniValue& services)
    


    l0rinc commented at 10:11 am on April 28, 2025:

    The new additions don’t seem to be covered by any tests https://corecheck.dev/bitcoin/bitcoin/pulls/31886

    Actually the whole file is a blood-bath: https://maflcko.github.io/b-c-cov/total.coverage/src/bitcoin-cli.cpp.gcov.html

    But since the following functional tests seem to cover this file:

    • interface_bitcoin_cli.py
    • interface_rpc.py
    • rpc_deriveaddresses.py
    • rpc_generate.py
    • tool_signet_miner.py
    • wallet_createwallet.py
    • wallet_descriptor.py
    • wallet_importdescriptors.py
    • wallet_listreceivedby.py
    • wallet_multiwallet.py

    And interface_bitcoin_cli.py seems the closest, we could test the two scenarios like:

     0diff --git a/test/functional/interface_bitcoin_cli.py b/test/functional/interface_bitcoin_cli.py
     1index 2618c12e9f..17b27855c3 100755
     2--- a/test/functional/interface_bitcoin_cli.py
     3+++ b/test/functional/interface_bitcoin_cli.py
     4@@ -83,6 +83,18 @@ class TestBitcoinCli(BitcoinTestFramework):
     5     def skip_test_if_missing_module(self):
     6         self.skip_if_no_cli()
     7 
     8+    def test_netinfo_and_version_cli(self):
     9+        """Test -netinfo output format."""
    10+        self.log.info("Test -netinfo with header + separate local services line")
    11+        out = self.nodes[0].cli('-netinfo').send_cli().splitlines()
    12+        assert out[0].startswith("Bitcoin Core client ")
    13+        assert any(re.match(r"^Local services:.+network", line) for line in out)
    14+
    15+        self.log.info("Test -netinfo header includes services line")
    16+        det = self.nodes[0].cli('-netinfo', '3').send_cli().splitlines()
    17+        assert re.match(r"^Bitcoin Core client.+services nwl$", det[0])
    18+        assert not any(line.startswith("Local services:") for line in det)
    19+
    20     def run_test(self):
    21         """Main test logic"""
    22         self.generate(self.nodes[0], BLOCKS)
    23@@ -379,6 +391,8 @@ class TestBitcoinCli(BitcoinTestFramework):
    24             self.log.info("*** Wallet not compiled; cli getwalletinfo and -getinfo wallet tests skipped")
    25             self.generate(self.nodes[0], 25)  # maintain block parity with the wallet_compiled conditional branch
    26 
    27+        self.test_netinfo_and_version_cli()
    28+
    29         self.log.info("Test -version with node stopped")
    30         self.stop_node(0)
    31         cli_response = self.nodes[0].cli('-version').send_cli()
    
  15. in src/bitcoin-cli.cpp:493 in 724546e28a
    488+            std::string s{ToLower((services[i].get_str()))};
    489+            std::ranges::replace(s, '_', ' ');
    490+            v.push_back(s);
    491+        }
    492+        return Join(v, ", ");
    493+    }
    


    l0rinc commented at 11:13 am on April 28, 2025:

    We’re currently getting each string, recreating it as lowercase, iterating it again for potential replacement, adding it to a vector, which we’re recreating at the end again as a string.

    We could refactor it slightly to be similar to FormatServices, by doing the lowercase and replace operations once on the final joined string instead of processing each string individually:

    0    static std::string ServicesList(const UniValue& services)
    1    {
    2        std::string res{services.size() ? services[0].get_str() : ""};
    3        for (size_t i{1}; i < services.size(); ++i) res += ", " + services[i].get_str();
    4        for (auto& c: res) c = (c == '_' ? ' ' : ToLower(c));
    5        return res;
    6    }
    

    Which constructs the final string directly, sanitizing it in one separate step.

    Or if you prefer the sorted alternative I suggested above:

    0    static std::string ServicesList(const UniValue& services)
    1    {
    2        std::set<std::string> sorted;
    3        for (auto& s :  services.getValues()) sorted.emplace(s.get_str());
    4        auto res{Join(sorted, ", ")};
    5        for (auto& c : res) c = (c == '_' ? ' ' : ToLower(c));
    6        return res;
    7    }
    

    But if you decide to keep the original, consider the following nits:

    Nit - extra parenthesis:

    0std::string s{ToLower(services[i].get_str())};
    

    Nit - consistent brace init:

    0for (size_t i{0}; i < services.size(); ++i) {
    

    Nit - can be made static:

    0static std::string ServicesList(const UniValue& services)
    

    Nit - we can iterate the values directly:

    0for (auto& s : services.getValues())
    
  16. in src/bitcoin-cli.cpp:487 in 724546e28a
    480@@ -481,6 +481,16 @@ class NetinfoRequestHandler : public BaseRequestHandler
    481         }
    482         return str;
    483     }
    484+    std::string ServicesList(const UniValue& services)
    485+    {
    486+        std::vector<std::string> v;
    487+        for (size_t i = 0; i < services.size(); ++i) {
    


    l0rinc commented at 12:04 pm on April 28, 2025:

    should we maybe sort to avoid an order like:

    0Local services: network, witness, network limited, p2p v2
    

    it may make sense to have:

    0Local services: network, network limited, p2p v2, witness
    
  17. l0rinc changes_requested
  18. l0rinc commented at 12:33 pm on April 28, 2025: contributor

    Concept ACK

    I have testes it manually, seems to work correctly, but I would encourage the author to cover it with some simple functional tests and to consider sorting the values to make sure similar values end up closer together

  19. danielabrozzoni commented at 11:28 am on July 11, 2025: contributor
    Hey @jonatack, I noticed there are still a couple of comments open. Let me know if you’d like any help addressing them :)

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: 2025-07-15 18:13 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me