💬 achow101 commented on pull request "test: expect that files may disappear from /proc/PID/fd/":
(https://github.com/bitcoin/bitcoin/pull/31614#issuecomment-2649491412)
ACK b2e9fdc00f5c40c241a37739f7b73b74c2181e39

💬 achow101 commented on pull request "test: Fuzz the human-readable part of bech32 as well":
(https://github.com/bitcoin/bitcoin/pull/30623#issuecomment-2649499608)
ACK 9b7023d31a3ec95f66b45f0ecb47e79762d74854

🚀 achow101 merged a pull request: "test: expect that files may disappear from /proc/PID/fd/"
(https://github.com/bitcoin/bitcoin/pull/31614)

🚀 achow101 merged a pull request: "test: Fuzz the human-readable part of bech32 as well"
(https://github.com/bitcoin/bitcoin/pull/30623)

📝 crStiv opened a pull request: "streams: Add stream position validation in BufferedFile::AdvanceStream"
(https://github.com/bitcoin/bitcoin/pull/31839)
Description:
Add safety check in BufferedFile::AdvanceStream to prevent potential integer overflow when calculating bytes_until_source_pos. This ensures that m_read_pos never exceeds nSrcPos, which could lead to undefined behavior.

The change:
- Adds validation before calculating (nSrcPos - m_read_pos)
- Throws std::ios_base::failure if invalid state is detected

💬 ryanofsky commented on pull request "multiprocess: Add capnp wrapper for Chain interface":
(https://github.com/bitcoin/bitcoin/pull/29409#issuecomment-2649739813)
Rebased 64b833854a34d87cde4e0ca4173d75012c401a7a -> 4fa7f72cb9d8f0290f56293989ec6ea950162c5b ([`pr/ipc-chain.12`](https://github.com/ryanofsky/bitcoin/commits/pr/ipc-chain.12) -> [`pr/ipc-chain.13`](https://github.com/ryanofsky/bitcoin/commits/pr/ipc-chain.13), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/ipc-chain.12-rebase..pr/ipc-chain.13)) due to various conflicts.

@pseudoramdom, with this update, errors you posted should be resolved

💬 rhysbeynon commented on pull request "Updated MacOS icon to more closely fit Apple's design standards":
(https://github.com/bitcoin-core/gui/pull/852#issuecomment-2649860867)
> > The .icns file was compiled using Apple's own "iconutil" cli program.
>
> What are steps to reproduce the new `src/qt/res/icons/bitcoin.icns` file?

I cloned bitcoin.icns, used the command `iconutil -c iconset bitcoin.icns` to convert the assembled icon file into a png IconSet. I then used Affinity Photo to make programmable size changes to each png identically. With the changed icon set, I then used `iconutil -c icns bitcoin.iconset` to assemble the modified .icns file. This process is
...

✅ maflcko closed a pull request: "streams: Add stream position validation in BufferedFile::AdvanceStream"
(https://github.com/bitcoin/bitcoin/pull/31839)

💬 maflcko commented on pull request "streams: Add stream position validation in BufferedFile::AdvanceStream":
(https://github.com/bitcoin/bitcoin/pull/31839#issuecomment-2649979750)
The prior assert with the exact same check covers this. This is adding dead code.

Is this AI generated?

💬 maflcko commented on pull request "build: Make config warnings fatal if -DWCONFIGURE_ERROR=ON":
(https://github.com/bitcoin/bitcoin/pull/31665#issuecomment-2649985978)
I still think there are two errors in the pull description, see https://github.com/bitcoin/bitcoin/pull/31665#issuecomment-2610142158

💬 maflcko commented on pull request "qa: Fix `wallet_multiwallet.py`":
(https://github.com/bitcoin/bitcoin/pull/31410#issuecomment-2650006905)
re-ACK 9025657baaf99fcf630cc1a37baec11b072196fa only change is adding a warning log for the skipped test 🥋

<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+krxU1A3Yux4bpwZNL
...

💬 maflcko commented on pull request "contrib: fix `test_deterministic_coverage.sh` script for out-of-tree builds":
(https://github.com/bitcoin/bitcoin/pull/31588#issuecomment-2650025151)
> Got a big diff, indicating a lot of test non-determinism:

I haven't looked in detail, but I presume much is due to randomness. On one hand, one could go with https://github.com/bitcoin/bitcoin/pull/31486 for the unit tests as well. On the other hand, some unit tests are fuzz tests (drawing inputs from a random seed, which is recorded), so they'd be a bit more limited if the randomness was made deterministic. But I think either way is fine.

💬 maflcko commented on pull request "test: add missing sync to p2p_tx_download.py":
(https://github.com/bitcoin/bitcoin/pull/31837#discussion_r1950370246)
There are not many remaining cases of `send_message`, so in theory it could be renamed to `send_without_ping` to make the misuse more apparent? Though, that'd be a follow-up.

💬 maflcko commented on pull request "test: add missing sync to p2p_tx_download.py":
(https://github.com/bitcoin/bitcoin/pull/31837#issuecomment-2650037065)
lgtm ACK 8fe552fe6e0f1fb3600d4c5bc53b4873def6e94e

💬 Sjors commented on pull request "ci: build multiprocess on most jobs":
(https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2650047665)
Same error, but seems to happen at a later step:

```
[15:53:41.937] [ 67%] Linking CXX executable bitcoin-node
[15:53:41.939] cd /ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu/src && /usr/bin/cmake -E cmake_link_script CMakeFiles/bitcoin-node.dir/link.txt --verbose=1
[15:53:41.955] /usr/bin/clang++-20 -ftrivial-auto-var-init=pattern -O2 -g -fsanitize=fuzzer,address,undefined,float-divide-by-zero,integer -fstack-protector-all -fcf-protection=full -fstack-clash-protection -Wl,-z,rel
...

💬 laanwj commented on pull request "Limit retries in GetRNDRRS to avoid infinite loop":
(https://github.com/bitcoin/bitcoin/pull/31826#discussion_r1950400146)
yielding a 1000 timeshares to check if the RNG is working is probably overkill, i'd say if it fails like, 10 times in a row that's already enough indication we shouldn't use it?

💬 laanwj commented on pull request "Limit retries in GetRNDRRS to avoid infinite loop":
(https://github.com/bitcoin/bitcoin/pull/31826#discussion_r1950408783)
As we already use `GetRNDR` in `SeedHardwareFast` which is called way more often than `SeedHardwareSlow` (including *in* `SeedSlow`) i'm not sure if calling it here has any additional value. Let's just skip this loop if rndrrs isn't supported.

💬 laanwj commented on pull request "Limit retries in GetRNDRRS to avoid infinite loop":
(https://github.com/bitcoin/bitcoin/pull/31826#issuecomment-2650101231)
Please update the PR title to reflect the new set of changes.

💬 maflcko commented on issue "Networking tests fail on emulated big-endian systems":
(https://github.com/bitcoin/bitcoin/issues/31812#issuecomment-2650105950)
Ah, I see you are manually running the tests. I thought you were running the s390x ci task.

In the task, the rpc_bind test is excluded:

https://github.com/bitcoin/bitcoin/blob/79f02d56ef78c22caff39713d660cabf54d452ac/ci/test/00_setup_env_s390x.sh#L14

The comment links to https://github.com/bitcoin/bitcoin/issues/17765#issuecomment-602068547

So the exclusion back then may have been due to a permission error on aarch64 or s390x, looking at the discussion. Also, the qemu wrapping back then was
...

💬 Sjors commented on pull request "guix: Notarize MacOS app bundle and codesign all MacOS and Windows binaries":
(https://github.com/bitcoin/bitcoin/pull/31407#issuecomment-2650126441)
Guix hashes for macOS and Windows, before code-sign:

```
bd11279800610b70a1feed4451d9fa3a137ed75f26c794363c5ef82d820c592e guix-build-46e44a35b858/output/arm64-apple-darwin/SHA256SUMS.part
3a911d1a009222c042a84864dccd58462ce3167ea5f407830bf98d370fa356e4 guix-build-46e44a35b858/output/arm64-apple-darwin/bitcoin-46e44a35b858-arm64-apple-darwin-codesigning.tar.gz
2b1c4d096f27ae2a650ce3fe954304c53bb51b7ddac5dd533cabcc862a75f684 guix-build-46e44a35b858/output/arm64-apple-darwin/bitcoin-46e44a
...