feat(chain): add new list_ordered_canonical_txs method

[oleonardolima]

Description

It introduces a new method for topological ordering canonical transactions, list_ordered_canonical_txs. It now ensures the dependency-based transaction processing, guaranteeing that parent transactions always appears before their children transaction.

The existing list_canonical_txs and try_list_canonical_txs methods have been deprecated in favor of the new ordered version.

Notes to the reviewers

"[...] For those reviewing, and wondering why we don't have a fallible try version of this method, it's because we don't have a fallible ChainOracle implementation - we will get rid of ChainOracle trait soon anyway."

This PR is intended for a point release so that bdk_wallet 2.x users can get a topologically sorted list of transactions (we need a point release on bdk_wallet 2.x as well).

It might be useful to take a look at the new test scenarios that I've added, it shows some specific scenarios where the current implementation and output of canonical_txs didn't output the transactions in topological order.

Let me know if you think the TopologicalIter algorithm and/or API could be improved.

Changelog notice

#### Added
- New `list_ordered_canonical_txs` method to `TxGraph` that returns canonical transactions in topological order, ensuring parent transactions always appear before their children

#### Deprecated
- `list_canonical_txs` method - use `list_ordered_canonical_txs` instead for guaranteed topological ordering
- `try_list_canonical_txs` method - use `list_ordered_canonical_txs` instead for guaranteed topological ordering

Checklists

All Submissions:

• I followed the contribution guidelines

New Features:

• I've added tests for the new feature
• I've added docs for the new feature

[evanlinjin]

Nice diagrams

[evanlinjin]

The internals have completely changed. I don't think a point release would make any sense (we also have already merged other breaking changes). I recommend to just break the API.

Get rid of CanonicalIter. Just have a single function that returns impl Iterator<Item = ...>.

[ValuedMammal]

I don't think you have to deprecate list_canonical_txs. It should just return the txs in topological order. CanonicalIter already canonicalizes the graph topologically (albeit reversed), only that it has an issue with splitting packages of related txs that should be canonicalized as a single batch.

[oleonardolima]

I don't think you have to deprecate list_canonical_txs. It should just return the txs in topological order. [...]

I think it's good to guide users towards the method with correct output, but I'm fine with keeping both too.

CanonicalIter already canonicalizes the graph topologically (albeit reversed), only that it has an issue with splitting packages of related txs that should be canonicalized as a single batch.

It's not only reversed, but as we start canonicalizing from any given anchored tx and walk ancestors, it fails to exhaust all the spending ones from it and it's ancestors.

[evanlinjin]

@ValuedMammal That's a good point about not deprecating list_canonical_txs since it still serves a purpose. We should update it's docs to direct users to the new method; list_ordered_canonical_txs.

Doing CanonicalIter.collect::<Vec<_>>().into_iter().rev() will not pass test_list_canonical_txs_topologically.

[evanlinjin]

Looks good. Happy to ACK after we un-deprecate list_canonical_txs and update it's docs to point to list_ordered_canonical_txs.

Also, shouldn't this PR point to the chain-0.23.x branch?

[oleonardolima]

Looks good. Happy to ACK after we un-deprecate list_canonical_txs and update it's docs to point to list_ordered_canonical_txs.

Also, shouldn't this PR point to the chain-0.23.x branch?

Thanks, I've removed the deprecation in 9664402.

It's valid to note that before merging the release/chain-23.x branch needs to be updated and then this PR base should be updated as well. As it will be the same commit for this base, a rebase shouldn't be needed.

[ValuedMammal]

ACK tested locally, and I agree with the test expectations. No real opinion on deprecating list_canonical_txs. I looked for a way to rework CanonicalIter but realized it will be better to just add an extra layer of processing that does the topological sort.