When ENABLE_IPC option is on, build with libmultiprocess subtree and
`add_subdirectory(src/ipc/libmultiprocess)` instead of external package
and `find_package(Libmultiprocess)` by default.
Behavior can be toggled with `WITH_EXTERNAL_LIBMULTIPROCESS` option. Using a
subtree should be more convenient for most bitcoin developers, but using an
external package is more convenient for developing in the libmultiprocess
repository.
The `WITH_EXTERNAL_LIBMULTIPROCESS` option is also used to avoid needing to
changing the depends build here. But in later commits, the depends build is
switched to use the add_subdirectory build as well.
Co-authored-by: Cory Fields <cory-nospam-@coryfields.com>
Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
Rename WITH_MULTIPROCESS to ENABLE_IPC, because ENABLE_IPC is a more accurate
name for the feature. It controls whether the src/ipc/ directory is built and
whether IPC features like -ipcbind, -ipcconnect, and -ipcfd are available. It
does NOT currently enable multiprocess features which are implemented in #10102
building on top of the IPC features. It will also no longer (as of the next
commit), control whether a find_package call is made so the "WITH_" prefix is
also inappropriate.
-BEGIN VERIFY SCRIPT-
git grep -l WITH_MULTIPROCESS | xargs sed -i s/WITH_MULTIPROCESS/ENABLE_IPC/g
-END VERIFY SCRIPT-
This adds an initial version of the txgraph module, with the TxGraph class.
It encapsulates knowledge about the fees, sizes, and dependencies between all
mempool transactions, but nothing else.
In particular, it lacks knowledge about txids, inputs, outputs, CTransactions,
... and so forth. Instead, it exposes a generic TxGraph::Ref type to reference
nodes in the TxGraph, which can be passed around and stored by layers on top.
This change:
1. Collects build artifacts in dedicated locations.
2. Allows running bitcoin-chainstate.exe with bitcoinkernel.dll directly
from the build tree on Windows.
c4c5cf174883cb53256e869f0d1673e29576a97c cmake: Fix passing `APPEND_*FLAGS` to `secp256k1` subtree (Hennadii Stepanov)
eb540a262953baf9ed920f98efc4044a353b278a cmake: Remove `core_sanitizer_{cxx,linker}_flags` helper variables (Hennadii Stepanov)
Pull request description:
On the master branch @ 70e20ea024ce4f39abc4022e1ba19d5a6db2a207, the `APPEND_CPPFLAGS`, `APPEND_CFLAGS` and `APPEND_LDFLAGS` are not correctly applied when building C code in the `secp256k1` subtree, as intended.
This behaviour occurs due to two issues:
1. The command here: 70e20ea024/src/CMakeLists.txt (L77)
does not affect the code in `add_subdirectory(secp256k1)` above it.
2. `APPEND_LDFLAGS` is not passed to the subtree's build system at all.
This PR fixes both issues.
Additionally, the helper variables `core_sanitizer_cxx_flags` and `core_sanitizer_linker_flags` have been removed.
ACKs for top commit:
theuni:
utACK c4c5cf174883cb53256e869f0d1673e29576a97c.
TheCharlatan:
ACK c4c5cf174883cb53256e869f0d1673e29576a97c
Tree-SHA512: 707acfa623f0436e34e9e6ba8ce2979e0fde5e196e2242fd1cde4c50f433938549781193d8a06419a0866bbe6d69d76f8383d973afbd87d944407963b318c5c9
`OBJECT` libraries have historically exhibited poor support for various
features, both in the past and now. For example, see one of the latest
issues:
- https://gitlab.kitware.com/cmake/cmake/-/issues/24058
Furthermore, CMake maintainers have acknowledged:
> In general, however, where there is a choice, static libraries will
> typically be the more convenient choice in CMake projects.
This change:
1. Converts the `bitcoin_clientversion` library from an `OBJECT` library
to a `STATIC` library.
2. Removes an obsolete workaround.
Also known as Ephemeral Dust.
We try to ensure that dust is spent in blocks by requiring:
- ephemeral dust tx is 0-fee
- ephemeral dust tx only has one dust output
- If the ephemeral dust transaction has a child,
the dust is spent by by that child.
0-fee requirement means there is no incentive to mine
a transaction which doesn't have a child bringing its
own fees for the transaction package.
0f4bc635854597e15ea6968767fc4e5cf5bdd790 [fuzz] txdownloadman and txdownload_impl (glozow)
699643f23a1bd0346e36bd90c83ba1b0b0a5c3fe [unit test] MempoolRejectedTx (glozow)
fa584cbe727b62853a410623b3d7c738e11cbffd [p2p] add TxDownloadOptions bool to make TxRequestTracker deterministic (glozow)
f803c8ce8dd88d9d0fd7857f63d76045b1e2bcaa [p2p] filter 1p1c for child txid in recent rejects (glozow)
5269d57e6d78e90baa0b40629f60a2d1d63e2992 [p2p] don't process orphan if in recent rejects (glozow)
2266eba43a973345351f2b0a8296523fb7de5576 [p2p] don't find 1p1cs for reconsiderable txns that are AlreadyHaveTx (glozow)
fa7027d0fc1fb2eb4148ba9741e1736f61d7e164 [refactor] add CheckIsEmpty and GetOrphanTransactions, remove access to TxDownloadMan internals (glozow)
969b07237b990b7eb6f3d24914ccc872202d8a0f [refactor] wrap {Have,Get}TxToReconsider in txdownload (glozow)
f150fb94e7dbb3c1f4fca32a0abf063943ca676d [refactor] make AlreadyHaveTx and Find1P1CPackage private to TxDownloadImpl (glozow)
1e08195135bc54f7a8b28560ae10943b1fef0d83 [refactor] move new tx logic to txdownload (glozow)
257568eab5baba07571fe2c68759e843d215d4a9 [refactor] move invalid package processing to TxDownload (glozow)
c4ce0c1218d0a3a2e9b22701f26391b8a9107196 [refactor] move invalid tx processing to TxDownload (glozow)
c6b21749ca0aea70908773d865e67511ca141ae6 [refactor] move valid tx processing to TxDownload (glozow)
a8cf3b6e845741e4b992beced564397779bfb7da [refactor] move Find1P1CPackage to txdownload (glozow)
f497414ce76a4cf44fa669e3665746cc17710fc6 [refactor] put peerman tasks at the end of ProcessInvalidTx (glozow)
6797bc42a762f431a986852fa74b1775aea8ba38 [p2p] restrict RecursiveDynamicUsage of orphans added to vExtraTxnForCompact (glozow)
798cc8f5aac9bf2111ea88d4a4c3817d34e089e2 [refactor] move Find1P1CPackage into ProcessInvalidTx (glozow)
416fbc952b209817a37e76c09fff5d17be7a72d0 [refactor] move new orphan handling to ProcessInvalidTx (glozow)
c8e67b9169bddc0bdfefa10e9cf7f9c22847e237 [refactor] move ProcessInvalidTx and ProcessValidTx definitions down (glozow)
3a41926d1b59dc9bbabc38cdc461c169426d94e7 [refactor] move notfound processing to txdownload (glozow)
042a97ce7fc672021cdb1dee62a550ef19c208fb [refactor] move tx inv/getdata handling to txdownload (glozow)
58e09f244b4bf07d31bc8dd4e939c2dc4dc74f3a [p2p] don't log tx invs when in IBD (glozow)
288865338f50d5b00758236aa4a59546a41c88c1 [refactor] rename maybe_add_extra_compact_tx to first_time_failure (glozow)
f48d36cd97e9b27dfa105c35e0fe67cba47056d1 [refactor] move peer (dis)connection logic to TxDownload (glozow)
f61d9e4b4b80842d520c490a1012044c0816679a [refactor] move AlreadyHaveTx to TxDownload (glozow)
84e4ef843db3443278d6eb70ff89fa254fcc6631 [txdownload] add read-only reference to mempool (glozow)
af918349de52e654927d50279de64f548a8b53d6 [refactor] move ValidationInterface functions to TxDownloadManager (glozow)
f6c860efb1221e1eadc3acebd6b0b885b9cc291a [doc] fix typo in m_lazy_recent_confirmed_transactions doc (glozow)
5f9004e1550f726ca9dc9a08c865fa8f2e4b92e8 [refactor] add TxDownloadManager wrapping TxOrphanage, TxRequestTracker, and bloom filters (glozow)
Pull request description:
Part of #27463.
This PR does 3 things:
(1) It modularizes transaction download logic into a `TxDownloadManager`. Transaction download logic refers to the process of deciding what transactions to request, download, and validate.[1] There should be no behavior changes. Using `--color_moved=dimmed_zebra -w` may help.
(2) It adds unit and fuzz (🪄) testing for transaction download.
(3) It makes a few small behavioral changes:
- Stop (debug-only) logging tx invs during IBD
- Just like all other transactions, require orphans have RecursiveDynamicUsage < 100k before adding to vExtraTxnForCompact
- Don't return a 1p1c that contains a parent or child in recent rejects. Don't process any orphan already in recent rejects. These cases should not happen in actual node operation; it's just to allow tighter sanity checks during fuzzing.
There are several benefits to this interface, such as:
- Unit test coverage and fuzzing for logic that currently isn't feasible to test as thoroughly (without lots of overhead) and/or currently only lightly tested through `assert_debug_log` (not good) in functional tests.
- When we add more functionality (e.g. package relay messages, more robust orphan handling), the vast majority of it will be within `TxDownloadManager` instead of `PeerManager`, making it easier to review and test. See #28031 for what this looks like.
- `PeerManager` will no longer know anything about / have access to `TxOrphanage`, `TxRequestTracker` or the rejection caches. Its primary interface with `TxDownloadManager` would be much simpler:
- Passing on `ValidationInterface` callbacks
- Telling `txdownloadman` when a peer {connects, disconnects}
- Telling `txdownloadman`when a {transaction, package} is {accepted, rejected} from mempool
- Telling `txdownloadman` when invs, notfounds, and txs are received.
- Getting instructions on what to download.
- Getting instructions on what {transactions, packages, orphans} to validate.
- Get whether a peer `HaveMoreWork` for the `ProessMessages` loop
- (todo) Thread-safety can be handled internally.
[1]: This module is concerned with tx *download*, not upload. It excludes transaction announcements/gossip which happens after we download/accept a transaction. Txreconciliation (erlay) is excluded from this module, as it only relates to deciding which `inv`s to send or helping the other peer decide which `inv`s to send. It is independent from this logic.
ACKs for top commit:
achow101:
light ACK 0f4bc635854597e15ea6968767fc4e5cf5bdd790
theStack:
ACK 0f4bc635854597e15ea6968767fc4e5cf5bdd790
instagibbs:
reACK 0f4bc635854597e15ea6968767fc4e5cf5bdd790
naumenkogs:
ACK 0f4bc635854597e15ea6968767fc4e5cf5bdd790
Tree-SHA512: 84ab8ef8a0fc705eb829d7f7d6885f28944aaa42b03172f256a42605677b3e783919bb900d4e3b8589f85a0c387dfbd972bcd61d252d44a88c6aaa90e4bf920f
This module is going to be responsible for managing everything related
to transaction download, including txrequest, orphan transactions and
package relay. It will be responsible for managing usage of the
TxOrphanage and instructing PeerManager:
- what tx or package-related messages to send to which peer
- whether a tx or package-related message is allowed or useful
- what transactions are available to try accepting to mempool
Future commits will consolidate the interface and re-delegate
interactions from PeerManager to TxDownloadManager.
5c7cacf649a6b474b876a7d219c7dc683a25e33d ci: Remove natpmp build option and libnatpmp dependency (laanwj)
7e7ec984da50f45491b994aaab180e7735ad1d8f doc: Remove mention of natpmp build options (laanwj)
061c3e32a26c6c04bf734d62627403758d7e51d9 depends: Drop natpmp and associated option from depends (laanwj)
20a18bf6aa38e87f72e2645482d00d0c77a344f5 build: Drop libnatpmp from build system (laanwj)
7b04709862f48e9020c7bef79cb31dd794cf91d0 qt: Changes for built-in PCP+NAT-PMP (laanwj)
52f8ef66c61b82457a161f3b90cc87f57d1dda80 net: Replace libnatpmp with built-in NATPMP+PCP implementation in mapport (laanwj)
97c97177cdb2f596aa7d4a65c4bde87de50a96f2 net: Add PCP and NATPMP implementation (laanwj)
d72df63d16941576b3523cfeaa49985cf3cd4d42 net: Use GetLocalAddresses in Discover (laanwj)
e02030432b77abbf27bb4f67d879d3ad6d6366e6 net: Add netif utility (laanwj)
754e4254388ec8ac1be6cf807bf300cd43fd3da5 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
- https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1658764974 avoid unreachable nets (not given to -onlynet=)
- https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1658949236 could announce an addr:port where we do not listen (no -bind)
- https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1684368824 could announce the wrong port because it uses GetListenPort()
- https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1679709347 if we requested one port but another was assigned, then which one to use in the renewal?
- https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1772017020 Use `GetAdapterAddresses` to discover local addresses for Windows
ACKs for top commit:
Sjors:
ACK 5c7cacf649a6b474b876a7d219c7dc683a25e33d
achow101:
ACK 5c7cacf649a6b474b876a7d219c7dc683a25e33d
vasild:
ACK 5c7cacf649a6b474b876a7d219c7dc683a25e33d
Tree-SHA512: e35b69e56d5f5449a3d48a799f9b7b65107c65eeb3e245c2c1e9d42221e469ca5ead90afae423160601cd664dd553a51c859e04f4492f335b064aae3bf23e3bc
bitcoin-build-info.h should always be generated before clientversion.cpp
is compiled due to the following explicit dependency in src/CMakeLists.txt:
add_dependencies(bitcoin_clientversion generate_build_info)
Hence there is no need to gate the inclusion of that header with an
extra define.
Now that this file is not in a subfolder anymore, prefix it with
"bitcoin-" to avoid potential collisions. Also add "info" for a more
descriptive name.
