💬 fanquake commented on issue "cmake: Cannot find Qt 6 on SunOS / illumos (OpenIndiana Distribution)":
(https://github.com/bitcoin/bitcoin/issues/32536#issuecomment-2888278841)
> If we want to provide binary packages for download on a website, we take the role of package maintainers. In packaging scripts, we need to setup a build environment so that all dependencies can be found. There are countless ways to do that.

The only way we will produce anything binary, that would end up being distributed on our website, is via our Guix build; so any approach taken needs to happen inside Guix (all blobs/binaries must be reproducible).

> Such build scripts ideally should be m
...

👍 TheCharlatan approved a pull request: "refactor: validation: mark CheckBlockIndex as const"
(https://github.com/bitcoin/bitcoin/pull/32364#pullrequestreview-2848233855)
ACK 3e6ac5bf772751c66cdcd015dcb7e6ce4ea2cc77

🤔 luke-jr reviewed a pull request: "config: allow setting -proxy per network"
(https://github.com/bitcoin/bitcoin/pull/32425#pullrequestreview-2848222887)
Probably an improvement as-is, but might want to think through the name proxy logic

💬 luke-jr commented on pull request "config: allow setting -proxy per network":
(https://github.com/bitcoin/bitcoin/pull/32425#discussion_r2094078095)
I'm not sure this logic is ideal. If you set an all-network proxy, but then disable it for IPv6, you might be expecting name resolution to use the proxy. Or maybe not.

💬 luke-jr commented on pull request "config: allow setting -proxy per network":
(https://github.com/bitcoin/bitcoin/pull/32425#discussion_r2094077060)
These conditions aren't needed. `SetProxy` itself just returns false if it's not valid.

💬 luke-jr commented on pull request "config: allow setting -proxy per network":
(https://github.com/bitcoin/bitcoin/pull/32425#discussion_r2094087242)
string_view?

💬 maflcko commented on issue "cmake: Cannot find Qt 6 on SunOS / illumos (OpenIndiana Distribution)":
(https://github.com/bitcoin/bitcoin/issues/32536#issuecomment-2888283003)
Are there any known users on this platform? Maybe this can be closed for now, until there are a few users. Then, a `build-*.md` can be added with any needed details. Whether or not to do a guix build can be decided then as well.

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

💬 luke-jr commented on pull request "config: allow setting -proxy per network":
(https://github.com/bitcoin/bitcoin/pull/32425#discussion_r2094091911)
Did you want to indent this?

💬 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