Skip to content

Expose a Balance for inbound HTLCs even without a preimage #1673

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 25, 2022
Merged

Expose a Balance for inbound HTLCs even without a preimage #1673

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4c92313
Expose a `Balance` for inbound HTLCs even without a preimage
TheBlueMatt Aug 17, 2022
39cede6
Rename MaybeClaimableHTLCAwaitingTimeout for consistency
TheBlueMatt Aug 24, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 28 additions & 5 deletions lightning/src/chain/channelmonitor.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -578,14 +578,25 @@ pub enum Balance {
/// HTLCs which we sent to our counterparty which are claimable after a timeout (less on-chain
/// fees) if the counterparty does not know the preimage for the HTLCs. These are somewhat
/// likely to be claimed by our counterparty before we do.
MaybeClaimableHTLCAwaitingTimeout {
/// The amount available to claim, in satoshis, excluding the on-chain fees which will be
/// required to do so.
MaybeTimeoutClaimableHTLC {
/// The amount potentially available to claim, in satoshis, excluding the on-chain fees
/// which will be required to do so.
claimable_amount_satoshis: u64,
/// The height at which we will be able to claim the balance if our counterparty has not
/// done so.
claimable_height: u32,
},
/// HTLCs which we received from our counterparty which are claimable with a preimage which we
/// do not currently have. This will only be claimable if we receive the preimage from the node
/// to which we forwarded this HTLC before the timeout.
MaybePreimageClaimableHTLC {
/// The amount potentially available to claim, in satoshis, excluding the on-chain fees
/// which will be required to do so.
claimable_amount_satoshis: u64,
/// The height at which our counterparty will be able to claim the balance if we have not
/// yet received the preimage and claimed it ourselves.
expiry_height: u32,
},
/// The channel has been closed, and our counterparty broadcasted a revoked commitment
/// transaction.
///
Expand Down Expand Up @@ -1542,7 +1553,7 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
confirmation_height: conf_thresh,
});
} else {
return Some(Balance::MaybeClaimableHTLCAwaitingTimeout {
return Some(Balance::MaybeTimeoutClaimableHTLC {
claimable_amount_satoshis: htlc.amount_msat / 1000,
claimable_height: htlc.cltv_expiry,
});
Expand All @@ -1565,6 +1576,11 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
timeout_height: htlc.cltv_expiry,
});
}
} else if htlc_resolved.is_none() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fwiw, tests pass when this is just else

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, right, it'd be unreachable otherwise as we have a separate else clause for resolved.is_some && !spend_pending (and spend_pending would require that we have the preimage, which ends up in the previous clause). I'm still gonna leave it as-is if that's okay.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sg, it reads fine IMO

return Some(Balance::MaybePreimageClaimableHTLC {
claimable_amount_satoshis: htlc.amount_msat / 1000,
expiry_height: htlc.cltv_expiry,
});
}
None
}
Expand Down Expand Up @@ -1722,12 +1738,19 @@ impl<Signer: Sign> ChannelMonitor<Signer> {
for (htlc, _, _) in us.current_holder_commitment_tx.htlc_outputs.iter() {
if htlc.transaction_output_index.is_none() { continue; }
if htlc.offered {
res.push(Balance::MaybeClaimableHTLCAwaitingTimeout {
res.push(Balance::MaybeTimeoutClaimableHTLC {
claimable_amount_satoshis: htlc.amount_msat / 1000,
claimable_height: htlc.cltv_expiry,
});
} else if us.payment_preimages.get(&htlc.payment_hash).is_some() {
claimable_inbound_htlc_value_sat += htlc.amount_msat / 1000;
} else {
// As long as the HTLC is still in our latest commitment state, treat
// it as potentially claimable, even if it has long-since expired.
res.push(Balance::MaybePreimageClaimableHTLC {
claimable_amount_satoshis: htlc.amount_msat / 1000,
expiry_height: htlc.cltv_expiry,
});
}
}
res.push(Balance::ClaimableOnChannelClose {
Expand Down
Loading