Add ability to set shutdown script when closing channel

[benthecarman]

Closes #2216

This will require the user to have commit_upfront_shutdown_pubkey = false in their config when they open the channel. If this is being supported it may be a good idea to set this to default false instead. In the docs it explains that setting commit_upfront_shutdown_pubkey = true by default doesn't add any real security so maybe this is a good idea.

[codecov-commenter]

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.02 🎉

Comparison is base (101c09f) 91.57% compared to head (5f4efcc) 91.60%.

❗ Current head 5f4efcc differs from pull request most recent head 12b59b2. Consider uploading reports for the commit 12b59b2 to get more accurate results

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2219      +/-   ##
==========================================
+ Coverage   91.57%   91.60%   +0.02%     
==========================================
  Files         104      104              
  Lines       51990    52010      +20     
  Branches    51990    52010      +20     
==========================================
+ Hits        47610    47643      +33     
+ Misses       4380     4367      -13     

Impacted Files	Coverage Δ
lightning/src/ln/channel.rs	90.08% <100.00%> (+0.16%)	⬆️
lightning/src/ln/channelmanager.rs	88.80% <100.00%> (ø)
lightning/src/ln/shutdown_tests.rs	98.15% <100.00%> (+0.09%)	⬆️

... and 3 files 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.

[TheBlueMatt]

Hmm, you can already do this via SignerProvider::get_shutdown_scriptpubkey, which is only called after shutdown if commit_upfront_shutdown_pubkey is unset. Its indeed a bit nicer to pass it to the shutdown method but its kinda awkward we'd end up with several ways to pass the shutdown script in.

If we want to move it out of the SignerProvider, we could consider moving it into the config explicitly (ie making commit_upfront_shutdown_pubkey an optional script instead) and then requiring the user pass it in via shutdown, as here. We'd also like to merge SignerProvider::get_destination_script and SignerProvider::get_shutdown_script, though, which this may prevent :/

[benthecarman]

For context the reason I'd like something like this is we want to keep our impl of get shutdown script that goes to our onchain wallet. But will have cases where we want to close a channel directly to another wallet. To do this by using the signing provider wouldn't really work (we don't know which channel is requesting an address) and it'd be much better to be able to pass it in on demand.

[TheBlueMatt]

Hmm, I'd really like to avoid having a way to set this in two places, especially given its already the case that we may or may not use it. Would it suffice for you if the SignerProvider::get*_script methods took the channel_id (which they really should...)?

[benthecarman]

Hmm, I'd really like to avoid having a way to set this in two places, especially given its already the case that we may or may not use it. Would it suffice for you if the SignerProvider::get*_script methods took the channel_id (which they really should...)?

With that we could hack it together (we'd probably just have a map in memory of channel ids and addresses) but i feel like this api would be cleaner on the user side.

[TheBlueMatt]

I still hate this, but I think I hate it less than the crap our users would have to do to accomplish this if we don't do it, so I think I've come around.

[TheBlueMatt]

Needs rebase after your err pr landed.

[benthecarman]

rebased + responded to @tnull's nits