924f25f6fc766cb89a23fb1a6ad632947742da9f bench: Match ConnectBlock tx output counts (monlovesmango)
Pull request description:
There turned out to be a mismatch in the tx output counts which caused 'ConnectBlockMixedEcdsaSchnorr' benchmark to run slower than 'ConnectBlockAllEcdsa' and 'ConnectBlockAllSchnorr'. This commit makes the tx output counts uniform across all benchmarks.
This commit also renames the 'taproot_tx' variable to 'tx' to reflect that this variable represents a general tx and not just a taproot tx.
ACKs for top commit:
davidgumberg:
Tested ACK 924f25f6fc766cb89a23fb
Prabhat1308:
reACK [`924f25f`](924f25f6fc)
janb84:
re ACK [924f25f](924f25f6fc)
josibake:
ACK 924f25f6fc
Tree-SHA512: bbf33e0c31b0c46571fd5d6ecd32426e7e823f9e156fd3d39a975bd5f0c1b6cd3dda55fa869cb0954c68dcf28cf4d0a0af40a72e440c1c78380b5b98e1eb6615
fac978fb213fbd6668194d8b9810ddd10f8a7323 test: Remove fragile and ancient release 0.17 wallet test (MarcoFalke)
Pull request description:
The test checks that the 0.17 wallet rejects wallet files created in "the future".
This is nice, and good to know. However,
* The 0.17 release is ancient and should be unused outside of tests, especially to load future wallets.
* The test intermittently fails, due to ancient RPC server bugs, that were fixed in the meantime. [1]
* Albeit they are not identical, the 0.18 release is still checked in this test, so any theoretical bug that would be caught by 0.17 is hopefully still caught by 0.18 as well.
So fix all issues by removing the test case.
[1] For example from https://api.cirrus-ci.com/v1/task/6161588714995712/logs/ci.log:
```
190/321 - [1mwallet_backwards_compatibility.py --descriptors[0m failed, Duration: 23 s
[17:21:40.700]
[17:21:40.700] [1mstdout:
[17:21:40.700] [0m2025-04-02T21:21:16.575000Z TestFramework (INFO): PRNG seed is: 5772716217847090743
[17:21:40.700] 2025-04-02T21:21:16.580000Z TestFramework (INFO): Initializing test directory /ci_container_base/ci/scratch/test_runner/test_runner_₿_🏃_20250402_210134/wallet_backwards_compatibility_134
[17:21:40.700] 2025-04-02T21:21:26.378000Z TestFramework (INFO): Test wallet backwards compatibility...
[17:21:40.700] 2025-04-02T21:21:33.191000Z TestFramework (INFO): Testing 0.19 addmultisigaddress case (#18075)
[17:21:40.700] 2025-04-02T21:21:33.637000Z TestFramework (INFO): Test that a wallet made on master can be opened on:
[17:21:40.700] 2025-04-02T21:21:33.637000Z TestFramework (INFO): - 250000
[17:21:40.700] 2025-04-02T21:21:34.055000Z TestFramework (INFO): - 240001
[17:21:40.700] 2025-04-02T21:21:34.435000Z TestFramework (INFO): - 230000
[17:21:40.700] 2025-04-02T21:21:34.858000Z TestFramework (INFO): - 220000
[17:21:40.700] 2025-04-02T21:21:35.614000Z TestFramework (INFO): - 210000
[17:21:40.700] 2025-04-02T21:21:35.707000Z TestFramework (INFO): Test descriptor wallet incompatibility on:
[17:21:40.700] 2025-04-02T21:21:35.707000Z TestFramework (INFO): - 200100
[17:21:40.700] 2025-04-02T21:21:35.878000Z TestFramework (INFO): - 190100
[17:21:40.700] 2025-04-02T21:21:36.021000Z TestFramework (INFO): - 180100
[17:21:40.700] 2025-04-02T21:21:36.319000Z TestFramework (INFO): Test descriptor wallet incompatibility with 0.17
[17:21:40.700] 2025-04-02T21:21:37.328000Z TestFramework (INFO): Test that 0.21 cannot open wallet containing tr() descriptors
[17:21:40.700] 2025-04-02T21:21:37.356000Z TestFramework (INFO): Test that a wallet can upgrade to and downgrade from master, from:
[17:21:40.700] 2025-04-02T21:21:37.361000Z TestFramework (INFO): - 250000
[17:21:40.700] 2025-04-02T21:21:37.665000Z TestFramework (INFO): - 240001
[17:21:40.700] 2025-04-02T21:21:37.970000Z TestFramework (INFO): - 230000
[17:21:40.700] 2025-04-02T21:21:38.439000Z TestFramework (INFO): - 220000
[17:21:40.700] 2025-04-02T21:21:38.793000Z TestFramework (INFO): - 210000
[17:21:40.700] 2025-04-02T21:21:39.470000Z TestFramework (INFO): Stopping nodes
[17:21:40.700]
[17:21:40.700]
[17:21:40.700] [1mstderr:
[17:21:40.700] [0mTraceback (most recent call last):
[17:21:40.700] File "/ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu/test/functional/wallet_backwards_compatibility.py", line 389, in <module>
[17:21:40.700] BackwardsCompatibilityTest(__file__).main()
[17:21:40.700] File "/ci_container_base/test/functional/test_framework/test_framework.py", line 206, in main
[17:21:40.700] exit_code = self.shutdown()
[17:21:40.700] File "/ci_container_base/test/functional/test_framework/test_framework.py", line 379, in shutdown
[17:21:40.700] self.stop_nodes()
[17:21:40.700] File "/ci_container_base/test/functional/test_framework/test_framework.py", line 643, in stop_nodes
[17:21:40.700] node.stop_node(wait=wait, wait_until_stopped=False)
[17:21:40.700] File "/ci_container_base/test/functional/test_framework/test_node.py", line 397, in stop_node
[17:21:40.700] self.stop()
[17:21:40.700] File "/ci_container_base/test/functional/test_framework/coverage.py", line 50, in __call__
[17:21:40.700] return_val = self.auth_service_proxy_instance.__call__(*args, **kwargs)
[17:21:40.700] File "/ci_container_base/test/functional/test_framework/authproxy.py", line 132, in __call__
[17:21:40.700] response, status = self._request('POST', self.__url.path, postdata.encode('utf-8'))
[17:21:40.700] File "/ci_container_base/test/functional/test_framework/authproxy.py", line 106, in _request
[17:21:40.700] return self._get_response()
[17:21:40.700] File "/ci_container_base/test/functional/test_framework/authproxy.py", line 169, in _get_response
[17:21:40.700] http_response = self.__conn.getresponse()
[17:21:40.700] File "/usr/lib/python3.10/http/client.py", line 1375, in getresponse
[17:21:40.700] response.begin()
[17:21:40.700] File "/usr/lib/python3.10/http/client.py", line 318, in begin
[17:21:40.700] version, status, reason = self._read_status()
[17:21:40.700] File "/usr/lib/python3.10/http/client.py", line 287, in _read_status
[17:21:40.700] raise RemoteDisconnected("Remote end closed connection without"
[17:21:40.700] http.client.RemoteDisconnected: Remote end closed connection without response
[17:21:40.700] [node 10] Cleaning up leftover process
[17:21:40.700] [node 9] Cleaning up leftover process
[17:21:40.700] [node 8] Cleaning up leftover process
[17:21:40.700] [node 7] Cleaning up leftover process
[17:21:40.700] [node 6] Cleaning up leftover process
[17:21:40.700] [node 5] Cleaning up leftover process
[17:21:40.700] [node 4] Cleaning up leftover process
[17:21:40.700] [node 3] Cleaning up leftover process
[17:21:40.700] [node 2] Cleaning up leftover process
[17:21:40.700] [node 1] Cleaning up leftover process
[17:21:40.700] [node 0] Cleaning up leftover process
ACKs for top commit:
laanwj:
Code review ACK fac978fb213fbd6668194d8b9810ddd10f8a7323
janb84:
Re ACK [fac978f](fac978fb21)
pablomartin4btc:
re ACK fac978fb213fbd6668194d8b9810ddd10f8a7323
BrandonOdiwuor:
Code Review ACK fac978fb213fbd6668194d8b9810ddd10f8a7323
Tree-SHA512: 13acdfc6be4293a0ff45ae20b26ba60636e130097da380b7b51716faaa950320462399bef55e74b3cedc82944586dcc1bfd078babb96edb03c4efdb8f40af5a4
b639417b39cced5223953fca65cdcd5d84475127 net: Add Tor extended SOCKS5 error codes (laanwj)
Pull request description:
Add support for reporting Tor extended SOCKS5 error codes as defined here:
- https://spec.torproject.org/socks-extensions.html#extended-error-codes
- https://gitlab.torproject.org/tpo/core/arti/-/blob/main/crates/tor-socksproto/src/msg.rs?ref_type=heads#L183
These give a more direct indication of the problem in case of errors connecting to hidden services, for example:
```
2025-04-02T10:34:13Z [net] Socks5() connect to [elided].onion:8333 failed: onion service descriptor can not be found
```
In the C Tor implementation, to get these one should set the "ExtendedErrors" flag on the "SocksPort" definition, introduced in version 0.4.3.1.
In Arti, extended error codes are always enabled.
Also, report the raw error code in case of unknown reply values.
ACKs for top commit:
1440000bytes:
utACK b639417b39
w0xlt:
utACK b639417b39
pablomartin4btc:
utACK b639417b39cced5223953fca65cdcd5d84475127
Tree-SHA512: b30e65cb0f5c9183701373b0ee64cdec40680a3de1a1a365b006538c4d0b7ca8a047d7c6f81a7f5b8a36bae3a20b47a4c2a9850423c7034866e3837fa8fdbfe2
e419b0e17f8acfe577c35c62a8a71a19aad249f3 refactor: Remove manual CDBBatch size estimation (Lőrinc)
8b5e19d8b5bf59ae7e178fd7c27ab724d1a433be refactor: Delegate to LevelDB for CDBBatch size estimation (Lőrinc)
751077c6e25e010cff85fe9793a8c5b843350f98 Coins: Add `kHeader` to `CDBBatch::size_estimate` (Lőrinc)
Pull request description:
### Summary
The manual batch size estimation of `CDBBatch` serialized size was [added](e66dbde6d1) when LevelDB [didn't expose this functionality yet](https://github.com/google/leveldb/commit/69e2bd2).
The PR refactors the logic to use the native `leveldb::WriteBatch::ApproximateSize()` function, structured in 3 focused commits to incrementally replace the old behavior safely.
### Context
The previous manual size calculation initialized the estimate to 0, instead of LevelDB's header size (containing an 8-byte sequence number followed by a 4-byte count).
This PR corrects that and transitions to the now-available native LevelDB function for improved accuracy and maintainability.
### Approach
The fix and refactor follow a strangle pattern over three commits:
* correct the initialization bug in the existing manual calculation, isolating the fix and ensuring the subsequent assertions use the corrected logic;
* introduce the native `ApproximateSize()` method alongside the corrected manual one, adding assertions to verify their equivalence at runtime;
* remove the verified manual calculation logic and assertions, leaving only the native method.
ACKs for top commit:
sipa:
utACK e419b0e17f8acfe577c35c62a8a71a19aad249f3
TheCharlatan:
ACK e419b0e17f8acfe577c35c62a8a71a19aad249f3
laanwj:
Code review ACK e419b0e17f8acfe577c35c62a8a71a19aad249f3
Tree-SHA512: a12b973dd480d4ffec4ec89a119bf0b6f73bde4e634329d6e4cc3454b867f2faf3742b78ec4a3b6d98ac4fb28fb2174f44ede42d6c701ed871987a7274560691
459807d566cfddad12b2953a506fd07a129145d8 test: remove strict restrictions on rpc_deprecated (Pol Espinasa)
Pull request description:
Removed the wallet restrictions for `rpc_deprecated.py` and added specific test case for the current deprecated rpc.
`skip_test_if_missing_module` will skip the whole test when the wallet is missing, even if a part of the test is non-wallet related. This PR ensures that other tests not related to wallet can be ran and only this specific test will be skipped if there's no wallet
For more context check https://github.com/bitcoin/bitcoin/pull/31278#discussion_r2011661090
ACKs for top commit:
maflcko:
lgtm ACK 459807d566cfddad12b2953a506fd07a129145d8
rkrux:
ACK 459807d
Tree-SHA512: 922b0fafe8fb5bd88a677ce8be5c3fe2fdd4d0aadcd32cc11738a714cd6f765f07e7e7158c829f8338db0d46a15c030437a1ea09a3187c072bebebb4ca53ad85
f974359e218edc3f31685015e234d00243a79452 test: Add encodable PUSHDATA1 examples to feature_taproot (Greg Sanders)
Pull request description:
Inspired by discussion in https://github.com/bitcoin/bitcoin/pull/31640#issuecomment-2743492906 I made an example adding coverage I think is missing, with some extra commentary that might help future contributors (including myself when I forget how it works again).
Open for suggestions how we can make it more welcoming beyond this.
cc darosior EthanHeilman sipa
ACKs for top commit:
janb84:
Re-ACK [f974359](f974359e21)
rkrux:
ACK f974359e218edc3f31685015e234d00243a79452
Tree-SHA512: 7544d41c39c13d245a8a33522e53f22b4dd7593c069631978303e5a349cd12cf9d45bed648c391618c4732831232c4b82b8de2bf6cba5bf5e1232501db926122
58914ab459c46c518c47c5082aec25ac0d03ab11 fuzz: assert min diff between FeeFrac and CFeeRate (Pieter Wuille)
0c6bcfd8f73bfd8524c01b302dc4a27665abf5c3 feefrac: support both rounding up and down for Evaluate (Pieter Wuille)
ecf956ec9d3badeb940f85588003aac4c6d2190b feefrac: add support for evaluating at given size (Pieter Wuille)
7963aecead968d126545d3730da5d9942c3f9518 feefrac: add helper functions for 96-bit division (Pieter Wuille)
800c0dea9af773b77b89233528efe265fe154db1 feefrac: rework comments around Mul/MulFallback (Pieter Wuille)
fcfe008db25ef14fdb4dc0e1620bd03c0065b840 feefrac fuzz: use arith_uint256 instead of ad-hoc multiply (Pieter Wuille)
46ff4220bff01944f436e1c405d038082b2c87af arith_uint256: modernize comparison operators (Pieter Wuille)
Pull request description:
The `FeeFrac` type represents a fraction, intended to be used for sats/vbyte or sats/WU. This PR adds functionality to evaluate that feerate for a given size, in order to obtain the fee it corresponds with (rounding down, or rounding up).
The motivation here is being able to do accurate feerate evaluations in cluster mempool block building heuristics (where rounding down is needed), but in principle this makes it possible to use `FeeFrac` as a more accurate replacement for `CFeeRate` (where for feerate estimation rounding up is desirable). Because of this, both rounding modes are implemented.
Unit tests are included for known-correct values, plus a fuzz test that verifies the result using `arith_uint256`.
ACKs for top commit:
l0rinc:
ACK 58914ab459c46c518c47c5082aec25ac0d03ab11
ismaelsadeeq:
reACK 58914ab459c46c518c47c5082aec25ac0d03ab11
glozow:
light code review ACK 58914ab459c46c518c47c5082aec25ac0d03ab11
Tree-SHA512: 362b88454bf355cae1f12d6430b1bb9ab66824140e12b27db7c48385f1e8db936da7d0694fb5aad2a00eb9e5fe3083a3a2c0cc40b2a68e2d37e07b3481d4eeae
Rather than use an ad-hoc reimplementation of wide multiplication inside the
fuzz test, reuse arith_uint256, which already has this. It's larger than what we
need here, but performance isn't a concern in this test, and it does what we need.
Since C++20, operator!= is implicitly defaulted using operator==, and
operator<, operator<=, operator>, and operator>= are defaulted using
operator<=>, so it suffices to just provide these two.
Remove the manual batch size estimation logic (`SizeEstimate()` method and `size_estimate` member) from `CDBBatch`.
Size is now determined solely by the `ApproximateSize()` method introduced in the previous commit, which delegates to the native LevelDB function.
The manual calculation is no longer necessary as LevelDB now provides this functionality directly, and the previous commit verified that the native function's results matched the manual estimation.
Assertions comparing the two methods are removed from `txdb.cpp`.
Co-authored-by: Wladimir J. van der Laan <laanwj@protonmail.com>
Serialized batch size can be queried via the underlying LevelDB implementation calling the native `leveldb::WriteBatch::ApproximateSize()`.
The previous manual calculation was added in e66dbde6d1 as part of https://github.com/bitcoin/bitcoin/pull/10195. At that time (April 2017), the version of LevelDB used by Bitcoin Core (and even the latest source) lacked a native function for this. LevelDB added this capability in 69e2bd224b, merged later that year.
The old manual estimation method (`SizeEstimate()`) is kept temporarily in this commit, and assertions are added in `txdb.cpp` to verify its results against `ApproximateSize()` during batch writes. This ensures the native function behaves as expected before removing the manual calculation in the subsequent commit.
The initialization of the manual `size_estimate` in `CDBBatch::Clear()` is corrected from `0` to `kHeader` (LevelDB's fixed batch header size).
This aligns the manual estimate with LevelDB's actual size immediately after clearing, fixing discrepancies that would otherwise be caught by tests in the next commit (e.g., `coins_tests`, `validation_chainstatemanager_tests`).
fadf8f078e80fcc3a08f7f6f3291b43ae2bafeea test: Remove confusing and failing system time test (MarcoFalke)
Pull request description:
This was just added as a sanity check in fa013664ae23d0682a195b9bded85bc19c99536e by myself.
However, the test uses system time, so it may obviously (albeit rarely) fail.
Fix it by removing it.
Can be tested by running two bash loops at the same time:
`while ( ./bld-cmake/bin/test_bitcoin -t util_tests/util_time_GetTime ) ; do true ; done`
`while ( date -s "$(date -d 'now + 0.015 seconds' '+%Y-%m-%d %H:%M:%S.%3N')" && date -s "$(date -d 'now - 0.015 seconds' '+%Y-%m-%d %H:%M:%S.%3N')" ) ; do true ; done`
Eventually, it will fail:
```
test/util_tests.cpp(595): error: in "util_tests/util_time_GetTime": check ms_0 < GetTime<std::chrono::milliseconds>() has failed
test/util_tests.cpp(596): error: in "util_tests/util_time_GetTime": check us_0 < GetTime<std::chrono::microseconds>() has failed
*** 2 failures are detected in the test module "Bitcoin Core Test Suite"
ACKs for top commit:
janb84:
ACK [fadf8f0](fadf8f078e)
mabu44:
Tested ACK fadf8f078e80fcc3a08f7f6f3291b43ae2bafeea
hebasto:
ACK fadf8f078e80fcc3a08f7f6f3291b43ae2bafeea, tested on Ubuntu 24.10.
Tree-SHA512: fc468546f46a12804802df4f0e64d2898aca3db4df69602e5919ac31646c2fcb1e75b614fc2d1a3959c3db10fb0e315da5886d348b41589dba7cb43e618444a1
fa10a1ded5b747e9db6d6c1942fceb279f1abedc ci: Use GITHUB_BASE_REF over hard-coded master (MarcoFalke)
fa0d0be05c0e012e29a4640c3066c81066cd6d2e ci: Merge master in test-each-commit task (take 2) (MarcoFalke)
Pull request description:
Calling the script `.github/ci-test-each-commit-exec.sh`, which merges `master`, obviously doesn't work, if the script itself is missing.
Fix it by a move-only to first merge `master` and then call the script.
ACKs for top commit:
l0rinc:
Code review ACK fa10a1ded5b747e9db6d6c1942fceb279f1abedc
sipa:
ACK fa10a1ded5b747e9db6d6c1942fceb279f1abedc, this fixed the CI issue in #31444.
Tree-SHA512: bcab2b03cb46d456e29f8d4237312a4525b9acd819578b26b4d5670ca14e075cf473b77b235b3063e06422325b627587f12dec7b4fbba134086d162c67dc81b3
* Run git config earlier and only once
* Run git merge in the yaml, before calling the bash script
* Run git reset in the yaml as well, for symmetry
* Replace "git merge --abort" with "git reset --hard", because it does
not fail when already up to date and no merge was started.
c5a7ffd1e8ccae12e034face4293bc8d5a6556b7 preserve llvm profile env (Prabhat Verma)
Pull request description:
While generating `profraw` for fuzz tests using steps in [PR 32206](https://github.com/bitcoin/bitcoin/pull/32206) , the profraw was not being built at the desired location and only one `default.profraw` was being created which was being overwritten for multiple fuzz targets. This PR fixes that.
ACKs for top commit:
maflcko:
lgtm ACK c5a7ffd1e8ccae12e034face4293bc8d5a6556b7
mabu44:
ACK c5a7ffd1e8ccae12e034face4293bc8d5a6556b7
Tree-SHA512: 11f74caa8cba6f841aa899a5e294f658aed1b6a3d4cf68992609ea99fadb4a092b2350ffacea5c2d5eb377eb10082de018f27a1d6486a72460cb3905aaa15664
faa807bdf8c3002a28005b4765604f518a6f2736 ci: Merge master in test-each-commit task (MarcoFalke)
Pull request description:
The `test-each-commit` task will often fail, when the CI config yaml is updated along with code changes.
This is because, GitHub seems to be merging the CI config on a fresh pull with the current target branch (`master`). However, the code changes are not.
A tedious workaround would be for every developer to rebase on every intermittent (https://github.com/bitcoin/bitcoin/issues/31946#issuecomment-2740911853) and non-intermittent CI issue.
However, fix this instead by merging with `master`.
ACKs for top commit:
laanwj:
ACK faa807bdf8c3002a28005b4765604f518a6f2736
hebasto:
ACK faa807bdf8c3002a28005b4765604f518a6f2736.
Tree-SHA512: 4849bd558dc6cdc7d86b95164ccee32ab7c08c9b7d31cf8ec5c8e9a2251fc819630f8fa9b929ed39e8e033c67bb006f0beb33e0de216e1224680be88c5fa0161
There turned out to be a mismatch in the tx output counts which caused
'ConnectBlockMixedEcdsaSchnorr' benchmark to run slower than
'ConnectBlockAllEcdsa' and 'ConnectBlockAllSchnorr'. This commit makes
the tx output counts uniform across all benchmarks.
This commit also renames the 'taproot_tx' variable to 'tx' to reflect
that this variable represents a general tx and not just a taproot tx.
The new helper will be used to fix a crash in the
wallet migration process (watch-only, non-blank,
private keys disabled, empty wallet - no scripts
or addresses imported).
Co-authored-by: Matias Furszyfer <mfurszy@protonmail.com>
2e751f559ac8c76655c14ce3825e1b0bdf81da98 doc: Amend Qt 6 dependency packages for Ubuntu (Hennadii Stepanov)
Pull request description:
On older systems, such as Ubuntu 22.04, `qt6-tools-dev-tools` and `libgl-dev` are not treated as dependencies of `qt6-tools-dev` and `qt6-base-dev`, respectively. This PR explicitly lists them in the installation documentation.
Fixes https://github.com/bitcoin/bitcoin/issues/32210.
ACKs for top commit:
maflcko:
lgtm ACK 2e751f559ac8c76655c14ce3825e1b0bdf81da98
laanwj:
Code review ACK 2e751f559ac8c76655c14ce3825e1b0bdf81da98
Tree-SHA512: a6997c74c83789cb5fe5b97a719b8ff6e2180d5f6ae5502ccccfce3a22394d25eef05204ecda0a6deb368de77975e2a1da89b5749eff01a979f2f60843efebff
Add a class TorsStreamIsolationCredentialsGenerator that generates
unique credentials based on a randomly generated session prefix
and an atomic counter.
This makes sure that different launches of the application won't share
the same credentials, and thus circuits, even in edge cases.
Example with `-debug=proxy`:
```
2025-03-31T16:30:27Z [proxy] SOCKS5 sending proxy authentication 0afb2da441f5c105-0:0afb2da441f5c105-0
2025-03-31T16:30:31Z [proxy] SOCKS5 sending proxy authentication 0afb2da441f5c105-1:0afb2da441f5c105-1
```
Thanks to hodlinator for the idea.
This change updates the vcpkg manifest baseline from the "2024.09.30
Release" to the "2025.03.19 Release", with the following package
changes:
- boost: 1.85.0#1,2 --> 1.87.0
- qtbase: 6.7.2#3 -> 6.8.2#1
- qttools: 6.7.2#1 -> 6.8.2
- sqlite3: 3.46.1 --> 3.49.1
On older systems, such as Ubuntu 22.04, `qt6-tools-dev-tools` and
`libgl-dev` are not treated as dependencies of `qt6-tools-dev` and
`qt6-base-dev`, respectively. This change explicitly lists them in the
installation documentation.
a40bd374aaf8e10116fa664fa7b480d85ee2fe59 Get*Union: disallow nulltpr Refs (Greg Sanders)
57433502e6756b0dea7332026f3abf926daf0e35 CountDistinctClusters: nullptrs disallowed (Greg Sanders)
8bca0d325a4ffa13a171eaed83096003aa28520e TxGraphImpl::Compact: m_main_clusterset.m_removed is always empty (Greg Sanders)
2c5cf987e96a3e8f783d370a8bc8914f5a4a9682 TxGraphImpl::PullIn: only allowed when staging exists (Greg Sanders)
Pull request description:
Was looking at my local coverage report, and noticed a few spots that will not or cannot be hit.
CountDistinctClusters, GetAncestorsUnion, and GetDescendantsUnion accept nullptrs, but the test harness never employs them. Disallow them.
We never call PullIn whenever there isn't staging, so just enforce that invariant via assertion.
Remaining places that are not covered:
1) Relinearize: Currently we seem to always start with a cold (not known to be optimal) cluster, and after one attempt at linearization result into something optimal. This means we never shortcircuit, nor run PostLinearization, nor store the quality as ACCEPTABLE. Reducing iterations causes these lines to be hit. sipa says he will take this on as varying the amount of iterations was meant to be done eventually anyways.
2) We never do a move assignment operator when the lvalue already has a `m_graph` (so we never call UnlinkRef) 3358b1d105/src/txgraph.cpp (L2097)
3) We never use the move constructor: 3358b1d105/src/txgraph.cpp (L2108)
ACKs for top commit:
sipa:
utACK a40bd374aaf8e10116fa664fa7b480d85ee2fe59
glozow:
utACK a40bd374aaf8e10116fa664fa7b480d85ee2fe59
Tree-SHA512: ca88297222e80e0d590889698899f892b9335cfa587a76a6c6ca62c8d846f208b6b0b9a9b1829bafabdb929a1a0c3a75f23edf7dd2b4f5e2dad0235e5bc68ba3
f00345727b8d2bf73409db5cd342e476671e6425 doc: Update `dependencies.md` for Qt 6 (Hennadii Stepanov)
80b917991ed7ff931f0a9211cebf859f674776c4 build, msvc: Update `vcpkg.json` for Qt 6 (Hennadii Stepanov)
30dd1f1644e0441b5310f1eceecfd6a5abc45f68 ci: Update for Qt 6 (Hennadii Stepanov)
629d292f4d846978c682c5f497240c62d62f4bd1 test: Update sanitizer suppressions for Qt 6 (Hennadii Stepanov)
551e13abf82522bad7fdde4ff4bd15d2c8f88b23 guix: Adjust for Qt 6 (Hennadii Stepanov)
c3e9bd086c498e9f1cbdc505949cb17ac1b39f7e qt: Fix compiling for Windows (Hennadii Stepanov)
ab399c4db2e98aee8e01323c4c6cca48b520dc7f depends: Add `native_qt` package (Hennadii Stepanov)
248613eb3ee034bf143821a51635e697dc114e6c depends: Factor out Qt modules' details (Hennadii Stepanov)
0268f52a4cd2b7aa63934526437bcf6912e47d3c depends: Introduce customizable `$(package)_patches_path` variables (Hennadii Stepanov)
5e794e62024eef612e1fbb71c76ea54d17435c14 depends: Bump `qt` package up to 6.7.3 (Hennadii Stepanov)
6d4214925fadc36d26aa58903db5788c742e68c6 cmake: Require Qt 6 to build GUI (Hennadii Stepanov)
Pull request description:
The currently used Qt 5.15 is approaching [EOL](https://www.qt.io/blog/qt-5.15-extended-support-for-subscription-license-holders) and will reach it before the Bitcoin Core v30 release. The recent migration of the build system to CMake makes it possible to switch to Qt 6.
This PR updates the OS runtime compatibility requirements for the Bitcoin Core GUI as follows:
### 1. Linux
Starting with Qt 6.5.0, the `libxcb-cursor0` package is required to be installed at runtime.
### 2. Windows
Cross-compiling does not support LTO. We have to re-add it in a follow-up.
A new style plugin causes minor visual glitches, such as
which will be fixed in follow-ups.
### 3. macOS
`bitcoin-qt` now uses the [Metal](https://developer.apple.com/metal/) backend.
---
**IMPORTANT.** Don't forget to install [Ninja](https://ninja-build.org/).
---
For historical context, please refer to:
- https://github.com/bitcoin/bitcoin/issues/20627
- https://github.com/bitcoin/bitcoin/pull/24798
---
UPD 2024-10-09. Qt 6.8 has been [released](https://www.qt.io/blog/qt-6.8-released), but it has some [drawbacks](https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2402990346) for us. As a result, this PR will stick to Qt 6.7.
UPD 2025-03-18: [Standard support for Qt 5.15 will end after 26th of May 2025](https://www.qt.io/blog/extended-security-maintenance-for-qt-5.15-begins-may-2025)
ACKs for top commit:
laanwj:
re-ACK f00345727b8d2bf73409db5cd342e476671e6425
hodlinator:
re-ACK f00345727b8d2bf73409db5cd342e476671e6425
Tree-SHA512: 367f722e6c3ea4700b5395871c40b6df8c8062fdc822107090449ea4ae4ad2db75cc53a982a678f4c48ce8f9b2d43ed10e6d23b06165ab78713f161db712d895
With newly introduced libmultiprocess subtree, there's no need for depends
system to download and track changes to the upstream repository.
Note that adding the libmultiprocess subtree does not allow dropping
libmultiprocess packages from the depends build, because libmultiprocess
includes a code generation tool called mpgen, and in cross-compiled builds,
bitcoin core's cmake build system doesn't have access to a native toolchain and
can't build mpgen itself, so the depends system (or the native environment if
not using depends) needs to supply it.
Move parts of the int_get_build_id into a new int_get_build_properties
function. There is no change in behavior. This just organizes assignments
better so some build properties can be used to help compute build ids in the
next commit.
Without this change linter produces errors about:
- Use of std::filesystem the libmultiprocess example program.
- Use of locale-dependent functions in example program, in the build time code
generator, and in the runtime library for debug logging.
- Include guards not beginning with BITCOIN_
