Skip to content

Drop miniupnp dependency #31130

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 10 commits into from
Oct 28, 2024
Merged

Drop miniupnp dependency #31130

merged 10 commits into from
Oct 28, 2024

Conversation

darosior
Copy link
Member

@darosior darosior commented Oct 22, 2024 •
edited by fanquake
Loading

This PR removes UPnP IGD support and drops our miniupnp dependency.

Miniupnpc is a C library (somewhat) maintained by a single person which had several vulnerabilities in the past (a couple dozens are listed here), some of which directly affected our software (RCE in 2015, OOM in 2020).

The main purpose of this functionality is to have more (non-data-center) reachable nodes on the network. For a non-technical user running Bitcoin Core at home, the software would automatically open a port on their router to receive incoming connections. This way, users not able to manually open a port on their router would still provide the network with more resources and enhance its diversity.

However, due to past vulnerabilities (and a worry about unknown future ones) in miniupnpc this feature was disabled by default in #6795. Having it disabled by default kills (most of?) the purpose of having this functionality in the first place: someone technical enough to understand the -upnp startup option or the "enable UPnP" setting is most likely able to open a port on his box in the first place.

In addition, laanwj implemented PCP with a NAT-PMP fallback directly in Bitcoin Core in #30043. If we ever want to re-enable automatic NAT traversal by default in Bitcoin Core, this is the best option (and in my opinion the only sane one). The NAT-PMP fallback makes it so compatibility shouldn't be (much of) an issue.

On balance, i believe that keeping this functionality and this barely maintained C dependency has higher costs than benefits. Therefore i propose that we get rid of it.

@DrahtBot
Copy link
Contributor

DrahtBot commented Oct 22, 2024 •
edited
Loading

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.

Type Reviewers
ACK 1440000bytes, laanwj, i-am-yuvi, jarolrod
Concept ACK TheCharlatan, theStack, stickies-v, kristapsk, kevkevinpal, fjahr, Christewart, pythcoiner, theuni, sipa
Stale ACK maflcko, edilmedeiros

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #31158 (build, ci: Fix linking bitcoin-chainstate.exe to bitcoinkernel.dll on Windows by hebasto)
  • #31073 (ci: Split out native fuzz jobs for macOS and windows by dergoegge)
  • #31042 (build: Rename PACKAGE_* variables to CLIENT_* by hebasto)
  • #30997 (build: Switch to Qt 6 by hebasto)
  • #30634 (ci: Use clang-19 from apt.llvm.org by maflcko)
  • #29641 (scripted-diff: Use LogInfo over LogPrintf [WIP, NOMERGE, DRAFT] by maflcko)
  • #28710 (Remove the legacy wallet and BDB dependency by achow101)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

@TheCharlatan
Copy link
Contributor

Concept ACK

@hebasto
Copy link
Member

hebasto commented Oct 22, 2024 •
edited
Loading

To fix CI failure:

--- a/src/mapport.cpp
+++ b/src/mapport.cpp
@@ -2,8 +2,6 @@
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
-#include <bitcoin-build-config.h> // IWYU pragma: keep
-
 #include <mapport.h>
 
 #include <clientversion.h>

@laanwj
Copy link
Member

laanwj commented Oct 22, 2024 •
edited
Loading

Big concept ACK on this. As mentioned, MiniUPnP is some pretty dangerous code that resulted in the only known exploitable RCE in bitcoin core in history. It does brittle C-based string manipulation to generate and parse XML and HTTP which still has a high risk of having further bugs.

For that reason it's been disabled by default for years, there is no path toward enabling it, and meanwhile i doubt many people have been going through the hassle of enabling UPnP in the settings to open a port.

#30043 and follow-ups provide a reasonable replacement, which might be enabled by default at some point, when we are confident enough about it. #31022 adds testing of its network interaction and may help there.

Besides that, in general it's good to no longer have miniupnp be part of the GUIX build, as it's a sparsely maintained library which could make it target for xz-like backdooring.

@DrahtBot
Copy link
Contributor

🚧 At least one of the CI tasks failed.
Debug: https://github.com/bitcoin/bitcoin/runs/31889998343

Hints

Try to run the tests locally, according to the documentation. However, a CI failure may still
happen due to a number of reasons, for example:

  • Possibly due to a silent merge conflict (the changes in this pull request being
    incompatible with the current code in the target branch). If so, make sure to rebase on the latest
    commit of the target branch.

  • A sanitizer issue, which can only be found by compiling with the sanitizer and running the
    affected test.

  • An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

@theStack
Copy link
Contributor

Concept ACK

This was referenced Oct 23, 2024
Closed
Merged
Merged
Merged
Draft
Merged
stickies-v
stickies-v reviewed Oct 23, 2024
View reviewed changes
Copy link
Contributor

@stickies-v stickies-v left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Concept ACK

@kristapsk
Copy link
Contributor

Concept ACK

@kevkevinpal
Copy link
Contributor

Concept ACK

There have been instances where there have been breakage when using newer minor versions of miniupnp aswell to the other mentioned reasons
#30266 -> #30283 -> #30301

@kevkevinpal
Copy link
Contributor

Might be worth adding a release note?

@fjahr
Copy link
Contributor

fjahr commented Oct 23, 2024

Concept ACK but shouldn't this go through a deprecation cycle?

laanwj
laanwj reviewed Oct 23, 2024
View reviewed changes
@laanwj
Copy link
Member

laanwj commented Oct 23, 2024

Concept ACK but shouldn't this go through a deprecation cycle?

imo a real deprecation cycle is not needed here. This has teetered on the brink of being removed for so long already. Also, deprecation cycles are generally for APIs, and this doesn't add or remove any RPC or REST functionality.

