Use fallback value for Version and User Agent during peer connection #673

pull jonatack wants to merge 1 commits into bitcoin-core:master from jonatack:2022-09-display-fallback-for-gui-peers-version-and-user-agent changing 1 files +6 −2

jonatack commented at 1:21 PM on October 5, 2022: contributor

During connection setup for a peer, getpeerinfo returns "version": 0, "subver": "" and the GUI Peers window displays 0 and an empty field, respectively.

Give these fields the same behavior as the other fields in the GUI Peers window: display the fallback value in src/qt/forms/debugwindow.ui (i.e. N/A) until a valid result is available after the peer connection completes.

An alternative would be to display nothing for both, as is the case currently for User Agent.

gui: use fallback value for Version and User Agent during peer connection

During connection setup for a peer, getpeerinfo returns "version": 0, "subver": ""
and the GUI Peers window displays 0 and an empty field, respectively.

Give these fields the same behavior as the other fields in the GUI Peers window:
display the fallback value in src/qt/forms/debugwindow.ui (i.e. "N/A") until a
valid result is available after the peer connection completes.

c2a21c0670

jarolrod commented at 4:39 AM on October 14, 2022: member

ACK c2a21c06703fd20e314da8431220776c8f1cc08d

We are made aware of these fields so quickly that it doesn't really make a difference, but at least we aren't showing incorrect information if someone is looking.
jonatack commented at 10:40 PM on October 18, 2022: contributor

Thanks @jarolrod! I noticed this because these fields can occasionally remain in the "connection setup" stage for a few seconds.
furszy approved
furszy commented at 1:14 PM on October 21, 2022: member

code ACK c2a21c06
maflcko merged this on Oct 24, 2022
maflcko closed this on Oct 24, 2022
jonatack deleted the branch on Oct 24, 2022
sidhujag referenced this in commit a153534101 on Oct 25, 2022
in src/qt/rpcconsole.cpp:1184 in c2a21c0670
```
1177 | @@ -1178,8 +1178,12 @@ void RPCConsole::updateDetailWidget()
1178 |      ui->peerPingTime->setText(GUIUtil::formatPingTime(stats->nodeStats.m_last_ping_time));
1179 |      ui->peerMinPing->setText(GUIUtil::formatPingTime(stats->nodeStats.m_min_ping_time));
1180 |      ui->timeoffset->setText(GUIUtil::formatTimeOffset(stats->nodeStats.nTimeOffset));
1181 | -    ui->peerVersion->setText(QString::number(stats->nodeStats.nVersion));
1182 | -    ui->peerSubversion->setText(QString::fromStdString(stats->nodeStats.cleanSubVer));
1183 | +    if (stats->nodeStats.nVersion) {
1184 | +        ui->peerVersion->setText(QString::number(stats->nodeStats.nVersion));
1185 | +    }
1186 | +    if (!stats->nodeStats.cleanSubVer.empty()) {
```
luke-jr commented at 5:35 PM on October 27, 2022:

What if the peer actually sends an empty subver?

jonatack commented at 6:10 PM on October 27, 2022:

In that case only the version is displayed.

<img width="567" alt="Screenshot 2022-10-27 at 10 57 48" src="https://user-images.githubusercontent.com/2415484/198366226-3c9908e0-709c-47f4-b9ba-b8c8ea91f0d2.png">

luke-jr commented at 8:15 PM on October 27, 2022:

That has a non-empty subver... "/Satoshi:22.0.0/"

jonatack commented at 8:39 PM on October 27, 2022:

Maybe I'm confused; I'm aware of the -uacomment config option, but how would one send an empty subver (is it a conf option in a different implementation)?

luke-jr commented at 1:20 AM on October 28, 2022:

It would have to be a very old or hacked/custom client. But showing "N/A" in that case seems wrong?

jonatack commented at 6:14 PM on October 28, 2022:

"N/A" for such a case (have never seen it tbh) doesn't seem unreasonable to me, but we could display nothing ("") if the subver is empty and the peer connection is set up.
jonatack cross-referenced this on Nov 4, 2022 from issue Bugfix: Address broken things around Peers detail view by luke-jr
bitcoin-core locked this on Oct 28, 2023

Contributors

Linked (view graph)

#677 Bugfix: Address broken things around Peers detail view