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 counterparty node ids in PaymentForwarded #3458

Merged
merged 3 commits into from
Dec 11, 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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 21 additions & 4 deletions lightning/src/events/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1184,6 +1184,16 @@ pub enum Event {
/// caveat described for the `total_fee_earned_msat` field. Moreover it will be `None` for
/// events generated or serialized by versions prior to 0.0.122.
next_user_channel_id: Option<u128>,
/// The node id of the previous node.
///
/// This is only `None` for HTLCs received prior to 0.1 or for events serialized by
/// versions prior to 0.1
prev_node_id: Option<PublicKey>,
/// The node id of the next node.
///
/// This is only `None` for HTLCs received prior to 0.1 or for events serialized by
/// versions prior to 0.1
next_node_id: Option<PublicKey>,
/// The total fee, in milli-satoshis, which was earned as a result of the payment.
///
/// Note that if we force-closed the channel over which we forwarded an HTLC while the HTLC
Expand Down Expand Up @@ -1601,8 +1611,8 @@ impl Writeable for Event {
}
&Event::PaymentForwarded {
prev_channel_id, next_channel_id, prev_user_channel_id, next_user_channel_id,
total_fee_earned_msat, skimmed_fee_msat, claim_from_onchain_tx,
outbound_amount_forwarded_msat,
prev_node_id, next_node_id, total_fee_earned_msat, skimmed_fee_msat,
claim_from_onchain_tx, outbound_amount_forwarded_msat,
} => {
7u8.write(writer)?;
write_tlv_fields!(writer, {
Expand All @@ -1614,6 +1624,8 @@ impl Writeable for Event {
(7, skimmed_fee_msat, option),
(9, prev_user_channel_id, option),
(11, next_user_channel_id, option),
(13, prev_node_id, option),
(15, next_node_id, option),
});
},
&Event::ChannelClosed { ref channel_id, ref user_channel_id, ref reason,
Expand Down Expand Up @@ -1981,6 +1993,8 @@ impl MaybeReadable for Event {
let mut next_channel_id = None;
let mut prev_user_channel_id = None;
let mut next_user_channel_id = None;
let mut prev_node_id = None;
let mut next_node_id = None;
let mut total_fee_earned_msat = None;
let mut skimmed_fee_msat = None;
let mut claim_from_onchain_tx = false;
Expand All @@ -1994,11 +2008,14 @@ impl MaybeReadable for Event {
(7, skimmed_fee_msat, option),
(9, prev_user_channel_id, option),
(11, next_user_channel_id, option),
(13, prev_node_id, option),
(15, next_node_id, option),
});
Ok(Some(Event::PaymentForwarded {
prev_channel_id, next_channel_id, prev_user_channel_id,
next_user_channel_id, total_fee_earned_msat, skimmed_fee_msat,
claim_from_onchain_tx, outbound_amount_forwarded_msat,
next_user_channel_id, prev_node_id, next_node_id,
total_fee_earned_msat, skimmed_fee_msat, claim_from_onchain_tx,
outbound_amount_forwarded_msat,
}))
};
f()
Expand Down
3 changes: 3 additions & 0 deletions lightning/src/ln/channelmanager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7309,6 +7309,7 @@ where
HTLCSource::PreviousHopData(hop_data) => {
let prev_channel_id = hop_data.channel_id;
let prev_user_channel_id = hop_data.user_channel_id;
let prev_node_id = hop_data.counterparty_node_id;
let completed_blocker = RAAMonitorUpdateBlockingAction::from_prev_hop_data(&hop_data);
self.claim_funds_from_hop(hop_data, payment_preimage, None,
|htlc_claim_value_msat, definitely_duplicate| {
Expand Down Expand Up @@ -7358,6 +7359,8 @@ where
next_channel_id: Some(next_channel_id),
prev_user_channel_id,
next_user_channel_id,
prev_node_id,
next_node_id: next_channel_counterparty_node_id,
total_fee_earned_msat,
skimmed_fee_msat,
claim_from_onchain_tx: from_onchain,
Expand Down
6 changes: 5 additions & 1 deletion lightning/src/ln/functional_test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2360,7 +2360,8 @@ pub fn expect_payment_forwarded<CM: AChannelManager, H: NodeHolder<CM=CM>>(
match event {
Event::PaymentForwarded {
prev_channel_id, next_channel_id, prev_user_channel_id, next_user_channel_id,
total_fee_earned_msat, skimmed_fee_msat, claim_from_onchain_tx, ..
prev_node_id, next_node_id, total_fee_earned_msat,
skimmed_fee_msat, claim_from_onchain_tx, ..
} => {
if allow_1_msat_fee_overpay {
// Aggregating fees for blinded paths may result in a rounding error, causing slight
Expand All @@ -2379,6 +2380,7 @@ pub fn expect_payment_forwarded<CM: AChannelManager, H: NodeHolder<CM=CM>>(
// Is the event prev_channel_id in one of the channels between the two nodes?
assert!(node.node().list_channels().iter().any(|x|
x.counterparty.node_id == prev_node.node().get_our_node_id() &&
prev_node.node().get_our_node_id() == prev_node_id.unwrap() &&
x.channel_id == prev_channel_id.unwrap() &&
x.user_channel_id == prev_user_channel_id.unwrap()
));
Expand All @@ -2393,11 +2395,13 @@ pub fn expect_payment_forwarded<CM: AChannelManager, H: NodeHolder<CM=CM>>(
if total_fee_earned_msat.is_none() {
assert!(node.node().list_channels().iter().any(|x|
x.counterparty.node_id == next_node.node().get_our_node_id() &&
next_node.node().get_our_node_id() == next_node_id.unwrap() &&
x.channel_id == next_channel_id.unwrap()
));
} else {
assert!(node.node().list_channels().iter().any(|x|
x.counterparty.node_id == next_node.node().get_our_node_id() &&
next_node.node().get_our_node_id() == next_node_id.unwrap() &&
x.channel_id == next_channel_id.unwrap() &&
x.user_channel_id == next_user_channel_id.unwrap()
));
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
API Updates
===========

To improve clarity and uniqueness in identifying forwarded payments, the `PaymentForwarded`
event now includes counterparty node IDs alongside `ChannelIds`. This change resolves
potential ambiguity in cases like v1 0conf channel opens, where `ChannelIds` alone may not
be unique.
Loading