Skip to content

[0.18] net: Log to net category for exceptions in ProcessMessages #17974

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jan 23, 2020
Merged

[0.18] net: Log to net category for exceptions in ProcessMessages #17974

merged 1 commit into from
Jan 23, 2020

Conversation

instagibbs
Copy link
Member

Backport of #17762, currently only backported to 0.19.

This seems like something we should opportunistically plug in case wiseguys decide it's a vector to exploit to try and fill people's disks.

@practicalswift
Copy link
Contributor

ACK 314ba6c

@maflcko
Copy link
Member

maflcko commented Jan 21, 2020

Please use the backport script or include the backport metatdata manually. Otherwise it is hard to verify this and see where it came from without any references to the original commit/pull.

@instagibbs
Copy link
Member Author

@MarcoFalke is there a TLDR for the script with deps etc or the metadata required to be included where? This should probably be a CONTIRBUTION.md thing?

@DrahtBot DrahtBot added the P2P label Jan 21, 2020
@maflcko
Copy link
Member

maflcko commented Jan 21, 2020

I think if you cherry-pick the commit from 0.19, it should include all metadata.

There is a python helper script: https://github.com/bitcoin-core/bitcoin-maintainer-tools#backport

Not sure if this should be generally made available for any contributor. Asking for a "needs backport" tag should be sufficient

@fanquake fanquake added this to the 0.18.2 milestone Jan 21, 2020
@fanquake
Copy link
Member

@instagibbs You can take a look at 1a6a534 for an example of the metadata required.

@laanwj @instagibbs
net: Log to net category for exceptions in ProcessMessages
c89611e 
Remove the forest of special exceptions, and simply log a short
message to the NET logging category when an exception happens during
packet processing. It is not good to panick end users with errors
that any peer can generate (let alone writing to stderr).

Github-Pull: bitcoin#17762
Rebased-From: 4d88c3d
@instagibbs instagibbs force-pushed the bp18_network_exceptions branch from 314ba6c to c89611e Compare January 22, 2020 16:54
@instagibbs
Copy link
Member Author

cherry-picked from 0.19

@practicalswift
Copy link
Contributor

ACK c89611e

Reviewers of this PR might want to review the somewhat related PR #17828 ("net: Use log categories when logging events that P2P peers can trigger arbitrarily").

@maflcko maflcko changed the title net: Log to net category for exceptions in ProcessMessages [0.18] net: Log to net category for exceptions in ProcessMessages Jan 22, 2020
@maflcko
Copy link
Member

maflcko commented Jan 22, 2020

ACK c89611e, checked that this is a cherry-pick from 0.19 💐

Show signature and timestamp

Signature:

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

ACK c89611ebd33a86c0fe14adecc100065f16ea16d, checked that this is a cherry-pick from 0.19 💐
-----BEGIN PGP SIGNATURE-----

iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
pUhbXgwAndLCk84m8EK4oVBy9To8XcTNlaKwHHkb7ZG1f5FvFOwCbNhBh8yabG9E
Cs7TRlNtgbqkxuSDrINXbJUJCwpX7VAZYnvM4wO8kaEwp36KdZH4MxuvsYEYgIxD
7QeNltUlOz3lKZxGAzWHht4NSC+FKLZa/MdmqLRbEiOsQnQHup/dww472bQsfXcD
fd7ymEt6MCvu7wuqRuj7QSMsuRvOcCjKr94cE2zPbpUCdkcfLRCurroCAK3hIVyQ
ZRM+Su+JtvJ6qKp7ncbI4beQcWrAXMxUjUCCC9+4gXsvvoFcnPmEq2EncLT8mD4Z
pJZzXBpda3Ob2aG7qDTYyKHJ0WSHg6Sq2NuYNpagbAE8xu2jTwxE3ObkWkzOnqZh
gmqnB5jHngLfRYLWqN4P7AnoGtqwmoxlsDrhUA1rwS68fP0Ky9B1NEo8cJL+l7zC
JyV/uyvFJkE8m/KLcExm9YzKMwYvZoSngdZgQzEBpT+IojerWhS5uJJ/Or6V5no3
yoMJTIRe
=Ms73
-----END PGP SIGNATURE-----

Timestamp of file with hash 06c44dd31613f3547029de3f00dec7998591efe3347b67cf28d33ccd930136f9 -

fanquake added a commit that referenced this pull request Jan 23, 2020
@fanquake
Merge #17974: [0.18] net: Log to net category for exceptions in Proce…
8b67698 
…ssMessages

c89611e net: Log to net category for exceptions in ProcessMessages (Wladimir J. van der Laan)

Pull request description:

  Backport of #17762, currently only backported to 0.19.

  This seems like something we should opportunistically plug in case wiseguys decide it's a vector to exploit to try and fill people's disks.

ACKs for top commit:
  practicalswift:
    ACK c89611e
  MarcoFalke:
    ACK c89611e, checked that this is a cherry-pick from 0.19 💐

Tree-SHA512: e48daf64a14d98a78cadd0774a597e5833a1ff19f05527dfc42f3cc38532c1c3bd1acd925c8e0c484e01fbc8c604ee2bcfc0cec0333e9af570b103a6241b657d
@fanquake fanquake merged commit c89611e into bitcoin:0.18 Jan 23, 2020
@fanquake fanquake mentioned this pull request Jan 23, 2020
Merged
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
Backport P2P
Projects
None yet
Milestone
0.18.2
Development

Successfully merging this pull request may close these issues.
6 participants
@instagibbs @practicalswift @maflcko @fanquake @DrahtBot @laanwj