Add AttributionData to OutboundHTLCOutcome::Success

AttributionData is needed as part of the outbound HTLC outcome when
revoke_and_ack has happened and the AttributionData is decoded to get
the hold times for inclusion in the PaymentPathSuccessful event.

Author: joostjager

lightning/src/ln/channel.rs

@@ -348,11 +348,11 @@ impl From<&OutboundHTLCState> for OutboundHTLCStateDetails {
 			// the state yet.
 			OutboundHTLCState::RemoteRemoved(_) =>
 				OutboundHTLCStateDetails::Committed,
-			OutboundHTLCState::AwaitingRemoteRevokeToRemove(OutboundHTLCOutcome::Success(_)) =>
+			OutboundHTLCState::AwaitingRemoteRevokeToRemove(OutboundHTLCOutcome::Success(_, _)) =>
 				OutboundHTLCStateDetails::AwaitingRemoteRevokeToRemoveSuccess,
 			OutboundHTLCState::AwaitingRemoteRevokeToRemove(OutboundHTLCOutcome::Failure(_)) =>
 				OutboundHTLCStateDetails::AwaitingRemoteRevokeToRemoveFailure,
-			OutboundHTLCState::AwaitingRemovedRemoteRevoke(OutboundHTLCOutcome::Success(_)) =>
+			OutboundHTLCState::AwaitingRemovedRemoteRevoke(OutboundHTLCOutcome::Success(_, _)) =>
 				OutboundHTLCStateDetails::AwaitingRemoteRevokeToRemoveSuccess,
 			OutboundHTLCState::AwaitingRemovedRemoteRevoke(OutboundHTLCOutcome::Failure(_)) =>
 				OutboundHTLCStateDetails::AwaitingRemoteRevokeToRemoveFailure,
@@ -387,9 +387,9 @@ impl OutboundHTLCState {
 	#[rustfmt::skip]
 	fn preimage(&self) -> Option<PaymentPreimage> {
 		match self {
-			OutboundHTLCState::RemoteRemoved(OutboundHTLCOutcome::Success(preimage))
-			| OutboundHTLCState::AwaitingRemoteRevokeToRemove(OutboundHTLCOutcome::Success(preimage))
-			| OutboundHTLCState::AwaitingRemovedRemoteRevoke(OutboundHTLCOutcome::Success(preimage)) => {
+			OutboundHTLCState::RemoteRemoved(OutboundHTLCOutcome::Success(preimage, _))
+			| OutboundHTLCState::AwaitingRemoteRevokeToRemove(OutboundHTLCOutcome::Success(preimage, _))
+			| OutboundHTLCState::AwaitingRemovedRemoteRevoke(OutboundHTLCOutcome::Success(preimage, _)) => {
 				Some(*preimage)
 			},
 			_ => None,
@@ -402,14 +402,14 @@ impl OutboundHTLCState {
 enum OutboundHTLCOutcome {
 	/// We started always filling in the preimages here in 0.0.105, and the requirement
 	/// that the preimages always be filled in was added in 0.2.
-	Success(PaymentPreimage),
+	Success(PaymentPreimage, #[allow(dead_code)] Option<AttributionData>),
 	Failure(HTLCFailReason),
 }
 
 impl<'a> Into<Option<&'a HTLCFailReason>> for &'a OutboundHTLCOutcome {
 	fn into(self) -> Option<&'a HTLCFailReason> {
 		match self {
-			OutboundHTLCOutcome::Success(_) => None,
+			OutboundHTLCOutcome::Success(_, _) => None,
 			OutboundHTLCOutcome::Failure(ref r) => Some(r),
 		}
 	}
@@ -4159,9 +4159,9 @@ where
 		// transaction).
 		let mut removed_outbound_total_msat = 0;
 		for ref htlc in self.pending_outbound_htlcs.iter() {
-			if let OutboundHTLCState::AwaitingRemoteRevokeToRemove(OutboundHTLCOutcome::Success(_)) = htlc.state {
+			if let OutboundHTLCState::AwaitingRemoteRevokeToRemove(OutboundHTLCOutcome::Success(_, _)) = htlc.state {
 				removed_outbound_total_msat += htlc.amount_msat;
-			} else if let OutboundHTLCState::AwaitingRemovedRemoteRevoke(OutboundHTLCOutcome::Success(_)) = htlc.state {
+			} else if let OutboundHTLCState::AwaitingRemovedRemoteRevoke(OutboundHTLCOutcome::Success(_, _)) = htlc.state {
 				removed_outbound_total_msat += htlc.amount_msat;
 			}
 		}
@@ -4424,9 +4424,9 @@ where
 
 		let mut removed_outbound_total_msat = 0;
 		for ref htlc in self.pending_outbound_htlcs.iter() {
-			if let OutboundHTLCState::AwaitingRemoteRevokeToRemove(OutboundHTLCOutcome::Success(_)) = htlc.state {
+			if let OutboundHTLCState::AwaitingRemoteRevokeToRemove(OutboundHTLCOutcome::Success(_, _)) = htlc.state {
 				removed_outbound_total_msat += htlc.amount_msat;
-			} else if let OutboundHTLCState::AwaitingRemovedRemoteRevoke(OutboundHTLCOutcome::Success(_)) = htlc.state {
+			} else if let OutboundHTLCState::AwaitingRemovedRemoteRevoke(OutboundHTLCOutcome::Success(_, _)) = htlc.state {
 				removed_outbound_total_msat += htlc.amount_msat;
 			}
 		}
@@ -6694,7 +6694,7 @@ where
 	fn mark_outbound_htlc_removed(&mut self, htlc_id: u64, outcome: OutboundHTLCOutcome) -> Result<&OutboundHTLCOutput, ChannelError> {
 		for htlc in self.context.pending_outbound_htlcs.iter_mut() {
 			if htlc.htlc_id == htlc_id {
-				if let OutboundHTLCOutcome::Success(ref payment_preimage) = outcome {
+				if let OutboundHTLCOutcome::Success(ref payment_preimage, ..) = outcome {
 					let payment_hash = PaymentHash(Sha256::hash(&payment_preimage.0[..]).to_byte_array());
 					if payment_hash != htlc.payment_hash {
 						return Err(ChannelError::close(format!("Remote tried to fulfill HTLC ({}) with an incorrect preimage", htlc_id)));
@@ -6738,7 +6738,7 @@ where
 
 		self.mark_outbound_htlc_removed(
 			msg.htlc_id,
-			OutboundHTLCOutcome::Success(msg.payment_preimage),
+			OutboundHTLCOutcome::Success(msg.payment_preimage, None),
 		)
 		.map(|htlc| (htlc.source.clone(), htlc.amount_msat, htlc.skimmed_fee_msat))
 	}
@@ -7078,9 +7078,9 @@ where
 				log_trace!(logger, "Updating HTLC {} to AwaitingRemoteRevokeToRemove due to commitment_signed in channel {}.",
 					&htlc.payment_hash, &self.context.channel_id);
 				// Swap against a dummy variant to avoid a potentially expensive clone of `OutboundHTLCOutcome::Failure(HTLCFailReason)`
-				let mut reason = OutboundHTLCOutcome::Success(PaymentPreimage([0u8; 32]));
+				let mut reason = OutboundHTLCOutcome::Success(PaymentPreimage([0u8; 32]), None);
 				mem::swap(outcome, &mut reason);
-				if let OutboundHTLCOutcome::Success(preimage) = reason {
+				if let OutboundHTLCOutcome::Success(preimage, _) = reason {
 					// If a user (a) receives an HTLC claim using LDK 0.0.104 or before, then (b)
 					// upgrades to LDK 0.0.114 or later before the HTLC is fully resolved, we could
 					// have a `Success(None)` reason. In this case we could forget some HTLC
@@ -7525,19 +7525,22 @@ where
 						&htlc.payment_hash
 					);
 					// We really want take() here, but, again, non-mut ref :(
-					if let OutboundHTLCOutcome::Failure(mut reason) = outcome.clone() {
-						if let (Some(timestamp), Some(now)) = (htlc.send_timestamp, now) {
-							let elapsed_millis = now.saturating_sub(timestamp).as_millis();
-							let elapsed_units = elapsed_millis / HOLD_TIME_UNIT_MILLIS;
-							let hold_time = u32::try_from(elapsed_units).unwrap_or(u32::MAX);
-							reason.set_hold_time(hold_time);
-						}
+					match outcome.clone() {
+						OutboundHTLCOutcome::Failure(mut reason) => {
+							if let (Some(timestamp), Some(now)) = (htlc.send_timestamp, now) {
+								let elapsed_millis = now.saturating_sub(timestamp).as_millis();
+								let elapsed_units = elapsed_millis / HOLD_TIME_UNIT_MILLIS;
+								let hold_time = u32::try_from(elapsed_units).unwrap_or(u32::MAX);
+								reason.set_hold_time(hold_time);
+							}
 
-						revoked_htlcs.push((htlc.source.clone(), htlc.payment_hash, reason));
-					} else {
-						finalized_claimed_htlcs.push(htlc.source.clone());
-						// They fulfilled, so we sent them money
-						value_to_self_msat_diff -= htlc.amount_msat as i64;
+							revoked_htlcs.push((htlc.source.clone(), htlc.payment_hash, reason));
+						},
+						OutboundHTLCOutcome::Success(..) => {
+							finalized_claimed_htlcs.push(htlc.source.clone());
+							// They fulfilled, so we sent them money
+							value_to_self_msat_diff -= htlc.amount_msat as i64;
+						},
 					}
 					false
 				} else {
@@ -7621,7 +7624,7 @@ where
 				{
 					log_trace!(logger, " ...promoting outbound AwaitingRemoteRevokeToRemove {} to AwaitingRemovedRemoteRevoke", &htlc.payment_hash);
 					// Swap against a dummy variant to avoid a potentially expensive clone of `OutboundHTLCOutcome::Failure(HTLCFailReason)`
-					let mut reason = OutboundHTLCOutcome::Success(PaymentPreimage([0u8; 32]));
+					let mut reason = OutboundHTLCOutcome::Success(PaymentPreimage([0u8; 32]), None);
 					mem::swap(outcome, &mut reason);
 					htlc.state = OutboundHTLCState::AwaitingRemovedRemoteRevoke(reason);
 					require_commitment = true;
@@ -10751,7 +10754,7 @@ where
 			if let &mut OutboundHTLCState::AwaitingRemoteRevokeToRemove(ref mut outcome) = &mut htlc.state {
 				log_trace!(logger, " ...promoting outbound AwaitingRemoteRevokeToRemove {} to AwaitingRemovedRemoteRevoke", &htlc.payment_hash);
 				// Swap against a dummy variant to avoid a potentially expensive clone of `OutboundHTLCOutcome::Failure(HTLCFailReason)`
-				let mut reason = OutboundHTLCOutcome::Success(PaymentPreimage([0u8; 32]));
+				let mut reason = OutboundHTLCOutcome::Success(PaymentPreimage([0u8; 32]), None);
 				mem::swap(outcome, &mut reason);
 				htlc.state = OutboundHTLCState::AwaitingRemovedRemoteRevoke(reason);
 			}
@@ -12532,15 +12535,15 @@ where
 				},
 				&OutboundHTLCState::AwaitingRemoteRevokeToRemove(ref outcome) => {
 					3u8.write(writer)?;
-					if let OutboundHTLCOutcome::Success(preimage) = outcome {
+					if let OutboundHTLCOutcome::Success(preimage, _) = outcome {
 						preimages.push(Some(preimage));
 					}
 					let reason: Option<&HTLCFailReason> = outcome.into();
 					reason.write(writer)?;
 				},
 				&OutboundHTLCState::AwaitingRemovedRemoteRevoke(ref outcome) => {
 					4u8.write(writer)?;
-					if let OutboundHTLCOutcome::Success(preimage) = outcome {
+					if let OutboundHTLCOutcome::Success(preimage, _) = outcome {
 						preimages.push(Some(preimage));
 					}
 					let reason: Option<&HTLCFailReason> = outcome.into();
@@ -12940,7 +12943,7 @@ where
 						let outcome = match option {
 							Some(r) => OutboundHTLCOutcome::Failure(r),
 							// Initialize this variant with a dummy preimage, the actual preimage will be filled in further down
-							None => OutboundHTLCOutcome::Success(PaymentPreimage([0u8; 32])),
+							None => OutboundHTLCOutcome::Success(PaymentPreimage([0u8; 32]), None),
 						};
 						OutboundHTLCState::RemoteRemoved(outcome)
 					},
@@ -12949,7 +12952,7 @@ where
 						let outcome = match option {
 							Some(r) => OutboundHTLCOutcome::Failure(r),
 							// Initialize this variant with a dummy preimage, the actual preimage will be filled in further down
-							None => OutboundHTLCOutcome::Success(PaymentPreimage([0u8; 32])),
+							None => OutboundHTLCOutcome::Success(PaymentPreimage([0u8; 32]), None),
 						};
 						OutboundHTLCState::AwaitingRemoteRevokeToRemove(outcome)
 					},
@@ -12958,7 +12961,7 @@ where
 						let outcome = match option {
 							Some(r) => OutboundHTLCOutcome::Failure(r),
 							// Initialize this variant with a dummy preimage, the actual preimage will be filled in further down
-							None => OutboundHTLCOutcome::Success(PaymentPreimage([0u8; 32])),
+							None => OutboundHTLCOutcome::Success(PaymentPreimage([0u8; 32]), None),
 						};
 						OutboundHTLCState::AwaitingRemovedRemoteRevoke(outcome)
 					},
@@ -13227,6 +13230,7 @@ where
 			match &mut htlc.state {
 				OutboundHTLCState::AwaitingRemoteRevokeToRemove(OutboundHTLCOutcome::Success(
 					ref mut preimage,
+					..,
 				)) => {
 					// This variant was initialized like this further above
 					debug_assert_eq!(preimage, &PaymentPreimage([0u8; 32]));
@@ -13235,6 +13239,7 @@ where
 				},
 				OutboundHTLCState::AwaitingRemovedRemoteRevoke(OutboundHTLCOutcome::Success(
 					ref mut preimage,
+					..,
 				)) => {
 					// This variant was initialized like this further above
 					debug_assert_eq!(preimage, &PaymentPreimage([0u8; 32]));