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

devp2p: add snappy compression to ping/pong #1442

Merged
merged 2 commits into from
Aug 31, 2021
Merged

devp2p: add snappy compression to ping/pong #1442

merged 2 commits into from
Aug 31, 2021

Conversation

acolytec3
Copy link
Contributor

Fixes #1421

When we did the original devp2p v5 in #1399 to add snappy compression to inbound/outbound devp2p messages, we didn't add snappy compression to the ping/pong messages since I wrongly interpreted them as not having a body so therefore not needing snappy. This proved incorrect as we are still RLP encoding an empty array for the body and this also needs snappy compression. Geth will now maintain connection through the ping/pong process.

@codecov
Copy link

codecov bot commented Aug 30, 2021

Codecov Report

Merging #1442 (09ea12c) into master (866f865) will decrease coverage by 0.17%.
The diff coverage is 0.00%.

Impacted file tree graph

Flag Coverage Δ
block 86.76% <ø> (+0.13%) ⬆️
blockchain 83.56% <ø> (ø)
client 81.48% <ø> (-0.83%) ⬇️
common 94.23% <ø> (ø)
devp2p 82.76% <0.00%> (+0.09%) ⬆️
ethash 86.56% <ø> (ø)
tx 88.36% <ø> (+0.11%) ⬆️
vm 82.78% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

@acolytec3
Copy link
Contributor Author

This may also fix #1416 but I don't have any way to prove that. I think if we don't see that invalid remainder error, #1416 can probably be closed as well.

Copy link
Contributor

@ryanio ryanio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice, lgtm!

@holgerd77
Copy link
Member

Great find, thanks for the deep analysis here! 😀

@holgerd77 holgerd77 merged commit 327d2d4 into master Aug 31, 2021
@holgerd77 holgerd77 deleted the snappy-fix branch August 31, 2021 06:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Client: Connection to sync peer gets lost quickly
3 participants