DummyTransport when using other transport

[nazar-pc]

Summary

When building swarm with .with_other_transport() in 0.53.1 I notice a bunch of these in logs:

2023-11-17T06:49:48.958626Z DEBUG Swarm::poll: libp2p_core::transport::choice: Failed to dial address using libp2p_core::transport::dummy::DummyTransport<(libp2p_identity::peer_id::PeerId, libp2p_core::muxing::boxed::StreamMuxerBox)> address=/ip4/38.242.232.164/udp/30433/quic-v1/p2p/12D3KooWNq4jwEogz2mwEP1EzjCNFngc9GWk5QtNpqxrdxfHLyrm
2023-11-17T06:49:48.958675Z DEBUG Swarm::poll: libp2p_core::transport::choice: Failed to dial address using libp2p_core::transport::dummy::DummyTransport<(libp2p_identity::peer_id::PeerId, libp2p_core::muxing::boxed::StreamMuxerBox)> address=/ip4/38.242.232.164/udp/20681/quic-v1/p2p/12D3KooWNq4jwEogz2mwEP1EzjCNFngc9GWk5QtNpqxrdxfHLyrm
2023-11-17T06:49:48.958706Z DEBUG Swarm::poll: libp2p_core::transport::choice: Failed to dial address using libp2p_core::transport::dummy::DummyTransport<(libp2p_identity::peer_id::PeerId, libp2p_core::muxing::boxed::StreamMuxerBox)> address=/ip4/38.242.232.164/udp/49969/quic-v1/p2p/12D3KooWNq4jwEogz2mwEP1EzjCNFngc9GWk5QtNpqxrdxfHLyrm

Expected behavior

I do not expect to see dummy transport in logs unless configured

Actual behavior

Dummy transport is still used due to the following: https://github.com/libp2p/rust-libp2p/blob/2ecc7cf1253e5a884d093fc51b4ba35c45a57842/libp2p/src/builder/phase/tcp.rs#L181-L183 https://github.com/libp2p/rust-libp2p/blob/2ecc7cf1253e5a884d093fc51b4ba35c45a57842/libp2p/src/builder/phase/tcp.rs#L102-L112 https://github.com/libp2p/rust-libp2p/blob/441c242555b13bf8144df72c1f3a64e049c139d3/libp2p/src/builder/phase/quic.rs#L64-L72 https://github.com/libp2p/rust-libp2p/blob/441c242555b13bf8144df72c1f3a64e049c139d3/libp2p/src/builder/phase/other_transport.rs#L25-L58

So we're going from Quic(Tcp) to Quic(Dummy) to Dummy to Dummy(TransportUserProvided).

Relevant log output

No response

Possible Solution

Solution here would be to somehow replace the transport with a custom one rather than chaining it with Dummy transport.

Generally I don't really understand why would you un-wrap TCP and UDP (that user didn't ask to do) and chain dummy with custom transport instead of simply replacing the transport. A bit strange implementation to be honest.

Version

0.53.1

Would you like to work on fixing this bug ?

Maybe

[thomaseizinger]

Generally I don't really understand why would you un-wrap TCP and UDP (that user didn't ask to do) and chain dummy with custom transport instead of simply replacing the transport. A bit strange implementation to be honest.

It is the best we could come up with given the type-system constraints whilst not having the builder explode in combinations. Definitely open to a better implementation if you've got any ideas! :)

[thomaseizinger]

I do not expect to see dummy transport in logs unless configured

I wonder if we can fix this by checking whether we just dialed a dummy transport. A somewhat dirty but functional implementation could to check whether the name of the type contains dummy.

[nazar-pc]

To be clear, this is only really an issue with logs (and probably not related to dummy transport now that I think about it) that happens when trying to debug something.

Maybe additional method Transport::can_dial() would make sense to introduce here that would try all transports quietly one by one to find which one supports the dial of a particular address instead of attempting dial on each transport and falling back to the next one.

[thomaseizinger]

To be clear, this is only really an issue with logs (and probably not related to dummy transport now that I think about it) that happens when trying to debug something.

Yeah fully understand. I think we can fix the logs by testing whether type_name::<T>() contains the string "dummy".

Maybe additional method Transport::can_dial() would make sense to introduce here that would try all transports quietly one by one to find which one supports the dial of a particular address instead of attempting dial on each transport and falling back to the next one.

We'd still have to deal with the possibility of the address not being supported in dial so I am not sure we'd be gaining much from that.

[nazar-pc]

We're just getting fewer unnecessary log messages, nothing more than that.

[thomaseizinger]

To be clear, this is only really an issue with logs (and probably not related to dummy transport now that I think about it) that happens when trying to debug something.

Yeah fully understand. I think we can fix the logs by testing whether type_name::<T>() contains the string "dummy".

Would you be open to sending a PR for this idea?

[nazar-pc]

I'll look into it