💬 sdaftuar commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1763454509)
Thanks! Taken.

💬 sdaftuar commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1763448694)
Dropped this test for now, and left a todo to add more tests to exercise different methods of mempool submission and ensure we never violate the cluster limits.

💬 sdaftuar commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1763330087)
Removed (it was needed in an intermediate commit).

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

💬 sdaftuar commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1763536059)
Taken.

💬 sdaftuar commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1761997443)
good point. i wonder if we should just drop the feerate diagram language altogether and leave it at "insufficient fee"?

💬 sdaftuar commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1763532380)
Thanks! Taken.

💬 sdaftuar commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1763554660)
Thanks, removed.

💬 sdaftuar commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1763546504)
Ah, right. Given that this falls back to the vsize limit being exercised, I think this isn't really worth including? Will leave this for now.

👋 maflcko's pull request is ready for review: "ci: Use clang-19 in msan tasks"
(https://github.com/bitcoin/bitcoin/pull/30639)

🤔 jonatack reviewed a pull request: "Drop -dbcache limit"
(https://github.com/bitcoin/bitcoin/pull/28358#pullrequestreview-2310356555)
ACK bb3b980dfd96d9a89e6b04aef2859ce0555c6807

Agee with more freedom/responsibility for the user and the ability to reduce flushing to disk for better IBD performance.

It's probably a good idea to avoid using strike-through tags in PR descriptions, as they aren't rendered well by the merge script and look like emphasis instead.

Due to recent UTXO set growth, the current maximum value for `-dbcache` of 16GB is ~just months away from being~ insufficient (for those who wish to complete
...

🤔 instagibbs reviewed a pull request: "cluster mempool: extend DepGraph functionality"
(https://github.com/bitcoin/bitcoin/pull/30857#pullrequestreview-2306885844)
First run through.

suggestion for clusterlin_add_dependencies harness: https://github.com/instagibbs/bitcoin/commit/c5c44da5a0a630a6725ca85e90e4c67b2eb1f8bc

I think this gets similar or better coverage and is easier for me to understand?

Tried augmenting `RemoveTransactions` coverage with adding things back in post deletion, and it's tripping up SanityCheck. Take a look?

```
diff --git a/src/test/fuzz/cluster_linearize.cpp b/src/test/fuzz/cluster_linearize.cpp
index be06898a6e..05d
...

💬 instagibbs commented on pull request "cluster mempool: extend DepGraph functionality":
(https://github.com/bitcoin/bitcoin/pull/30857#discussion_r1761348362)
?
```Suggestion
assert((depgraph.Descendants(child) & children) == SetType::Singleton(child));
```

💬 instagibbs commented on pull request "cluster mempool: extend DepGraph functionality":
(https://github.com/bitcoin/bitcoin/pull/30857#discussion_r1761406415)
nit
```Suggestion
// Compute the ancestors of parents that are not already ancestors of child.
```

💬 instagibbs commented on pull request "cluster mempool: extend DepGraph functionality":
(https://github.com/bitcoin/bitcoin/pull/30857#discussion_r1761301820)
commit 77e07eba11391868f83db22bf2280e02b5a8cd78

add a motivation for these additions in commit message?

💬 instagibbs commented on pull request "cluster mempool: extend DepGraph functionality":
(https://github.com/bitcoin/bitcoin/pull/30857#discussion_r1761676550)
I don't see where these are being deleted? The only removal below is removing things that aren't in `Positions()`, which the newly added transactions should be? Likely misreading this.

💬 instagibbs commented on pull request "cluster mempool: extend DepGraph functionality":
(https://github.com/bitcoin/bitcoin/pull/30857#discussion_r1761630898)
in commit message for 7510a65127db610df4be9cdd95eaef0c23b3e2ff

"This commits starts introducing support" are we expecting more support later?

💬 instagibbs commented on pull request "cluster mempool: extend DepGraph functionality":
(https://github.com/bitcoin/bitcoin/pull/30857#discussion_r1761348880)
?
```Suggestion
assert((depgraph.Ancestors(parent) & parents) == SetType::Singleton(parent));
```

💬 instagibbs commented on pull request "cluster mempool: extend DepGraph functionality":
(https://github.com/bitcoin/bitcoin/pull/30857#discussion_r1763548308)
Ergonomically speaking, I expect this to be used to remove a set of "direct conflicts" and all descendants of that set, so this wouldn't cause invalid views of the underlying cluster's graph relationships.

Is this correct?

💬 instagibbs commented on pull request "cluster mempool: extend DepGraph functionality":
(https://github.com/bitcoin/bitcoin/pull/30857#discussion_r1763550167)
do we want to leave a note that now-unused entries not at the end can be non-empty/dirty? If that's not possible, a comment explaining that would be helpful too.