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
Do not add addresses if they are already there in peerdata #325
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great catch and solve! Would you mind adding some tests for this PR for the case we "add duplicated addrs"? Except for the tests, this looks good to me.
for addr in addrs: | ||
if addr not in self.addrs: | ||
self.addrs.append(addr) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What I'm thinking is probably we should change addrs
from List[Multiaddr]
to Set[Multiaddr]
to reduce time complexity.
However, this solution works for me so far. 👍
Edited: removed the thought on replacing List
with Set
since it is already mentioned in the PR.
i'd suggest we use a do you want to update the PR @moshemalawach ? if not, i'll move this to a tracking issue and we can go ahead and merge this in. |
and +1 on a test for this scenario :) |
pyaleph depends on this version of `py-libp2p` and on `py-substrate-interface`. `py-substrate-interface` requires `base58>=2.0.1`, which conflicts with the dependency on `base58>=1.0.3,<2.0.0` in this setup.py.
Fix dependency issue on base58 with pyaleph
In peerdata, if you call add_addrs, the function doesn't check if the addresses are already there.
This can result in the list growing huge if we reconnect often to the same hosts.
An alternative solution to the one proposed would be to change self.addrs to a set and not a list. But that would need some work in places where we use addrs[0] (like in swarm).