Store inbound committed update_adds in Channel

XXX

Author: valentinewallace

lightning/src/ln/channel.rs

@@ -211,7 +211,9 @@ enum InboundHTLCState {
 	/// channel (before it can then get forwarded and/or removed).
 	/// Implies AwaitingRemoteRevoke.
 	AwaitingAnnouncedRemoteRevoke(InboundHTLCResolution),
-	Committed,
+	Committed {
+		update_add_htlc_opt: Option<msgs::UpdateAddHTLC>,
+	},
 	/// Removed by us and a new commitment_signed was sent (if we were AwaitingRemoteRevoke when we
 	/// created it we would have put it in the holding cell instead). When they next revoke_and_ack
 	/// we'll drop it.
@@ -234,7 +236,7 @@ impl From<&InboundHTLCState> for Option<InboundHTLCStateDetails> {
 				Some(InboundHTLCStateDetails::AwaitingRemoteRevokeToAdd),
 			InboundHTLCState::AwaitingAnnouncedRemoteRevoke(_) =>
 				Some(InboundHTLCStateDetails::AwaitingRemoteRevokeToAdd),
-			InboundHTLCState::Committed =>
+			InboundHTLCState::Committed { .. } =>
 				Some(InboundHTLCStateDetails::Committed),
 			InboundHTLCState::LocalRemoved(InboundHTLCRemovalReason::FailRelay(_)) =>
 				Some(InboundHTLCStateDetails::AwaitingRemoteRevokeToRemoveFail),
@@ -253,7 +255,7 @@ impl fmt::Display for InboundHTLCState {
 			InboundHTLCState::RemoteAnnounced(_) => write!(f, "RemoteAnnounced"),
 			InboundHTLCState::AwaitingRemoteRevokeToAnnounce(_) => write!(f, "AwaitingRemoteRevokeToAnnounce"),
 			InboundHTLCState::AwaitingAnnouncedRemoteRevoke(_) => write!(f, "AwaitingAnnouncedRemoteRevoke"),
-			InboundHTLCState::Committed => write!(f, "Committed"),
+			InboundHTLCState::Committed { .. } => write!(f, "Committed"),
 			InboundHTLCState::LocalRemoved(_) => write!(f, "LocalRemoved"),
 		}
 	}
@@ -265,7 +267,7 @@ impl InboundHTLCState {
 			InboundHTLCState::RemoteAnnounced(_) => !generated_by_local,
 			InboundHTLCState::AwaitingRemoteRevokeToAnnounce(_) => !generated_by_local,
 			InboundHTLCState::AwaitingAnnouncedRemoteRevoke(_) => true,
-			InboundHTLCState::Committed => true,
+			InboundHTLCState::Committed { .. } => true,
 			InboundHTLCState::LocalRemoved(_) => !generated_by_local,
 		}
 	}
@@ -293,7 +295,7 @@ impl InboundHTLCState {
 				},
 				InboundHTLCResolution::Resolved { .. } => false,
 			},
-			InboundHTLCState::Committed | InboundHTLCState::LocalRemoved(_) => false,
+			InboundHTLCState::Committed { .. } | InboundHTLCState::LocalRemoved(_) => false,
 		}
 	}
 }
@@ -4066,7 +4068,7 @@ where
 
 		if self.pending_inbound_htlcs.iter()
 			.any(|htlc| match htlc.state {
-				InboundHTLCState::Committed => false,
+				InboundHTLCState::Committed { .. } => false,
 				// An HTLC removal from the local node is pending on the remote commitment.
 				InboundHTLCState::LocalRemoved(_) => true,
 				// An HTLC add from the remote node is pending on the local commitment.
@@ -4495,7 +4497,7 @@ where
 				(InboundHTLCState::RemoteAnnounced(..), _) => true,
 				(InboundHTLCState::AwaitingRemoteRevokeToAnnounce(..), _) => true,
 				(InboundHTLCState::AwaitingAnnouncedRemoteRevoke(..), _) => true,
-				(InboundHTLCState::Committed, _) => true,
+				(InboundHTLCState::Committed { .. }, _) => true,
 				(InboundHTLCState::LocalRemoved(..), true) => true,
 				(InboundHTLCState::LocalRemoved(..), false) => false,
 			})
