From d1e8d9ced595efe1dbcddde480fccc0d3f98184d Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Wed, 30 Jun 2021 18:35:36 -0400 Subject: [PATCH] Refactor PaymentReceived event for keysend receives --- lightning/src/ln/chanmon_update_fail_tests.rs | 57 ++++++--- lightning/src/ln/channelmanager.rs | 14 ++- lightning/src/ln/functional_test_utils.rs | 24 ++-- lightning/src/ln/functional_tests.rs | 59 +++++++--- lightning/src/util/events.rs | 111 ++++++++++++------ 5 files changed, 182 insertions(+), 83 deletions(-) diff --git a/lightning/src/ln/chanmon_update_fail_tests.rs b/lightning/src/ln/chanmon_update_fail_tests.rs index 1e76117f..3d958292 100644 --- a/lightning/src/ln/chanmon_update_fail_tests.rs +++ b/lightning/src/ln/chanmon_update_fail_tests.rs @@ -28,7 +28,7 @@ use ln::msgs::{ChannelMessageHandler, ErrorAction, RoutingMessageHandler}; use routing::router::get_route; use util::config::UserConfig; use util::enforcing_trait_impls::EnforcingSigner; -use util::events::{Event, MessageSendEvent, MessageSendEventsProvider}; +use util::events::{Event, MessageSendEvent, MessageSendEventsProvider, PaymentPurpose}; use util::errors::APIError; use util::ser::{ReadableArgs, Writeable}; use util::test_utils::TestBroadcaster; @@ -220,11 +220,16 @@ fn do_test_simple_monitor_temporary_update_fail(disconnect: bool, persister_fail 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 payment_preimage, ref payment_secret, amt, user_payment_id: _ } => { + Event::PaymentReceived { ref payment_hash, ref purpose, amt } => { assert_eq!(payment_hash_1, *payment_hash); - assert!(payment_preimage.is_none()); - assert_eq!(payment_secret_1, *payment_secret); assert_eq!(amt, 1000000); + match &purpose { + PaymentPurpose::InvoicePayment { payment_preimage, payment_secret, .. } => { + assert!(payment_preimage.is_none()); + assert_eq!(payment_secret_1, *payment_secret); + }, + _ => panic!("expected PaymentPurpose::InvoicePayment") + } }, _ => panic!("Unexpected event"), } @@ -589,11 +594,16 @@ 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 payment_preimage, ref payment_secret, amt, user_payment_id: _ } => { + Event::PaymentReceived { ref payment_hash, ref purpose, amt } => { assert_eq!(payment_hash_2, *payment_hash); - assert!(payment_preimage.is_none()); - assert_eq!(payment_secret_2, *payment_secret); assert_eq!(amt, 1000000); + match &purpose { + PaymentPurpose::InvoicePayment { payment_preimage, payment_secret, .. } => { + assert!(payment_preimage.is_none()); + assert_eq!(payment_secret_2, *payment_secret); + }, + _ => panic!("expected PaymentPurpose::InvoicePayment") + } }, _ => panic!("Unexpected event"), } @@ -704,11 +714,16 @@ 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, payment_preimage, payment_secret, amt, user_payment_id: _ } => { + Event::PaymentReceived { payment_hash, ref purpose, amt } => { assert_eq!(payment_hash, our_payment_hash); - assert!(payment_preimage.is_none()); - assert_eq!(our_payment_secret, payment_secret); assert_eq!(amt, 1000000); + match &purpose { + PaymentPurpose::InvoicePayment { payment_preimage, payment_secret, .. } => { + assert!(payment_preimage.is_none()); + assert_eq!(our_payment_secret, *payment_secret); + }, + _ => panic!("expected PaymentPurpose::InvoicePayment") + } }, _ => panic!("Unexpected event"), }; @@ -1712,20 +1727,30 @@ 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 payment_preimage, ref payment_secret, amt, user_payment_id: _ } => { + Event::PaymentReceived { ref payment_hash, ref purpose, amt } => { assert_eq!(payment_hash_2, *payment_hash); - assert!(payment_preimage.is_none()); - assert_eq!(payment_secret_2, *payment_secret); assert_eq!(1_000_000, amt); + match &purpose { + PaymentPurpose::InvoicePayment { payment_preimage, payment_secret, .. } => { + assert!(payment_preimage.is_none()); + assert_eq!(payment_secret_2, *payment_secret); + }, + _ => panic!("expected PaymentPurpose::InvoicePayment") + } }, _ => panic!("Unexpected event"), } match events[1] { - Event::PaymentReceived { ref payment_hash, ref payment_preimage, ref payment_secret, amt, user_payment_id: _ } => { + Event::PaymentReceived { ref payment_hash, ref purpose, amt } => { assert_eq!(payment_hash_3, *payment_hash); - assert!(payment_preimage.is_none()); - assert_eq!(payment_secret_3, *payment_secret); assert_eq!(1_000_000, amt); + match &purpose { + PaymentPurpose::InvoicePayment { payment_preimage, payment_secret, .. } => { + assert!(payment_preimage.is_none()); + assert_eq!(payment_secret_3, *payment_secret); + }, + _ => panic!("expected PaymentPurpose::InvoicePayment") + } }, _ => panic!("Unexpected event"), } diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 19a8ad08..3c833081 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -2327,10 +2327,12 @@ impl ChannelMana } else if total_value == payment_data.total_msat { new_events.push(events::Event::PaymentReceived { payment_hash, - payment_preimage: inbound_payment.get().payment_preimage, - payment_secret: payment_data.payment_secret, + purpose: events::PaymentPurpose::InvoicePayment { + payment_preimage: inbound_payment.get().payment_preimage, + payment_secret: payment_data.payment_secret, + user_payment_id: inbound_payment.get().user_payment_id, + }, amt: total_value, - user_payment_id: inbound_payment.get().user_payment_id, }); // Only ever generate at most one PaymentReceived // per registered payment_hash, even if it isn't @@ -3778,7 +3780,7 @@ impl ChannelMana /// The [`PaymentHash`] (and corresponding [`PaymentPreimage`]) must be globally unique. This /// method may return an Err if another payment with the same payment_hash is still pending. /// - /// `user_payment_id` will be provided back in [`PaymentReceived::user_payment_id`] events to + /// `user_payment_id` will be provided back in [`PaymentPurpose::InvoicePayment::user_payment_id`] events to /// allow tracking of which events correspond with which calls to this and /// [`create_inbound_payment`]. `user_payment_id` has no meaning inside of LDK, it is simply /// copied to events and otherwise ignored. It may be used to correlate PaymentReceived events @@ -3812,7 +3814,7 @@ impl ChannelMana /// /// [`create_inbound_payment`]: Self::create_inbound_payment /// [`PaymentReceived`]: events::Event::PaymentReceived - /// [`PaymentReceived::user_payment_id`]: events::Event::PaymentReceived::user_payment_id + /// [`PaymentPurpose::InvoicePayment::user_payment_id`]: events::PaymentPurpose::InvoicePayment::user_payment_id pub fn create_inbound_payment_for_hash(&self, payment_hash: PaymentHash, min_value_msat: Option, invoice_expiry_delta_secs: u32, user_payment_id: u64) -> Result { self.set_payment_hash_secret_map(payment_hash, None, min_value_msat, invoice_expiry_delta_secs, user_payment_id) } @@ -5098,7 +5100,7 @@ pub mod bench { use routing::router::get_route; use util::test_utils; use util::config::UserConfig; - use util::events::{Event, MessageSendEvent, MessageSendEventsProvider}; + use util::events::{Event, MessageSendEvent, MessageSendEventsProvider, PaymentPurpose}; use bitcoin::hashes::Hash; use bitcoin::hashes::sha256::Hash as Sha256; diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index 0adea15c..8172766b 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -23,7 +23,7 @@ use ln::msgs::{ChannelMessageHandler,RoutingMessageHandler}; use util::enforcing_trait_impls::EnforcingSigner; use util::test_utils; use util::test_utils::TestChainMonitor; -use util::events::{Event, MessageSendEvent, MessageSendEventsProvider}; +use util::events::{Event, MessageSendEvent, MessageSendEventsProvider, PaymentPurpose}; use util::errors::APIError; use util::config::UserConfig; use util::ser::{ReadableArgs, Writeable, Readable}; @@ -976,11 +976,16 @@ macro_rules! expect_payment_received { let events = $node.node.get_and_clear_pending_events(); assert_eq!(events.len(), 1); match events[0] { - Event::PaymentReceived { ref payment_hash, ref payment_preimage, ref payment_secret, amt, user_payment_id: _ } => { + Event::PaymentReceived { ref payment_hash, ref purpose, amt } => { assert_eq!($expected_payment_hash, *payment_hash); - assert!(payment_preimage.is_none()); - assert_eq!($expected_payment_secret, *payment_secret); assert_eq!($expected_recv_value, amt); + match purpose { + PaymentPurpose::InvoicePayment { payment_preimage, payment_secret, .. } => { + assert!(payment_preimage.is_none()); + assert_eq!($expected_payment_secret, *payment_secret); + }, + _ => {}, + } }, _ => panic!("Unexpected event"), } @@ -1069,10 +1074,15 @@ pub fn pass_along_path<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_path if payment_received_expected { assert_eq!(events_2.len(), 1); match events_2[0] { - Event::PaymentReceived { ref payment_hash, ref payment_preimage, ref payment_secret, amt, user_payment_id: _ } => { + Event::PaymentReceived { ref payment_hash, ref purpose, amt} => { assert_eq!(our_payment_hash, *payment_hash); - assert!(payment_preimage.is_none()); - assert_eq!(our_payment_secret, *payment_secret); + match &purpose { + PaymentPurpose::InvoicePayment { payment_preimage, payment_secret, .. } => { + assert!(payment_preimage.is_none()); + assert_eq!(our_payment_secret, *payment_secret); + }, + _ => {}, + } assert_eq!(amt, recv_value); }, _ => panic!("Unexpected event"), diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index a149ed23..26e90b97 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -30,7 +30,7 @@ use ln::msgs; use ln::msgs::{ChannelMessageHandler,RoutingMessageHandler,HTLCFailChannelUpdate, ErrorAction}; use util::enforcing_trait_impls::EnforcingSigner; use util::{byte_utils, test_utils}; -use util::events::{Event, MessageSendEvent, MessageSendEventsProvider}; +use util::events::{Event, MessageSendEvent, MessageSendEventsProvider, PaymentPurpose}; use util::errors::APIError; use util::ser::{Writeable, ReadableArgs}; use util::config::UserConfig; @@ -2105,20 +2105,30 @@ 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 payment_preimage, ref payment_secret, amt, user_payment_id: _ } => { + Event::PaymentReceived { ref payment_hash, ref purpose, amt } => { assert_eq!(our_payment_hash_21, *payment_hash); - assert!(payment_preimage.is_none()); - assert_eq!(our_payment_secret_21, *payment_secret); assert_eq!(recv_value_21, amt); + match &purpose { + PaymentPurpose::InvoicePayment { payment_preimage, payment_secret, .. } => { + assert!(payment_preimage.is_none()); + assert_eq!(our_payment_secret_21, *payment_secret); + }, + _ => panic!("expected PaymentPurpose::InvoicePayment") + } }, _ => panic!("Unexpected event"), } match events[1] { - Event::PaymentReceived { ref payment_hash, ref payment_preimage, ref payment_secret, amt, user_payment_id: _ } => { + Event::PaymentReceived { ref payment_hash, ref purpose, amt } => { assert_eq!(our_payment_hash_22, *payment_hash); - assert!(payment_preimage.is_none()); - assert_eq!(our_payment_secret_22, *payment_secret); assert_eq!(recv_value_22, amt); + match &purpose { + PaymentPurpose::InvoicePayment { payment_preimage, payment_secret, .. } => { + assert!(payment_preimage.is_none()); + assert_eq!(our_payment_secret_22, *payment_secret); + }, + _ => panic!("expected PaymentPurpose::InvoicePayment") + } }, _ => panic!("Unexpected event"), } @@ -3740,11 +3750,16 @@ 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 payment_preimage, ref payment_secret, amt, user_payment_id: _ } => { + Event::PaymentReceived { ref payment_hash, ref purpose, amt } => { assert_eq!(payment_hash_1, *payment_hash); - assert!(payment_preimage.is_none()); - assert_eq!(payment_secret_1, *payment_secret); assert_eq!(amt, 1000000); + match &purpose { + PaymentPurpose::InvoicePayment { payment_preimage, payment_secret, .. } => { + assert!(payment_preimage.is_none()); + assert_eq!(payment_secret_1, *payment_secret); + }, + _ => panic!("expected PaymentPurpose::InvoicePayment") + } }, _ => panic!("Unexpected event"), } @@ -4138,10 +4153,15 @@ fn test_drop_messages_peer_disconnect_dual_htlc() { 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 payment_preimage, ref payment_secret, amt: _, user_payment_id: _ } => { + Event::PaymentReceived { ref payment_hash, ref purpose, .. } => { assert_eq!(payment_hash_2, *payment_hash); - assert!(payment_preimage.is_none()); - assert_eq!(payment_secret_2, *payment_secret); + match &purpose { + PaymentPurpose::InvoicePayment { payment_preimage, payment_secret, .. } => { + assert!(payment_preimage.is_none()); + assert_eq!(payment_secret_2, *payment_secret); + }, + _ => panic!("expected PaymentPurpose::InvoicePayment") + } }, _ => panic!("Unexpected event"), } @@ -8648,9 +8668,14 @@ fn test_preimage_storage() { let events = nodes[1].node.get_and_clear_pending_events(); assert_eq!(events.len(), 1); match events[0] { - Event::PaymentReceived { payment_preimage, user_payment_id, .. } => { - assert_eq!(user_payment_id, 42); - claim_payment(&nodes[0], &[&nodes[1]], payment_preimage.unwrap()); + Event::PaymentReceived { ref purpose, .. } => { + match &purpose { + PaymentPurpose::InvoicePayment { payment_preimage, user_payment_id, .. } => { + assert_eq!(*user_payment_id, 42); + claim_payment(&nodes[0], &[&nodes[1]], payment_preimage.unwrap()); + }, + _ => panic!("expected PaymentPurpose::InvoicePayment") + } }, _ => panic!("Unexpected event"), } @@ -8714,7 +8739,7 @@ fn test_secret_timeout() { let events = nodes[1].node.get_and_clear_pending_events(); assert_eq!(events.len(), 1); match events[0] { - Event::PaymentReceived { payment_preimage, payment_secret, user_payment_id, .. } => { + Event::PaymentReceived { purpose: PaymentPurpose::InvoicePayment { payment_preimage, payment_secret, user_payment_id }, .. } => { assert!(payment_preimage.is_none()); assert_eq!(user_payment_id, 42); assert_eq!(payment_secret, our_payment_secret); diff --git a/lightning/src/util/events.rs b/lightning/src/util/events.rs index 0fc7c6b3..876dfe6a 100644 --- a/lightning/src/util/events.rs +++ b/lightning/src/util/events.rs @@ -27,6 +27,45 @@ use prelude::*; use core::time::Duration; use core::ops::Deref; +/// Some information provided on receipt of payment depends on whether the payment received is a +/// spontaneous payment or a "conventional" lightning payment that's paying an invoice. +#[derive(Clone, Debug)] +pub enum PaymentPurpose { + /// Information for receiving a payment that we generated an invoice for. + InvoicePayment { + /// The preimage to the payment_hash, if the payment hash (and secret) were fetched via + /// [`ChannelManager::create_inbound_payment`]. If provided, this can be handed directly to + /// [`ChannelManager::claim_funds`]. + /// + /// [`ChannelManager::create_inbound_payment`]: crate::ln::channelmanager::ChannelManager::create_inbound_payment + /// [`ChannelManager::claim_funds`]: crate::ln::channelmanager::ChannelManager::claim_funds + payment_preimage: Option, + /// The "payment secret". This authenticates the sender to the recipient, preventing a + /// number of deanonymization attacks during the routing process. + /// It is provided here for your reference, however its accuracy is enforced directly by + /// [`ChannelManager`] using the values you previously provided to + /// [`ChannelManager::create_inbound_payment`] or + /// [`ChannelManager::create_inbound_payment_for_hash`]. + /// + /// [`ChannelManager`]: crate::ln::channelmanager::ChannelManager + /// [`ChannelManager::create_inbound_payment`]: crate::ln::channelmanager::ChannelManager::create_inbound_payment + /// [`ChannelManager::create_inbound_payment_for_hash`]: crate::ln::channelmanager::ChannelManager::create_inbound_payment_for_hash + payment_secret: PaymentSecret, + /// This is the `user_payment_id` which was provided to + /// [`ChannelManager::create_inbound_payment_for_hash`] or + /// [`ChannelManager::create_inbound_payment`]. It has no meaning inside of LDK and is + /// simply copied here. It may be used to correlate PaymentReceived events with invoice + /// metadata stored elsewhere. + /// + /// [`ChannelManager::create_inbound_payment`]: crate::ln::channelmanager::ChannelManager::create_inbound_payment + /// [`ChannelManager::create_inbound_payment_for_hash`]: crate::ln::channelmanager::ChannelManager::create_inbound_payment_for_hash + user_payment_id: u64, + }, + /// Because this is a spontaneous payment, the payer generated their own preimage rather than us + /// (the payee) providing a preimage. + SpontaneousPayment(PaymentPreimage), +} + /// An Event which you should probably take some action in response to. /// /// Note that while Writeable and Readable are implemented for Event, you probably shouldn't use @@ -63,37 +102,13 @@ pub enum Event { PaymentReceived { /// The hash for which the preimage should be handed to the ChannelManager. payment_hash: PaymentHash, - /// The preimage to the payment_hash, if the payment hash (and secret) were fetched via - /// [`ChannelManager::create_inbound_payment`]. If provided, this can be handed directly to - /// [`ChannelManager::claim_funds`]. - /// - /// [`ChannelManager::create_inbound_payment`]: crate::ln::channelmanager::ChannelManager::create_inbound_payment - /// [`ChannelManager::claim_funds`]: crate::ln::channelmanager::ChannelManager::claim_funds - payment_preimage: Option, - /// The "payment secret". This authenticates the sender to the recipient, preventing a - /// number of deanonymization attacks during the routing process. - /// It is provided here for your reference, however its accuracy is enforced directly by - /// [`ChannelManager`] using the values you previously provided to - /// [`ChannelManager::create_inbound_payment`] or - /// [`ChannelManager::create_inbound_payment_for_hash`]. - /// - /// [`ChannelManager`]: crate::ln::channelmanager::ChannelManager - /// [`ChannelManager::create_inbound_payment`]: crate::ln::channelmanager::ChannelManager::create_inbound_payment - /// [`ChannelManager::create_inbound_payment_for_hash`]: crate::ln::channelmanager::ChannelManager::create_inbound_payment_for_hash - payment_secret: PaymentSecret, /// The value, in thousandths of a satoshi, that this payment is for. Note that you must /// compare this to the expected value before accepting the payment (as otherwise you are /// providing proof-of-payment for less than the value you expected!). amt: u64, - /// This is the `user_payment_id` which was provided to - /// [`ChannelManager::create_inbound_payment_for_hash`] or - /// [`ChannelManager::create_inbound_payment`]. It has no meaning inside of LDK and is - /// simply copied here. It may be used to correlate PaymentReceived events with invoice - /// metadata stored elsewhere. - /// - /// [`ChannelManager::create_inbound_payment`]: crate::ln::channelmanager::ChannelManager::create_inbound_payment - /// [`ChannelManager::create_inbound_payment_for_hash`]: crate::ln::channelmanager::ChannelManager::create_inbound_payment_for_hash - user_payment_id: u64, + /// Information for claiming this received payment, based on whether the purpose of the + /// payment is to pay an invoice or to send a spontaneous payment. + purpose: PaymentPurpose, }, /// Indicates an outbound payment we made succeeded (ie it made it all the way to its target /// and we got back the payment preimage for it). @@ -145,13 +160,26 @@ 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 payment_preimage, ref payment_secret, ref amt, ref user_payment_id } => { + &Event::PaymentReceived { ref payment_hash, ref amt, ref purpose } => { 1u8.write(writer)?; + let mut payment_secret = None; + let mut user_payment_id = None; + let payment_preimage; + match &purpose { + PaymentPurpose::InvoicePayment { payment_preimage: preimage, payment_secret: secret, user_payment_id: id } => { + payment_secret = Some(secret); + payment_preimage = *preimage; + user_payment_id = Some(id); + }, + PaymentPurpose::SpontaneousPayment(preimage) => { + payment_preimage = Some(*preimage); + } + } write_tlv_fields!(writer, { (0, payment_hash, required), - (2, payment_secret, required), + (2, payment_secret, option), (4, amt, required), - (6, user_payment_id, required), + (6, user_payment_id, option), (8, payment_preimage, option), }); }, @@ -201,22 +229,31 @@ impl MaybeReadable for Event { let f = || { let mut payment_hash = PaymentHash([0; 32]); let mut payment_preimage = None; - let mut payment_secret = PaymentSecret([0; 32]); + let mut payment_secret = None; let mut amt = 0; - let mut user_payment_id = 0; + let mut user_payment_id = None; read_tlv_fields!(reader, { (0, payment_hash, required), - (2, payment_secret, required), + (2, payment_secret, option), (4, amt, required), - (6, user_payment_id, required), + (6, user_payment_id, option), (8, payment_preimage, option), }); + let purpose = match payment_secret { + Some(secret) => PaymentPurpose::InvoicePayment { + payment_preimage, + payment_secret: secret, + user_payment_id: if let Some(id) = user_payment_id { + id + } else { return Err(msgs::DecodeError::InvalidValue) } + }, + None if payment_preimage.is_some() => PaymentPurpose::SpontaneousPayment(payment_preimage.unwrap()), + None => return Err(msgs::DecodeError::InvalidValue), + }; Ok(Some(Event::PaymentReceived { payment_hash, - payment_preimage, - payment_secret, amt, - user_payment_id, + purpose, })) }; f() -- 2.30.2