4ae9a10ada95ab8c1ab01472948d348d9538f3bb doc: add release notes for dbcache bump (Andrew Toth)
c510d126efb6ce9623e0e005829c13a110f65b0e doc: update dbcache default in reduce-memory.md (Andrew Toth)
027cac852796f643e2b6036fd3652dd3ca7785a5 qt: show GetDefaultDBCache() in settings (Andrew Toth)
5b34f251847a3c66b444959800b70e9863b10bbe dbcache: bump default from 450MB -> 1024MB if enough memory (Andrew Toth)
Pull request description:
Alternative to #34641
This increases the default `dbcache` value from `450MiB` to `1024MiB` if:
- `dbcache` is unset
- The system is 64 bit
- At least 4GiB of RAM is detected
Otherwise fallback to previous `450MiB` default.
This should be simple enough to get into v31. The bump to 1GiB shows significant performance increases in #34641. It also alleviates concerns of too high default for steady state, and of lowering the current dbcache for systems with less RAM.
This change only changes bitcoind behavior, while kernel still defaults to 450 MiB.
ACKs for top commit:
ajtowns:
ACK 4ae9a10ada95ab8c1ab01472948d348d9538f3bb
kevkevinpal:
reACK [4ae9a10](4ae9a10ada)
svanstaa:
ACK [4ae9a10](4ae9a10ada)
achow101:
ACK 4ae9a10ada95ab8c1ab01472948d348d9538f3bb
sipa:
ACK 4ae9a10ada95ab8c1ab01472948d348d9538f3bb
Tree-SHA512: ee3acf1fb08523ac80e37ec8f0caca226ffde6667f3a75ae6f4f4f54bc905a883ebcf1bf0e8a8a15c7cfabff96c23225825b3fff4506b9ab9936bf2c0a2c2513
20ae9b98eab20117344cf31f7cde39cadd70ca22 Extend functional test for setBlockIndexCandidates UB (marcofleon)
854a6d5a9a0e40329a2852efb2a8dfec4b54886e validation: fix UB in LoadChainTip (marcofleon)
9249e6089ec4e2eb63f0896961f04d9dbe14651a validation: remove LoadChainTip call from ActivateSnapshot (marcofleon)
Pull request description:
Addresses https://github.com/bitcoin/bitcoin/issues/34503. See this issue for more details as well.
Fixes a bug where, under certain conditions, `setBlockIndexCandidates` had blocks in it that were worse than the tip. The block index candidate set uses `nSequenceId` as a sort key, so modifying this field while blocks are in the set results in undefined behavior. This PR populates `setBlockIndexCandidates` after the `nSequenceId` modifications, avoiding the UB.
ACKs for top commit:
achow101:
ACK 20ae9b98eab20117344cf31f7cde39cadd70ca22
sedited:
Re-ACK 20ae9b98eab20117344cf31f7cde39cadd70ca22
sipa:
Code review ACK 20ae9b98eab20117344cf31f7cde39cadd70ca22
Tree-SHA512: 121c170bb70fb6365089d578db63c811e7926e129d7206e569947f7a1f6c5ddc8d5f4937b80f1ba1b7d7daa42789b143ca5b56f154b7ab968a1cd55f925f378d
97e7e79435c69e90cb7f056c704c275421bf0892 test: Enable `system_tests/run_command` "stdin" test on Windows (Hennadii Stepanov)
a4324ce09546d80ab847dbfce715f015139ed593 test: Remove `system_tests/run_command` runtime dependencies (Hennadii Stepanov)
Pull request description:
`system_tests` currently rely on `cat`, `echo`, `false` and `sh` being available in `PATH` at runtime.
This PR:
1. Removes these dependencies.
2. Reduces the number of platform-specific code paths.
The change is primarily motivated by my work on maintaining the [`bitcoin-core`](https://packages.guix.gnu.org/packages/bitcoin-core) package in Guix. It enables the removal of the existing `bash` and `coreutils` native inputs, which in turn makes it possible to drop the implicit dependency on `qtbase@5` (see https://codeberg.org/guix/guix/pulls/4386#issuecomment-8613333).
ACKs for top commit:
maflcko:
re-ACK 97e7e79435c69e90cb7f056c704c275421bf0892 👓
janb84:
ACK 97e7e79435c69e90cb7f056c704c275421bf0892
sedited:
ACK 97e7e79435c69e90cb7f056c704c275421bf0892
Tree-SHA512: 1375c676f85c75d571df1ddfc3a4405767dbf0ed7bfea2927c93ec01b29f9f7ae3383e546d2658f595e8ffafa9ab20bba6fcc628a9f5ebdb288bbef03b645fb6
0a6724aaae973659b6c44eb5a062aaaaf57be870 doc: Update Windows build notes (Hennadii Stepanov)
473e5f8efcdabc584b99f72c364cf15afad1ca97 qt: Add patch to fix SFINAE warnings in QAnyStringView with gcc16 (Hennadii Stepanov)
3cb4d6066b83348411c43587605b624f067dbbf3 qt: add patches to fix SFINAE errors/warnings with gcc16 (Cory Fields)
d7e972a90d59bbf705bd06f5d2606cccd1dcdcf9 qt: add patch to fix build with gcc16 (Cory Fields)
19693a8c914174692e01d65434b386b522cebde7 depends: Update Qt to 6.8.3 (Hennadii Stepanov)
c55584575a99ff565131207916eccb3223e508a3 cmake: Fix `FindQt` module (Hennadii Stepanov)
Pull request description:
This PR updates the `qt` package in depends to the latest open-source [6.8.3](https://www.qt.io/blog/qt-6.8.3-released) LTS release.
The update includes numerous bugfixes, which allows us to drop `qtbase_plugins_windows11style.patch`.
Additionally, it includes [patches](https://github.com/bitcoin/bitcoin/issues/34569#issuecomment-3892793262) for compatibility with GCC 16 (along with one extra patch), and incorporates a [commit](8f1b55d1d5) from https://github.com/bitcoin/bitcoin/pull/32709.
Closes https://github.com/bitcoin/bitcoin/issues/34569.
ACKs for top commit:
achow101:
ACK 0a6724aaae973659b6c44eb5a062aaaaf57be870
sedited:
ACK 0a6724aaae973659b6c44eb5a062aaaaf57be870
Tree-SHA512: b66fe6f75bae00fb5c525c5fad56d39273f53f6bfd58206da8a55c6e41d14533137c72fb03e9537ba3a3d0b3463b6dcbef6a88ac2f4559fa6e9abf045fe1beaa
cbdb891de23d98dae3cfb3c4d993d6111df0aaa4 test: Wait for txospender index to be synced in rpc_gettxspendingprevout (Ava Chow)
2db5c049bb3cf9a323c100ce9d4957a937c01624 test: Sync mempools after tx creation in rpc_gettxspendingprevout (Ava Chow)
Pull request description:
On slower runs, the txospender index may not be synced yet when the tests of its behavior begin, causing intermittent failures. Wait for them to be synced before starting the tests.
The tests also query mempool txs from other nodes, make sure mempools are synced before doing so.
The first commit with the following diff reproduces #34735:
```
diff --git a/src/index/txospenderindex.cpp b/src/index/txospenderindex.cpp
index d451bb1e0a4..e786f05a98c 100644
--- a/src/index/txospenderindex.cpp
+++ b/src/index/txospenderindex.cpp
@@ -129,6 +129,7 @@ static std::vector<std::pair<COutPoint, CDiskTxPos>> BuildSpenderPositions(const
bool TxoSpenderIndex::CustomAppend(const interfaces::BlockInfo& block)
{
+ UninterruptibleSleep(100ms);
WriteSpenderInfos(BuildSpenderPositions(block));
return true;
}
```
Fixes#34735
ACKs for top commit:
furszy:
ACK cbdb891de23d98dae3cfb3c4d993d6111df0aaa4
andrewtoth:
ACK cbdb891de23d98dae3cfb3c4d993d6111df0aaa4
sedited:
ACK cbdb891de23d98dae3cfb3c4d993d6111df0aaa4
rkrux:
crACK cbdb891de23d98dae3cfb3c4d993d6111df0aaa4
Tree-SHA512: ba1ab6216ac3b647a5f9e20f4899e51e643bf20829e698aa63c127c88ed4e33afa222bebeae1210fc869c46288e99c0b344330197913e244ffe1a6aca943568d
15c4889497b96037e41019a8f43090af841b36ec index: document TxoSpenderIndex::FindSpender (furszy)
f8b9595aaa966c373b02e6227dc799fed6d038ba test: Add missing txospenderindex coverage in feature_init (Fabian Jahr)
a1074d852a7a46b746fb4ed90d94cb4cc346f9b3 index, rpc, test: Misc formatting fixes (Fabian Jahr)
Pull request description:
This addresses my own comments in the last review of #24539: https://github.com/bitcoin/bitcoin/pull/24539#pullrequestreview-3829110465
The first commit fixes three small formatting errors.
The second commit adds some missing coverage in `feature_init` and refactors the code a bit as well so these misses don't happen so easily in the future.
The third commit is by furzy:
> TxoSpenderIndex::FindSpender returns an Expected<optional<TxoSpender>> but
the two levels of the return type were undocumented, making it unclear what a returned
nullopt means. So added doc clarifying each return case.
ACKs for top commit:
furszy:
ACK 15c4889497b96037e41019a8f43090af841b36ec
sedited:
ACK 15c4889497b96037e41019a8f43090af841b36ec
rkrux:
crACK 15c4889497b96037e41019a8f43090af841b36ec
Tree-SHA512: 2e0f060a54b558d2967ebae0835cf81bd86c2d8d983d670a48d1bee7d347f186623e75db7ae311ca1566807f715c1b3fa67cf734c9467d35e13b84d082f28253
37d49f5de6221aab90f99671413836479e6a71d1 doc: mention Miniscript expressions inside reference (Pieter Wuille)
771f7642bc03c829a7649016c69b0ebde688a4f1 doc: fix typo in descriptors.md (Pieter Wuille)
708b84999b5cc1ba760c7dcc4aa5252b8d136c06 doc: reference descriptor BIPs in descriptors.md (Pieter Wuille)
8f2a869a19ac6c642443a0e31348f2b34e26cbfa doc: do not list descriptor RPCs or history (Pieter Wuille)
65a8b6c2ef52244f21638004ffdacf6a2ed2c097 doc: mention musig() in descriptors.md (Pieter Wuille)
Pull request description:
This brings doc/descriptors.md up to date:
* Stop trying to exhaustively list all RPCs that involve descriptors. They're used everywhere.
* Stop trying to give the history of descriptor support, we have release notes for that.
* Mention that wallets are now built around descriptors (especially with legacy wallets gone).
* Mention `musig()` KEY expressions in the specification part.
* Mention the miniscript expressions in the specification part.
* Reference the relevant output descriptor BIPs in the text.
ACKs for top commit:
achow101:
ACK 37d49f5de6221aab90f99671413836479e6a71d1
darosior:
utACK 37d49f5de6221aab90f99671413836479e6a71d1
Tree-SHA512: 2581be9b5d1a7099806d6f830b3a452505f8493d0e493a80b8a50e383f93f3e2c8a2d72a64fdae0adfe63d3c2eeb54a61a059108cd861e58c3d85f2bc576364b
2a7a4f608a42acef5cdf7370f0d191119fdb94c9 depends: Allow building Qt packages after interruption (Hennadii Stepanov)
Pull request description:
If a build is interrupted, a standard `mkdir` command may fail if the directory already exists. Switching to `mkdir -p` ensures the build can resume gracefully.
Fix#34712.
ACKs for top commit:
sedited:
ACK 2a7a4f608a42acef5cdf7370f0d191119fdb94c9
Tree-SHA512: fad2dce89a7cb68a8a539924d98698fe650802d19c84f216fa65e3c23c1a33ab6acf9f4c98c27381194c2958efa92e9dd8fb5e6bd40098efbcc60f156fd45370
bff8a7a80d2a198e84443a696438925d4c303c7b subprocess: replace __USING_WINDOWS__ with WIN32 (kevkevinpal)
Pull request description:
## Summary
Motivated by https://github.com/bitcoin/bitcoin/pull/34385#pullrequestreview-3826616188
In `subprocess.h` we are now renaming `__USING_WINDOWS__` with `WIN32`
In the rest of the codebase, we are using `WIN32`, so it makes sense to update `subprocess.h` to match that.
---
Use the following `grep` to assert there is no `__USING_WINDOWS__` in the codebase
```
grep -nri --exclude-dir=build "WIN32" ./ -I
rep -nri --exclude-dir=build "__USING_WINDOWS__" ./ -I
```
ACKs for top commit:
sedited:
ACK bff8a7a80d2a198e84443a696438925d4c303c7b
hebasto:
ACK bff8a7a80d2a198e84443a696438925d4c303c7b, I have reviewed the code and it looks OK.
Tree-SHA512: 18c3c8b87cf880053bbf69f837a0a135c5da51cfb15ab1d9fd554d8f931b2ea8202cf0f4d5e6f317d6234480128c2f41a7a1a9d9bd0504697a3c4c6a21797762
5c005363a880c136cc44ff2456a402e398fcbf44 test: improve `wallet_backup` test (rkrux)
04d95157485e31b6a8a0f2eb2d7a65023b4199ac test: improve `wallet_assumeutxo` func test (rkrux)
Pull request description:
Relates to #34354
While the actual fix of the issue is in another PR, this one improves the
affected tests by trying to reduce the chain notifications that
need to be processed while simulating erroneous wallet restoration
scenarios.
ACKs for top commit:
maflcko:
lgtm ACK 5c005363a880c136cc44ff2456a402e398fcbf44
furszy:
ACK 5c005363a880c136cc44ff2456a402e398fcbf44
w0xlt:
ACK 5c005363a880c136cc44ff2456a402e398fcbf44
brunoerg:
code review ACK 5c005363a880c136cc44ff2456a402e398fcbf44
Tree-SHA512: 176e3ea8275c7aa082af695f5b76d82c079ff9a7178855b4ce95504edb8ce89b59a772e2d38dd43e997a5bea3d64be700b74cfec7bbc6992538f837877ab7222
d76ec4de148724e6b384efb0a4180722ed30db10 fuzz: make sure PSBT serialization roundtrips (Antoine Poinsot)
Pull request description:
~~Invalid public keys were accepted in Musig2 partial signatures. Because we serialize invalid keys as the empty byte string, this would lead us to creating an invalid PSBT serializations.~~
~~This can be checked by reverting the first commit with the fix and simply running the target against the existing qa-assets corpus for the `psbt` harness.~~
This patch found the issue fixed in #34219 with a single run against the existing qa-assets corpus. It is useful to make sure there are no similar bugs, and we don't introduce roundtrip regressions outside of the specifc instance of accepting invalid public keys in Musig2 fields.
*(Edited on March 4 to only contain the fuzz harness patch)*
ACKs for top commit:
davidgumberg:
crACK d76ec4de14
achow101:
ACK d76ec4de148724e6b384efb0a4180722ed30db10
dergoegge:
utACK d76ec4de148724e6b384efb0a4180722ed30db10
brunoerg:
code review ACK d76ec4de148724e6b384efb0a4180722ed30db10
Tree-SHA512: ab5f8d4e6a1781ecdef825e1a0e2793a6b553f36c923a4a35cb1af4070eead9d9780f6cc9a76235aa03462e52a129d15e61f631490b43651dc4395f3f1c005f3
Fix the from-disk subtest to use a separate node so it builds on a
clean genesis block, rather than the leftover chain from the
in-memory subtest.
Change from a two-way to a three-way block race. The UB in the old
LoadChainTip (mutating nSequenceId, a sort key, while the block is
in setBlockIndexCandidates) corrupts the internal tree structure,
resulting in a failed erase that leaves stale blocks in the set
alongside the tip. With only two competing blocks, this is caught
by libstdc++ but not by libc++. A three-way split triggers the bug
on both implementations.
To trigger CheckBlockIndex (where the crashing assertion is), replace
the restart loop with sending a new block after a single restart.
faa68ed4bdce6540f99d23c9b200ca575794a1b2 test: Fix intermittent issue in wallet_assumeutxo.py (MarcoFalke)
Pull request description:
The test has many issues:
* It fails intermittently, due to the use of `-stopatheight` (https://github.com/bitcoin/bitcoin/issues/34710)
* Using `-stopatheight` is expensive, because it requires two restarts, making the test slow
* The overwritten `def setup_network` does not store the extra args via the `add_nodes` call, so if code is added in the future to restart a node, it may not pick up its global extra args.
Fix all issues by:
* Adding and using a fast `dumb_sync_blocks` util helper to achieve what `-stopatheight` doesn't achieve
* Calling `self.add_nodes(self.num_nodes, self.extra_args)` in the overridden `setup_network`
Can be tested via this diff:
```diff
diff --git a/src/node/kernel_notifications.cpp b/src/node/kernel_notifications.cpp
index ab0e5ccb69..49384c10b8 100644
--- a/src/node/kernel_notifications.cpp
+++ b/src/node/kernel_notifications.cpp
@@ -61,2 +61,3 @@ kernel::InterruptResult KernelNotifications::blockTip(SynchronizationState state
if (m_stop_at_height && index.nHeight >= m_stop_at_height) {
+ LogInfo("Send shutdown signal after reaching stop height");
if (!m_shutdown_request()) {
diff --git a/src/util/tokenpipe.cpp b/src/util/tokenpipe.cpp
index c982fa6fc4..a5565ebd36 100644
--- a/src/util/tokenpipe.cpp
+++ b/src/util/tokenpipe.cpp
@@ -4,2 +4,3 @@
#include <util/tokenpipe.h>
+#include <util/time.h>
@@ -60,2 +61,3 @@ int TokenPipeEnd::TokenRead()
ssize_t result = read(m_fd, &token, 1);
+ UninterruptibleSleep(500ms);
if (result < 0) {
```
On master: The test fails
On this pull: The test passes
Fixes https://github.com/bitcoin/bitcoin/issues/34710
ACKs for top commit:
kevkevinpal:
ACK [faa68ed](faa68ed4bd)
achow101:
ACK faa68ed4bdce6540f99d23c9b200ca575794a1b2
w0xlt:
ACK faa68ed4bdce6540f99d23c9b200ca575794a1b2
Tree-SHA512: 6fcd52b6f6a5fa5a5e41e7b3cf5c733af62af9c60271e7d22c266aca90f2af67f91ffe80a3ed8b8d1a91d001700870f493207998bac988c4e3671a3a0dba7ba7
The removal of the chain tip from setBlockIndexCandidates was
happening after nSequenceId was modified. Since the set uses
nSequenceId as a sort key, modifying it while the element is in the
set is undefined behavior, which can cause the erase to fail.
With assumeutxo, a second form of UB exists: two chainstates each
have their own candidate set, but share the same CBlockIndex
objects. Calling LoadChainTip on one chainstate mutates nSequenceIds
that are also in the other chainstate's set.
Fix by populating setBlockIndexCandidates after all changes to
nSequenceId.
This call is a no-op. PopulateAndValidateSnapshot already sets both
the chain tip and the coins cache best block to the snapshot block,
so LoadChainTip always hits the early return when it finds that the
two match (tip->GetBlockHash() == coins_cache.GetBestBlock()).
f51665bee72c26d3f3cc6813b6c02adad5f0af6a psbt: validate pubkeys in MuSig2 pubnonce/partial sig deserialization (tboy1337)
Pull request description:
The previous fix for invalid MuSig2 pubkeys (bitcoin/bitcoin#34010) only
addressed the PSBT_IN_MUSIG2_PARTICIPANT_PUBKEYS field. However, the
PSBT_IN_MUSIG2_PUB_NONCE and PSBT_IN_MUSIG2_PARTIAL_SIG fields also
deserialize pubkeys without validation, which could lead to crashes when
invalid pubkeys are processed.
This commit adds validation to the DeserializeMuSig2ParticipantDataIdentifier
function to ensure all pubkeys in MuSig2 pubnonce and partial signature
fields are fully valid elliptic curve points.
The fix:
- Validates both aggregate and participant pubkeys in MuSig2 pubnonce and
partial signature deserialization
- Throws std::ios_base::failure with descriptive error messages for invalid
pubkeys
- Prevents potential crashes from invalid elliptic curve points
- Maintains backward compatibility for valid PSBTs
This completes the fix for issues [#33999](https://github.com/bitcoin/bitcoin/issues/33999) and [#34201](https://github.com/bitcoin/bitcoin/issues/34201).
ACKs for top commit:
rkrux:
lgtm ACK f51665bee72c26d3f3cc6813b6c02adad5f0af6a
w0xlt:
ACK f51665bee7
darosior:
utACK f51665bee72c26d3f3cc6813b6c02adad5f0af6a
Tree-SHA512: 8454d77a05aa003a3121b0a5975e8a000125ee0d62343bfa625a75db113358ba7a210ae0376ca1666957b7de7005e06e5a54c95170430ee5e9e1416719b8af53
1c1de334e9c0e3b4c72c3f3fe45b1f4df5ce4502 test: avoid interface_ipc.py race and null pointer dereference (Ryan Ofsky)
Pull request description:
Avoid race condition in `run_deprecated_mining_test` caused by creating and immediately destroying an unused worker thread. This is leading to test failures reported by maflcko in #34711
ACKs for top commit:
optout21:
utACK 1c1de334e9c0e3b4c72c3f3fe45b1f4df5ce4502
l0rinc:
Tested ACK 1c1de334e9c0e3b4c72c3f3fe45b1f4df5ce4502
w0xlt:
ACK 1c1de334e9c0e3b4c72c3f3fe45b1f4df5ce4502
enirox001:
ACK 1c1de33
Tree-SHA512: d0af9676a46e991a3f4fda3795c02d1998d30de24991436b8ada425585c6699ff32a7057ca7a0ef2925f782fd3bf73e0381f5d4325e4f1c09f487fce1de49e45
b87a1c27c99821cea995112114eb40afa93417f2 doc: Improve dependencies.md IPC documentation (Ryan Ofsky)
Pull request description:
Improve dependencies.md to document IPC dependencies better ([preview](https://github.com/ryanofsky/bitcoin/blob/pr/ipc-depdoc/doc/dependencies.md#build-1)). Specific changes are listed in the commit message.
This PR is based on #33623 by willcl-ark which made similar changes in the 29.x branch. This PR could also be backported to 30.x (it merges cleanly, and master and 30.x both have the same version requirements).
ACKs for top commit:
l0rinc:
ACK b87a1c27c99821cea995112114eb40afa93417f2
sedited:
ACK b87a1c27c99821cea995112114eb40afa93417f2
Tree-SHA512: 566b5372d189f0ad04f978ddefbd8c200dcc19b25af02c73275c5faf7529943680ec59703bda11640cf7920466b4cdd46305cac4d3770e2303de37694ae78909
Cross-compiling Qt 6.8 for Windows requires GCC 13.1 or newer, which
exceeds the currently documented minimum.
See https://doc.qt.io/qt-6.8/windows.html.
fa7612f2536b0ef47334105cf657879cdcc3a579 ci: Download script_assets_test.json for Windows CI (MarcoFalke)
7777a13306babd17e6c5956b2efb7c7886e8fc7c test: Move Fetching-print to download_from_url util (MarcoFalke)
faf96286ce69937d141d06f2a71b9fce3a5b89aa test: move-only download_from_url to stand-alone util file (MarcoFalke)
fa0cc1c5a4a760280afd7942fc3b554d1781eb09 test: [doc] Remove outdated comment (MarcoFalke)
Pull request description:
Fixes https://github.com/bitcoin/bitcoin/issues/34670 by adding a new `download_script_assets` Python helper and calling it.
ACKs for top commit:
hebasto:
re-ACK fa7612f2536b0ef47334105cf657879cdcc3a579.
janb84:
re ACK fa7612f2536b0ef47334105cf657879cdcc3a579
hodlinator:
utACK fa7612f2536b0ef47334105cf657879cdcc3a579
Tree-SHA512: 73c2cb3a31f231174566fb880b82de92734b1679ef000f8d793d774b7e5f5a7b8c7994a3998ca78821115bdc80c16aada69cf596e92c083cf9b9a95c7cee16ea
5e35a9069d655419cada6070e1f00a28d4a877f9 interpreter: remove clang-tidy suppression (fanquake)
4089682f5cfecd04b8d983be5f82d00c0c418bf4 ci: use Clang 22 in tidy task (fanquake)
7ea076f996dee27a640b8b18b2b58be8011dfe6e tidy: remove deprecated header (fanquake)
eb17f29aa5791136af0be5d5554d41ce34a53aa5 tidy: clang-tidy is required (fanquake)
Pull request description:
Changes needed for moving to Clang 22 in the tidy job.
ACKs for top commit:
maflcko:
lgtm ACK 5e35a9069d655419cada6070e1f00a28d4a877f9
hebasto:
ACK 5e35a9069d655419cada6070e1f00a28d4a877f9, I have reviewed the code and it looks OK.
Tree-SHA512: 9ca6e841f7480b8abd78d5621d08a5bf80c2ff4facd7a0d76038ac1771bbf3d37dc2df19fa27583679177e4618db6294e2f2bb2129d9c25a53338b49ed71aac2
```bash
15 | #warning The ClangTidyModuleRegistry.h header is deprecated and will be removed in LLVM 24. All of the symbols it used to define have been moved into ClangTidyModule.h.
| ^~~~~~~
[100%] Linking CXX shared module libbitcoin-tidy.so
```
98e8af4bb991fd8edeb15c0fb8afa66bff6b5cac wallet: Drain validation interface queue after notifications disconnect (Ava Chow)
52992ebe1c55c8f7219b824f05d22fbc18acb794 interfaces: Add waitForNotifications() to call SyncWithValidationInterfaceQueue() (Ava Chow)
Pull request description:
When the wallet disconnects chain notifications, it is expecting no further notifications to execute, but this is not the case. This results in test failures such as in #34354. Instead of disconnecting the notifications and continuing shutdown, we should wait for the validation interface queue to be drained before the rest of wallet shutdown. This is achieved by adding an `interfaces::Chain::waitForNotifications()` function which calls `SyncWithValidationInterfaceQueue()`.
Fixes#34354
ACKs for top commit:
stickies-v:
utACK 98e8af4bb991fd8edeb15c0fb8afa66bff6b5cac
furszy:
ACK 98e8af4bb991fd8edeb15c0fb8afa66bff6b5cac
rkrux:
crACK 98e8af4bb991fd8edeb15c0fb8afa66bff6b5cac
sedited:
ACK 98e8af4bb991fd8edeb15c0fb8afa66bff6b5cac
Tree-SHA512: 263628556f740cb633d3970c22a0dfdb52a644bd1d0cd5a69c2970524edbb0e25d592cb39fc9bf1d0c281eebce09578526e2958dffee9026fc7473db35bd0dec
Avoid race condition in run_deprecated_mining_test caused by creating and
immediately destroying an unused worker thread. This leads to test failures
reported by maflcko in #34711
da7f70a5322843b70f29456a8bc2227209a0718b test: use port 0 for I2P addresses in p2p_private_broadcast.py (Vasil Dimov)
a8ebcfd34c63f142064b4f5ef7d52299739d4cd6 test: let connections happen in any order in p2p_private_broadcast.py (Vasil Dimov)
67696b207f370e902c8d5fb765e4ff10f6c9e1b4 net: extend log message to include attempted connection type (Vasil Dimov)
Pull request description:
If the following two events happen:
* (likely) the automatic 10 initial connections are not made to all
networks
* (unlikely) the network-specific logic kicks in almost immediately.
It is using exponential distribution with a mean of 5 minutes
(`rng.rand_exp_duration(EXTRA_NETWORK_PEER_INTERVAL)`).
So if both happen, then the 11th connection may not be the expected
private broadcast, but a network-specific connection.
Fix this by retrieving the connection type from
`destinations_factory()`. This is more flexible because it allows
connections to happen in any order and does not break if e.g. the 11th
connection is not the expected first private broadcast.
This also makes the test run faster:
before: 19-44 sec
now: 10-25 sec
because for example there is no need to wait for the initial 10
automatic outbound connections to be made in order to proceed.
Fixes: https://github.com/bitcoin/bitcoin/issues/34387
ACKs for top commit:
achow101:
ACK da7f70a5322843b70f29456a8bc2227209a0718b
andrewtoth:
ACK da7f70a5322843b70f29456a8bc2227209a0718b
mzumsande:
Code Review ACK da7f70a5322843b70f29456a8bc2227209a0718b
Tree-SHA512: 7c293e59c15c148a438e0119343b05eb278205640658c99336d4caf4848c5bae92b48e15f325fa616cbc9d5f394649abfa02406a76e802cffbd3d312a22a6885
