Skip to content

Construct addresses using MultiAddr, defaulting to IPv6 #46

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
Apr 18, 2023

Conversation

DougAnderson444
Copy link
Contributor

@DougAnderson444 DougAnderson444 commented Apr 14, 2023

Rust-Peer (server) improvements

I was working in my own repo, but I figure I would start using this repo instead of building in a parallel silo. Here are some changes I would propose for this repo.

I know this Rust server is meant for the cloud, but seeing as I am a big fan of "running a peer at home" this PR started out as changing the default IP to v6 so home servers are easier to connect, and then I added a few more cleanup items along the way. EC2 should run on IPv6 addresses just fine too, even though it has a public IP.

Summary of changes

  • use IPv6 by default, as they offer globally available IP addresses (IPv4 is so 1998)
  • note that ipv6.is_global() is only available on nightly, I hope that's not a problem? I added a toolchain config for that. If not we can code around it with logic I suppose... use !ip.is_looback() to only add external addresses
  • use address variables (address_webrtc & address_quic)
  • only add globally available external addresses to the swarm
  • construct address by building with MultiAddr.with(..)
  • move constants to beginning of main.rs
  • match on Opt listen_address (String) to ensure it's a valid IP address (IpAddr)
  • cleanup/remove unused deps, & ping

Remaining work

  • I left the dependencies as-is for now, whenever I moved off the branches things broke, and I wanted to get these changes in incrementally. It's late at night now, so perhaps we can migrate/patch the deps in another PR?

@vercel
Copy link

vercel bot commented Apr 14, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
universal-connectivity ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 18, 2023 11:40am

@p-shahi
Copy link
Member

p-shahi commented Apr 14, 2023

Thank you @DougAnderson444 🙌

I'm guessing you tested locally?

(IPv4 is so 1998)

@marten-seemann would agree 😂 , he pointed out the same in the go-peer pull request. Yeah let's enable v6!
I'll get the v6 elastic IPs set up.

I left the dependencies as-is for now, whenever I moved off the branches things broke, and I wanted to get these changes in incrementally. It's late at night now, so perhaps we can migrate/patch the deps in another PR?

libp2p/rust-libp2p#3625 was merged into master so we're super close to moving to not relying on my forked repo. However, I still need to apply p-shahi/rust-libp2p@201c7a8 onto the master branch to get it working. I'll take care of this in a separate PR

@DougAnderson444
Copy link
Contributor Author

I'm guessing you tested locally?

Yes I tested locally, works like a charm. I didn't test on EC2 of course, I'd have to leave that to you.

libp2p/rust-libp2p#3625 was merged into master so we're super close to moving to not relying on my forked repo. However, I still need to apply p-shahi/rust-libp2p@201c7a8 onto the master branch to get it working. I'll take care of this in a separate PR

Sounds great! Yes I've been watching that PR as well, glad to see mergify finally did it's thing.

@DougAnderson444
Copy link
Contributor Author

However, I still need to apply p-shahi/rust-libp2p@201c7a8 onto the master branch to get it working.

Ah, I see what you mean, WebRTCDirect changes were merged here multiformats/rust-multiaddr#78 by @mxinden but v0.18.0 hasn't yet been released, nor used in libp2p core

@p-shahi
Copy link
Member

p-shahi commented Apr 14, 2023

Let's hold off on merging this for bit

@p-shahi
Copy link
Member

p-shahi commented Apr 17, 2023

@DougAnderson444 can you fix the merge conflicts? and then I'll merge

@DougAnderson444
Copy link
Contributor Author

Yes merging now.

Is there any particular reason why webrtc (webrtc-rs) is a dependency in Cargo.toml? Libp2p-webrtc should provide everything rust needs for the transport

@p-shahi
Copy link
Member

p-shahi commented Apr 17, 2023

Is there any particular reason why webrtc (webrtc-rs) is a dependency in Cargo.toml? Libp2p-webrtc should provide everything rust needs for the transport

I think you can remove that and multiaddr

@p-shahi p-shahi requested a review from thomaseizinger April 17, 2023 19:43
@DougAnderson444
Copy link
Contributor Author

I have removed the dead dependencies, and removed the dep on the rust toolchain channel (no more need for nightly to use !ip.is_looback()

@maschad
Copy link
Member

maschad commented Apr 18, 2023

Thanks for this @DougAnderson444 🚀

It seems there is still a conflict in the Cargo.toml , once that's resolved we should be good to land this.

@DougAnderson444
Copy link
Contributor Author

@maschad all deconflicted over here.

Copy link
Member

@maschad maschad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@maschad maschad merged commit b136592 into libp2p:main Apr 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants