🤔 maflcko reviewed a pull request: "Fix windows libc++ `fs::path` `fstream` compile errors"
(https://github.com/bitcoin/bitcoin/pull/33550#pullrequestreview-3359282710)
re-ACK c864a4c1940d682f7eb6fdb3b91b18d638b59330 🌥

<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: re-ACK c864a4c1940d682f7eb6
...

💬 maflcko commented on pull request "Fix windows libc++ `fs::path` `fstream` compile errors":
(https://github.com/bitcoin/bitcoin/pull/33550#discussion_r2447142735)
nit in c864a4c1940d682f7eb6fdb3b91b18d638b59330: I understand that you probably used the std namespace here and then used `auto` to silence the linter? However, this seems to go in the wrong direction. If we think our `fs::` namespace should be preferred over the `std::` fs one, we should try to use it as much as possible and only call `std_path` when needed. I guess restoring `fs::path` here would require some overloads in the wrapper class, or restoring some `fs::path` constructors below. But
...

💬 alexanderwiederin commented on pull request "kernel: Introduce C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-3425341211)
re-ACK https://github.com/bitcoin/bitcoin/pull/30595/commits/edf99b88e644c7d2a2db434c8db298c9c5303cf9

💬 fanquake commented on pull request "wallet: refactor to remove redundant sighash calculation":
(https://github.com/bitcoin/bitcoin/pull/33665#issuecomment-3425407567)
https://github.com/bitcoin/bitcoin/actions/runs/18676072881/job/53246386885?pr=33665#step:9:3427:
```bash
Run script_sign with args ['/home/admin/actions-runner/_work/_temp/build/bin/fuzz', '-runs=1', PosixPath('/home/admin/actions-runner/_work/_temp/ci/scratch/qa-assets/fuzz_corpora/script_sign')]INFO: Running with entropic power schedule (0xFF, 100).
INFO: Seed: 2902865681
INFO: Loaded 1 modules (615865 inline 8-bit counters): 615865 [0x56e10e6f9bb8, 0x56e10e790171),
INFO: Loaded 1 PC
...

💬 fanquake commented on pull request "build: Move CMAKE_SKIP_INSTALL_RPATH from CMake to Guix script":
(https://github.com/bitcoin/bitcoin/pull/33470#issuecomment-3425433100)
Guix Build
```bash
0dd1c0d4567f1eabfa23b88b1d26ca584ac97381f9572abaff3e0ad37f3b7ecf guix-build-4b41f99d57d8/output/aarch64-linux-gnu/SHA256SUMS.part
75bf3226050a5aad14bcf59b55dbf8b59ad7c287389c8ef42161bbff6eea070e guix-build-4b41f99d57d8/output/aarch64-linux-gnu/bitcoin-4b41f99d57d8-aarch64-linux-gnu-debug.tar.gz
d45f01491dd6254066689e9516a3c96aee30ea6f60648f06dfdb8c364194908e guix-build-4b41f99d57d8/output/aarch64-linux-gnu/bitcoin-4b41f99d57d8-aarch64-linux-gnu.tar.gz
14338708b2ba862f2
...

💬 maflcko commented on pull request "doc: add AGENTS.md":
(https://github.com/bitcoin/bitcoin/pull/33662#issuecomment-3425448023)
lgtm ACK 08cf213ec7e671dd4162d4ae4d0d20810547a444

Seems harmless to add and could be useful information when it is present.

💬 instagibbs commented on pull request "p2p: implement sender-initiated package relay":
(https://github.com/bitcoin/bitcoin/pull/33500#discussion_r2406742176)
wer're sending the parent here, I don't think we're marking the parent as in the known inventory, which means we will keep sending it even if it's just new children, if f.e. the parent is 0-fee.

🤔 instagibbs reviewed a pull request: "p2p: implement sender-initiated package relay"
(https://github.com/bitcoin/bitcoin/pull/33500#pullrequestreview-3305146357)
just dropping this before I forget

🚀 fanquake merged a pull request: "build: Move CMAKE_SKIP_INSTALL_RPATH from CMake to Guix script"
(https://github.com/bitcoin/bitcoin/pull/33470)

💬 fanquake commented on pull request "build: Remove CMAKE_SKIP_BUILD_RPATH and SKIP_BUILD_RPATH settings":
(https://github.com/bitcoin/bitcoin/pull/33247#issuecomment-3425470273)
Please rebase. Please also remove all the (what looks like AI generated) text from the commit messages & PR description. You should also post your Guix build result.

💬 fanquake commented on pull request "fix: invalid plist format and update values to meet macOS 1.0 standard":
(https://github.com/bitcoin/bitcoin/pull/33654#discussion_r2447310112)
> the correct minimum version is 10.14.0

Our minimum supported version is 14.0.

✅ fanquake closed a pull request: "fix: invalid plist format and update values to meet macOS 1.0 standard"
(https://github.com/bitcoin/bitcoin/pull/33654)

💬 optout21 commented on pull request "test: Add test on skip heights in CBlockIndex":
(https://github.com/bitcoin/bitcoin/pull/33661#issuecomment-3425525600)
> For reference https://maflcko.github.io/b-c-cov/test_bitcoin.coverage/src/chain.cpp.gcov.html is pretty well covered otherwise, but these tests can help with the specifics.

Thanks for the link! Indirectly it is tested extensively, as the skip mechanism is used while searching in the chain. However, there are no tests focusing on this specifically, and the why and how on the exact skip generation algorithm is not documented.

💬 maflcko commented on pull request "ci: detect outbound internet traffic generated while running tests":
(https://github.com/bitcoin/bitcoin/pull/31349#issuecomment-3425621674)
> I can't reproduce locally.

Are you sure? Above I tried on Fedora podman-docker, and today it also failed on Ubuntu podman-docker:

```
(root)# docker --version
Emulate Docker CLI using podman. Create /etc/containers/nodocker to quiet msg.
podman version 4.9.3

💬 glozow commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2447496922)
> I also considered having all cluster size limits being treated as RECONSIDERABLE, but decided that is also not great, partly because we certainly shouldn't let a transaction whose cluster size just with its own unconfirmed ancestors be treated as RECONSIDERABLE

👍 definitely think we should not allow reconsideration in that case

Sounds good to me to make it non-reconsiderable for now, and maybe change it in the future in a consistent way. I agree it's possible for a child to make a tx no
...

💬 glozow commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2447498384)
Great, marking resolved and will refer to this in a followup!

💬 ajtowns commented on pull request "node: change a tx-relay on/off flag to enum":
(https://github.com/bitcoin/bitcoin/pull/33567#discussion_r2447506448)
(Adding `using enum TxBroadcastMethod` would keep the type safety without requiring the prefix)

💬 waketraindev commented on pull request "doc: add AGENTS.md":
(https://github.com/bitcoin/bitcoin/pull/33662#issuecomment-3425662444)
Why not simply create a label to flag them if their behavior is causing offense? What’s the purpose of disclosure, particularly if it’s an inadvertent one the user didn’t intend or consent to?

📝 maflcko opened a pull request: "ci: Drop libFuzzer from msan fuzz task"
(https://github.com/bitcoin/bitcoin/pull/33666)
libFuzzer is mostly unmaintained (https://llvm.org/docs/LibFuzzer.html#status), and it isn't really needed by the CI tasks. While it provides some additional stats like rss or the max input byte size, they are not essential.

Also, there seems to be a history of intermittent false-positive msan warnings (https://github.com/bitcoin/bitcoin/pull/33600#issuecomment-3391921802).

It is unclear what exactly is causing the false-positives, so just disable libFuzzer in this task for now, to work ar
...

💬 optout21 commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#issuecomment-3425689192)
Concept ACK c9bfd298be422de7e989fe244fb4281c507068a3