Skip to content

Commit e5b999d

Browse files
TheBlueMattwpaulino
authored andcommitted
Fix matching of second-stage HTLC claim in get_htlc_balance
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.
1 parent 620244d commit e5b999d

File tree

1 file changed

+14
-3
lines changed

1 file changed

+14
-3
lines changed

lightning/src/chain/channelmonitor.rs

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1751,7 +1751,19 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
17511751
},
17521752
OnchainEvent::MaturingOutput {
17531753
descriptor: SpendableOutputDescriptor::DelayedPaymentOutput(ref descriptor) }
1754-
if descriptor.outpoint.index as u32 == htlc_commitment_tx_output_idx => {
1754+
if event.transaction.as_ref().map(|tx| tx.input.iter().enumerate()
1755+
.any(|(input_idx, inp)|
1756+
Some(inp.previous_output.txid) == confirmed_txid &&
1757+
inp.previous_output.vout == htlc_commitment_tx_output_idx &&
1758+
// A maturing output for an HTLC claim will always be at the same
1759+
// index as the HTLC input. This is true pre-anchors, as there's
1760+
// only 1 input and 1 output. This is also true post-anchors,
1761+
// because we have a SIGHASH_SINGLE|ANYONECANPAY signature from our
1762+
// channel counterparty.
1763+
descriptor.outpoint.index as usize == input_idx
1764+
))
1765+
.unwrap_or(false)
1766+
=> {
17551767
debug_assert!(holder_delayed_output_pending.is_none());
17561768
holder_delayed_output_pending = Some(event.confirmation_threshold());
17571769
},
@@ -1892,8 +1904,7 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitor<Signer> {
18921904
/// confirmations on the claim transaction.
18931905
///
18941906
/// Note that for `ChannelMonitors` which track a channel which went on-chain with versions of
1895-
/// LDK prior to 0.0.111, balances may not be fully captured if our counterparty broadcasted
1896-
/// a revoked state.
1907+
/// LDK prior to 0.0.111, not all or excess balances may be included.
18971908
///
18981909
/// See [`Balance`] for additional details on the types of claimable balances which
18991910
/// may be returned here and their meanings.

0 commit comments

Comments
 (0)