protocols/{relay,dcutr}: Replace std::time::SystemTime with instant::SystemTime

[thomaseizinger]

Description

I tried to safeguard against this by banning std::time::SystemTime::now via clippy.toml but unfortunately, it doesn't work because instant re-exports std when not compiling for wasm so it still gets flagged.

Links to any relevant issues

• Fixes Error running a relay v2 client on wasm32-unknown-unknown #2984

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

[fgimenez]

Thanks for taking care @thomaseizinger I'm getting panics bc of this one too https://github.com/libp2p/rust-libp2p/blob/master/protocols/dcutr/src/protocol/outbound.rs#L29

[mxinden]

Just one small nit.

Thanks @thomaseizinger for the fix.

@fgimenez I am not sure how your WASM setup looks like. In case your local WASM node can not listen for incoming connections, I don't see a way how you can succeed in hole punching and thus I don't see the need for libp2p-dcutr. I am not saying we shouldn't do this change. Just a heads up. This will e.g. change for the browser world once we have WebRTC support. See libp2p/specs#412.

[fgimenez]

@fgimenez I am not sure how your WASM setup looks like.

Thanks for the heads up @mxinden, let me explain a bit the setup I've come up with, please don't hesitate to let me know if anything is wrong or should be done in a different way. My goal was to verify connectivity between WASM browser nodes through a relay v2 node, I was aiming to achieve something similar to what was described in this comment libp2p/js-libp2p-webrtc-direct#98 (comment). I have a wasm-pack based test case that spins up 2 browser nodes and a backend relay node, the setup looks like this:

• Each browser node runs as a separate wasm-pack process with headless chrome. The code they run is based on https://github.com/libp2p/rust-libp2p/blob/master/protocols/dcutr/examples/dcutr.rs and both nodes are set in listen mode (they are instructed to listen on the relayed multiaddr) with two differences:
  • the transport uses libp2p_wasm_ext::ExtTransport::new(libp2p_wasm_ext::ffi::websocket_transport())
  • clients are not set to listen on all interfaces, only on the relayed multiaddr.
• The backend relay server is based on https://github.com/libp2p/rust-libp2p/blob/master/protocols/relay/examples/relay_v2.rs
• All the nodes run locally

Regarding the test case itself:

• One of the browsers dials on the relayed multiaddr of the other, something like: /ip4/127.0.0.1/tcp/38615/ws/p2p/12D3KooWDpJ7As7BWAwRMfu1VU2WCqNjvq387JEYKDBj4kx6nXTN/p2p-circuit/p2p/12D3KooWP285Hw3CSTdr9oU6Ezz4hDoi6XS5vfDjjNeTJ1uFMGvp
• each browser node checks that it receives a ping from the other.

The test is passing with the changes in this PR, I've attached logs for the 3 nodes
node-logs.zip

In case your local WASM node can not listen for incoming connections, I don't see a way how you can succeed in hole punching and thus I don't see the need for libp2p-dcutr. I am not saying we shouldn't do this change. Just a heads up. This will e.g. change for the browser world once we have WebRTC support. See libp2p/specs#412.

As explained above, this setup is working fine just with websockets, and as I understand it, it requires libp2p-dcutr to work, is that correct? please let me know if I'm missing something, or if this is not the right approach.

[mxinden]

As explained above, this setup is working fine just with websockets, and as I understand it, it requires libp2p-dcutr to work, is that correct? please let me know if I'm missing something, or if this is not the right approach.

@fgimenez I am not sure I am following. Is the final connection between the two nodes a relayed or a direct connection?

DCUtR upgrades a relayed connection to a direct connection. In case you can not establish a direct connection (between two browsers), I don't see how DCUtR can help you.

libp2p/specs#412 will give you browser-to-server connectivity. For your WASM node you would need to adapt ExtTransport for the browser's WebRTC stack. This is a medium size undertaking. Contributions would be greatly appreciated.

Long term, libp2p/specs#412 will give you browser-to-browser connectivity including hole punching.

[fgimenez]

@fgimenez I am not sure I am following. Is the final connection between the two nodes a relayed or a direct connection?

DCUtR upgrades a relayed connection to a direct connection. In case you can not establish a direct connection (between two browsers), I don't see how DCUtR can help you.

@mxinden you are right, one of the browsers is dialing the other on the relayed address, following the hole-punching example I added the DCUtR behaviour but I can see in the logs how the upgrade fails, the error comes from https://github.com/libp2p/rust-libp2p/blob/master/protocols/dcutr/src/protocol/inbound.rs#L57
I was confused because the test just checks for ping events, and each browser is able to receive a ping from the other one, will check more deeply how this works.

libp2p/specs#412 will give you browser-to-server connectivity. For your WASM node you would need to adapt ExtTransport for the browser's WebRTC stack. This is a medium size undertaking. Contributions would be greatly appreciated.

Awesome, would love to work on it, should be similar to what is done here for websockets right? https://github.com/libp2p/rust-libp2p/tree/master/transports/wasm-ext/src

Long term, libp2p/specs#412 will give you browser-to-browser connectivity including hole punching.

Very cool 👍

[mxinden]

libp2p/specs#412 will give you browser-to-server connectivity. For your WASM node you would need to adapt ExtTransport for the browser's WebRTC stack. This is a medium size undertaking. Contributions would be greatly appreciated.

Awesome, would love to work on it, should be similar to what is done here for websockets right? https://github.com/libp2p/rust-libp2p/tree/master/transports/wasm-ext/src

Yes. Correct. Though it does include an additional Noise handshake and message framing. See libp2p/specs#412.