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,
 		}
 	}
 }
@@ -4091,7 +4093,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.
@@ -4520,7 +4522,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,
 			})
@@ -7303,7 +7305,7 @@ where
 					payment_preimage_arg
 				);
 				match htlc.state {
-					InboundHTLCState::Committed => {},
+					InboundHTLCState::Committed { .. } => {},
 					InboundHTLCState::LocalRemoved(ref reason) => {
 						if let &InboundHTLCRemovalReason::Fulfill(_, _) = reason {
 						} else {
@@ -7396,7 +7398,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,
@@ -7531,7 +7533,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)));
 					},
@@ -8690,7 +8692,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 {
@@ -8729,14 +8731,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),
+								};
 							},
 						}
 					}
@@ -9266,7 +9273,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
@@ -14467,6 +14474,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 {
@@ -14486,8 +14494,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)?;
@@ -14860,6 +14869,7 @@ where
 			(69, holding_cell_held_htlc_flags, optional_vec), // Added in 0.2
 			(71, holder_commitment_point_previous_revoked, option), // Added in 0.3
 			(73, holder_commitment_point_last_revoked, option), // Added in 0.3
+			(75, inbound_committed_update_adds, optional_vec),
 		});
 
 		Ok(())
@@ -14943,7 +14953,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 {
@@ -15229,6 +15239,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),
@@ -15278,6 +15289,7 @@ where
 			(69, holding_cell_held_htlc_flags_opt, optional_vec), // Added in 0.2
 			(71, holder_commitment_point_previous_revoked_opt, option), // Added in 0.3
 			(73, holder_commitment_point_last_revoked_opt, option), // Added in 0.3
+			(75, inbound_committed_update_adds_opt, optional_vec),
 		});
 
 		let holder_signer = signer_provider.derive_channel_signer(channel_keys_id);
@@ -15401,6 +15413,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| {
@@ -15985,7 +16008,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 {
@@ -16831,7 +16854,7 @@ mod tests {
 			amount_msat: 1000000,
 			cltv_expiry: 500,
 			payment_hash: PaymentHash::from(payment_preimage_0),
-			state: InboundHTLCState::Committed,
+			state: InboundHTLCState::Committed { update_add_htlc_opt: None },
 		});
 
 		let payment_preimage_1 =
@@ -16841,7 +16864,7 @@ mod tests {
 			amount_msat: 2000000,
 			cltv_expiry: 501,
 			payment_hash: PaymentHash::from(payment_preimage_1),
-			state: InboundHTLCState::Committed,
+			state: InboundHTLCState::Committed { update_add_htlc_opt: None },
 		});
 
 		let payment_preimage_2 =
@@ -16881,7 +16904,7 @@ mod tests {
 			amount_msat: 4000000,
 			cltv_expiry: 504,
 			payment_hash: PaymentHash::from(payment_preimage_4),
-			state: InboundHTLCState::Committed,
+			state: InboundHTLCState::Committed { update_add_htlc_opt: None },
 		});
 
 		// commitment tx with all five HTLCs untrimmed (minimum feerate)
@@ -17270,7 +17293,7 @@ mod tests {
 			amount_msat: 2000000,
 			cltv_expiry: 501,
 			payment_hash: PaymentHash::from(payment_preimage_1),
-			state: InboundHTLCState::Committed,
+			state: InboundHTLCState::Committed { update_add_htlc_opt: None },
 		});
 
 		chan.context.pending_outbound_htlcs.clear();
@@ -17521,7 +17544,7 @@ mod tests {
 			amount_msat: 5000000,
 			cltv_expiry: 920150,
 			payment_hash: PaymentHash::from(htlc_in_preimage),
-			state: InboundHTLCState::Committed,
+			state: InboundHTLCState::Committed { update_add_htlc_opt: None },
 		}));
 
 		chan.context.pending_outbound_htlcs.extend(
@@ -17595,7 +17618,7 @@ mod tests {
 			amount_msat: 100000,
 			cltv_expiry: 920125,
 			payment_hash: htlc_0_in_hash,
-			state: InboundHTLCState::Committed,
+			state: InboundHTLCState::Committed { update_add_htlc_opt: None },
 		});
 
 		let htlc_1_in_preimage =
@@ -17613,7 +17636,7 @@ mod tests {
 			amount_msat: 49900000,
 			cltv_expiry: 920125,
 			payment_hash: htlc_1_in_hash,
-			state: InboundHTLCState::Committed,
+			state: InboundHTLCState::Committed { update_add_htlc_opt: None },
 		});
 
 		chan.context.pending_outbound_htlcs.extend(
@@ -17721,7 +17744,7 @@ mod tests {
 				amount_msat,
 				cltv_expiry,
 				payment_hash,
-				state: InboundHTLCState::Committed,
+				state: InboundHTLCState::Committed { update_add_htlc_opt: None },
 			}),
 		);