💬 instagibbs commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r2098113781)
```Suggestion
const LinearizationChunking linchunking(m_depgraph, m_linearization);
```

💬 instagibbs commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r2098239806)
nit: leave a comment that deps is also being added to here, briefly forgot

💬 instagibbs commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r2098190802)
nit: 3 out of the N fields are being set here, could we be specific which ones for clarity?

💬 instagibbs commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r2098240846)
```Suggestion
// Append explicit deps, and sort it by child.
```

💬 instagibbs commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r2098254652)
```Suggestion
// Sort deps by parent by graph index. The order is unchanged from now on.
```

💬 instagibbs commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r2098251956)
```Suggestion
// Fill m_deps_left in trim_data and initially populate trim_heap. Because of the sort above, all
```

💬 instagibbs commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r2098265434)
```Suggestion
// a map from GraphIndex to TrimTxData via locate_fn, and its ordering will not change anymore.
```

💬 instagibbs commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r2098308679)
```Suggestion
Assume(!ret.empty());
return ret;
```

💬 instagibbs commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r2109385109)
https://github.com/instagibbs/bitcoin/tree/2025-05-applytrim

Worked on a test attempts to cover the claim here. It ended up being more complicated than that (claim a bit wrong?) due to the lazy application of dependencies. For this property to hold, I believe you have to touch the TxGraph in any way that applies dependencies when undersized, *then* transition to oversized graph for the circular dependencies to not occur in the implied graph.

Without the dependencies applied, the linearizat
...

🤔 sipa reviewed a pull request: "rpc: Round verificationprogress to 1 for a recent tip"
(https://github.com/bitcoin/bitcoin/pull/32528#pullrequestreview-2871433824)
utACK fab1e02086ceebd7d96417b7a386fe61158bfda9

💬 luke-jr commented on pull request "subprocess: Let shell parse command on non-Windows systems":
(https://github.com/bitcoin/bitcoin/pull/32577#issuecomment-2912964132)
Probably should really go through the shell on Windows too...

💬 Kixunil commented on pull request "contrib: add xor-blocks tool to obfuscate blocks directory":
(https://github.com/bitcoin/bitcoin/pull/32451#discussion_r2109501741)
Yes, but it loses the error information so you'd silently continue even if write failed leading to data corruption.

💬 Kixunil commented on pull request "contrib: add xor-blocks tool to obfuscate blocks directory":
(https://github.com/bitcoin/bitcoin/pull/32451#discussion_r2109507090)
On the next run it'll crash failing to convert into `[u8; 8]`. While not catastrophic, it makes the program more annoying for end users.

🤔 l0rinc requested changes to a pull request: "improve MallocUsage() accuracy"
(https://github.com/bitcoin/bitcoin/pull/28531#pullrequestreview-2871139248)
Thanks for getting back to this change.
We should synchronize it with `CCoinsMap`, chich already claims to account for an "overhead of 1 or 2 pointers".
Also, the test should pass before the change as well, so it would be helpful to add it as a separate commit before the change to make it easy to see how it behaves before and after the change as well.
The test still contains a lot of repetition, please see my suggestion on how to compact it a bit more.
There are still uncovered parts of the
...

💬 l0rinc commented on pull request "improve MallocUsage() accuracy":
(https://github.com/bitcoin/bitcoin/pull/28531#discussion_r2109307830)
Instead of empirically, can we add documentation or source code links here?

💬 l0rinc commented on pull request "improve MallocUsage() accuracy":
(https://github.com/bitcoin/bitcoin/pull/28531#discussion_r2109228294)
@LarryRuane I think you should probably be the author of the commit, given that you've already added @theStack as a co-author

💬 l0rinc commented on pull request "improve MallocUsage() accuracy":
(https://github.com/bitcoin/bitcoin/pull/28531#discussion_r2109304532)
```suggestion
// The memory used by an unordered_set or unordered_map is the sum of the
```

💬 l0rinc commented on pull request "improve MallocUsage() accuracy":
(https://github.com/bitcoin/bitcoin/pull/28531#discussion_r2109380457)
it makes sense to add the `/*max_mempool_size_bytes=*` for primitives - but we don't usually do it for self-explanatory variables:
```suggestion
chainstate.GetCoinsCacheSizeState(MAX_COINS_CACHE_BYTES, MAX_MEMPOOL_CACHE_BYTES),
```

💬 l0rinc commented on pull request "improve MallocUsage() accuracy":
(https://github.com/bitcoin/bitcoin/pull/28531#discussion_r2109325757)
If we're already checking equality, what's the point of checking that it's not equal to something else?
This should also enable us inlining `empty_cache`

💬 l0rinc commented on pull request "improve MallocUsage() accuracy":
(https://github.com/bitcoin/bitcoin/pull/28531#discussion_r2109331421)
I really dislike that we're exposing the loop variable just to see if we finished without transitioning.
We seem to exit at worst around `32415` for me locally, maybe we can reduce the attempt count and extract it to a variable:
```C++
constexpr size_t MAX_ATTEMPTS{50'000}; // runaway-loop safety cap
```
And we can add a worst-case condition for the very last iteration which should always be `LARGE`, e.g.:
```C++
// OK → LARGE
for (size_t i{1}; i <= MAX_ATTEMPTS; ++i) {

...