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
TheBlueMatt 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 numberDiff line numberDiff 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 DownExpand 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() {

@valentinewallacevalentinewallaceAug 25, 2022

Copy link
Copy Markdown
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

@TheBlueMattTheBlueMattAug 25, 2022

Copy link
Copy Markdown
CollaboratorAuthor

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.

@valentinewallacevalentinewallaceAug 25, 2022

Copy link
Copy Markdown
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 DownExpand 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