Preserve insertion order of manually selected utxos if TxOrdering::Untouched

[nymius]

Description

On my attempt to fix bitcoindevkit/bdk#1794 in bitcoindevkit/bdk#1798, I broke the assumption that insertion order is preserved when TxBuilder::ordering is TxOrdering::Untouched. Some users are relying in this assumption, so here I'm trying to restore it back, without adding a new dependency for this single use case like #252, or creating a new struct just to track this.

In this fourth alternative solution I'm going back to use Vec to store the manually selected UTxOs.

I was reluctant to do it in this way because HashMap seems a better solution giving its property of avoiding duplicates, but as we also want to keep the secuential nature of the insertion of UTxOs in TxBuilder, here is an alternative aligned with that principle.

May replace #252
May replace #261 .
Fixes #244

Notes to the reviewers

Also, as I was working on this, I came back to the following tests:

• test_prexisting_foreign_utxo_have_no_precedence_over_local_utxo_with_same_outpoint
• test_prexisting_local_utxo_have_precedence_over_foreign_utxo_with_same_outpoint

Motivated during the implementation and review of bitcoindevkit/bdk#1798.

Which required the underlying structure to also hold the properties of no duplication for manually selected UTxOs, as the structures were accessed directly for these cases.

The test tries to cover the case when there are two wallets using the same descriptor, one tracks transactions and the other does not. The first passes UTxOs belonging to the second one and this one creates transactions using the add_foreign_utxo interface.
In this case it was expected for any LocalUtxo of the offline wallet to supersede any conflicting foreign UTxO. But, the simulation of this case went against the borrowing constraints of rust.
By how costly was to reproduce this behavior for me in the tests, I would like to have second opinions in the feasibility of the test case.

Changelog notice

No public APIs are changed by these commits.

Checklists

Important

This pull request DOES NOT break the existing API

• I've signed all my commits
• I followed the contribution guidelines
• I ran cargo +nightly fmt and cargo clippy before committing
• I've added tests for the new code
• I've expanded docs addressing new code
• I've added tests to reproduce the issue which are now passing
• I'm linking the issue being fixed by this PR

[coveralls]

Pull Request Test Coverage Report for Build 15949866545

Details

• 109 of 120 (90.83%) changed or added relevant lines in 2 files are covered.
• 14 unchanged lines in 5 files lost coverage.
• Overall coverage decreased (-1.1%) to 84.455%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
wallet/src/wallet/tx_builder.rs	83	84	98.81%
wallet/src/wallet/mod.rs	26	36	72.22%

Files with Coverage Reduction	New Missed Lines	%
wallet/src/descriptor/dsl.rs	1	95.34%
wallet/src/wallet/changeset.rs	2	85.0%
wallet/src/descriptor/policy.rs	3	79.07%
wallet/src/descriptor/template.rs	4	98.04%
wallet/src/wallet/mod.rs	4	81.03%

Totals
Change from base Build 15719064446:	-1.1%
Covered Lines:	6427
Relevant Lines:	7610

---

💛 - Coveralls

[ValuedMammal]

I would like to have second opinions in the feasibility of the test case.

Since our logic is meant to enforce the precedence of local utxos, it makes sense to keep the test, but I agree there could be a simpler, easy to maintain version of it. ValuedMammal/bdk_wallet@49b5cef

[ValuedMammal]

For consistency with the rest of the codebase I want to stick with the "UTXO" style acronym.

[nymius]

Thanks for the review!
I think I have addressed all the comments.
I just changed the occurrences of {utxos,UTxOs} in the lines related to the changes I did, to avoid cluttering the diff with other lines not related to the present PR.

[ValuedMammal]

ACK d6d204b

[oleonardolima]

utACK

I left a question and a single nit, but overall it looks good to me! Agree with the new version of test suggested by VM's, it looks much better.

[ValuedMammal]

reACK 0522114

[oleonardolima]

ACK 0522114