💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >10% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2429350877)
> I have tried std::jthread
I looked at this, but it doesn't really add anything to this implementation. We could have a `std::stop_token` for each thread, but we would have to `request_stop()` each `jthread` before releasing the semaphore in the destructor anyway. So it doesn't let us remove the destructor, and saves a line for not having to declare `m_request_stop`. I don't think it's worth it to use `jthread`s here.
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2429350877)
> I have tried std::jthread
I looked at this, but it doesn't really add anything to this implementation. We could have a `std::stop_token` for each thread, but we would have to `request_stop()` each `jthread` before releasing the semaphore in the destructor anyway. So it doesn't let us remove the destructor, and saves a line for not having to declare `m_request_stop`. I don't think it's worth it to use `jthread`s here.
💬 optout21 commented on pull request "[IBD] coins: reduce lookups in dbcache layer propagation":
(https://github.com/bitcoin/bitcoin/pull/33602#discussion_r2429366620)
Fair enough. It's called in more places with 'true' than with 'false', and the original version was corresponding to the 'true' case, that's the 'true' alternative looks more a default.
What is important is that comments are added to all places where it is called with 'false'.
(https://github.com/bitcoin/bitcoin/pull/33602#discussion_r2429366620)
Fair enough. It's called in more places with 'true' than with 'false', and the original version was corresponding to the 'true' case, that's the 'true' alternative looks more a default.
What is important is that comments are added to all places where it is called with 'false'.
⚠️ diegoviola opened an issue: "Qt6 version of Bitcoin Core (bitcoin-qt) flickers on Wayland"
(https://github.com/bitcoin-core/gui/issues/903)
### Issues, reports or feature requests related to the GUI should be opened directly on the GUI repo
- [x] I still think this issue should be opened here
### Report
So I've just tried the `v30.0` release and I have it up and running on Arch Linux, bitcoin-qt is running natively on Wayland (sway).
After using the GUI for a while, I have noticed that the whole window would flicker, e.g. when switching between "Overview" and "Transactions".
This never happened before, and I think the reason fo
...
(https://github.com/bitcoin-core/gui/issues/903)
### Issues, reports or feature requests related to the GUI should be opened directly on the GUI repo
- [x] I still think this issue should be opened here
### Report
So I've just tried the `v30.0` release and I have it up and running on Arch Linux, bitcoin-qt is running natively on Wayland (sway).
After using the GUI for a while, I have noticed that the whole window would flicker, e.g. when switching between "Overview" and "Transactions".
This never happened before, and I think the reason fo
...
📝 brunoerg opened a pull request: "test: P2SH sig ops are only counted with `SCRIPT_VERIFY_P2SH`"
(https://github.com/bitcoin/bitcoin/pull/33624)
This PR adds a test case for `GetTransactionSigOpCost` to check that P2SH sig ops are only counted when `SCRIPT_VERIFY_P2SH` flag is set.
Kills the following [mutant](https://corecheck.dev/mutation/src/consensus/tx_verify.cpp#L150):
```diff
diff --git a/src/consensus/tx_verify.cpp b/src/consensus/tx_verify.cpp
index 9d09872597..cc7cdaaf8f 100644
--- a/src/consensus/tx_verify.cpp
+++ b/src/consensus/tx_verify.cpp
@@ -147,7 +147,7 @@ int64_t GetTransactionSigOpCost(const CTransaction&
...
(https://github.com/bitcoin/bitcoin/pull/33624)
This PR adds a test case for `GetTransactionSigOpCost` to check that P2SH sig ops are only counted when `SCRIPT_VERIFY_P2SH` flag is set.
Kills the following [mutant](https://corecheck.dev/mutation/src/consensus/tx_verify.cpp#L150):
```diff
diff --git a/src/consensus/tx_verify.cpp b/src/consensus/tx_verify.cpp
index 9d09872597..cc7cdaaf8f 100644
--- a/src/consensus/tx_verify.cpp
+++ b/src/consensus/tx_verify.cpp
@@ -147,7 +147,7 @@ int64_t GetTransactionSigOpCost(const CTransaction&
...
🤔 sipa reviewed a pull request: "coins: fix `cachedCoinsUsage` accounting in `CCoinsViewCache`"
(https://github.com/bitcoin/bitcoin/pull/32313#pullrequestreview-3335978023)
ACK 24d861da7894add47747eff69dd3fc71fbcdd7d0
(https://github.com/bitcoin/bitcoin/pull/32313#pullrequestreview-3335978023)
ACK 24d861da7894add47747eff69dd3fc71fbcdd7d0
💬 achow101 commented on issue "`generatetoaddress` 2-3x slower on v30 compared to v29":
(https://github.com/bitcoin/bitcoin/issues/33618#issuecomment-3402142153)
Ah, sorry, misread the issue.
This is expected behavior as a consequence of updating the wallet's best block hash every time a block contains a transaction that belongs to the wallet. What's happening is that we are writing to the wallet for each of these blocks now, whereas previously we were not. I think this behavior is preferred over the previous, even if it is slower.
(https://github.com/bitcoin/bitcoin/issues/33618#issuecomment-3402142153)
Ah, sorry, misread the issue.
This is expected behavior as a consequence of updating the wallet's best block hash every time a block contains a transaction that belongs to the wallet. What's happening is that we are writing to the wallet for each of these blocks now, whereas previously we were not. I think this behavior is preferred over the previous, even if it is slower.
🤔 w0xlt reviewed a pull request: "coins: fix `cachedCoinsUsage` accounting in `CCoinsViewCache`"
(https://github.com/bitcoin/bitcoin/pull/32313#pullrequestreview-3335987656)
ACK https://github.com/bitcoin/bitcoin/pull/32313/commits/24d861da7894add47747eff69dd3fc71fbcdd7d0
(https://github.com/bitcoin/bitcoin/pull/32313#pullrequestreview-3335987656)
ACK https://github.com/bitcoin/bitcoin/pull/32313/commits/24d861da7894add47747eff69dd3fc71fbcdd7d0
💬 hebasto commented on issue "Qt6 version of Bitcoin Core (bitcoin-qt) flickers on Wayland":
(https://github.com/bitcoin-core/gui/issues/903#issuecomment-3402149403)
> After using the GUI for a while, I have noticed that the whole window would flicker, e.g. when switching between "Overview" and "Transactions".
Does it look similar to https://github.com/bitcoin/bitcoin/issues/33432?
(https://github.com/bitcoin-core/gui/issues/903#issuecomment-3402149403)
> After using the GUI for a while, I have noticed that the whole window would flicker, e.g. when switching between "Overview" and "Transactions".
Does it look similar to https://github.com/bitcoin/bitcoin/issues/33432?
💬 l0rinc commented on pull request "randomenv: Fix MinGW dllimport warning for `environ`":
(https://github.com/bitcoin/bitcoin/pull/33570#issuecomment-3402151700)
My understanding is that https://github.com/msys2-contrib/mingw-w64/blob/cdb052f1d4056cd510cb83197b55868427b87476/mingw-w64-headers/crt/stdlib.h#L704
expands `environ` to `_environ`, and in https://github.com/msys2-contrib/mingw-w64/blob/cdb052f1d4056cd510cb83197b55868427b87476/mingw-w64-headers/crt/stdlib.h#L262-L264, _environ is further defined as a macro expanding to `(* __p__environ())` (similar problem described in https://github.com/msys2/MINGW-packages/issues/13925#issuecomment-13128204
...
(https://github.com/bitcoin/bitcoin/pull/33570#issuecomment-3402151700)
My understanding is that https://github.com/msys2-contrib/mingw-w64/blob/cdb052f1d4056cd510cb83197b55868427b87476/mingw-w64-headers/crt/stdlib.h#L704
expands `environ` to `_environ`, and in https://github.com/msys2-contrib/mingw-w64/blob/cdb052f1d4056cd510cb83197b55868427b87476/mingw-w64-headers/crt/stdlib.h#L262-L264, _environ is further defined as a macro expanding to `(* __p__environ())` (similar problem described in https://github.com/msys2/MINGW-packages/issues/13925#issuecomment-13128204
...
💬 zaidmstrr commented on pull request "miner: fix empty mempool case for waitNext()":
(https://github.com/bitcoin/bitcoin/pull/33566#discussion_r2429445328)
Using `std::accumulate` in line 526 seems good, but using it in line 530 has some performance impact. `std::accumulate` search for the whole vector despite the condition already being met. With this it increases the best-case time complexity and some average-case complexity from `Ω(k)` (where k <= n) to `Ω(n)`. Any caller calling this with frequent polling might face some milliseconds of performance issues in some cases.
I feel like the approach in commit [2e8fff3](https://github.com/bitcoin/bi
...
(https://github.com/bitcoin/bitcoin/pull/33566#discussion_r2429445328)
Using `std::accumulate` in line 526 seems good, but using it in line 530 has some performance impact. `std::accumulate` search for the whole vector despite the condition already being met. With this it increases the best-case time complexity and some average-case complexity from `Ω(k)` (where k <= n) to `Ω(n)`. Any caller calling this with frequent polling might face some milliseconds of performance issues in some cases.
I feel like the approach in commit [2e8fff3](https://github.com/bitcoin/bi
...
🤔 zaidmstrr reviewed a pull request: "miner: fix empty mempool case for waitNext()"
(https://github.com/bitcoin/bitcoin/pull/33566#pullrequestreview-3336048702)
Concept ACK [8f76732](https://github.com/bitcoin/bitcoin/pull/33566/commits/8f7673257a1a86717c1d83770dc857fc254df107)
(https://github.com/bitcoin/bitcoin/pull/33566#pullrequestreview-3336048702)
Concept ACK [8f76732](https://github.com/bitcoin/bitcoin/pull/33566/commits/8f7673257a1a86717c1d83770dc857fc254df107)
💬 diegoviola commented on issue "Qt6 version of Bitcoin Core (bitcoin-qt) flickers on Wayland":
(https://github.com/bitcoin-core/gui/issues/903#issuecomment-3402205029)
> Does it look similar to [bitcoin/bitcoin#33432](https://github.com/bitcoin/bitcoin/issues/33432)?
Yes, it does look like the same issue.
(https://github.com/bitcoin-core/gui/issues/903#issuecomment-3402205029)
> Does it look similar to [bitcoin/bitcoin#33432](https://github.com/bitcoin/bitcoin/issues/33432)?
Yes, it does look like the same issue.
💬 glozow commented on issue "Minor Release 29.2":
(https://github.com/bitcoin/bitcoin/issues/33586#issuecomment-3402216675)
Please review the website PR: https://github.com/bitcoin-core/bitcoincore.org/pull/1183
(https://github.com/bitcoin/bitcoin/issues/33586#issuecomment-3402216675)
Please review the website PR: https://github.com/bitcoin-core/bitcoincore.org/pull/1183
💬 instagibbs commented on pull request "test: Fix reorg patterns in tests to use proper fork-based approach":
(https://github.com/bitcoin/bitcoin/pull/32587#issuecomment-3402220995)
https://github.com/bitcoin/bitcoin/pull/33616#discussion_r2428574156
looks like you forgot to remove the definition from `mempool_updatefromblock.py`
(https://github.com/bitcoin/bitcoin/pull/32587#issuecomment-3402220995)
https://github.com/bitcoin/bitcoin/pull/33616#discussion_r2428574156
looks like you forgot to remove the definition from `mempool_updatefromblock.py`
💬 l0rinc commented on issue "`generatetoaddress` 2-3x slower on v30 compared to v29":
(https://github.com/bitcoin/bitcoin/issues/33618#issuecomment-3402229976)
I've generated a differential flamegraph comparing the commits before and after the regression:
<img width="1000" alt="Image" src="https://github.com/user-attachments/assets/f739b2ac-0aa8-4ccb-9a84-f3239ffa0d0e" />
and
<img width="1000" alt="Image" src="https://github.com/user-attachments/assets/fa12d922-4d18-4620-9c6d-799376a95179" />
<details>
<summary>Flame graphs</summary>
I've generated a differential flamegraph comparing the commits before and after the regression:
<img width="1000" alt="Image" src="https://github.com/user-attachments/assets/f739b2ac-0aa8-4ccb-9a84-f3239ffa0d0e" />
and
<img width="1000" alt="Image" src="https://github.com/user-attachments/assets/fa12d922-4d18-4620-9c6d-799376a95179" />
<details>
<summary>Flame graphs</summary>
![Image](https://github.com/
...
💬 hebasto commented on issue "Qt6 version of Bitcoin Core (bitcoin-qt) flickers on Wayland":
(https://github.com/bitcoin-core/gui/issues/903#issuecomment-3402230782)
> So I've just tried the `v30.0` release and I have it up and running on Arch Linux, bitcoin-qt is running natively on Wayland (sway).
Could you please share the first 10 lines of your debug log file?
(https://github.com/bitcoin-core/gui/issues/903#issuecomment-3402230782)
> So I've just tried the `v30.0` release and I have it up and running on Arch Linux, bitcoin-qt is running natively on Wayland (sway).
Could you please share the first 10 lines of your debug log file?
💬 Raimo33 commented on pull request "miner: fix empty mempool case for waitNext()":
(https://github.com/bitcoin/bitcoin/pull/33566#discussion_r2429469983)
you're talking about theoretical complexity. but I'm almost certain that the `std::accumulate` approach is orders of magnitude faster in practice when you measure absolute time. this is because avoiding a branch at every iteration allows for optimizations such as SIMD vectorization, or even simply a better data access pattern which is more cache friendly.
I'd love to see some benchmarks!
(https://github.com/bitcoin/bitcoin/pull/33566#discussion_r2429469983)
you're talking about theoretical complexity. but I'm almost certain that the `std::accumulate` approach is orders of magnitude faster in practice when you measure absolute time. this is because avoiding a branch at every iteration allows for optimizations such as SIMD vectorization, or even simply a better data access pattern which is more cache friendly.
I'd love to see some benchmarks!
💬 diegoviola commented on issue "Qt6 version of Bitcoin Core (bitcoin-qt) flickers on Wayland":
(https://github.com/bitcoin-core/gui/issues/903#issuecomment-3402245632)
> > So I've just tried the `v30.0` release and I have it up and running on Arch Linux, bitcoin-qt is running natively on Wayland (sway).
>
> Could you please share the first 10 lines of your debug log file?
```
2025-10-14T14:47:07Z Bitcoin Core version v30.0 (release build)
2025-10-14T14:47:07Z Qt 6.10.0 (dynamic), plugin=wayland
2025-10-14T14:47:07Z No static plugins.
2025-10-14T14:47:07Z Style: fusion / QFusionStyle
2025-10-14T14:47:07Z System: Arch Linux, x86_64-little_endian-lp64
2025-10-1
...
(https://github.com/bitcoin-core/gui/issues/903#issuecomment-3402245632)
> > So I've just tried the `v30.0` release and I have it up and running on Arch Linux, bitcoin-qt is running natively on Wayland (sway).
>
> Could you please share the first 10 lines of your debug log file?
```
2025-10-14T14:47:07Z Bitcoin Core version v30.0 (release build)
2025-10-14T14:47:07Z Qt 6.10.0 (dynamic), plugin=wayland
2025-10-14T14:47:07Z No static plugins.
2025-10-14T14:47:07Z Style: fusion / QFusionStyle
2025-10-14T14:47:07Z System: Arch Linux, x86_64-little_endian-lp64
2025-10-1
...
💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >10% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2429481952)
I couldn't get it to work on CI anyway
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2429481952)
I couldn't get it to work on CI anyway
💬 Sjors commented on pull request "miner: fix empty mempool case for waitNext()":
(https://github.com/bitcoin/bitcoin/pull/33566#discussion_r2429496328)
I think the reduced code complexity is worth losing the early return. After cluster cluster mempool this calculation will probably be replaced anyway.
(https://github.com/bitcoin/bitcoin/pull/33566#discussion_r2429496328)
I think the reduced code complexity is worth losing the early return. After cluster cluster mempool this calculation will probably be replaced anyway.