feat(networking): Implement support for UPnP #444

petarjuki7 · 2025-07-20T17:57:04Z

Issue Addressed

Addresses issue #407

Proposed Changes

Added libp2p::upnp functionality.
Changed the CLI arguments so the user can opt-out of upnp with --disable-upnp.
Listening to upnp events.

Additional Info

This PR currently doesn't do anything on upnp events regarding updating ENR ports.
Relates to #255.

petarjuki7 · 2025-07-20T18:02:04Z

Still in draft because not sure how to respond to events.
cc: @dknopik

dknopik · 2025-07-21T11:20:03Z

Nice start! Regarding the events, I think we have to do #255 first (now that I think about it again). After all, it makes no sense to get external ports via UPnP if we can not advertise them.

Let me know if you want to take a look at #255 as well (no pressure though!). Else, I can tackle it in the next few days and let you know when it's ready.

petarjuki7 · 2025-07-23T16:08:36Z

Let me know if you want to take a look at #255 as well (no pressure though!). Else, I can tackle it in the next few days and let you know when it's ready.

Hi, sorry for not responding sooner, had something urgent to take care off. Sure, I can take a look, I will write questions / ideas in that issue then.

petarjuki7 · 2025-08-21T16:12:32Z

Lighthouse also has this part:
https://github.com/sigp/lighthouse/blob/stable/beacon_node/network/src/nat.rs

Where they "manually" construct port mappings for addresses they know are listening to (provided from the config).
(https://github.com/sigp/lighthouse/blob/cfb1f7331064b758c6786e4e1dc15507af5ff5d1/beacon_node/network/src/service.rs#L229-#L244).

Not sure if we also need it or it will be caught by the "regular" libp2p behaviour

diegomrsantos · 2025-09-10T19:13:14Z

@dknopik @petarjuki7 how about we continue this?

dknopik · 2025-09-10T20:52:01Z

@diegomrsantos sure, feel free to continue on this already, I'll focus on testing the release for now

diegomrsantos · 2025-09-10T21:02:23Z

@diegomrsantos sure, feel free to continue on this already, I'll focus on testing the release for now

I meant @petarjuki7 :)

dknopik · 2025-09-16T07:16:40Z

@petarjuki7 As this is not a fix and will not end up in the first mainnet release, unstable was in fact the adequate target branch

diegomrsantos · 2025-09-25T15:18:49Z

@petarjuki7 is this ready for review?

petarjuki7 · 2025-09-29T08:29:49Z

@petarjuki7 is this ready for review?

Sorry, hadn't seen this sooner, yes it is.

Copilot

Pull Request Overview

This PR implements UPnP support to automatically establish external port mappings for the Anchor network client. Users can opt-out of UPnP with the --disable-upnp CLI flag.

Added UPnP behavior to the network stack with event handling for port mapping
Introduced CLI option to disable UPnP functionality
Configured UPnP to update ENR ports when external addresses are mapped

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
anchor/network/src/network.rs	Implements UPnP event handling and ENR port updates
anchor/network/src/config.rs	Adds upnp_enabled configuration field
anchor/network/src/behaviour.rs	Integrates UPnP behavior into the network behavior stack
anchor/network/Cargo.toml	Adds upnp feature to libp2p dependency
anchor/client/src/config.rs	Maps CLI disable_upnp flag to network configuration
anchor/client/src/cli.rs	Adds --disable-upnp CLI argument

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-09-29T15:40:45Z

anchor/network/src/network.rs

+                                        }
+
+                                    },
+                                    libp2p::upnp::Event::ExpiredExternalAddr(_) => {},


The ExpiredExternalAddr event is ignored but should likely update the ENR to remove the expired port mapping. Consider implementing proper cleanup logic to maintain accurate ENR records.

Suggested change

libp2p::upnp::Event::ExpiredExternalAddr(_) => {},

