💬 sipa commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2417694634)
I have reordered the statements.

💬 sipa commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2417695849)
I have reordered these statements and added comments.

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

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

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

💬 sipa commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2417699157)
Fixed by doing the introduction of abstraction functions first, before making Cluster an abstract class.

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

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

💬 maflcko commented on pull request "depends: Update URL for `qrencode` package source tarball":
(https://github.com/bitcoin/bitcoin/pull/33494#issuecomment-3387143955)
If someone wants to locally restore it manually, drahtbot happens to have the old version as well:

```
$ curl -LOf 'https://drahtbot.space/depends_download_fallback/qrencode-4.1.1.tar.gz'
% Total % Received % Xferd Average Speed Time Time Time Current
Dload Upload Total Spent Left Speed
100 530k 100 530k 0 0 363k 0 0:00:01 0:00:01 --:--:-- 362k

$ sha256sum ./qrencode-4.1.1.tar.gz
da448ed4f52aba6bcb0cd48cac0
...

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