Skip to content
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

[Output] Properly log reason(s) for increasing a peer's DoS score. #611

Merged
merged 1 commit into from May 24, 2018

Conversation

Projects
None yet
4 participants
@Fuzzbawls
Copy link
Collaborator

Fuzzbawls commented May 12, 2018

Many of the MN related DoS checks had their log messages output only if
the client was running in debug mode, leading to unexplained peer bans.

@presstab

This comment has been minimized.

Copy link
Collaborator

presstab commented May 14, 2018

utACK

@Mrs-X
Copy link
Collaborator

Mrs-X left a comment

Concept utACK, but a NIT: please re-add the commands (mnget, mnb, dsee, etc.) to the log because they help a lot when you try to debug the process-flow between nodes.

@Fuzzbawls

This comment has been minimized.

Copy link
Collaborator Author

Fuzzbawls commented May 18, 2018

@Mrs-X could we maybe make the message text a bit more descriptive without using the cryptic message types?

like, instead of mnvote - signature invalid' and fbvote - signature invalidwe could do CBudgetManager::ProcessMessage() : invalid proposal vote signatureandCBudgetManager::ProcessMessage() : invalid final budget vote signature`

@Mrs-X

This comment has been minimized.

Copy link
Collaborator

Mrs-X commented May 19, 2018

I'm for the best of both, CBudgetManager::ProcessMessage() : mnvote - invalid proposal vote signature :-)
I wouldn't want to have the type of message itself removed. Back then™ it was actually added just for those messages.

@Fuzzbawls Fuzzbawls force-pushed the Fuzzbawls:2018_log-misbehaving branch May 23, 2018

@Fuzzbawls

This comment has been minimized.

Copy link
Collaborator Author

Fuzzbawls commented May 23, 2018

@Mrs-X updated to re-add the message type identifiers to the log outputs

[Output] Properly log reason(s) for increasing a peer's DoS score.
Many of the MN related DoS checks had their log messages output only if
the client was running in debug mode, leading to unexplained peer bans.

@Fuzzbawls Fuzzbawls force-pushed the Fuzzbawls:2018_log-misbehaving branch to fe14f5f May 23, 2018

@Warrows
Copy link
Collaborator

Warrows left a comment

utACK

@Mrs-X

Mrs-X approved these changes May 24, 2018

Copy link
Collaborator

Mrs-X left a comment

ACK (at least for most of the messages) and merging...

@Mrs-X Mrs-X merged commit fe14f5f into PIVX-Project:master May 24, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

Mrs-X added a commit that referenced this pull request May 24, 2018

Merge #611: [Output] Properly log reason(s) for increasing a peer's D…
…oS score.

fe14f5f [Output] Properly log reason(s) for increasing a peer's DoS score. (Fuzzbawls)

Tree-SHA512: 2fbffab2189e05b90ae812ec8d7406ad1c7c207785baba418803a39bdd63c209a878209c70f835595987a4bb57186309f58392fa871557233f0bde30c8fd44ab

@wafflebot wafflebot bot removed the review label May 24, 2018

@Fuzzbawls Fuzzbawls added this to the 3.1.1 milestone Jun 28, 2018

Fuzzbawls added a commit to Fuzzbawls/PIVX that referenced this pull request Jul 6, 2018

[Output] Properly log reason(s) for increasing a peer's DoS score.
Many of the MN related DoS checks had their log messages output only if
the client was running in debug mode, leading to unexplained peer bans.

Github-Pull: PIVX-Project#611
Rebased-From: fe14f5f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.