Skip to content

adds node_id to Event::Payment{Received, Claimed} #1766

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 1 commit into from
Nov 28, 2022
Merged

adds node_id to Event::Payment{Received, Claimed} #1766

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
2 changes: 1 addition & 1 deletion lightning-invoice/src/utils.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1083,7 +1083,7 @@ mod test {
nodes[fwd_idx].node.process_pending_htlc_forwards();

let payment_preimage_opt = if user_generated_pmt_hash { None } else { Some(payment_preimage) };
expect_payment_received!(&nodes[fwd_idx], payment_hash, payment_secret, payment_amt, payment_preimage_opt);
expect_payment_received!(&nodes[fwd_idx], payment_hash, payment_secret, payment_amt, payment_preimage_opt, route.paths[0].last().unwrap().pubkey);
do_claim_payment_along_route(&nodes[0], &vec!(&vec!(&nodes[fwd_idx])[..]), false, payment_preimage);
let events = nodes[0].node.get_and_clear_pending_events();
assert_eq!(events.len(), 2);
Expand Down
15 changes: 10 additions & 5 deletions lightning/src/ln/chanmon_update_fail_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,9 +200,10 @@ fn do_test_simple_monitor_temporary_update_fail(disconnect: bool) {
let events_3 = nodes[1].node.get_and_clear_pending_events();
assert_eq!(events_3.len(), 1);
match events_3[0] {
Event::PaymentReceived { ref payment_hash, ref purpose, amount_msat } => {
Event::PaymentReceived { ref payment_hash, ref purpose, amount_msat, receiver_node_id } => {
assert_eq!(payment_hash_1, *payment_hash);
assert_eq!(amount_msat, 1_000_000);
assert_eq!(receiver_node_id.unwrap(), nodes[1].node.get_our_node_id());
match &purpose {
PaymentPurpose::InvoicePayment { payment_preimage, payment_secret, .. } => {
assert!(payment_preimage.is_none());
Expand Down Expand Up @@ -567,9 +568,10 @@ fn do_test_monitor_temporary_update_fail(disconnect_count: usize) {
let events_5 = nodes[1].node.get_and_clear_pending_events();
assert_eq!(events_5.len(), 1);
match events_5[0] {
Event::PaymentReceived { ref payment_hash, ref purpose, amount_msat } => {
Event::PaymentReceived { ref payment_hash, ref purpose, amount_msat, receiver_node_id } => {
assert_eq!(payment_hash_2, *payment_hash);
assert_eq!(amount_msat, 1_000_000);
assert_eq!(receiver_node_id.unwrap(), nodes[1].node.get_our_node_id());
match &purpose {
PaymentPurpose::InvoicePayment { payment_preimage, payment_secret, .. } => {
assert!(payment_preimage.is_none());
Expand Down Expand Up @@ -682,9 +684,10 @@ fn test_monitor_update_fail_cs() {
let events = nodes[1].node.get_and_clear_pending_events();
assert_eq!(events.len(), 1);
match events[0] {
Event::PaymentReceived { payment_hash, ref purpose, amount_msat } => {
Event::PaymentReceived { payment_hash, ref purpose, amount_msat, receiver_node_id } => {
assert_eq!(payment_hash, our_payment_hash);
assert_eq!(amount_msat, 1_000_000);
assert_eq!(receiver_node_id.unwrap(), nodes[1].node.get_our_node_id());
match &purpose {
PaymentPurpose::InvoicePayment { payment_preimage, payment_secret, .. } => {
assert!(payment_preimage.is_none());
Expand Down Expand Up @@ -1645,9 +1648,10 @@ fn test_monitor_update_fail_claim() {
let events = nodes[0].node.get_and_clear_pending_events();
assert_eq!(events.len(), 2);
match events[0] {
Event::PaymentReceived { ref payment_hash, ref purpose, amount_msat } => {
Event::PaymentReceived { ref payment_hash, ref purpose, amount_msat, receiver_node_id } => {
assert_eq!(payment_hash_2, *payment_hash);
assert_eq!(1_000_000, amount_msat);
assert_eq!(receiver_node_id.unwrap(), nodes[0].node.get_our_node_id());
match &purpose {
PaymentPurpose::InvoicePayment { payment_preimage, payment_secret, .. } => {
assert!(payment_preimage.is_none());
Expand All @@ -1659,9 +1663,10 @@ fn test_monitor_update_fail_claim() {
_ => panic!("Unexpected event"),
}
match events[1] {
Event::PaymentReceived { ref payment_hash, ref purpose, amount_msat } => {
Event::PaymentReceived { ref payment_hash, ref purpose, amount_msat, receiver_node_id } => {
assert_eq!(payment_hash_3, *payment_hash);
assert_eq!(1_000_000, amount_msat);
assert_eq!(receiver_node_id.unwrap(), nodes[0].node.get_our_node_id());
match &purpose {
PaymentPurpose::InvoicePayment { payment_preimage, payment_secret, .. } => {
assert!(payment_preimage.is_none());
Expand Down
24 changes: 24 additions & 0 deletions lightning/src/ln/channelmanager.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3323,6 +3323,12 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
));
}
}
let phantom_shared_secret = claimable_htlc.prev_hop.phantom_shared_secret;
let mut receiver_node_id = self.our_network_pubkey;
if phantom_shared_secret.is_some() {
receiver_node_id = self.keys_manager.get_node_id(Recipient::PhantomNode)
.expect("Failed to get node_id for phantom node recipient");
}
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Could re-add the blank line before the macro.


macro_rules! check_total_value {
($payment_data: expr, $payment_preimage: expr) => {{
Expand Down Expand Up @@ -3365,6 +3371,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
} else if total_value == $payment_data.total_msat {
htlcs.push(claimable_htlc);
new_events.push(events::Event::PaymentReceived {
receiver_node_id: Some(receiver_node_id),
payment_hash,
purpose: purpose(),
amount_msat: total_value,
Expand Down Expand Up @@ -3407,6 +3414,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
let purpose = events::PaymentPurpose::SpontaneousPayment(preimage);
e.insert((purpose.clone(), vec![claimable_htlc]));
new_events.push(events::Event::PaymentReceived {
receiver_node_id: Some(receiver_node_id),
payment_hash,
amount_msat: outgoing_amt_msat,
purpose,
Expand Down Expand Up @@ -4070,6 +4078,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
let mut claimed_any_htlcs = false;
let mut channel_state_lock = self.channel_state.lock().unwrap();
let channel_state = &mut *channel_state_lock;
let mut receiver_node_id = Some(self.our_network_pubkey);
for htlc in sources.iter() {
let chan_id = match self.short_to_chan_info.read().unwrap().get(&htlc.prev_hop.short_channel_id) {
Some((_cp_id, chan_id)) => chan_id.clone(),
Expand Down Expand Up @@ -4101,6 +4110,12 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
break;
}
}
let phantom_shared_secret = htlc.prev_hop.phantom_shared_secret;
if phantom_shared_secret.is_some() {
let phantom_pubkey = self.keys_manager.get_node_id(Recipient::PhantomNode)
.expect("Failed to get node_id for phantom node recipient");
receiver_node_id = Some(phantom_pubkey)
}

claimable_amt_msat += htlc.value;
}
Expand Down Expand Up @@ -4150,6 +4165,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F

if claimed_any_htlcs {
self.pending_events.lock().unwrap().push(events::Event::PaymentClaimed {
receiver_node_id,
payment_hash,
purpose: payment_purpose,
amount_msat: claimable_amt_msat,
Expand Down Expand Up @@ -7447,6 +7463,13 @@ impl<'a, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
if let Some((payment_purpose, claimable_htlcs)) = claimable_htlcs.remove(&payment_hash) {
log_info!(args.logger, "Re-claiming HTLCs with payment hash {} as we've released the preimage to a ChannelMonitor!", log_bytes!(payment_hash.0));
let mut claimable_amt_msat = 0;
let mut receiver_node_id = Some(our_network_pubkey);
let phantom_shared_secret = claimable_htlcs[0].prev_hop.phantom_shared_secret;
if phantom_shared_secret.is_some() {
let phantom_pubkey = args.keys_manager.get_node_id(Recipient::PhantomNode)
.expect("Failed to get node_id for phantom node recipient");
receiver_node_id = Some(phantom_pubkey)
}
for claimable_htlc in claimable_htlcs {
claimable_amt_msat += claimable_htlc.value;

Expand Down Expand Up @@ -7474,6 +7497,7 @@ impl<'a, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
}
}
pending_events_read.push(events::Event::PaymentClaimed {
receiver_node_id,
payment_hash,
purpose: payment_purpose,
amount_msat: claimable_amt_msat,
Expand Down
11 changes: 6 additions & 5 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 @@ -1469,20 +1469,20 @@ macro_rules! expect_pending_htlcs_forwardable_from_events {
}
}}
}

#[macro_export]
#[cfg(any(test, feature = "_bench_unstable", feature = "_test_utils"))]
macro_rules! expect_payment_received {
($node: expr, $expected_payment_hash: expr, $expected_payment_secret: expr, $expected_recv_value: expr) => {
expect_payment_received!($node, $expected_payment_hash, $expected_payment_secret, $expected_recv_value, None)
expect_payment_received!($node, $expected_payment_hash, $expected_payment_secret, $expected_recv_value, None, $node.node.get_our_node_id())
};
($node: expr, $expected_payment_hash: expr, $expected_payment_secret: expr, $expected_recv_value: expr, $expected_payment_preimage: expr) => {
($node: expr, $expected_payment_hash: expr, $expected_payment_secret: expr, $expected_recv_value: expr, $expected_payment_preimage: expr, $expected_receiver_node_id: expr) => {
let events = $node.node.get_and_clear_pending_events();
assert_eq!(events.len(), 1);
match events[0] {
$crate::util::events::Event::PaymentReceived { ref payment_hash, ref purpose, amount_msat } => {
$crate::util::events::Event::PaymentReceived { ref payment_hash, ref purpose, amount_msat, receiver_node_id } => {
assert_eq!($expected_payment_hash, *payment_hash);
assert_eq!($expected_recv_value, amount_msat);
assert_eq!($expected_receiver_node_id, receiver_node_id.unwrap());
match purpose {
$crate::util::events::PaymentPurpose::InvoicePayment { payment_preimage, payment_secret, .. } => {
assert_eq!(&$expected_payment_preimage, payment_preimage);
Expand Down Expand Up @@ -1774,8 +1774,9 @@ pub fn do_pass_along_path<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_p
if payment_received_expected {
assert_eq!(events_2.len(), 1);
match events_2[0] {
Event::PaymentReceived { ref payment_hash, ref purpose, amount_msat } => {
Event::PaymentReceived { ref payment_hash, ref purpose, amount_msat, receiver_node_id } => {
assert_eq!(our_payment_hash, *payment_hash);
assert_eq!(node.node.get_our_node_id(), receiver_node_id.unwrap());
match &purpose {
PaymentPurpose::InvoicePayment { payment_preimage, payment_secret, .. } => {
assert_eq!(expected_preimage, *payment_preimage);
Expand Down
9 changes: 6 additions & 3 deletions lightning/src/ln/functional_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1956,9 +1956,10 @@ fn test_channel_reserve_holding_cell_htlcs() {
let events = nodes[2].node.get_and_clear_pending_events();
assert_eq!(events.len(), 2);
match events[0] {
Event::PaymentReceived { ref payment_hash, ref purpose, amount_msat } => {
Event::PaymentReceived { ref payment_hash, ref purpose, amount_msat, receiver_node_id } => {
assert_eq!(our_payment_hash_21, *payment_hash);
assert_eq!(recv_value_21, amount_msat);
assert_eq!(nodes[2].node.get_our_node_id(), receiver_node_id.unwrap());
match &purpose {
PaymentPurpose::InvoicePayment { payment_preimage, payment_secret, .. } => {
assert!(payment_preimage.is_none());
Expand All @@ -1970,9 +1971,10 @@ fn test_channel_reserve_holding_cell_htlcs() {
_ => panic!("Unexpected event"),
}
match events[1] {
Event::PaymentReceived { ref payment_hash, ref purpose, amount_msat } => {
Event::PaymentReceived { ref payment_hash, ref purpose, amount_msat, receiver_node_id } => {
assert_eq!(our_payment_hash_22, *payment_hash);
assert_eq!(recv_value_22, amount_msat);
assert_eq!(nodes[2].node.get_our_node_id(), receiver_node_id.unwrap());
match &purpose {
PaymentPurpose::InvoicePayment { payment_preimage, payment_secret, .. } => {
assert!(payment_preimage.is_none());
Expand Down Expand Up @@ -3734,9 +3736,10 @@ fn do_test_drop_messages_peer_disconnect(messages_delivered: u8, simulate_broken
let events_2 = nodes[1].node.get_and_clear_pending_events();
assert_eq!(events_2.len(), 1);
match events_2[0] {
Event::PaymentReceived { ref payment_hash, ref purpose, amount_msat } => {
Event::PaymentReceived { ref payment_hash, ref purpose, amount_msat, receiver_node_id } => {
assert_eq!(payment_hash_1, *payment_hash);
assert_eq!(amount_msat, 1_000_000);
assert_eq!(receiver_node_id.unwrap(), nodes[1].node.get_our_node_id());
match &purpose {
PaymentPurpose::InvoicePayment { payment_preimage, payment_secret, .. } => {
assert!(payment_preimage.is_none());
Expand Down
2 changes: 1 addition & 1 deletion lightning/src/ln/onion_route_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1229,7 +1229,7 @@ fn test_phantom_failure_reject_payment() {
nodes[1].node.process_pending_htlc_forwards();
expect_pending_htlcs_forwardable_ignore!(nodes[1]);
nodes[1].node.process_pending_htlc_forwards();
expect_payment_received!(nodes[1], payment_hash, payment_secret, recv_amt_msat);
expect_payment_received!(nodes[1], payment_hash, payment_secret, recv_amt_msat, None, route.paths[0].last().unwrap().pubkey);
nodes[1].node.fail_htlc_backwards(&payment_hash);
expect_pending_htlcs_forwardable_and_htlc_handling_failed_ignore!(nodes[1], vec![HTLCDestination::FailedPayment { payment_hash }]);
nodes[1].node.process_pending_htlc_forwards();
Expand Down
26 changes: 24 additions & 2 deletions lightning/src/util/events.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,13 @@ pub enum Event {
/// [`ChannelManager::claim_funds`]: crate::ln::channelmanager::ChannelManager::claim_funds
/// [`ChannelManager::fail_htlc_backwards`]: crate::ln::channelmanager::ChannelManager::fail_htlc_backwards
PaymentReceived {
/// The node that received the payment.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you add a note here and on the version below saying that "this is useful to identify payments which were received via [phantom node payments]. This field will always be filled in when the event was generated by LDK versions 0.0.113 and above"? If you do the [phantom node payments] you'll have to add a blank line then a location for the link to point to, like we do here: https://github.com/lightningdevkit/rust-lightning/blob/main/lightning/src/chain/keysinterface.rs#L399

/// This is useful to identify payments which were received via [phantom node payments].
/// This field will always be filled in when the event was generated by LDK versions
/// 0.0.113 and above.
///
/// [phantom node payments]: crate::chain::keysinterface::PhantomKeysManager
receiver_node_id: Option<PublicKey>,
/// The hash for which the preimage should be handed to the ChannelManager. Note that LDK will
/// not stop you from registering duplicate payment hashes for inbound payments.
payment_hash: PaymentHash,
Expand All @@ -366,6 +373,13 @@ pub enum Event {
///
/// [`ChannelManager::claim_funds`]: crate::ln::channelmanager::ChannelManager::claim_funds
PaymentClaimed {
/// The node that received the payment.
/// This is useful to identify payments which were received via [phantom node payments].
/// This field will always be filled in when the event was generated by LDK versions
/// 0.0.113 and above.
///
/// [phantom node payments]: crate::chain::keysinterface::PhantomKeysManager
receiver_node_id: Option<PublicKey>,
/// The payment hash of the claimed payment. Note that LDK will not stop you from
/// registering duplicate payment hashes for inbound payments.
payment_hash: PaymentHash,
Expand Down Expand Up @@ -739,7 +753,7 @@ impl Writeable for Event {
// We never write out FundingGenerationReady events as, upon disconnection, peers
// drop any channels which have not yet exchanged funding_signed.
},
&Event::PaymentReceived { ref payment_hash, ref amount_msat, ref purpose } => {
&Event::PaymentReceived { ref payment_hash, ref amount_msat, ref purpose, ref receiver_node_id } => {
1u8.write(writer)?;
let mut payment_secret = None;
let payment_preimage;
Expand All @@ -754,6 +768,7 @@ impl Writeable for Event {
}
write_tlv_fields!(writer, {
(0, payment_hash, required),
(1, receiver_node_id, option),
(2, payment_secret, option),
(4, amount_msat, required),
(6, 0u64, required), // user_payment_id required for compatibility with 0.0.103 and earlier
Expand Down Expand Up @@ -854,10 +869,11 @@ 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 } => {
&Event::PaymentClaimed { ref payment_hash, ref amount_msat, ref purpose, ref receiver_node_id } => {
19u8.write(writer)?;
write_tlv_fields!(writer, {
(0, payment_hash, required),
(1, receiver_node_id, option),
(2, purpose, required),
(4, amount_msat, required),
});
Expand Down Expand Up @@ -923,9 +939,11 @@ impl MaybeReadable for Event {
let mut payment_preimage = None;
let mut payment_secret = None;
let mut amount_msat = 0;
let mut receiver_node_id = None;
let mut _user_payment_id = None::<u64>; // For compatibility with 0.0.103 and earlier
read_tlv_fields!(reader, {
(0, payment_hash, required),
(1, receiver_node_id, option),
(2, payment_secret, option),
(4, amount_msat, required),
(6, _user_payment_id, option),
Expand All @@ -940,6 +958,7 @@ impl MaybeReadable for Event {
None => return Err(msgs::DecodeError::InvalidValue),
};
Ok(Some(Event::PaymentReceived {
receiver_node_id,
payment_hash,
amount_msat,
purpose,
Expand Down Expand Up @@ -1117,13 +1136,16 @@ impl MaybeReadable for Event {
let mut payment_hash = PaymentHash([0; 32]);
let mut purpose = None;
let mut amount_msat = 0;
let mut receiver_node_id = None;
read_tlv_fields!(reader, {
(0, payment_hash, required),
(1, receiver_node_id, option),
(2, purpose, ignorable),
(4, amount_msat, required),
});
if purpose.is_none() { return Ok(None); }
Ok(Some(Event::PaymentClaimed {
receiver_node_id,
payment_hash,
purpose: purpose.unwrap(),
amount_msat,
Expand Down