Skip to content

Commit c33eb23

Browse files
committed
Merge bitcoin/bitcoin#30043: net: Replace libnatpmp with built-in PCP+NATPMP implementation
5c7cacf ci: Remove natpmp build option and libnatpmp dependency (laanwj) 7e7ec98 doc: Remove mention of natpmp build options (laanwj) 061c3e3 depends: Drop natpmp and associated option from depends (laanwj) 20a18bf build: Drop libnatpmp from build system (laanwj) 7b04709 qt: Changes for built-in PCP+NAT-PMP (laanwj) 52f8ef6 net: Replace libnatpmp with built-in NATPMP+PCP implementation in mapport (laanwj) 97c9717 net: Add PCP and NATPMP implementation (laanwj) d72df63 net: Use GetLocalAddresses in Discover (laanwj) e020304 net: Add netif utility (laanwj) 754e425 crypto: Add missing WriteBE16 function (laanwj) Pull request description: Continues #30005. Closes #17012.. This PR adds PCP (Port Control Protocol) from [RFC6887](https://datatracker.ietf.org/doc/html/rfc6887). This adds, in addition to the existing IPv4 port mapping (which now uses PCP, with fallback to NAT-PMP), support for IPv6 pinholing-that is, opening a port on the firewall to make it reachable. PCP, like NAT-PMP is a simple UDP-based protocol, and the implementation is self-contained, so this gets rid of lthe libnatpnp dependency without adding a new one. It should otherwise be a drop-in replacement. NAT-PMP fallback is implemented so this will not make router support worse. For now it is disabled by default, though in the future (not in this PR) we could consider enable it by default to increase the number of connectable nodes without adding significant attack surface. To test: ```bash bitcoind -regtest -natpmp=1 -debug=net ``` (most of the changes in this PR are, ironically, removing the libnatpmp dependency and associated build system and build docs) ## TODO - [x] Default gateway discovery on Linux / FreeBSD - [x] Default gateway discovery on Windows - [x] Default gateway discovery on MacOS - [x] Either solve FreeBSD compile issue (probably upstream issue) or remove FreeBSD support ## Things to consider for follow-up PRs - bitcoin/bitcoin#30043 (comment) avoid unreachable nets (not given to -onlynet=) - bitcoin/bitcoin#30043 (comment) could announce an addr:port where we do not listen (no -bind) - bitcoin/bitcoin#30043 (comment) could announce the wrong port because it uses GetListenPort() - bitcoin/bitcoin#30043 (comment) if we requested one port but another was assigned, then which one to use in the renewal? - bitcoin/bitcoin#30043 (comment) Use `GetAdapterAddresses` to discover local addresses for Windows ACKs for top commit: Sjors: ACK 5c7cacf achow101: ACK 5c7cacf vasild: ACK 5c7cacf Tree-SHA512: e35b69e56d5f5449a3d48a799f9b7b65107c65eeb3e245c2c1e9d42221e469ca5ead90afae423160601cd664dd553a51c859e04f4492f335b064aae3bf23e3bc
2 parents f3c74c4 + 5c7cacf commit c33eb23

38 files changed

+1049
-283
lines changed

