UPNP Support - Minor fixes #6

mbaxter · 2019-07-03T21:39:27Z

PR description

Fix a few small issues:

DefaultP2PNetwork
- Make sure NAT setup is handled before enode url is constructed
- Request UDP port forwarding for discovery service
- Add a test
Request port forwarding in JsonRpcHttpService after port is fully resolved
Fix some minor style issues in UpnpNatManager
- Use final variables where possible
- Use Optional over nullable values
Implement shutdown method in PantheonUpnpServiceConfiguration
Make package names consistent with directory structure
Remove some unnecessary dependencies

Make sure nat environment is configured before setting local node. Request discovery port forwarding for UDP port. Add a test.

shemnon · 2019-07-03T21:49:19Z

ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/network/DefaultP2PNetwork.java

@@ -378,7 +378,7 @@ private void configureNatEnvironment() {
            .get()
            .requestPortForward(
                this.config.getDiscovery().getBindPort(),
-                UpnpNatManager.Protocol.TCP,
+                UpnpNatManager.Protocol.UDP,


Do we need both TCP and UDP or is discovery UDP only?

Discovery is UDP only. There's another request for rlpx TCP port below.

mbaxter added 7 commits July 3, 2019 16:44

Fix nat setup ordering and discovery port forwarding

e1d386b

Make sure nat environment is configured before setting local node. Request discovery port forwarding for UDP port. Add a test.

Setup json-rpc port forwarding after port is fully resolved

79dc1ea

Remove unnecessary dependencies

1ba824c

Shutdown services in the reverse order they were started

74ba64c

Make package name match directory structure

34d40dc

Style - use final where possible, use Optional over Nullable values

083f897

Shutdown configuration executor

2f0a5f7

shemnon reviewed Jul 3, 2019

View reviewed changes

mbaxter mentioned this pull request Jul 3, 2019

Add UPnP Support PegaSysEng/pantheon#1334

Merged

shemnon approved these changes Jul 3, 2019

View reviewed changes

notlesh merged commit e461e69 into notlesh:upnp_support Jul 3, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

UPNP Support - Minor fixes #6

UPNP Support - Minor fixes #6

mbaxter commented Jul 3, 2019

shemnon Jul 3, 2019

mbaxter Jul 3, 2019

UPNP Support - Minor fixes #6

UPNP Support - Minor fixes #6

Conversation

mbaxter commented Jul 3, 2019

PR description

shemnon Jul 3, 2019

Choose a reason for hiding this comment

mbaxter Jul 3, 2019

Choose a reason for hiding this comment