This repository was archived by the owner on Oct 28, 2021. It is now read-only.
Rephrase error messages in EthereumPeer::validate and re-order validation conditions#5559
Merged
Rephrase error messages in EthereumPeer::validate and re-order validation conditions#5559
Conversation
Reorder validation so that checks go from the broadest condition (network ID) to the most targeted (unexpected status message) - also rephrase log messages and include host / peer values. The result is that the logs are easier to read and contain more useful information.
Also move static constant to anonymous namespace
halfalicious
force-pushed
the
sync-status-checks
branch
from
ae258c3 to
ad580b4
Compare
halfalicious
requested review from
chfast and
gumb0
and removed request for
gumb0
halfalicious
force-pushed
the
sync-status-checks
branch
from
ad580b4 to
5b24eda
Compare
halfalicious
force-pushed
the
sync-status-checks
branch
from
5b24eda to
8f57a25
Compare
gumb0
approved these changes
Apr 11, 2019
chfast
approved these changes
Apr 11, 2019
Codecov Report
@@ Coverage Diff @@
## master #5559 +/- ##
==========================================
+ Coverage 62.16% 62.17% +<.01%
==========================================
Files 347 347
Lines 28968 28975 +7
Branches 3281 3281
==========================================
+ Hits 18008 18015 +7
- Misses 9782 9785 +3
+ Partials 1178 1175 -3 |
gumb0
reviewed
Apr 11, 2019
| error << "Protocol version mismatch. Host protocol version: " << _hostProtocolVersion | ||
| << ", peer protocol version: " << m_protocolVersion; | ||
| else if (m_genesisHash != _hostGenesisHash) | ||
| error << "Genesis hash mismatch. Host genesis hash: " << _hostGenesisHash |
Member
There was a problem hiding this comment.
Too bad this doesn't use operator<< overloads for logging, so it outputs long full hash. Maybe change it to outputting .abridged() ?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix #5554
Rephrase the error messages in EthereumPeer::validate so that they are more descriptive and contain the peer and host values. Also order the validation conditions from broadest (network ID) to most targeted (unexpected status message) so that the sync logs are easier to read.