From: 志宇 <hello@evanlinjin.me>
Date: Thu, 6 Mar 2025 00:05:57 +0000 (+1100)
Subject: Merge bitcoindevkit/bdk#1798: Refactor/use iterators to preselect utxos
X-Git-Tag: wallet-1.2.0~19
X-Git-Url: http://internal-gitweb-vhost/script/%22https:/database/scripts/enum.FromScriptError.html?a=commitdiff_plain;h=1975835f37629cc3697fcd7930b1786538437037;p=bdk

Merge bitcoindevkit/bdk#1798: Refactor/use iterators to preselect utxos

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
---

1975835f37629cc3697fcd7930b1786538437037