Skip to content
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.

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

Include pending HTLC's in ChannelDetails #2442

Merged
merged 1 commit into from
Feb 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions fuzz/src/router.rs
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,8 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
config: None,
feerate_sat_per_1000_weight: None,
channel_shutdown_state: Some(channelmanager::ChannelShutdownState::NotShuttingDown),
pending_inbound_htlcs: Vec::new(),
pending_outbound_htlcs: Vec::new(),
});
}
Some(&$first_hops_vec[..])
Expand Down
314 changes: 314 additions & 0 deletions lightning/src/ln/channel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,72 @@ enum InboundHTLCState {
LocalRemoved(InboundHTLCRemovalReason),
}

/// Exposes the state of pending inbound HTLCs.
///
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we get a brief description of the HTLC state machine at a high level? I dont think we have anywhere else in the API which describes it, and we generally try to avoid APIs which assume knowledge of how lightning works under the hood.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

/// At a high level, an HTLC being forwarded from one Lightning node to another Lightning node goes
/// through the following states in the state machine:
/// - Announced for addition by the originating node through the update_add_htlc message.
/// - Added to the commitment transaction of the receiving node and originating node in turn
/// through the exchange of commitment_signed and revoke_and_ack messages.
/// - Announced for resolution (fulfillment or failure) by the receiving node through either one of
/// the update_fulfill_htlc, update_fail_htlc, and update_fail_malformed_htlc messages.
/// - Removed from the commitment transaction of the originating node and receiving node in turn
/// through the exchange of commitment_signed and revoke_and_ack messages.
///
/// This can be used to inspect what next message an HTLC is waiting for to advance its state.
#[derive(Clone, Debug, PartialEq)]
pub enum InboundHTLCStateDetails {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Once we make this pub we'll need to write a ton of docs - IMO each variant should have, at least, a description of whether this HTLC is included in the current local commitment transaction (ie the one signed by our peer which we can broadcast at any time) and whether it will be included in the next remote commitment transaction (ie the next thing we need to sign that determines if we can send another HTLC or not).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added documentation here based on InboundHTLCState and OutboundHTLCState.

/// We have added this HTLC in our commitment transaction by receiving commitment_signed and
/// returning revoke_and_ack. We are awaiting the appropriate revoke_and_ack's from the remote
/// before this HTLC is included on the remote commitment transaction.
AwaitingRemoteRevokeToAdd,
/// This HTLC has been included in the commitment_signed and revoke_and_ack messages on both sides
/// and is included in both commitment transactions.
///
/// This HTLC is now safe to either forward or be claimed as a payment by us. The HTLC will
/// remain in this state until the forwarded upstream HTLC has been resolved and we resolve this
/// HTLC correspondingly, or until we claim it as a payment. If it is part of a multipart
/// payment, it will only be claimed together with other required parts.
Committed,
/// We have received the preimage for this HTLC and it is being removed by fulfilling it with
/// update_fulfill_htlc. This HTLC is still on both commitment transactions, but we are awaiting
/// the appropriate revoke_and_ack's from the remote before this HTLC is removed from the remote
/// commitment transaction after update_fulfill_htlc.
AwaitingRemoteRevokeToRemoveFulfill,
/// The HTLC is being removed by failing it with update_fail_htlc or update_fail_malformed_htlc.
/// This HTLC is still on both commitment transactions, but we are awaiting the appropriate
/// revoke_and_ack's from the remote before this HTLC is removed from the remote commitment
/// transaction.
AwaitingRemoteRevokeToRemoveFail,
}

impl From<&InboundHTLCState> for Option<InboundHTLCStateDetails> {
fn from(state: &InboundHTLCState) -> Option<InboundHTLCStateDetails> {
match state {
InboundHTLCState::RemoteAnnounced(_) => None,
InboundHTLCState::AwaitingRemoteRevokeToAnnounce(_) =>
Some(InboundHTLCStateDetails::AwaitingRemoteRevokeToAdd),
InboundHTLCState::AwaitingAnnouncedRemoteRevoke(_) =>
Some(InboundHTLCStateDetails::AwaitingRemoteRevokeToAdd),
InboundHTLCState::Committed =>
Some(InboundHTLCStateDetails::Committed),
InboundHTLCState::LocalRemoved(InboundHTLCRemovalReason::FailRelay(_)) =>
Some(InboundHTLCStateDetails::AwaitingRemoteRevokeToRemoveFail),
InboundHTLCState::LocalRemoved(InboundHTLCRemovalReason::FailMalformed(_)) =>
Some(InboundHTLCStateDetails::AwaitingRemoteRevokeToRemoveFail),
InboundHTLCState::LocalRemoved(InboundHTLCRemovalReason::Fulfill(_)) =>
Some(InboundHTLCStateDetails::AwaitingRemoteRevokeToRemoveFulfill),
}
}
}