.github/workflows/ci.yml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -67,12 +67,12 @@ jobs:
6767
echo "TEST_BASE=$(git rev-list -n$((${{ env.MAX_COUNT }} + 1)) --reverse HEAD $EXCLUDE_MERGE_BASE_ANCESTORS | head -1)" >> "$GITHUB_ENV"
6868
- run: |
6969
sudo apt-get update
70-
sudo apt-get install clang ccache build-essential cmake pkg-config python3-zmq libevent-dev libboost-dev libsqlite3-dev libdb++-dev systemtap-sdt-dev libminiupnpc-dev libnatpmp-dev libzmq3-dev qtbase5-dev qttools5-dev qttools5-dev-tools qtwayland5 libqrencode-dev -y
70+
sudo apt-get install clang ccache build-essential cmake pkg-config python3-zmq libevent-dev libboost-dev libsqlite3-dev libdb++-dev systemtap-sdt-dev libminiupnpc-dev libzmq3-dev qtbase5-dev qttools5-dev qttools5-dev-tools qtwayland5 libqrencode-dev -y
7171
- name: Compile and run tests
7272
run: |
7373
# Run tests on commits after the last merge commit and before the PR head commit
7474
# Use clang++, because it is a bit faster and uses less memory than g++
75-
git rebase --exec "echo Running test-one-commit on \$( git log -1 ) && CC=clang CXX=clang++ cmake -B build -DWITH_ZMQ=ON -DBUILD_GUI=ON -DBUILD_BENCH=ON -DBUILD_FUZZ_BINARY=ON -DWITH_BDB=ON -DWITH_NATPMP=ON -DWITH_MINIUPNPC=ON -DWITH_USDT=ON && cmake --build build -j $(nproc) && ctest --test-dir build -j $(nproc) && ./build/test/functional/test_runner.py -j $(( $(nproc) * 2 ))" ${{ env.TEST_BASE }}
75+
git rebase --exec "echo Running test-one-commit on \$( git log -1 ) && CC=clang CXX=clang++ cmake -B build -DWITH_ZMQ=ON -DBUILD_GUI=ON -DBUILD_BENCH=ON -DBUILD_FUZZ_BINARY=ON -DWITH_BDB=ON -DWITH_MINIUPNPC=ON -DWITH_USDT=ON && cmake --build build -j $(nproc) && ctest --test-dir build -j $(nproc) && ./build/test/functional/test_runner.py -j $(( $(nproc) * 2 ))" ${{ env.TEST_BASE }}
7676
7777
macos-native-arm64:
7878
name: 'macOS 14 native, arm64, no depends, sqlite only, gui'
@@ -105,7 +105,7 @@ jobs:
105105
run: |
106106
# A workaround for "The `brew link` step did not complete successfully" error.
107107
brew install --quiet python@3 || brew link --overwrite python@3
108-
brew install --quiet ninja pkg-config gnu-getopt ccache boost libevent miniupnpc libnatpmp zeromq qt@5 qrencode
108+
brew install --quiet ninja pkg-config gnu-getopt ccache boost libevent miniupnpc zeromq qt@5 qrencode
109109
110110
- name: Set Ccache directory
111111
run: echo "CCACHE_DIR=${RUNNER_TEMP}/ccache_dir" >> "$GITHUB_ENV"

