-
Notifications
You must be signed in to change notification settings - Fork 37.7k
cli: -netinfo quick updates/fixups for 0.21 #20115
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
Probably @ mentions should be removed from the OP. |
Yes. Removed @ mentions. |
ACK f639a88 Build, ran functional & unit tests, used |
Concept ACK: I'm a big fan of |
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
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. |
8b0aa43
to
f639a88
Compare
f639a88
to
f3e30bf
Compare
Tested ACK 43ee49f Build and used -netinfo on mainnet, testnet and signet. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK 43ee49f, tested on Linux Mint 20 (x86_64).
f3e30bf: isn't the https://github.com/bitcoin-core/bitcoin-devwiki/wiki/0.21.0-Release-Notes-Draft designed for this purpose?
43ee49f: does id
column require variable-width too?
Thanks for reviewing!
Yes, that wiki was not yet created when the PR was opened. I'm not sure what the process is in this case. Edit: dropped the release note.
Yes, it's already variable-width. |
Indeed :) Sorry for noise. |
43ee49f
to
bab058e
Compare
Thank you @Emzy, @hebasto, @0xB10C and @practicalswift for reviewing and testing. Updated per
|
- add new signet chain - update change "uptime" column name to "age" per suggestion by 0xB10C (Timo) - add an additional digit to mping field width - change m_networks_size from size_t to uint8_t, as size_t was a holdover from m_networks_size being defined as size_t m_networks.size() in a draft - order Peer struct members by decreasing memory size
as it has a wide possible range and the new name ("age" instead of "uptime") is much shorter.
bab058e
to
70bd277
Compare
70bd277
to
398045b
Compare
ACK 398045b Light code review, tested on MacOS Catalina with Signet and all 0-4 arguments. A couple of (very) minor non-urgent observations not relevant for 0.21 (but maybe for a future follow up PR)
Currently
Potential improvement
|
Tested ACK 398045b Build and used -netinfo on mainnet, testnet and signet on Ubuntu 20.04.1 LTS. |
Thanks for testing and the feedback!
Yes, flexibility for different buffer/window widths. Especially with the new Tor v3 addresses, which are much longer. The "version" column actually combines two getpeerinfo fields into one: version and sub-version. So in a way, it's more info ;) |
398045b cli -netinfo: print oversized/extreme ping times as "-" (Jon Atack) 773f4c9 cli -netinfo: handle longer tor v3 local addresses (Jon Atack) 33e9874 cli -netinfo: make age column variable-width (Jon Atack) f8a1c4d cli -netinfo: various quick updates and fixes (Jon Atack) Pull request description: Quick fixups and updates for v0.21.0: - [x] handle larger BIP155 `addrv2` addresses - [x] add Signet chain - [x] add an additional space between the `net` and `mping` columns; add missing `tinyformat` and `algorithm` headers - [x] s/uptime/age/ per 0xB10C suggestion, and make the column auto-adjusting variable width - [x] display `-` for oversized mping/ping times like `1.17348e+06`, as reported by practicalswift Edit: removed the release note commit, as this PR was not merged before the notes were moved to the wiki. It's here: ``` - A new `bitcoin-cli -netinfo` command returns a network peer connections dashboard that displays data from the `getpeerinfo` and `getnetworkinfo` RPCs in a human-readable format. An optional integer argument from `0` to `4` may be passed to see various levels of detail. (bitcoin#19643) ``` ACKs for top commit: michaelfolkson: ACK 398045b Emzy: Tested ACK 398045b Tree-SHA512: 0625ee840141bafbfcaf8f1fce53f8f850ae91721b2bdad4279372da87c18a1fe3a214d90bfdbbabdf6da38d58290d7dd0f1109b4e2ca5d20cacf417d6ced0f9
Thanks @michaelfolkson, I'll do this in the next -netinfo update to use the |
Added the release note in the PR description to the wiki at https://github.com/bitcoin-core/bitcoin-devwiki/wiki/0.21.0-Release-Notes-Draft. |
Summary: - update change "uptime" column name to "age" per suggestion by 0xB10C (Timo) - add an additional digit to mping field width - change m_networks_size from size_t to uint8_t, as size_t was a holdover from m_networks_size being defined as size_t m_networks.size() in a draft - order Peer struct members by decreasing memory size This is a backport of [[bitcoin/bitcoin#20115 | core#20115]] [1/4] bitcoin/bitcoin@f8a1c4d Test Plan: `ninja && src/bitcoind --daemon && sleep 5 && src/bitcoin-cli -netinfo 4 && src/bitcoin-cli stop` Reviewers: #bitcoin_abc, Fabien Reviewed By: #bitcoin_abc, Fabien Differential Revision: https://reviews.bitcoinabc.org/D10707
Summary: It has a wide possible range and the new name ("age" instead of "uptime") is much shorter. This is a backport of [[bitcoin/bitcoin#20115 | core#20115]] [2/4] bitcoin/bitcoin@33e9874 Depends on D10707 Test Plan: `ninja && src/bitcoind --daemon && sleep 5 && src/bitcoin-cli -netinfo 4 && src/bitcoin-cli stop` Reviewers: #bitcoin_abc, Fabien Reviewed By: #bitcoin_abc, Fabien Differential Revision: https://reviews.bitcoinabc.org/D10708
Summary: This is a backport of [[bitcoin/bitcoin#20115 | core#20115]] [3/4] bitcoin/bitcoin@773f4c9 Depends on D10708 Test Plan: `ninja && src/bitcoind --daemon && sleep 5 && src/bitcoin-cli -netinfo 4 && src/bitcoin-cli stop` Reviewers: #bitcoin_abc, Fabien Reviewed By: #bitcoin_abc, Fabien Differential Revision: https://reviews.bitcoinabc.org/D10709
Summary: This concludes backport of [[bitcoin/bitcoin#20115 | core#20115]] [4/4] bitcoin/bitcoin@398045b Depends on D10709 Test Plan: `ninja && src/bitcoind --daemon && sleep 5 && src/bitcoin-cli -netinfo 4 && src/bitcoin-cli stop` Reviewers: #bitcoin_abc, Fabien Reviewed By: #bitcoin_abc, Fabien Differential Revision: https://reviews.bitcoinabc.org/D10710
Quick fixups and updates for v0.21.0:
addrv2
addressesnet
andmping
columns; add missingtinyformat
andalgorithm
headers-
for oversized mping/ping times like1.17348e+06
, as reported by practicalswiftEdit: removed the release note commit, as this PR was not merged before the notes were moved to the wiki. It's here: