-
Notifications
You must be signed in to change notification settings - Fork 275
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
Use fallback value for Version and User Agent during peer connection #673
Use fallback value for Version and User Agent during peer connection #673
Conversation
…tion 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.
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 c2a21c0
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.
Thanks @jarolrod! I noticed this because these fields can occasionally remain in the "connection setup" stage for a few seconds. |
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.
code ACK c2a21c0
…gent during peer connection c2a21c0 gui: use fallback value for Version and User Agent during peer connection (Jon Atack) Pull request description: 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. ACKs for top commit: jarolrod: ACK c2a21c0 furszy: code ACK c2a21c0 Tree-SHA512: 4f0060fa9abde120a2bb48c9dcc87894d9bb70c33e6ab43b22400a4bcd0ceff0fa098adf7f385b0a7a4cf5d7053463b36fe1232e19a8d5025eecd8db9833f73b
if (stats->nodeStats.nVersion) { | ||
ui->peerVersion->setText(QString::number(stats->nodeStats.nVersion)); | ||
} | ||
if (!stats->nodeStats.cleanSubVer.empty()) { |
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.
What if the peer actually sends an empty subver?
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.
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.
That has a non-empty subver... "/Satoshi:22.0.0/"
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.
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)?
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.
It would have to be a very old or hacked/custom client. But showing "N/A" in that case seems wrong?
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.
"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.
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.