💬 hebasto commented on issue "test: Running thread * was not suspended. False leaks are possible.":
(https://github.com/bitcoin/bitcoin/issues/32542#issuecomment-2888292948)
> ... I'd also think that running `ctest` vs running `test_bitcoin` directly, should generally result in the same output, otherwise if sanitizers are producing warnings, or other relevant output, that is currently being "hidden" by `ctest`.

cc @purpleKarrot

💬 TheCharlatan commented on pull request "validation: Add eligible ancestors of reconsidered block to setBlockIndexCandidates":
(https://github.com/bitcoin/bitcoin/pull/30479#discussion_r2094104836)
Nice catch! How about removing the default argument altogether:
```diff
diff --git a/src/chain.h b/src/chain.h
index c6d1640768..f5bfdb2fb4 100644
--- a/src/chain.h
+++ b/src/chain.h
@@ -295 +295 @@ public:
- bool IsValid(enum BlockStatus nUpTo = BLOCK_VALID_TRANSACTIONS) const
+ bool IsValid(enum BlockStatus nUpTo) const
diff --git a/src/test/fuzz/chain.cpp b/src/test/fuzz/chain.cpp
index 0363f317b6..09053e4815 100644
--- a/src/test/fuzz/chain.cpp
+++ b/src/test/fuzz/chain.cpp
...

👋 hebasto's pull request is ready for review: "build: Use clang-cl to build on Windows natively"
(https://github.com/bitcoin/bitcoin/pull/31507)

💬 romanz commented on pull request "index: store per-block transaction locations for efficient lookups":
(https://github.com/bitcoin/bitcoin/pull/32541#issuecomment-2888339681)
Fixed a few lint issues (following [SonarQube run](https://sonarcloud.io/project/issues?issueStatuses=OPEN%2CCONFIRMED&sinceLeakPeriod=true&branch=32541-1f629743e6c6ba4fab77ef1a8578bb82f0a70566&id=aureleoules_bitcoin)).

💬 hebasto commented on pull request "build: Use clang-cl to build on Windows natively":
(https://github.com/bitcoin/bitcoin/pull/31507#discussion_r2094105826)
More details have been added to the commit message.

> Also, any reason to not fix the issues in leveldb, rather than adding more suppressions?

I'll consider this option.

⚠️ hebasto opened an issue: "`AddressBookTests` failure"
(https://github.com/bitcoin-core/gui/issues/874)
From https://cirrus-ci.com/task/5063835456897024:
```
[11:58:03.176] ********* Start testing of AddressBookTests *********
[11:58:03.176] Config: Using QtTest library 6.7.3, Qt 6.7.3 (arm-little_endian-ilp32-eabi-hardfloat static release build; by GCC 13.3.0), ubuntu 24.04
[11:58:03.176] PASS : AddressBookTests::initTestCase()
[11:58:03.176] QDEBUG : AddressBookTests::addressBookTests() NotifyUnload
[11:58:03.176] QWARN : AddressBookTests::addressBookTests() This plugin does not support propa
...

💬 hebasto commented on issue "`AddressBookTests` failure":
(https://github.com/bitcoin-core/gui/issues/874#issuecomment-2888345971)
Also in https://github.com/bitcoin/bitcoin/actions/runs/15084925283/job/42406255562.

💬 romanz commented on issue "intermittent issue in rpc_signer.py (enumeratesigners timeout)":
(https://github.com/bitcoin/bitcoin/issues/32524#issuecomment-2888369623)
Another instance: https://cirrus-ci.com/task/4688904676179968?logs=ci#L4390

💬 purpleKarrot commented on issue "test: Running thread * was not suspended. False leaks are possible.":
(https://github.com/bitcoin/bitcoin/issues/32542#issuecomment-2888371321)
CTest is instructed to pass some arguments to `test_bitcoin`: https://github.com/bitcoin/bitcoin/blob/7710a31f0cb69a04529f39840196826d0b9770ab/src/test/CMakeLists.txt#L193-L195

Maybe that is the reason? That being said, CTest has builtin support for memory checking using sanitizers.

💬 laanwj commented on pull request "[DONOTMERGE] subprocess: replace `fs::directory_iterator` with `readdir`":
(https://github.com/bitcoin/bitcoin/pull/32529#issuecomment-2888390983)
> I haven't looked at the code, but the CI passed 50 times: https://cirrus-ci.com/task/6662743735926784

Okay. So unless the original code is using `directory_iterator` wrong (which i don't think so), we can be reasonably sure there's an intermittent bug in the implementation of it on at least some libc++ versions.

👍 TheCharlatan approved a pull request: "validation: stricter internal handling of invalid blocks"
(https://github.com/bitcoin/bitcoin/pull/31405#pullrequestreview-2848281583)
Re-ACK ed172d3002da68a3ddbd5d13e7d3f0618c1378d4

💬 l0rinc commented on pull request "blocks: avoid recomputing block header hash in `ReadBlock`":
(https://github.com/bitcoin/bitcoin/pull/32487#discussion_r2094147733)
We're not always checking this, only when the index is available.
But after this change this check is for free, no need to recalculate the header's hash anymore.

📝 TheCharlatan opened a pull request: "kernel: Remove dependency on clientversion"
(https://github.com/bitcoin/bitcoin/pull/32543)
Including a Bitcoin-Core specific version does not make sense if used by external applications.

💬 romanz commented on pull request "rest: fetch spent transaction outputs by blockhash":
(https://github.com/bitcoin/bitcoin/pull/32540#issuecomment-2888466453)
Force-pushed to fix a typo in commit description.

💬 katesalazar commented on issue "Option to use dark theme for Windows":
(https://github.com/bitcoin-core/gui/issues/378#issuecomment-2888477115)
There is too low contrast on the buttons in that pic

🤔 polespinasa reviewed a pull request: "rpc: Round verificationprogress to 1 for a recent tip"
(https://github.com/bitcoin/bitcoin/pull/32528#pullrequestreview-2848344096)
cACK

💬 polespinasa commented on pull request "rpc: Round verificationprogress to 1 for a recent tip":
(https://github.com/bitcoin/bitcoin/pull/32528#discussion_r2094159634)
> In case we know there are unprocessed blocks in the chain (because we have their headers), we shouldn't say progress: 1

I agree we should not return 1 if we didn't verify the last block we know (we don't want Core to lie to users 😉)

What if we do something like:
```c++
if (pindex->nHeight == m_best_header->nHeight && pindex->nChainWork == m_best_header->nChainWork) {
const auto block_time{pindex->GetBlockTime() + Ticks<std::chrono::seconds>(2h)};
} else {
const auto block_
...

📝 theStack opened a pull request: "scripted-diff: test: remove 'descriptors=True' argument for `createwallet` calls"
(https://github.com/bitcoin/bitcoin/pull/32544)
Descriptor wallets are already created by default [since v23.0](https://github.com/bitcoin/bitcoin/blob/7710a31f0cb69a04529f39840196826d0b9770ab/doc/release-notes/release-notes-23.0.md?plain=1#L171), but since the recent legacy wallet removal the `descriptors` parameter *must* be True for the `createwallet` RPC (see commit 9f04e02ffaee0fe64027dc56c7bea3885254321a), i.e. still passing it wouldn't contain any information for test readers anymore. So simply drop them in the functional tests in orde
...

💬 1440000bytes commented on pull request "scripted-diff: test: remove 'descriptors=True' argument for `createwallet` calls":
(https://github.com/bitcoin/bitcoin/pull/32544#issuecomment-2888488756)
Concept ACK

The argument `descriptors` could also be removed from `createwallet` RPC.

🤔 tapcrafter reviewed a pull request: "init: Configure reachable networks before we start the RPC server"
(https://github.com/bitcoin/bitcoin/pull/32539#pullrequestreview-2848348655)
tACK 12ff4be9c724c752fe67835419be3ff4d0e65990

<details>

<summary>Test protocol</summary>

## Current error message on master (7710a31f0cb69a04529f39840196826d0b9770ab):

```
$ bitcoind -regtest -rpcallowip=fc00:dead:beef::/64 -rpcuser=u -rpcpassword=p

2025-05-17T16:24:29Z Command-line arg: regtest=""
2025-05-17T16:24:29Z Command-line arg: rpcallowip="fc00:dead:beef::/64"
2025-05-17T16:24:29Z Command-line arg: rpcpassword=****
2025-05-17T16:24:29Z Command-line arg: rpcuser=****

...