f append to pending_events in outbound_payment

and get rid of an unnecessary mut

Author: valentinewallace

lightning/src/ln/channelmanager.rs

@@ -3478,10 +3478,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
 		// being fully configured. See the docs for `ChannelManagerReadArgs` for more.
 		match source {
 			HTLCSource::OutboundRoute { ref path, ref session_priv, ref payment_id, ref payment_params, .. } => {
-				let (path_failure, full_failure_ev) = self.pending_outbound_payments.fail_htlc(source, payment_hash, onion_error, path, session_priv, payment_id, payment_params, self.probing_cookie_secret, &self.secp_ctx, &self.logger);
-				let mut pending_events = self.pending_events.lock().unwrap();
-				if let Some(ev) = path_failure { pending_events.push(ev); }
-				if let Some(ev) = full_failure_ev { pending_events.push(ev); }
+				self.pending_outbound_payments.fail_htlc(source, payment_hash, onion_error, path, session_priv, payment_id, payment_params, self.probing_cookie_secret, &self.secp_ctx, &self.pending_events, &self.logger);
 			},
 			HTLCSource::PreviousHopData(HTLCPreviousHopData { ref short_channel_id, ref htlc_id, ref incoming_packet_shared_secret, ref phantom_shared_secret, ref outpoint }) => {
 				log_trace!(self.logger, "Failing HTLC with payment_hash {} backwards from us with {:?}", log_bytes!(payment_hash.0), onion_error);
@@ -3765,20 +3762,14 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
 	}
 
 	fn finalize_claims(&self, sources: Vec<HTLCSource>) {
-		let mut path_success_evs = self.pending_outbound_payments.finalize_claims(sources);
-		if path_success_evs.len() == 0 { return }
-		let mut pending_events = self.pending_events.lock().unwrap();
-		pending_events.append(&mut path_success_evs);
+		self.pending_outbound_payments.finalize_claims(sources, &self.pending_events);
 	}
 
 	fn claim_funds_internal(&self, channel_state_lock: MutexGuard<ChannelHolder<<K::Target as KeysInterface>::Signer>>, source: HTLCSource, payment_preimage: PaymentPreimage, forwarded_htlc_value_msat: Option<u64>, from_onchain: bool, next_channel_id: [u8; 32]) {
 		match source {
 			HTLCSource::OutboundRoute { session_priv, payment_id, path, .. } => {
 				mem::drop(channel_state_lock);
-				let (payment_sent_ev, path_success_ev) = self.pending_outbound_payments.claim_htlc(payment_id, payment_preimage, session_priv, path, from_onchain, &self.logger);
-				let mut pending_events = self.pending_events.lock().unwrap();
-				if let Some(ev) = payment_sent_ev { pending_events.push(ev); }
-				if let Some(ev) = path_success_ev { pending_events.push(ev); }
+				self.pending_outbound_payments.claim_htlc(payment_id, payment_preimage, session_priv, path, from_onchain, &self.pending_events, &self.logger);
 			},
 			HTLCSource::PreviousHopData(hop_data) => {
 				let prev_outpoint = hop_data.outpoint;

lightning/src/ln/outbound_payment.rs

@@ -561,24 +561,24 @@ impl OutboundPayments {
 
 	pub(super) fn claim_htlc<L: Deref>(
 		&self, payment_id: PaymentId, payment_preimage: PaymentPreimage, session_priv: SecretKey,
-		path: Vec<RouteHop>, from_onchain: bool, logger: &L)
-	-> (Option<events::Event>, Option<events::Event>) where L::Target: Logger {
-		let mut payment_sent_ev = None;
-		let mut path_success_ev = None;
+		path: Vec<RouteHop>, from_onchain: bool, pending_events: &Mutex<Vec<events::Event>>, logger: &L)
+	 where L::Target: Logger {
 		let mut session_priv_bytes = [0; 32];
 		session_priv_bytes.copy_from_slice(&session_priv[..]);
 		let mut outbounds = self.pending_outbound_payments.lock().unwrap();
+		let mut pending_events = pending_events.lock().unwrap();
 		if let hash_map::Entry::Occupied(mut payment) = outbounds.entry(payment_id) {
 			if !payment.get().is_fulfilled() {
 				let payment_hash = PaymentHash(Sha256::hash(&payment_preimage.0).into_inner());
 				let fee_paid_msat = payment.get().get_pending_fee_msat();
-				payment_sent_ev =
-					Some(events::Event::PaymentSent {
+				pending_events.push(
+					events::Event::PaymentSent {
 						payment_id: Some(payment_id),
 						payment_preimage,
 						payment_hash,
 						fee_paid_msat,
-					});
+					}
+				);
 				payment.get_mut().mark_fulfilled();
 			}
 
@@ -591,31 +591,31 @@ impl OutboundPayments {
 				// irrevocably fulfilled.
 				if payment.get_mut().remove(&session_priv_bytes, Some(&path)) {
 					let payment_hash = Some(PaymentHash(Sha256::hash(&payment_preimage.0).into_inner()));
-					path_success_ev =
-						Some(events::Event::PaymentPathSuccessful {
+					pending_events.push(
+						events::Event::PaymentPathSuccessful {
 							payment_id,
 							payment_hash,
 							path,
-						});
+						}
+					);
 				}
 			}
 		} else {
 			log_trace!(logger, "Received duplicative fulfill for HTLC with payment_preimage {}", log_bytes!(payment_preimage.0));
 		}
-		(payment_sent_ev, path_success_ev)
 	}
 
-	pub(super) fn finalize_claims(&self, mut sources: Vec<HTLCSource>) -> Vec<events::Event> {
+	pub(super) fn finalize_claims(&self, sources: Vec<HTLCSource>, pending_events: &Mutex<Vec<events::Event>>) {
 		let mut outbounds = self.pending_outbound_payments.lock().unwrap();
-		let mut success_events = Vec::new();
-		for source in sources.drain(..) {
+		let mut pending_events = pending_events.lock().unwrap();
+		for source in sources {
 			if let HTLCSource::OutboundRoute { session_priv, payment_id, path, .. } = source {
 				let mut session_priv_bytes = [0; 32];
 				session_priv_bytes.copy_from_slice(&session_priv[..]);
 				if let hash_map::Entry::Occupied(mut payment) = outbounds.entry(payment_id) {
 					assert!(payment.get().is_fulfilled());
 					if payment.get_mut().remove(&session_priv_bytes, None) {
-						success_events.push(
+						pending_events.push(
 							events::Event::PaymentPathSuccessful {
 								payment_id,
 								payment_hash: payment.get().payment_hash(),
@@ -626,7 +626,6 @@ impl OutboundPayments {
 				}
 			}
 		}
-		success_events
 	}
 
 	pub(super) fn remove_stale_resolved_payments(&self, pending_events: &Mutex<Vec<events::Event>>) {
@@ -672,8 +671,8 @@ impl OutboundPayments {
 		&self, source: &HTLCSource, payment_hash: &PaymentHash, onion_error: &HTLCFailReason,
 		path: &Vec<RouteHop>, session_priv: &SecretKey, payment_id: &PaymentId,
 		payment_params: &Option<PaymentParameters>, probing_cookie_secret: [u8; 32],
-		secp_ctx: &Secp256k1<secp256k1::All>, logger: &L)
-	-> (Option<events::Event>, Option<events::Event>) where L::Target: Logger {
+		secp_ctx: &Secp256k1<secp256k1::All>, pending_events: &Mutex<Vec<events::Event>>, logger: &L)
+	where L::Target: Logger {
 		let mut session_priv_bytes = [0; 32];
 		session_priv_bytes.copy_from_slice(&session_priv[..]);
 		let mut outbounds = self.pending_outbound_payments.lock().unwrap();
@@ -682,11 +681,11 @@ impl OutboundPayments {
 		if let hash_map::Entry::Occupied(mut payment) = outbounds.entry(*payment_id) {
 			if !payment.get_mut().remove(&session_priv_bytes, Some(&path)) {
 				log_trace!(logger, "Received duplicative fail for HTLC with payment_hash {}", log_bytes!(payment_hash.0));
-				return (None, None);
+				return
 			}
 			if payment.get().is_fulfilled() {
 				log_trace!(logger, "Received failure of HTLC with payment_hash {} after payment completion", log_bytes!(payment_hash.0));
-				return (None, None);
+				return
 			}
 			if payment.get().remaining_parts() == 0 {
 				all_paths_failed = true;
@@ -700,7 +699,7 @@ impl OutboundPayments {
 			}
 		} else {
 			log_trace!(logger, "Received duplicative fail for HTLC with payment_hash {}", log_bytes!(payment_hash.0));
-			return (None, None);
+			return
 		}
 		let mut retry = if let Some(payment_params_data) = payment_params {
 			let path_last_hop = path.last().expect("Outbound payments must have had a valid path");
@@ -756,7 +755,9 @@ impl OutboundPayments {
 				}
 			}
 		};
-		(Some(path_failure), full_failure_ev)
+		let mut pending_events = pending_events.lock().unwrap();
+		pending_events.push(path_failure);
+		if let Some(ev) = full_failure_ev { pending_events.push(ev); }
 	}
 
 	pub(super) fn abandon_payment(&self, payment_id: PaymentId) -> Option<events::Event> {