impl_writeable_tlv_based_enum_upgradable!(InboundHTLCStateDetails,
(0, AwaitingRemoteRevokeToAdd) => {},
(2, Committed) => {},
(4, AwaitingRemoteRevokeToRemoveFulfill) => {},
(6, AwaitingRemoteRevokeToRemoveFail) => {};
);

struct InboundHTLCOutput {
htlc_id: u64,
amount_msat: u64,
Expand All @@ -166,6 +232,48 @@ struct InboundHTLCOutput {
state: InboundHTLCState,
}

/// Exposes details around pending inbound HTLCs.
#[derive(Clone, Debug, PartialEq)]
pub struct InboundHTLCDetails {
/// The HTLC ID.
/// The IDs are incremented by 1 starting from 0 for each offered HTLC.
/// They are unique per channel and inbound/outbound direction, unless an HTLC was only announced
/// and not part of any commitment transaction.
pub htlc_id: u64,
/// The amount in msat.
pub amount_msat: u64,
/// The block height at which this HTLC expires.
pub cltv_expiry: u32,
/// The payment hash.
pub payment_hash: PaymentHash,
/// The state of the HTLC in the state machine.
/// Determines on which commitment transactions the HTLC is included and what message the HTLC is
/// waiting for to advance to the next state.
/// See [`InboundHTLCStateDetails`] for information on the specific states.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Docs should say when this can be None.

pub state: Option<InboundHTLCStateDetails>,
/// Whether the HTLC has an output below the local dust limit. If so, the output will be trimmed
Copy link
Collaborator

Choose a reason for hiding this comment

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

Something about burning to fee on closure?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added.

/// from the local commitment transaction and added to the commitment transaction fee.
/// For non-anchor channels, this takes into account the cost of the second-stage HTLC
/// transactions as well.
///
/// When the local commitment transaction is broadcasted as part of a unilateral closure,
/// the value of this HTLC will therefore not be claimable but instead burned as a transaction
/// fee.
///
/// Note that dust limits are specific to each party. An HTLC can be dust for the local
/// commitment transaction but not for the counterparty's commitment transaction and vice versa.
pub is_dust: bool,
}

impl_writeable_tlv_based!(InboundHTLCDetails, {
(0, htlc_id, required),
(2, amount_msat, required),
(4, cltv_expiry, required),
(6, payment_hash, required),
(7, state, upgradable_option),
(8, is_dust, required),
});

#[cfg_attr(test, derive(Clone, Debug, PartialEq))]
enum OutboundHTLCState {
/// Added by us and included in a commitment_signed (if we were AwaitingRemoteRevoke when we
Expand Down Expand Up @@ -199,6 +307,72 @@ enum OutboundHTLCState {
AwaitingRemovedRemoteRevoke(OutboundHTLCOutcome),
}

/// Exposes the state of pending outbound HTLCs.
///
/// At a high level, an HTLC being forwarded from one Lightning node to another Lightning node goes
/// through the following states in the state machine:
/// - Announced for addition by the originating node through the update_add_htlc message.
/// - Added to the commitment transaction of the receiving node and originating node in turn
/// through the exchange of commitment_signed and revoke_and_ack messages.
/// - Announced for resolution (fulfillment or failure) by the receiving node through either one of
/// the update_fulfill_htlc, update_fail_htlc, and update_fail_malformed_htlc messages.
/// - Removed from the commitment transaction of the originating node and receiving node in turn
/// through the exchange of commitment_signed and revoke_and_ack messages.
///
/// This can be used to inspect what next message an HTLC is waiting for to advance its state.
#[derive(Clone, Debug, PartialEq)]
pub enum OutboundHTLCStateDetails {
/// We are awaiting the appropriate revoke_and_ack's from the remote before the HTLC is added
/// on the remote's commitment transaction after update_add_htlc.
AwaitingRemoteRevokeToAdd,
/// The HTLC has been added to the remote's commitment transaction by sending commitment_signed
/// and receiving revoke_and_ack in return.
///
/// The HTLC will remain in this state until the remote node resolves the HTLC, or until we
/// unilaterally close the channel due to a timeout with an uncooperative remote node.
Committed,
/// The HTLC has been fulfilled successfully by the remote with a preimage in update_fulfill_htlc,
/// and we removed the HTLC from our commitment transaction by receiving commitment_signed and
/// returning revoke_and_ack. We are awaiting the appropriate revoke_and_ack's from the remote
/// for the removal from its commitment transaction.
AwaitingRemoteRevokeToRemoveSuccess,
/// The HTLC has been failed by the remote with update_fail_htlc or update_fail_malformed_htlc,
/// and we removed the HTLC from our commitment transaction by receiving commitment_signed and
/// returning revoke_and_ack. We are awaiting the appropriate revoke_and_ack's from the remote
/// for the removal from its commitment transaction.
AwaitingRemoteRevokeToRemoveFailure,
}

impl From<&OutboundHTLCState> for OutboundHTLCStateDetails {
fn from(state: &OutboundHTLCState) -> OutboundHTLCStateDetails {
match state {
OutboundHTLCState::LocalAnnounced(_) =>
OutboundHTLCStateDetails::AwaitingRemoteRevokeToAdd,
OutboundHTLCState::Committed =>
OutboundHTLCStateDetails::Committed,
// RemoteRemoved states are ignored as the state is transient and the remote has not committed to
// the state yet.
OutboundHTLCState::RemoteRemoved(_) =>
OutboundHTLCStateDetails::Committed,
OutboundHTLCState::AwaitingRemoteRevokeToRemove(OutboundHTLCOutcome::Success(_)) =>
OutboundHTLCStateDetails::AwaitingRemoteRevokeToRemoveSuccess,
OutboundHTLCState::AwaitingRemoteRevokeToRemove(OutboundHTLCOutcome::Failure(_)) =>
OutboundHTLCStateDetails::AwaitingRemoteRevokeToRemoveFailure,
OutboundHTLCState::AwaitingRemovedRemoteRevoke(OutboundHTLCOutcome::Success(_)) =>
OutboundHTLCStateDetails::AwaitingRemoteRevokeToRemoveSuccess,
OutboundHTLCState::AwaitingRemovedRemoteRevoke(OutboundHTLCOutcome::Failure(_)) =>
OutboundHTLCStateDetails::AwaitingRemoteRevokeToRemoveFailure,
}
}
}

impl_writeable_tlv_based_enum_upgradable!(OutboundHTLCStateDetails,
(0, AwaitingRemoteRevokeToAdd) => {},
(2, Committed) => {},
(4, AwaitingRemoteRevokeToRemoveSuccess) => {},
(6, AwaitingRemoteRevokeToRemoveFailure) => {};
);

#[derive(Clone)]
#[cfg_attr(test, derive(Debug, PartialEq))]
enum OutboundHTLCOutcome {
Expand Down Expand Up @@ -237,6 +411,53 @@ struct OutboundHTLCOutput {
skimmed_fee_msat: Option<u64>,
}

/// Exposes details around pending outbound HTLCs.
#[derive(Clone, Debug, PartialEq)]
pub struct OutboundHTLCDetails {
/// The HTLC ID.
/// The IDs are incremented by 1 starting from 0 for each offered HTLC.
/// They are unique per channel and inbound/outbound direction, unless an HTLC was only announced
/// and not part of any commitment transaction.
///
/// Not present when we are awaiting a remote revocation and the HTLC is not added yet.
pub htlc_id: Option<u64>,
/// The amount in msat.
pub amount_msat: u64,
/// The block height at which this HTLC expires.
pub cltv_expiry: u32,
/// The payment hash.
pub payment_hash: PaymentHash,
/// The state of the HTLC in the state machine.
/// Determines on which commitment transactions the HTLC is included and what message the HTLC is
/// waiting for to advance to the next state.
/// See [`OutboundHTLCStateDetails`] for information on the specific states.
pub state: Option<OutboundHTLCStateDetails>,
/// The extra fee being skimmed off the top of this HTLC.
pub skimmed_fee_msat: Option<u64>,
/// Whether the HTLC has an output below the local dust limit. If so, the output will be trimmed
/// from the local commitment transaction and added to the commitment transaction fee.
/// For non-anchor channels, this takes into account the cost of the second-stage HTLC
/// transactions as well.
///
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it worth noting here (and in the inbound field for the same) that an HTLC could be dust for our commitment tx but not our counterparty's commitment tx or vice versa?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a note that this is specific to each party for the inbound and outbound case.

/// When the local commitment transaction is broadcasted as part of a unilateral closure,
/// the value of this HTLC will therefore not be claimable but instead burned as a transaction
/// fee.
///
/// Note that dust limits are specific to each party. An HTLC can be dust for the local
/// commitment transaction but not for the counterparty's commitment transaction and vice versa.
pub is_dust: bool,
}

impl_writeable_tlv_based!(OutboundHTLCDetails, {
(0, htlc_id, required),
(2, amount_msat, required),
(4, cltv_expiry, required),
(6, payment_hash, required),
(7, state, upgradable_option),
(8, skimmed_fee_msat, required),
(10, is_dust, required),
});

/// See AwaitingRemoteRevoke ChannelState for more info
#[cfg_attr(test, derive(Clone, Debug, PartialEq))]
enum HTLCUpdateAwaitingACK {
Expand Down Expand Up @@ -1994,6 +2215,99 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
stats
}

/// Returns information on all pending inbound HTLCs.
pub fn get_pending_inbound_htlc_details(&self) -> Vec<InboundHTLCDetails> {
let mut holding_cell_states = new_hash_map();
for holding_cell_update in self.holding_cell_htlc_updates.iter() {
match holding_cell_update {
HTLCUpdateAwaitingACK::ClaimHTLC { htlc_id, .. } => {
holding_cell_states.insert(
htlc_id,
InboundHTLCStateDetails::AwaitingRemoteRevokeToRemoveFulfill,
);
},
HTLCUpdateAwaitingACK::FailHTLC { htlc_id, .. } => {
holding_cell_states.insert(
htlc_id,
InboundHTLCStateDetails::AwaitingRemoteRevokeToRemoveFail,
);
},
HTLCUpdateAwaitingACK::FailMalformedHTLC { htlc_id, .. } => {
holding_cell_states.insert(
htlc_id,
InboundHTLCStateDetails::AwaitingRemoteRevokeToRemoveFail,
);
},
// Outbound HTLC.
HTLCUpdateAwaitingACK::AddHTLC { .. } => {},
}
}
let mut inbound_details = Vec::new();
let htlc_success_dust_limit = if self.get_channel_type().supports_anchors_zero_fee_htlc_tx() {
0
} else {
let dust_buffer_feerate = self.get_dust_buffer_feerate(None) as u64;
dust_buffer_feerate * htlc_success_tx_weight(self.get_channel_type()) / 1000
};
let holder_dust_limit_success_sat = htlc_success_dust_limit + self.holder_dust_limit_satoshis;
for htlc in self.pending_inbound_htlcs.iter() {
if let Some(state_details) = (&htlc.state).into() {
inbound_details.push(InboundHTLCDetails{
htlc_id: htlc.htlc_id,
amount_msat: htlc.amount_msat,
cltv_expiry: htlc.cltv_expiry,
payment_hash: htlc.payment_hash,
state: Some(holding_cell_states.remove(&htlc.htlc_id).unwrap_or(state_details)),
is_dust: htlc.amount_msat / 1000 < holder_dust_limit_success_sat,
});
}
}
inbound_details
}

/// Returns information on all pending outbound HTLCs.
pub fn get_pending_outbound_htlc_details(&self) -> Vec<OutboundHTLCDetails> {
let mut outbound_details = Vec::new();
let htlc_timeout_dust_limit = if self.get_channel_type().supports_anchors_zero_fee_htlc_tx() {
0
} else {
let dust_buffer_feerate = self.get_dust_buffer_feerate(None) as u64;
dust_buffer_feerate * htlc_success_tx_weight(self.get_channel_type()) / 1000
};
let holder_dust_limit_timeout_sat = htlc_timeout_dust_limit + self.holder_dust_limit_satoshis;
for htlc in self.pending_outbound_htlcs.iter() {
outbound_details.push(OutboundHTLCDetails{
htlc_id: Some(htlc.htlc_id),
amount_msat: htlc.amount_msat,
cltv_expiry: htlc.cltv_expiry,
payment_hash: htlc.payment_hash,
skimmed_fee_msat: htlc.skimmed_fee_msat,
state: Some((&htlc.state).into()),
is_dust: htlc.amount_msat / 1000 < holder_dust_limit_timeout_sat,
});
}
for holding_cell_update in self.holding_cell_htlc_updates.iter() {
if let HTLCUpdateAwaitingACK::AddHTLC {
amount_msat,
cltv_expiry,
payment_hash,
skimmed_fee_msat,
..
} = *holding_cell_update {
outbound_details.push(OutboundHTLCDetails{
htlc_id: None,
amount_msat: amount_msat,
cltv_expiry: cltv_expiry,
payment_hash: payment_hash,
skimmed_fee_msat: skimmed_fee_msat,
state: Some(OutboundHTLCStateDetails::AwaitingRemoteRevokeToAdd),
is_dust: amount_msat / 1000 < holder_dust_limit_timeout_sat,
});
}
}
outbound_details
}

/// Get the available balances, see [`AvailableBalances`]'s fields for more info.
/// Doesn't bother handling the
/// if-we-removed-it-already-but-haven't-fully-resolved-they-can-still-send-an-inbound-HTLC
Expand Down