💬 l0rinc commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#issuecomment-3393553555)
Thank you for considering the suggestions, I find the new layout a lot easier to follow.

I have left a few more nits, but I'm fine doing them in a follow-up.
I like these memory optimizations, my only concern is that we're likely doing a lot heavier copying because of the regular compactions (especially since most move constructors seem unused based on previous fuzzing reports). As mentioned, in a follow-up we can experiment with leaving a tiny buffer.

I have also added a few redundant as
...

💬 l0rinc commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2422338701)
This method is called from two places, one of which is dropping the pre-cacululated cluster level that we could access through `FindClusterAndLevel` - the other call doesn't seem to, I guess that's why you decided to always recalculate.
It seems to me it wouldn't complicate much to provide it when it's available and recalculate when it's not, something like:
```patch
diff --git a/src/txgraph.cpp b/src/txgraph.cpp
--- a/src/txgraph.cpp (revision 262762bbb6f7157ba8c54972909c9214448c304b)
+++ b/src
...

💬 l0rinc commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2422371348)
nit: for consistency:
```suggestion
Assume(!GetTxCount());
```

💬 l0rinc commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2422371564)
nit: likely missed because of the extra space
```suggestion
if (!GetTxCount()) return 0;
```

💬 l0rinc commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2422454996)
nit:
```suggestion
void GenericClusterImpl::AddDependencies(SetType parents, DepGraphIndex child) noexcept
```

💬 l0rinc commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2422374209)
nit: I'd put the assume before the first usage of the validated value

💬 l0rinc commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2422890998)
nit: it seems to me this can now be private

💬 l0rinc commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2422453665)
nit: for consistency we could add the name hints here as well

💬 l0rinc commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2422457512)
nit: kinda' unrelated but my linter is complaining here that `Member 'm_known_end_of_cluster' is not initialized in this constructor`.

💬 l0rinc commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2422459512)
```suggestion
/** Like FindCluster, but also return what level the match was found in (-1 if not found). */
```

💬 l0rinc commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2422889784)
nit: in a follow-up we could make this: `assert(!sequences.contains(cluster.m_sequence));`

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >10% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2423072675)
Added a log.

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >10% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2423072759)
Changed to `FIXED_SEED`.

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >10% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2423072787)
Done!

📝 fanquake opened a pull request: "doc: archive release notes for v30.0"
(https://github.com/bitcoin/bitcoin/pull/33601)
Archive v30.0 release notes.

💬 TheCharlatan commented on pull request "index: store per-block transaction locations for efficient lookups":
(https://github.com/bitcoin/bitcoin/pull/32541#issuecomment-3393623978)
> I would try this approach in a separate bindex branch, to evaluate its performance.

What came of this @romanz?

💬 romanz commented on pull request "index: store per-block transaction locations for efficient lookups":
(https://github.com/bitcoin/bitcoin/pull/32541#issuecomment-3393628474)
Sorry, unfortunately I didn't have the time to implement and benchmark the new approach in the last month - but I expect it to have similar lookup performance, and since it wouldn'd require an additional Bitcoin Core index, I will prefer it over my original approach. WDYT?

💬 epcgrs commented on pull request "contrib: Add bash completion for new bitcoin command":
(https://github.com/bitcoin/bitcoin/pull/33385#issuecomment-3393630750)
ACK

[b5d45f0](https://github.com/bitcoin/bitcoin/pull/33385/commits/b5d45f060161a19d6e9286a434713d36c97b3701)

<img width="574" height="260" alt="image" src="https://github.com/user-attachments/assets/a67e3216-babe-4282-8f36-ed3fef4197c1" />

💬 TheCharlatan commented on pull request "index: store per-block transaction locations for efficient lookups":
(https://github.com/bitcoin/bitcoin/pull/32541#issuecomment-3393634975)
> I expect it to have similar lookup performance, and since it wouldn'd require an additional Bitcoin Core index, I will prefer it over my original approach.

If the performance is similar, that sounds like the better course of action. I guess you'd still need the REST endpoint for retrieving the actual transaction?

✅ fjahr closed a pull request: "refactor: Split multithreaded case out of CheckInputScripts"
(https://github.com/bitcoin/bitcoin/pull/32575)