ETCM-541 attempt UPnP port mapping to aid in peer discovery & connection #929
Conversation
jvdp
requested review from
AnastasiiaL,
aakoshh,
bsuieric,
leo-bogastry and
ten15bit
![pullrequest[bot]](https://avatars.githubusercontent.com/in/3434?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ This pull request was sent to the PullRequest network.
@jvdp you can click here to see the review status or cancel the code review job - or - cancel by adding [!pr] to the title of the pull request.
| } | ||
|
|
||
| # Try automatic port forwarding via UPnP | ||
| automatic-port-forwarding = true |
There was a problem hiding this comment.
Should this be on by default?
There was a problem hiding this comment.
For it to be useful, I think so. Someone who wouldn't map the ports themselves probably won't be inclined to enable this manually either, right?
aakoshh
left a comment
aakoshh
left a comment
There was a problem hiding this comment.
Nice work 👍
Couple of comments about logging and repeatability.
src/main/scala/io/iohk/ethereum/network/discovery/DiscoveryServiceBuilder.scala
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
This looks nice and clean overall. I didn't see any issues beyond what was already pointed out.
Reviewed with ❤️ by PullRequest
aakoshh
left a comment
aakoshh
left a comment
There was a problem hiding this comment.
Awesome, thanks for persevering with me! 🏁
With this addition Mantis will try to use UPnP to open an UDP port for Kademlia / peer discovery and a TCP port for the Ethereum protocol. Can be disabled via
mantis.network.automatic-port-forwarding = false(defaults to true.)