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

check for self in one more spot #4521

Merged
merged 4 commits into from
Oct 19, 2022
Merged

Conversation

macfarla
Copy link
Contributor

@macfarla macfarla commented Oct 12, 2022

Signed-off-by: Sally MacFarlane macfarla.github@gmail.com

When regularly checking connections to maintained peers (static nodes), ensure connection to self is not initiated.

Documentation

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

Changelog

Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>
Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>
@@ -359,8 +359,10 @@ void checkMaintainedConnectionPeers() {
if (!localNode.isReady()) {
return;
}
final EnodeURL localEnodeURL = localNode.getPeer().getEnodeURL();
maintainedPeers
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a good reason why self exists in the maintainedPeers set? Or is that being looked at separately?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the way the code is now, at startup, you don't know your own enode at the time the static nodes are added to the maintainedPeers set, so you can't avoid adding your own enode

maintainedPeers
.streamPeers()
.filter(peer -> !peer.getEnodeURL().getNodeId().equals(localEnodeURL.getNodeId()))
Copy link
Contributor

Choose a reason for hiding this comment

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

Is enoudeURL::equals working in this case?

Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>
@garyschulte garyschulte enabled auto-merge (squash) October 19, 2022 10:28
@garyschulte garyschulte merged commit ec13809 into hyperledger:main Oct 19, 2022
@macfarla macfarla deleted the static-peers branch October 19, 2022 10:30
macfarla added a commit to jflo/besu that referenced this pull request Jan 10, 2023
* check for self in one more spot

Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>
Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>
eum602 pushed a commit to lacchain/besu that referenced this pull request Nov 3, 2023
* check for self in one more spot

Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants