-
Notifications
You must be signed in to change notification settings - Fork 236
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
feat: remove old version peer from peer store on fork #2849
feat: remove old version peer from peer store on fork #2849
Conversation
Please add a test case to ensure that the staled peers are actually removed. |
If we don't remove the old version peers, will there be security issues? |
There are no safety issues, there are efficiency issues |
Okay, I see. Why do we not remove old version peers inside |
|
Thanks, I do not have any additional questions about it. |
Not sure it is a good idea, since there's a very high probability that most the peers will switch to the fork. |
This is just to clean up unnecessary node information so that it does not make meaningless broadcasts. There is no ban. If the node upgrades the client, it will be added back. |
This PR title and commit msg is easily misunderstood, suggest |
Could you expand this with more details? What's the performance impact if we do not purge the peer store on fork? |
The existing peer store rules are based on the detection of whether the session connection is successful. If it succeeds, the node information will be kept in the peer store. In the fork version, the old and new networks can establish a session, but the corresponding protocol cannot be opened, and the network is isolated through the protocol version. Formally because the session can be established, there will be a large number of invalid old version node information in the peer store. These data cannot be deleted by the current methods, and the dissemination of these invalid information in the network will result in a significant drop in the efficiency of node discovery |
d3c0f26
to
19003f5
Compare
19003f5
to
ccada30
Compare
bors merge=quake,yangby-cryptape |
Build succeeded: |
RefCell