libp2p::upnp::Event::ExpiredExternalAddr(addr) => {

info!(%addr, "UPnP route expired");

let mut iter = addr.iter();

let is_ipv6 = {

let addr = iter.next();

matches!(addr, Some(Protocol::Ip6(_)))

};

match iter.next() {

Some(Protocol::Udp(_udp_port)) => match iter.next() {

Some(Protocol::QuicV1) => {

if let Err(e) = self.discovery().try_update_port(false, is_ipv6, 0) {

warn!(error = e, "Failed to remove UDP port from ENR");

}

}

_ => {

trace!(%addr, "UPnP expired multiaddr from unknown transport");

}

},

Some(Protocol::Tcp(_tcp_port)) => {

if let Err(e) = self.discovery().try_update_port(true, is_ipv6, 0) {

warn!(error = e, "Failed to remove TCP port from ENR");

}

}

_ => {

trace!(%addr, "UPnP expired multiaddr from unknown transport");

}

}

},

(in reply to copilot)

This does not really make sense. There is no advantage of setting the port to 0 instead of just leaving it as is.

If we are still publicly reachable, discv5 will notify us of our new external address and if we are not, our ENR will not be propagated anyway.

Copilot · 2025-09-29T15:40:46Z

anchor/network/src/network.rs

+                                            let addr = iter.next();
+                                            matches!(addr, Some(Protocol::Ip6(_)))


The variable name 'addr' is used for both the full multiaddr and the first protocol component, which is confusing. Consider renaming the inner variable to 'first_protocol' or similar for clarity.

Suggested change

let addr = iter.next();

matches!(addr, Some(Protocol::Ip6(_)))

let first_protocol = iter.next();

matches!(first_protocol, Some(Protocol::Ip6(_)))

dknopik · 2025-09-29T20:29:05Z

anchor/network/src/network.rs

+                                match upnp_event {
+                                    libp2p::upnp::Event::NewExternalAddr(addr) => {
+                                        info!(%addr, "UPnP route established");
+                                        let mut iter = addr.iter();
+                                        let is_ipv6 = {
+                                            let addr = iter.next();
+                                            matches!(addr, Some(Protocol::Ip6(_)))
+                                        };
+                                        match iter.next() {
+                                            Some(Protocol::Udp(udp_port)) => match iter.next() {
+                                                Some(Protocol::QuicV1) => {
+                                                    if let Err(e) =
+                                                        self.discovery().try_update_port(false, is_ipv6, udp_port)
+                                                    {
+                                                        warn!(error = e, "Failed to update ENR");
+                                                    }
+                                                }
+                                                _ => {
+                                                    trace!(%addr, "UPnP address mapped multiaddr from unknown transport");
+                                                }
+                                            },
+                                            Some(Protocol::Tcp(tcp_port)) => {
+                                                if let Err(e) = self.discovery().try_update_port(true, is_ipv6, tcp_port) {
+                                                    warn!(error = e, "Failed to update ENR");
+                                                }
+                                            }
+                                            _ => {
+                                                trace!(%addr, "UPnP address mapped multiaddr from unknown transport");
+                                            }
+                                        }
+
+                                    },
+                                    libp2p::upnp::Event::ExpiredExternalAddr(_) => {},
+                                    libp2p::upnp::Event::GatewayNotFound => info!("UPnP not available."),
+                                    libp2p::upnp::Event::NonRoutableGateway => info!("UPnP is available but gateway is not exposed to public network"),
+                                }


Can we move this into it's own function?

dknopik · 2025-09-29T20:33:11Z

anchor/network/src/network.rs

+                                        }
+
+                                    },
+                                    libp2p::upnp::Event::ExpiredExternalAddr(_) => {},


(in reply to copilot)

This does not really make sense. There is no advantage of setting the port to 0 instead of just leaving it as is.

If we are still publicly reachable, discv5 will notify us of our new external address and if we are not, our ENR will not be propagated anyway.

petarjuki7 changed the title ~~add_upnp_behaviour~~ Implement support for UPnP Jul 20, 2025

petarjuki7 changed the title ~~Implement support for UPnP~~ feat(networking): Implement support for UPnP Jul 20, 2025

