Prefer Utxo::Local over Utxo::Foreign in OldestFirstCoinSelection

[nymius]

Description

The comments in the OldestFirstCoinSelection implementation stated the following:

// For utxo that doesn't exist in DB, they will have lowest priority to be selected

But this was not honored in the code.

This PR enforces this and ensures the expected behaviour through the two following new tests:

• test_oldest_first_coin_selection_uses_all_optional_with_foreign_utxo_locals_sorted_first
• test_oldest_first_coin_selection_uses_only_all_optional_local_utxos_not_a_single_foreign

Fixes #264

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 15949711702

Details

• 68 of 68 (100.0%) changed or added relevant lines in 1 file are covered.
• 14 unchanged lines in 5 files lost coverage.
• Overall coverage decreased (-0.9%) to 84.663%

---

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.04%

Totals
Change from base Build 15719064446:	-0.9%
Covered Lines:	6525
Relevant Lines:	7707

---

💛 - Coveralls

[MaxFangX]

Thanks for the quick fix!

[nymius]

@MaxFangX thanks for looking at the code so close!
Just for clarification, if you're using TxBuilder to create transactions, the only way to have foreign UTxOs in your selection is through the add_foreign_utxo method, which adds UTxOs to the manual selected list. That list would be passed to the coin selection implementation as the required_utxos. So, there is no way through that API to stumble upon this issue.
If you are not using TxBuilder::create_tx, then you could create your optional_utxos with as many foreign UTxOs as you want, and in that case the outcome would be affected.

[evanlinjin]

ACK d23d07b

[oleonardolima]

utACK 500120c

[ValuedMammal]

ACK 345cc6e

[oleonardolima]

ACK 345cc6e