💬 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`.

💬 TumaBitcoiner commented on 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#discussion_r2550237244)
```suggestion
const auto result{model->wallet().fillPSBT(std::nullopt, /*sign=*/false, /*bip32derivs=*/true, /*n_signed=*/nullptr, psbtx, complete)};
```
Should this be defined as `result` too, as you did in other parts of the code?

💬 TumaBitcoiner commented on 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#discussion_r2550238619)
```suggestion
assert(result== PSBTResult::OK);
```
As above.

💬 TumaBitcoiner commented on 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#discussion_r2550248118)
```suggestion
const auto result{model->wallet().fillPSBT(std::nullopt, /*sign=*/false, /*bip32derivs=*/true, /*n_signed=*/nullptr, psbtx, complete)};
```

💬 TumaBitcoiner commented on 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#discussion_r2550248662)
```suggestion
assert(result== PSBTResult::OK);
```

💬 TumaBitcoiner commented on 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#discussion_r2550251277)
```suggestion
if (result != PSBTResult::OK || complete) {
```

💬 TumaBitcoiner commented on 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#discussion_r2550250397)
```suggestion
const auto result{wallet().fillPSBT(std::nullopt, /*sign=*/false, /*bip32derivs=*/true, nullptr, psbtx, complete)};
```

💬 TumaBitcoiner commented on 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#discussion_r2550261850)
```suggestion
if (result != PSBTResult::OK) return false;
```

💬 TumaBitcoiner commented on 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#discussion_r2550261153)
```suggestion
auto result{wallet.FillPSBT(psbtx, complete, std::nullopt, /*sign=*/true, /*bip32derivs=*/false)};
```

💬 TumaBitcoiner commented on 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#discussion_r2550264017)
```suggestion
const auto result{pwallet->FillPSBT(psbtx, complete, std::nullopt, /*sign=*/true, /*bip32derivs=*/false)};
```