petarjuki7 marked this pull request as draft July 20, 2025 18:00

petarjuki7 mentioned this pull request Jul 28, 2025

feat(networking): update ports in ENR #457

Merged

petarjuki7 marked this pull request as ready for review August 21, 2025 16:12

petarjuki7 self-assigned this Sep 15, 2025

petarjuki7 added 2 commits September 15, 2025 15:31

add_upnp_behaviour

583ad97

implement NewExternalAddr event handler

588f715

petarjuki7 force-pushed the implement_upnp_support branch from 5cb9b63 to 588f715 Compare September 15, 2025 13:33

petarjuki7 changed the base branch from unstable to release-v0.3.0 September 15, 2025 13:33

petarjuki7 force-pushed the implement_upnp_support branch from 588f715 to 93c84d7 Compare September 16, 2025 10:41

petarjuki7 changed the base branch from release-v0.3.0 to unstable September 16, 2025 10:42

petarjuki7 force-pushed the implement_upnp_support branch from 93c84d7 to 588f715 Compare September 16, 2025 10:49

Merge branch 'unstable' into implement_upnp_support

ed10747

diegomrsantos requested a review from Copilot September 29, 2025 15:40

Copilot AI reviewed Sep 29, 2025

View reviewed changes

diegomrsantos requested review from dknopik and jxs September 29, 2025 17:15

dknopik reviewed Sep 29, 2025

View reviewed changes

-                                    libp2p::upnp::Event::ExpiredExternalAddr(_) => {},
+                                    libp2p::upnp::Event::ExpiredExternalAddr(addr) => {
+                                        info!(%addr, "UPnP route expired");
+                                        let mut iter = addr.iter();
+                                        let is_ipv6 = {
+                                            let addr = iter.next();
+                                            matches!(addr, Some(Protocol::Ip6(_)))
+                                        };
+                                        match iter.next() {
+                                            Some(Protocol::Udp(_udp_port)) => match iter.next() {
+                                                Some(Protocol::QuicV1) => {
+                                                    if let Err(e) = self.discovery().try_update_port(false, is_ipv6, 0) {
+                                                        warn!(error = e, "Failed to remove UDP port from ENR");
+                                                    }
+                                                }
+                                                _ => {
+                                                    trace!(%addr, "UPnP expired multiaddr from unknown transport");
+                                                }
+                                            },
+                                            Some(Protocol::Tcp(_tcp_port)) => {
+                                                if let Err(e) = self.discovery().try_update_port(true, is_ipv6, 0) {
+                                                    warn!(error = e, "Failed to remove TCP port from ENR");
+                                                }
+                                            }
+                                            _ => {
+                                                trace!(%addr, "UPnP expired multiaddr from unknown transport");
+                                            }
+                                        }
+                                    },

		let addr = iter.next();
		matches!(addr, Some(Protocol::Ip6(_)))

feat(networking): Implement support for UPnP #444

Are you sure you want to change the base?

feat(networking): Implement support for UPnP #444

Conversation

petarjuki7 commented Jul 20, 2025

Issue Addressed

Proposed Changes

Additional Info

Uh oh!

petarjuki7 commented Jul 20, 2025

Uh oh!

dknopik commented Jul 21, 2025

Uh oh!

petarjuki7 commented Jul 23, 2025

Uh oh!

petarjuki7 commented Aug 21, 2025

Uh oh!

diegomrsantos commented Sep 10, 2025

Uh oh!

dknopik commented Sep 10, 2025

Uh oh!

diegomrsantos commented Sep 10, 2025

Uh oh!

dknopik commented Sep 16, 2025

Uh oh!

diegomrsantos commented Sep 25, 2025

Uh oh!

petarjuki7 commented Sep 29, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Sep 29, 2025

Choose a reason for hiding this comment

Uh oh!

dknopik Sep 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 29, 2025

Choose a reason for hiding this comment

Uh oh!

dknopik Sep 29, 2025

Choose a reason for hiding this comment

Uh oh!

dknopik Sep 29, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!