rpc: Human readable network services #16787

darosior · 2019-09-02T10:11:46Z

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.

laanwj · 2019-09-02T11:58:11Z

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"]

sipa · 2019-09-02T16:15:45Z

I prefer @laanwj's version as well.

darosior · 2019-09-02T19:39:53Z

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

fanquake

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

master (33f9750):

    "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,

practicalswift · 2019-09-03T12:02:54Z

Concept ACK -- nice usability improvement!

maflcko

a documentation nit

src/rpc/net.cpp

darosior · 2019-09-03T13:38:40Z

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

DrahtBot · 2019-09-03T13:57:11Z

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

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.

laanwj · 2019-09-03T20:08:23Z

Code review ACK 1e29f044dd3580f53d44b8293893c287100aa48d

src/rpc/net.cpp

darosior · 2019-09-04T14:58:22Z

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

mzumsande

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.

doc/release-notes-16787.md

src/rpc/net.cpp

laanwj · 2019-09-05T11:42:24Z

(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 …

…tworkinfo'

darosior · 2019-09-05T13:26:29Z

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

maflcko · 2019-09-05T17:48:12Z

unsigned ACK 66740f4

promag

Should have tests for these new fields?

src/rpc/util.cpp

src/rpc/net.cpp

laanwj · 2019-09-10T07:02:50Z

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

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

darosior · 2019-09-10T19:13:10Z

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

Will do

…tworkinfo` 1d524c6 tests: rename 'test_getnetworkinginfo' in 'test_getnetworkinfo' (darosior) 07a8f65 tests: add a test for the 'servicesnames' RPC field (darosior) Pull request description: As per #16787 (comment), fixes #16844. This adds a test for both commands in the first commit and renames the test for `getnetworkinfo` in the second commit. ACKs for top commit: laanwj: ACK 1d524c6 Tree-SHA512: 8267dce4d54356debab75014e6f9ba885b892da605ed32f26a5446c232992fcae761861bb678adbdb942815d4706f3768c70deee6afec68f219b23605475be01

Github-Pull: bitcoin#16787 Rebased-From: 6564f58

This is a backport of bitcoin/bitcoin/pull/16787

* Update travis.yml according to travis backend errors and warns Namely: - root: deprecated key sudo (The key `sudo` has no effect anymore.) - language: value minimal is an alias for shell, using shell - root: key matrix is an alias for jobs, using jobs * Reduce time threshold to force travis to save cache and abort * Add getchaintxstats RPC this is a port of core #9733 * [RPC] Add an uptime command that displays the amount of time that bitcoind has been running this is a port of Core #10400 * rpc/net: decode the services flags in a new entry This is a backport of bitcoin/bitcoin/pull/16787 Co-authored-by: Pieter Wuille <[email protected]> Co-authored-by: Ricardo Velhote <[email protected]> Co-authored-by: Darosior <[email protected]>

Summary: This adds human readable network services to the output of `getpeerinfo` and `getnetworkinfo` Backport of Core [[bitcoin/bitcoin#16787 | PR16787]] Test Plan: Run `src/bitcoin-cli getnetworkinfo` and check the `localservicesnames` field. More tests coming in PR16850 Reviewers: O1 Bitcoin ABC, #bitcoin_abc, Fabien Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, Fabien Differential Revision: https://reviews.bitcoinabc.org/D7948

merge bitcoin#16787, bitcoin#17474, bitcoin#17687, bitcoin#18165, bitcoin#18877, bitcoin#18960, bitcoin#19010, bitcoin#19044, bitcoin#19070: block filters

darosior force-pushed the services_for_humans branch from 42fd0ca to 6e47bf2 Compare September 2, 2019 10:13

darosior mentioned this pull request Sep 2, 2019

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

Closed

fanquake added the RPC/REST/ZMQ label Sep 2, 2019

fanquake changed the title ~~Human readable network services~~ rpc: Human readable network services Sep 2, 2019

darosior force-pushed the services_for_humans branch from 6e47bf2 to fcbbb7e Compare September 2, 2019 19:31

fanquake reviewed Sep 3, 2019

View reviewed changes

fanquake requested review from Sjors and sdaftuar September 3, 2019 01:51

maflcko reviewed Sep 3, 2019

View reviewed changes

src/rpc/net.cpp Outdated Show resolved Hide resolved

src/rpc/net.cpp Outdated Show resolved Hide resolved

darosior force-pushed the services_for_humans branch from 0774c03 to 1e29f04 Compare September 3, 2019 13:37

maflcko reviewed Sep 3, 2019

View reviewed changes

src/rpc/net.cpp Outdated Show resolved Hide resolved

maflcko reviewed Sep 3, 2019

View reviewed changes

src/rpc/net.cpp Outdated Show resolved Hide resolved

darosior force-pushed the services_for_humans branch from 1e29f04 to 10adfa5 Compare September 4, 2019 09:08

mzumsande reviewed Sep 4, 2019

View reviewed changes

doc/release-notes-16787.md Outdated Show resolved Hide resolved

src/rpc/net.cpp Outdated Show resolved Hide resolved

maflcko reviewed Sep 4, 2019

View reviewed changes

src/rpc/net.cpp Outdated Show resolved Hide resolved

darosior and others added 2 commits September 5, 2019 15:11

rpc/net: decode the services flags in a new entry

6564f58

doc: add a release note for the new field in 'getpeerinfo' and 'getne…

66740f4

…tworkinfo'

darosior force-pushed the services_for_humans branch from 10adfa5 to 66740f4 Compare September 5, 2019 13:12

promag reviewed Sep 5, 2019

View reviewed changes

src/rpc/util.cpp Show resolved Hide resolved

src/rpc/util.cpp Show resolved Hide resolved

src/rpc/util.cpp Show resolved Hide resolved

src/rpc/net.cpp Show resolved Hide resolved

laanwj merged commit 66740f4 into bitcoin:master Sep 10, 2019

maflcko mentioned this pull request Sep 10, 2019

test: Add smoke test for getpeerinfo['servicesnames'] #16844

Closed

darosior deleted the services_for_humans branch September 10, 2019 19:07

darosior mentioned this pull request Sep 10, 2019

test: servicesnames field in getpeerinfo and getnetworkinfo #16850

Merged

tynes mentioned this pull request Sep 20, 2019

RPC: Expose service names bcoin-org/bcoin#856

Merged

luke-jr pushed a commit to bitcoinknots/bitcoin that referenced this pull request Sep 21, 2019

rpc/net: decode the services flags in a new entry

8971be7

Github-Pull: bitcoin#16787 Rebased-From: 6564f58

tynes mentioned this pull request Sep 25, 2019

http: expose human readable service names handshake-org/hsd#254

Merged

sickpig pushed a commit to sickpig/BitcoinUnlimited that referenced this pull request Jan 20, 2020

rpc/net: decode the services flags in a new entry

922cab0

This is a backport of bitcoin/bitcoin/pull/16787

sickpig mentioned this pull request Jan 20, 2020

[port][rpc] Add getchaintxstats and uptime RPC calls BitcoinUnlimited/BitcoinUnlimited#2070

Merged

sickpig pushed a commit to sickpig/BitcoinUnlimited that referenced this pull request Jan 22, 2020

rpc/net: decode the services flags in a new entry

91d9bf9

This is a backport of bitcoin/bitcoin/pull/16787

sickpig pushed a commit to sickpig/BitcoinUnlimited that referenced this pull request Jan 31, 2020

rpc/net: decode the services flags in a new entry

716432b

This is a backport of bitcoin/bitcoin/pull/16787

sickpig pushed a commit to sickpig/BitcoinUnlimited that referenced this pull request Feb 6, 2020

rpc/net: decode the services flags in a new entry

ff34445

This is a backport of bitcoin/bitcoin/pull/16787

sickpig pushed a commit to sickpig/BitcoinUnlimited that referenced this pull request Feb 7, 2020

rpc/net: decode the services flags in a new entry

38ade11

This is a backport of bitcoin/bitcoin/pull/16787

sickpig pushed a commit to sickpig/BitcoinUnlimited that referenced this pull request Feb 10, 2020

rpc/net: decode the services flags in a new entry

c787d89

This is a backport of bitcoin/bitcoin/pull/16787

sickpig pushed a commit to sickpig/BitcoinUnlimited that referenced this pull request Feb 11, 2020

rpc/net: decode the services flags in a new entry

e7f15db

This is a backport of bitcoin/bitcoin/pull/16787

kwvg added a commit to kwvg/dash that referenced this pull request Sep 18, 2021

merge bitcoin#16787: Human readable network services

58340ce

bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021

rpc: Human readable network services #16787

rpc: Human readable network services #16787

Uh oh!

Conversation

darosior commented Sep 2, 2019 • edited by maflcko Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

laanwj commented Sep 2, 2019

Uh oh!

sipa commented Sep 2, 2019

Uh oh!

darosior commented Sep 2, 2019

Uh oh!

fanquake left a comment

Choose a reason for hiding this comment

Uh oh!

practicalswift commented Sep 3, 2019

Uh oh!

maflcko left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

darosior commented Sep 3, 2019

Uh oh!

DrahtBot commented Sep 3, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Conflicts

Uh oh!

laanwj commented Sep 3, 2019

Uh oh!

Uh oh!

Uh oh!

darosior commented Sep 4, 2019

Uh oh!

mzumsande left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

laanwj commented Sep 5, 2019

Uh oh!

darosior commented Sep 5, 2019

Uh oh!

maflcko commented Sep 5, 2019

Uh oh!

promag left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

laanwj commented Sep 10, 2019

Uh oh!

darosior commented Sep 10, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

darosior commented Sep 2, 2019 •

edited by maflcko

Loading

DrahtBot commented Sep 3, 2019 •

edited

Loading