💬 sipa commented on pull request "Replace cluster linearization algorithm with SFL":
(https://github.com/bitcoin/bitcoin/pull/32545#discussion_r2615788225)
I have reverted this change. My benchmarks show that the time per cost is quite similar to before, which I'm now mentioning in the commit message. This will be overhauled in #34023.

💬 sipa commented on pull request "Replace cluster linearization algorithm with SFL":
(https://github.com/bitcoin/bitcoin/pull/32545#discussion_r2615788367)
Done.

💬 sipa commented on pull request "Replace cluster linearization algorithm with SFL":
(https://github.com/bitcoin/bitcoin/pull/32545#discussion_r2615788964)
I have just increased it to 1T iterations, which ought to be enough for anyone.

💬 sipa commented on pull request "Replace cluster linearization algorithm with SFL":
(https://github.com/bitcoin/bitcoin/pull/32545#discussion_r2615789740)
Done, except renaming to `m_transaction_idxs`, because I'm too lazy to update all the affected commits right now. Will leave this open.

💬 sipa commented on pull request "Replace cluster linearization algorithm with SFL":
(https://github.com/bitcoin/bitcoin/pull/32545#discussion_r2615790625)
It's to avoid reallocating; this formula gives the maximum number of dependencies a cluster with a given number of transactions can have. I have added a comment. Does this suffice?

💬 sipa commented on pull request "Replace cluster linearization algorithm with SFL":
(https://github.com/bitcoin/bitcoin/pull/32545#discussion_r2615790818)
Indeed! Added as comment.

💬 sipa commented on pull request "Replace cluster linearization algorithm with SFL":
(https://github.com/bitcoin/bitcoin/pull/32545#discussion_r2615791108)
I was considering that already, done.

💬 sipa commented on pull request "Replace cluster linearization algorithm with SFL":
(https://github.com/bitcoin/bitcoin/pull/32545#discussion_r2615793339)
The point here is to prevent against potential bugs added in the future, which result in optimality just never being reached anymore. Absent bounds on expected iteration count, this would render a large portion of this test powerless (because anything in `is_optimal`, which includes all quality checks) won't be reached anymore.

Added a comment. Does this explain it?

💬 ajtowns commented on pull request "Make `transaction_indentifier` hex string constructor evaluated at comptime":
(https://github.com/bitcoin/bitcoin/pull/34063#discussion_r2615793433)
Would it be better to provide an `operator""_txid` and `_wtxid` along the lines of hexliterals in util/strencodings.h? They could be made available only to test code that way.

Adding `contstexpr` to the constructors in primitives/transaction_identifier.h and writing:

```c++
consteval auto operator""_txid(const char* str, size_t n) { return Txid::FromUint256(uint256{std::string_view{str, n}}); }
consteval auto operator""_wtxid(const char* str, size_t n) { return Wtxid::FromUint256(uint256
...

💬 sipa commented on pull request "Replace cluster linearization algorithm with SFL":
(https://github.com/bitcoin/bitcoin/pull/32545#discussion_r2615794234)
I have not done this (yet), because it's hard to maintain in #34023, where the cost function becomes a lot more complicated.

💬 sipa commented on pull request "Replace cluster linearization algorithm with SFL":
(https://github.com/bitcoin/bitcoin/pull/32545#discussion_r2615794420)
Done.

💬 sipa commented on pull request "Replace cluster linearization algorithm with SFL":
(https://github.com/bitcoin/bitcoin/pull/32545#discussion_r2615795954)
Good observation, I have added a comment.

(historically, `MakeTopological` was actually using `MergeSequence` before an overhaul, which may explain why this wasn't explained already)

💬 sipa commented on pull request "Replace cluster linearization algorithm with SFL":
(https://github.com/bitcoin/bitcoin/pull/32545#discussion_r2615796069)
Done.

💬 sipa commented on pull request "Replace cluster linearization algorithm with SFL":
(https://github.com/bitcoin/bitcoin/pull/32545#discussion_r2615796436)
This was a result of commit surgery gone wrong. Fixed.

💬 l0rinc commented on pull request "Make `transaction_indentifier` hex string constructor evaluated at comptime":
(https://github.com/bitcoin/bitcoin/pull/34063#discussion_r2615797682)
Had a similar attempt in https://github.com/bitcoin/bitcoin/pull/31991#discussion_r1995691590 - what do you think @maflcko?

💬 sipa commented on pull request "Replace cluster linearization algorithm with SFL":
(https://github.com/bitcoin/bitcoin/pull/32545#issuecomment-3648454810)
I have addressed most of @instagibbs's comments above, and also rewritten `SpanningForestState::SanityCheck` to no longer rely on walking clusters. It's a bit more verbose now, but maybe easier to follow.

⚠️ sajadvilsin-cmyk opened an issue: "ok"
(https://github.com/bitcoin/bitcoin/issues/34065)

⚠️ sajadvilsin-cmyk opened an issue: "ok"
(https://github.com/bitcoin/bitcoin/issues/34066)

⚠️ sajadvilsin-cmyk opened an issue: "ok"
(https://github.com/bitcoin/bitcoin/issues/34067)

⚠️ sajadvilsin-cmyk opened an issue: "ok"
(https://github.com/bitcoin/bitcoin/issues/34068)

⚠️ sajadvilsin-cmyk opened an issue: "ok"
(https://github.com/bitcoin/bitcoin/issues/34069)