💬 stickies-v commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2115826232)
I think it's unintuitive that this function returns `true` when the operation failed. It seems like the rate limiting can be carved out pretty much as-is into a separate `bool NeedsRateLimiting()` function? There are only 2 callsites of `FormatLogStrAndRateLimit`, and the first one (`StartLogging`) doesn't look like it needs rate limiting because we already have a max buffer size?

💬 stickies-v commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2115869345)
nit: It would be nice if we could catch this in debug builds (ideally even at a fraction, say 20% of the limit), to help inform if our default limit needs to change. `Assume()` is an option, but that would probably break unit tests in debug builds.

nit, because 1MB is small enough that it should prevent issues even on devices with little disk size (unless an attacker finds multiple log vulnerabilities), and large enough that no single unconditional logging statement should ever hit that in t
...

💬 fanquake commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2115877367)
@dergoegge might remember why this was added?

💬 stickies-v commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2115895206)
fwiw: I can't find any discussion about it in https://github.com/bitcoin/bitcoin/pull/21603, and the option was present from the first version of the PR (https://github.com/bitcoin/bitcoin/commit/4648b6d207139ec0ab2994f56c0a47f81cdf516a#diff-b1e19192258d83199d8adaa5ac31f067af98f63554bfdd679bd8e8073815e69d)

💬 polespinasa commented on pull request "rpc: generateblock to allow multiple outputs":
(https://github.com/bitcoin/bitcoin/pull/32468#issuecomment-2922380999)
Rebased on top of master.

2c855694c795a31f13a473d6a6eb80c5d765e8ed implements allowing single address or descriptor without using an array or json structure as sugested in https://github.com/bitcoin/bitcoin/pull/32468#discussion_r2112898168 and https://github.com/bitcoin/bitcoin/pull/32468#discussion_r2097722076. But makes `rpc_help.py` fail. (I still don't know why)

💬 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-2922397804)
> Would be nice to explain this with commands to test and observe the difference in behavior. This would make it easier to understand the goals and test them.

Sure. On master (4b1d48a6866b24f0ed027334c6de642fc848d083):
```bash
# flags in C(XX)FLAGS
make -C depends print-linux_CFLAGS
linux_CFLAGS=-pipe -std=c11
make -C depends print-linux_CXXFLAGS
linux_CXXFLAGS=-pipe -std=c++20
```

make -C depends/ -j20 NO_QT=1 NO_WALLET=1 NO_ZMQ=1 NO_USDT=1
```bash
# base cflags (-pipe -std=c11)
...

🤔 TheCharlatan reviewed a pull request: "Add bitcoin-{node,gui} to release binaries for IPC"
(https://github.com/bitcoin/bitcoin/pull/31802#pullrequestreview-2881363308)
Concept ACK

Guix builds (aarch64)
```
eb7c84cea8ea007cba8605b256f176cfc2c4ee0229fd93a80fca0e0d5fe2f71d guix-build-af227a0e7d97/output/aarch64-linux-gnu/SHA256SUMS.part
b41de2faee47dee6947724e0bbe0dfb69fd91f7a1eb571956b03adda51c73f73 guix-build-af227a0e7d97/output/aarch64-linux-gnu/bitcoin-af227a0e7d97-aarch64-linux-gnu-debug.tar.gz
dfff9d1b7989a31dfa762195c65e09b6427d99bf97bc774fcadf6ed86be4ddbc guix-build-af227a0e7d97/output/aarch64-linux-gnu/bitcoin-af227a0e7d97-aarch64-linux-gnu.ta
...

💬 fanquake commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-2922429542)
> 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?

📝 theStack opened a pull request: "fs: use `ftruncate` in `AllocateFileRange` on OpenBSD"
(https://github.com/bitcoin/bitcoin/pull/32645)
Closes issue #32643.

Tested on OpenBSD 7.7 (amd64) by running a signet IBD, with the following patch on top to ensure the fallback isn't executed anymore:
```diff
diff --git a/src/util/fs_helpers.cpp b/src/util/fs_helpers.cpp
index 84ac3690c9..c6dff2d29d 100644
--- a/src/util/fs_helpers.cpp
+++ b/src/util/fs_helpers.cpp
@@ -208,6 +208,7 @@ void AllocateFileRange(FILE* file, unsigned int offset, unsigned int length)
// OpenBSD doesn't have fallocate or posix_fallocate, use ftruncat
...

💬 fanquake commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-2922437173)
cc @theStack ^

📝 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
...