Add UPnP Support #1334

notlesh · 2019-04-25T21:02:21Z

This begins work on Consensys/protocol-BountiedWork#1

PR description

My initial work to wrap jupnp functionality. This is largely pulled from my repository at https://github.com/notlesh/jupnp-igd-tools where I developed and tested this code.

This is currently a work-in-progress.

This begins work on Consensys/protocol-BountiedWork#1

CLAassistant · 2019-04-25T21:02:29Z

All committers have signed the CLA.

shemnon

This is a good start. Before committing it to the main repository we will need to see it wired into the startup of Pantheon and see it mapping ports.

shemnon · 2019-04-30T14:46:25Z

ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/upnp/UpnpNatManager.java

+   * @param type is the type descriptor of the desired service
+   * @return the first instance of the given type, or null if none
+   */
+  @SuppressWarnings("rawtypes")


Please update the code so that it won't throw raw type warnings. This is not a warning we generally suppress.

The problem here is that the underlying library (jupnp) returns raw types itself. If you have a good solution, I'll gladly implement it.

Example: the registry listener "bleeds" this rawtype through an interface method: https://github.com/jupnp/jupnp/blob/ca13ee84ae4a03be93c6bb109b27668d5406c1c5/bundles/org.jupnp/src/main/java/org/jupnp/registry/DefaultRegistryListener.java#L85

And, for reference, Device: https://github.com/jupnp/jupnp/blob/master/bundles/org.jupnp/src/main/java/org/jupnp/model/meta/Device.java

ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/upnp/UpnpNatManager.java

* Make the contract size limit configurable in the genesis config, making it possible to override milestone based configurations in a private network.

…g#1234)

…SysEng#1224) * [PIE-1224] Different request limits for different request types - create `EthLimits` with max size of response according to geth (https://github.com/ethereum/go-ethereum/blob/master/eth/downloader/downloader.go) - update `EthServer` to use those constants limits instead of the default one - remove requestLimit from EthServer and EthProtocolManager constructor fix PIE-1224 * eth wire protocol request limits - add fields in `SynchronizerConfiguration` to configure per type request limit - update `EthServer` constructor to add new fields - update `EthProtocolManager` and subclasses to support new fields - update unit tests accordingly * Update SynchronizerConfiguration.java * Update EthProtocolManagerTestUtil.java * Update EthServerTest.java * Update EthProtocolManagerTest.java * Update EthServerTest.java * fix after review discussion - remove per type request limit configuration from `SynchronizerConfiguration`. - add `EthServer` constructor without new fields that use default values. - update unit tests accordingly - update instanciation of different `PantheonController` accordingly * Update EthServerTest.java * fix review - spotless - fix clean up review comment * spotlessApply

) * Add clarity to error messages in eea_sendRawTransaction * Refactor private acceptance tests and add tests for exception

- change option name - update warning condition on dependency check

…egaSysEng#1226) * [PRIV-41] Use metrics system for private state db - Use PrivacyParametersBuilder to build PrivacyParameters - refactor PrivacyParameters to expose default options - refactor test builders to use PrivacyParameters.DEFAULT - Use URI in Enclave * Fix: enclave tests from bad merge * Fix privacy acceptance tests after db configuration changes * Switch to use nested class for PrivacyParametersBuilder

There is no need to keep entire blocks during import after they have been imported. Keep just the hashes instead.

* EWP options management - create `EthereumWireProtocolConfiguration` object to hold per request limits - add `EthereumWireProtocolConfiguration` in `SynchronizerConfiguration` - create `PositiveNumber` class to handle command line options representing a positive number (a request limit for example) - add unit tests to cover the new options fixes PIE-1502 * Update EthereumWireProtocolConfiguration.java * Update PantheonCommandTest.java * fix tests * spotlessApply * fix tests and javadoc * Update pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java Co-Authored-By: abdelhamidbakhta <[email protected]> * Update pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java Co-Authored-By: abdelhamidbakhta <[email protected]> * Update pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java Co-Authored-By: abdelhamidbakhta <[email protected]> * Update pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java Co-Authored-By: abdelhamidbakhta <[email protected]> * fix * fix PR discussion * Update EthereumWireProtocolConfiguration.java

