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.

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

Provide the HTLCs that settled a payment. #2478

Merged
merged 1 commit into from
Aug 21, 2023
Merged

Provide the HTLCs that settled a payment. #2478

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
47 changes: 46 additions & 1 deletion lightning/src/events/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,37 @@ impl_writeable_tlv_based_enum!(PaymentPurpose,
(2, SpontaneousPayment)
);

/// Information about an HTLC that is part of a payment that can be claimed.
#[derive(Clone, Debug, PartialEq, Eq)]
pub struct ClaimedHTLC {
/// The `channel_id` of the channel over which the HTLC was received.
pub channel_id: [u8; 32],
/// The `user_channel_id` of the channel over which the HTLC was received. This is the value
/// passed in to [`ChannelManager::create_channel`] for outbound channels, or to
/// [`ChannelManager::accept_inbound_channel`] for inbound channels if
/// [`UserConfig::manually_accept_inbound_channels`] config flag is set to true. Otherwise
/// `user_channel_id` will be randomized for an inbound channel.
///
waterson marked this conversation as resolved.
Show resolved Hide resolved
/// This field will be zero for a payment that was serialized prior to LDK version 0.0.117. (This
/// should only happen in the case that a payment was claimable prior to LDK version 0.0.117, but
/// was not actually claimed until after upgrading.)
///
/// [`ChannelManager::create_channel`]: crate::ln::channelmanager::ChannelManager::create_channel
/// [`ChannelManager::accept_inbound_channel`]: crate::ln::channelmanager::ChannelManager::accept_inbound_channel
/// [`UserConfig::manually_accept_inbound_channels`]: crate::util::config::UserConfig::manually_accept_inbound_channels
pub user_channel_id: u128,
/// The block height at which this HTLC expires.
pub cltv_expiry: u32,
/// The amount (in msats) of this part of an MPP.
pub value_msat: u64,
}
impl_writeable_tlv_based!(ClaimedHTLC, {
Comment on lines +105 to +106
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: separate with newline, but so nitty feel free to ignore.

(0, channel_id, required),
(2, user_channel_id, required),
(4, cltv_expiry, required),
(6, value_msat, required),
});

