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

🤔 rkrux reviewed a pull request: "wallet, rpc: Move (Un)LockCoin WalletBatch creation out of RPC"
(https://github.com/bitcoin/bitcoin/pull/32593#pullrequestreview-2892695590)
ACK 6135e05

```
git range-diff 11d0513...6135e05
```

The PR is a refactor that improves code readability and robustness upto some extent.

💬 bigspider commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#issuecomment-2935594462)
> Are your e2e tests running against the latest version of this branch?

The container is rebuilt every time a PR is merged, and the musig2 test never failed over the last 6+ months since they were added.

I rebuilt the container today and confirmed it works on the tagged commit for 2.4.0 release ([this](https://github.com/LedgerHQ/app-bitcoin-new/actions/runs/15418931868/job/43388642661) is the CI run).

> But when I pass a PSBT without the Bitcoin Core public nonce, it goes ahead and doe
...

💬 rkrux commented on pull request "rpc: Use type-safe HelpException":
(https://github.com/bitcoin/bitcoin/pull/32660#discussion_r2124061046)
TIL, ty!

💬 instagibbs commented on pull request "p2p: Add mutation check inside compact block's FillBlock":
(https://github.com/bitcoin/bitcoin/pull/32646#discussion_r2124068316)
done

💬 instagibbs commented on pull request "p2p: Add mutation check inside compact block's FillBlock":
(https://github.com/bitcoin/bitcoin/pull/32646#discussion_r2124083652)
Actually, this strategy is unneeded. Both merkle checks can fail due to collisions, and either may need full blocks fetched if they're from the first peer. Pushed a change that is simpler and cleaned up READ_STATUS_CHECKBLOCK_FAILED which is no longer necessary.

💬 Sjors commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#issuecomment-2935730736)
> If only some nonces are present, then the device assumes Round 1 already happened, but it will fail to run Round 2.

Ok, that explains issue (2).

But not why it failed with (1), because the policy variant with `older` fallback triggers the error without a public nonce from Bitcoin Core for the key path.

> The workflow I'm expecting is that the coordinator would give the same PSBT in parallel to all the signers, which execute round 1, then all nonces are added to the PSBT, and eah signe
...