💬 sipa commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2417948078)
I have removed some of them through use of `GetTxCount()` as suggested elsewhere.

💬 sipa commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2417948453)
Nice catch. Removed.

💬 sipa commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2417948776)
Moved it.

💬 sipa commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2417949550)
Added a commit that fixes some outdated comments.

💬 sipa commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2417950481)
Fixed in a commit that fixes some outdated comments.

💬 sipa commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2417951078)
Indeed, fixed.

💬 sipa commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2417952778)
Nice catch.

Even better: the fact that this was possible indicated that the function was never called on empty clusters (because `TotalMemoryUsage()` is not 0, even for empty clusters). I've thus changed it to an `Assume()` for non-emptiness.

💬 sipa commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2417953257)
This was a leftover, gone.

💬 sipa commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2417954153)
Addressed by introducing a separate `FindCluster` and `FindClusterAndLevel` function.

💬 sipa commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2417954576)
Done, in a new comment-fixing commit.

💬 sipa commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2417955778)
Done.

It's simply that in a singleton cluster, the ancestors of a transaction, the descendants of a transaction, and in fact the entire cluster are always just the same single element.

💬 sipa commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2417958188)
Nice. I've replaced it with an `Assume` that the adding always happens at the end.

This made me realize that there is probably a more efficient `GenericClusterImpl::Merge` implementation possible (which just translates the indices in the old cluster to the new one by appending an offset to all). It's pretty nontrivial to do, so I'm leaving that for a potential follow-up if anyone cares.

💬 sipa commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2417959306)
This is now dealt with more consistently in a new commit that introduces a range of valid transaction counts for each `Cluster` implementation.

💬 sipa commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2417961019)
Why not both?

They're independent notions: one is a policy-configurable per-TxGraph maximum cluster count, and the other is a Cluster-implementation dependent limit.

Done.

💬 sipa commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2417961661)
Right.

💬 sipa commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2417961927)
Done.

💬 sipa commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2417962125)
Fixed.

💬 sipa commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2417962445)
Good idea, done.

💬 sipa commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2417963146)
Fair point, done.

💬 sipa commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2417964329)
Indeed. Changed into an `assert(false);`, plus a comment explaining why, and a `SingletonClusterImpl::SanityCheck()` check to verify that claim.

📝 amishhaa opened a pull request: "Contrib: Updated macdeployqtplus to remove deprecated --deep signing"
(https://github.com/bitcoin/bitcoin/pull/33592)
**Description**
Removed the deprecated --deep flag from codesign in macdeployqtplus and replaced it with an explicit recursive signing process for all binaries, frameworks, and plugins.
Fixes #32486

<img width="624" height="370" alt="Screenshot 2025-10-10 at 2 36 16 AM" src="https://github.com/user-attachments/assets/6329b67e-e400-4d8d-947c-25bceab4cb55" />


**Steps To Reproduce**
N/A since not a bug only deprecating an arg

**Steps To Test**
- Set up the app normally.
- Signing w
...