/// When the payment path failure took place and extra details about it. [`PathFailure::OnPath`] may
/// contain a [`NetworkUpdate`] that needs to be applied to the [`NetworkGraph`].
///
Expand Down Expand Up @@ -470,6 +501,12 @@ pub enum Event {
/// The purpose of the claimed payment, i.e. whether the payment was for an invoice or a
/// spontaneous payment.
purpose: PaymentPurpose,
/// The HTLCs that comprise the claimed payment. This will be empty for events serialized prior
/// to LDK version 0.0.117.
htlcs: Vec<ClaimedHTLC>,
waterson marked this conversation as resolved.
Show resolved Hide resolved
/// The sender-intended sum total of all the MPP parts. This will be `None` for events
/// serialized prior to LDK version 0.0.117.
sender_intended_total_msat: Option<u64>,
waterson marked this conversation as resolved.
Show resolved Hide resolved
},
/// Indicates an outbound payment we made succeeded (i.e. it made it all the way to its target
/// and we got back the payment preimage for it).
Expand Down Expand Up @@ -1035,13 +1072,15 @@ impl Writeable for Event {
// We never write the OpenChannelRequest events as, upon disconnection, peers
// drop any channels which have not yet exchanged funding_signed.
},
&Event::PaymentClaimed { ref payment_hash, ref amount_msat, ref purpose, ref receiver_node_id } => {
&Event::PaymentClaimed { ref payment_hash, ref amount_msat, ref purpose, ref receiver_node_id, ref htlcs, ref sender_intended_total_msat } => {
19u8.write(writer)?;
write_tlv_fields!(writer, {
(0, payment_hash, required),
(1, receiver_node_id, option),
(2, purpose, required),
(4, amount_msat, required),
(5, *htlcs, optional_vec),
(7, sender_intended_total_msat, option),
});
},
&Event::ProbeSuccessful { ref payment_id, ref payment_hash, ref path } => {
Expand Down Expand Up @@ -1366,17 +1405,23 @@ impl MaybeReadable for Event {
let mut purpose = UpgradableRequired(None);
let mut amount_msat = 0;
let mut receiver_node_id = None;
let mut htlcs: Option<Vec<ClaimedHTLC>> = Some(vec![]);
let mut sender_intended_total_msat: Option<u64> = None;
read_tlv_fields!(reader, {
(0, payment_hash, required),
(1, receiver_node_id, option),
(2, purpose, upgradable_required),
(4, amount_msat, required),
(5, htlcs, optional_vec),
(7, sender_intended_total_msat, option),
});
Ok(Some(Event::PaymentClaimed {
receiver_node_id,
payment_hash,
purpose: _init_tlv_based_struct_field!(purpose, upgradable_required),
amount_msat,
htlcs: htlcs.unwrap_or(vec![]),
sender_intended_total_msat,
}))
};
f()
Expand Down
53 changes: 46 additions & 7 deletions lightning/src/ln/channelmanager.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,7 @@ pub(super) enum HTLCForwardInfo {
pub(crate) struct HTLCPreviousHopData {
// Note that this may be an outbound SCID alias for the associated channel.
short_channel_id: u64,
user_channel_id: Option<u128>,
htlc_id: u64,
incoming_packet_shared_secret: [u8; 32],
phantom_shared_secret: Option<[u8; 32]>,
Expand Down Expand Up @@ -221,6 +222,17 @@ struct ClaimableHTLC {
counterparty_skimmed_fee_msat: Option<u64>,
}

impl From<&ClaimableHTLC> for events::ClaimedHTLC {
fn from(val: &ClaimableHTLC) -> Self {
events::ClaimedHTLC {
channel_id: val.prev_hop.outpoint.to_channel_id(),
user_channel_id: val.prev_hop.user_channel_id.unwrap_or(0),
cltv_expiry: val.cltv_expiry,
value_msat: val.value,
}
}
}

/// A payment identifier used to uniquely identify a payment to LDK.
///
/// This is not exported to bindings users as we just use [u8; 32] directly
Expand Down Expand Up @@ -496,11 +508,15 @@ struct ClaimingPayment {
amount_msat: u64,
payment_purpose: events::PaymentPurpose,
receiver_node_id: PublicKey,
htlcs: Vec<events::ClaimedHTLC>,
sender_intended_value: Option<u64>,
}
impl_writeable_tlv_based!(ClaimingPayment, {
(0, amount_msat, required),
(2, payment_purpose, required),
(4, receiver_node_id, required),
(5, htlcs, optional_vec),
(7, sender_intended_value, option),
});

struct ClaimablePayment {
Expand Down Expand Up @@ -3781,6 +3797,7 @@ where
if let PendingHTLCRouting::Forward { short_channel_id, .. } = payment.forward_info.routing {
let htlc_source = HTLCSource::PreviousHopData(HTLCPreviousHopData {
short_channel_id: payment.prev_short_channel_id,
user_channel_id: Some(payment.prev_user_channel_id),
outpoint: payment.prev_funding_outpoint,
htlc_id: payment.prev_htlc_id,
incoming_packet_shared_secret: payment.forward_info.incoming_shared_secret,
Expand Down Expand Up @@ -3828,6 +3845,7 @@ where

let htlc_source = HTLCSource::PreviousHopData(HTLCPreviousHopData {
short_channel_id: prev_short_channel_id,
user_channel_id: Some(prev_user_channel_id),
outpoint: prev_funding_outpoint,
htlc_id: prev_htlc_id,
incoming_packet_shared_secret: incoming_shared_secret,
Expand Down Expand Up @@ -3932,7 +3950,7 @@ where
for forward_info in pending_forwards.drain(..) {
match forward_info {
HTLCForwardInfo::AddHTLC(PendingAddHTLCInfo {
prev_short_channel_id, prev_htlc_id, prev_funding_outpoint, prev_user_channel_id: _,
prev_short_channel_id, prev_htlc_id, prev_funding_outpoint, prev_user_channel_id,
forward_info: PendingHTLCInfo {
incoming_shared_secret, payment_hash, outgoing_amt_msat, outgoing_cltv_value,
routing: PendingHTLCRouting::Forward { onion_packet, .. }, skimmed_fee_msat, ..
Expand All @@ -3941,6 +3959,7 @@ where
log_trace!(self.logger, "Adding HTLC from short id {} with payment_hash {} to channel with short id {} after delay", prev_short_channel_id, log_bytes!(payment_hash.0), short_chan_id);
let htlc_source = HTLCSource::PreviousHopData(HTLCPreviousHopData {
short_channel_id: prev_short_channel_id,
user_channel_id: Some(prev_user_channel_id),
outpoint: prev_funding_outpoint,
htlc_id: prev_htlc_id,
incoming_packet_shared_secret: incoming_shared_secret,
Expand Down Expand Up @@ -4022,6 +4041,7 @@ where
let claimable_htlc = ClaimableHTLC {
prev_hop: HTLCPreviousHopData {
short_channel_id: prev_short_channel_id,
user_channel_id: Some(prev_user_channel_id),
outpoint: prev_funding_outpoint,
htlc_id: prev_htlc_id,
incoming_packet_shared_secret: incoming_shared_secret,
Expand Down Expand Up @@ -4051,6 +4071,7 @@ where
);
failed_forwards.push((HTLCSource::PreviousHopData(HTLCPreviousHopData {
short_channel_id: $htlc.prev_hop.short_channel_id,
user_channel_id: $htlc.prev_hop.user_channel_id,
outpoint: prev_funding_outpoint,
htlc_id: $htlc.prev_hop.htlc_id,
incoming_packet_shared_secret: $htlc.prev_hop.incoming_packet_shared_secret,
Expand Down Expand Up @@ -4782,7 +4803,7 @@ where
&self.pending_events, &self.logger)
{ self.push_pending_forwards_ev(); }
},
HTLCSource::PreviousHopData(HTLCPreviousHopData { ref short_channel_id, ref htlc_id, ref incoming_packet_shared_secret, ref phantom_shared_secret, ref outpoint }) => {
HTLCSource::PreviousHopData(HTLCPreviousHopData { ref short_channel_id, ref htlc_id, ref incoming_packet_shared_secret, ref phantom_shared_secret, ref outpoint, .. }) => {
log_trace!(self.logger, "Failing HTLC with payment_hash {} backwards from us with {:?}", log_bytes!(payment_hash.0), onion_error);
let err_packet = onion_error.get_encrypted_failure_packet(incoming_packet_shared_secret, phantom_shared_secret);

Expand Down Expand Up @@ -4869,9 +4890,11 @@ where
}
}

let htlcs = payment.htlcs.iter().map(events::ClaimedHTLC::from).collect();
let sender_intended_value = payment.htlcs.first().map(|htlc| htlc.total_msat);
let dup_purpose = claimable_payments.pending_claiming_payments.insert(payment_hash,
ClaimingPayment { amount_msat: payment.htlcs.iter().map(|source| source.value).sum(),
payment_purpose: payment.purpose, receiver_node_id,
payment_purpose: payment.purpose, receiver_node_id, htlcs, sender_intended_value
});
if dup_purpose.is_some() {
debug_assert!(false, "Shouldn't get a duplicate pending claim event ever");
Expand Down Expand Up @@ -5129,9 +5152,20 @@ where
match action {
MonitorUpdateCompletionAction::PaymentClaimed { payment_hash } => {
let payment = self.claimable_payments.lock().unwrap().pending_claiming_payments.remove(&payment_hash);
if let Some(ClaimingPayment { amount_msat, payment_purpose: purpose, receiver_node_id }) = payment {
if let Some(ClaimingPayment {
amount_msat,
Copy link
Collaborator

Choose a reason for hiding this comment

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

At
least
personally,
I
find
the
rustfmt
comically-vertical
flows
to
be
kinda
hard
to
read
:)

payment_purpose: purpose,
receiver_node_id,
htlcs,
sender_intended_value: sender_intended_total_msat,
}) = payment {
self.pending_events.lock().unwrap().push_back((events::Event::PaymentClaimed {
payment_hash, purpose, amount_msat, receiver_node_id: Some(receiver_node_id),
payment_hash,
purpose,
amount_msat,
receiver_node_id: Some(receiver_node_id),
htlcs,
sender_intended_total_msat,
}, None));
}
},
Expand Down Expand Up @@ -6006,6 +6040,7 @@ where
log_info!(self.logger, "Failed to forward incoming HTLC: detected duplicate intercepted payment over short channel id {}", scid);
let htlc_source = HTLCSource::PreviousHopData(HTLCPreviousHopData {
short_channel_id: prev_short_channel_id,
user_channel_id: Some(prev_user_channel_id),
outpoint: prev_funding_outpoint,
htlc_id: prev_htlc_id,
incoming_packet_shared_secret: forward_info.incoming_shared_secret,
Expand Down Expand Up @@ -7124,6 +7159,7 @@ where
if height >= htlc.forward_info.outgoing_cltv_value - HTLC_FAIL_BACK_BUFFER {
let prev_hop_data = HTLCSource::PreviousHopData(HTLCPreviousHopData {
short_channel_id: htlc.prev_short_channel_id,
user_channel_id: Some(htlc.prev_user_channel_id),
htlc_id: htlc.prev_htlc_id,
incoming_packet_shared_secret: htlc.forward_info.incoming_shared_secret,
phantom_shared_secret: None,
Expand Down Expand Up @@ -7899,7 +7935,8 @@ impl_writeable_tlv_based!(HTLCPreviousHopData, {
(1, phantom_shared_secret, option),
(2, outpoint, required),
(4, htlc_id, required),
(6, incoming_packet_shared_secret, required)
(6, incoming_packet_shared_secret, required),
(7, user_channel_id, option),
});

impl Writeable for ClaimableHTLC {
Expand Down Expand Up @@ -9147,7 +9184,7 @@ where
.expect("Failed to get node_id for phantom node recipient");
receiver_node_id = Some(phantom_pubkey)
}
for claimable_htlc in payment.htlcs {
for claimable_htlc in &payment.htlcs {
claimable_amt_msat += claimable_htlc.value;

// Add a holding-cell claim of the payment to the Channel, which should be
Expand Down Expand Up @@ -9183,6 +9220,8 @@ where
payment_hash,
purpose: payment.purpose,
amount_msat: claimable_amt_msat,
htlcs: payment.htlcs.iter().map(events::ClaimedHTLC::from).collect(),
sender_intended_total_msat: payment.htlcs.first().map(|htlc| htlc.total_msat),
}, None));
}
}
Expand Down
50 changes: 44 additions & 6 deletions lightning/src/ln/functional_test_utils.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use crate::chain::{BestBlock, ChannelMonitorUpdateStatus, Confirm, Listen, Watch
use crate::sign::EntropySource;
use crate::chain::channelmonitor::ChannelMonitor;
use crate::chain::transaction::OutPoint;
use crate::events::{ClosureReason, Event, HTLCDestination, MessageSendEvent, MessageSendEventsProvider, PathFailure, PaymentPurpose, PaymentFailureReason};
use crate::events::{ClaimedHTLC, ClosureReason, Event, HTLCDestination, MessageSendEvent, MessageSendEventsProvider, PathFailure, PaymentPurpose, PaymentFailureReason};
use crate::events::bump_transaction::{BumpTransactionEventHandler, Wallet, WalletSource};
use crate::ln::{PaymentPreimage, PaymentHash, PaymentSecret};
use crate::ln::channelmanager::{AChannelManager, ChainParameters, ChannelManager, ChannelManagerReadArgs, RAACommitmentOrder, PaymentSendFailure, RecipientOnionFields, PaymentId, MIN_CLTV_EXPIRY_DELTA};
Expand Down Expand Up @@ -933,6 +933,21 @@ macro_rules! check_added_monitors {
}
}

/// Checks whether the claimed HTLC for the specified path has the correct channel information.
///
/// This will panic if the path is empty, if the HTLC's channel ID is not actually a channel that
/// connects the final two nodes in the path, or if the `user_channel_id` is incorrect.
pub fn check_claimed_htlc_channel<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, path: &[&Node<'a, 'b, 'c>], htlc: &ClaimedHTLC) {
let mut nodes = path.iter().rev();
let dest = nodes.next().expect("path should have a destination").node;
let prev = nodes.next().unwrap_or(&origin_node).node;
let dest_channels = dest.list_channels();
let ch = dest_channels.iter().find(|ch| ch.channel_id == htlc.channel_id)
.expect("HTLC's channel should be one of destination node's channels");
assert_eq!(htlc.user_channel_id, ch.user_channel_id);
assert_eq!(ch.counterparty.node_id, prev.get_our_node_id());
TheBlueMatt marked this conversation as resolved.
Show resolved Hide resolved
}

pub fn _reload_node<'a, 'b, 'c>(node: &'a Node<'a, 'b, 'c>, default_config: UserConfig, chanman_encoded: &[u8], monitors_encoded: &[&[u8]]) -> TestChannelManager<'b, 'c> {
let mut monitors_read = Vec::with_capacity(monitors_encoded.len());
for encoded in monitors_encoded {
Expand Down Expand Up @@ -2271,11 +2286,34 @@ pub fn pass_claimed_payment_along_route<'a, 'b, 'c>(origin_node: &Node<'a, 'b, '
let claim_event = expected_paths[0].last().unwrap().node.get_and_clear_pending_events();
assert_eq!(claim_event.len(), 1);
match claim_event[0] {
Event::PaymentClaimed { purpose: PaymentPurpose::SpontaneousPayment(preimage), .. }|
Event::PaymentClaimed { purpose: PaymentPurpose::InvoicePayment { payment_preimage: Some(preimage), ..}, .. } =>
assert_eq!(preimage, our_payment_preimage),
Event::PaymentClaimed { purpose: PaymentPurpose::InvoicePayment { .. }, payment_hash, .. } =>
assert_eq!(&payment_hash.0, &Sha256::hash(&our_payment_preimage.0)[..]),
Event::PaymentClaimed {
purpose: PaymentPurpose::SpontaneousPayment(preimage),
amount_msat,
ref htlcs,
.. }
| Event::PaymentClaimed {
purpose: PaymentPurpose::InvoicePayment { payment_preimage: Some(preimage), ..},
ref htlcs,
amount_msat,
..
} => {
assert_eq!(preimage, our_payment_preimage);
assert_eq!(htlcs.len(), expected_paths.len()); // One per path.
assert_eq!(htlcs.iter().map(|h| h.value_msat).sum::<u64>(), amount_msat);
expected_paths.iter().zip(htlcs).for_each(|(path, htlc)| check_claimed_htlc_channel(origin_node, path, htlc));
},
Event::PaymentClaimed {
purpose: PaymentPurpose::InvoicePayment { .. },
payment_hash,
amount_msat,
ref htlcs,
..
} => {
assert_eq!(&payment_hash.0, &Sha256::hash(&our_payment_preimage.0)[..]);
assert_eq!(htlcs.len(), expected_paths.len()); // One per path.
assert_eq!(htlcs.iter().map(|h| h.value_msat).sum::<u64>(), amount_msat);
expected_paths.iter().zip(htlcs).for_each(|(path, htlc)| check_claimed_htlc_channel(origin_node, path, htlc));
}
_ => panic!(),
}

Expand Down