6c9ee92ffe net: don't check if the listening socket is valid (Vasil Dimov)
Pull request description:
_This is a piece of #21878, chopped off to ease review._
Listening sockets in `CConnman::vhListenSocket` are always valid
(underlying file descriptor is not `INVALID_SOCKET`).
ACKs for top commit:
theStack:
Code-review ACK 6c9ee92ffe🔌
Tree-SHA512: b2e29711c6a0c7c85467ca61cfd7fb734eb06bd83a41f88735901caf90aec095ca80707ce5bb897d39c80fdec16819dbf5a84979c9b1ab3dc3fb8b08cebe7c61
faad05c6d2 Crash debug builds when mempool ConsensusScriptChecks fails (MarcoFalke)
Pull request description:
Currently a bug in the function might sneak around our testing infrastructure.
Fix that by turning bugs into crashes during tests.
ACKs for top commit:
glozow:
utACK faad05c6d2, there's something seriously wrong with the code if this returns false, good to throw in debug mode
Tree-SHA512: dfea1cd9ce3f1c303f49cca1417cd5c77c6ed12849aaff7b6ab1b6060f2f0c9cf5d4689017355d11f66639bab35823f65f848e6979042fa875181509dfd5d3d7
6aed8b7e9b [test] tx processing before and after ibd (glozow)
b9e105b664 [net_processing] ignore all transactions during ibd (glozow)
Pull request description:
This is basically a mini, IBD-only version of #21224
Incoming transactions aren't really relevant until we're caught up. That's why we send a giant feefilter and don't send tx getdatas, but we also shouldn't process them if peers send them anyway. Simply ignore them.
ACKs for top commit:
jnewbery:
reACK 6aed8b7e9b
laanwj:
Code review ACK 6aed8b7e9b
Tree-SHA512: 8e1616bf355f9d0b180bdbc5461f24c757dc5d7bc7bf651470f3b0bffcca5d5e68287106255b5cede2d96b42bce448a0f8c0649de35a530c5e079f7c89c70a35
- Actually generate an assumeutxo hash and display it
- Add nchaintx to output (necessary for use in chainparams entry)
- Add path of serialized UTXO file to output
No behavior change.
Parameterize removeForReorg using a CChain and callable that
encapsulates validation logic. The mempool shouldn't need to know a
bunch of details about coinbase maturity and lock finality. Instead,
just pass in a callable function that says true/false. Breaks circular
dependency by removing txmempool's dependency on validation.
Each entry's lockpoints are independent of one another, so there isn't
any reason to update lockpoints for entries that will be removed.
Separating the loops also allows us to move validation logic out and
leave lockpoints in txmempool.
fae239208d wallet: Split signmessage from rpcwallet (MarcoFalke)
Pull request description:
rpcwallet is the file that takes longest to compile, especially with sanitizers enabled it can take several 10s of seconds.
Allow faster incremental and parallel builds by starting to split it up. First, split off `signmessage`, which is unrelated to other stuff such as wallet file handling, wallet encryption, tx creation, or wallet status/info.
ACKs for top commit:
ryanofsky:
Code review ACK fae239208d. Confirmed move only
meshcollider:
Code review ACK fae239208d
Tree-SHA512: 250445cd544e39376f225871270cdcae462f16cfd9d25ede4b148e915642bfac9ee7ef3e8eccdd2443dc74dbf794d3bcd5fe5c58b1d05a2dcec70b8e03b37dff
a99ed89865 psbt: sign without finalizing (Andrew Chow)
Pull request description:
It can be useful to sign an input with `walletprocesspsbt` but not finalize that input if it is complete. This PR adds another option to `walletprocesspsbt` to be able to do that. We will still finalize by default.
This does not materially change the PSBT workflow since `finalizepsbt` needs to be called in order to extract the tx for broadcast.
ACKs for top commit:
meshcollider:
utACK a99ed89865
Sjors:
utACK a99ed89
Tree-SHA512: c88e5d3222109c5f4e763b1b9d97ce4655f68f2985a4509caab2d4e7f5bac5047328fd69696e82a330f5c5a333e0312568ae293515689b77a4747ca2f17caca6
As node operators are free to set their mempool policies however they
please, it's possible for package transaction(s) to already be in the
mempool. We definitely don't want to reject the entire package in that
case (as that could be a censorship vector).
We should still return the successful result to the caller, so add
another result type to MempoolAcceptResult.
2bc51c5c32 [tracing] tracepoints to utxocache add, spent and uncache (Arnab Sen)
a26e8eef43 [tracing] tracepoint for utxocache flushes (Arnab Sen)
Pull request description:
This PR adds some of the UTXO set cache tracepoints proposed in https://github.com/bitcoin/bitcoin/issues/20981#issuecomment-802688809. The first tracepoints were added in bitcoin#22006.
tracepoint | description
-- | --
`utxocache:flush` | Is called after the caches and indexes are flushed
`utxocache:add` | when a new coin is added to the UTXO cache
`utxocache:spent` | when a coin is spent
`utxocache:uncache` | when coin is removed from the UTXO cache
The tracepoints are further documented in `docs/tracing.md` and the usage is shown via the two newly added example scripts in `contrib/tracing/`.
ACKs for top commit:
laanwj:
Code and documentation review ACK 2bc51c5c32
Tree-SHA512: d6b4f435d3260de4c48b36956f9311f65ab3b52cd03b1e0a4ba9cf47a774d8c4b31878e222b11e0ba5d233a68f7567f8a367b12a6392f688c10c11529341e837
fa2c991ec9 refactor: Use underlying type of isminetype for isminefilter (MarcoFalke)
Pull request description:
This does not change behavior, but it would be good for code clarity and to avoid `-Wimplicit-int-conversion` compiler warnings to use the an int of the same width for both `isminetype` and `isminefilter`.
ACKs for top commit:
laanwj:
Code review ACK fa2c991ec9
shaavan:
crACK fa2c991ec9
promag:
Code review ACK fa2c991ec9.
Tree-SHA512: b3e255de7c9b1dea272bc8cb9386b339fe701f18580e03e997c270cac6453088ca2032e26e39f536d66cd1b6fda3e96bdbdc6e960879030e635338d0916277e6
459e208276 Exit early for an empty vChecks in CCheckQueue::Add (Hennadii Stepanov)
c43aa62343 Avoid excessive lock contention in CCheckQueue::Add (Hennadii Stepanov)
Pull request description:
This PR significantly reduces lock contention in the `CCheckQueue` class by releasing a mutex before calling `std::condition_variable::notify_one` and `std::condition_variable::notify_all`.
From C++ [docs](https://en.cppreference.com/w/cpp/thread/condition_variable/notify_one):
> The notifying thread does not need to hold the lock on the same mutex as the one held by the waiting thread(s); in fact doing so is a pessimization, since the notified thread would immediately block again, waiting for the notifying thread to release the lock.
Related to:
- #23167
- #23223
ACKs for top commit:
martinus:
ACK 459e208, codereview and tested. I first thought this introduced a segfault in `psbt_wallet_tests/psbt_updater_test` because that test failed for me, but thats a different issue fixed in #23403.
vasild:
ACK 459e208276
theStack:
Code-review ACK 459e208276
Tree-SHA512: c197858656392ba3ebcd638d713cf93c9fb48b7b3bad193209490d2828f9c7e3ae4dee6f84674f2f34dceed894139562e29579ee7299e06756c8c990caddc5ed
f13a22a631 wallet: Call load handlers without cs_wallet locked (João Barbosa)
Pull request description:
Don't have `cs_wallet` locked while calling each `context.wallet_load_fns`. A load handler can always lock `cs_wallet` if needed.
The lock was added in 1c7e25db0c to satisfy TSAN. With 44c430ffac most of the code requiring the lock is in `CWallet::AttachChain`. A comment is added to warn about wallets_mutex and cs_wallet lock ordering.
ACKs for top commit:
meshcollider:
re-utACK f13a22a631
ryanofsky:
Code review ACK f13a22a631. Only change since last review is adding a lock order comment
jonatack:
ACK f13a22a631
Tree-SHA512: d51976c3aae4bebc2d1997c88edff712d21fc5523801f5614062a10f826e164579973aeb1981bb1cbc243ecff6af3250362f544c02a79e5d135cbbca1704be62
This allows filters to be reconstructed when the best known block is
the Genesis block without needing to reindex.
It fixes Init errors seen in #23289.
fa4e09924b refactor: Replace validation.h include with forward-decl in miner.h (MarcoFalke)
fa0739a7d3 style: Sort file list after rename (MarcoFalke)
fa53e3a58c scripted-diff: Move miner to src/node (MarcoFalke)
Pull request description:
It is impossible to run the miner without a node (validation, chainstate, mempool, rpc, ...). Also, the module is in the node library. Thus, it should be moved to `src/node`.
Also, replace the `validation.h` include in the header with a forward-declaration.
ACKs for top commit:
theStack:
Code-review ACK fa4e09924b
Tree-SHA512: 791e6caa5839d8dc83b0f58f3f49bc0a7e3c1710822e8a44dede254c87b6f7531a0586fb95e8a067c181457a3895ad6041718aa2a2fac64cfc136bf04bb851d5
d8ee8f3cd3 refactor: Make CWalletTx sync state type-safe (Russell Yanofsky)
Pull request description:
Current `CWalletTx` state representation makes it possible to set inconsistent states that won't be handled correctly by wallet sync code or serialized & deserialized back into the same form.
For example, it is possible to call `setConflicted` without setting a conflicting block hash, or `setConfirmed` with no transaction index. And it's possible update individual `m_confirm` and `fInMempool` data fields without setting an overall consistent state that can be serialized and handled correctly.
Fix this without changing behavior by using `std::variant`, instead of an enum and collection of fields, to represent sync state, so state tracking code is safer and more legible.
This is a first step to fixing state tracking bugs https://github.com/bitcoin-core/bitcoin-devwiki/wiki/Wallet-Transaction-Conflict-Tracking, by adding an extra margin of safety that can prevent new bugs from being introduced as existing bugs are fixed.
ACKs for top commit:
laanwj:
re-ACK d8ee8f3cd3
jonatack:
Code review ACK d8ee8f3cd3
Tree-SHA512: b9f15e9d99dbdbdd3ef7a76764e11f66949f50e6227e284126f209e4cb106af6d55e9a9e8c7d4aa216ddc92c6d5acc6f4aa4746f209bbd77f03831b51a2841c3
68e5aafde3 build: add `--enable-lto` configuration option (fanquake)
Pull request description:
It's been 5 years since using LTO was first suggested for use when building Bitcoin Core, and it's time to revisit it again. Compilers, and their LTO implementations, have matured, and Bitcoin Core has come a long way in terms of pruning dependencies which may have proved troublesome (i.e Boost previously had issues when using LTO). We'll have even less Boost code after moving to `std::filesystem` (#20744).
Experimenting with LTO came up on IRC last night:
> sipa: jamesob: i'm interested in knowing whether "-flto" and/or "-fdata-sections -ffunction-sections -Wl,--gc-sections" are possible/beneficial with our current compiler suite; what would be a good way to have your test infrastructure benchmark things?
So this PR just adds the bare minimum to make it easier to configure, compile and perform some bench-marking using `-flto`. This PR doesn't do anything depends wise, however if we decide this is what we want to do, I'll expand the changes here.
I had previously had a PR open (#18605) to perform link time garbage collection (`-ffunction-sections -fdata-sections` & `-Wl,--gc-sections`), however moving straight to using LTO would be preferable.
Note that our minimum required set of compilers, GCC 8.1 and Clang 7, all support the `-flto` option.
Related #18579.
Previous discussion: #10616, #14277.
Previous related PRs: #10800 (`-flto`), #16791 (ThinLTO).
Guix build:
```bash
bash-5.1# find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
1f3a7c5be4169aaa444b481d3e65a7bb72da9007fee6e6c416ded2e70f97374b guix-build-68e5aafde3e8/output/aarch64-linux-gnu/SHA256SUMS.part
fa8f4cf223d9aaf0b2c1ef55ce61256a19cd1ad7f42b99d0b98c9a52fe6ad8ba guix-build-68e5aafde3e8/output/aarch64-linux-gnu/bitcoin-68e5aafde3e8-aarch64-linux-gnu-debug.tar.gz
9a9967078cd1849b4e85db619e1f55d305c6d44e9e013067c0e8d62c1ba54087 guix-build-68e5aafde3e8/output/aarch64-linux-gnu/bitcoin-68e5aafde3e8-aarch64-linux-gnu.tar.gz
18c71f30722102baaf3dfda67f7c7aac38723510b142e8df8ee7063c5d499368 guix-build-68e5aafde3e8/output/arm-linux-gnueabihf/SHA256SUMS.part
0854cc0d17c045a118df2a24e4cf36d727e7e7e2dea37c2492ee21b71cb79b4b guix-build-68e5aafde3e8/output/arm-linux-gnueabihf/bitcoin-68e5aafde3e8-arm-linux-gnueabihf-debug.tar.gz
215256897dde4e8412ed60473376c694a80c5479fb08039107fb62435f2816ef guix-build-68e5aafde3e8/output/arm-linux-gnueabihf/bitcoin-68e5aafde3e8-arm-linux-gnueabihf.tar.gz
5fad0d9d12bc514ec46ed5d66fd29b7da1376a4a69c3b692936f1ab2356e2f85 guix-build-68e5aafde3e8/output/dist-archive/bitcoin-68e5aafde3e8.tar.gz
4f32989d4ab1946048ca7caee9a983fa875be262282562f5a3e040f4bf92158e guix-build-68e5aafde3e8/output/powerpc64-linux-gnu/SHA256SUMS.part
ae45df309ae8ada52891efac0a369a69fed4ab93847a7bc4150a62230df4c8d7 guix-build-68e5aafde3e8/output/powerpc64-linux-gnu/bitcoin-68e5aafde3e8-powerpc64-linux-gnu-debug.tar.gz
0ced227de15cb578567131271e2effe80681b4d7a436c92bf1caec735a576fa4 guix-build-68e5aafde3e8/output/powerpc64-linux-gnu/bitcoin-68e5aafde3e8-powerpc64-linux-gnu.tar.gz
26fc5d2ccc1bc17ee0a146cacada6f4909d90c136ae640c8337332adce414ee0 guix-build-68e5aafde3e8/output/powerpc64le-linux-gnu/SHA256SUMS.part
9956b544d90a62a8ba9fc9dc6b6b7f0efe193357332ec19e88053a89d4aab37e guix-build-68e5aafde3e8/output/powerpc64le-linux-gnu/bitcoin-68e5aafde3e8-powerpc64le-linux-gnu-debug.tar.gz
be8e39ceea1d36086ce5fa93bfb138c68d3bdf0dd6950b192dfa27a65cce3836 guix-build-68e5aafde3e8/output/powerpc64le-linux-gnu/bitcoin-68e5aafde3e8-powerpc64le-linux-gnu.tar.gz
a7755edc394972885c4c77a7798007e5ba4126b177c4ff6224275c4fb8f3b1c4 guix-build-68e5aafde3e8/output/riscv64-linux-gnu/SHA256SUMS.part
b6d252993d8aae7582ad6385fe53c61c54c284c68ece6cb2b2d1ac9554e06139 guix-build-68e5aafde3e8/output/riscv64-linux-gnu/bitcoin-68e5aafde3e8-riscv64-linux-gnu-debug.tar.gz
bb4860f3bbd815f800333124ff901d880741792ab47097f49bda3a6931144da0 guix-build-68e5aafde3e8/output/riscv64-linux-gnu/bitcoin-68e5aafde3e8-riscv64-linux-gnu.tar.gz
3dd17deed5c5935fb28b62dfc7afca5caab0d67862cdcbf3337edae73e1d0c4c guix-build-68e5aafde3e8/output/x86_64-apple-darwin19/SHA256SUMS.part
fa2d68c54fda0816188c81ce2201a77340b82645da2ffe412526f92c297a82df guix-build-68e5aafde3e8/output/x86_64-apple-darwin19/bitcoin-68e5aafde3e8-osx-unsigned.dmg
f6e5accdcd201f522b6426e4d8cc9b3643d4d43a57d268fa0e79ea9a34cfac01 guix-build-68e5aafde3e8/output/x86_64-apple-darwin19/bitcoin-68e5aafde3e8-osx-unsigned.tar.gz
4e5a127df957d1c73b65925d685f6620e7bc5667efcb6dcd98be76effc22fc12 guix-build-68e5aafde3e8/output/x86_64-apple-darwin19/bitcoin-68e5aafde3e8-osx64.tar.gz
56ccd216a69acafacbdc6bae0bdcc1faa50b6a51be1aebfa7068206c88b3241a guix-build-68e5aafde3e8/output/x86_64-linux-gnu/SHA256SUMS.part
77b93dd5fad322636853e5b0244ffafd97cc97f3b4b4ee755d5f830b75d77d13 guix-build-68e5aafde3e8/output/x86_64-linux-gnu/bitcoin-68e5aafde3e8-x86_64-linux-gnu-debug.tar.gz
1feda932fc127b900316a232432b91e46e57ee12a81e12a7d888fdc3296219c1 guix-build-68e5aafde3e8/output/x86_64-linux-gnu/bitcoin-68e5aafde3e8-x86_64-linux-gnu.tar.gz
aa7c53ab4164b3736049065c3c24391fc5bd7f26b4bda4aa877c378f0636a125 guix-build-68e5aafde3e8/output/x86_64-w64-mingw32/SHA256SUMS.part
5e76148e67aef7e91e70074bfadc08e94373449ac3b966f4343b04d230c778fd guix-build-68e5aafde3e8/output/x86_64-w64-mingw32/bitcoin-68e5aafde3e8-win-unsigned.tar.gz
34123e3d818beeb70113caeda66945bc7cb9d9e987515d5b149bd17b4b38da90 guix-build-68e5aafde3e8/output/x86_64-w64-mingw32/bitcoin-68e5aafde3e8-win64-debug.zip
2bba7f40a2b23c6ea3d47c4f564ab54201bf27f7f57103a98cc9bceea4e70c4d guix-build-68e5aafde3e8/output/x86_64-w64-mingw32/bitcoin-68e5aafde3e8-win64-setup-unsigned.exe
0e7e124144af4a92a4344cf70a3b7c06fbd2b8782aee7ede7263893afa3a5ef0 guix-build-68e5aafde3e8/output/x86_64-w64-mingw32/bitcoin-68e5aafde3e8-win64.zip
```
ACKs for top commit:
laanwj:
Code review ACK 68e5aafde3
Tree-SHA512: 5c25249cc178b9d54159e268390c974b739df9458d773e23c14b14d808f87f7afe314058b3c068601a9132042321973b0c9b6f81becb925665eca2738ae9a613
3726a45958 refactor: replace RecursiveMutex m_added_nodes_mutex with Mutex (Sebastian Falbesoner)
7d52ff5c38 refactor: replace RecursiveMutex m_addr_fetches_mutex with Mutex (Sebastian Falbesoner)
d51d2a3bb5 scripted-diff: rename node vector/mutex members in CConnman (Sebastian Falbesoner)
574cc4271a refactor: remove RecursiveMutex cs_totalBytesRecv, use std::atomic instead (Sebastian Falbesoner)
Pull request description:
This PR is related to #19303 and gets rid of the following RecursiveMutex members in class `CConnman`:
* for `cs_totalBytesRecv`, protecting `nTotalBytesRecv`, `std::atomic` is used instead (the member is only increment at one and read at another place, so this is sufficient)
* for `m_addr_fetches_mutex`, protecting `m_addr_fetches`, a regular `Mutex` is used instead (there is no chance that within one critical section, another one is called)
* for `cs_vAddedNodes`, protecting `vAddedNodes`, a regular `Mutex` is used instead (there is no chance that within one critical section, another one is called)
Additionally, the PR takes the chance to rename all node vector members (vNodes, vAddedNodes) and its corresponding mutexes (cs_vNodes, cs_vAddedNodes) to match the coding guidelines via a scripted-diff.
ACKs for top commit:
vasild:
ACK 3726a45958
promag:
Code review ACK 3726a45958.
hebasto:
re-ACK 3726a45958
Tree-SHA512: 4f5ad41ba2eca397795080988c1739c6abb44c1204dddaa75cc38a396fa821fbe1010694ba7bead1b606beaa677661e66da2a5dca233b2937214f63a54848348
