💬 hebasto commented on pull request "ci: Add Windows + UCRT jobs for cross-compiling and native testing":
(https://github.com/bitcoin/bitcoin/pull/33764#discussion_r2550080275)
> Yeah, I guess there were some silent conflicts.
Indeed. Fixed.

💬 hebasto commented on pull request "ci: Add Windows + UCRT jobs for cross-compiling and native testing":
(https://github.com/bitcoin/bitcoin/pull/33764#issuecomment-3563434095)
> [#33764 (comment)](https://github.com/bitcoin/bitcoin/pull/33764#discussion_r2549961718)

Thanks! Fixed.

💬 diegoviola commented on pull request "Fix bitcoin-qt visual glitches on Wayland":
(https://github.com/bitcoin-core/gui/pull/904#issuecomment-3563438977)
> For example, on Fedora 43 with Gnome 49 and Wayland

Ah, I haven't tested this on GNOME. I suspect the use of `showNormal()` likely helps with the desired window behavior, which is why the revert works better.

💬 zaidmstrr commented on pull request "rpc: Handle -named argument parsing where '=' character is used":
(https://github.com/bitcoin/bitcoin/pull/32821#discussion_r2550098336)
Not a very strict reason, but to allow more flexibility and not to break the search for minimal whitespaces or indentation changes.

💬 hebasto commented on pull request "cmake: make missing Python interpreter behaviour more explicit":
(https://github.com/bitcoin/bitcoin/pull/33278#discussion_r2550139808)
> Concept ACK
>
> I am however unable to trigger the fatal error on `Ubuntu 24.04.3 LTS` with `Python 3.9.23`

I cannot reproduce the issue:
```
$ cat /etc/os-release | head -1
PRETTY_NAME="Ubuntu 24.04.3 LTS"
$ python3 --version
Python 3.9.23
$ cmake -B build
<snip>
-- Could NOT find Python3 (missing: Python3_EXECUTABLE Interpreter) (Required is at least version "3.10")
Reason given by package:
Interpreter: Wrong version for the interpreter "/home/hebasto/.pyenv/shim
...

📝 Sjors opened a pull request: "mining: add getMemoryLoad() and track template non-mempool memory footprint"
(https://github.com/bitcoin/bitcoin/pull/33922)
Implements the template memory footprint tracking discussed #33899, but does not yet impose a limit.

💬 hodlinator commented on pull request "miniscript refactor: Remove unique_ptr-indirection (#30866 follow-up)":
(https://github.com/bitcoin/bitcoin/pull/31713#discussion_r2550158500)
Agree that would be slightly more optimal, avoiding some needless pushes/pops. Taken in latest push + changed `auto& i` to `Node& n` for clarity.

💬 Sjors commented on pull request "mining: add getMemoryLoad() and track template non-mempool memory footprint":
(https://github.com/bitcoin/bitcoin/pull/33922#issuecomment-3563543871)
I haven't benchmarked this yet on mainnet, so I'm not sure if checking every (unique) transaction for mempool presence is unacceptably expensive.

If people prefer, I could also add a way for the `getblocktemplate` RPC to opt-out of the memory bookkeeping, since it holds on to one template max and no longer than a minute.

💬 mzumsande commented on pull request "test: assumeutxo: add missing tests in wallet_assumeutxo.py":
(https://github.com/bitcoin/bitcoin/pull/30455#discussion_r2550173315)
hmm, the conflicting a0eaa4492548800ba1b2cdd8232195ab5d5c49c7 was merged on August 8. I thought we had regular re-runs of the CI with respect to current master that should have failed - or was the CI already red yesterday and I missed it?

💬 Sjors commented on issue "Block template memory management (for IPC clients)":
(https://github.com/bitcoin/bitcoin/issues/33899#issuecomment-3563559444)
> We would just need to replace `size_t` with a more complicated data structure that's aware of transaction counts (like `map<CTransaction*, long>`) and update this in the `BlockTemplateImpl` constructor and destructor.

I implemented this part #33922.

💬 fanquake commented on pull request "test: assumeutxo: add missing tests in wallet_assumeutxo.py":
(https://github.com/bitcoin/bitcoin/pull/30455#discussion_r2550230893)
> or was the CI already red yesterday and I missed it?

The CI was green yesterday. The open/close today kicked the CI.

💬 maflcko commented on pull request "test: assumeutxo: add missing tests in wallet_assumeutxo.py":
(https://github.com/bitcoin/bitcoin/pull/30455#discussion_r2550234606)
https://github.com/bitcoin/bitcoin/pull/33145#issuecomment-3409554826

💬 hodlinator commented on pull request "build: Embedded ASMap [3/3]: Build binary dump header file":
(https://github.com/bitcoin/bitcoin/pull/28792#discussion_r2550235052)
Close, fixed the *feature_asmap.py* failure by implementing special logic in the move-ctor. But I recommend keeping it as a non-const member so that it can be moved from instead of copied using the default move-ctor.

<details><summary>Not recommended diff for the curious</summary>

```diff
--- a/src/netgroup.h
+++ b/src/netgroup.h
@@ -17,7 +17,10 @@
class NetGroupManager {
public:
NetGroupManager(const NetGroupManager&) = delete;
- NetGroupManager(NetGroupManager&&) = defau
...

💬 andrewtoth commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2550252802)
Memory ordering is the default `std::memory_order_seq_cst`. I don't think we need to worry about performance here, so we can use the default strongest ordering.

💬 instagibbs commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#issuecomment-3563683917)
@sdaftuar if that's the real concern, shouldn't we be ideally dealing with that at TxGraph level? Something analogue to `m_acceptable_iters`? Is it that SFL alone only "fixes" it by just making it a lot less likely to run a long time?

👍 ismaelsadeeq approved a pull request: "doc: clarify and cleanup macOS fuzzing notes"
(https://github.com/bitcoin/bitcoin/pull/33921#pullrequestreview-3493432014)
ACK 6e37b3b4861733ca97ec5d27d0bf52d187b6a2c9

💬 Sjors commented on pull request "mining: add getMemoryLoad() and track template non-mempool memory footprint":
(https://github.com/bitcoin/bitcoin/pull/33922#discussion_r2550274232)
Maybe it's safer to track by Wtxid?

💬 l0rinc commented on pull request "merkle: pre‑reserve leaves to prevent reallocs with odd vtx count":
(https://github.com/bitcoin/bitcoin/pull/32497#issuecomment-3563724551)
> Unless observable benefits can be shown

The change itself is trivial, it's just a **+1** for the reserve before calling the Merkle calculation.
I don't mind simplifying the change if you think that's easier to review (I could also just bump `leaves.reserve(block.vtx.size())` to `leaves.reserve(block.vtx.size() + 1)` without the refactors).
Originally it was similar to that, but based on [concerns](https://github.com/bitcoin/bitcoin/pull/32497#discussion_r2264185044) to decouple callers fr
...

💬 Sjors commented on pull request "mining: add getMemoryLoad() and track template non-mempool memory footprint":
(https://github.com/bitcoin/bitcoin/pull/33922#discussion_r2550291196)
b9306b79b8f5667a2679236af8792bb1c36db817: in addition, we might be wiping the dummy coinbase from the template later: https://github.com/Sjors/bitcoin/pull/106

🤔 TumaBitcoiner reviewed a pull request: "wallet/refactor: change PSBTError to PSBTResult and remove std::optional<common::PSBTResult> and return common::PSBTResult"
(https://github.com/bitcoin/bitcoin/pull/32958#pullrequestreview-3493400959)
I made some suggestions to keep consistency between naming and files. Sometimes `result` was used, other times `err`.