💬 maflcko commented on pull request "ci: Enable more shellcheck":
(https://github.com/bitcoin/bitcoin/pull/32970#discussion_r2213780607)
No this won't work, because some elements contain spaces. `SC2086` is about this, to warn about word splitting.

For example. `DCMAKE_CXX_FLAGS` will be split:


```
[13:10:44.210] + cmake -S /ci_container_base -B /ci_container_base/ci/scratch/build-arm-linux-gnueabihf -DBUILD_BENCH=ON -DBUILD_FUZZ_BINARY=ON -DCMAKE_TOOLCHAIN_FILE=/ci_container_base/depends/arm-linux-gnueabihf/toolchain.cmake -DWERROR=ON -DCMAKE_INSTALL_PREFIX=/ci_container_base/ci/scratch/out -Werror=dev -DREDUCE_EXPORTS=
...

💬 hebasto commented on pull request "init: [gui] Avoid UB/crash in InitAndLoadChainstate":
(https://github.com/bitcoin/bitcoin/pull/32987#issuecomment-3084665767)
Concept ACK.

👍 hebasto approved a pull request: "ci: Enable more shellcheck"
(https://github.com/bitcoin/bitcoin/pull/32970#pullrequestreview-3030212877)
ACK fa1fd074685ca96b9bd3855e9e6fe730a4f6462c.

💬 sfsegreto commented on issue "ARM Windows build and release":
(https://github.com/bitcoin/bitcoin/issues/31388#issuecomment-3084688654)
Thanks @maflcko , I understand this application can run over emulation, but can you help clarify a bit more what is blocking from having Windows Arm builds of this project? Is it because QT /guix building on a Linux system can't make (or distribute) a WinArm binary? Having a fully Arm build of this project running on Windows Arm would be faster and more efficient than running over the emulation.

💬 maflcko commented on pull request "ci: Enable more shellcheck":
(https://github.com/bitcoin/bitcoin/pull/32970#discussion_r2213801696)
Yeah, the eval hack should work:

```
$ export T="-DREDUCE_EXPORTS=ON -DCMAKE_CXX_FLAGS='-Wno-psabi -Wno-error=maybe-uninitialized'"; eval "T=($T)"; for i in "${T[@]}"; do echo "_${i}_" ; done
_-DREDUCE_EXPORTS=ON_
_-DCMAKE_CXX_FLAGS=-Wno-psabi -Wno-error=maybe-uninitialized_

✅ josibake closed an issue: "ci: improve "test each commit" job to handle more complex scenarios"
(https://github.com/bitcoin/bitcoin/issues/32991)

💬 josibake commented on issue "ci: improve "test each commit" job to handle more complex scenarios":
(https://github.com/bitcoin/bitcoin/issues/32991#issuecomment-3084692489)
Following up, this was my mistake! AFAICT, this happened because I had two squashes into the `refresh-secp256k1` branch. I attempted to reproduce this on my fork and couldn't get a failure, and the only difference I can see is that the original failure had two squashes followed by a merge commit.

Thanks for taking a look @maflcko and for the help in reproducing.

💬 hebasto commented on issue "ARM Windows build and release":
(https://github.com/bitcoin/bitcoin/issues/31388#issuecomment-3084699194)
> ... can you help clarify a bit more what is blocking from having Windows Arm builds of this project?

From https://github.com/bitcoin/bitcoin/issues/31388#issuecomment-2854518383:

> ... the required toolchain isn’t even provided by major distributions.

💬 fanquake commented on pull request "init: [gui] Avoid UB/crash in InitAndLoadChainstate":
(https://github.com/bitcoin/bitcoin/pull/32987#issuecomment-3084727391)
Is there any way to write an actual GUI test for this? Having to add an obscure, single-use option to bitcoind, to simulate crashing behaviour in the GUI, seems like a last resort?

💬 HowHsu commented on pull request "checkqueue: implement a new scriptcheck worker pool with atomic variables":
(https://github.com/bitcoin/bitcoin/pull/32791#issuecomment-3084736836)
> Are the first two commits related to the PR? Seem REST-related?
>

No, they appears here after I rebase against the origin/master and then push -f.

> Benchmark results suggest a 12-thread crossover to become faster than master, any chance that I'm reading this wrong?

the master behaves a little bit better when the number of threads is small, just like the table I put in the PR description. The crossover point is machine/hardware related, and it is usually around 12~15 threads. After
...

💬 maflcko commented on pull request "checkqueue: implement a new scriptcheck worker pool with atomic variables":
(https://github.com/bitcoin/bitcoin/pull/32791#issuecomment-3084744741)
> With #31132 , this one makes sense again, need to test them together in all-cache-miss case

If you are waiting on that one, it could make sense to mark this one as draft for now? If not, you'll have to rebase.

📝 HowHsu converted_to_draft a pull request: "checkqueue: implement a new scriptcheck worker pool with atomic variables"
(https://github.com/bitcoin/bitcoin/pull/32791)
TL;DR
This patchset proposes a new design of scriptcheck worker pool with
atomic variables to leverage more cpu resource on a machine which the
current mutex version cannot. Achieve about 45% boost in performance.

Main Idea:

The current version of worker pool use mutex to do the synchronization
between worker threads, which makes the performance downgrade after
worker_threads_num >= 14. One root cause is the high contention on the
queue's m
...

💬 HowHsu commented on pull request "checkqueue: implement a new scriptcheck worker pool with atomic variables":
(https://github.com/bitcoin/bitcoin/pull/32791#issuecomment-3084752079)
> > With #31132 , this one makes sense again, need to test them together in all-cache-miss case
>
> If you are waiting on that one, it could make sense to mark this one as draft for now? If not, you'll have to rebase.

Done. Thanks Malmö.

🤔 mzumsande reviewed a pull request: "init: [gui] Avoid UB/crash in InitAndLoadChainstate"
(https://github.com/bitcoin/bitcoin/pull/32987#pullrequestreview-3030318662)
I wonder if this GUI interactive reindex feature is worth all the troubles - seems like it got broken multiple times in the past without users really noticing / complaining?! (Just from memory, and I didn't check how many releases were affected)

Given that the need for a reindex should be somewhat rare anyway, would it really be much of a degradation of user experience to remove the feature and have GUI users restart with `-reindex` non-interactively like they would have to with `bitcoind`?

💬 hebasto commented on pull request "init: [gui] Avoid UB/crash in InitAndLoadChainstate":
(https://github.com/bitcoin/bitcoin/pull/32987#issuecomment-3084769976)
> Given that the need for a reindex should be somewhat rare anyway, would it really be much of a degradation of user experience to remove the feature and have GUI users restart with `-reindex` non-interactively like they would have to with `bitcoind`?

I believe it would.

💬 maflcko commented on pull request "test: refactor: overhaul block hash determination for `CBlock{,Header}` objects":
(https://github.com/bitcoin/bitcoin/pull/32868#issuecomment-3084778278)
all good now

re-ACK 5fa34951ead2eebcced919537f5e27526f61d909 🎩

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: re-ACK 5fa3
...

📝 darosior opened a pull request: "Enable `-natpmp` by default"
(https://github.com/bitcoin/bitcoin/pull/33004)
This turns the default for NAT hole-punching (with [PCP](https://en.wikipedia.org/wiki/Port_Control_Protocol) or [NAT-PMP](https://en.wikipedia.org/wiki/NAT_Port_Mapping_Protocol)) to on. Closes #31663.

💬 maflcko commented on pull request "rpc: generateblock to allow multiple outputs":
(https://github.com/bitcoin/bitcoin/pull/32468#discussion_r2213866487)
> I am also thinking about splitting up the "make this arg optional". Seems like a small and easy preparatory pull request?

Any thoughts on this? Making it optional is a non-breaking change that should be easy to review.

💬 sfsegreto commented on issue "ARM Windows build and release":
(https://github.com/bitcoin/bitcoin/issues/31388#issuecomment-3084818744)
Can you be extremely specific please? What required toolchain is not provided and by what major distributions?

💬 mzumsande commented on pull request "init: [gui] Avoid UB/crash in InitAndLoadChainstate":
(https://github.com/bitcoin/bitcoin/pull/32987#discussion_r2213871104)
could use `-test=<option>` instead.

💬 ishaanam commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2213901825)
Done