📝 instagibbs opened a pull request: "p2p: Add mutation check inside compact block's FillBlock"
(https://github.com/bitcoin/bitcoin/pull/32646)
Since #29412, we have not allowed mutated blocks to continue being processed immediately the block is received, but this is only done for the legacy BLOCK message.

Extend these checks as belt-and-suspenders to not allow similar mutation strategies to affect relay by honest peers by applying the check inside
PartiallyDownloadedBlock::FillBlock, immediately before retruning READ_STATUS_OK.

This does result in a behavior change: a node giving a compact block that decodes successfully, but is
...

💬 instagibbs commented on pull request "p2p: Add mutation check inside compact block's FillBlock":
(https://github.com/bitcoin/bitcoin/pull/32646#issuecomment-2922447715)
cc @dergoegge @Crypt-iQ

💬 instagibbs commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2115983196)
bytes could mean serialized bytes, vbytes is less ambiguous, even if the term itself is more bespoke

💬 wilfred1005 commented on pull request "test: cover invalid codesep positions for signature in taproot":
(https://github.com/bitcoin/bitcoin/pull/32301#issuecomment-2922505039)
ISIKWEVWEN WILFRED OSARO VENTURE LTD

💬 instagibbs commented on pull request "test: cover invalid codesep positions for signature in taproot":
(https://github.com/bitcoin/bitcoin/pull/32301#issuecomment-2922510417)
rebased due to merge conflict

💬 instagibbs commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2922535795)
rebase required due to silent merge conflict

💬 purpleKarrot commented on pull request "build: add a depends dependency provider":
(https://github.com/bitcoin/bitcoin/pull/32595#issuecomment-2922613939)
> I can't tell from this if you are saying to use depends to "fill in" any missing system packages,

Yes. The idea is to provide an approach to build core with minimal effort. If you NACK mixing system packages with vendored dependencies, are you saying that nobody will ever want that, or that nobody *should* ever want that?

💬 theStack commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-2922647658)
> > OpenBSD are not supported yet, the latter due to a fairly trivial upstream issue.
>
> If it's trivial, I'd say you should pull the patch into depends, rather than opting into a different default behaviour for an entire platform?

Looks like there is no working patch available yet, as the currently proposed PR (https://github.com/capnproto/capnproto/pull/1907) has some blocking issues that are still untackled (https://github.com/capnproto/capnproto/pull/1907#discussion_r1452594740, https
...

💬 Sjors commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-2922674669)
I dropped the word "trivial" from the description and instead linked to the PR.

💬 hebasto commented on pull request "deps: Bump lief to 0.16.6":
(https://github.com/bitcoin/bitcoin/pull/32431#issuecomment-2922695159)
My Guix build:
```
aarch64
74539281616c591d2b7157152ad5959cb7a68b9505ea3fcb74004fb19df34ca4 guix-build-240fe8754d21/output/aarch64-linux-gnu/SHA256SUMS.part
08a9f2153670c0a07819b73694857f448baffdea276f94e2be6e24732bfd80ef guix-build-240fe8754d21/output/aarch64-linux-gnu/bitcoin-240fe8754d21-aarch64-linux-gnu-debug.tar.gz
70026671b2e698c28d11426d35da71422b4163b52524a3c4442f7dd2606f1c84 guix-build-240fe8754d21/output/aarch64-linux-gnu/bitcoin-240fe8754d21-aarch64-linux-gnu.tar.gz
376d53e4
...

💬 fanquake commented on pull request "build: add a depends dependency provider":
(https://github.com/bitcoin/bitcoin/pull/32595#issuecomment-2922745196)
> Yes. The idea is to provide an approach to build core with minimal effort.

I think `make -C depends && cmake -B --toolchain` and `apt install your_dependencies && cmake -B` are both already pretty minimal effort. It's not really clear why a developer would `apt install x`, but not `apt install x y`, and then would need a behaviour to fallback to depends for `y`?

I'm also not currently aware of a platform that we support, where the dependencies needed to compile Core, aren't available vi
...

💬 fanquake 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-2922748283)
Guix Build:
```bash
c118e9965e3dc46a58e78ca4bb1573bf799d10b0a5a33c978ed5873801ded1e4 guix-build-51f6aa8ac47b/output/aarch64-linux-gnu/SHA256SUMS.part
abe0a4b2fee4bc928b73a0d8bfaeb7441a45afc01147f71ec76339844fa1a502 guix-build-51f6aa8ac47b/output/aarch64-linux-gnu/bitcoin-51f6aa8ac47b-aarch64-linux-gnu-debug.tar.gz
444bccc99b7a8910b62aef674ac9a0249f895c78ed19268017651eefeef5abd2 guix-build-51f6aa8ac47b/output/aarch64-linux-gnu/bitcoin-51f6aa8ac47b-aarch64-linux-gnu.tar.gz
ec1a9597e32d60a8
...

📝 fanquake opened a pull request: "build: add -Wthread-safety-pointer"
(https://github.com/bitcoin/bitcoin/pull/32647)
This will become available, and on by default in Clang 21:

> ThreadSafetyAnalysis now supports -Wthread-safety-pointer, which
> enables warning on passing or returning pointers to guarded variables
> as function arguments or return value respectively. Note that
> ThreadSafetyAnalysis still does not perform alias analysis. The
> feature will be default-enabled with -Wthread-safety in a future release.

See https://github.com/llvm/llvm-project/blob/main/clang/docs/ReleaseNotes.rst.

Upd
...

💬 Sjors commented on pull request "cmake: Move internal binaries from bin/ to libexec/":
(https://github.com/bitcoin/bitcoin/pull/31679#issuecomment-2922787970)
tACK d253fa9ebe4305118417a02aa72cc06810662ac6

I tested on top of af227a0e7d9754623d36925c56599f9b921f5cb3 from #31802 on macOS 15.5 and Windows 11 (not impacted).

The `bitcoin test` command is perhaps a bit too well hidden. Maybe it's better to drop the `help` command and put a bit more emphasis on the `--all` option:

```
Commands:
gui [ARGS] Start GUI, equivalent to running 'bitcoin-qt [ARGS]' or 'bitcoin-gui [ARGS]'
...
tx ...

Use 'bitcoin --help --all' for additional
...

💬 stickies-v commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2922814814)
re-ACK a189d636184b1c28fa4a325b56c1fab8f44527b1

No changes since 35bcd8eed38130445aef5ebe217ab42248fa6f18 except rebasing and removing the `-datacarriersize=100000` no-op in `mempool_updatefromblock.py` that would cause the test to fail just for using a deprecated option.

💬 Crypt-iQ commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2116212200)
Yup I had thought about this and wasn't sure how to evaluate the overhead. I'll change it.

💬 Crypt-iQ commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2116215143)
Yup, this makes sense as well. Can change.

💬 Crypt-iQ commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2116217752)
I think callers just need to be aware that they cannot set the rate limit very high in certain cases. Maybe this change and a comment will suffice?

💬 Sjors commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2922826515)
re-ACK a189d636184b1c28fa4a325b56c1fab8f44527b1

Just a rebase and minor test tweak since my previous review.

💬 polespinasa commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2922833884)
re-ACK a189d636184b1c28fa4a325b56c1fab8f44527b1

Just a rebase due to silent merge conflict since my prev review