💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >40% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2576143396)
we could add comments for these cases as well:
```C++
if (i % 3 == 0) {
// External input
txid = Txid::FromUint256(uint256(i));
} else if (i % 3 == 1) {
// Internal spend (prev tx)
txid = prevhash;
} else {
// Test shortid collisions (looks internal, but is external)
```

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >40% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2576158123)
we could also add an in-memory leveldb here to simplify the code and make it more realistic:
```suggestion
CCoinsViewDB db{DBParams{.path = "", .cache_bytes = 1_MiB, .memory_only = true, .wipe_data = true}, CoinsViewOptions{}};
CCoinsViewCache main_cache{&db};
CoinsViewCacheAsync view{main_cache, db};
```

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >40% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2575797791)
I prefer simple but fully functioning chunks that converge towards a feature (as someone illustrated: "skateboard -> bicycle -> scooter -> motorcycle -> car" instead of "left wheels -> right wheels -> doors -> wipers -> radio antenna -> windows -> etc").
So if we can carve out chunks (such as the internal spend guard, or a single-threaded fetcher first), we could guide the reviewer instead of having a big-bang change that's really hard to fully comprehend as such.

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >40% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2575840858)
wouldn't that percolate to the database layer?

✅ l0rinc closed an issue: "Startup crash on macOS with GCC 15.2: `std::source_location::file_name()` is `nullptr`"
(https://github.com/bitcoin/bitcoin/issues/33964)

💬 l0rinc commented on issue "Startup crash on macOS with GCC 15.2: `std::source_location::file_name()` is `nullptr`":
(https://github.com/bitcoin/bitcoin/issues/33964#issuecomment-3595607653)
Not sure how to create a reliable reproducer for this, closing for now

🚀 fanquake merged a pull request: "cmake: Set `WITH_ZMQ` to `ON` in Windows presets"
(https://github.com/bitcoin/bitcoin/pull/33971)

⚠️ maflcko opened an issue: "integer overflow in FUZZ=package_rbf"
(https://github.com/bitcoin/bitcoin/issues/33981)
Oss-Fuzz: https://issues.oss-fuzz.com/issues/464820824

Input: https://github.com/user-attachments/files/23850907/clusterfuzz-testcase-minimized-package_rbf-5766839826448384.bin.not.txt

Reproduce:

`UBSAN_OPTIONS="suppressions=$(pwd)/test/sanitizer_suppressions/ubsan:print_stacktrace=1:halt_on_error=1:report_error_type=1" FUZZ=package_rbf ./bld-cmake/bin/fuzz -runs=1 /tmp/tmp.hLXaXiiFIP/clusterfuzz-testcase-minimized-package_rbf-5766839826448384.bin.not.txt`

```
src/util/feefrac.h:128:14: runt
...

💬 l0rinc commented on pull request "precalculate SipHash constant salt XORs":
(https://github.com/bitcoin/bitcoin/pull/30442#issuecomment-3595752609)
Thanks for the review @laanwj and @vasild.

> Results look like there is no difference between baseline and PR. Some small differences, within noise.

Your benchmarking machine had quite the fluctuation, I usually try to stabilize the system before measuring these, see:
https://github.com/bitcoin/bitcoin/blob/fe434a469534766f18d7560d968deed37193835f/src/bench/nanobench.h#L2078

Plotting the result you posted for Clang:
<img width="1473" height="791" alt="image" src="https://github.com/us
...

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