Create and Sign PSBTs for spendable outputs

[benthecarman]

Closes #2285

The goal here is to be able to better batch spendable outputs with other transactions. This should let you create and sign PSBTs according to your spendable outputs and only needing the KeyManager for signing.

[TheBlueMatt]

I'm not really a fan of pushing every spend of this through the PSBT flow, but having a PSBT flow option is definitely good. If we're gonna do this though we should really support using the PSBT to provide the outputs + change output, in addition to excess inputs. Really maybe we should return a set of PSBT inputs which the user can munge into a PSBT and then we can sign it, I think (or maybe we just need a method to convert a descriptor to a PSBT input)? I admit I'm not super farmiliar with "normal" PSBT flows.

[benthecarman]

Really maybe we should return a set of PSBT inputs which the user can munge into a PSBT and then we can sign it, I think (or maybe we just need a method to convert a descriptor to a PSBT input)? I admit I'm not super farmiliar with "normal" PSBT flows.

PSBTs have kinda been bastardized. No one really passes around the individual pieces, normally it is just passing around whole PSBTs with the information you need in them and then merging PSBTs together until you get the transaction you want. PSBTv2 fixes this but hasn't really seen any adoption.

[benthecarman]

If we're gonna do this though we should really support using the PSBT to provide the outputs + change output, in addition to excess inputs.

Yeah I think that could be a good idea, will try and implement that.

[benthecarman]

If we're gonna do this though we should really support using the PSBT to provide the outputs + change output, in addition to excess inputs.

Yeah I think that could be a good idea, will try and implement that.

Hmm, this actually is more complicated than expected, most of it isn't too hard but we need to handle the case where the user provides a PSBT with an input (or inputs) that do not have the previous utxo available, if we don't have that information we can't calculate the total input value, change amt, fee rate, etc. We also will be unable to expected_max_weight because we won't know how the user will sign the other inputs and what possible hidden script paths it could have. Imo the current API is probably simpler for the user given all the edge cases, they get a PSBT and they can handle it in whatever way they like.

[TheBlueMatt]

Grr, I'd really expect those kinds of utilities to be either in BDK or in rust-bitcoin - we should be able to return the descriptors convert them to psbt inputs, then call one bdk function which munges it all together and applies the right fee...at a minimum I'd like to have such a utility available (that converts descriptors into psbt inputs) and then build something that turns those into a transactions for users and try to upstream it to rust-bitcoin/BDK.

That said, we can probably just not support not having the input transaction in the PSBT.

[benthecarman]

Added a helper function for creating a psbt input. Still think the functions I already had will be useful for the majority of users.

[TheBlueMatt]

This LGTM, I think, its somewhat unrelated to this specific PR, but we really need to move to an error enum here and not just return Err(()), if you want to do it here I'd appreciate it but if not that's okay. Either way lets get another reviewer.

[TheBlueMatt]

Oh these don't have outpoints? Ugh, oh well, still probably useful just annoying.

[benthecarman]

Yeah PSBTv0 has a lot of weird stuff like that

[codecov-commenter]

Codecov Report

Patch coverage: 91.35% and project coverage change: -0.02 ⚠️

Comparison is base (f569e9f) 90.94% compared to head (a079ed4) 90.92%.

❗ Current head a079ed4 differs from pull request most recent head 8c0479a. Consider uploading reports for the commit 8c0479a to get more accurate results

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2286      +/-   ##
==========================================
- Coverage   90.94%   90.92%   -0.02%     
==========================================
  Files         104      104              
  Lines       52760    52799      +39     
  Branches    52760    52799      +39     
==========================================
+ Hits        47983    48008      +25     
- Misses       4777     4791      +14     

Impacted Files	Coverage Δ
lightning/src/sign/mod.rs	88.51% <91.35%> (-0.31%)	⬇️

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[dunxen]

Would be cool to enumerate these.

[benthecarman]

Responded to @wpaulino and @dunxen's review.

On the error types, will try to do that in a followup PR, seems kinda out of scope for this PR and it will probably end up being a pretty large change.

[wpaulino]

CI is failing but LGTM otherwise.

[wpaulino]

One more on the anchors build:

error[E0061]: this method takes 6 arguments but 5 arguments were supplied
    --> lightning/src/ln/monitor_tests.rs:2350:49
     |
2350 |             let spend_tx = nodes[0].keys_manager.backing.spend_spendable_outputs(
     |                                                          ^^^^^^^^^^^^^^^^^^^^^^^
2351 |                 &[&outputs[0]], Vec::new(), Script::new_op_return(&[]), 253, &Secp256k1::new(),