💬 hodlinator commented on pull request "merkle: pre‑reserve leaves to prevent reallocs with odd vtx count":
(https://github.com/bitcoin/bitcoin/pull/32497#discussion_r2576533841)
> `ComputeMerkleRoot` is usually called without mutation checks

That is true for witness-version of the roots but we still perform the checks for the non-witness root. Every time net_processing receives a regular block message (unless we are loading blocks), we check whether the block is mutated:

https://github.com/bitcoin/bitcoin/blob/fa283d28e261416f0785450ab687af199103776a/src/net_processing.cpp#L4647

`IsBlockMutated()` always calls `CheckMerkleRoot()` which always calls `BlockMerkle
...

💬 l0rinc commented on pull request "script: Remove dead code from OP_CHECKMULTISIG":
(https://github.com/bitcoin/bitcoin/pull/33977#issuecomment-3595815575)
Code review ACK 53ca0dc3cfdad8c9fd994c3758a477e319379db1

The lines are safe to remove since they're side-effect free and the mentioned extra element is included in the initial stack size validation while the cleanup loop stops before popping it.
The comment above it likely doesn't need updating since it's seems to refer to the line below.
The `SCRIPT_ERR_INVALID_STACK_OPERATION` script error is used elsewhere, so we don't need to clean that up either.

🚀 fanquake merged a pull request: "cmake: Make `BUILD_KERNEL_TEST` depend on `BUILD_KERNEL_LIB`"
(https://github.com/bitcoin/bitcoin/pull/33972)

💬 l0rinc commented on pull request "merkle: pre‑reserve leaves to prevent reallocs with odd vtx count":
(https://github.com/bitcoin/bitcoin/pull/32497#discussion_r2576550988)
I can also just revert the benchmark, this is mostly a correctness (reserve the actual amount, not one less) and memory fix (don't reserve 3x the needed memory), I don't expect it to speed up execution a lot, but I do expect it to fragment the memory less, see https://github.com/bitcoin/bitcoin/pull/32497#issuecomment-3563724551

💬 hodlinator commented on pull request "merkle: pre‑reserve leaves to prevent reallocs with odd vtx count":
(https://github.com/bitcoin/bitcoin/pull/32497#discussion_r2576589632)
I prefer doing the reserve-adjustment in the benchmark as well and doing the reserve inside of the benchmarked section (edited https://github.com/bitcoin/bitcoin/pull/32497#discussion_r2569912274 earlier today).

Makes sense that it could have a higher impact on memory fragmentation than execution speed.

💬 hebasto commented on pull request "build: Use clang-cl to build on Windows natively":
(https://github.com/bitcoin/bitcoin/pull/31507#issuecomment-3595926695)
Rebased to resolve conflicts.

💬 maflcko commented on pull request "build: Bump VS minimum supported version to 18.0":
(https://github.com/bitcoin/bitcoin/pull/33861#discussion_r2576627097)
could remove the version/year here, as the docs and presets should already be clear on it? (no other ci task is having the version/year in the title)

💬 fanquake commented on pull request "build: Use clang-cl to build on Windows natively":
(https://github.com/bitcoin/bitcoin/pull/31507#discussion_r2576634300)
Is this going to be overlinking now?

💬 fanquake commented on pull request "build: Use clang-cl to build on Windows natively":
(https://github.com/bitcoin/bitcoin/pull/31507#discussion_r2576639205)
Why not use the builtins? Not sure I understand the TODO.

💬 hebasto commented on pull request "build: Bump VS minimum supported version to 18.0":
(https://github.com/bitcoin/bitcoin/pull/33861#discussion_r2576641110)
Thanks! Updated.

💬 fanquake commented on pull request "build: Use clang-cl to build on Windows natively":
(https://github.com/bitcoin/bitcoin/pull/31507#discussion_r2576643197)
> -Wno-error=missing-braces
> -Wno-unused-private-field

Why are these needed/are they being fixed?

💬 fanquake commented on pull request "guix: Use UCRT runtime for Windows release binaries":
(https://github.com/bitcoin/bitcoin/pull/33593#issuecomment-3595955919)
Shouldn't this be removing the other CI (#33764):
> MSVCRT-related CI jobs should be removed from the CI framework once the migration to UCRT is complete.

💬 Nonthawat100iQ commented on pull request "build: Use clang-cl to build on Windows natively":
(https://github.com/bitcoin/bitcoin/pull/31507#discussion_r2576666265)
@

💬 Nonthawat100iQ commented on pull request "build: Use clang-cl to build on Windows natively":
(https://github.com/bitcoin/bitcoin/pull/31507#discussion_r2576666312)
@

✅ maflcko closed a pull request: "test: add test for coin write failure"
(https://github.com/bitcoin/bitcoin/pull/33980)

💬 maflcko commented on pull request "test: add test for coin write failure":
(https://github.com/bitcoin/bitcoin/pull/33980#issuecomment-3595989321)
> This implementation adds comprehensive test coverage for coin database write failures in Bitcoin Core. Previously, there were **no tests** for this critical failure scenario.

This is wrong. You'll have to prove which lines/paths are uncovered and show that they are now covered.


Closing as an LLM generated "AI agent" patch. Please note that contributors are required to fully understand their authored code themselves.

Also, the explanation is obviously wrong and hallucinated.

If yo
...

💬 pinheadmz commented on pull request "Replace libevent with our own HTTP and socket-handling implementation":
(https://github.com/bitcoin/bitcoin/pull/32061#issuecomment-3596044151)
Push to 5f6842030a4d82b81d640f1109c543dabd0938ef:
- More nits and suggestions from Sonarcloud

💬 hebasto commented on pull request "build: Use clang-cl to build on Windows natively":
(https://github.com/bitcoin/bitcoin/pull/31507#discussion_r2576763610)
> > -Wno-error=missing-braces

From https://github.com/bitcoin/bitcoin/actions/runs/19820405203/job/56781245720:
```
D:\a\bitcoin\bitcoin\src\test\i2p_tests.cpp(116,34): warning : suggest braces around initialization of subobject [-Wmissing-braces] [D:\a\bitcoin\bitcoin\build\src\test\test_bitcoin.vcxproj]
C:\Program Files (x86)\Windows Kits\10\Include\10.0.26100.0\shared\ws2ipdef.h(251,33): note: expanded from macro 'IN6ADDR_LOOPBACK_INIT'
D:\a\bitcoin\bitcoin\src\test\i2p_tests.cpp(159
...

⚠️ laanwj opened an issue: "build: `guix` package was removed from debian"
(https://github.com/bitcoin/bitcoin/issues/33982)
Apparently the `guix` debian package was no longer maintained, and was removed from debian recently (see https://lwn.net/Articles/1035491/). Ubuntu and other debian derivatives will likely follow.

This means we need new setup instructions for guix on debian in `contrib/guix/INSTALL.md`.

💬 fanquake commented on issue "build: `guix` package was removed from debian":
(https://github.com/bitcoin/bitcoin/issues/33982#issuecomment-3596149271)
Related discussion in #33574.