Add upnp only for p2p nat method (#3250)

* new natmethod enum

Signed-off-by: Frank Li <b439988l@gmail.com>

* more tests

Signed-off-by: Frank Li <b439988l@gmail.com>

* more test

Signed-off-by: Frank Li <b439988l@gmail.com>

besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java

@@ -1613,6 +1613,9 @@ public class BesuCommandTest extends CommandTestAbstract {
     parseCommand("--nat-method", "UPNP");
     verify(mockRunnerBuilder).natMethod(eq(NatMethod.UPNP));
 
+    parseCommand("--nat-method", "UPNPP2PONLY");
+    verify(mockRunnerBuilder).natMethod(eq(NatMethod.UPNPP2PONLY));
+
     parseCommand("--nat-method", "AUTO");
     verify(mockRunnerBuilder).natMethod(eq(NatMethod.AUTO));
 
@@ -1634,7 +1637,7 @@ public class BesuCommandTest extends CommandTestAbstract {
     assertThat(commandOutput.toString(UTF_8)).isEmpty();
     assertThat(commandErrorOutput.toString(UTF_8))
         .contains(
-            "Invalid value for option '--nat-method': expected one of [UPNP, DOCKER, KUBERNETES, AUTO, NONE] (case-insensitive) but was 'invalid'");
+            "Invalid value for option '--nat-method': expected one of [UPNP, UPNPP2PONLY, DOCKER, KUBERNETES, AUTO, NONE] (case-insensitive) but was 'invalid'");
   }
 
   @Test

ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/network/DefaultP2PNetwork.java

@@ -45,6 +45,7 @@ import org.hyperledger.besu.ethereum.p2p.rlpx.wire.messages.DisconnectMessage.Di
 import org.hyperledger.besu.ethereum.storage.StorageProvider;
 import org.hyperledger.besu.nat.NatMethod;
 import org.hyperledger.besu.nat.NatService;
+import org.hyperledger.besu.nat.core.NatManager;
 import org.hyperledger.besu.nat.core.domain.NatServiceType;
 import org.hyperledger.besu.nat.core.domain.NetworkProtocol;
 import org.hyperledger.besu.nat.upnp.UpnpNatManager;
@@ -67,6 +68,7 @@ import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicReference;
+import java.util.function.Consumer;
 import java.util.function.Supplier;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
@@ -236,15 +238,17 @@ public class DefaultP2PNetwork implements P2PNetwork {
                     : configuredDiscoveryPort)
             .join();
 
-    natService.ifNatEnvironment(
+    final Consumer<? super NatManager> natAction =
-        NatMethod.UPNP,
         natManager -> {
           UpnpNatManager upnpNatManager = (UpnpNatManager) natManager;
           upnpNatManager.requestPortForward(
               discoveryPort, NetworkProtocol.UDP, NatServiceType.DISCOVERY);
           upnpNatManager.requestPortForward(
               listeningPort, NetworkProtocol.TCP, NatServiceType.RLPX);
-        });
+        };
+
+    natService.ifNatEnvironment(NatMethod.UPNP, natAction);
+    natService.ifNatEnvironment(NatMethod.UPNPP2PONLY, natAction);
 
     setLocalNode(address, listeningPort, discoveryPort);
 

ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/network/DefaultP2PNetworkTest.java

@@ -253,6 +253,30 @@ public final class DefaultP2PNetworkTest {
     Assertions.assertThat(network.getLocalEnode().get().getIpAsString()).isEqualTo(externalIp);
   }
 
+  @Test
+  public void start_withNatManagerUpnpP2p() {
+    final String externalIp = "127.0.0.3";
+    config.getRlpx().setBindPort(30303);
+    config.getDiscovery().setBindPort(30301);
+
+    final UpnpNatManager upnpNatManager = mock(UpnpNatManager.class);
+    when(upnpNatManager.getNatMethod()).thenReturn(NatMethod.UPNPP2PONLY);
+    when(upnpNatManager.queryExternalIPAddress())
+        .thenReturn(CompletableFuture.completedFuture(externalIp));
+
+    final NatService natService = spy(new NatService(Optional.of(upnpNatManager)));
+    final P2PNetwork network = builder().natService(natService).build();
+
+    network.start();
+    verify(upnpNatManager)
+        .requestPortForward(eq(config.getRlpx().getBindPort()), eq(NetworkProtocol.TCP), any());
+    verify(upnpNatManager)
+        .requestPortForward(
+            eq(config.getDiscovery().getBindPort()), eq(NetworkProtocol.UDP), any());
+
+    Assertions.assertThat(network.getLocalEnode().get().getIpAsString()).isEqualTo(externalIp);
+  }
+
   @Test
   public void handlePeerBondedEvent_forListeningPeer() {
     final DefaultP2PNetwork network = network();

nat/src/main/java/org/hyperledger/besu/nat/NatMethod.java

@@ -16,6 +16,7 @@ package org.hyperledger.besu.nat;
 
 public enum NatMethod {
   UPNP,
+  UPNPP2PONLY,
   DOCKER,
   KUBERNETES,
   AUTO,

nat/src/test/java/org/hyperledger/besu/nat/NatServiceTest.java

@@ -115,6 +115,24 @@ public class NatServiceTest {
     assertThat(resultIp).isEqualTo(externalIp);
   }
 
+  @Test
+  public void assertQueryExternalIpWorksProperlyWithUpNpP2pOnly() {
+    final String fallbackExternalIp = "127.0.0.1";
+    final String externalIp = "127.0.0.3";
+    final NatManager natManager = mock(NatManager.class);
+    when(natManager.queryExternalIPAddress())
+        .thenReturn(CompletableFuture.completedFuture(externalIp));
+    when(natManager.getNatMethod()).thenReturn(NatMethod.UPNPP2PONLY);
+
+    final NatService natService = new NatService(Optional.of(natManager), true);
+
+    final String resultIp = natService.queryExternalIPAddress(fallbackExternalIp);
+
+    verify(natManager).queryExternalIPAddress();
+
+    assertThat(resultIp).isEqualTo(externalIp);
+  }
+
   @Test
   public void assertThatQueryExternalIpWorksProperlyWithoutNat() {