💬 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

💬 ishaanam commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2213903403)
I've added a `bulk_vout` function to `script_util.py` which is used by `MiniWallet` and these tests.

💬 maflcko commented on issue "ARM Windows build and release":
(https://github.com/bitcoin/bitcoin/issues/31388#issuecomment-3084872811)
the cross-compilation toolchain doesn't exist, so it isn't provided by any distribution at all

💬 ishaanam commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2213914676)
I've added a version 2 assertion everywhere that `sendall` is used in the tests.

💬 ishaanam commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2213915214)
I've added a test for alice spending change

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

💬 ishaanam commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2213916090)
Yes, this was an outdated comment which I've now removed

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

💬 ishaanam commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2213917014)
Yes, I've updated both of the places where I was doing this.

💬 ishaanam commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2213923610)
Yes, because when Bob confirms, it is removed from `mempool_conflicts` and Alice's transaction is considered "Inactive". I've added a part to this test case where Alice evicts Bob's transaction.