✅ fanquake closed an issue: "guix: Unable to reproduce macOS SDK tarball on Fedora 40"
(https://github.com/bitcoin/bitcoin/issues/31873)

🚀 fanquake merged a pull request: "contrib: turn off compression of macOS SDK to fix determinism (across distros)"
(https://github.com/bitcoin/bitcoin/pull/32009)

💬 l0rinc commented on pull request "merkle: pre‑reserve leaves to prevent reallocs with odd vtx count":
(https://github.com/bitcoin/bitcoin/pull/32497#discussion_r2569155777)
My reasoning was that I want the benchmark to reflect the actual usage, see: https://github.com/bitcoin/bitcoin/pull/32497#issuecomment-2973633245

It wouldn't be fair to see the `ComputeMerkleRoot` performance without the leaf constructions (and get an unrealistic speedup). What do you think?

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2569185787)
> peer=%d%s", node.GetId(), node.LogIP(fLogIPs)

This was improved by https://github.com/bitcoin/bitcoin/pull/28521 (my comment on it: https://github.com/bitcoin/bitcoin/pull/28521#pullrequestreview-2354306033), but maybe it can be improved further. Anyway this pattern is being repeated in many places, so this issue is bigger than the current PR. Concept ACK on further dedups :)

💬 l0rinc commented on pull request "merkle: pre‑reserve leaves to prevent reallocs with odd vtx count":
(https://github.com/bitcoin/bitcoin/pull/32497#discussion_r2569184143)
Thanks for checking, I think it's the best solution regarding C++ simplicity and CPU support.

💬 l0rinc commented on pull request "merkle: pre‑reserve leaves to prevent reallocs with odd vtx count":
(https://github.com/bitcoin/bitcoin/pull/32497#discussion_r2569177747)
Good point, I can use it in a [few more places](https://github.com/bitcoin/bitcoin/compare/e3b02f1539e59a6a20a00b4161219b5aba6fefed..14a4dca89adaab6f1ad42af6716f9065f1abe2e1), pushed.

🤔 l0rinc reviewed a pull request: "validation: fetch block inputs on parallel threads >20% faster IBD"
(https://github.com/bitcoin/bitcoin/pull/31132#pullrequestreview-3515011880)
This new design achieves the best results I've seen so far across all platforms measured, excellent job!!

It evolved from an `InputFetcher` helper (where the parallelism happened before `ConnectBlock` was called) to `CoinsViewCacheAsync`, a multithreaded `CCoinsViewCache` subclass whose worker threads run in parallel with `ConnectBlock` itself.

Internal spends are still filtered using short txid prefixes stored in a sorted `std::vector` and checked via `std::binary_search`. In the rare case of
...

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2568405428)
nit: other comments use the bare verb form

```suggestion
* Retrieve the coin from the cache even if it is spent, without calling
```

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2568602665)
I wonder if we could change this benchmark to use an actual LevelDB-backed view instead of a synthetic delay – and maybe reuse the setup from https://github.com/bitcoin/bitcoin/pull/32554 for a more realistic measurement?

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2568436113)
This highlights that our dbcache layering is a bit messy – having to static_cast the base view to CCoinsViewCache here is quite ugly. It would be good to clean this up in a follow-up.

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2568645746)
Why do we need this cache to be constructed with deterministic=true here? Is the deterministic ordering actually required for the fuzz harness, or could we drop that?

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2568636267)
nit: the formatter should enforce a trailing newline at the end of C++ source files; could you add one here?

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2568638933)
What if we added a second run inside the same benchmark that calls `StartFetching` again or a series of `AccessCoin` calls - to exercise the “everything is already in the cache” path (similar to a large -dbcache scenario)?

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2568644363)

Same idea as in the bench: would it be feasible to fuzz this against an actual LevelDB-backed view instead of a synthetic cache, or is that too expensive / complicated for now?

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2568598322)
We’re [moving towards Flush not returning a value](https://github.com/bitcoin/bitcoin/pull/33866/files#diff-f0ed73d62dae6ca28ebd3045e5fc0d5d02eaaacadb4c2a292985a3fbd7e1c77cR257) – can we avoid adding new code that relies on its return? Not sure it’s possible before that other PR lands, but worth keeping in mind.

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2568610778)
nit: now that this is `CoinsViewCacheAsync`, maybe rename the thread prefix from "inputfetcher.%i" to something like "coinsviewcacheasync.%i" or similar, to reflect the current design.

📝 billymcbip opened a pull request: "Add a separate ScriptError for empty pubkeys encountered in Tapscript"
(https://github.com/bitcoin/bitcoin/pull/33961)
We currently have two callsites for `SCRIPT_ERR_PUBKEYTYPE`:
- Policy error behind the `SCRIPT_VERIFY_STRICTENC` flag: https://github.com/bitcoin/bitcoin/blob/4de26b111f4d9b116d0d4930f8bbd69889b3cf75/src/script/interpreter.cpp#L220
- Consensus error: https://github.com/bitcoin/bitcoin/blob/4de26b111f4d9b116d0d4930f8bbd69889b3cf75/src/script/interpreter.cpp#L368

It would be good for readability and testability to have separate errors for both cases, as they are distinct. We have more granula
...

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2569278996)
Changed to use `count_seconds()`.

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2569321223)
Yes, changed to use `pnode.LogIP(fLogIPs)`.

📝 ANtutov opened a pull request: "refactor: replace manual promise with SyncWithValidationInterfaceQueue"
(https://github.com/bitcoin/bitcoin/pull/33962)
BroadcastTransaction() now waits for validation callbacks using the built-in validation_signals>SyncWithValidationInterfaceQueue() instead of creating a local std::promise and scheduling a lambda. This removes an unnecessary allocation and uses the canonical API.