💬 theStack commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2123779414)
in commit 9e3a1d05bf066df31a6e124638046e70669eb470: should this be
```suggestion
if (!all_bip32) {
error = "musig(): musig() derivation requires all participants to be xpubs";
```
instead? Ranged would mean ending with `/*` IIUC, which is not necessarily the case.

💬 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_r2123834199)
I believe that if somebody is using `-debug=bench` or `-debug=leveldb`, the fact that they are using a category should bypass the rate limit if `LogDebug` is used. I think the mitigation should be in place whether or not we're in IBD -- if we have any inbound peers, we need to be cautious.

💬 maflcko commented on pull request "doc: Remove build instruction for running `clang-tidy`":
(https://github.com/bitcoin/bitcoin/pull/32662#issuecomment-2935293250)
Shouldn't this be excluded in the tidy CI task as well?

Also, would be good to mention that this will now print errors about the missing auto-generated files.

⚠️ fanquake opened an issue: "guix: nsis (Windows installer creator) is broken upstream"
(https://github.com/bitcoin/bitcoin/issues/32674)
This will become a issue when we want to bump the time-machine next, and it'd be nicer if this is fixed and shipped upstream, rather than us having to work around the issue:

```bash
# guix --version
guix (GNU Guix) 4b9d14378fcc3d8dd4eea36b541fe87e198fd7b8

# guix build --no-substitutes nsis-x86_64
Source/Plugins.cpp:278:18: required from ‘static void PrintPluginDirsHelper::print(const C&, const char*) [with C = std::map<std::__cxx11::basic_string<wchar_t>, std::__cxx11::basic_string<wchar_t>,
...

💬 pinheadmz commented on pull request "p2p: protect addnode peers during IBD":
(https://github.com/bitcoin/bitcoin/pull/32051#issuecomment-2935375333)
@achow101 re: slow preferred peer "causing congestion" -- wouldn't this only affect the single user who explicitly set the preferred peer?

💬 pinheadmz commented on issue "Potential data race on fHavePruned flag":
(https://github.com/bitcoin/bitcoin/issues/21627#issuecomment-2935386395)
I haven't been focusing on this, wanna take a stab at it?

💬 hebasto commented on pull request "ci, iwyu: Treat warnings as errors for specific directories":
(https://github.com/bitcoin/bitcoin/pull/31308#discussion_r2123924524)
> Is there an issue open for this upstream?

Please see https://github.com/include-what-you-use/include-what-you-use/issues/1763.

💬 hebasto commented on pull request "ci, iwyu: Treat warnings as errors for specific directories":
(https://github.com/bitcoin/bitcoin/pull/31308#discussion_r2123928871)
The issue could be worked around in different ways. For example, by using an OS image with the default GCC >= 14.

💬 hebasto commented on issue "guix: nsis (Windows installer creator) is broken upstream":
(https://github.com/bitcoin/bitcoin/issues/32674#issuecomment-2935438865)
Does it depend on the build machine architecture?

💬 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_r2123954582)
See https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2115570067, will change. Imo, having to determine whether we are in IBD or have zero peer connections is heading into too much complexity. This PR shouldn't cause IBD debug logs to be rate-limited, but if it does we should fix that.

💬 pinheadmz commented on pull request "tests: Expand HTTP coverage to assert libevent behavior":
(https://github.com/bitcoin/bitcoin/pull/32408#issuecomment-2935446085)
Friendly ping @fjahr and @polespinasa for reviewing some HTTP tests ? <3

💬 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_r2123959106)
This never gets cleared. I haven't calculated the overhead of the map, but it could make sense to clear it periodically.

💬 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_r2123974464)
note: this is part of my suggestion in https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2115841360

💬 kevkevinpal commented on pull request "test: added fuzz coverage for consensus/merkle.cpp":
(https://github.com/bitcoin/bitcoin/pull/32243#discussion_r2123975697)
ok I pushed and seems like the cirrus ci fuzzer had more coverage than my local machine did
`[12:12:07.876] merkle #156207 DONE cov: 2645 ft: 12427 corp: 710/97Kb lim: 301 exec/s: 2560 rss: 212Mb`

💬 kevkevinpal commented on pull request "test: added fuzz coverage for consensus/merkle.cpp":
(https://github.com/bitcoin/bitcoin/pull/32243#issuecomment-2935484202)
> I pushed [caaed67](https://github.com/bitcoin/bitcoin/pull/32243/commits/caaed67232f701341d98cc14d8a014b8c232e2ff) which does not include `ConsumeTransaction` but I am still trying to see if this will add more coverage compared to [f99fd0b](https://github.com/bitcoin/bitcoin/commit/f99fd0b87dcfaf84784ce423f78a950ad377b36b)

Looks like the runner had significant coverage, should be good for review

`[12:12:07.876] merkle #156207 DONE cov: 2645 ft: 12427 corp: 710/97Kb lim: 301 exec/s: 2560
...

🤔 hodlinator reviewed a pull request: "p2p: protect addnode peers during IBD"
(https://github.com/bitcoin/bitcoin/pull/32051#pullrequestreview-2892483202)
Concept ACK 93b07997e9a38523f5ab850aa32ca57983fd2552

💬 hodlinator commented on pull request "p2p: protect addnode peers during IBD":
(https://github.com/bitcoin/bitcoin/pull/32051#discussion_r2123888732)
Is there any risk that this log message becomes spammy if we keep the peer connected? - Should we do something like reset `state.m_downloading_since` to `current_time`, clear `state.vBlocksInFlight` or add another `state`-field to throttle the log?

💬 ryanofsky commented on issue "upstream: capnp V2 doesn't support compilation with GCC (yet?)":
(https://github.com/bitcoin/bitcoin/issues/32669#issuecomment-2935513870)
Thanks for finding this! I think best source of information about the v2 branch is probably https://capnproto.org/news/#whats-planned-for-20

My impression about the branch is that it's not intending to implement many significant changes, but more to be an API break and allow cleaning up some things like promise cancellations and EOF handling that they haven't been able to fix due to wanting to preserve backwards compatibility. They are also requiring C++20 and dropping support for -fno-exceptio
...

👍 willcl-ark approved a pull request: "depends: use "mkdir -p" when installing xproto"
(https://github.com/bitcoin/bitcoin/pull/32568#pullrequestreview-2892672240)
ACK df9ebbf659d5d1282289f36d7f9ee7103aa33a17

Sorry, my guix setup was to blame here. This is now fixed and I finally get matching hashes:

```
src/core/bitcoin on  pr-32568 [$] via △ v3.31.6 via 🐍 v3.12.10 via ❄️ impure (nix-shell-env) took 1h13m34s
❯ find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
c07d19f1279ac1255fff06fff094eea425aa2d228b2cf4fef8ffa366fac344dc guix-build-df9ebbf659d5/output/aarch64-linux-gnu/SHA2
...

💬 tnndbtc commented on pull request "fee estimate test: fix #31944 by handling a legitimate scenario that …":
(https://github.com/bitcoin/bitcoin/pull/32615#discussion_r2124010335)
@maflcko Oh, you mean the data point I generated. I ran the original test with --randomseed=298652989016148828, which is a good randomseed, and output the calculated result of **fee**:

rand_fee = float(fee_increment) * (1.1892 ** random.randint(0, 28))

fee = min_fee - fee_increment + satoshi_round(rand_fee, rounding=ROUND_DOWN) # output this fee as the data point that can be used to generate deterministic result.

Then, for each calling of function **small_txpuzzle_randfee** , s
...