Sanity check for ChannelManager and KeysInterface

[vss96]

This fixes #1174.

[TheBlueMatt]

Thanks!

While you're here, can you also add a similar script check in KeysManager::spend_spendable_outputs in the second for loop in the SpendableOutputDescriptor::StaticOutput match variant?

[codecov-commenter]

Codecov Report

Merging #1250 (442d455) into main (34cdca9) will decrease coverage by 0.03%.
The diff coverage is 85.71%.

❗ Current head 442d455 differs from pull request most recent head 2f01d68. Consider uploading reports for the commit 2f01d68 to get more accurate results

@@            Coverage Diff             @@
##             main    #1250      +/-   ##
==========================================
- Coverage   90.42%   90.39%   -0.04%     
==========================================
  Files          70       70              
  Lines       38087    38131      +44     
==========================================
+ Hits        34441    34467      +26     
- Misses       3646     3664      +18     

Impacted Files	Coverage Δ
lightning/src/ln/channelmanager.rs	84.23% <75.00%> (-0.03%)	⬇️
lightning/src/chain/keysinterface.rs	95.81% <100.00%> (+0.05%)	⬆️
lightning/src/routing/router.rs	91.81% <0.00%> (-0.25%)	⬇️
lightning-invoice/src/de.rs	80.73% <0.00%> (-0.21%)	⬇️
lightning/src/ln/functional_tests.rs	97.27% <0.00%> (-0.10%)	⬇️
lightning/src/ln/channel.rs	89.35% <0.00%> (-0.03%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 34cdca9...2f01d68. Read the comment docs.

[vss96]

@TheBlueMatt I created the script from segwit script hash for spendable_outputs:

let payment_script = bitcoin::Address::p2wsh(&witness_script, Network::Testnet).script_pubkey();
assert_eq!(payment_script, output.script_pubkey);

I ran the tests locally and they seem to be failing. Maybe I'm doing something wrong?

[TheBlueMatt]

let payment_script = bitcoin::Address::p2wsh(&witness_script, Network::Testnet).script_pubkey();

This should also be a p2wpkh, not p2wsh.

[vss96]

@TheBlueMatt I'm done with the changes for the pr but I'm facing issues with squashing the commits 😭

[TheBlueMatt]

The Bitcoin Core docs have some discussion of how to do it at https://github.com/bitcoin/bitcoin/blob/master/doc/productivity.md

[vss96]

@TheBlueMatt I was trying to rebase and squash the 6 commits but by the time I'm done with it, the branch is behind by like 390 commits and when I force push I see a lot of other changes.

[TheBlueMatt]

You shouldn't need to rebase unless there is a conflict (there currently is not). If you're using rebase, it may be simplest to rebase on top of 34cdca91baa, which is the current commit this is based on.

[vss96]

That worked! I was trying to do git rebase -i HEAD~6 and that included the merged commits as well and was causing some issues.

[vss96]

@TheBlueMatt Valentine had pointed out how the docs for the methods in keysinterface mention the conditions under which Err is returned, so I went ahead and updated them.

[valentinewallace]

Thanks for the docs updates!

[TheBlueMatt]

Thanks!