f - Add payment hash to PaymentPathSuccessful

Author: jkczyz

lightning/src/ln/channelmanager.rs

@@ -449,6 +449,7 @@ pub(crate) enum PendingOutboundPayment {
 	/// and add a pending payment that was already fulfilled.
 	Fulfilled {
 		session_privs: HashSet<[u8; 32]>,
+		payment_hash: Option<PaymentHash>,
 	},
 }
 
@@ -472,23 +473,32 @@ impl PendingOutboundPayment {
 		}
 	}
 
+	fn payment_hash(&self) -> Option<PaymentHash> {
+		match self {
+			PendingOutboundPayment::Legacy { .. } => None,
+			PendingOutboundPayment::Retryable { payment_hash, .. } => Some(*payment_hash),
+			PendingOutboundPayment::Fulfilled { payment_hash, .. } => *payment_hash,
+		}
+	}
+
 	fn mark_fulfilled(&mut self) {
 		let mut session_privs = HashSet::new();
 		core::mem::swap(&mut session_privs, match self {
 			PendingOutboundPayment::Legacy { session_privs } |
 			PendingOutboundPayment::Retryable { session_privs, .. } |
-			PendingOutboundPayment::Fulfilled { session_privs }
+			PendingOutboundPayment::Fulfilled { session_privs, .. }
 				=> session_privs
 		});
-		*self = PendingOutboundPayment::Fulfilled { session_privs };
+		let payment_hash = self.payment_hash();
+		*self = PendingOutboundPayment::Fulfilled { session_privs, payment_hash };
 	}
 
 	/// panics if path is None and !self.is_fulfilled
 	fn remove(&mut self, session_priv: &[u8; 32], path: Option<&Vec<RouteHop>>) -> bool {
 		let remove_res = match self {
 			PendingOutboundPayment::Legacy { session_privs } |
 			PendingOutboundPayment::Retryable { session_privs, .. } |
-			PendingOutboundPayment::Fulfilled { session_privs } => {
+			PendingOutboundPayment::Fulfilled { session_privs, .. } => {
 				session_privs.remove(session_priv)
 			}
 		};
@@ -529,7 +539,7 @@ impl PendingOutboundPayment {
 		match self {
 			PendingOutboundPayment::Legacy { session_privs } |
 			PendingOutboundPayment::Retryable { session_privs, .. } |
-			PendingOutboundPayment::Fulfilled { session_privs } => {
+			PendingOutboundPayment::Fulfilled { session_privs, .. } => {
 				session_privs.len()
 			}
 		}
@@ -3503,6 +3513,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 						pending_events.push(
 							events::Event::PaymentPathSuccessful {
 								payment_id,
+								payment_hash: payment.get().payment_hash(),
 								path,
 							}
 						);
@@ -3546,9 +3557,11 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 						// TODO: We should have a second monitor event that informs us of payments
 						// irrevocably fulfilled.
 						if payment.get_mut().remove(&session_priv_bytes, Some(&path)) {
+							let payment_hash = Some(PaymentHash(Sha256::hash(&payment_preimage.0).into_inner()));
 							pending_events.push(
 								events::Event::PaymentPathSuccessful {
 									payment_id,
+									payment_hash,
 									path,
 								}
 							);
@@ -5576,6 +5589,7 @@ impl_writeable_tlv_based_enum_upgradable!(PendingOutboundPayment,
 	},
 	(1, Fulfilled) => {
 		(0, session_privs, required),
+		(1, payment_hash, option),
 	},
 	(2, Retryable) => {
 		(0, session_privs, required),
@@ -6355,15 +6369,17 @@ mod tests {
 			_ => panic!("Unexpected event"),
 		}
 		match events[1] {
-			Event::PaymentPathSuccessful { payment_id: ref actual_payment_id, ref path } => {
+			Event::PaymentPathSuccessful { payment_id: ref actual_payment_id, ref payment_hash, ref path } => {
 				assert_eq!(payment_id, *actual_payment_id);
+				assert_eq!(our_payment_hash, *payment_hash.as_ref().unwrap());
 				assert_eq!(route.paths[0], *path);
 			},
 			_ => panic!("Unexpected event"),
 		}
 		match events[2] {
-			Event::PaymentPathSuccessful { payment_id: ref actual_payment_id, ref path } => {
+			Event::PaymentPathSuccessful { payment_id: ref actual_payment_id, ref payment_hash, ref path } => {
 				assert_eq!(payment_id, *actual_payment_id);
+				assert_eq!(our_payment_hash, *payment_hash.as_ref().unwrap());
 				assert_eq!(route.paths[0], *path);
 			},
 			_ => panic!("Unexpected event"),

lightning/src/ln/functional_test_utils.rs

@@ -1104,21 +1104,25 @@ macro_rules! expect_payment_sent {
 		} else {
 			assert_eq!(events.len(), 1);
 		}
-		match events[0] {
-			Event::PaymentSent { payment_id: _, ref payment_preimage, ref payment_hash, ref fee_paid_msat } => {
+		let expected_payment_id = match events[0] {
+			Event::PaymentSent { ref payment_id, ref payment_preimage, ref payment_hash, ref fee_paid_msat } => {
 				assert_eq!($expected_payment_preimage, *payment_preimage);
 				assert_eq!(expected_payment_hash, *payment_hash);
 				assert!(fee_paid_msat.is_some());
 				if $expected_fee_msat_opt.is_some() {
 					assert_eq!(*fee_paid_msat, $expected_fee_msat_opt);
 				}
+				payment_id.unwrap()
 			},
 			_ => panic!("Unexpected event"),
-		}
+		};
 		if $expect_paths {
 			for i in 1..events.len() {
 				match events[i] {
-					Event::PaymentPathSuccessful { .. } => {},
+					Event::PaymentPathSuccessful { payment_id, payment_hash, .. } => {
+						assert_eq!(payment_id, expected_payment_id);
+						assert_eq!(payment_hash, Some(expected_payment_hash));
+					},
 					_ => panic!("Unexpected event"),
 				}
 			}

lightning/src/util/events.rs

@@ -216,7 +216,7 @@ pub enum Event {
 		/// Note that this serves as a payment receipt, if you wish to have such a thing, you must
 		/// store it somehow!
 		payment_preimage: PaymentPreimage,
-		/// The hash which was given to [`ChannelManager::send_payment`].
+		/// The hash that was given to [`ChannelManager::send_payment`].
 		///
 		/// [`ChannelManager::send_payment`]: crate::ln::channelmanager::ChannelManager::send_payment
 		payment_hash: PaymentHash,
@@ -240,7 +240,9 @@ pub enum Event {
 		/// [`ChannelManager::send_payment`]: crate::ln::channelmanager::ChannelManager::send_payment
 		/// [`ChannelManager::retry_payment`]: crate::ln::channelmanager::ChannelManager::retry_payment
 		payment_id: Option<PaymentId>,
-		/// The hash which was given to ChannelManager::send_payment.
+		/// The hash that was given to [`ChannelManager::send_payment`].
+		///
+		/// [`ChannelManager::send_payment`]: crate::ln::channelmanager::ChannelManager::send_payment
 		payment_hash: PaymentHash,
 		/// Indicates the payment was rejected for some reason by the recipient. This implies that
 		/// the payment has failed, not just the route in question. If this is not set, you may
@@ -349,6 +351,10 @@ pub enum Event {
 		/// [`ChannelManager::send_payment`]: crate::ln::channelmanager::ChannelManager::send_payment
 		/// [`ChannelManager::retry_payment`]: crate::ln::channelmanager::ChannelManager::retry_payment
 		payment_id: PaymentId,
+		/// The hash that was given to [`ChannelManager::send_payment`].
+		///
+		/// [`ChannelManager::send_payment`]: crate::ln::channelmanager::ChannelManager::send_payment
+		payment_hash: Option<PaymentHash>,
 		/// The payment path that was successful.
 		path: Vec<RouteHop>,
 	},
@@ -451,11 +457,12 @@ impl Writeable for Event {
 					(2, transaction, required)
 				})
 			},
-			&Event::PaymentPathSuccessful { ref payment_id, ref path } => {
+			&Event::PaymentPathSuccessful { ref payment_id, ref payment_hash, ref path } => {
 				13u8.write(writer)?;
 				write_tlv_fields!(writer, {
 					(0, payment_id, required),
-					(2, VecWriteWrapper(path), required)
+					(2, payment_hash, option),
+					(4, VecWriteWrapper(path), required)
 				})
 			},
 			// Note that, going forward, all new events must only write data inside of
@@ -623,12 +630,14 @@ impl MaybeReadable for Event {
 			13u8 => {
 				let f = || {
 					let mut payment_id = PaymentId([0; 32]);
+					let mut payment_hash = None;
 					let mut path = VecReadWrapper(Vec::new());
 					read_tlv_fields!(reader, {
 						(0, payment_id, required),
-						(2, path, required),
+						(2, payment_hash, option),
+						(4, path, required),
 					});
-					Ok(Some(Event::PaymentPathSuccessful { payment_id, path: path.0 } ))
+					Ok(Some(Event::PaymentPathSuccessful { payment_id, payment_hash, path: path.0 } ))
 				};
 				f()
 			},