rust-libp2p icon indicating copy to clipboard operation
rust-libp2p copied to clipboard

transports/tcp: simplify IfWatcher integration

Open elenaf9 opened this issue 2 years ago • 0 comments

Description

Draft until if-watch-2.0.0 is published.

Update to if-watch version v2.0.0, which allows us to simplify our integration in the tcp transport quite a bit:

  • Since IfWatcher::new is not async anymore we do not need the IfWatch wrapper and related logic anymore
  • Use if_watch::IfWatcher for both runtimes. According to this discussion, this should be ok now: https://github.com/libp2p/rust-libp2p/pull/2289#discussion_r917523292. However, I just saw now that for cfg(not(any(target_os = "ios", target_os = "linux", target_os = "macos", target_os = "windows"))) if-watch still uses async-io. Given that this won't apply for the vast majority of users I'd say this is acceptable. Alternative is to do fix https://github.com/mxinden/if-watch/issues/21 before continuing here.
  • Remove unnecessary address tracking from InAddr. See description in 9b1dc6b2b6ea7cac7db96212d6c28110111aa70d.

Related discussions: https://github.com/libp2p/rust-libp2p/pull/2289#discussion_r917403346.

Open Questions

  • We could remove InAddr completely and in case of InAddr::One just directly push an TransportEvent::NewAddress into our event queue when creating the listener. See https://github.com/elenaf9/rust-libp2p/commit/c4f2124185d9de9ac0d2dd1dbeb3c919fae6bce2. But not sure if that really simplifies things. I guess for other transports that don't have whole port-reuse logic this might make more sense.

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

elenaf9 avatar Aug 11 '22 15:08 elenaf9