🚀 glozow merged a pull request: "[30.x] Finalise v30.0"
(https://github.com/bitcoin/bitcoin/pull/33559)

💬 glozow commented on issue "Release Schedule for 30.0":
(https://github.com/bitcoin/bitcoin/issues/32275#issuecomment-3387337526)
v30.0 tagged: https://github.com/bitcoin/bitcoin/releases/tag/v30.0

💬 trevarj commented on pull request "[WIP] doc: update Guix install instructions":
(https://github.com/bitcoin/bitcoin/pull/33574#issuecomment-3387394913)
While using Guix's `install.sh` is my preferred method of installation on a "foreign distro", I will acknowledge that it isn't always a smooth experience. For example, when a user isn't using bash as their shell and doesn't know how Guix works they could miss the necessary env vars.

**A crazy idea** could be to offer a `.deb` package that is created using `guix pack -f deb -m manifest.scm`, where the manifest contains `guix` itself and the packages within `contrib/guix/manifest.scm`. Users on
...

💬 sipa commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2417943637)
I've moved the implementation out of the class definition, so that it doesn't need to move when the abstraction is added.

💬 sipa commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2417947258)
Good idea, I've done something like this (with the repetition made optional).



> And given that `GetMainMemoryUsage` has some surprising side-effects (`SplitAll` & `ApplyDependencies`), we could also check that it's not changing counts or staging presence.

There is no need for that; comparing with the simulation will catch that as those are well-defined properties we check (either when invoking inspector methods in the simulation, or in the full comparison at the end).

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