⚠️ fanquake opened an issue: "build: (initial?) failure to build xproto on Alpine"
(https://github.com/bitcoin/bitcoin/issues/32494)
```bash
# podman run -it alpine
apk update && apk add pkgconf patch python3 cmake g++ bash curl gcc git make && \
git clone https://github.com/bitcoin/bitcoin && \
cd bitcoin && \
make -C depends qt -j13
```

This will fail with
```bash
Staging xproto...
make[1]: Entering directory '/bitcoin/depends/work/build/aarch64-unknown-linux-musl/xproto/7.0.31-7e4871430ec'
Making install in specs
make[2]: Entering directory '/bitcoin/depends/work/build/aarch64-unknown-linux-musl/xproto/7.0.31-7e4871430
...

💬 jsarenik commented on issue "Allow for rescan using block filters with pruned node":
(https://github.com/bitcoin/bitcoin/issues/21267#issuecomment-2879773571)
Just what I noticed and it seems related to this issue: After using `removeprunedfunds` followed by `rescanblockchain start stop` I see spent transactions in my descriptor wallet's `listunspent` output. 100% reproducible on regtest.

What I found so far is that after `removeprunedfunds` a `rescanblockchain` called without parameters results in a usable wallet but when called with some block range like `rescanblockchain 1026 1146` (with my tip height 1146) I end up with lots of already-spent outp
...

💬 stickies-v commented on pull request "kernel: Separate UTXO set access from validation functions":
(https://github.com/bitcoin/bitcoin/pull/32317#discussion_r2088705047)
It seems like list initializing `coin` is triggering MSVC to copy-initialize a `Coin` temporary, causing a second user-defined conversion and subsequent error. I don't really understand why direct initialization rules are different in this regard, but using it does seem to make MSVC happy: https://github.com/stickies-v/bitcoin/actions/runs/15018875180/job/42203094843

Fix: https://github.com/stickies-v/bitcoin/commit/6f1bc11c336db6dcea2954a03520b7064f0f0e6d

💬 vasild commented on pull request "net: replace manual reference counting of CNode with shared_ptr":
(https://github.com/bitcoin/bitcoin/pull/32015#discussion_r2088714491)
`m_node_mutex` is a recursive mutex, allowing multiple nested locks from a single thread. But see https://github.com/bitcoin/bitcoin/pull/32326 which changes it to a non-recursive mutex which is easier to reason about.

💬 vasild commented on pull request "net: replace manual reference counting of CNode with shared_ptr":
(https://github.com/bitcoin/bitcoin/pull/32015#issuecomment-2879829734)
`8b93e0894f...bd6c57f387`: address suggestions

💬 vasild commented on pull request "net: replace manual reference counting of CNode with shared_ptr":
(https://github.com/bitcoin/bitcoin/pull/32015#discussion_r2088718421)
Because the only caller of `GenerateWaitSockets()` has a vector so I decided to pass a const reference to a vector. However, that was the case as well before this PR and the change from raw pointer to `shared_ptr` (the aim of this PR) is unrelated to that. Changed back to `std::span`.

💬 vasild commented on pull request "net: replace manual reference counting of CNode with shared_ptr":
(https://github.com/bitcoin/bitcoin/pull/32015#discussion_r2088722206)
Yes, I meant that the location of the deletes is unchanged. Before we had `DeleteNode()` which did `FinalizeNode()` + `delete` and after that commit, `DeleteNode()` is expanded into the callers that do `FinalizeNode()` directly instead of calling `DeleteNode()` + `clear()` on the vector which is equivalent to the `delete` that was in `DeleteNode()`.

Elaborated a little bit in the commit message.

💬 maflcko commented on pull request "test: add skip_if_running_under_valgrind()":
(https://github.com/bitcoin/bitcoin/pull/32492#issuecomment-2879855860)
lgtm ACK 75a185ea3db3177e8e479ee61a39bcb51e08d9a6

I haven't tested or confirmed this, but the diff looks plausible.

💬 vasild commented on pull request "net: replace manual reference counting of CNode with shared_ptr":
(https://github.com/bitcoin/bitcoin/pull/32015#discussion_r2088731770)
I did not want to bloat this PR with changes to the interface of `ProcessMessages()` and `SendMessages()`. They take a raw pointer. Since they do not take ownership and the object is going to be alive for the duration of the call and there will always be an object (will never pass `nullptr`), then those functions can take `CNode&`.

Here is a patch to do that on top of this PR. Could be a followup. I do not want to bloat this PR with this unless reviewers want it.

<details>
<summary>[patch
...

💬 vasild commented on pull request "net: replace manual reference counting of CNode with shared_ptr":
(https://github.com/bitcoin/bitcoin/pull/32015#issuecomment-2879863257)
`bd6c57f387...20657a7c8e`: rebase

⚠️ fanquake opened an issue: "test: consider Clang `type` sanitizer"
(https://github.com/bitcoin/bitcoin/issues/32495)
Clang 20 introduced an experimental type sanitizer that "detects violations of C/C++ type-based aliasing rules." See here for more info: https://clang.llvm.org/docs/TypeSanitizer.html. Someone might want to run this against our code, to see if it works, or detects real issues. This could likely be done with:
```bash
make -C depends/ CC=clang CXX=clang++ CXXFLAGS="-stdlib=libc++" NM=llvm-nm AR=llvm-ar RANLIB=llvm-ranlib STRIP=llvm-strip
cmake -B build --toolchain /bitcoin/depends/x86_64-pc-linux-
...

💬 maflcko commented on pull request "test: fix two intermittent failures in wallet_basic.py":
(https://github.com/bitcoin/bitcoin/pull/32483#issuecomment-2879883748)
review ACK e7ad86e1ca3b0b2f2795e91c2f9959486c67dd90 🎩

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: review ACK e7ad86e1ca3b
...

📝 fanquake opened a pull request: "depends: add Qt `-ltcg` for Darwin, drop it for Windows"
(https://github.com/bitcoin/bitcoin/pull/32496)
The related Windows patches were dropped in 5e794e62024eef612e1fbb71c76ea54d17435c14, and "Cross-compiling does not support LTO." (from #30997).

Using this with native Darwin builds works ok.

💬 luke-jr commented on pull request "qt: drop unused watch-only functionality":
(https://github.com/bitcoin/bitcoin/pull/32459#issuecomment-2879908057)
Shouldn't this be on the GUI repo?

💬 luke-jr commented on pull request "fs: remove `_POSIX_C_SOURCE` defining":
(https://github.com/bitcoin/bitcoin/pull/32460#issuecomment-2879919952)
>From what I can tell, compilers on Linux systems, will be defining _GNU_SOURCE

`_GNU_SOURCE` isn't supposed to ever be defined by default, and I don't see us defining it anywhere...?

💬 maflcko commented on issue "test: wallet_reorgsrestore.py failure under valgrind":
(https://github.com/bitcoin/bitcoin/issues/32493#issuecomment-2879940657)
Does it always happen, or did this start at some point in the git history?

👍 laanwj approved a pull request: "cmake: Add application manifests when cross-compiling for Windows"
(https://github.com/bitcoin/bitcoin/pull/32396#pullrequestreview-2839952072)
re-ACK c0f41523973e33a8cd104a8ebf4906a4d99f3eed

💬 fanquake commented on pull request "test: fix two intermittent failures in wallet_basic.py":
(https://github.com/bitcoin/bitcoin/pull/32483#issuecomment-2880021915)
@achow101 note that you've ACK'd a commit has not in this PR, but for an equivalent change.

✅ fanquake closed an issue: "failure in wallet_basic.py --descriptors"
(https://github.com/bitcoin/bitcoin/issues/27249)

✅ fanquake closed an issue: "qa: Failure in wallet_basic.py spendzeroconfchange test"
(https://github.com/bitcoin/bitcoin/issues/32456)