💬 l0rinc commented on pull request "test: Enhance GetTxSigOpCost tests for coinbase transactions":
(https://github.com/bitcoin/bitcoin/pull/32840#discussion_r2492222322)
this is already a coinbase tx, basically equivalent to the above, let's put this before mutating the spending tx, it's not related to it

💬 l0rinc commented on pull request "test: Enhance GetTxSigOpCost tests for coinbase transactions":
(https://github.com/bitcoin/bitcoin/pull/32840#discussion_r2492225315)
After we make this a coinbase, it's confusing to refer to it as a "spending" transaction...
We could add another helper that copies it and makes it a coinbase

💬 l0rinc commented on pull request "test: Enhance GetTxSigOpCost tests for coinbase transactions":
(https://github.com/bitcoin/bitcoin/pull/32840#discussion_r2492179386)
it seems to me `scriptWitness` is actually empty here, so the comment and code are a bit confusing here.
I think a `BOOST_REQUIRE(!spendingTx.vin[0].scriptWitness.IsNull());` would document the requirements here better than a comment.
`BOOST_CHECK_EQUAL(GetTransactionSigOpCost(CTransaction(creationTx), coins, flags), 0);` already checks that coinbases don't have sigops

💬 l0rinc commented on pull request "test: Enhance GetTxSigOpCost tests for coinbase transactions":
(https://github.com/bitcoin/bitcoin/pull/32840#discussion_r2492229011)
I know you didn't introduce them, but these are basically self-contained tests that could be split out to separate `BOOST_AUTO_TEST_CASE`
* GetSigOpCount
* GetTxSigOpCost_Multisig
* GetTxSigOpCost_MultisigP2SH
* GetTxSigOpCost_P2WPKH
* GetTxSigOpCost_P2WPKH_P2SH
* GetTxSigOpCost_P2WSH
* GetTxSigOpCost_P2WSH_P2SH

💬 l0rinc commented on pull request "ci, iwyu: Fix warnings in `src/kernel` and treat them as errors":
(https://github.com/bitcoin/bitcoin/pull/33779#issuecomment-3488414728)
code review ACK a8a33bc0c0a11093418debc36db8ac63bf90e687

💬 Ianilfy commented on something "":
(https://github.com/bitcoin/bitcoin/commit/1c7e820ded0846ef6ab4be9616b0de452336ef64#commitcomment-169646520)
I need help on my phrase

👍 hebasto approved a pull request: "guix: disable libsanitizer in Linux GCC build"
(https://github.com/bitcoin/bitcoin/pull/33780#pullrequestreview-3419058547)
ACK 5c41fa2918c8fee36d0e0375e753249f1efa7c07.

My Guix build:
```
x86_64
db958b4fc80e95686eca42fdba140aa3e150c003d5bdef6e3ea34872cd674c8a guix-build-5c41fa2918c8/output/aarch64-linux-gnu/SHA256SUMS.part
1f70bdae06dc815d12821b2d7ebb17bdb0b1ca84e775ce7c1bafdbb89d745d59 guix-build-5c41fa2918c8/output/aarch64-linux-gnu/bitcoin-5c41fa2918c8-aarch64-linux-gnu-debug.tar.gz
f0cb17846e91bef73901a0f9de4614f31b9881ac445370aa5183ff8a5caa1a35 guix-build-5c41fa2918c8/output/aarch64-linux-gnu/bitcoin-5c41fa
...

📝 kwsantiago opened a pull request: "build: Upgrade depends to Qt6"
(https://github.com/bitcoin/bitcoin/pull/33787)
Upgrades the depends system to build Qt 6.7.3, preparing the codebase for Qt6 compatibility.

## Changes

- Add Qt6 (6.7.3) package definitions and patches
- Update Qt build configuration to support both Qt5 and Qt6
- Add automatic Qt version detection in depends toolchain
- Add libxcb_util_cursor package for Qt6 Linux builds
- Remove obsolete Qt5-specific patches
- Add initial `wintaskbar` infrastructure for Qt6 compatibility, which is covered in https://github.com/bitcoinknots/bitcoin
...

📝 luke-jr opened a pull request: "[29.x] Backport fixes for CVE-2025 46598"
(https://github.com/bitcoin/bitcoin/pull/33788)

💬 luke-jr commented on pull request "[29.x] Backport fixes for CVE-2025 46598":
(https://github.com/bitcoin/bitcoin/pull/33788#discussion_r2492518838)
Note: Not in master commit (another approach is used for this test)

💬 luke-jr commented on pull request "[29.x] Backport fixes for CVE-2025 46598":
(https://github.com/bitcoin/bitcoin/pull/33788#discussion_r2492519413)
Note: reject_reason could be None here, so a check was added (not in master commit)

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#issuecomment-3488760623)
Benchmarked the latest up to block 921129 and it's 16% faster :rocket:. Not as fast as some of @l0rinc's numbers but it's on a laptop with an internal NVMe SSD. This change will see the most benefit for disk IO with higher latency, like network connected storage.

| Command | Mean [s] | Min [s] | Max [s] | Relative |
|:---|---:|---:|---:|---:|
| `echo d606c36a13ca2a055d1a4eb4c623fb6aa45405b2 && /usr/bin/time ./build/bin/bitcoind -printtoconsole=0 -connect=192.168.2.171 -stopatheight=921129`
...

💬 dongcarl commented on pull request "kernel: Introduce C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-3489173537)
What a milestone. Congrats @TheCharlatan!

🤔 trevarj reviewed a pull request: "guix: Use UCRT runtime for Windows release binaries"
(https://github.com/bitcoin/bitcoin/pull/33593#pullrequestreview-3419999713)
@hebasto thanks for working in my suggestion!

ACK dbb8359

💬 purpleKarrot commented on pull request "depends: add zeromq patch to fix mingw CMake file install location":
(https://github.com/bitcoin/bitcoin/pull/33778#issuecomment-3489804291)
> The current path is valid per the CMake [docs](https://cmake.org/cmake/help/latest/command/find_package.html#config-mode-search-procedure).
>
> UPD. ... on Windows, but not when cross-compiling.

When taking cross-compiling into account, there are not two, but three scenarios how mingw can be used:

1. Building on Windows
2. Building on Linux, but packaging for Windows
3. Building on and packaging for Linux

CMake searches for packages in different paths depending on the **build hos
...

🤔 maflcko reviewed a pull request: "guix: use GCC 14.3.0 over 13.3.0"
(https://github.com/bitcoin/bitcoin/pull/33775#pullrequestreview-3420453767)
concept ack. 14.3 should be sufficiently propagated and tested by now, to rely on it for release builds

💬 maflcko commented on pull request "guix: use GCC 14.3.0 over 13.3.0":
(https://github.com/bitcoin/bitcoin/pull/33775#discussion_r2493421654)
url is wrong, and the `maybe-uninitialized` below can be removed?

📝 frankomosh opened a pull request: "doc: add cmake help option in Windows build docs"
(https://github.com/bitcoin/bitcoin/pull/33789)
Follow-up to #33088.

Adds `cmake -B build -LH` documentation to Windows build guides, similar to Unix build documentation.

Based on the suggestion and example provided by @stickies-v in #33088, with minor adjustment to match existing indented code block format in `build-windows.md`.

Tested for:
- WSL Ubuntu with mingw-w64 cross-compilation
- Windows 11 with Visual Studio 2022 (MSVC)

💬 yuvicc commented on pull request "build: Introduce internal kernel library":
(https://github.com/bitcoin/bitcoin/pull/28690#issuecomment-3489978539)
ACK a9ec7cdc0a1f6d4a3bd532649e79459074ed9f3b

💬 hodlinator commented on pull request "miniscript refactor: Remove unique_ptr-indirection (#30866 follow-up)":
(https://github.com/bitcoin/bitcoin/pull/31713#issuecomment-3490078075)
> The latest https://corecheck.dev/bitcoin/bitcoin/pulls/31713 Sonar warnings seem like false positives.

Took a closer look at https://sonarcloud.io/project/issues?issueStatuses=OPEN%2CCONFIRMED&branch=31713-a1df8547ca0711d99f05d1b31bc758a8068ba512&id=aureleoules_bitcoin&open=AZpLftZOTlsoQg18lt0A

`miniscript::Node` on master and in the PR violates the "Rule of Zero" (https://sonarcloud.io/organizations/aureleoules/rules?open=cpp%3AS3624&rule_key=cpp%3AS3624&tab=why) where if one has a cust
...