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

[Issue-3867] Limit outbound eth message sizes #4034

Merged
merged 12 commits into from
Jul 5, 2022

Conversation

mbaxter
Copy link
Contributor

@mbaxter mbaxter commented Jun 30, 2022

PR description

Builds on #4002.

Add logic to limit the size of outbound eth subprotocol messages:

  • Explicitly limit the size of messages constructed in response to queries from other nodes (requests for headers, bodies, etc).
  • Add logging to surface issues with any other types of messages that are generated by the node internally.

This should prevent nodes from being disconnected from their peers for sending messages that exceed the 10MB limit.

Fixed Issue(s)

Fixes #3867

Documentation

  • I thought about documentation and added the doc-change-required label to this PR if
    updates are required.

Changelog

Signed-off-by: Meredith Baxter <meredith.baxter@palm.io>
Signed-off-by: Meredith Baxter <meredith.baxter@palm.io>
Signed-off-by: Meredith Baxter <meredith.baxter@palm.io>
Signed-off-by: Meredith Baxter <meredith.baxter@palm.io>
Signed-off-by: Meredith Baxter <meredith.baxter@palm.io>
Signed-off-by: Meredith Baxter <meredith.baxter@palm.io>
Signed-off-by: Meredith Baxter <meredith.baxter@palm.io>
Signed-off-by: Meredith Baxter <meredith.baxter@palm.io>
Signed-off-by: Meredith Baxter <meredith.baxter@palm.io>
Signed-off-by: Meredith Baxter <meredith.baxter@palm.io>
Signed-off-by: Meredith Baxter <meredith.baxter@palm.io>
@mbaxter mbaxter marked this pull request as ready for review July 1, 2022 20:46
@mbaxter mbaxter requested a review from macfarla July 1, 2022 20:46
Copy link
Contributor

@macfarla macfarla left a comment

Choose a reason for hiding this comment

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

LGTM - I support the addition of (minimal) comments where that helps make it readable! Tests look great.


assertThat(ethMessages.dispatch(new EthMessage(ethPeer, GetNodeDataMessage.create(hashes))))
.contains(NodeDataMessage.create(expectedResult));
}
Copy link
Contributor

Choose a reason for hiding this comment

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

nice

@mbaxter mbaxter enabled auto-merge (squash) July 5, 2022 13:57
@mbaxter mbaxter merged commit 3dce6a9 into hyperledger:main Jul 5, 2022
eum602 pushed a commit to lacchain/besu that referenced this pull request Nov 3, 2023
Signed-off-by: Meredith Baxter <meredith.baxter@palm.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Besu peers send messages exceeding the 10MB message size limit
2 participants