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

1. 
   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.

2. 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
3. 
   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.

4. 
   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.
5. furszy approved
6. 
   furszy commented at 1:14 pm on October 21, 2022: member

   code ACK c2a21c06
7. maflcko merged this on Oct 24, 2022
8. maflcko closed this on Oct 24, 2022

   ---

9. jonatack deleted the branch on Oct 24, 2022
10. sidhujag referenced this in commit a153534101 on Oct 25, 2022
11. 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.

    ---

    

    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.
12. jonatack cross-referenced this on Nov 4, 2022 from issue Bugfix: Address broken things around Peers detail view by luke-jr
13. bitcoin-core locked this on Oct 28, 2023

Contributors
jonatack jarolrod furszy luke-jr