fix: do not override known address after identify is done #1485
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Another fix would be to not add the addresses at all to the address book. |
Identify protocol being executed it means that both peers are somehow connected. Hence, it should not override working multiaddresses but only add to it. Fixes libp2p#1484
fryorcraken
force-pushed
the
fix/identify-override-address
branch
from
ed28e63
to
f11a039
Compare
This was referenced Nov 21, 2022
|
@fryorcraken Identify returns trusted peer addresses so it makes sense to replace old addresses in the address book. Why is your peer sending non-working addresses? Maybe it makes sense to add support to disable identify service altogether?! |
achingbrain
requested changes
Nov 25, 2022
There was a problem hiding this comment.
If we don't evict old addresses, there's a risk the list of known addresses for a remote peer will grow and grow, eventually growing beyond the maxDialsPerPeer limit which will cause dials to fail in a hard to debug way.
The question really is, why is the remote peer sending you 0.0.0.0 addresses?
|
Continuing discussion at #1484 (comment) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Identify protocol being executed it means that both peers are somehow connected. Hence, it should not override working multiaddresses but only add to it.
Fixes #1484