* EWP options management - create `EthereumWireProtocolConfiguration` object to hold per request limits - add `EthereumWireProtocolConfiguration` in `SynchronizerConfiguration` - create `PositiveNumber` class to handle command line options representing a positive number (a request limit for example) - add unit tests to cover the new options fixes PIE-1502 * Update EthereumWireProtocolConfiguration.java * Update PantheonCommandTest.java * fix tests * spotlessApply * fix tests and javadoc * Update pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java Co-Authored-By: abdelhamidbakhta <[email protected]> * Update pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java Co-Authored-By: abdelhamidbakhta <[email protected]> * Update pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java Co-Authored-By: abdelhamidbakhta <[email protected]> * Update pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java Co-Authored-By: abdelhamidbakhta <[email protected]> * fix * fix PR discussion * Update EthereumWireProtocolConfiguration.java * fix PR 1246 discussion PegaSysEng#1246 (comment)

mbaxter · 2019-06-28T18:54:49Z

Hey @notlesh - can you ping me with a top-level comment when you're ready for another review?

notlesh · 2019-07-02T21:41:34Z

@mbaxter I have a couple questions regarding your PR review, and beyond that the only outstanding item is moving the upnp package. Can you take a quick look at the non-resolved conversations (where you'll find my questions)?

You can also find me in gitter if you'd like -- I may be quiet, but I'll respond to PMs.

… they can be managed properly

notlesh · 2019-07-03T20:17:20Z

@mbaxter I believe I've addressed all of your feedback from your last review, please feel free to re-review. Thanks!

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

mbaxter · 2019-07-03T21:52:51Z

Pushed up a few small changes here. I think once those are in this should be merge ready!

UPNP Support - Minor fixes

notlesh · 2019-07-03T22:49:11Z

Looks good to me.

shemnon · 2019-07-04T20:58:19Z

Talked with Meredith and it looks good to go once we fix the unit test failure.

Generally we don't throw exceptions during start and stop. If we are already started we just return immediately (from a guard block) and same with stop if we are not started. In both cases we LOG.warn our failures. The impact is that this is hard to unit test the logging with Log4J2, the log results tend to be flakey. So if we fixed the start and stop methods to match the pattern in other services then the two unit tests turn into don't throw tests, verifying startup (with the two start) and no startup (with the stop before start) with the mocks.

…sts to reflect this convention

shemnon · 2019-07-06T16:18:41Z

LGTM. The build error has to do with the reference tests that were updated, there is a git submodule that may be breaking it. Those look to be unrelated to your changes and possibly may be a Jenkins issue, so I'll take the tasks to see what needs to be done to fix it. Hopefully this is the last hurdle.

Initial commit of UPnP functionality using jupnp

056f51e

This begins work on Consensys/protocol-BountiedWork#1

notlesh added 2 commits April 27, 2019 12:32

Improve documentation (esp. javadocs) for UpnpNatManager

a2fc9db

Tweak comments (appease spotless)

0e9882b

shemnon reviewed Apr 30, 2019

View reviewed changes

Add basic unit tests for UpnpNatManager

588d24b

notlesh changed the title ~~Initial commit of UPnP functionality using jupnp~~ WIP: Initial commit of UPnP functionality using jupnp May 3, 2019

notlesh and others added 23 commits May 3, 2019 00:56

Remove dead code and excessive logging from UpnpNatManager

3c8df09

Network vs node configuration (PegaSysEng#1223)

898197e

Make contract size limit configurable. (PegaSysEng#1227)

1167230

* Make the contract size limit configurable in the genesis config, making it possible to override milestone based configurations in a private network.

custom deserializer for TopicsParameter (PegaSysEng#1235)

039c620

Updated eea_getTransactionReceipt to include extra parameter

7066859

Updated Permissions Configuration File for new CLI options (PegaSysEn…

03b008c

…g#1234)

Enable pipeline chain downloader for fast sync. (PegaSysEng#1232)

2df50ff

[PRIV-45] Improve eea_sendRawTransaction error messages (PegaSysEng#1238

954a8fc

) * Add clarity to error messages in eea_sendRawTransaction * Refactor private acceptance tests and add tests for exception

Added permissioning overview (PegaSysEng#1242)

ecd1667

Update PantheonCommand.java (PegaSysEng#1245)

133c1f3

- change option name - update warning condition on dependency check

Reduce memory usage in import (PegaSysEng#1239)

69e3bda

There is no need to keep entire blocks during import after they have been imported. Keep just the hashes instead.

added note (PegaSysEng#1244)

f70a619

Fixed link (PegaSysEng#1249)

8889cf2

Added note about using node permissioning only (PegaSysEng#1243)

1eafb7d

Choose sync target based on td rather than height (PegaSysEng#1256)

1b4bceb

Added 1.0.3 updates (PegaSysEng#1250)

8b1f604

added content on free gas (PegaSysEng#1252)

bb09192

[PAN-2433] Fix netty pipeline (PegaSysEng#1257)

55b33d2

[Gradle Release Plugin] - pre tag commit: '1.0.3'.

fa17cd8

notlesh added 3 commits July 1, 2019 16:00

Create enum for TCP/UDP Protocol instead of using String

bedbb12

Shut down UpnpNatManager in Runner (same place it's started)

776c51f

Advertise detected NAT external IP address in PeerDiscoveryAgent

16a71a9

notlesh added 3 commits July 3, 2019 11:41

Track CompletableFuture's created for UPnP RemoteService discovery so…

0e08f04

… they can be managed properly

Merge remote-tracking branch 'upstream/master' into upnp_support

970bbcb

Move NAT functionality to its own root-level subproject

9733681

notlesh force-pushed the upnp_support branch from cb28151 to 9733681 Compare July 3, 2019 20:04

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

mbaxter changed the title ~~WIP: Initial commit of UPnP functionality using jupnp~~ Add UPnP Support Jul 3, 2019

Merge pull request #6 from mbaxter/upnp_support

e461e69

UPNP Support - Minor fixes

notlesh and others added 2 commits July 5, 2019 11:58

Don't throw in UpnpNatManager.start() when already started; update te…

e75c957

…sts to reflect this convention

Merge branch 'master' into upnp_support

70bf74d

update submodule reference

15d6176

shemnon approved these changes Jul 6, 2019

View reviewed changes

shemnon merged commit 16934ef into PegaSysEng:master Jul 6, 2019

colltoaction mentioned this pull request Aug 23, 2019

Implement UPnP for peer discovery rsksmart/rskj#978

Open

SergioDemianLerner mentioned this pull request Oct 22, 2019

Add UPnP Support rsksmart/rskj#1036

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add UPnP Support #1334

Add UPnP Support #1334

notlesh commented Apr 25, 2019

CLAassistant commented Apr 25, 2019 •

edited

Loading

shemnon left a comment

shemnon Apr 30, 2019

notlesh May 3, 2019

mbaxter commented Jun 28, 2019

notlesh commented Jul 2, 2019 •

edited

Loading

notlesh commented Jul 3, 2019

mbaxter commented Jul 3, 2019

notlesh commented Jul 3, 2019

shemnon commented Jul 4, 2019

shemnon commented Jul 6, 2019

Add UPnP Support #1334

Add UPnP Support #1334

Conversation

notlesh commented Apr 25, 2019

PR description

CLAassistant commented Apr 25, 2019 • edited Loading

shemnon left a comment

Choose a reason for hiding this comment

shemnon Apr 30, 2019

Choose a reason for hiding this comment

notlesh May 3, 2019

Choose a reason for hiding this comment

mbaxter commented Jun 28, 2019

notlesh commented Jul 2, 2019 • edited Loading

notlesh commented Jul 3, 2019

mbaxter commented Jul 3, 2019

notlesh commented Jul 3, 2019

shemnon commented Jul 4, 2019

shemnon commented Jul 6, 2019

CLAassistant commented Apr 25, 2019 •

edited

Loading

notlesh commented Jul 2, 2019 •

edited

Loading