diff --git a/p2p/net/nat/nat.go b/p2p/net/nat/nat.go index 28ffd4a5b2..ebf61f21d8 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,8 @@ 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) + // 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") +}