Bugfix: Address broken things around Peers detail view #677
Conversation
luke-jr
commented
Nov 4, 2022
luke-jr
commented
- Ensure data for previously-selected peer doesn't remain when a new peer is selected (reset all fields to N/A before loading data for a new peer)
- Load user-agent along with peer version (so a blank UA actually shows as blank)
- Display actually having no permissions as "None" rather than "N/A"
…fore loading data Without this, !fNodeStateStatsAvailable will leave fields with the data of the previously selected peer
|
Concept ACK. This also cleanly implements #673 (comment). |
| @@ -3224,6 +3224,10 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, | |||
| // Change version | |||
| const int greatest_common_version = std::min(nVersion, PROTOCOL_VERSION); | |||
| pfrom.SetCommonVersion(greatest_common_version); | |||
| { | |||
There was a problem hiding this comment.
we really shouldn't have this change being performed from this repo :)
There was a problem hiding this comment.
@hebasto ping seen. nothing to do here though.
There was a problem hiding this comment.
ping seen. nothing to do here though.
This PR modifies src/net_processing.cpp, and we review such changes in the main repo, generally.
Mind moving this PR or the first commit from this repo into the main one?
There was a problem hiding this comment.
Doesn't seem worth splitting up such a minor change IMO
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. ConflictsNo conflicts as of last run. |
|
🤔 There hasn't been much activity lately and the CI seems to be failing. If no one reviewed the current pull request by commit hash, a rebase can be considered. While the CI failure may be a false positive, the CI hasn't been running for some time, so there may be a real issue hiding as well. A rebase triggers the latest CI and makes sure that no silent merge conflicts have snuck in. |
|
Why the first commit "net: Ensure CNode.cleanSubVer is always assigned before nVersion" is needed? |
|
🤔 There hasn't been much activity lately and the CI seems to be failing. If no one reviewed the current pull request by commit hash, a rebase can be considered. While the CI failure may be a false positive, the CI hasn't been running for some time, so there may be a real issue hiding as well. A rebase triggers the latest CI and makes sure that no silent merge conflicts have snuck in. |
2 similar comments
|
🤔 There hasn't been much activity lately and the CI seems to be failing. If no one reviewed the current pull request by commit hash, a rebase can be considered. While the CI failure may be a false positive, the CI hasn't been running for some time, so there may be a real issue hiding as well. A rebase triggers the latest CI and makes sure that no silent merge conflicts have snuck in. |
|
🤔 There hasn't been much activity lately and the CI seems to be failing. If no one reviewed the current pull request by commit hash, a rebase can be considered. While the CI failure may be a false positive, the CI hasn't been running for some time, so there may be a real issue hiding as well. A rebase triggers the latest CI and makes sure that no silent merge conflicts have snuck in. |
