mirrors/rust-lightning
1
0
Fork 0
mirror of https://github.com/lightningdevkit/rust-lightning.git synced 2025-02-25 07:17:40 +01:00
Code Issues Projects Releases Packages Wiki Activity

Merge pull request #1180 from valentinewallace/2021-11-remove-user-pmt-id

Browse source 
Remove user_payment_id
This commit is contained in:
Matt Corallo 2021-11-22 16:40:37 +00:00 • committed by GitHub
commit 58539b8440
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 37 additions and 58 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
fuzz/src/chanmon_consistency.rs
View file

@ -284,7 +284,7 @@ fn get_payment_secret_hash(dest: &ChanMan, payment_id: &mut u8) -> Option<(Payme
let mut payment_hash;
for _ in 0..256 {
payment_hash = PaymentHash(Sha256::hash(&[*payment_id; 1]).into_inner());
if let Ok(payment_secret) = dest.create_inbound_payment_for_hash(payment_hash, None, 3600, 0) {
if let Ok(payment_secret) = dest.create_inbound_payment_for_hash(payment_hash, None, 3600) {
return Some((payment_secret, payment_hash));
}
*payment_id = payment_id.wrapping_add(1);

2
fuzz/src/full_stack.rs
View file

@ -529,7 +529,7 @@ pub fn do_test(data: &[u8], logger: &Arc<dyn Logger>) {
let payment_hash = PaymentHash(Sha256::from_engine(sha).into_inner());
// Note that this may fail - our hashes may collide and we'll end up trying to
// double-register the same payment_hash.
let _ = channelmanager.create_inbound_payment_for_hash(payment_hash, None, 1, 0);
let _ = channelmanager.create_inbound_payment_for_hash(payment_hash, None, 1);
},
9 => {
for payment in payments_received.drain(..) {

1
lightning-invoice/src/utils.rs
View file

@ -63,7 +63,6 @@ where
let (payment_hash, payment_secret) = channelmanager.create_inbound_payment(
amt_msat,
DEFAULT_EXPIRY_TIME.try_into().unwrap(),
0,
);
let our_node_pubkey = channelmanager.get_our_node_id();
let mut invoice = InvoiceBuilder::new(network)

26
lightning/src/ln/channelmanager.rs
View file

@ -413,6 +413,9 @@ struct PeerState {
///
/// For users who don't want to bother doing their own payment preimage storage, we also store that
/// here.
///
/// Note that this struct will be removed entirely soon, in favor of storing no inbound payment data
/// and instead encoding it in the payment secret.
struct PendingInboundPayment {
/// The payment secret that the sender must use for us to accept this payment
payment_secret: PaymentSecret,
@ -2887,7 +2890,6 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
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,
});
@ -4523,7 +4525,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
}
}
fn set_payment_hash_secret_map(&self, payment_hash: PaymentHash, payment_preimage: Option<PaymentPreimage>, min_value_msat: Option<u64>, invoice_expiry_delta_secs: u32, user_payment_id: u64) -> Result<PaymentSecret, APIError> {
fn set_payment_hash_secret_map(&self, payment_hash: PaymentHash, payment_preimage: Option<PaymentPreimage>, min_value_msat: Option<u64>, invoice_expiry_delta_secs: u32) -> Result<PaymentSecret, APIError> {
assert!(invoice_expiry_delta_secs <= 60*60*24*365); // Sadly bitcoin timestamps are u32s, so panic before 2106
let payment_secret = PaymentSecret(self.keys_manager.get_secure_random_bytes());
@ -4533,7 +4535,8 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
match payment_secrets.entry(payment_hash) {
hash_map::Entry::Vacant(e) => {
e.insert(PendingInboundPayment {
payment_secret, min_value_msat, user_payment_id, payment_preimage,
payment_secret, min_value_msat, payment_preimage,
user_payment_id: 0, // For compatibility with version 0.0.103 and earlier
// We assume that highest_seen_timestamp is pretty close to the current time -
// its updated when we receive a new block with the maximum time we've seen in
// a header. It should never be more than two hours in the future.
@ -4565,12 +4568,12 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
/// [`PaymentReceived`]: events::Event::PaymentReceived
/// [`PaymentReceived::payment_preimage`]: events::Event::PaymentReceived::payment_preimage
/// [`create_inbound_payment_for_hash`]: Self::create_inbound_payment_for_hash
pub fn create_inbound_payment(&self, min_value_msat: Option<u64>, invoice_expiry_delta_secs: u32, user_payment_id: u64) -> (PaymentHash, PaymentSecret) {
pub fn create_inbound_payment(&self, min_value_msat: Option<u64>, invoice_expiry_delta_secs: u32) -> (PaymentHash, PaymentSecret) {
let payment_preimage = PaymentPreimage(self.keys_manager.get_secure_random_bytes());
let payment_hash = PaymentHash(Sha256::hash(&payment_preimage.0).into_inner());
(payment_hash,
self.set_payment_hash_secret_map(payment_hash, Some(payment_preimage), min_value_msat, invoice_expiry_delta_secs, user_payment_id)
self.set_payment_hash_secret_map(payment_hash, Some(payment_preimage), min_value_msat, invoice_expiry_delta_secs)
.expect("RNG Generated Duplicate PaymentHash"))
}
@ -4584,12 +4587,6 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> 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 [`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
/// with invoice metadata stored elsewhere.
///
/// `min_value_msat` should be set if the invoice being generated contains a value. Any payment
/// received for the returned [`PaymentHash`] will be required to be at least `min_value_msat`
/// before a [`PaymentReceived`] event will be generated, ensuring that we do not provide the
@ -4618,9 +4615,8 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
///
/// [`create_inbound_payment`]: Self::create_inbound_payment
/// [`PaymentReceived`]: events::Event::PaymentReceived
/// [`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<u64>, invoice_expiry_delta_secs: u32, user_payment_id: u64) -> Result<PaymentSecret, APIError> {
self.set_payment_hash_secret_map(payment_hash, None, min_value_msat, invoice_expiry_delta_secs, user_payment_id)
pub fn create_inbound_payment_for_hash(&self, payment_hash: PaymentHash, min_value_msat: Option<u64>, invoice_expiry_delta_secs: u32) -> Result<PaymentSecret, APIError> {
self.set_payment_hash_secret_map(payment_hash, None, min_value_msat, invoice_expiry_delta_secs)
}
#[cfg(any(test, feature = "fuzztarget", feature = "_test_utils"))]
@ -6682,7 +6678,7 @@ pub mod bench {
payment_preimage.0[0..8].copy_from_slice(&payment_count.to_le_bytes());
payment_count += 1;
let payment_hash = PaymentHash(Sha256::hash(&payment_preimage.0[..]).into_inner());
let payment_secret = $node_b.create_inbound_payment_for_hash(payment_hash, None, 7200, 0).unwrap();
let payment_secret = $node_b.create_inbound_payment_for_hash(payment_hash, None, 7200).unwrap();
$node_a.send_payment(&route, payment_hash, &Some(payment_secret)).unwrap();
let payment_event = SendEvent::from_event($node_a.get_and_clear_pending_msg_events().pop().unwrap());

2
lightning/src/ln/functional_test_utils.rs
View file

@ -1007,7 +1007,7 @@ macro_rules! get_payment_preimage_hash {
let payment_preimage = PaymentPreimage([*payment_count; 32]);
*payment_count += 1;
let payment_hash = PaymentHash(Sha256::hash(&payment_preimage.0[..]).into_inner());
let payment_secret = $dest_node.node.create_inbound_payment_for_hash(payment_hash, $min_value_msat, 7200, 0).unwrap();
let payment_secret = $dest_node.node.create_inbound_payment_for_hash(payment_hash, $min_value_msat, 7200).unwrap();
(payment_preimage, payment_hash, payment_secret)
}
}

38
lightning/src/ln/functional_tests.rs
View file

@ -1152,7 +1152,7 @@ fn test_duplicate_htlc_different_direction_onchain() {
let (payment_preimage, payment_hash, _) = route_payment(&nodes[0], &vec!(&nodes[1])[..], 900_000);
let (route, _, _, _) = get_route_and_payment_hash!(nodes[1], nodes[0], 800_000);
let node_a_payment_secret = nodes[0].node.create_inbound_payment_for_hash(payment_hash, None, 7200, 0).unwrap();
let node_a_payment_secret = nodes[0].node.create_inbound_payment_for_hash(payment_hash, None, 7200).unwrap();
send_along_route_with_secret(&nodes[1], route, &[&[&nodes[0]]], 800_000, payment_hash, node_a_payment_secret);
// Provide preimage to node 0 by claiming payment
@ -4957,7 +4957,7 @@ fn test_duplicate_payment_hash_one_failure_one_success() {
let (our_payment_preimage, duplicate_payment_hash, _) = route_payment(&nodes[0], &vec!(&nodes[1], &nodes[2])[..], 900000);
let payment_secret = nodes[3].node.create_inbound_payment_for_hash(duplicate_payment_hash, None, 7200, 0).unwrap();
let payment_secret = nodes[3].node.create_inbound_payment_for_hash(duplicate_payment_hash, None, 7200).unwrap();
// We reduce the final CLTV here by a somewhat arbitrary constant to keep it under the one-byte
// script push size limit so that the below script length checks match
// ACCEPTED_HTLC_SCRIPT_WEIGHT.
@ -5160,30 +5160,30 @@ fn do_test_fail_backwards_unrevoked_remote_announce(deliver_last_raa: bool, anno
let (_, payment_hash_2, _) = route_payment(&nodes[0], &[&nodes[2], &nodes[3], &nodes[4]], ds_dust_limit*1000); // not added < dust limit + HTLC tx fee
let (route, _, _, _) = get_route_and_payment_hash!(nodes[1], nodes[5], ds_dust_limit*1000);
// 2nd HTLC:
send_along_route_with_secret(&nodes[1], route.clone(), &[&[&nodes[2], &nodes[3], &nodes[5]]], ds_dust_limit*1000, payment_hash_1, nodes[5].node.create_inbound_payment_for_hash(payment_hash_1, None, 7200, 0).unwrap()); // not added < dust limit + HTLC tx fee
send_along_route_with_secret(&nodes[1], route.clone(), &[&[&nodes[2], &nodes[3], &nodes[5]]], ds_dust_limit*1000, payment_hash_1, nodes[5].node.create_inbound_payment_for_hash(payment_hash_1, None, 7200).unwrap()); // not added < dust limit + HTLC tx fee
// 3rd HTLC:
send_along_route_with_secret(&nodes[1], route, &[&[&nodes[2], &nodes[3], &nodes[5]]], ds_dust_limit*1000, payment_hash_2, nodes[5].node.create_inbound_payment_for_hash(payment_hash_2, None, 7200, 0).unwrap()); // not added < dust limit + HTLC tx fee
send_along_route_with_secret(&nodes[1], route, &[&[&nodes[2], &nodes[3], &nodes[5]]], ds_dust_limit*1000, payment_hash_2, nodes[5].node.create_inbound_payment_for_hash(payment_hash_2, None, 7200).unwrap()); // not added < dust limit + HTLC tx fee
// 4th HTLC:
let (_, payment_hash_3, _) = route_payment(&nodes[0], &[&nodes[2], &nodes[3], &nodes[4]], 1000000);
// 5th HTLC:
let (_, payment_hash_4, _) = route_payment(&nodes[0], &[&nodes[2], &nodes[3], &nodes[4]], 1000000);
let (route, _, _, _) = get_route_and_payment_hash!(nodes[1], nodes[5], 1000000);
// 6th HTLC:
send_along_route_with_secret(&nodes[1], route.clone(), &[&[&nodes[2], &nodes[3], &nodes[5]]], 1000000, payment_hash_3, nodes[5].node.create_inbound_payment_for_hash(payment_hash_3, None, 7200, 0).unwrap());
send_along_route_with_secret(&nodes[1], route.clone(), &[&[&nodes[2], &nodes[3], &nodes[5]]], 1000000, payment_hash_3, nodes[5].node.create_inbound_payment_for_hash(payment_hash_3, None, 7200).unwrap());
// 7th HTLC:
send_along_route_with_secret(&nodes[1], route, &[&[&nodes[2], &nodes[3], &nodes[5]]], 1000000, payment_hash_4, nodes[5].node.create_inbound_payment_for_hash(payment_hash_4, None, 7200, 0).unwrap());
send_along_route_with_secret(&nodes[1], route, &[&[&nodes[2], &nodes[3], &nodes[5]]], 1000000, payment_hash_4, nodes[5].node.create_inbound_payment_for_hash(payment_hash_4, None, 7200).unwrap());
// 8th HTLC:
let (_, payment_hash_5, _) = route_payment(&nodes[0], &[&nodes[2], &nodes[3], &nodes[4]], 1000000);
// 9th HTLC:
let (route, _, _, _) = get_route_and_payment_hash!(nodes[1], nodes[5], ds_dust_limit*1000);
send_along_route_with_secret(&nodes[1], route, &[&[&nodes[2], &nodes[3], &nodes[5]]], ds_dust_limit*1000, payment_hash_5, nodes[5].node.create_inbound_payment_for_hash(payment_hash_5, None, 7200, 0).unwrap()); // not added < dust limit + HTLC tx fee
send_along_route_with_secret(&nodes[1], route, &[&[&nodes[2], &nodes[3], &nodes[5]]], ds_dust_limit*1000, payment_hash_5, nodes[5].node.create_inbound_payment_for_hash(payment_hash_5, None, 7200).unwrap()); // not added < dust limit + HTLC tx fee
// 10th HTLC:
let (_, payment_hash_6, _) = route_payment(&nodes[0], &[&nodes[2], &nodes[3], &nodes[4]], ds_dust_limit*1000); // not added < dust limit + HTLC tx fee
// 11th HTLC:
let (route, _, _, _) = get_route_and_payment_hash!(nodes[1], nodes[5], 1000000);
send_along_route_with_secret(&nodes[1], route, &[&[&nodes[2], &nodes[3], &nodes[5]]], 1000000, payment_hash_6, nodes[5].node.create_inbound_payment_for_hash(payment_hash_6, None, 7200, 0).unwrap());
send_along_route_with_secret(&nodes[1], route, &[&[&nodes[2], &nodes[3], &nodes[5]]], 1000000, payment_hash_6, nodes[5].node.create_inbound_payment_for_hash(payment_hash_6, None, 7200).unwrap());
// Double-check that six of the new HTLC were added
// We now have six HTLCs pending over the dust limit and six HTLCs under the dust limit (ie,
@ -7164,7 +7164,7 @@ fn test_check_htlc_underpaying() {
let payee = Payee::from_node_id(nodes[1].node.get_our_node_id()).with_features(InvoiceFeatures::known());
let route = get_route(&nodes[0].node.get_our_node_id(), &payee, nodes[0].network_graph, None, 10_000, TEST_FINAL_CLTV, nodes[0].logger, &scorer).unwrap();
let (_, our_payment_hash, _) = get_payment_preimage_hash!(nodes[0]);
let our_payment_secret = nodes[1].node.create_inbound_payment_for_hash(our_payment_hash, Some(100_000), 7200, 0).unwrap();
let our_payment_secret = nodes[1].node.create_inbound_payment_for_hash(our_payment_hash, Some(100_000), 7200).unwrap();
nodes[0].node.send_payment(&route, our_payment_hash, &Some(our_payment_secret)).unwrap();
check_added_monitors!(nodes[0], 1);
@ -8018,7 +8018,7 @@ fn test_preimage_storage() {
create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known()).0.contents.short_channel_id;
{
let (payment_hash, payment_secret) = nodes[1].node.create_inbound_payment(Some(100_000), 7200, 42);
let (payment_hash, payment_secret) = nodes[1].node.create_inbound_payment(Some(100_000), 7200);
let (route, _, _, _) = get_route_and_payment_hash!(nodes[0], nodes[1], 100_000);
nodes[0].node.send_payment(&route, payment_hash, &Some(payment_secret)).unwrap();
check_added_monitors!(nodes[0], 1);
@ -8035,8 +8035,7 @@ fn test_preimage_storage() {
match events[0] {
Event::PaymentReceived { ref purpose, .. } => {
match &purpose {
PaymentPurpose::InvoicePayment { payment_preimage, user_payment_id, .. } => {
assert_eq!(*user_payment_id, 42);
PaymentPurpose::InvoicePayment { payment_preimage, .. } => {
claim_payment(&nodes[0], &[&nodes[1]], payment_preimage.unwrap());
},
_ => panic!("expected PaymentPurpose::InvoicePayment")
@ -8056,11 +8055,11 @@ fn test_secret_timeout() {
create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known()).0.contents.short_channel_id;
let (payment_hash, payment_secret_1) = nodes[1].node.create_inbound_payment(Some(100_000), 2, 0);
let (payment_hash, payment_secret_1) = nodes[1].node.create_inbound_payment(Some(100_000), 2);
// We should fail to register the same payment hash twice, at least until we've connected a
// block with time 7200 + CHAN_CONFIRM_DEPTH + 1.
if let Err(APIError::APIMisuseError { err }) = nodes[1].node.create_inbound_payment_for_hash(payment_hash, Some(100_000), 2, 0) {
if let Err(APIError::APIMisuseError { err }) = nodes[1].node.create_inbound_payment_for_hash(payment_hash, Some(100_000), 2) {
assert_eq!(err, "Duplicate payment hash");
} else { panic!(); }
let mut block = {
@ -8075,16 +8074,16 @@ fn test_secret_timeout() {
}
};
connect_block(&nodes[1], &block);
if let Err(APIError::APIMisuseError { err }) = nodes[1].node.create_inbound_payment_for_hash(payment_hash, Some(100_000), 2, 0) {
if let Err(APIError::APIMisuseError { err }) = nodes[1].node.create_inbound_payment_for_hash(payment_hash, Some(100_000), 2) {
assert_eq!(err, "Duplicate payment hash");
} else { panic!(); }
// If we then connect the second block, we should be able to register the same payment hash
// again with a different user_payment_id (this time getting a new payment secret).
// again (this time getting a new payment secret).
block.header.prev_blockhash = block.header.block_hash();
block.header.time += 1;
connect_block(&nodes[1], &block);
let our_payment_secret = nodes[1].node.create_inbound_payment_for_hash(payment_hash, Some(100_000), 2, 42).unwrap();
let our_payment_secret = nodes[1].node.create_inbound_payment_for_hash(payment_hash, Some(100_000), 2).unwrap();
assert_ne!(payment_secret_1, our_payment_secret);
{
@ -8102,9 +8101,8 @@ fn test_secret_timeout() {
let events = nodes[1].node.get_and_clear_pending_events();
assert_eq!(events.len(), 1);
match events[0] {
Event::PaymentReceived { purpose: PaymentPurpose::InvoicePayment { payment_preimage, payment_secret, user_payment_id }, .. } => {
Event::PaymentReceived { purpose: PaymentPurpose::InvoicePayment { payment_preimage, payment_secret }, .. } => {
assert!(payment_preimage.is_none());
assert_eq!(user_payment_id, 42);
assert_eq!(payment_secret, our_payment_secret);
// We don't actually have the payment preimage with which to claim this payment!
},
@ -8124,7 +8122,7 @@ fn test_bad_secret_hash() {
let random_payment_hash = PaymentHash([42; 32]);
let random_payment_secret = PaymentSecret([43; 32]);
let (our_payment_hash, our_payment_secret) = nodes[1].node.create_inbound_payment(Some(100_000), 2, 0);
let (our_payment_hash, our_payment_secret) = nodes[1].node.create_inbound_payment(Some(100_000), 2);
let (route, _, _, _) = get_route_and_payment_hash!(nodes[0], nodes[1], 100_000);
// All the below cases should end up being handled exactly identically, so we macro the

24
lightning/src/util/events.rs
View file

@ -60,15 +60,6 @@ pub enum PaymentPurpose {
/// [`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.
@ -352,13 +343,11 @@ impl Writeable for Event {
&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 } => {
PaymentPurpose::InvoicePayment { payment_preimage: preimage, payment_secret: secret } => {
payment_secret = Some(secret);
payment_preimage = *preimage;
user_payment_id = Some(id);
},
PaymentPurpose::SpontaneousPayment(preimage) => {
payment_preimage = Some(*preimage);
@ -368,7 +357,7 @@ impl Writeable for Event {
(0, payment_hash, required),
(2, payment_secret, option),
(4, amt, required),
(6, user_payment_id, option),
(6, 0u64, required), // user_payment_id required for compatibility with 0.0.103 and earlier
(8, payment_preimage, option),
});
},
@ -457,21 +446,18 @@ impl MaybeReadable for Event {
let mut payment_preimage = None;
let mut payment_secret = None;
let mut amt = 0;
let mut user_payment_id = None;
let mut _user_payment_id = None; // For compatibility with 0.0.103 and earlier
read_tlv_fields!(reader, {
(0, payment_hash, required),
(2, payment_secret, option),
(4, amt, required),
(6, user_payment_id, option),
(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) }
payment_secret: secret
},
None if payment_preimage.is_some() => PaymentPurpose::SpontaneousPayment(payment_preimage.unwrap()),
None => return Err(msgs::DecodeError::InvalidValue),