rust-libp2p
rust-libp2p copied to clipboard
fix(swarm): Showcase impossibility of dialing self by ip.
Description
This PR addresses issue #4460.
However, the issue described in #4460 is not actually an issue, and, as the update test showcases, errors were already raised when swarm tried to dial self.
The updated test adds ::1
and 127.0.0.1
version of self.
Change checklist
I do not believe there must be any changelog items as the code did not change at all.
- [x] I have performed a self-review of my own code
- [ ] (Not required) I have made corresponding changes to the documentation
- [ ] (Not required) I have added tests that prove my fix is effective or that my feature works
- [ ] (Not required) A changelog entry has been made in the appropriate crates
This pull request has merge conflicts. Could you please resolve them @vnermolaev? 🙏
I am getting a failure with Cargo.lock
; however, my changes do not concern it.
What is the actual error that is returned here? I think to correctly fix #4460, we should catch this case explicitly and not return on the kernel to fail dialing 0.0.0.0
.
The error returned is
SwarmEvent::OutgoingConnectionError {
peer_id,
// multiaddr,
error: DialError::Transport(errors),
..
}
It is already the case that libp2p
fails on dialling 0.0.0.0
as shown by the original test. However, this test did not list ::1
as an erroneous case, which already is. I just added an ipv6 address to showcase that.
#4460 requests to disallow dialling self, which is already the case. As I see it, there are two options:
- keep things intact because
libp2p
won't dial self. This requires the small changes I introduced, - filter the localhost versions from the list of addresses to dial with a warning, then ofc no dial error will be returned.
I believe the former is better because it already works and produces expected errors. In the latter case, imagine the list of addresses consists of only the localhost versions, which will be filtered out, then one shall receive an error stating that no addresses were present. Such an error is confusing and requires a log investigation.