💬 ismaelsadeeq commented on pull request "fuzz: Extend mini_miner fuzz coverage to max block weight":
(https://github.com/bitcoin/bitcoin/pull/31803#discussion_r1949993496)
Nice moving this here, prevents creating `block_adjusted_max_weight` variable in each loop iteration.

💬 hodlinator commented on pull request "logging: Ensure -debug=0/none behaves consistently with -nodebug":
(https://github.com/bitcoin/bitcoin/pull/31767#discussion_r1949981506)
Nice touch on adding a check for non-existence of abc!
https://github.com/bitcoin/bitcoin/blob/bb8309723cdd3ce0378fbe9708bd600dfaa157aa/test/functional/feature_logging.py#L113

💬 hodlinator commented on pull request "logging: Ensure -debug=0/none behaves consistently with -nodebug":
(https://github.com/bitcoin/bitcoin/pull/31767#discussion_r1949995455)
Seems like you forgot to remove this? (`last_negated` should theoretically be able to be marked `const`, but makes line go over 80 chars).
```suggestion
```

💬 achow101 commented on pull request "wallet: Automatically repair corrupted metadata with doubled derivation path":
(https://github.com/bitcoin/bitcoin/pull/29124#issuecomment-2649431826)
Updated this to work even after the legacy wallet is deleted.

💬 hodlinator commented on pull request "qa: Fix `wallet_multiwallet.py`":
(https://github.com/bitcoin/bitcoin/pull/31410#issuecomment-2649437799)
> > Attempted to run Guix build [e717fb5](https://github.com/bitcoin/bitcoin/commit/e717fb5a429d9d00e008fa1eb2b85179050be8fe) cross-built for Windows, but it fails already on test "0" / line 133:
>
> Is this failure introduced by this PR, or does it occur on the master branch as well?

I think the *way* cross-built binaries fail was new thanks to your introduced "0. Baseline, no errors."-test.

With the same executable (e717fb5a429d = this PR which only includes Python changes) on master
...

💬 davidgumberg commented on pull request "build: Make config warnings fatal if -DWCONFIGURE_ERROR=ON":
(https://github.com/bitcoin/bitcoin/pull/31665#issuecomment-2649486439)
Changed the `test each commit` CI job to use `-DWARN_INCOMPATIBLE_BDB=OFF`, otherwise it results in a fatal error if `-DWCONFIGURE_ERROR=ON`.

💬 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