Tcp transport port_reuse_dialing test missing some assert

[stormshield-pj50]

Summary

The port_reuse_dialing test in transports/tcp/lib.rs seems to be incomplete, it does not check for port reuse.

Expected behaviour

In this test the dialer outgoing connection must reuse the listener port. So an assert like this should be added near line 930 in the port_reuse_dialing/dialer function:

assert!(tcp.port_reuse.local_dial_addr(&listener.listen_addr.ip()).is_some());

Actual behaviour

The assert is missing and if I add it it fails. It seems to be a consequence of the commit 2ad905f. Previously the listen_addrs field in PortReuse was a Arc<RwLock<HashSet<(IpAddr, Port)>>> so it could be shared between GenTcpConfig and TcpListenStream. One ugly work around to make the assert pass in the test is to clone listener.port_reuse into tcp:

tcp.port_reuse = listener.port_reuse.clone(); 

Possible Solution

Don't know if only the test is broken or if libp2p port reuse feature is also broken.

Would you like to work on fixing this bug?

Yes

[mxinden]

Thanks for the bug report @Pj50. Great job catching this! This was my mistake.

For others following along here is where the HashMap instead of the Arc is being cloned.

rust-libp2p/transports/tcp/src/lib.rs

Line 327 in d21cd5f

TcpListenStream::<T>::new(socket.into(), self.port_reuse.clone())

Would you like to work on fixing this bug?

Yes

Great! Let me know in case you need any help.

[mxinden]

Friendly ping @Pj50. Still interested in providing a patch?

[stormshield-pj50]

Sorry being late for a patch, I had to work on other topics last week. Working on it now.

[mxinden]

Closing here with #2670 merged.