f un-option chan creation height and fix comments and sp

Author: TheBlueMatt

lightning/src/ln/channel.rs

@@ -457,9 +457,10 @@ pub(super) struct Channel<Signer: Sign> {
 	funding_tx_confirmed_in: Option<BlockHash>,
 	funding_tx_confirmation_height: u32,
 	short_channel_id: Option<u64>,
-	/// If this channel was created in recent software, this will be set to the height at which
-	/// channel creation was intiated. This is used to close if funding is never broadcasted.
-	channel_creation_height: Option<u32>,
+	/// Either the height at which this channel was created or the height at which it was last
+	/// serialized by older software.
+	/// We use this to close if funding is never broadcasted.
+	channel_creation_height: u32,
 
 	counterparty_dust_limit_satoshis: u64,
 	#[cfg(test)]
@@ -715,7 +716,7 @@ impl<Signer: Sign> Channel<Signer> {
 			funding_tx_confirmed_in: None,
 			funding_tx_confirmation_height: 0,
 			short_channel_id: None,
-			channel_creation_height: Some(current_chain_heigt),
+			channel_creation_height: current_chain_heigt,
 
 			feerate_per_kw: feerate,
 			counterparty_dust_limit_satoshis: 0,
@@ -982,7 +983,7 @@ impl<Signer: Sign> Channel<Signer> {
 			funding_tx_confirmed_in: None,
 			funding_tx_confirmation_height: 0,
 			short_channel_id: None,
-			channel_creation_height: Some(current_chain_heigt),
+			channel_creation_height: current_chain_heigt,
 
 			feerate_per_kw: msg.feerate_per_kw,
 			channel_value_satoshis: msg.funding_satoshis,
@@ -4148,18 +4149,16 @@ impl<Signer: Sign> Channel<Signer> {
 					data: format!("Funding transaction was un-confirmed. Locked at {} confs, now have {} confs.", self.minimum_depth.unwrap(), funding_tx_confirmations),
 				});
 			}
-		} else if let Some(creation_height) = self.channel_creation_height {
-			if !self.is_outbound() && self.funding_tx_confirmed_in.is_none() &&
-			   creation_height + FUNDING_CONF_DEADLINE_BLOCKS <= height {
-				log_info!(logger, "Closing channel {} due to funding timeout", log_bytes!(self.channel_id));
-				// If funding_tx_confirmed_in is unset, the channel must not be active
-				assert!(non_shutdown_state <= ChannelState::ChannelFunded as u32);
-				assert_eq!(non_shutdown_state & ChannelState::OurFundingLocked as u32, 0);
-				return Err(msgs::ErrorMessage {
-					channel_id: self.channel_id(),
-					data: format!("Funding transaction failed to confirm in {} blocks", FUNDING_CONF_DEADLINE_BLOCKS),
-				});
-			}
+		} else if !self.is_outbound() && self.funding_tx_confirmed_in.is_none() &&
+				self.channel_creation_height + FUNDING_CONF_DEADLINE_BLOCKS <= height {
+			log_info!(logger, "Closing channel {} due to funding timeout", log_bytes!(self.channel_id));
+			// If funding_tx_confirmed_in is unset, the channel must not be active
+			assert!(non_shutdown_state <= ChannelState::ChannelFunded as u32);
+			assert_eq!(non_shutdown_state & ChannelState::OurFundingLocked as u32, 0);
+			return Err(msgs::ErrorMessage {
+				channel_id: self.channel_id(),
+				data: format!("Funding transaction failed to confirm within {} blocks", FUNDING_CONF_DEADLINE_BLOCKS),
+			});
 		}
 
 		Ok((None, timed_out_htlcs))
@@ -5188,17 +5187,18 @@ impl<Signer: Sign> Writeable for Channel<Signer> {
 			(5, self.config, required),
 			(7, self.shutdown_scriptpubkey, option),
 			(9, self.target_closing_feerate_sats_per_kw, option),
-			(11, self.channel_creation_height, option),
+			(11, self.channel_creation_height, required),
 		});
 
 		Ok(())
 	}
 }
 
 const MAX_ALLOC_SIZE: usize = 64*1024;
-impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel<Signer>
+impl<'a, Signer: Sign, K: Deref> ReadableArgs<(&'a K, u32)> for Channel<Signer>
 		where K::Target: KeysInterface<Signer = Signer> {
-	fn read<R : io::Read>(reader: &mut R, keys_source: &'a K) -> Result<Self, DecodeError> {
+	fn read<R : io::Read>(reader: &mut R, args: (&'a K, u32)) -> Result<Self, DecodeError> {
+		let (keys_source, serialized_height) = args;
 		let ver = read_ver_prefix!(reader, SERIALIZATION_VERSION);
 
 		let user_id = Readable::read(reader)?;
@@ -5422,7 +5422,7 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel<Signer>
 
 		let mut announcement_sigs = None;
 		let mut target_closing_feerate_sats_per_kw = None;
-		let mut channel_creation_height = None;
+		let mut channel_creation_height = Some(serialized_height);
 		read_tlv_fields!(reader, {
 			(0, announcement_sigs, option),
 			(1, minimum_depth, option),
@@ -5487,7 +5487,7 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel<Signer>
 			funding_tx_confirmed_in,
 			funding_tx_confirmation_height,
 			short_channel_id,
-			channel_creation_height,
+			channel_creation_height: channel_creation_height.unwrap(),
 
 			counterparty_dust_limit_satoshis,
 			holder_dust_limit_satoshis,

lightning/src/ln/channelmanager.rs

@@ -5357,7 +5357,7 @@ impl<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
 		let mut by_id = HashMap::with_capacity(cmp::min(channel_count as usize, 128));
 		let mut short_to_id = HashMap::with_capacity(cmp::min(channel_count as usize, 128));
 		for _ in 0..channel_count {
-			let mut channel: Channel<Signer> = Channel::read(reader, &args.keys_manager)?;
+			let mut channel: Channel<Signer> = Channel::read(reader, (&args.keys_manager, best_block_height))?;
 			let funding_txo = channel.get_funding_txo().ok_or(DecodeError::InvalidValue)?;
 			funding_txo_set.insert(funding_txo.clone());
 			if let Some(ref mut monitor) = args.channel_monitors.get_mut(&funding_txo) {

lightning/src/ln/functional_tests.rs

@@ -8029,6 +8029,8 @@ fn test_channel_conf_timeout() {
 	let _funding_tx = create_chan_between_nodes_with_value_init(&nodes[0], &nodes[1], 1_000_000, 100_000, InitFeatures::known(), InitFeatures::known());
 
 	// The outbound node should wait forever for confirmation:
+	// This matches `channel::FUNDING_CONF_DEADLINE_BLOCKS` and BOLT 2's suggested timeout, thus is
+	// copied here instead of directly referencing the constant.
 	connect_blocks(&nodes[0], 2016);
 	assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
 
@@ -8044,7 +8046,7 @@ fn test_channel_conf_timeout() {
 	match close_ev[0] {
 		MessageSendEvent::HandleError { action: ErrorAction::SendErrorMessage { ref msg }, ref node_id } => {
 			assert_eq!(*node_id, nodes[0].node.get_our_node_id());
-			assert_eq!(msg.data, "Funding transaction failed to confirm in 2016 blocks");
+			assert_eq!(msg.data, "Funding transaction failed to confirm within 2016 blocks");
 		},
 		_ => panic!("Unexpected event"),
 	}