Fix matching of second-stage HTLC claim in get_htlc_balance

[wpaulino]

We incorrectly assumed that the descriptor's output index from second-stage HTLC transaction would always match the HTLC's output index in the commitment transaction. This doesn't make any sense though, we need to make sure we map the descriptor to it's corresponding HTLC in the commitment. Instead, we check that the transaction from which the descriptor originated from spends the HTLC in question.

Note that pre-anchors, second-stage HTLC transactions are always 1 input-1 output, so previously we would only match if the HTLC was the first output in the commitment transaction. Post-anchors, they are malleable, so we can aggregate multiple HTLC claims into a single transaction making this even more likely to happen. Unfortunately, we lacked proper coverage in this area so the bug went unnoticed. To address this, we aim to extend our existing coverage of get_claimable_balances to anchor outputs channels in the following commits.

---

Depends on #2605 and #2606.

This replaces #2593. It turned out that only the second commit was necessary, as the third commit is just another way of doing the second commit, and the first commit is only a pre-requisite for the third commit. Rather than changing HTLC amounts to ensure proper coverage when HTLCs have a different output index than 0 in the commitment transaction, we extend the existing get_claimable_balances test to cover anchor outputs channels instead. This automatically results in HTLCs having a different output index than 0 because the two anchor outputs in commitment transactions usually come first due to the output sorting.

[codecov-commenter]

Codecov Report

Attention: 10 lines in your changes are missing coverage. Please review.

Comparison is base (620244d) 89.05% compared to head (fd66a29) 89.00%.
Report is 9 commits behind head on main.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2610      +/-   ##
==========================================
- Coverage   89.05%   89.00%   -0.06%     
==========================================
  Files         112      112              
  Lines       86705    86930     +225     
  Branches    86705    86930     +225     
==========================================
+ Hits        77217    77369     +152     
- Misses       7259     7330      +71     
- Partials     2229     2231       +2     

Files	Coverage Δ
lightning/src/chain/channelmonitor.rs	84.72% <100.00%> (-0.14%)	⬇️
lightning/src/ln/functional_test_utils.rs	91.38% <81.81%> (-0.11%)	⬇️
lightning/src/ln/monitor_tests.rs	98.06% <98.17%> (-0.44%)	⬇️

... and 12 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[ariard]

Post-merge Code Review ACK fd66a29

Didn’t look into the tests deeply, though this code approach is far better than #2593 as now we have one code branch to handle balance detection of maturing output of second-stage HTLC transactions.

[ariard]

To be canonical this is a property of the transaction signature bip143 digest algorithm where the sha_single_output is computed from the index of the spent input. This is also true for bip341 digest algorithm, however if transaction digest algorithm computation of sighash flags change in the future, we would have to update this code in consequence (and indeed more complicated sighash flags have been proposed in the past).