[Net] Add NAT-PMP port forwarding support #2338
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fuzzbawls
force-pushed
the
2021_net-add-natpmp
branch
from
66fbe6f to
6e17346
Compare
Collaborator
Author
|
rebased now that #2330 has been merged, ready for review. |
Fuzzbawls
force-pushed
the
2021_net-add-natpmp
branch
from
6e17346 to
927f293
Compare
Fuzzbawls
requested review from
furszy and
random-zebra
and removed request for
random-zebra
random-zebra
left a comment
random-zebra
left a comment
There was a problem hiding this comment.
Code review and lightly tested ACK.
Just few minor discussion points for the GUI controls.
Fuzzbawls
force-pushed
the
2021_net-add-natpmp
branch
from
927f293 to
5876841
Compare
Collaborator
Author
|
rebased to resolve conflicts and addressed feedback |
random-zebra
previously approved these changes
Jun 4, 2021
random-zebra
left a comment
random-zebra
left a comment
There was a problem hiding this comment.
Code ACK 587684114b2609f13d199c442d0b5fcd9fba1b88
furszy
reviewed
Jun 7, 2021
furszy
left a comment
furszy
left a comment
There was a problem hiding this comment.
Code review 587684114b2609f13d199c442d0b5fcd9fba1b88.
Fuzzbawls
force-pushed
the
2021_net-add-natpmp
branch
from
5876841 to
fc52bd3
Compare
This commit does not change behavior.
# Conflicts: # src/mapport.cpp
-BEGIN VERIFY SCRIPT- sed -i 's/g_upnp_interrupt/g_mapport_interrupt/' src/mapport.cpp sed -i 's/g_upnp_thread/g_mapport_thread/' src/mapport.cpp sed -i 's/LOCAL_UPNP/LOCAL_MAPPED/' src/mapport.cpp sed -i 's/\bupnp\b/mapport/' src/mapport.cpp sed -i 's/LOCAL_UPNP, /LOCAL_MAPPED,/' src/net.h -END VERIFY SCRIPT-
This commit does not change behavior. It is a prerequisite for NAT-PMP support adding.
Fuzzbawls
force-pushed
the
2021_net-add-natpmp
branch
from
fc52bd3 to
8143139
Compare
The source we are currently using is from 2015. The upstream repo has received a small number of bug fixes and improvements since then. Including one that fixes an issue for Windows users: miniupnp/libnatpmp#13. The source we are currently using is the most recent "official" release, however I don't think it's worth waiting for a new one. The maintainer was prompted to do so in Oct 2020, then again in Jan of this year, and no release has eventuated. Given libnatpmp is a new inclusion into our repository, I think we should be using this newer source. This also cleans up a few warnings we currently see in depends builds: ```bash Extracting libnatpmp... /home/ubuntu/bitcoin/depends/sources/libnatpmp-20150609.tar.gz: OK Preprocessing libnatpmp... Configuring libnatpmp... Building libnatpmp... make[1]: Entering directory '/home/ubuntu/bitcoin/depends/work/build/x86_64-w64-mingw32/libnatpmp/20150609-13efa1beb87' x86_64-w64-mingw32-gcc -Os -fPIC -Wall -DENABLE_STRNATPMPERR -c -o natpmp.o natpmp.c x86_64-w64-mingw32-gcc -Os -fPIC -Wall -DENABLE_STRNATPMPERR -c -o getgateway.o getgateway.c natpmp.c:42: warning: "EWOULDBLOCK" redefined 42 | #define EWOULDBLOCK WSAEWOULDBLOCK | In file included from natpmp.c:38: /usr/share/mingw-w64/include/errno.h:166: note: this is the location of the previous definition 166 | #define EWOULDBLOCK 140 | natpmp.c:43: warning: "ECONNREFUSED" redefined 43 | #define ECONNREFUSED WSAECONNREFUSED | In file included from natpmp.c:38: /usr/share/mingw-w64/include/errno.h:110: note: this is the location of the previous definition 110 | #define ECONNREFUSED 107 | natpmp.c:271:5: warning: ‘readnatpmpresponseorretry’ redeclared without dllimport attribute: previous dllimport ignored [-Wattributes] 271 | int readnatpmpresponseorretry(natpmp_t * p, natpmpresp_t * response) | ^~~~~~~~~~~~~~~~~~~~~~~~~ ar crs libnatpmp.a natpmp.o getgateway.o make[1]: Leaving directory '/home/ubuntu/bitcoin/depends/work/build/x86_64-w64-mingw32/libnatpmp/20150609-13efa1beb87' Staging libnatpmp... Postprocessing libnatpmp... Caching libnatpmp... ```
This fixes linking issues and mirrors what we do with miniupnpc.
furszy
reviewed
Jun 18, 2021
| ---------------------- | ||
|
|
||
| brew install autoconf automake berkeley-db4 libtool boost miniupnpc pkg-config python3 qt5 zmq libevent qrencode gmp libsodium rust | ||
| brew install autoconf automake berkeley-db4 libtool boost miniupnpc libnatpmp pkg-config python3 qt5 zmq libevent qrencode gmp libsodium rust |
There was a problem hiding this comment.
future nit: miniupnpc and libnatpmp are optional.
random-zebra
approved these changes
Jun 24, 2021
random-zebra
left a comment
random-zebra
left a comment
There was a problem hiding this comment.
utACK c198797 after rebase, and merging...
|
****Aeinsteroid54/BEPs: [NET] Add NAT-PMP post forwarding support#2337 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Built on top of #2330, this backports bitcoin#18077 and bitcoin#21209 to add NAT-PMP port forwarding support, which can function along side of or instead of UPnP.
Similar to how UPnP is treated, NAT-PMP support will be compiled in if the library is found, but the functionality is disabled by default unless passing
--enable-natpmp-defaulttoconfigure, or by using the-natpmpcommand line option at startup.A checkbox has also been added to the settings area of the GUI that allows for on-the-fly enabling/disabling of the port mapping features when saving the settings.