@Christewart
Copy link
Contributor

concept ACK

@edilmedeiros
Copy link
Contributor

Concept ACK

@edilmedeiros edilmedeiros mentioned this pull request Oct 23, 2024
Closed
darosior added a commit to darosior/bitcoin that referenced this pull request Oct 23, 2024
@darosior
doc: add release note for bitcoin#31130
625ec41
laanwj
laanwj approved these changes Oct 27, 2024
View reviewed changes
Copy link
Member

@laanwj laanwj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review ACK 40e5f26
i have also checked that:

  • no mentions of miniupnp as a dependency remain
  • -natpmp still works to forward ports on my router after this
  • -upnp, if still present in the configuration file or command line, gives a custom message and exits

yuvicc
yuvicc approved these changes Oct 27, 2024
View reviewed changes
Copy link
Contributor

@yuvicc yuvicc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested ACK 40e5f26

  • -upnp gives custom messages and exits
  • -natpmp works

@jarolrod
Copy link
Member

ACK 40e5f26

Tested GUI changes, and GUI changes are sufficient. Error message appears in GUI as well when moving from master with UPNP having been set in the gui, then to this PR branch with same settings.json file.

fanquake added a commit to fanquake/oss-fuzz that referenced this pull request Oct 28, 2024
@fanquake
bitcoin: drop no-longer existent depends options
a10acc7 
bitcoin/bitcoin#31130
bitcoin/bitcoin#30043
@fanquake fanquake mentioned this pull request Oct 28, 2024
Merged
@fanquake
Copy link
Member

Guix Build:

541e5a857759e6e34aa8020e70b2b89be7cadfb358aecb0d83c10df214c5b1f7  guix-build-40e5f26a3ff7/output/aarch64-linux-gnu/SHA256SUMS.part
3973c1c93d969f4b1a338c734df151db3bdfdca0fcb5800f126890a426d5b35d  guix-build-40e5f26a3ff7/output/aarch64-linux-gnu/bitcoin-40e5f26a3ff7-aarch64-linux-gnu-debug.tar.gz
4cdeb68207b4f71819618b53f1c463f368ccca2912617945caceb49049f30a80  guix-build-40e5f26a3ff7/output/aarch64-linux-gnu/bitcoin-40e5f26a3ff7-aarch64-linux-gnu.tar.gz
5da6d6e1ab2ac471dfcaf9276f54d3a81ed951700d78354d9b3431f7954e70f4  guix-build-40e5f26a3ff7/output/arm-linux-gnueabihf/SHA256SUMS.part
4d8126f08b2474408dbc72c44c557397edaa318b6abc43bbb27fbc0ff9081420  guix-build-40e5f26a3ff7/output/arm-linux-gnueabihf/bitcoin-40e5f26a3ff7-arm-linux-gnueabihf-debug.tar.gz
dc04c665dd446426532335a9049e93f739031db7568e6f17a7cb9f45c99d9687  guix-build-40e5f26a3ff7/output/arm-linux-gnueabihf/bitcoin-40e5f26a3ff7-arm-linux-gnueabihf.tar.gz
4ea112d12ac5c0c527fcef92bf02cba4f56ff75296131eb20b4b28c134460cd4  guix-build-40e5f26a3ff7/output/arm64-apple-darwin/SHA256SUMS.part
39ebe16080b018d1ddb20c486f66a5e6180da02670a8fcc91dcf2464bbe29e9d  guix-build-40e5f26a3ff7/output/arm64-apple-darwin/bitcoin-40e5f26a3ff7-arm64-apple-darwin-unsigned.tar.gz
52ed74eb097078de8a8b68233d113ad826f9e937d71081733e4ecd5dcbdf1024  guix-build-40e5f26a3ff7/output/arm64-apple-darwin/bitcoin-40e5f26a3ff7-arm64-apple-darwin-unsigned.zip
b5a67473c600ccca0f5416939b9e1eebc95caa624522fede3319a31f0276bba7  guix-build-40e5f26a3ff7/output/arm64-apple-darwin/bitcoin-40e5f26a3ff7-arm64-apple-darwin.tar.gz
9049e160d1cbb7ba3f616ee4ef4078ebbb7580af1feb03f1eedddaf17e3f574c  guix-build-40e5f26a3ff7/output/dist-archive/bitcoin-40e5f26a3ff7.tar.gz
d1bd12e1450bcd1e513a9b5bf6721d9a91da54db116651eae135f36cfd3bc3c5  guix-build-40e5f26a3ff7/output/powerpc64-linux-gnu/SHA256SUMS.part
38dacbe2edd48f9b3e54835cb4c15ebc0288abc80ae2e4d2f0376df2c9c64c33  guix-build-40e5f26a3ff7/output/powerpc64-linux-gnu/bitcoin-40e5f26a3ff7-powerpc64-linux-gnu-debug.tar.gz
4170b6b3ac4ee6cd2aa26c7767831469fd83dc2d1fe18145ec425cbcf917a38f  guix-build-40e5f26a3ff7/output/powerpc64-linux-gnu/bitcoin-40e5f26a3ff7-powerpc64-linux-gnu.tar.gz
6aeebc67d7052641176bfb7cd05a3c69b50780e97aebd2793db8bdd0c0a524d3  guix-build-40e5f26a3ff7/output/riscv64-linux-gnu/SHA256SUMS.part
393e5c2429d78c3fc9183a6cb8b9c56c98b01d45a8a85bd14d2ff1f73088626d  guix-build-40e5f26a3ff7/output/riscv64-linux-gnu/bitcoin-40e5f26a3ff7-riscv64-linux-gnu-debug.tar.gz
0103ad4699282b908cbba54afd82828ca9b30a8e13f447a4a6c2c50275835203  guix-build-40e5f26a3ff7/output/riscv64-linux-gnu/bitcoin-40e5f26a3ff7-riscv64-linux-gnu.tar.gz
c4c66663a8280577e0f116eaa42522d93a8535144018d9eb40f7a8b96bff636c  guix-build-40e5f26a3ff7/output/x86_64-apple-darwin/SHA256SUMS.part
75e0e43cdb0842c57f4b58ba100f8f5270137c236ac9c83c0586a8041df90729  guix-build-40e5f26a3ff7/output/x86_64-apple-darwin/bitcoin-40e5f26a3ff7-x86_64-apple-darwin-unsigned.tar.gz
8f23f92e1e542c5c5c933ef6e8fad89fe0a667946d8630dbbc9090425aa4b605  guix-build-40e5f26a3ff7/output/x86_64-apple-darwin/bitcoin-40e5f26a3ff7-x86_64-apple-darwin-unsigned.zip
f8b109eb63bd1cb37a18f664a123c3dce206394f3649510330efab33e0e2bdfd  guix-build-40e5f26a3ff7/output/x86_64-apple-darwin/bitcoin-40e5f26a3ff7-x86_64-apple-darwin.tar.gz
9b38c99dedbc0f7841587532cb7ccaf899006f9fcfe0f7bd35999aa6775bc41d  guix-build-40e5f26a3ff7/output/x86_64-linux-gnu/SHA256SUMS.part
65efd9a3fd967069db09c2e705449b8223809ac1d4e1f62f312eaeb059122ee6  guix-build-40e5f26a3ff7/output/x86_64-linux-gnu/bitcoin-40e5f26a3ff7-x86_64-linux-gnu-debug.tar.gz
4a9439f81cb98dc3943d8bc3c14dd3fa96dafe20401b8c8be97ee97ee0bd996c  guix-build-40e5f26a3ff7/output/x86_64-linux-gnu/bitcoin-40e5f26a3ff7-x86_64-linux-gnu.tar.gz
52a47e5908b85a1324e5c2d90dd55c188cb0712495a32801bd201adab6b4490f  guix-build-40e5f26a3ff7/output/x86_64-w64-mingw32/SHA256SUMS.part
186521473489789e6e2cd3bb4f9c3e901168282305b99c9e66741caffa8db4d8  guix-build-40e5f26a3ff7/output/x86_64-w64-mingw32/bitcoin-40e5f26a3ff7-win64-debug.zip
ceb9204a1c643b1aee480ed5fb24da436713753fa1ff966124174908743db342  guix-build-40e5f26a3ff7/output/x86_64-w64-mingw32/bitcoin-40e5f26a3ff7-win64-setup-unsigned.exe
f816302f2023d42df33fd3896f380c4734d2333ff7e8bb06004926aabeb25128  guix-build-40e5f26a3ff7/output/x86_64-w64-mingw32/bitcoin-40e5f26a3ff7-win64-unsigned.tar.gz
440d575cc18508ad40e9959ca5e479216232a76ecd46dc96d3a7528dee145475  guix-build-40e5f26a3ff7/output/x86_64-w64-mingw32/bitcoin-40e5f26a3ff7-win64.zip

