wallet: Add ConfirmationSpendPolicy to spend only [un]confirmed outputs

[stevenroose]

Description

Notes to the reviewers

Changelog notice

Checklists

All Submissions:

• I've signed all my commits
• I followed the contribution guidelines
• I ran cargo fmt and cargo clippy before committing

New Features:

• I've added tests for the new feature
• I've added docs for the new feature

Bugfixes:

• This pull request breaks the existing API
• I've added tests to reproduce the issue which are now passing
• I'm linking the issue being fixed by this PR

[stevenroose]

How do I run cargo fmt? If I do simply that, it changes almost all files.

[ErikDeSmedt]

This should close #1741

[ValuedMammal]

How do I run cargo fmt? If I do simply that, it changes almost all files.

We use the default rustfmt config (no toml file) using the stable toolchain set to the current rust_version, so cargo fmt should be enough..

[stevenroose]

It seems like this codebase doesn't allow using .find().is_some()? The CI says "help: consider using ...", sounds like a friendly suggestion, but it still gives a big red cross 😅

[stevenroose]

It's in a unit test even for crying out loud.

[stevenroose]

error: called `is_none()` after searching an `Iterator` with `find`
   --> crates/wallet/tests/wallet.rs:845:13
    |
845 |       assert!(ret
    |  _____________^
846 | |         .unsigned_tx
847 | |         .input
848 | |         .iter()
849 | |         .find(|i| i.previous_output.txid == confirmed_txid)
850 | |         .is_none());
    | |__________________^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#search_is_some
help: consider using
    |
845 ~     assert!(!ret
846 +         .unsigned_tx
847 +         .input
848 ~         .iter().any(|i| i.previous_output.txid == confirmed_txid));

sigh how is

assert!(!ret
    ... 5 lines later
    any());

More readable than using .iter().find(...).is_none()? That negation ! right after the macro ! is barely visible.

[stevenroose]

Fwiw, I pushed a commit to comply with CI. Didn't put my name on it, though.

[ValuedMammal]

Thanks for working on this @stevenroose

[stevenroose]

The CI failures seem to be about stuff I didn't touch. Can they be restarted?

[stevenroose]

Rebased.

[oleonardolima]

cACK 1bdf729

Thanks for working on this one, it looks pretty straight-forward. I was wondering about the name, but couldn't get to anything better 🤔 (should we stick with ...Forbidden usage as in ChangePolicy?).

I'd squash it into a single commit, and add the feat(wallet): prefix for conventional commits though.

[stevenroose]

Hmm, well, Change is more binary than Confirmed, I felt. You mean doing UnconfirmedAllowed, UnconfirmedForbidden, OnlyUnconfirmed but then the height one becomes awkward, ConfirmedAfterForbidden or something? Idk, I feel like the names with Only make more sense if you can say "confirmed" vs "unconfirmed". With change that's harder, you'd have to say OnlyNotChange or so 😅

Anyway, semantics, happy to bikeshed more :)

[stevenroose]

When it comes to squashing, I prefer not to attach my name to code I would not have written.. Like this

If you insist, I can put a disclaimer in my commit message. Since open-source is largely based on voluntary contributions, we should at least be entitled to pride in our work :) I'll put the prefix in.