Bundles

[cymqqqq]

Hi Team.
I made some changes to the sort_bundles in the stock.rs module.

What I changed:

1. Replaced duplicate dependency scans with a single pass (also builds DAG links once) in stock.rs:671
2. Replaced the previous repeated-subset/remove loop with deterministic Kahn topological sort in stock.rs:723
3. Fixed behavior to actually ignore missing input bundles BundleAbsent as the comment intended, useful for replace-style histories, in stock.rs:692
4. Kept BundlesInconsistency on cycles/corrupt graphs, with explicit consistency checks in stock.rs:760.

The final optimizations:

1. Less repeated code (no second full transition/input scan).
2. Better asymptotic behavior for larger bundle sets.
3. Deterministic output order still respects witness-height ordering when ties are possible.
   Your team can review the code to confirm if it makes sense.
   If rejected, I will close this PR :)
   Thanks!

[St333p]

Hi, thanks for your PR!

Before heading towards a detailed review, I'd try to align the high-level understanding of the current implementation and its room for improvement.

1. The dependency scan is split into two passes since we expect the typical cases to not need topological sort, since transition chains mined within a single block (or in reverse order, which is possible in the blinded case) seem to be pretty rare. So the current logic optimizes for the typical case by not allocating the dependency map if not needed. do you think our assumption does not hold in practice or do you consider the overhead of building the dependency map in the average case small enough to justify the performance gain in the worst case scenario (when topological sort is indeed required)?
2. The overall idea on topological sort looks pretty similar to me, the current implementation is also based on Kahn. I reckon that your version looks a bit more intuitive, but it also requires more preprocessing (L746:771) that cycles all bundles many times over, so performance gain does not look obvious to me, could you provide some reasoning or benchmark for it?
   Also not sure why you mention determinism, are you implying that the current implementation is not deterministic? If so, could you expand on that?
3. Could you describe a scenario in which the current code would break (best with a test case)? Besides, we recently removed the experimental support for replace transitions since it turned out to be broken, which was the only scenario in which missing bundles could have been acceptable. So we are considering to change this to stop ignoring them and return an error instead.
4. Some of your consistency checks seem to be checking the consistency of the data structures that were built within the method itself, which doesn't look too useful to me. The IndexInconsistency error is more meant to report inconsistencies in the wallet storage, which should signal the wallet to exercise extra care to avoid fund loss.

So, with regard to your stated optimizations:

1. Repeated code in this case allows to optimize for the typical scenario, so I'd like to have good reasons to remove it
2. Better asymptotic behavior needs to be proven
3. Lack of determinism or respect for witness-height ordering in the current implementation needs to be proven