Bitcoin Core Github
43 subscribers
122K links
Download Telegram
💬 hebasto commented on pull request "guix: Use UCRT runtime for Windows release binaries":
(https://github.com/bitcoin/bitcoin/pull/33593#issuecomment-3488313837)
Reworked per feedback from @trevarj.
💬 hebasto commented on pull request "guix: Use UCRT runtime for Windows release binaries":
(https://github.com/bitcoin/bitcoin/pull/33593#discussion_r2492270996)
Thanks! [Reworked](https://github.com/bitcoin/bitcoin/pull/33593#issuecomment-3488313837).
💬 hebasto commented on pull request "guix: Use UCRT runtime for Windows release binaries":
(https://github.com/bitcoin/bitcoin/pull/33593#issuecomment-3488354051)
My Guix build:
```
aarch64
4e80325f9cba1c286a999eeaa6fb5f06bb56ba03f118cfb6eaa86c8443318ccc guix-build-dbb835956abf/output/dist-archive/bitcoin-dbb835956abf.tar.gz
7557b85092d69ac4f6ebeb4881a41fe808ee684b1b2f87efde97a616f1d1b213 guix-build-dbb835956abf/output/x86_64-w64-mingw32/SHA256SUMS.part
3cf561ff53f83f822fd451b2f83161908daad89598d286de916a641640abe747 guix-build-dbb835956abf/output/x86_64-w64-mingw32/bitcoin-dbb835956abf-win64-codesigning.tar.gz
f607587780890278ffcc9c4b346bcecc999
...
👍 darosior approved a pull request: "script: remove dead code in `CountWitnessSigOps`"
(https://github.com/bitcoin/bitcoin/pull/33786#pullrequestreview-3418942549)
Neat. utACK 24bcad3d4df59690f30c9df8ebb62f0bddd0f1c7.

The need for a pointer was removed when `CTxInWitness` was moved into `CTxIn` in f6fb7acda4aefd01b8ef6cd77063bfc0c4f4ab36.
👋 l0rinc's pull request is ready for review: "script: remove dead code in `CountWitnessSigOps`"
(https://github.com/bitcoin/bitcoin/pull/33786)
👋 hebasto's pull request is ready for review: "ci, iwyu: Fix warnings in `src/kernel` and treat them as errors"
(https://github.com/bitcoin/bitcoin/pull/33779)
💬 hebasto commented on pull request "ci, iwyu: Fix warnings in `src/kernel` and treat them as errors":
(https://github.com/bitcoin/bitcoin/pull/33779#issuecomment-3488404139)
There are no conflicts with other contributors' PRs.

Friendly ping @l0rinc @maflcko @ryanofsky @willcl-ark who reviewed https://github.com/bitcoin/bitcoin/pull/31308, and @TheCharlatan, the kernel expert :)
🤔 l0rinc requested changes to a pull request: "test: Enhance GetTxSigOpCost tests for coinbase transactions"
(https://github.com/bitcoin/bitcoin/pull/32840#pullrequestreview-3418794264)
I have gone through the cases, I think we should take this opportunity and unify the test to use BOOST_ checkers for better error messages, to split the big test into smaller self-contained tests (otherwise the first failure will break the remaining ones - though this will result in some setup-repetition, but we can add helpers for those).

I have applied the cleanup that I would like to see here, if you decide to accept any of it, please do it in multiple focused commits.

<details>
<summary>De
...
💬 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
👍 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`
...