💬 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

💬 maflcko commented on pull request "refactor: Construct g_verify_flag_names on first use":
(https://github.com/bitcoin/bitcoin/pull/33600#issuecomment-3425693525)
For reference, the msan false-positive can also be worked around via https://github.com/bitcoin/bitcoin/pull/33666. However, I still think this pull makes sense, because it is already reviewed and still comes with the other mentioned benefits at basically no cost. (If there were a cost, we could go with the compile-time stuff, but that diff is a bit larger, see above).

💬 maflcko commented on pull request "test: Run bench sanity checks in parallel with functional tests":
(https://github.com/bitcoin/bitcoin/pull/33142#issuecomment-3425916353)
(rebased)