💬 Sjors commented on pull request "wallet: Add `exportwatchonlywallet` RPC to export a watchonly version of a wallet":
(https://github.com/bitcoin/bitcoin/pull/32489#discussion_r2101952111)
In 52583d3269cc747ff58b6d0b8c213a1ac521aeb3 "wallet: Use Descriptor::CanSelfExpand() in CanGetAddresses()": would be nice to have to test that illustrates a case where this matters

💬 Sjors commented on pull request "wallet: Add `exportwatchonlywallet` RPC to export a watchonly version of a wallet":
(https://github.com/bitcoin/bitcoin/pull/32489#discussion_r2101947190)
In c61c87a453c1e5c1ad2f7d9c5b0e04c2d6df87ec "wallet, rpc: Push the normalized parent descriptor": typo in the commit message "prividing".

I'm a bit surprised such a change doesn't break a test.

Would also be good to explain why this needs to happen.

💬 fanquake commented on pull request "subprocess: Handle quoted tokens properly":
(https://github.com/bitcoin/bitcoin/pull/32577#issuecomment-2900376297)
> (please note that the src/util/subprocess.h code is C++11 compatible).

It seems a bit odd that we've imported a dependency that is pinned to an older C++ version than what we want to use, especially when we are trying to upstream changes to it. This being a nuisance has come up multiple times already (also here: https://github.com/bitcoin/bitcoin/pull/32343#issuecomment-2844382194). What is the longer term plan here?

💬 fanquake commented on pull request "allocators: Apply manual ASan poisoning to `PoolResource`":
(https://github.com/bitcoin/bitcoin/pull/32581#issuecomment-2900383024)
Concept ACK

💬 fanquake commented on pull request "fs: remove `_POSIX_C_SOURCE` defining":
(https://github.com/bitcoin/bitcoin/pull/32460#issuecomment-2900383285)
> It would be even better to use ftruncate on openbsd.

I haven't got anything OpenBSD handy to test on, but happy for someone to followup with these changes.

👍 TheCharlatan approved a pull request: "checkqueue: make the queue non-optional for CCheckQueueControl and drop legacy locking macro usage"
(https://github.com/bitcoin/bitcoin/pull/32467#pullrequestreview-2860315670)
Re-ACK fd290730f530a8b76a9607392f49830697cdd7c5

🚀 fanquake merged a pull request: "cmake: Remove `ENABLE_{SSE41,AVX2,X86_SHANI,ARM_SHANI}` from `bitcoin-build-config.h`"
(https://github.com/bitcoin/bitcoin/pull/32551)

🤔 luke-jr reviewed a pull request: "rest: fetch spent transaction outputs by blockhash"
(https://github.com/bitcoin/bitcoin/pull/32540#pullrequestreview-2860390650)
Not sure we should export a custom binary format for undo data like this. At least there should be a JSON option?

💬 luke-jr commented on pull request "rest: fetch spent transaction outputs by blockhash":
(https://github.com/bitcoin/bitcoin/pull/32540#discussion_r2102026426)
nit: braces required

💬 fanquake commented on pull request "cmake: Remove `ENABLE_{SSE41,AVX2,X86_SHANI,ARM_SHANI}` from `bitcoin-build-config.h`":
(https://github.com/bitcoin/bitcoin/pull/32551#issuecomment-2900447124)
Only 800b7cc42ca63f2a6b245a4d327c7092289da6e1 needs backporting here.

💬 l0rinc commented on issue "intermittent issue in rpc_signer.py (enumeratesigners timeout) under GLIBCXX debug mode":
(https://github.com/bitcoin/bitcoin/issues/32524#issuecomment-2900452144)
Another one in https://github.com/bitcoin/bitcoin/pull/32404/checks?check_run_id=42637298331 - retriggering the build makes it pass.

<details>
<summary>CI logs</summary>

```bash
[13:30:22.121] node0 2025-05-21T17:30:07.606048Z [msghand] [logging/timer.h:58] [Log] [lock] Enter: lock contention connman.m_nodes_mutex, ./net.h:1648 started
[13:30:22.121] node0 2025-05-21T17:30:07.606205Z [msghand] [logging/timer.h:58] [Log] [lock] Enter: lock contention connman.m_nodes_mutex, ./net.h:1648 complete
...

💬 dergoegge commented on pull request "allocators: Apply manual ASan poisoning to `PoolResource`":
(https://github.com/bitcoin/bitcoin/pull/32581#issuecomment-2900452384)
> What's the depper explanation for this?

At a high level `PoolResource` allocates larger chunks of memory (by default 262144 bytes per chunk) and allows for smaller memory section within those chunks to be allocated (in a way that is optimized for node based containers, e.g. `std::unordered_map`).

ASan instruments calls to e.g. `malloc`/`free` and `new`/`delete` to detect UaF and other issues, so for `PoolResource` it'll be aware of the allocations of the larger chunks. However, it does n
...

💬 fanquake commented on pull request "Use subprocess library for notifications":
(https://github.com/bitcoin/bitcoin/pull/32566#discussion_r2102038837)
I think I agree. Given we don't actually test or ship anything for iOS, it seems like we can clean up the code now, and figure out what to do if/when this might be an issue later.

💬 hodlinator commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2102049556)
Wasn't able to use the small Godbolt sample to divine the reason for my 2x Clang speedup yet.

Fell back to `objdump -gSC build/bin/bench_bitcoin` and clearly see:
```asm
*reinterpret_cast<uint64_t*>(target.data()) ^= rot_key;
1107c0: f3 0f 6f 4c d1 f0 movdqu -0x10(%rcx,%rdx,8),%xmm1
1107c6: f3 0f 6f 14 d1 movdqu (%rcx,%rdx,8),%xmm2
1107cb: 66 0f ef c8 pxor %xmm0,%xmm1
1107cf: 66 0f ef d0 pxor %xmm0,%x
...

🤔 janb84 reviewed a pull request: "depends: use "mkdir -p" when installing xproto"
(https://github.com/bitcoin/bitcoin/pull/32568#pullrequestreview-2860431537)
ACK df9ebbf659d5d1282289f36d7f9ee7103aa33a17

Breaks on master, builds with this commit:

<details>
Breaks on Master :


```
make[3]: Leaving directory '/bitcoin/depends/work/build/aarch64-unknown-linux-musl/xproto/7.0.31-fb91398a650'
make[2]: *** [Makefile:838: install-am] Error 2
make[2]: Leaving directory '/bitcoin/depends/work/build/aarch64-unknown-linux-musl/xproto/7.0.31-fb91398a650'
make[1]: *** [Makefile:535: install-recursive] Error 1
make[1]: Leaving directory '/bitcoin/
...

👍 vasild approved a pull request: "tests: Expand HTTP coverage to assert libevent behavior"
(https://github.com/bitcoin/bitcoin/pull/32408#pullrequestreview-2860441380)
ACK f16c8c67bf137e64fbeea1242431baaa915a5c53

Maybe the check for the timeout can be improved, ongoing discussion at https://github.com/bitcoin/bitcoin/pull/32408#discussion_r2077834027 (non-blocker IMO)

💬 l0rinc commented on pull request "allocators: Apply manual ASan poisoning to `PoolResource`":
(https://github.com/bitcoin/bitcoin/pull/32581#issuecomment-2900521959)
Thanks for clarifying.
Though https://github.com/google/sanitizers/wiki/AddressSanitizerManualPoisoning seems quite old (2015), a recent https://blog.trailofbits.com/2024/05/16/understanding-addresssanitizer-better-memory-safety-for-your-code article also confirms `Incorrect access to memory managed by a custom allocator won’t raise an error unless the allocator performs annotations`.
The sanitizer description also mentioned thread safety warnings and alignment restrictions - but I think we sh
...

📝 fanquake opened a pull request: "depends: hard-code necessary c(xx)flags rather than setting them per-host"
(https://github.com/bitcoin/bitcoin/pull/32584)

The per-host flag values now represent the mandatory flags that cannot be overridden by the environment. Additionally, these flags (-pipe and -std=xx) will no longer be passed into the CMake build.

Pulled out of #31920.

💬 fanquake commented on pull request "build: create Depends build type for depends and use it by default for depends builds":
(https://github.com/bitcoin/bitcoin/pull/31920#issuecomment-2900545835)
> It might be nice to move the first commit https://github.com/bitcoin/bitcoin/commit/40a01e9d30b0c7deabd5996867f248637f3d1a08 into a separate PR because it seems straightforwardly good (deduplicating code and moving flags that aren't host specific out of host-specific configurations) and it would remove complexity from this PR.

I've done that in #32584, as I've got some changes that will build on top of this.

💬 willcl-ark commented on pull request "depends: use "mkdir -p" when installing xproto":
(https://github.com/bitcoin/bitcoin/pull/32568#issuecomment-2900567471)
ok I just did a lazy rebuild of x86_64 only and got the same hashes again:

```
❯ find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
302a02a715fa2015c097b0c8bbe376822e8760f0a440adae9f1cc1df013667d1 guix-build-df9ebbf659d5/output/dist-archive/bitcoin-df9ebbf659d5.tar.gz
b37096b3a3d59f275a0f5135aba1166046f5b4bb5d129439308e4c2d339462ed guix-build-df9ebbf659d5/output/x86_64-linux-gnu/SHA256SUMS.part
410ad822e7b74648c5005d1378
...