💬 sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2346831052)
Thank you! In the process of looking at this again, I noticed that the `mempool_tests` unit test is failing in some intermediate commits here as well. Will go back over that as well.

💬 sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2346831222)
Done.

💬 sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2346831407)
Sounds good, I added a commit that does this, if it looks right I'll squash into place.

💬 sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2346831450)
Addressed with the inlining of `CalculateParentsOf()`.

💬 sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2346831547)
This seems to work if I do:

```c++
const auto& [worst_chunk, feeperweight] = m_txgraph->GetWorstMainChunk();
```

Fixed now.

💬 sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2346831633)
Thanks, fixed.

💬 sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2346831692)
Done, and I renamed this variable to `chunk_txs` for clarity.

💬 sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2346831790)
Removed.

💬 sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2346831841)
Oops, reverted my change.

💬 sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2346831889)
Thanks, fixed the comment.

💬 sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2346831920)
Ah, that is much better, thanks.

💬 sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2346831966)
Oops, eliminated now.

💬 sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2346831990)
Eliminated the comment.

💬 sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2346832046)
Looks like this was leftover from some earlier efforts I had to replace `setEntries` with a vector of `txiter`s instead, but I've lost track of where I was at with that. Eliminated this new `Entries` type for now.

💬 sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2346832310)
Looks like a rebase issue (I think there may have been an earlier version of the code where there was a bigger change, and this accidental line reordering was what was left over). Eliminated this commit.

💬 sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2346832329)
Thanks, good point! Making this 100x for now.

💬 sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2346832356)
I added a later commit to the CTxMemPool class as a whole that adds some documentation for this variable; do you think that is sufficient or would it be helpful to add more?

💬 sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2346832438)
Thanks, fixed.

💬 sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2346832459)
This function is gone now.

💬 sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2346832531)
Thanks, eliminated.