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

bugfix(op-node): syncClient incorrectly removes peer issue #50

Merged
merged 6 commits into from
Oct 17, 2023

Conversation

welkin22
Copy link
Contributor

@welkin22 welkin22 commented Sep 18, 2023

Background

When the staticPeer is configured with a DNS address (such as /dns/xxx.com/tcp/xxx/p2p/xxx), a peer can obtain multiple IP addresses through domain name resolution. However, in Libp2p, dialing concurrently can result in one peer corresponding to multiple connections, which is exacerbated by multiple IP addresses. If one of the connections is disconnected due to an unstable network or other reasons, the Notifiee.Disconnected event is triggered. At this time, the peer is removed from the syncClient, and the processing flow of the peerLoop exits. The log displays the message stopped syncing loop of peer. However, the peer still has other connections that can be used for p2p synchronization. Therefore, we should not remove this peer unless we determine that all of its connections are disconnected and there are no available connections, just like the check performed by the monitorStaticPeers function.

Solution

Add a condition to determine when to remove a peer from the syncClient: only remove the peer if it cannot establish a connection.

@welkin22 welkin22 changed the title bugfix: syncClient incorrectly removes peer issue bugfix(op-node): syncClient incorrectly removes peer issue Sep 18, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Oct 5, 2023

This PR is stale because it has been open 14 days with no activity. Remove stale label or comment or this will be closed in 5 days.

@github-actions github-actions bot added the Stale label Oct 5, 2023
@owen-reorg owen-reorg removed the Stale label Oct 7, 2023
@welkin22
Copy link
Contributor Author

The same changes have been submitted to the Optimism chain: ethereum-optimism/optimism#7348 and were approved

@owen-reorg owen-reorg merged commit 965e3a1 into bnb-chain:develop Oct 17, 2023
9 checks passed
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