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

💬 vasild commented on pull request "util: explicitly close all AutoFiles that have been written":
(https://github.com/bitcoin/bitcoin/pull/29307#discussion_r2102112756)
Note that this here is in a bench test where we operate on a real file and an error to close it means something really went wrong badly, e.g. a disk IO error. Such a condition should stop the test with a failure.

In contrast, in fuzz tests some of the "files" we operate on are fuzzed files which mean they are not really backed on disk and their operations are mocked to sometimes return an error, e.g. to simulate a broken disk. Those mocked errors are to be expected and should not make the tes
...

🤔 Sjors reviewed a pull request: "descriptors: MuSig2"
(https://github.com/bitcoin/bitcoin/pull/31244#pullrequestreview-2860429843)
Some more questions about 35db4f2dcfc3435e10935581ffa447ffe219cc1e.

💬 Sjors commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2102121605)
In 35db4f2dcfc3435e10935581ffa447ffe219cc1e "descriptor: Add MuSigPubkeyProvider": could avoid old school `for i`, make it slightly shorter and have the intention be more clear:

```diff
--- a/src/script/descriptor.cpp
+++ b/src/script/descriptor.cpp
@@ -691,9 +691,9 @@ public:
std::string ToString(StringType type=StringType::PUBLIC) const override
{
std::string out = "musig(";
- for (size_t i = 0; i < m_participants.size(); ++i) {
- const auto& pub
...

💬 Sjors commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2102052043)
In 35db4f2dcfc3435e10935581ffa447ffe219cc1e "descriptor: Add MuSigPubkeyProvider": no tests break if I delete this loop. What's the typical use case for `Clone()`? Maybe `Check()` in `descriptor_tests.cpp` should do this for all valid descriptors?

💬 Sjors commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2102178194)
In 35db4f2dcfc3435e10935581ffa447ffe219cc1e "descriptor: Add MuSigPubkeyProvider": just to check my own understanding, by ranged participants you mean the case of `musig(alice/*, bob)`, i.e. derivation before aggregation?

I assume that if the participants are ranged, then you can't also have a range after derivation, e.g. `musig(alice/*, bob)/*` isn't allowed. Where do we check this?

💬 vasild commented on pull request "util: explicitly close all AutoFiles that have been written":
(https://github.com/bitcoin/bitcoin/pull/29307#discussion_r2102187863)
Fuzzed `FILE*`, which comes from `fuzzed_file_provider.open()` is expected to have a failing `fclose()`. Should not fail the test for that, I replied there: https://github.com/bitcoin/bitcoin/pull/29307#discussion_r2102112756

💬 hebasto commented on pull request "depends: hard-code necessary c(xx)flags rather than setting them per-host":
(https://github.com/bitcoin/bitcoin/pull/32584#issuecomment-2900712416)
Concept ACK.

💬 hebasto commented on pull request "subprocess: Handle quoted tokens properly":
(https://github.com/bitcoin/bitcoin/pull/32577#issuecomment-2900733155)
> What is the longer term plan here?

The initial plan was to maintain our own code independently of the upstream repository. However, there have been a few requests to upstream our changes. At this point, I still believe we shouldn't concern ourselves with upstream and should instead take full advantage of C++20.

💬 fanquake commented on pull request "random: Use modern Windows randomness functions":
(https://github.com/bitcoin/bitcoin/pull/32400#issuecomment-2900742217)
Guix Build:
```bash
d9e24e61f22de8b9d83cbbec4830bdb9192ab55ac90aba125a53ec9fba719d67 guix-build-6b4bcc162345/output/aarch64-linux-gnu/SHA256SUMS.part
9780961f5ce0c23db0f6ec4b9930728344a3cb4706e4a36dff2364f12905acf8 guix-build-6b4bcc162345/output/aarch64-linux-gnu/bitcoin-6b4bcc162345-aarch64-linux-gnu-debug.tar.gz
2f281a8e266bab0631e40a738db21a4f4865aa2e91e9dfadc2d208b05d5a2f8b guix-build-6b4bcc162345/output/aarch64-linux-gnu/bitcoin-6b4bcc162345-aarch64-linux-gnu.tar.gz
1639f2ab6e353d5a
...

💬 vasild commented on pull request "util: explicitly close all AutoFiles that have been written":
(https://github.com/bitcoin/bitcoin/pull/29307#issuecomment-2900799235)
`9057ab8b80...c7b68dceb2`: address suggestions

💬 vasild commented on pull request "util: explicitly close all AutoFiles that have been written":
(https://github.com/bitcoin/bitcoin/pull/29307#discussion_r2102277623)
Done, patch taken. Thanks!

💬 vasild commented on pull request "util: explicitly close all AutoFiles that have been written":
(https://github.com/bitcoin/bitcoin/pull/29307#discussion_r2102278565)
Done.