💬 sipa commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2417702683)
Instead of adding an `IsEmpty()`, I ended up using the added `GetTxCount()` virtual function in many places.

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

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

💬 sipa commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2417705281)
Did rephrase a bit more elaborately.

💬 sipa commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2417706361)
I have moved these changes to a separate commit, introducing the notion of intended ranges of transaction counts for each Cluster implementation.

👍 darosior approved a pull request: "[30.x] Finalise v30.0"
(https://github.com/bitcoin/bitcoin/pull/33559#pullrequestreview-3320283291)
ACK d615eb6998eeccb9106854dffa95f36b319177e1

💬 maflcko commented on pull request "Revert "depends: Update URL for `qrencode` package source tarball"":
(https://github.com/bitcoin/bitcoin/pull/33577#issuecomment-3387165608)
lgtm. Seems fine to temporarily revert. A new pull can be created to properly try it again some time in the future.



review ACK e4335a31920cd390d936cd51cc4478a234db1276 💤

<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}"
RUTRmVTMeKV5npGrKx1
...

📝 sdaftuar opened a pull request: "Cluster mempool followups"
(https://github.com/bitcoin/bitcoin/pull/33591)
As suggested in the main cluster mempool PR (https://github.com/bitcoin/bitcoin/pull/28676#pullrequestreview-3177119367), I've pulled out some of the non-essential optimizations and cleanups into this separate PR.

Will continue to add more commits here to address non-blocking suggestions/improvements as they come up.

💬 sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#issuecomment-3387174668)
At @glozow's suggestion I've removed some optimization/cleanup commits from this PR and opened #33591 for non-blocking cleanups/followups.

💬 glozow commented on pull request "depends: Update URL for `qrencode` package source tarball":
(https://github.com/bitcoin/bitcoin/pull/33494#issuecomment-3387203032)
fwiw @achow101 has already done so

💬 purpleKarrot commented on pull request "cmake: Use builtin support for .manifest files":
(https://github.com/bitcoin/bitcoin/pull/33585#issuecomment-3387213880)
> This doesn't Guix build:

I added a workaround for upstream issue [23244](https://gitlab.kitware.com/cmake/cmake/-/issues/23244). It should pass now.

🚀 glozow merged a pull request: "Revert "depends: Update URL for `qrencode` package source tarball""
(https://github.com/bitcoin/bitcoin/pull/33577)

💬 plebhash commented on issue "RFC: Cancelling waitNext calls in the IPC mining interface":
(https://github.com/bitcoin/bitcoin/issues/33575#issuecomment-3387283609)
hmm thinking about it deeper, I guess I could simply save the `waitNext` future into a `mut` variable so it's still available across the unified loop iterations, and then replace it whenever it either returns or is cancelled (via `interruptWait`) due to `CoinbaseOutputConstraints`

so I wouldn't say your original gut feeling is totally wrong!

although I still appreciate the benefits of separating concerns into different loops... I guess I'll experiment with both approaches whenever `interruptWa
...

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