CMakeLists.txt

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -121,11 +121,6 @@ option(REDUCE_EXPORTS "Attempt to reduce exported symbols in the resulting execu
121121
option(WERROR "Treat compiler warnings as errors." OFF)
122122
option(WITH_CCACHE "Attempt to use ccache for compiling." ON)
123123

124-
option(WITH_NATPMP "Enable NAT-PMP." OFF)
125-
if(WITH_NATPMP)
126-
find_package(NATPMP MODULE REQUIRED)
127-
endif()
128-
129124
option(WITH_MINIUPNPC "Enable UPnP." OFF)
130125
if(WITH_MINIUPNPC)
131126
find_package(MiniUPnPc MODULE REQUIRED)
@@ -239,7 +234,6 @@ if(BUILD_FOR_FUZZING)
239234
set(BUILD_WALLET_TOOL OFF)
240235
set(BUILD_GUI OFF)
241236
set(ENABLE_EXTERNAL_SIGNER OFF)
242-
set(WITH_NATPMP OFF)
243237
set(WITH_MINIUPNPC OFF)
244238
set(WITH_ZMQ OFF)
245239
set(BUILD_TESTS OFF)
@@ -621,9 +615,7 @@ if(ENABLE_WALLET)
621615
message(" - legacy wallets (Berkeley DB) ..... ${WITH_BDB}")
622616
endif()
623617
message(" external signer ..................... ${ENABLE_EXTERNAL_SIGNER}")
624-
message(" port mapping:")
625-
message(" - using NAT-PMP .................... ${WITH_NATPMP}")
626-
message(" - using UPnP ....................... ${WITH_MINIUPNPC}")
618+
message(" port mapping using UPnP ............. ${WITH_MINIUPNPC}")
627619
message(" ZeroMQ .............................. ${WITH_ZMQ}")
628620
message(" USDT tracing ........................ ${WITH_USDT}")
629621
message(" QR code (GUI) ....................... ${WITH_QRENCODE}")

CMakePresets.json

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,6 @@
8686
"WITH_BDB": "ON",
8787
"WITH_MINIUPNPC": "ON",
8888
"WITH_MULTIPROCESS": "ON",
89-
"WITH_NATPMP": "ON",
9089
"WITH_QRENCODE": "ON",
9190
"WITH_SQLITE": "ON",
9291
"WITH_USDT": "ON",

ci/test/00_setup_env_mac_native.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ export LC_ALL=C.UTF-8
1111
export PIP_PACKAGES="--break-system-packages zmq"
1212
export GOAL="install"
1313
export CMAKE_GENERATOR="Ninja"
14-
export BITCOIN_CONFIG="-DBUILD_GUI=ON -DWITH_ZMQ=ON -DWITH_MINIUPNPC=ON -DWITH_NATPMP=ON -DREDUCE_EXPORTS=ON"
14+
export BITCOIN_CONFIG="-DBUILD_GUI=ON -DWITH_ZMQ=ON -DWITH_MINIUPNPC=ON -DREDUCE_EXPORTS=ON"
1515
export CI_OS_NAME="macos"
1616
export NO_DEPENDS=1
1717
export OSX_SDK=""

ci/test/00_setup_env_native_asan.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ else
1919
fi
2020

2121
export CONTAINER_NAME=ci_native_asan
22-
export PACKAGES="systemtap-sdt-dev clang-18 llvm-18 libclang-rt-18-dev python3-zmq qtbase5-dev qttools5-dev qttools5-dev-tools libevent-dev libboost-dev libdb5.3++-dev libminiupnpc-dev libnatpmp-dev libzmq3-dev libqrencode-dev libsqlite3-dev ${BPFCC_PACKAGE}"
22+
export PACKAGES="systemtap-sdt-dev clang-18 llvm-18 libclang-rt-18-dev python3-zmq qtbase5-dev qttools5-dev qttools5-dev-tools libevent-dev libboost-dev libdb5.3++-dev libminiupnpc-dev libzmq3-dev libqrencode-dev libsqlite3-dev ${BPFCC_PACKAGE}"
2323
export NO_DEPENDS=1
2424
export GOAL="install"
2525
export BITCOIN_CONFIG="\

ci/test/00_setup_env_native_previous_releases.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ export CONTAINER_NAME=ci_native_previous_releases
1010
export CI_IMAGE_NAME_TAG="docker.io/ubuntu:22.04"
1111
# Use minimum supported python3.9 (or best effort 3.10) and gcc-11, see doc/dependencies.md
1212
export PACKAGES="gcc-11 g++-11 python3-zmq"
13-
export DEP_OPTS="NO_UPNP=1 NO_NATPMP=1 DEBUG=1 CC=gcc-11 CXX=g++-11"
13+
export DEP_OPTS="NO_UPNP=1 DEBUG=1 CC=gcc-11 CXX=g++-11"
1414
export TEST_RUNNER_EXTRA="--previous-releases --coverage --extended --exclude feature_dbcrash" # Run extended tests so that coverage does not fail, but exclude the very slow dbcrash
1515
export RUN_UNIT_TESTS_SEQUENTIAL="true"
1616
export RUN_UNIT_TESTS="false"

ci/test/00_setup_env_native_tidy.sh

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ export LC_ALL=C.UTF-8
99
export CI_IMAGE_NAME_TAG="docker.io/ubuntu:24.04"
1010
export CONTAINER_NAME=ci_native_tidy
1111
export TIDY_LLVM_V="18"
12-
export PACKAGES="clang-${TIDY_LLVM_V} libclang-${TIDY_LLVM_V}-dev llvm-${TIDY_LLVM_V}-dev libomp-${TIDY_LLVM_V}-dev clang-tidy-${TIDY_LLVM_V} jq libevent-dev libboost-dev libminiupnpc-dev libnatpmp-dev libzmq3-dev systemtap-sdt-dev qtbase5-dev qttools5-dev qttools5-dev-tools libqrencode-dev libsqlite3-dev libdb++-dev"
12+
export PACKAGES="clang-${TIDY_LLVM_V} libclang-${TIDY_LLVM_V}-dev llvm-${TIDY_LLVM_V}-dev libomp-${TIDY_LLVM_V}-dev clang-tidy-${TIDY_LLVM_V} jq libevent-dev libboost-dev libminiupnpc-dev libzmq3-dev systemtap-sdt-dev qtbase5-dev qttools5-dev qttools5-dev-tools libqrencode-dev libsqlite3-dev libdb++-dev"
1313
export NO_DEPENDS=1
1414
export RUN_UNIT_TESTS=false
1515
export RUN_FUNCTIONAL_TESTS=false
@@ -18,7 +18,7 @@ export RUN_CHECK_DEPS=true
1818
export RUN_TIDY=true
1919
export GOAL="install"
2020
export BITCOIN_CONFIG="\
21-
-DWITH_ZMQ=ON -DBUILD_GUI=ON -DBUILD_BENCH=ON -DWITH_NATPMP=ON -DWITH_MINIUPNPC=ON -DWITH_USDT=ON -DWITH_BDB=ON -DWARN_INCOMPATIBLE_BDB=OFF \
21+
-DWITH_ZMQ=ON -DBUILD_GUI=ON -DBUILD_BENCH=ON -DWITH_MINIUPNPC=ON -DWITH_USDT=ON -DWITH_BDB=ON -DWARN_INCOMPATIBLE_BDB=OFF \
2222
-DENABLE_HARDENING=OFF \
2323
-DCMAKE_C_COMPILER=clang-${TIDY_LLVM_V} \
2424
-DCMAKE_CXX_COMPILER=clang++-${TIDY_LLVM_V} \

ci/test/00_setup_env_native_valgrind.sh

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,14 @@ export LC_ALL=C.UTF-8
88

99
export CI_IMAGE_NAME_TAG="docker.io/ubuntu:24.04"
1010
export CONTAINER_NAME=ci_native_valgrind
11-
export PACKAGES="valgrind clang-16 llvm-16 libclang-rt-16-dev python3-zmq libevent-dev libboost-dev libdb5.3++-dev libminiupnpc-dev libnatpmp-dev libzmq3-dev libsqlite3-dev"
11+
export PACKAGES="valgrind clang-16 llvm-16 libclang-rt-16-dev python3-zmq libevent-dev libboost-dev libdb5.3++-dev libminiupnpc-dev libzmq3-dev libsqlite3-dev"
1212
export USE_VALGRIND=1
1313
export NO_DEPENDS=1
1414
export TEST_RUNNER_EXTRA="--exclude feature_init,rpc_bind,feature_bind_extra" # feature_init excluded for now, see https://github.com/bitcoin/bitcoin/issues/30011 ; bind tests excluded for now, see https://github.com/bitcoin/bitcoin/issues/17765#issuecomment-602068547
1515
export GOAL="install"
1616
# TODO enable GUI
1717
export BITCOIN_CONFIG="\
18-
-DWITH_ZMQ=ON -DWITH_BDB=ON -DWITH_NATPMP=ON -DWITH_MINIUPNPC=ON -DWARN_INCOMPATIBLE_BDB=OFF -DBUILD_GUI=OFF \
18+
-DWITH_ZMQ=ON -DWITH_BDB=ON -DWITH_MINIUPNPC=ON -DWARN_INCOMPATIBLE_BDB=OFF -DBUILD_GUI=OFF \
1919
-DCMAKE_C_COMPILER=clang-16 \
2020
-DCMAKE_CXX_COMPILER=clang++-16 \
2121
"

cmake/module/FindNATPMP.cmake

Lines changed: 0 additions & 32 deletions
This file was deleted.

depends/Makefile

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,6 @@ NO_WALLET ?=
4242
NO_ZMQ ?=
4343
NO_UPNP ?=
4444
NO_USDT ?=
45-
NO_NATPMP ?=
4645
MULTIPROCESS ?=
4746
LTO ?=
4847
NO_HARDEN ?=
@@ -159,13 +158,12 @@ sqlite_packages_$(NO_SQLITE) = $(sqlite_packages)
159158
wallet_packages_$(NO_WALLET) = $(bdb_packages_) $(sqlite_packages_)
160159

161160
upnp_packages_$(NO_UPNP) = $(upnp_packages)
162-
natpmp_packages_$(NO_NATPMP) = $(natpmp_packages)
163161

164162
zmq_packages_$(NO_ZMQ) = $(zmq_packages)
165163
multiprocess_packages_$(MULTIPROCESS) = $(multiprocess_packages)
166164
usdt_packages_$(NO_USDT) = $(usdt_$(host_os)_packages)
167165

168-
packages += $($(host_arch)_$(host_os)_packages) $($(host_os)_packages) $(boost_packages_) $(libevent_packages_) $(qt_packages_) $(wallet_packages_) $(upnp_packages_) $(natpmp_packages_) $(usdt_packages_)
166+
packages += $($(host_arch)_$(host_os)_packages) $($(host_os)_packages) $(boost_packages_) $(libevent_packages_) $(qt_packages_) $(wallet_packages_) $(upnp_packages_) $(usdt_packages_)
169167
native_packages += $($(host_arch)_$(host_os)_native_packages) $($(host_os)_native_packages)
170168

171169
ifneq ($(zmq_packages_),)
@@ -233,7 +231,6 @@ $(host_prefix)/toolchain.cmake : toolchain.cmake.in $(host_prefix)/.stamp_$(fina
233231
-e 's|@bdb_packages@|$(bdb_packages_)|' \
234232
-e 's|@sqlite_packages@|$(sqlite_packages_)|' \
235233
-e 's|@upnp_packages@|$(upnp_packages_)|' \
236-
-e 's|@natpmp_packages@|$(natpmp_packages_)|' \
237234
-e 's|@usdt_packages@|$(usdt_packages_)|' \
238235
-e 's|@no_harden@|$(NO_HARDEN)|' \
239236
-e 's|@multiprocess@|$(MULTIPROCESS)|' \

0 commit comments

Comments
 (0)