f - refactor check_total_value, check purpose matches other htlcs

Author: alecchendev

lightning/src/ln/channelmanager.rs

@@ -3548,26 +3548,26 @@ where
 								}
 
 								macro_rules! check_total_value {
-									($payment_secret: expr, $payment_preimage: expr, $is_keysend: expr) => {{
+									($payment_secret: expr, $payment_preimage: expr) => {{
 										let mut payment_claimable_generated = false;
-										let purpose = if $is_keysend {
+										let is_keysend = match claimable_htlc.onion_payload {
+											OnionPayload::Spontaneous(_) => true,
+											OnionPayload::Invoice { .. } => false,
+										};
+										let purpose = if is_keysend {
 											events::PaymentPurpose::SpontaneousPayment(
-												$payment_preimage.expect("Should never call check_total_value with $is_keysend as true but no preimage")
+												$payment_preimage.expect("Should never call check_total_value with keysend payment but no preimage")
 											)
 										} else {
 											events::PaymentPurpose::InvoicePayment {
 												payment_preimage: $payment_preimage,
-												payment_secret: $payment_secret.expect("Should never call check_total_value with $is_keysend as false but no payment secret"),
+												payment_secret: $payment_secret.expect("Should never call check_total_value with non-keysend payment but no payment secret"),
 											}
 										};
 										let mut claimable_payments = self.claimable_payments.lock().unwrap();
 										if claimable_payments.pending_claiming_payments.contains_key(&payment_hash) {
 											fail_htlc!(claimable_htlc, payment_hash);
 										}
-										if $is_keysend && $payment_secret.is_none() && claimable_payments.claimable_payments.get(&payment_hash).is_some() {
-											log_trace!(self.logger, "Failing new keysend HTLC with payment_hash {} for duplicative payment hash", log_bytes!(payment_hash.0));
-											fail_htlc!(claimable_htlc, payment_hash);
-										}
 										let ref mut claimable_payment = claimable_payments.claimable_payments
 											.entry(payment_hash)
 											// Note that if we insert here we MUST NOT fail_htlc!()
@@ -3577,6 +3577,11 @@ where
 													purpose: purpose.clone(), htlcs: Vec::new(), onion_fields: None,
 												}
 											});
+										if purpose != claimable_payment.purpose {
+											let log_keysend = |keysend| if keysend { "keysend" } else { "non-keysend" };
+											log_trace!(self.logger, "Failing new {} HTLC with payment_hash {} as we already had an existing {} HTLC with the same payment hash", log_keysend(is_keysend), log_bytes!(payment_hash.0), log_keysend(!is_keysend));
+											fail_htlc!(claimable_htlc, payment_hash);
+										}
 										if let Some(earlier_fields) = &mut claimable_payment.onion_fields {
 											if earlier_fields.check_merge(&mut onion_fields).is_err() {
 												fail_htlc!(claimable_htlc, payment_hash);
@@ -3585,12 +3590,6 @@ where
 											claimable_payment.onion_fields = Some(onion_fields);
 										}
 										let ref mut htlcs = &mut claimable_payment.htlcs;
-										if !htlcs.is_empty() && !$is_keysend {
-											if let OnionPayload::Spontaneous(_) = htlcs[0].onion_payload {
-												log_trace!(self.logger, "Failing new HTLC with payment_hash {} as we already had an existing keysend HTLC with the same payment hash", log_bytes!(payment_hash.0));
-												fail_htlc!(claimable_htlc, payment_hash);
-											}
-										}
 										let mut total_value = claimable_htlc.sender_intended_value;
 										let mut earliest_expiry = claimable_htlc.cltv_expiry;
 										for htlc in htlcs.iter() {
@@ -3670,10 +3669,10 @@ where
 														fail_htlc!(claimable_htlc, payment_hash);
 													}
 												}
-												check_total_value!(Some(payment_data.payment_secret), payment_preimage, false);
+												check_total_value!(Some(payment_data.payment_secret), payment_preimage);
 											},
 											OnionPayload::Spontaneous(preimage) => {
-												check_total_value!(payment_data.as_ref().map(|d| d.payment_secret), Some(preimage), true);
+												check_total_value!(payment_data.as_ref().map(|d| d.payment_secret), Some(preimage));
 											}
 										}
 									},
@@ -3691,7 +3690,7 @@ where
 												log_bytes!(payment_hash.0), payment_data.total_msat, inbound_payment.get().min_value_msat.unwrap());
 											fail_htlc!(claimable_htlc, payment_hash);
 										} else {
-											let payment_claimable_generated = check_total_value!(Some(payment_data.payment_secret), inbound_payment.get().payment_preimage, false);
+											let payment_claimable_generated = check_total_value!(Some(payment_data.payment_secret), inbound_payment.get().payment_preimage);
 											if payment_claimable_generated {
 												inbound_payment.remove_entry();
 											}