2f83b4508fe2369c6336c9579517f3a4eb25d923 test(wallet): check there are no duplicates across required and optional utxos (nymius)
39df2b940a161d6b5468e6a6c36f888c14a3e24e refactor(wallet): remove coin_selection::filter_duplicates (nymius)
79bd7da87603c8dfe65bee4bd7061c56e39d0953 refactor(wallet): use iterators and adaptors in preselect_utxos (nymius)
Pull request description:
### Description
There were multiple calls for de-duplication of selected UTxOs in `Wallet::create_tx`: ([1](https://github.com/bitcoindevkit/bdk/blob/
abc305612160c0e2ce85c7bba4c2c162ff488adc/crates/wallet/src/wallet/mod.rs#L2016-L2020)) and ([2](https://github.com/bitcoindevkit/bdk/blob/
abc305612160c0e2ce85c7bba4c2c162ff488adc/crates/wallet/src/wallet/mod.rs#L1452)).
As the test [`test_filter_duplicates`](https://github.com/bitcoindevkit/bdk/blob/master/crates/wallet/src/wallet/coin_selection.rs#L1666-L1695) shows, there are four possible cases for duplication of UTxOs while feeding the coin selection algorithms.
1. no duplication: out of concern
2. duplication in the required utxos only: covered by the source of `required_utxos`, `Wallet::list_unspent`, which [roots back the provided `UTxOs` to a `HashMap`](https://github.com/bitcoindevkit/bdk/blob/
a5335a18431dfecea1f524af44c953b79a96867c/crates/chain/src/tx_graph.rs#L911-L912) which should avoid any duplication by definition
3. duplication in the optional utxos only: is the only one possible as optional `UTxOs` are stored in a `Vec` and no checks are performed about the duplicity of their members.
4. duplication across the required and optional utxos: is already covered by `Wallet::preselect_utxos`, which avoid the processing of required UTxOs while listing the unspent available UTxOs in the wallet.
This refactor does the following:
- Refactors `TxParams::utxos` type to be `HashSet<LocalOutput>` avoiding the duplication case 3
- Keeps avoiding case 4 without overhead and allowing a query closer to O(1) on avg. to cover duplication case 4 (before was O(n) where n is the size of required utxos) thanks to the above change.
- Still covers case 2 because the [source of `required_utxos`, `Wallet::list_unspent`](https://github.com/bitcoindevkit/bdk/blob/
a5335a18431dfecea1f524af44c953b79a96867c/crates/chain/src/tx_graph.rs#L911-L912) comes from a `HashMap` which should avoid duplication by definition.
- Moves the computation of the `WeightedUtxos` to the last part of UTxO filtering, allowing the unification of the computation for local outputs.
- Removes some extra allocations done for helpers structures or intermediate results while filtering UTxOs.
- Allows for future integration of UTxO filtering methods for other utilities, e.g.: filter locked utxos
```rust
.filter(|utxo| !self.is_utxo_locked(utxo.outpoint, self.latest_checkpoint().height()))
```
- Adds more comments for each filtering step.
- Differentiates `foreign_utxos`, which should include a provided satisfation weight to use them effectively, and `utxos`, manually selected UTxOs for which the wallet can compute their satisfaction weight without external resources.
With these changes all four cases would be covered, and `coin_selection::filter_duplicates` is no longer needed.
Fixes #1794.
### Notes to the reviewers
I added three test to cover the interesting cases for duplication:
- there are no duplicates in required utxos
- there are no duplicates in optional utxos
- there are no duplicates across optional and required utxos
the three of them have been prefixed with `not_duplicated_utxos*` to allow its joint execution under the command:
```bash
cargo test -- not_duplicated_utxos
```
because the guarantees for the three conditions above are spread in different parts of the code.
### Changelog notice
No changes to public APIs.
### Checklists
* [x] I've signed all my commits
* [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
* [x] I ran `cargo fmt` and `cargo clippy` before committing
* [x] This pull request does not break the existing API
* [x] I've added tests to reproduce the issue which are now passing
* [x] I'm linking the issue being fixed by this PR
ACKs for top commit:
evanlinjin:
ACK
2f83b4508fe2369c6336c9579517f3a4eb25d923
Tree-SHA512: 82317acce8a7e83e4ea1a500605142026ab10d6842d4f278b09563566f1eb4d295f77a9c9616eff067211f0faa9f1e94370fc0ec3af48e6e1baef53b46979db7