From a2993c18f7e9a005e23feff49d7f3296188be631 Mon Sep 17 00:00:00 2001
From: sukun <sukunrt@gmail.com>
Date: Fri, 10 Jan 2025 18:25:24 +0000
Subject: [PATCH] nat: ignore mapping if external port is 0 (#3094)

Co-authored-by: wlynxg <liuauthor@foxmail.com>
---
 p2p/net/nat/nat.go      |  6 +++++-
 p2p/net/nat/nat_test.go | 15 +++++++++++++++
 2 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/p2p/net/nat/nat.go b/p2p/net/nat/nat.go
index 28ffd4a5b2..23af58075a 100644
--- a/p2p/net/nat/nat.go
+++ b/p2p/net/nat/nat.go
@@ -107,7 +107,8 @@ func (nat *NAT) GetMapping(protocol string, port int) (addr netip.AddrPort, foun
 		return netip.AddrPort{}, false
 	}
 	extPort, found := nat.mappings[entry{protocol: protocol, port: port}]
-	if !found {
+	// The mapping may have an invalid port.
+	if !found || extPort == 0 {
 		return netip.AddrPort{}, false
 	}
 	return netip.AddrPortFrom(nat.extAddr, uint16(extPort)), true
@@ -135,6 +136,9 @@ func (nat *NAT) AddMapping(ctx context.Context, protocol string, port int) error
 	// do it once synchronously, so first mapping is done right away, and before exiting,
 	// allowing users -- in the optimistic case -- to use results right after.
 	extPort := nat.establishMapping(ctx, protocol, port)
+	// Don't validate the mapping here, we refresh the mappings based on this map.
+	// We can try getting a port again in case it succeeds. In the worst case,
+	// this is one extra LAN request every few minutes.
 	nat.mappings[entry{protocol: protocol, port: port}] = extPort
 	return nil
 }
diff --git a/p2p/net/nat/nat_test.go b/p2p/net/nat/nat_test.go
index 772c876c72..e370fc8907 100644
--- a/p2p/net/nat/nat_test.go
+++ b/p2p/net/nat/nat_test.go
@@ -68,3 +68,18 @@ func TestRemoveMapping(t *testing.T) {
 	_, found = nat.GetMapping("tcp", 10000)
 	require.False(t, found, "didn't expect port mapping for deleted mapping")
 }
+
+func TestAddMappingInvalidPort(t *testing.T) {
+	mockNAT, reset := setupMockNAT(t)
+	defer reset()
+
+	mockNAT.EXPECT().GetExternalAddress().Return(net.IPv4(1, 2, 3, 4), nil)
+	nat, err := DiscoverNAT(context.Background())
+	require.NoError(t, err)
+
+	mockNAT.EXPECT().AddPortMapping(gomock.Any(), "tcp", 10000, gomock.Any(), MappingDuration).Return(0, nil)
+	require.NoError(t, nat.AddMapping(context.Background(), "tcp", 10000))
+
+	_, found := nat.GetMapping("tcp", 10000)
+	require.False(t, found, "didn't expect a port mapping for invalid nat-ed port")
+}