test: Move bump fee + foreign utxo tests

[ValuedMammal]

These files are added to wallet/tests directory

• add_foreign_utxo.rs
• persisted_wallet.rs
• build_fee_bump.rs
• common.rs

fixes #238

Checklists

All Submissions:

• I followed the contribution guidelines

[coveralls]

Pull Request Test Coverage Report for Build 15828049596

Details

• 0 of 0 changed or added relevant lines in 0 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage remained the same at 85.55%

---

Totals
Change from base Build 15719064446:	0.0%
Covered Lines:	7377
Relevant Lines:	8623

---

💛 - Coveralls

[tvpeter]

Could you consider separating the taproot-related test cases into a different module? There are many test cases associated with taproot.

[ValuedMammal]

About this, I was thinking about more ways to separate the tests. In addition to the build_fee_bump and add_foreign_utxo tests, we could have a tests/persisted_wallet.rs file for the persistence related tests.

In general it makes sense for tests to be grouped according to the functional unit being tested, so for example the tests for creating transactions/PSBTs can go together, but I think further separating files by address type (legacy, segwit, etc) is less important.

For now the tests/wallet.rs file still contains all of the tests related to addresses, creating txs, and signing. I tried to minimize the disruption and potential conflict with other PRs. Any other suggestions, let me know @tvpeter @oleonardolima.

[tvpeter]

Yes, this is more ideal

In general it makes sense for tests to be grouped according to the functional unit being tested, so for example the tests for creating transactions/PSBTs can go together, but I think further separating files by address type (legacy, segwit, etc) is less important.

[oleonardolima]

For now the tests/wallet.rs file still contains all of the tests related to addresses, creating txs, and signing. I tried to minimize the disruption and potential conflict with other PRs. Any other suggestions, let me know @tvpeter @oleonardolima.

I agree with the changes from this PR, good work!

As you've mentioned in the call, I also think that the tests related exclusively to signing could be moved to a specific module (it can be done in a follow-up PR or kept as it is). However, they'll probably be removed once the signing feature is removed, and we're relying on rust-bitcoin's Psbt::sign. Maybe it'll make it easier to remove? NBD though.

[tvpeter]

tACK 22e28b2

Thank you for working on this Mammal.

I did not proceed with #238 since they are related.

There are some warnings related to bitcoin::key::TweakedKeypair::to_inner but once you rebase, they should be gone.

I have left a comment on whether it is a good idea to separate taproot-related test cases into their module.

Thank you

[tvpeter]

tACK ea39339

[oleonardolima]

ACK ea39339