fix(relay): malformed reported relayed addr when relay peer is dialer

[stormshield-frb]

Description

I don't know if it is a bug or not, but in the NetworkBehaviour trait, there is a discrepancy with the remote multiaddr given to handle_established_Inbound_connection and the one given to handle_established_OUTbound_connection.

handle_established_Inbound_connection receive the remote_addr not suffixed with /p2p/<remote_peer_id> but handle_established_OUTbound_connection do receive addr suffixed with /p2p/<remote_peer_id>.

This is causing a weird behavior in the relay protocol because when creating the Handler, the remote address is passed down as is, and when a "relayed connection" is negotiated with a remote peer through the relay peer, the /p2p-circuit is simply pushed without checking the form of the multiaddr.

Because of that, we encountered some bug where address was reported like this: /ip4/1.2.3.4/udp/1234/quic/p2p-circuit (note the relay peer_id missing), which is invalid (here the specs of a correct relayed address).

This PR simply address the bug of the relay peer but maybe something should be done to make sure that the same format is received, both from incoming and outgoing connections. Since I don't know if it could have side effects else-were, I have just addressed the problem described above.

Notes & open questions

Change checklist

• I have performed a self-review of my own code
• I have made corresponding changes to the documentation
• I have added tests that prove my fix is effective or that my feature works
• A changelog entry has been made in the appropriate crates

[jxs]

Hi François, and thanks for this! @thomaseizinger do you know if there's reason for the mismatch in the suffix? If it's a bug I think it makes more sense to address it at the Swarm level

[thomaseizinger]

Hi François, and thanks for this! @thomaseizinger do you know if there's reason for the mismatch in the suffix? If it's a bug I think it makes more sense to address it at the Swarm level

I think it originated from an idea that we don't want to pass down duplicated information, i.e. the Peerid of the remote is already known so the multiaddr should not contain it. However, without type-level enforcement, this is kind of difficult.

Maybe debug_asserts should be added to Multiaddr to catch these bugs earlier?

[jxs]

went looking into this and the address passed to handle_established_outbound_connection may come from Swarm::dial which is probably what you are doing no @stormshield-frb? Nonetheless this PR makes sense to me, can we just leave a comment stating the situation?
Thanks!

[mergify]

This pull request has merge conflicts. Could you please resolve them @stormshield-frb? 🙏