nat: ignore mapping if external port is 0

closes: #3057
rest of the work is in: #3075

Co-authored-by: wlynxg <liuauthor@foxmail.com>

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
 }

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")
+}