💬 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)

🚀 fanquake merged a pull request: "test: fix two intermittent failures in wallet_basic.py"
(https://github.com/bitcoin/bitcoin/pull/32483)

💬 l0rinc commented on pull request "refactor: reenable `implicit-integer-sign-change` check for `serialize.h`":
(https://github.com/bitcoin/bitcoin/pull/32296#discussion_r2088818525)
Nice, I like that a lot more - addressed the reformatting and unification in separate commits, let me know if you think this is better.
Pushed a rebase separately to simplify review from GitHub as well (I know you do range-diff), and to make sure the latest CI is run against it.

💬 fanquake commented on pull request "test: fix two intermittent failures in wallet_basic.py":
(https://github.com/bitcoin/bitcoin/pull/32483#issuecomment-2880037334)
Backported to `29.x` in #32292.

⚠️ Sjors opened an issue: "Dropping unused legacy wallet code"
(https://github.com/bitcoin-core/gui/issues/873)
Tracking pull request(s) that remove any remaining legacy wallet code, either here or in the main repo:

- https://github.com/bitcoin/bitcoin/pull/32459

💬 luke-jr commented on pull request "[DRAFT] Expose AssumeUTXO Load Snapshot Functionality To The GUI":
(https://github.com/bitcoin-core/gui/pull/870#issuecomment-2880052845)
Seems like this would be more appropriate for the first-run intro page?

💬 Sjors commented on pull request "qt: drop unused watch-only functionality":
(https://github.com/bitcoin/bitcoin/pull/32459#issuecomment-2880054651)
Doesn't seem worth moving, and this PR also touches the interface, but I opened a tracking issue on the GUI repo that points here. There's probably more than what I found here.

💬 maflcko commented on pull request "refactor: reenable `implicit-integer-sign-change` check for `serialize.h`":
(https://github.com/bitcoin/bitcoin/pull/32296#discussion_r2088837960)
nit in the second commit: Would be good to just set this to the final version `template<typename Stream> void Serialize(Stream& s, int8_t a ) { ser_writedata8(s, uint8_t(a)); }` from the beginning. Otherwise there will be three style-fixup commits on top of each other, making review harder and the git log and git blame depth tedious to follow.