@fanquake fanquake merged commit 6e21ded into bitcoin:master Oct 28, 2024
16 checks passed
fanquake added a commit to fanquake/oss-fuzz that referenced this pull request Oct 28, 2024
@fanquake
bitcoin: drop no-longer existent depends options
d7b6415 
bitcoin/bitcoin#31130
bitcoin/bitcoin#30043
@darosior darosior deleted the 2410_drop_upnp branch October 28, 2024 14:53
@luisschwab luisschwab mentioned this pull request Oct 28, 2024
Closed
@Sjors
Copy link
Member

Sjors commented Oct 30, 2024

The GUI needs to handle this more gracefully:

Scherm­afbeelding 2024-10-30 om 12 13 44

A user would have to figure out that they need to manually edit settings.json or delete it and redo all their settings.

We should probably automatically delete it from settings.json. And then display a warning / info (instead of error) that tells users to consider the PCP setting in the GUI.

@jarolrod
Copy link
Member

A user would have to figure out that they need to manually edit settings.json or delete it and redo all their settings.

We should probably automatically delete it from settings.json. And then display a warning / info (instead of error) that tells users to consider the PCP setting in the GUI.

I posted about this on irc but no one commented on it :(

GUI should have option to remove the setting.

@laanwj
Copy link
Member

laanwj commented Oct 30, 2024

We should probably automatically delete it from settings.json. And then display a warning / info (instead of error) that tells users to consider the PCP setting in the GUI.

I posted about this on irc but no one commented on it :(

Yes. Please create a new issue for this

@Sjors Sjors mentioned this pull request Oct 30, 2024
Closed
@Sjors
Copy link
Member

Sjors commented Oct 30, 2024

Done in bitcoin-core/gui#843

fanquake added a commit to fanquake/oss-fuzz that referenced this pull request Nov 1, 2024
@fanquake
bitcoin: drop no-longer existent depends options
4534b12 
bitcoin/bitcoin#31130
bitcoin/bitcoin#30043
vitorguidi pushed a commit to google/oss-fuzz that referenced this pull request Nov 1, 2024
@fanquake
bitcoin: drop no-longer existent depends options (#12659)
e0b8762 
See bitcoin/bitcoin#31130.
See also bitcoin/bitcoin#30043.
@yuvicc yuvicc mentioned this pull request Nov 1, 2024
Closed
@Self-Hosting-Group Self-Hosting-Group mentioned this pull request Nov 11, 2024
Closed
TheCharlatan added a commit to TheCharlatan/rust-bitcoinkernel that referenced this pull request Nov 14, 2024
@TheCharlatan
Squashed 'libbitcoinkernel-sys/bitcoin/' changes from 1047757ea3b..48…
a73b2bd 
…158303fe2

48158303fe2 kernel: Add pure kernel bitcoin-chainstate
bf80d2f5009 kernel: Add block index utility functions to C header
a6ab5345e3b kernel: Add function to read block undo data from disk to C header
845b824d6c7 kernel: Add functions to read block from disk to C header
9324c8c4f67 kernel: Add function for copying  block data to C header
368fc93fd80 kernel: Add functions for the block validation state to C header
eb6e25ac007 kernel: Add validation interface to C header
cdce4484005 kernel: Add interrupt function to C header
7e47ec78768 kernel: Add import blocks function to C header
2b803d50747 kernel: Add chainstate load options for in-memory dbs in C header
ea92eb13c4a kernel: Add options for reindexing in C header
8254f2035a7 kernel: Add block validation to C header
ad7b880346e Kernel: Add chainstate loading to kernel C header
583820c4487 kernel: Add chainstate manager object to C header
ec137a086a0 kernel: Add notifications context option to C header
62a89689266 kerenl: Add chain params context option to C header
bb482dcbd30 kernel: Add kernel library context object
d114ccfdf8a kernel: Add logging to kernel library C header
44c65c46c43 kernel: Introduce initial kernel C header API
69c03134440 Merge bitcoin/bitcoin#31269: validation: Remove RECENT_CONSENSUS_CHANGE validation result
42282592943 Merge bitcoin/bitcoin#31000: bench: add support for custom data directory
36f5effa178 Merge bitcoin/bitcoin#31235: addrman: cap the `max_pct` to not exceed the maximum number of addresses
98ad249b69f Merge bitcoin/bitcoin#31277: doc: mention `descriptorprocesspsbt` in psbt.md
b0222bbb494 Merge bitcoin/bitcoin#30239: Ephemeral Dust
1dda1892b6b Merge bitcoin/bitcoin#31037: test: enhance p2p_orphan_handling
5c2e291060c bench: Add basic CheckEphemeralSpends benchmark
3f6559fa581 Add release note for ephemeral dust
71a6ab4b33d test: unit test for CheckEphemeralSpends
21d28b2f362 fuzz: add ephemeral_package_eval harness
127719f516a test: Add CheckMempoolEphemeralInvariants
e2e30e89ba4 functional test: Add ephemeral dust tests
4e68f901390 rpc: disallow in-mempool prioritisation of dusty tx
e1d3e81ab4d policy: Allow dust in transactions, spent in-mempool
04b2714fbbc functional test: Add new -dustrelayfee=0 test case
ebb6cd82baf doc: mention `descriptorprocesspsbt` in psbt.md
2b33322169b Merge bitcoin/bitcoin#31249: test: Add combinerawtransaction test to rpc_createmultisig
3fb6229dcfd Merge bitcoin/bitcoin#31271: doc: correct typos
fa66e0887ca bench: add support for custom data directory
ad9c2cceda9 test, bench: specialize working directory name
9c5775c331e addrman: cap the `max_pct` to not exceed the maximum number of addresses
8d340be9247 Merge bitcoin/bitcoin#31181: cmake: Revamp `FindLibevent` module
9a8e5adb161 Merge bitcoin/bitcoin#31267: refactor: Drop deprecated space in operator""_mst
726cbee9553 doc: correct typos
9fdfb73ca84 doc: fix typos
af6088701a2 Merge bitcoin/bitcoin#31237: doc: Add missing 'blank=true' option in offline-signing-tutorial.md
7a526653022 Merge bitcoin/bitcoin#31239: test: clarify log messages when handling SOCKS5 proxy connections
900b17239fb Merge bitcoin/bitcoin#31259: doc: Fix missing comma in JSON example in REST-interface.md
faf21625652 refactor: Drop deprecated space in operator""_mst
c889890e4a3 Merge bitcoin/bitcoin#31264: doc: Fixup bitcoin-wallet manpage chain selection args
0f6d20e43f2 Merge bitcoin/bitcoin#31163: scripted-diff: get rid of remaining "command" terminology in protocol.{h,cpp}
5acd5e7f874 Merge bitcoin/bitcoin#31257: ci: make ctest stop on failure
19f277711eb Merge bitcoin/bitcoin#26593: tracing: Only prepare tracepoint arguments when actually tracing
e80e4c6ff91 validation: Remove RECENT_CONSENSUS_CHANGE validation result
fa729ab4a27 doc: Fixup bitcoin-wallet manpage chain selection args
5e3b444022c doc: Fix missing comma in JSON example in REST-interface.md
0903ce8dbc2 Merge bitcoin/bitcoin#30592: Remove mempoolfullrbf
f842d0801e1 Merge bitcoin/bitcoin#29686: Update manpage descriptions
36a22e56833 ci: make ctest stop on failure
83fab3212c9 test: Add combinerawtransaction test to rpc_createmultisig
018e5fcc462 Merge bitcoin/bitcoin#31190: TxDownloadManager followups
3a5f6027e16 Merge bitcoin/bitcoin#31171: depends: Specify CMake generator explicitly
99d9a093cf6 test: clarify log messages when handling SOCKS5 proxy connections
c9e67e214f0 Merge bitcoin/bitcoin#31238: fuzz: Limit wallet_notifications iterations
564238aabf1 Merge bitcoin/bitcoin#31164: net: Use actual memory size in receive buffer accounting
fa461d7a43a fuzz: Limit wallet_notifications iterations
ec375de39ff doc: Add missing 'blank=true' option in offline-signing-tutorial.md
5a96767e3f5 depends, libevent: Do not install *.pc files and remove patches for them
ffda355b5a2 cmake, refactor: Move `HAVE_EVHTTP_...` to `libevent` interface
b619bdc3303 cmake: Revamp `FindLibevent` module
2c90f8e08c4 Merge bitcoin/bitcoin#31232: ci: `add second_deadlock_stack=1` to TSAN options
5dc94d13d41 fuzz fix: assert MAX_PEER_TX_ANNOUNCEMENTS is not exceeded
45e2f8f87d8 Merge bitcoin/bitcoin#31173: cmake: Add `FindQRencode` module and enable `libqrencode` package for MSVC
80cb630bd94 Merge bitcoin/bitcoin#31216: Update secp256k1 subtree to v0.6.0
5161c2618cd ci: add second_deadlock_stack=1 to TSAN options
85224f92d52 Merge bitcoin/bitcoin#30811: build: Unify `-logsourcelocations` format
9719d373dc2 Merge bitcoin/bitcoin#30634: ci: Use clang-19 from apt.llvm.org
97235c446e9 build: Disable secp256k1 musig module
9e5089dbb02 build, msvc: Enable `libqrencode` vcpkg package
30089b0cb61 cmake: Add `FindQRencode` module
65b19419366 Merge bitcoin/bitcoin#31186: msvc: Update vcpkg manifest
d3388720837 Merge bitcoin/bitcoin#31206: doc: Use relative hyperlinks in release-process.md
ffc05fca6f7 Merge bitcoin/bitcoin#31220: doc: Fix word order in developer-notes.md
9f2c8287a24 Merge bitcoin/bitcoin#31192: depends, doc: List packages required to build `qt` package separately
03cff2c1421 Merge bitcoin/bitcoin#31191: build: Make G_FUZZING constexpr, require -DBUILD_FOR_FUZZING=ON to fuzz
44939e5de1b doc: Fix word order in developer-notes.md
b934954ad10 Merge bitcoin/bitcoin#30670: doc: Extend developer-notes with file-name-only debugging fix
05aebe3790f Merge bitcoin/bitcoin#30930: netinfo: add peer services column and outbound-only option
0ba680d41b4 Update secp256k1 subtree to v0.6.0
2d46a89386d Squashed 'src/secp256k1/' changes from 2f2ccc46954..0cdc758a563
d22a234ed27 net: Use actual memory size in receive buffer accounting
047b5e2af1f streams: add DataStream::GetMemoryUsage
c3a6722f34a net: Use DynamicUsage(m_type) in CSerializedNetMsg::GetMemoryUsage
c6594c0b142 memusage: Add DynamicUsage for std::string
7596282a556 memusage: Allow counting usage of vectors with different allocators
6463117a292 Merge bitcoin/bitcoin#31208: doc: archive release notes for v27.2
788c1324f3d build: Unify `-logsourcelocations` format
4747f030956 depends, doc: List packages required to build `qt` package separately
1a05c86ae47 doc: archive release notes for v27.2
9f71cff6ab3 doc: Use relative hyperlinks in release-process.md
f1bcf3edc50 Merge bitcoin/bitcoin#31139: test: added test to assert TX decode rpc error on submitpackage rpc
975b115e1a2 Merge bitcoin/bitcoin#31198: init: warn, don't error, when '-upnp' is set
4a0251c05dd Merge bitcoin/bitcoin#31187: ci: Do not error on unused-member-function in test each commit
e001dc3dc6e Merge bitcoin/bitcoin#31203: fuzz: fix `implicit-integer-sign-change` in wallet_create_transaction
5a26cf7773e fuzz: fix `implicit-integer-sign-change` in wallet_create_transaction
a1b3ccae4be init: warn, don't error, when '-upnp' is set
c189eec848e doc: release note for mempoolrullrbf removal
d47297c6aab rpc: Mark fullrbf and bip125-replaceable as deprecated
04a5dcee8ab docs: remove requirement to signal bip125
fafbf8acf41 Make G_FUZZING constexpr, require -DBUILD_FOR_FUZZING=ON to execute a fuzz target
fae3cf0ffa6 ci: Temporarily disable macOS/Windows fuzz step
f6577b71741 build, msvc: Update vcpkg manifest baseline
16e16013bfa build, msvc: Document `libevent` version pinning
ec47cd2b508 build, msvc: Drop no longer needed `liblzma` version pinning
9a0734df5f1 build, msvc: Reorder keys in `vcpkg.json`
8351562bec6 [fuzz] allow negative time jumps in txdownloadman_impl
917ab810d93 [doc] comment fixups from n30110
f07a533dfcb Merge bitcoin/bitcoin#24214: Fix unsigned integer overflows in interpreter
62516105536 Merge bitcoin/bitcoin#31015: build: have "make test" depend on "make all"
4a31f8ccc9d Merge bitcoin/bitcoin#31156: test: Don't enforce BIP94 on regtest unless specified by arg
02be3dced71 Merge bitcoin/bitcoin#31166: key: clear out secret data in `DecodeExtKey`
54d07dd37d5 ci: Do not error on unused-member-function in test each commit
47f50c7af55 doc: add bitcoin-qt man description
40b82e3ab0a doc: add bitcoin-util man description
a7bf80f3a2d doc: add bitcoin-tx man description
3f9a5168323 doc: add bitcoin-wallet man description
d8c0bb23ef8 doc: add bitcoin-cli man description
09abccfa772 doc: add bitcoind man description
97b790e844a Merge bitcoin/bitcoin#29420: test: extend the SOCKS5 Python proxy to actually connect to a destination
6b73eb9a1a2 Merge bitcoin/bitcoin#31064: init: Correct coins db cache size setting
27d12cf17f2 Merge bitcoin/bitcoin#31043: rpc: getorphantxs follow-up
7b66815b16b Merge bitcoin/bitcoin#30110: refactor: TxDownloadManager + fuzzing
dc97e7f6dba Merge bitcoin/bitcoin#30903: cmake: Add `FindZeroMQ` module
1b0b9b4c787 Extend possible debugging fixes with file-name-only
da10e0bab4a Merge bitcoin/bitcoin#30942: test: Remove dead code from interface_zmq test
111a23d9b36 Remove -mempoolfullrbf option
e96ffa98b04 Merge bitcoin/bitcoin#31142: test: fix intermittent failure in p2p_seednode.py, don't connect to random IPs
54c4b09f083 Merge bitcoin/bitcoin#31042: build: Rename `PACKAGE_*` variables to `CLIENT_*`
e60cecc8115 doc: add release note for 31156
fc7dfb3df5b test: Don't enforce BIP94 on regtest unless specified by arg
fabe90c8242 ci: Use clang-19 from apt.llvm.org
0de3e96e333 tracing: use bitcoind pid in bcc tracing examples
411c6cfc6c2 tracing: only prepare tracepoint args if attached
d524c1ec066 tracing: dedup TRACE macros & rename to TRACEPOINT
70713303b63 scripted-diff: Rename `PACKAGE_*` variables to `CLIENT_*`
332655cb52c build: Rename `PACKAGE_*` variables to `CLIENT_*`
e6e29e3c94c scripted-diff: Clarify "user agent" variable name
e2ba8236715 depends: Specify CMake generator explicitly
1c7ca6e64de Merge bitcoin/bitcoin#31093: Introduce `g_fuzzing` global for fuzzing checks
6e21dedbf2b Merge bitcoin/bitcoin#31130: Drop miniupnp dependency
d7fd766feb2 test: added test to assert TX decode rpc error on submitpackage rpc
559a8dd9c0a key: clear out secret data in `DecodeExtKey`
4120c7543ee scripted-diff: get rid of remaining "command" terminology in protocol.{h,cpp}
2a52718d734 Merge bitcoin/bitcoin#31152: functional test: Additional package evaluation coverage
9de9c858d5a test: enhance p2p_orphan_handling
33af14b62e4 test: reduce assert_debug_log reliance
0ea84bc362f test: explicitly check boolean verbosity is disallowed
7a2e6b68cd9 doc: add rpc guidance for boolean verbosity avoidance
698f302df8b rpc: disallow boolean verbosity in getorphantxs
63f5e6ec795 test: add entry and expiration time checks
808a708107e rpc: add entry time to getorphantxs
56bf3027144 refactor: rename rpc_getorphantxs to rpc_orphans
7824f6b0770 test: check that getorphantxs is hidden
ac68fcca701 rpc: disallow undefined verbosity in getorphantxs
25dacae9c7f Merge bitcoin/bitcoin#31040: test: Assert that when we add the max orphan amount that we cannot add anymore and that a random orphan gets dropped
40e5f26a3ff mapport: remove dead code in DispatchMapPort
38fdf7c1fb1 mapport: drop outdated comments
915640e191b depends: zeromq: don't install .pc files and remove patches for them
6b8a74463b5 cmake: Add `FindZeroMQ` module
9a7206a34e3 Merge bitcoin/bitcoin#29536: fuzz: fuzz connman with non-empty addrman + ASMap
d4abaf8c9d9 Merge bitcoin/bitcoin#29608: optimization: Preallocate addresses in GetAddr based on nNodes
b7b24352906 doc: add release note for #31130
1b6dec98da3 depends: drop miniupnpc
953533d0214 doc: remove mentions of UPnP
94ad614482f ci: remove UPnP options
f32c34d0c3d functional test: Additional package evaluation coverage
87532fe5585 netinfo: allow setting an outbound-only peer list
9f243cd7fa6 Introduce `g_fuzzing` global for fuzzing checks
b95adf057a4 Merge bitcoin/bitcoin#31150: util: Treat Assume as Assert when evaluating at compile-time
8f24e492e20 Merge bitcoin/bitcoin#29991: depends: sqlite 3.46.1
2ef5004f78c Merge bitcoin/bitcoin#31146: ci: Temporary workaround for old CCACHE_DIR cirrus env
8c12fe828de Merge bitcoin/bitcoin#29936: fuzz: wallet: add target for `CreateTransaction`
5c299ecafe6 test: Assert that when we add the max orphan amount that we cannot add anymore and that a random orphan gets dropped
0f4bc635854 [fuzz] txdownloadman and txdownload_impl
699643f23a1 [unit test] MempoolRejectedTx
fa584cbe727 [p2p] add TxDownloadOptions bool to make TxRequestTracker deterministic
f803c8ce8dd [p2p] filter 1p1c for child txid in recent rejects
5269d57e6d7 [p2p] don't process orphan if in recent rejects
2266eba43a9 [p2p] don't find 1p1cs for reconsiderable txns that are AlreadyHaveTx
fa7027d0fc1 [refactor] add CheckIsEmpty and GetOrphanTransactions, remove access to TxDownloadMan internals
969b07237b9 [refactor] wrap {Have,Get}TxToReconsider in txdownload
f150fb94e7d [refactor] make AlreadyHaveTx and Find1P1CPackage private to TxDownloadImpl
1e08195135b [refactor] move new tx logic to txdownload
257568eab5b [refactor] move invalid package processing to TxDownload
c4ce0c1218d [refactor] move invalid tx processing to TxDownload
c6b21749ca0 [refactor] move valid tx processing to TxDownload
a8cf3b6e845 [refactor] move Find1P1CPackage to txdownload
f497414ce76 [refactor] put peerman tasks at the end of ProcessInvalidTx
6797bc42a76 [p2p] restrict RecursiveDynamicUsage of orphans added to vExtraTxnForCompact
798cc8f5aac [refactor] move Find1P1CPackage into ProcessInvalidTx
416fbc952b2 [refactor] move new orphan handling to ProcessInvalidTx
c8e67b9169b [refactor] move ProcessInvalidTx and ProcessValidTx definitions down
3a41926d1b5 [refactor] move notfound processing to txdownload
042a97ce7fc [refactor] move tx inv/getdata handling to txdownload
58e09f244b4 [p2p] don't log tx invs when in IBD
288865338f5 [refactor] rename maybe_add_extra_compact_tx to first_time_failure
f48d36cd97e [refactor] move peer (dis)connection logic to TxDownload
f61d9e4b4b8 [refactor] move AlreadyHaveTx to TxDownload
84e4ef843db [txdownload] add read-only reference to mempool
af918349de5 [refactor] move ValidationInterface functions to TxDownloadManager
f6c860efb12 [doc] fix typo in m_lazy_recent_confirmed_transactions doc
5f9004e1550 [refactor] add TxDownloadManager wrapping TxOrphanage, TxRequestTracker, and bloom filters
947f2925d55 Merge bitcoin/bitcoin#31124: util: Remove RandAddSeedPerfmon
7640cfdd624 Merge bitcoin/bitcoin#31118: doc: replace `-?` with `-h` and `-help`
74fb19317ae Merge bitcoin/bitcoin#30849: refactor: migrate `bool GetCoin` to return `optional<Coin>`
c16e909b3e2 Merge bitcoin/bitcoin#28574: wallet: optimize migration process, batch db transactions
a9598e5eaab build: drop miniupnpc dependency
a5fcfb7385c interfaces: remove now unused 'use_upnp' arg from 'mapPort'
038bbe7b200 daemon: remove UPnP support
844770b05eb qt: remove UPnP settings
dd92911732d Merge bitcoin/bitcoin#31148: ci: display logs of failed unit tests automatically
fa69a5f4b76 util: Treat Assume as Assert when evaluating at compile-time
0c79c343a9f Merge bitcoin/bitcoin#31147: cmake, qt, test: Remove problematic code
8523d8c0fc8 ci: display logs of failed tests automatically
2f40e453ccd Merge bitcoin/bitcoin#29450: build: replace custom `MAC_OSX` macro with existing `__APPLE__`
cb7c5ca824e Add gdb and lldb links to debugging troubleshooting
6c6b2442eda build: Replace MAC_OSX macro with existing __APPLE__
fb46d57d4e7 cmake, qt, test: Remove problematic code
fa9747a8961 ci: Temporary workaround for old CCACHE_DIR cirrus env
6c9fe7b73ea test: Prevent connection attempts to random IPs in p2p_seednodes.py
bb97b1ffa9f test: fix intermittent timeout in p2p_seednodes.py
57529ac4dbb test: set P2PConnection.p2p_connected_to_node in peer_connect_helper()
22cd0e888c7 test: support WTX INVs from P2PDataStore and fix a comment
ebe42c00aa4 test: extend the SOCKS5 Python proxy to actually connect to a destination
9bb92c0e7ff util: Remove RandAddSeedPerfmon
c98fc36d094 wallet: migration, consolidate external wallets db writes
7c9076a2d2e wallet: migration, consolidate main wallet db writes
9ef20e86d7f wallet: provide WalletBatch to 'SetupDescriptorScriptPubKeyMans'
34bf0795fc0 wallet: refactor ApplyMigrationData to return util::Result<void>
aacaaaa0d3a wallet: provide WalletBatch to 'RemoveTxs'
57249ff6697 wallet: introduce active db txn listeners
91e065ec175 wallet: remove post-migration signals connection
055c0532fc8 wallet: provide WalletBatch to 'DeleteRecords'
122d103ca22 wallet: introduce 'SetWalletFlagWithDB'
6052c7891dc wallet: decouple default descriptors creation from external signer setup
f2541d09e13 wallet: batch MigrateToDescriptor() db transactions
66c9936455f bench: add coverage for wallet migration process
33a28e252a7 Change default help arg to `-help` and mention `-h` and `-?` as alternatives
f0130ab1a1e doc: replace `-?` with `-h` for bench_bitcoin help
681ebcceca7 netinfo: rename and hoist max level constant to use in top-level help
e7d307ce8cf netinfo: clarify relaytxes and addr_relay_enabled help docs
eef2a9d4062 netinfo: add peer services column
3a4a788ee0d init: Correct coins db cache size setting
2957ca96119 build: have "make test" depend on "make all"
bbbbaa0d9ac Fix unsigned integer overflows in interpreter
c4dc81f9c69 test: Remove dead code from interface_zmq
c495731a316 fuzz: wallet: add target for `CreateTransaction`
3db68e29ec6 wallet: move `ImportDescriptors`/`FuzzedWallet` to util
552cae243a1 fuzz: cover `ASMapHealthCheck` in connman target
33b0f3ae966 fuzz: use `ConsumeNetGroupManager` in connman target
18c8a0945bd fuzz: move `ConsumeNetGroupManager` to util
fe624631aeb fuzz: fuzz `connman` with a non-empty addrman
0a12cff2a8e fuzz: move `AddrManDeterministic` to util
4feaa287284 refactor: Rely on returned value of GetCoin instead of parameter
46dfbf169b4 refactor: Return optional of Coin in GetCoin
e31bfb26c21 refactor: Remove unrealistic simulation state
ba621ffb9cb test: improve debug log message from P2PConnection::connection_made()
def6dd0c597 depends: sqlite 3.46.1
66082ca3488 Preallocate addresses in GetAddr based on nNodes
REVERT: 1047757ea3b kernel: Add pure kernel bitcoin-chainstate
REVERT: c568fdf75fd kernel: Add block index utility functions to C header
REVERT: 0f1da1dcba5 kernel: Add function to read block undo data from disk to C header
REVERT: 45af559c9f6 kernel: Add functions to read block from disk to C header
REVERT: 2a7f8a8240c kernel: Add function for copying  block data to C header
REVERT: b19f5336c03 kernel: Add functions for the block validation state to C header
REVERT: 9c0ffa913f4 kernel: Add validation interface to C header
REVERT: a93318c6152 kernel: Add interrupt function to C header
REVERT: 51053f33720 kernel: Add import blocks function to C header
REVERT: 6b0ada2af42 kernel: Add chainstate load options for in-memory dbs in C header
REVERT: 34427bfa9c7 kernel: Add options for reindexing in C header
REVERT: ca57311c969 kernel: Add block validation to C header
REVERT: 44156d84838 Kernel: Add chainstate loading to kernel C header
REVERT: 2cee46cdcc1 kernel: Add chainstate manager object to C header
REVERT: 7102c7ae45e kernel: Add notifications context option to C header
REVERT: ed628a2a3c4 kerenl: Add chain params context option to C header
REVERT: 27643297ff7 kernel: Add kernel library context object
REVERT: 2ba22cf3f90 kernel: Add logging to kernel library C header
REVERT: 873874c03e9 kernel: Introduce initial kernel C header API

git-subtree-dir: libbitcoinkernel-sys/bitcoin
git-subtree-split: 48158303fe276cb2f8fbc53ff31a4162d8f55c84
@ryanofsky ryanofsky mentioned this pull request Dec 4, 2024
Draft
@darosior darosior mentioned this pull request Jan 15, 2025
Closed
fanquake added a commit that referenced this pull request Feb 14, 2025
@fanquake
Merge #31157: Cleanups to port mapping module post UPnP drop
9491676 
70398ae mapport: make ProcessPCP void (Antoine Poinsot)
9e6cba2 mapport: remove unnecessary 'g_mapport_enabled' (Antoine Poinsot)
8fb45fc mapport: remove unnecessary 'g_mapport_current' variable (Antoine Poinsot)
1b223cb mapport: merge DispatchMapPort into StartMapPort (Antoine Poinsot)
9bd936f mapport: drop unnecessary function (Antoine Poinsot)
2a6536c mapport: rename 'use_pcp' to 'enable' (Antoine Poinsot)
c4e82b8 mapport: make 'enabled' and 'current' bool (Antoine Poinsot)

Pull request description:

  Followup to #31130, this does a couple cleanups to `src/mapport.*` to clarify the logic now that there is a single protocol option for port mapping.

ACKs for top commit:
  laanwj:
    Code review ACK 70398ae
  TheCharlatan:
    ACK 70398ae

Tree-SHA512: d9a3ab4fcd59a7cf4872415c40cc7ac3a98dfc5aa25e195d4df880bb588bac286c30c3471e9d9499de379a75f45dcd0a82019eba3cb9f342004ae1482d0ba075
ryanofsky
ryanofsky reviewed Feb 20, 2025
View reviewed changes
src/qt/optionsmodel.cpp
@@ -789,7 +776,6 @@ void OptionsModel::checkAndMigrate()
migrate_setting(SpendZeroConfChange, "bSpendZeroConfChange");
migrate_setting(ExternalSignerPath, "external_signer_path");
#endif
migrate_setting(MapPortUPnP, "fUseUPnP");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In commit "qt: remove UPnP settings" (844770b)

It probably would be better to replace this line with:

if (settings.contains("fUseUPnP")) {
    if (node().getPersistentSetting("upnp").isNull()) UpdateRwSetting(node(), "upnp", "", settings.value("fUseUPnP").toBool());
    settings.remove("fUseUPnP");
}

So the setting would still be migrated and a warning could be logged if it was enabled.

@glozow glozow mentioned this pull request Mar 3, 2025
Merged
4 tasks
rebroad pushed a commit to rebroad/bitcoin that referenced this pull request May 1, 2025
@darosior @rebroad
doc: add release note for bitcoin#31130
611f3d4
@nahuhh nahuhh mentioned this pull request Jun 30, 2025
Open
pinheadmz added a commit to pinheadmz/gui-qml that referenced this pull request Jul 11, 2025
@pinheadmz
remove UPnP
4de5fd6 
bitcoin/bitcoin#31130
@fanquake fanquake mentioned this pull request Jul 23, 2025
Closed
@nahuhh nahuhh mentioned this pull request Jul 24, 2025
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fanquake fanquake fanquake left review comments

@ryanofsky ryanofsky ryanofsky left review comments

@laanwj laanwj laanwj approved these changes

@fjahr fjahr Awaiting requested review from fjahr

@theStack theStack Awaiting requested review from theStack

@stickies-v stickies-v Awaiting requested review from stickies-v

@TheCharlatan TheCharlatan Awaiting requested review from TheCharlatan

@sipa sipa Awaiting requested review from sipa

@theuni theuni Awaiting requested review from theuni

@maflcko maflcko Awaiting requested review from maflcko

+2 more reviewers

@yuvicc yuvicc yuvicc approved these changes

@1440000bytes 1440000bytes 1440000bytes approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
Build system P2P
Projects
None yet
Milestone
29.0
Development

Successfully merging this pull request may close these issues.