4c07cf87e2288d4a8b0174ad64e52f455de9029e doc: document depends compiler configuration (will)
Pull request description:
Fixes: #33859
Previously one had to read the Makefile (and various *.mk configuration
files) to see how to correctly override CC and CXX when building native
depends packages.
Detail this in README.md to make it clearer.
ACKs for top commit:
maflcko:
review ACK 4c07cf87e2288d4a8b0174ad64e52f455de9029e 🛶
hebasto:
ACK 4c07cf87e2288d4a8b0174ad64e52f455de9029e.
sedited:
ACK 4c07cf87e2288d4a8b0174ad64e52f455de9029e
fanquake:
ACK 4c07cf87e2288d4a8b0174ad64e52f455de9029e
Tree-SHA512: ca61087b7c232c74782602a28d914e7bb35d1469472bb7862941c0fdec2277bf1d0e6604d4058c319c6a7b2cd04a70b7f0d0afbbb847e5c3812c6afb0cf1cfb3
22335474d768f99067856173ff2764b6db753f67 net: format peer+addr logs with `LogPeer` (Lőrinc)
e55ea534f74b2dd2a81e8f9972554303f7cf9c20 test: add pre-`LogPeer` net log assertion (Lőrinc)
736b17c0f0f23dca78e7266be6c98b24b0eda210 log: fix minor formatting in debug logs (Lőrinc)
9cf82bed3205d8405f56838d0e2ca0c72023a081 log: show placeholders for missing peer fields (Lőrinc)
Pull request description:
This is an alternative to #34293, but aims to address the remaining logging inconsistencies more broadly.
It extends the example fixed there to every instance, restores the original separator behavior, applies it consistently via a single helper, and adds tests for `logips` (covering both current and new behavior).
### Problem
After #28521 centralized peer address logging into `CNode::LogIP()`, the original comma separator before `peeraddr=` was lost, resulting in inconsistent formatting across net (and recent private broadcast) logs.
Some lines also had double spaces, empty fields, or mismatched format specifiers.
### Fix
Introduces `CNode::LogPeer(bool)` which always emits `peer=<id>` and, when `-logips=1`, appends `, peeraddr=<addr>`. This eliminates hand-rolled separators and makes peer identification predictable.
Minor issues (double spaces, empty placeholders, format specifiers) are fixed along the way in separate commits.
### Reproducer
Run with `-debug=net -logips=1` and observe peer log lines now show `peer=<id>, peeraddr=<addr>` (comma-separated). The new assertion in `feature_logging.py` automates this check.
ACKs for top commit:
naiyoma:
ACK 22335474d768f99067856173ff2764b6db753f67
vasild:
ACK 22335474d768f99067856173ff2764b6db753f67
sedited:
ACK 22335474d768f99067856173ff2764b6db753f67
Tree-SHA512: 562262a58c3042f139099ff4c41e3fc6a97505fe9603c2bf700a97fd0aa052954b47c14da0e50c1fc311db1ae6c04e6a92156c9b85e25c777a637b7766c1dafe
c6a6435cedac01a6800cac9285e2c599290e2e0d wallet: remove `DBErrors::NEED_REWRITE` enum value (rkrux)
61039d72a5da443a254b3229748fc64eee613757 wallet: remove unimplemented `RewriteDB` calls from SPKM (rkrux)
Pull request description:
ISTM that there is no implementation left of the `RewriteDB` method in any of the SPKMs, and thus, its call sites can be removed safely.
Also remove `DBErrors::NEED_REWRITE` enum value as its usage is outdated now.
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improve coverage are always welcome.
* All other changes should have accompanying unit tests (see `src/test/`) or
functional tests (see `test/`). Contributors should note which tests cover
modified code. If no tests exist for a region of modified code, new tests
should accompany the change.
* Bug fixes are most welcome when they come with steps to reproduce or an
explanation of the potential issue as well as reasoning for the way the bug
was fixed.
* Features are welcome, but might be rejected due to design or scope issues.
If a feature is based on a lot of dependencies, contributors should first
consider building the system outside of Bitcoin Core, if possible.
* Refactoring changes are only accepted if they are required for a feature or
bug fix or otherwise improve developer experience significantly. For example,
most "code style" refactoring changes require a thorough explanation why they
are useful, what downsides they have and why they *significantly* improve
developer experience or avoid serious programming bugs. Note that code style
is often a subjective matter. Unless they are explicitly mentioned to be
preferred in the [developer notes](/doc/developer-notes.md), stylistic code
changes are usually rejected.
-->
<!--
Bitcoin Core has a thorough review process and even the most trivial change
needs to pass a lot of eyes and requires non-zero or even substantial time
effort to review. There is a huge lack of active reviewers on the project, so
patches often sit for a long time.
-->
ACKs for top commit:
achow101:
ACK c6a6435cedac01a6800cac9285e2c599290e2e0d
furszy:
ACK c6a6435cedac01a6800cac9285e2c599290e2e0d
sedited:
ACK c6a6435cedac01a6800cac9285e2c599290e2e0d
Tree-SHA512: 050d3115ad436eb7728716c897c8d23c6dd0c6cd7a018dc023896d301b126bae5ba89b684811226a0ca4045b393b612296aeab3b3b5dfb7ed0bf443fc1eaef41
79571b918130e66436b2d43489835c38bb3ae3e3 threadpool: add ranged Submit overload (Andrew Toth)
Pull request description:
The current `ThreadPool::Submit` is not very efficient when we have a use case where we need to submit multiple tasks immediately. The `Submit` method must take the lock for each task, and notifies only a single worker thread. This will cause lock contention with the awakened worker thread trying to take the lock and the caller trying to submit the next task.
Introduce a `Submit` overload, which takes the lock once and submits a range of tasks, then notifies all worker threads after the lock is released.
This is needed for #31132 to be able to use `ThreadPool`.
ACKs for top commit:
l0rinc:
ACK 79571b918130e66436b2d43489835c38bb3ae3e3
rkrux:
ACK 79571b9
sedited:
Re-ACK 79571b918130e66436b2d43489835c38bb3ae3e3
willcl-ark:
ACK 79571b918130e66436b2d43489835c38bb3ae3e3
Tree-SHA512: 1fbe0c150f01b9ea5be3459cd10b817045af52eaf6f14a1a298a68853890da4033c1b21bdc6f995bb55029fb4ab536e9dbf58d98e2e1e12b25298fa3470b4ba6
CC/CXX only override compilers for target (host) packages. Native
build tool packages (native_capnp, native_qt, etc.) use separate
build_CC/build_CXX variables, which default to gcc/g++ on Linux.
On systems without gcc (e.g. Nix, Chimera Linux), native package
builds fail unless build_CC/build_CXX are also set explicitly.
Document how to override both sets of compilers.
fafdb8f635bc157f55e23890264d12170ecd41ae ci: Allow running iwyu ci in worktree (MarcoFalke)
fab73e213dee1057e9e759133767b17ec5b1f6ab ci: Reject unsafe execution of shell scripts (MarcoFalke)
Pull request description:
Currently, the iwyu CI fails to run in a git-worktree, or git-archive. This is due to the use of `git diff`.
Fix this by force-initializing a dummy git repo with a single dummy commit.
It may be possible to detect when `git diff` is not available in the directory, and only apply the fallback when needed, but the git history is not needed and it is easier to unconditionally apply the git init.
ACKs for top commit:
willcl-ark:
reACK fafdb8f635bc157f55e23890264d12170ecd41ae
hebasto:
ACK fafdb8f635bc157f55e23890264d12170ecd41ae, I have reviewed the code and it looks OK. Tested on Fedora 43.
sedited:
ACK fafdb8f635bc157f55e23890264d12170ecd41ae
Tree-SHA512: 572f1e2b9e215c2804095382498abb5b8636e3a49d5ba2a736b975e06afa2881d815b854a8a593d0f187c7c6b55034688e11f46d6814edfe7c29505197e80b18
b50376881929ff23b1148028ac6ca42617af04d8 fuzz: register PeerManager in process_message(s) (Eugene Siegel)
Pull request description:
This lets CValidationInterface callbacks be hit (just `BlockChecked` using the corpus from qa-assets). Also remove no-op SyncWithValidationInterfaceQueue since there are no validation interfaces registered in ResetChainman.
ACKs for top commit:
marcofleon:
code review ACK b50376881929ff23b1148028ac6ca42617af04d8
dergoegge:
utACK b50376881929ff23b1148028ac6ca42617af04d8
Tree-SHA512: 51c0c9a3396ee898b1dce2c65c2fc7b61ea6d04342d181b1b60be1677bc0c2828b9d970673f67501e3579ae002392cd5f0e3268aae1b216ff332e2654fd1fe14
02d047fd5b93d96f159db2b8e95fc39450505159 refactor: add overflow-safe `CeilDiv` helper (Lőrinc)
Pull request description:
### Problem
The codebase has many open-coded ceiling-division expressions (for example `(x+y-1)/y`) scattered across files.
These are less readable, duplicate logic, and can be overflow-prone in edge cases.
### Fix
Introduce a small overflow-safe integer helper, `CeilDiv()`, and use it in existing **unsigned** callsites where the conversion is straightforward and noise-free.
### What this PR does
* Adds `CeilDiv()` to `src/util/overflow.h` for unsigned integral inputs.
* Keeps the precondition check `assert(divisor > 0)`.
* Replaces selected unsigned ceiling-division expressions with `CeilDiv(...)`.
* Adds focused unit tests in `src/test/util_tests.cpp` for the migrated patterns.
---
This is a pure refactor with no intended behavioral change.
Signed arithmetic callsites are intentionally left unchanged in this PR.
This PR changed a few more things originally but based on feedback reverted to the simplest cases only.
ACKs for top commit:
rustaceanrob:
ACK 02d047fd5b93d96f159db2b8e95fc39450505159
hodlinator:
ACK 02d047fd5b93d96f159db2b8e95fc39450505159
sedited:
ACK 02d047fd5b93d96f159db2b8e95fc39450505159
Tree-SHA512: b09336031f487e6ce289822e0ffeb8cfc8cfe8a2f4f3f49470748dfbd0a6cbab97498674cb8686dd2bd4ab6dd0b79cfdf2da00041fee12d109892e1bc5dde0ff
0ebc6891e2168c3ff1c9ad2250e765446610e75e depends: delete Boost extra files (fanquake)
168997e9b5720e3c58e2fc1a073d0be3cd39d971 depends: disable Qt sbom generation (fanquake)
Pull request description:
1 followup to #34650, to disable sbom generation.
1 commit to Boost, to cleanup `.natvis` files that end up in share.
ACKs for top commit:
hebasto:
ACK 0ebc6891e2168c3ff1c9ad2250e765446610e75e, tested on Ubuntu 25.10.
Tree-SHA512: 728b51d798a30c54df915564446a7a8648eb4fc27adb8c18b8202df506e2ff61e74516cfe4d6af1af72279255fc75fe14cb43403632a978637781d59eb11fc0f
af041c405756d3b8bb04cb2ebd8c32cf237ac2a9 wallet: Always rewrite tx records during migration (Ava Chow)
Pull request description:
Since loading a wallet may change some parts of tx records (e.g. adding nOrderPos), we should rewrite the records instead of copying them so that the automatic upgrade does not need to be performed again when the wallet is loaded.
This is useful for future PRs I'm working on where we need to be sure about what data exists in a tx record in descriptor wallets.
ACKs for top commit:
rkrux:
lgtm ACK af041c405756d3b8bb04cb2ebd8c32cf237ac2a9
Eunovo:
ACK af041c4057
furszy:
ACK af041c405756d3b8bb04cb2ebd8c32cf237ac2a9
Tree-SHA512: 96984e46fe110a7749495965587b88a94d1297794e5d8b632b89dcdb2ebc1cd3070cd4458cf8e1b4ec0c76c4e56994f21867c44fa74f25739cbd9c0c911732a6
d339884f1dfac3749c6214ae896b4354cf9ee28e bench: add script verification benchmark for P2TR key path spends (Sebastian Falbesoner)
dd93362a1d874eb761ed7026956a490b5ed0472b bench: simplify script verification benchmark, generalize signing (Sebastian Falbesoner)
Pull request description:
We currently benchmark Schnorr signature verification only in the context of block validation ([`ConectBlock*`](8bb77f348e/src/bench/connectblock.cpp (L107)) benchmarks), but not individually for single inputs [1]. This PR adds a script verification benchmark for P2TR key path spends accordingly, by generalizing the already existing one for P2WPKH spends.
This should make it easier to quantify potential performance improvements like e.g. https://github.com/bitcoin-core/secp256k1/pull/1777, which allows to plug in our HW-optimized SHA256 functions to be used in libsecp256k1 (see the linked example commit f68bef06d9). IIRC from last CoreDev, the main speedup from this is expected for ECDSA signing though (as this involves quite a lot of hashing), but it still makes sense to have verification benchmarks available for both signature types as well.
(An alternative way could be to add benchmarks for the signing/verifying member functions `CKey::Sign{,Schnorr}`, `CPubKey::Verify` and `XOnlyPubKey::VerifySchnorr` directly, if we prefer that.)
[1] this claim can be practically verified by putting an `assert(false);` into `XOnlyPubKey::VerifySchnorr`: the three benchmarks crashing are `ConnectBlockAllSchnorr`, `ConnectBlockMixedEcdsaSchnorr` and `SignTransactionSchnorr` (as signing includes verification)
ACKs for top commit:
furszy:
ACK d339884f1dfac3749c6214ae896b4354cf9ee28e
sedited:
Re-ACK d339884f1dfac3749c6214ae896b4354cf9ee28e
w0xlt:
ACK d339884f1dfac3749c6214ae896b4354cf9ee28e
Tree-SHA512: efd20444984bdf1dab4d3d876fdbe2a3a838d7cebc0e31e26683009b81afe4dab8611e2c28c87e46fe8b7e305895c93e461b7934a5aaf293f72b19488b9cec60
73e385311055eb6c0e3ddb2221f927a3872af1fb test: add test for rebroadcast of transaction received via p2p (Martin Zumsande)
Pull request description:
The wallet doesn't only rebroadcast transactions it created, but also relevant transactions received via p2p.
Since this is not self-evident, add test coverage for it.
ACKs for top commit:
maflcko:
re-ACK 73e385311055eb6c0e3ddb2221f927a3872af1fb 🦌
vasild:
ACK 73e385311055eb6c0e3ddb2221f927a3872af1fb
furszy:
ACK 73e385311055eb6c0e3ddb2221f927a3872af1fb
danielabrozzoni:
tACK 73e385311055eb6c0e3ddb2221f927a3872af1fb
Tree-SHA512: b5249bb5eb6a124a98030816319aa4364d7aee9c28ee28750ebba5fc8d6bfc9d7960a97ed638611f4560e051760ec4a7a75d302a4cb106dbfadfe11adc604f22
b3046cca7182f3399a221757318d24e203092301 doc: add release notes for #26988 (stratospher)
675be930245e5c1ac2f7940bcf308430adeb18ee cli: modify -addrinfo to use getaddrmaninfo RPC endpoint (stratospher)
Pull request description:
Rework of `-addrinfo` CLI is done using `getaddrmaninfo` RPC proposed in https://github.com/bitcoin/bitcoin/pull/27511. This would be useful for users who want to know the total number of addresses the node knows about and can make connections to.
Currently, `-addrinfo` returns total number of addresses the node knows about after filtering them for quality + recency using [`isTerrible`](4b51290f71/src/addrman.cpp (L808)). However `isTerrible`addresses [don't matter](https://github.com/bitcoin/bitcoin/pull/26988#discussion_r1147725684) when selecting outbound peers to connect to. Total number of addresses the node knows about could be higher than what `-addrinfo` currently displays. See https://github.com/bitcoin/bitcoin/pull/24370.
ACKs for top commit:
ajtowns:
ACK b3046cca7182f3399a221757318d24e203092301
pablomartin4btc:
re-ACK b3046cca7182f3399a221757318d24e203092301
vasild:
ACK b3046cca7182f3399a221757318d24e203092301
sr-gi:
tACK b3046cca7182f3399a221757318d24e203092301
Tree-SHA512: 764b74f9e0e28a65f8644a31228ca70f6e2cd4c6a93d8f29093ed7a241cd20a81e24b4babace170d945fb28078793d52ec1f4bce898a6d478950fb29ce54af91
d67c8ed78894a8f5e3d6c5ab949c484e64241266 clusterlin: update SFL comments for deterministic order (Pieter Wuille)
Pull request description:
This was missed in #34257.
ACKs for top commit:
marcofleon:
ACK d67c8ed78894a8f5e3d6c5ab949c484e64241266
achow101:
ACK d67c8ed78894a8f5e3d6c5ab949c484e64241266
Tree-SHA512: e381da09eb686e69c0fb32cc16dff7ae108f13ecb07bc1466f504a7b4c773d4557599c659f6d2e9ba0037ed89179c2e187f383a917e0242c4c795cf6e1c9cec6
0690a5d0f27b52435782815b9d7520684486a9ad Update embedded asmap to 1772726400 (Fabian Jahr)
Pull request description:
This updates the currently embedded data ahead of the v31 release.
It currently uses the map from the 1772726400 run
- Pull request: https://github.com/bitcoin-core/asmap-data/pull/45
- Issue: https://github.com/bitcoin-core/asmap-data/issues/44
The attestation process to accompany these runs is still rather new but sigs are collected here: https://github.com/asmap/asmap.sigs/tree/main/1772726400
ACKs for top commit:
achow101:
ACK 0690a5d0f27b52435782815b9d7520684486a9ad
sedited:
ACK 0690a5d0f27b52435782815b9d7520684486a9ad
hodlinator:
ACK 0690a5d0f27b52435782815b9d7520684486a9ad FWIW
Tree-SHA512: 61c43ea1bd44425aa07056316220a0e9f3c4ec59cd9447277e114b2004fd7f9e525fb56ac0b3f590447d1a454fa3c636c7ac8c034a673a76126ce51e005cce49
46189fd5264580d0320976faf81aa2f45e1a7760 doc: update http worker thread names (rkrux)
Pull request description:
After using `Threadpool` for HTTP server in PR 33689, the previously
documented HTTP worker thread names are outdated. This commit makes
the corresponding changes to document new names for the HTTP worker
threads. Below is the output from the `thead list` command after
attaching `lldb` to `bitcoind`.
```zsh
thread #3: tid = 0xfe551, 0x00007ff80e3536f6 libsystem_kernel.dylib`__psynch_cvwait + 10, name = 'b-http_pool_0'
thread #4: tid = 0xfe552, 0x00007ff80e3536f6 libsystem_kernel.dylib`__psynch_cvwait + 10, name = 'b-http_pool_1'
```
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improve coverage are always welcome.
* All other changes should have accompanying unit tests (see `src/test/`) or
functional tests (see `test/`). Contributors should note which tests cover
modified code. If no tests exist for a region of modified code, new tests
should accompany the change.
* Bug fixes are most welcome when they come with steps to reproduce or an
explanation of the potential issue as well as reasoning for the way the bug
was fixed.
* Features are welcome, but might be rejected due to design or scope issues.
If a feature is based on a lot of dependencies, contributors should first
consider building the system outside of Bitcoin Core, if possible.
* Refactoring changes are only accepted if they are required for a feature or
bug fix or otherwise improve developer experience significantly. For example,
most "code style" refactoring changes require a thorough explanation why they
are useful, what downsides they have and why they *significantly* improve
developer experience or avoid serious programming bugs. Note that code style
is often a subjective matter. Unless they are explicitly mentioned to be
preferred in the [developer notes](/doc/developer-notes.md), stylistic code
changes are usually rejected.
-->
<!--
Bitcoin Core has a thorough review process and even the most trivial change
needs to pass a lot of eyes and requires non-zero or even substantial time
effort to review. There is a huge lack of active reviewers on the project, so
patches often sit for a long time.
-->
ACKs for top commit:
l0rinc:
ACK 46189fd5264580d0320976faf81aa2f45e1a7760
hebasto:
ACK 46189fd5264580d0320976faf81aa2f45e1a7760.
theStack:
ACK 46189fd5264580d0320976faf81aa2f45e1a7760
furszy:
ACK 46189fd5264580d0320976faf81aa2f45e1a7760
sedited:
ACK 46189fd5264580d0320976faf81aa2f45e1a7760
Tree-SHA512: dc17dcd942a562da0e5ec9b6185db12d7e8ab8539fd6a78e944e95a723040c03c069f6806b8fc2f070839cb7012709d434b9e3e3bce08744dd818abbfe72e3ff
9f3752c43778b7803f4c6810aa570542847f3261 ci: use latest versions of lint deps (fanquake)
Pull request description:
Use the latest available versions, except for LIEF, which is changed with Guix.
ACKs for top commit:
hebasto:
ACK 9f3752c43778b7803f4c6810aa570542847f3261, I've verified the releases against https://pypi.org and https://github.com/becheran/mlc.
Tree-SHA512: e6ed79bb7dc8601ed0708eb7b53cbf4cf843b69829c073c41e9d97be690b4b2bf9ea5ecf250e05cbacba4ad35df06aa3e2cb2ff319145a34e1a7831cf182ec21
6b20ad84e0a66c48976a599a7cc5b9b19cd49fab doc: update build guides pre v31 (fanquake)
Pull request description:
We are testing on FreeBSD 15 (nightly) and macOS 26 (CI).
ACKs for top commit:
maflcko:
lgtm ACK 6b20ad84e0a66c48976a599a7cc5b9b19cd49fab
hebasto:
ACK 6b20ad84e0a66c48976a599a7cc5b9b19cd49fab.
Tree-SHA512: c97528d8db762f44d6ada064162264eedf67b154de0f357b915b14d4260b6802b099d19156ab5747120e4c632b21a8c8de92e2a903bbfcfcf4376549694eef1f
8bc62ce173dd492ca862a692a88d526d406e0eb3 doc: Update asmap-data repository rule for file inclusion (Fabian Jahr)
Pull request description:
This updates the decision rule for file inclusion in the asmap-data repository after a collaborative run following discussion in the latest run: https://github.com/bitcoin-core/asmap-data/issues/44
The change is small but does a few things:
- No minimum number of participants recommended anymore
- Minimum number of matches set to 5 (which was previously the number of recommended min participants)
- The result that is taken does not need to see a match between the majority of participants, it only needs to have the most matches
- Participants that saw a match should sign for it
I also thought about adding an explicit tie-breaker if there are two results with the same number of matches but since both results are equally valid I think it won't be needed (?)
ACKs for top commit:
hodlinator:
ACK 8bc62ce173dd492ca862a692a88d526d406e0eb3
luisschwab:
ACK 8bc62ce173dd492ca862a692a88d526d406e0eb3
janb84:
Concept ACK 8bc62ce173dd492ca862a692a88d526d406e0eb3
Tree-SHA512: 439ba1275662122e226aa472dd96fa8ed0e1af3e91d695c711e082d7ba182883e1174f954d2531cdf7e05bdbeab5ef088f8bcbd39c7124afe82baf2026ed391b
This lets CValidationInterface callbacks be hit. Also remove
no-op SyncWithValidationInterfaceQueue since there are no validation
interfaces registered in ResetChainman.
c08f0c3c29b58223822c3fbf0ced69a886bfbf0d ci: remove TODOs from retry (fanquake)
Pull request description:
TODOs are good targets for LLMs to generate PRs. Remove these TODOs, which aren't needed, to prevent that.
ACKs for top commit:
maflcko:
lgtm ACK c08f0c3c29b58223822c3fbf0ced69a886bfbf0d
hebasto:
ACK c08f0c3c29b58223822c3fbf0ced69a886bfbf0d.
Tree-SHA512: 4561cd3221b06bc877c4dadd79c9be02839c8ce7dee924d118897bb0a96d223ce60e3f1c2dfb8b6eecf9abe96c188dc875134395ed15cac669b5ae1435fc1e5f
501a3dd4ad4a545a05663a78cec61575966045c7 walletdb: hash pubkey/privkey in one shot to avoid leaking secret data (Sebastian Falbesoner)
Pull request description:
In several places in the wallet DB module, byte strings containing serialized public keys and secret keys are created in order to be hashed. To avoid sensitive data lingering in memory (and potentially leaking), don't store the preimage, but hash both public key and secret key in one shot, using the overloaded `Hash` function:
d198635fa2/src/hash.h (L82-L88)
See e.g. #31166 and #31774 for similarly themed PRs (Note that in #31166 we used the explicit `memory_cleanse` approach though, as changing the allocator was not possible.)
ACKs for top commit:
davidgumberg:
crACK 501a3dd4ad
furszy:
ACK 501a3dd4ad4a545a05663a78cec61575966045c7
rkrux:
ACK 501a3dd
theuni:
ACK 501a3dd4ad4a545a05663a78cec61575966045c7
Tree-SHA512: 8a71685b26bf89fca181aed6512a8db843b6d1dc740a468bb33fb2a629a23167a9676c228d1077ad8db2df9db80f47e32ec013737e93df8ee6f4ba505d3d50c9
3a83715c2adb99c98867144690452074b3788b54 depends: link to upstream qt issue (fanquake)
Pull request description:
Follows up to:
https://github.com/bitcoin/bitcoin/pull/34650#discussion_r2837726376.
https://qt-project.atlassian.net/browse/QTBUG-144864
ACKs for top commit:
sedited:
ACK 3a83715c2adb99c98867144690452074b3788b54
hebasto:
ACK 3a83715c2adb99c98867144690452074b3788b54.
Tree-SHA512: a0ad902bb8d19cd065410dc3a9f15c83d4ff3807219b5fd6084f09a12c4003ef89fc28b4237311baa39c534d2ae1194b70b2d9dd036d4cf6f8a8385362f411b7
fa0587a306430b35d2852ee32f7eee2adb8c1e64 test: Remove fixed TODO in address_to_scriptpubkey (MarcoFalke)
Pull request description:
After commit d178082996dc3000f42816f89afcf3fa4d31e159 added the bech32 format, the TODO about adding other formats can be removed.
ACKs for top commit:
sedited:
ACK fa0587a306430b35d2852ee32f7eee2adb8c1e64
rkrux:
crACK fa0587a306
musaHaruna:
ACK [fa0587a](fa0587a306)
Tree-SHA512: 49a8a514bed11e945bd8f9b13b84ae14b4dbc8a7ebb7224b1746776d9dbf68abc3b53d67f1b7fff83bc4360b15324fb96611550f4aca808b16beb03bcbfd0a55
After using `Threadpool` for HTTP server in PR 33689, the previously
documented HTTP worker thread names are outdated. This commit makes
the corresponding changes to document new names for the HTTP worker
threads. Below is the output from the `thead list` command after
attaching `lldb` to `bitcoind`.
```zsh
thread #3: tid = 0xfe551, 0x00007ff80e3536f6 libsystem_kernel.dylib`__psynch_cvwait + 10, name = 'b-http_pool_0'
thread #4: tid = 0xfe552, 0x00007ff80e3536f6 libsystem_kernel.dylib`__psynch_cvwait + 10, name = 'b-http_pool_1'
```
a61907e5d9c1988eb1e018cae5831d6b94f4da7f doc: explain swapping in `reduce-memory.md` (Lőrinc)
Pull request description:
### Problem
Sustained [heavy swapping](https://en.wikipedia.org/wiki/Thrashing_(computer_science)) can grind execution to a halt, but today users get no direct warning from the node when this happens, and this caveat is not documented.
### Fix
We can’t easily detect heavy swap pressure in a reliable, cross-platform way, but we can document what swapping is and why it can severely degrade IBD performance.
---
Note: An earlier version of this PR attempted to detect and warn on heavy swapping (Linux-only), but it was changed to documentation based on review feedback.
ACKs for top commit:
ajtowns:
utACK a61907e5d9c1988eb1e018cae5831d6b94f4da7f
sedited:
ACK a61907e5d9c1988eb1e018cae5831d6b94f4da7f
Tree-SHA512: b21c40d07d78d890c19d3a17faad4ab4127688884dc433a1bdb63d18de07628c048227eba2f1258c6b542a71a986d4250f8abf8f8ffe0cda433ce0c8673978d4
fec58229fa671cb870ebf795b54b73b7e22a1eb6 contrib: Update fixed feeds (Ava Chow)
27fbdb009f2f007239c4845cad528487b30598bd makeseeds: Choose node info with most recent success when deduplicating (Ava Chow)
982883a1bc4d03efb78d296849a3b8fa04ded26d makeseeds: Update known user agents (Ava Chow)
Pull request description:
ACKs for top commit:
fjahr:
ACK fec58229fa671cb870ebf795b54b73b7e22a1eb6
Tree-SHA512: 2852a9a6a7c299ce04ee4dc438af9547d56a860858201ad2ccdea14640b17876e7e9841ce3a30030e2482cd04e9b386f7ede5c4e51582ebd09b9ce0a2a0bc43b
89386e700ebc232e2beab3a3f3ea0d1ae78ac203 kernel: Use fs:: namespace and unicode path in kernel tests (sedited)
Pull request description:
Add support for unicode characters in paths to the kernel tests by using our fs:: wrappers for std::filesystem calls and adding the windows application manifest to the binary. This exercises their handling through the kernel API.
ACKs for top commit:
hebasto:
ACK 89386e700ebc232e2beab3a3f3ea0d1ae78ac203.
w0xlt:
ACK 89386e700ebc232e2beab3a3f3ea0d1ae78ac203
Tree-SHA512: 7b541f482d84a66c89eec63aea0e7f7626bbbd62082ad7a7fb2c7a517296c291a6ff301c628e5e9e1d7b850ead89005141481a2bfd06d8a9081622e32f7340cc
faa016af542763492a63de8cc972f8f4b52a58cd refactor: Use aliasing shared_ptr in Sock::Wait (MarcoFalke)
Pull request description:
Currently, a no-op lambda is used as the deleter for the temporary shared pointer helper in `Sock::Wait`. This is perfectly fine, but has a few style issues:
* The lambda needs to be allocated on the heap
* It triggers a false-positive upstream GCC-16-trunk bug: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=123912
Fix all issues by just using an aliasing shared pointer, which points to `this`, but is otherwise empty (sits on the stack without any heap allocations).
ACKs for top commit:
hodlinator:
ACK faa016af542763492a63de8cc972f8f4b52a58cd
sedited:
ACK faa016af542763492a63de8cc972f8f4b52a58cd
vasild:
ACK faa016af542763492a63de8cc972f8f4b52a58cd
Tree-SHA512: b7330862204e79fb61f30694accb16f9a24e5722bd0ceb098ca27c877cff921afa00c0cfd953d4cbb355e6433706961a25b628efefdbe0b48bdec2941eaaee7a
fa9d0623a3153a2ea33540611c26378f05e0d7d4 doc: Use relative markdown links (MarcoFalke)
Pull request description:
Using fully resolved links (to the `master` branch) in markdown files is problematic, because:
* When the target file is (re)moved, such a link will be a 404
* When reading docs for a previous commit/release, one is redirected to the master branch on such a link
* When the target file has been updated, it may no longer apply to the commit/release one came from (changed options, etc)
Fix all issues by using relative markdown links. Note that this only works in markdown. Also, release notes are left as-is, because they will be shared stand-alone externally, so can't use relative links.
ACKs for top commit:
kevkevinpal:
tACK [fa9d062](fa9d0623a3)
l0rinc:
code review ACK fa9d0623a3153a2ea33540611c26378f05e0d7d4
rkrux:
tACK fa9d0623a3153a2ea33540611c26378f05e0d7d4
sedited:
ACK fa9d0623a3153a2ea33540611c26378f05e0d7d4
Tree-SHA512: 74cd661f20f93dc1af602ab4c6ff79673ff48fc956aca1cdd0039b127914a83fb61cff61ea92c8978c85fa500d1a1423bf9739bce261860fe037c8dfefb8acad
5b2c3960b929bd015911783fb59e3660b0810f03 test: clean up tx resurrection (re-org) test in feature_block.py (Sebastian Falbesoner)
Pull request description:
The following comment about ECDSA signatures created with the test framework not passing mempool policy has been obsolete for a long time (at least since 2019, see PR #15826):
8c07800b19/test/functional/feature_block.py (L1167-L1172)
so remove it. While at it, change the resurrected txs to be indeed standard valid, so the `acceptnonstdtxn=1` parameter can also be removed from the functional test.
Kudos to stratospher, who (IIRC) mentioned this outdated comment a while ago.
ACKs for top commit:
sedited:
ACK 5b2c3960b929bd015911783fb59e3660b0810f03
Bortlesboat:
Tested ACK 5b2c3960b929. Ran feature_block.py locally, passes. Nice cleanup — removing `-acceptnonstdtxn=1` by making the resurrection txs standard (P2PK) is the right approach. The old unsigned OP_TRUE workaround comment explains why this was needed, glad to see it gone.
stratospher:
ACK 5b2c3960. nice!
Tree-SHA512: 94517e432647a8e8db51fad90a26493c57ce9dfd924b17ce909ea40d50bc6279d974c6d2046b34f5f92b481ca5e0abdedaf412ce41b4cb904605c140a8dba583
a067ca34106817565e02daca52b3175266714c25 [doc] coin selection filters by max cluster count, not descendant (glozow)
f7be5fb8fc7d2a5831810a0b91666fc774b64b8f [refactor] rename variable to clarify it is unused and cluster count (glozow)
Pull request description:
Followup to #33629.
Fix misleading docs and variable names. Namely, `getTransactionAncestry` returns the cluster count, not max descendant count of ancestor set (not worth reimplementing as it is merely a heuristic). No behavior changes - I don’t think much needs to be changed for the first release containing cluster mempool.
Current `CoinEligibilityFilter`s enforce a maximum ancestor count (summed across all outputs, potentially overestimating) and max descendant count across ancestors of the output.
Since #33629, these filters have started using cluster count instead of max desc count across ancestors. The change isn’t dangerous, as the cluster count bounds descendant count as well. Currently, the wallet is essentially enforcing a mixture of both limits - this is good while we are transitioning. Note that the cluster count enforced is 25, not 64, since it's grabbing the node's descendant count limit. While it is not an apples-to-apples comparison, a cluster count limit of 25 helps us avoid busting legacy descendant limits (which will be common on the network for a while).
Potential things for the future, out of scope for this PR:
- When we get rid of the ancestor/descendant config options, `getPackageLimits` can probably be replaced with hard-coded values.
- Change the `OutputGroup`s to track the actual cluster count that results from spending these outputs and merging their clusters.
- Loosen from 25 after that policy is no longer common.
- Clean up `getPackageLimits`.
ACKs for top commit:
achow101:
ACK a067ca34106817565e02daca52b3175266714c25
ismaelsadeeq:
reACK a067ca34106817565e02daca52b3175266714c25
rkrux:
crACK a067ca34106817565e02daca52b3175266714c25
Tree-SHA512: d7cacd5bf668d42e26e8b83e42a42c280929c3bfd554c3db1de605e5939f8b36c14ecfd2839abeb4eec352363df1891b3420a693c250916391ab10a5ce26396b
Avoid storing the privkey in a vector, which could linger in memory
and potentially leak sensitive data. An alternative approach is to
use `secure_allocator` for the `std::vector` instances, but this
commit has the advantage of also deduplicating code at the same shot.
Thanks to @theuni for suggesting this.
44538f8ada868cb73ffa7a4bfb5267046d3f6b85 kernel: Add recent assumeutxo snapshot info (Ava Chow)
58c2e23fca05d201dfcea092ab032c943c9d5665 kernel: Update headerssync params (Ava Chow)
cf261b071f68b1967077a6d3baac10ea024e653a kernel: update chainTxData (Ava Chow)
8eaf1d26d446ecdc5c69b61334ee51a6f41050d3 kernel: update defaultAssumeValid and minimumChainWork (Ava Chow)
5ca0c55517d4c8e41a1dd9904545d82ee09adedf kernel: update assumed blockchain and chainstate sizes (Ava Chow)
Pull request description:
Update chainparams and headerssync params per the release process.
Also added new assumeutxo snapshots for each network. I've uploaded snapshots to https://achow101.com/files/utxo-snapshots/
ACKs for top commit:
Sjors:
ACK 44538f8ada868cb73ffa7a4bfb5267046d3f6b85
fjahr:
ACK 44538f8ada868cb73ffa7a4bfb5267046d3f6b85
janb84:
ACK 44538f8ada868cb73ffa7a4bfb5267046d3f6b85
sipa:
ACK 44538f8ada868cb73ffa7a4bfb5267046d3f6b85. I re-did all the mainnet parameters, but did not look closely at the other networks.
jaonoctus:
ACK 44538f8ada868cb73ffa7a4bfb5267046d3f6b85
Tree-SHA512: f9b6ccc967c5ef58f734245df459c3136491e9b6a0f6e36f4272bc0787e7b59eabe47a8c8b19a90267eca4a0b5851dfbf45153f96eac599c417f148b3cf264cf
f580cc7e9f26331f7f03a8bbc5722521eb159bb2 doc: Fix `fee` field in `getblock` RPC result (nervana21)
Pull request description:
The `fee` field in the `getblock` RPC result (verbosity 2 and 3) may be omitted when block undo data is not available. Marking it optional in the `RPCResult` aligns the documented schema with the runtime behavior.
ACKs for top commit:
mercie-ux:
ACK f580cc7e9f
satsfy:
ACK f580cc7e9f
instagibbs:
ACK f580cc7e9f26331f7f03a8bbc5722521eb159bb2
w0xlt:
ACK f580cc7e9f26331f7f03a8bbc5722521eb159bb2
luke-jr:
ACK f580cc7e9f26331f7f03a8bbc5722521eb159bb2
Tree-SHA512: e3b44a48e17e21b906967aef124688a34aea2c6af3b6df3c47693fd3002d33e824f764c0060a7ab07751b98567c29eb16f3b3c07bf2999db080ff7adfd087dfd
44feab23a7c6060a3b432c04e3f952c5a7104325 script: Fix undefined behavior in Clone() -- std::transform writes past end of empty vector (Weixie Cui)
Pull request description:
# Motivation
This patch fixes undefined behavior in Clone() in src/script/descriptor.cpp.
When std::transform is used with providers.begin() or subdescs.begin() as the output iterator, the vectors have been reserve()d but have size 0. Writing through begin() in that case writes past the logical end of the vector, which is undefined behavior.
ACKs for top commit:
maflcko:
lgtm ACK 44feab23a7c6060a3b432c04e3f952c5a7104325
rkrux:
ACK 44feab23a7c6060a3b432c04e3f952c5a7104325 because it gets rid of the possible undefined behaviour.
frankomosh:
Code Review ACK 44feab23a7c6060a3b432c04e3f952c5a7104325. Fix seems minimal and correct.
Tree-SHA512: 8af3b6d97c139b32bd47d4c452b6b16befdaa7028a7bc1b6de0ab1f0a8cb35eb068710316a2c07fa60856e17e25307931aa3125b4f41d0fe7726b435483a52db