@@ -7278,7 +7280,7 @@ where
 					payment_preimage_arg
 				);
 				match htlc.state {
-					InboundHTLCState::Committed => {},
+					InboundHTLCState::Committed { .. } => {},
 					InboundHTLCState::LocalRemoved(ref reason) => {
 						if let &InboundHTLCRemovalReason::Fulfill(_, _) = reason {
 						} else {
@@ -7371,7 +7373,7 @@ where
 
 		{
 			let htlc = &mut self.context.pending_inbound_htlcs[pending_idx];
-			if let InboundHTLCState::Committed = htlc.state {
+			if let InboundHTLCState::Committed { .. } = htlc.state {
 			} else {
 				debug_assert!(
 					false,
@@ -7506,7 +7508,7 @@ where
 		for (idx, htlc) in self.context.pending_inbound_htlcs.iter().enumerate() {
 			if htlc.htlc_id == htlc_id_arg {
 				match htlc.state {
-					InboundHTLCState::Committed => {},
+					InboundHTLCState::Committed { .. } => {},
 					InboundHTLCState::LocalRemoved(_) => {
 						return Err(ChannelError::Ignore(format!("HTLC {} was already resolved", htlc.htlc_id)));
 					},
@@ -8665,7 +8667,7 @@ where
 					false
 				};
 				if swap {
-					let mut state = InboundHTLCState::Committed;
+					let mut state = InboundHTLCState::Committed { update_add_htlc_opt: None };
 					mem::swap(&mut state, &mut htlc.state);
 
 					if let InboundHTLCState::AwaitingRemoteRevokeToAnnounce(resolution) = state {
@@ -8704,14 +8706,19 @@ where
 									PendingHTLCStatus::Forward(forward_info) => {
 										log_trace!(logger, " ...promoting inbound AwaitingAnnouncedRemoteRevoke {} to Committed, attempting to forward", &htlc.payment_hash);
 										to_forward_infos.push((forward_info, htlc.htlc_id));
-										htlc.state = InboundHTLCState::Committed;
+										// TODO: this is currently unreachable, so is it okay? will we lose a forward?
+										htlc.state = InboundHTLCState::Committed {
+											update_add_htlc_opt: None,
+										};
 									},
 								}
 							},
 							InboundHTLCResolution::Pending { update_add_htlc } => {
 								log_trace!(logger, " ...promoting inbound AwaitingAnnouncedRemoteRevoke {} to Committed", &htlc.payment_hash);
-								pending_update_adds.push(update_add_htlc);
-								htlc.state = InboundHTLCState::Committed;
+								pending_update_adds.push(update_add_htlc.clone());
+								htlc.state = InboundHTLCState::Committed {
+									update_add_htlc_opt: Some(update_add_htlc),
+								};
 							},
 						}
 					}
@@ -9241,7 +9248,7 @@ where
 					// in response to it yet, so don't touch it.
 					true
 				},
-				InboundHTLCState::Committed => true,
+				InboundHTLCState::Committed { .. } => true,
 				InboundHTLCState::LocalRemoved(_) => {
 					// We (hopefully) sent a commitment_signed updating this HTLC (which we can
 					// re-transmit if needed) and they may have even sent a revoke_and_ack back
@@ -14406,6 +14413,7 @@ where
 			}
 		}
 		let mut removed_htlc_attribution_data: Vec<&Option<AttributionData>> = Vec::new();
+		let mut inbound_committed_update_adds: Vec<Option<msgs::UpdateAddHTLC>> = Vec::new();
 		(self.context.pending_inbound_htlcs.len() as u64 - dropped_inbound_htlcs).write(writer)?;
 		for htlc in self.context.pending_inbound_htlcs.iter() {
 			if let &InboundHTLCState::RemoteAnnounced(_) = &htlc.state {
@@ -14425,8 +14433,9 @@ where
 					2u8.write(writer)?;
 					htlc_resolution.write(writer)?;
 				},
-				&InboundHTLCState::Committed => {
+				&InboundHTLCState::Committed { ref update_add_htlc_opt } => {
 					3u8.write(writer)?;
+					inbound_committed_update_adds.push(update_add_htlc_opt.clone());
 				},
 				&InboundHTLCState::LocalRemoved(ref removal_reason) => {
 					4u8.write(writer)?;
@@ -14793,6 +14802,7 @@ where
 			(65, self.quiescent_action, option), // Added in 0.2
 			(67, pending_outbound_held_htlc_flags, optional_vec), // Added in 0.2
 			(69, holding_cell_held_htlc_flags, optional_vec), // Added in 0.2
+			(71, inbound_committed_update_adds, optional_vec),
 		});
 
 		Ok(())
@@ -14876,7 +14886,7 @@ where
 						};
 						InboundHTLCState::AwaitingAnnouncedRemoteRevoke(resolution)
 					},
-					3 => InboundHTLCState::Committed,
+					3 => InboundHTLCState::Committed { update_add_htlc_opt: None },
 					4 => {
 						let reason = match <u8 as Readable>::read(reader)? {
 							0 => InboundHTLCRemovalReason::FailRelay(msgs::OnionErrorPacket {
@@ -15160,6 +15170,7 @@ where
 
 		let mut pending_outbound_held_htlc_flags_opt: Option<Vec<Option<()>>> = None;
 		let mut holding_cell_held_htlc_flags_opt: Option<Vec<Option<()>>> = None;
+		let mut inbound_committed_update_adds_opt: Option<Vec<Option<msgs::UpdateAddHTLC>>> = None;
 
 		read_tlv_fields!(reader, {
 			(0, announcement_sigs, option),
@@ -15207,6 +15218,7 @@ where
 			(65, quiescent_action, upgradable_option), // Added in 0.2
 			(67, pending_outbound_held_htlc_flags_opt, optional_vec), // Added in 0.2
 			(69, holding_cell_held_htlc_flags_opt, optional_vec), // Added in 0.2
+			(71, inbound_committed_update_adds_opt, optional_vec),
 		});
 
 		let holder_signer = signer_provider.derive_channel_signer(channel_keys_id);
@@ -15330,6 +15342,17 @@ where
 				return Err(DecodeError::InvalidValue);
 			}
 		}
+		if let Some(update_adds) = inbound_committed_update_adds_opt {
+			let mut iter = update_adds.into_iter();
+			for htlc in pending_inbound_htlcs.iter_mut() {
+				if let InboundHTLCState::Committed { ref mut update_add_htlc_opt } = htlc.state {
+					*update_add_htlc_opt = iter.next().ok_or(DecodeError::InvalidValue)?;
+				}
+			}
+			if iter.next().is_some() {
+				return Err(DecodeError::InvalidValue);
+			}
+		}
 
 		if let Some(attribution_data_list) = removed_htlc_attribution_data {
 			let mut removed_htlcs = pending_inbound_htlcs.iter_mut().filter_map(|status| {
@@ -15883,7 +15906,7 @@ mod tests {
 			amount_msat: htlc_amount_msat,
 			payment_hash: PaymentHash(Sha256::hash(&[42; 32]).to_byte_array()),
 			cltv_expiry: 300000000,
-			state: InboundHTLCState::Committed,
+			state: InboundHTLCState::Committed { update_add_htlc_opt: None },
 		});
 
 		node_a_chan.context.pending_outbound_htlcs.push(OutboundHTLCOutput {
@@ -17884,11 +17907,11 @@ mod tests {
 			assert_eq!(
 				check_v2_funding_inputs_sufficient(
 					220_000,
-					&[
-						funding_input_sats(200_000),
-						funding_input_sats(100_000),
-					],
-					true,
+				&[
+					funding_input_sats(200_000),
+					funding_input_sats(100_000),
+				],
+				true,
 					true,
 					2000,
 				).unwrap(),
@@ -17904,7 +17927,7 @@ mod tests {
 					220_000,
 					&[
 						funding_input_sats(100_000),
-					],
+				],
 					true,
 					true,
 					2000,
@@ -17942,8 +17965,8 @@ mod tests {
 					298032,
 					&[
 						funding_input_sats(200_000),
-						funding_input_sats(100_000),
-					],
+					funding_input_sats(100_000),
+				],
 					true,
 					true,
 					2200,