Make channel fields which are from accept_channel Optional

These fields are set with a dummy value, which we should generally
be avoiding since Rust gives us a nice `Option` type to use
instead.

Further, we stop rejecting channel_update messages outright when
the htlc_maximum_msat field includes the reserve values, which
nodes could reasonably do without it meriting a channel closure.

Author: TheBlueMatt

lightning/src/ln/channel.rs

@@ -396,7 +396,7 @@ pub(super) struct Channel<Signer: Sign> {
 	counterparty_max_htlc_value_in_flight_msat: u64,
 	//get_holder_max_htlc_value_in_flight_msat(): u64,
 	/// minimum channel reserve for self to maintain - set by them.
-	counterparty_selected_channel_reserve_satoshis: u64,
+	counterparty_selected_channel_reserve_satoshis: Option<u64>,
 	// get_holder_selected_channel_reserve_satoshis(channel_value_sats: u64): u64
 	counterparty_htlc_minimum_msat: u64,
 	holder_htlc_minimum_msat: u64,
@@ -405,7 +405,7 @@ pub(super) struct Channel<Signer: Sign> {
 	#[cfg(not(test))]
 	counterparty_max_accepted_htlcs: u16,
 	//implied by OUR_MAX_HTLCS: max_accepted_htlcs: u16,
-	minimum_depth: u32,
+	minimum_depth: Option<u32>,
 
 	counterparty_forwarding_info: Option<CounterpartyForwardingInfo>,
 
@@ -609,11 +609,11 @@ impl<Signer: Sign> Channel<Signer> {
 			counterparty_dust_limit_satoshis: 0,
 			holder_dust_limit_satoshis: MIN_DUST_LIMIT_SATOSHIS,
 			counterparty_max_htlc_value_in_flight_msat: 0,
-			counterparty_selected_channel_reserve_satoshis: 0,
+			counterparty_selected_channel_reserve_satoshis: None, // Filled in in accept_channel
 			counterparty_htlc_minimum_msat: 0,
 			holder_htlc_minimum_msat: if config.own_channel_config.our_htlc_minimum_msat == 0 { 1 } else { config.own_channel_config.our_htlc_minimum_msat },
 			counterparty_max_accepted_htlcs: 0,
-			minimum_depth: 0, // Filled in in accept_channel
+			minimum_depth: None, // Filled in in accept_channel
 
 			counterparty_forwarding_info: None,
 
@@ -851,11 +851,11 @@ impl<Signer: Sign> Channel<Signer> {
 			counterparty_dust_limit_satoshis: msg.dust_limit_satoshis,
 			holder_dust_limit_satoshis: MIN_DUST_LIMIT_SATOSHIS,
 			counterparty_max_htlc_value_in_flight_msat: cmp::min(msg.max_htlc_value_in_flight_msat, msg.funding_satoshis * 1000),
-			counterparty_selected_channel_reserve_satoshis: msg.channel_reserve_satoshis,
+			counterparty_selected_channel_reserve_satoshis: Some(msg.channel_reserve_satoshis),
 			counterparty_htlc_minimum_msat: msg.htlc_minimum_msat,
 			holder_htlc_minimum_msat: if config.own_channel_config.our_htlc_minimum_msat == 0 { 1 } else { config.own_channel_config.our_htlc_minimum_msat },
 			counterparty_max_accepted_htlcs: msg.max_accepted_htlcs,
-			minimum_depth: config.own_channel_config.minimum_depth,
+			minimum_depth: Some(config.own_channel_config.minimum_depth),
 
 			counterparty_forwarding_info: None,
 
@@ -1036,7 +1036,7 @@ impl<Signer: Sign> Channel<Signer> {
 			} else {
 				self.counterparty_max_commitment_tx_output.lock().unwrap()
 			};
-			debug_assert!(broadcaster_max_commitment_tx_output.0 <= value_to_self_msat as u64 || value_to_self_msat / 1000 >= self.counterparty_selected_channel_reserve_satoshis as i64);
+			debug_assert!(broadcaster_max_commitment_tx_output.0 <= value_to_self_msat as u64 || value_to_self_msat / 1000 >= self.counterparty_selected_channel_reserve_satoshis.unwrap() as i64);
 			broadcaster_max_commitment_tx_output.0 = cmp::max(broadcaster_max_commitment_tx_output.0, value_to_self_msat as u64);
 			debug_assert!(broadcaster_max_commitment_tx_output.1 <= value_to_remote_msat as u64 || value_to_remote_msat / 1000 >= Channel::<Signer>::get_holder_selected_channel_reserve_satoshis(self.channel_value_satoshis) as i64);
 			broadcaster_max_commitment_tx_output.1 = cmp::max(broadcaster_max_commitment_tx_output.1, value_to_remote_msat as u64);
@@ -1220,7 +1220,12 @@ impl<Signer: Sign> Channel<Signer> {
 	/// @local is used only to convert relevant internal structures which refer to remote vs local
 	/// to decide value of outputs and direction of HTLCs.
 	fn build_htlc_transaction(&self, prev_hash: &Txid, htlc: &HTLCOutputInCommitment, local: bool, keys: &TxCreationKeys, feerate_per_kw: u32) -> Transaction {
-		chan_utils::build_htlc_transaction(prev_hash, feerate_per_kw, if local { self.get_counterparty_selected_contest_delay() } else { self.get_holder_selected_contest_delay() }, htlc, &keys.broadcaster_delayed_payment_key, &keys.revocation_key)
+		chan_utils::build_htlc_transaction(prev_hash, feerate_per_kw,
+			if local {
+				self.get_counterparty_selected_contest_delay().unwrap()
+			} else {
+				self.get_holder_selected_contest_delay()
+			}, htlc, &keys.broadcaster_delayed_payment_key, &keys.revocation_key)
 	}
 
 	/// Per HTLC, only one get_update_fail_htlc or get_update_fulfill_htlc call may be made.
@@ -1517,10 +1522,10 @@ impl<Signer: Sign> Channel<Signer> {
 
 		self.counterparty_dust_limit_satoshis = msg.dust_limit_satoshis;
 		self.counterparty_max_htlc_value_in_flight_msat = cmp::min(msg.max_htlc_value_in_flight_msat, self.channel_value_satoshis * 1000);
-		self.counterparty_selected_channel_reserve_satoshis = msg.channel_reserve_satoshis;
+		self.counterparty_selected_channel_reserve_satoshis = Some(msg.channel_reserve_satoshis);
 		self.counterparty_htlc_minimum_msat = msg.htlc_minimum_msat;
 		self.counterparty_max_accepted_htlcs = msg.max_accepted_htlcs;
-		self.minimum_depth = msg.minimum_depth;
+		self.minimum_depth = Some(msg.minimum_depth);
 
 		let counterparty_pubkeys = ChannelPublicKeys {
 			funding_pubkey: msg.funding_pubkey,
@@ -2070,7 +2075,7 @@ impl<Signer: Sign> Channel<Signer> {
 			// Check that they won't violate our local required channel reserve by adding this HTLC.
 			let htlc_candidate = HTLCCandidate::new(msg.amount_msat, HTLCInitiator::RemoteOffered);
 			let local_commit_tx_fee_msat = self.next_local_commit_tx_fee_msat(htlc_candidate, None);
-			if self.value_to_self_msat < self.counterparty_selected_channel_reserve_satoshis * 1000 + local_commit_tx_fee_msat {
+			if self.value_to_self_msat < self.counterparty_selected_channel_reserve_satoshis.unwrap() * 1000 + local_commit_tx_fee_msat {
 				return Err(ChannelError::Close("Cannot accept HTLC that would put our balance under counterparty-announced channel reserve value".to_owned()));
 			}
 		}
@@ -3369,8 +3374,9 @@ impl<Signer: Sign> Channel<Signer> {
 		&self.channel_transaction_parameters.holder_pubkeys
 	}
 
-	fn get_counterparty_selected_contest_delay(&self) -> u16 {
-		self.channel_transaction_parameters.counterparty_parameters.as_ref().unwrap().selected_contest_delay
+	fn get_counterparty_selected_contest_delay(&self) -> Option<u16> {
+		self.channel_transaction_parameters.counterparty_parameters
+			.as_ref().map(|params| params.selected_contest_delay)
 	}
 
 	fn get_counterparty_pubkeys(&self) -> &ChannelPublicKeys {
@@ -3444,7 +3450,7 @@ impl<Signer: Sign> Channel<Signer> {
 		ChannelValueStat {
 			value_to_self_msat: self.value_to_self_msat,
 			channel_value_msat: self.channel_value_satoshis * 1000,
-			channel_reserve_msat: self.counterparty_selected_channel_reserve_satoshis * 1000,
+			channel_reserve_msat: self.counterparty_selected_channel_reserve_satoshis.unwrap() * 1000,
 			pending_outbound_htlcs_amount_msat: self.pending_outbound_htlcs.iter().map(|ref h| h.amount_msat).sum::<u64>(),
 			pending_inbound_htlcs_amount_msat: self.pending_inbound_htlcs.iter().map(|ref h| h.amount_msat).sum::<u64>(),
 			holding_cell_outbound_amount_msat: {
@@ -3561,7 +3567,7 @@ impl<Signer: Sign> Channel<Signer> {
 			self.funding_tx_confirmation_height = 0;
 		}
 
-		if funding_tx_confirmations < self.minimum_depth as i64 {
+		if funding_tx_confirmations < self.minimum_depth.unwrap_or(0) as i64 {
 			return None;
 		}
 
@@ -3716,10 +3722,10 @@ impl<Signer: Sign> Channel<Signer> {
 			// the funding transaction's confirmation count has dipped below minimum_depth / 2,
 			// close the channel and hope we can get the latest state on chain (because presumably
 			// the funding transaction is at least still in the mempool of most nodes).
-			if funding_tx_confirmations < self.minimum_depth as i64 / 2 {
+			if funding_tx_confirmations < self.minimum_depth.unwrap() as i64 / 2 {
 				return Err(msgs::ErrorMessage {
 					channel_id: self.channel_id(),
-					data: format!("Funding transaction was un-confirmed. Locked at {} confs, now have {} confs.", self.minimum_depth, funding_tx_confirmations),
+					data: format!("Funding transaction was un-confirmed. Locked at {} confs, now have {} confs.", self.minimum_depth.unwrap(), funding_tx_confirmations),
 				});
 			}
 		}
@@ -3814,7 +3820,7 @@ impl<Signer: Sign> Channel<Signer> {
 			max_htlc_value_in_flight_msat: Channel::<Signer>::get_holder_max_htlc_value_in_flight_msat(self.channel_value_satoshis),
 			channel_reserve_satoshis: Channel::<Signer>::get_holder_selected_channel_reserve_satoshis(self.channel_value_satoshis),
 			htlc_minimum_msat: self.holder_htlc_minimum_msat,
-			minimum_depth: self.minimum_depth,
+			minimum_depth: self.minimum_depth.unwrap(),
 			to_self_delay: self.get_holder_selected_contest_delay(),
 			max_accepted_htlcs: OUR_MAX_HTLCS,
 			funding_pubkey: keys.funding_pubkey,
@@ -4112,7 +4118,7 @@ impl<Signer: Sign> Channel<Signer> {
 
 		// Check self.counterparty_selected_channel_reserve_satoshis (the amount we must keep as
 		// reserve for the remote to have something to claim if we misbehave)
-		let chan_reserve_msat = self.counterparty_selected_channel_reserve_satoshis * 1000;
+		let chan_reserve_msat = self.counterparty_selected_channel_reserve_satoshis.unwrap() * 1000;
 		if pending_value_to_self_msat - amount_msat - commit_tx_fee_msat < chan_reserve_msat {
 			return Err(ChannelError::Ignore(format!("Cannot send value that would put our balance under counterparty-announced channel reserve value ({})", chan_reserve_msat)));
 		}
@@ -4317,8 +4323,7 @@ impl<Signer: Sign> Channel<Signer> {
 	}
 
 	pub fn channel_update(&mut self, msg: &msgs::ChannelUpdate) -> Result<(), ChannelError> {
-		let usable_channel_value_msat = (self.channel_value_satoshis - self.counterparty_selected_channel_reserve_satoshis) * 1000;
-		if msg.contents.htlc_minimum_msat >= usable_channel_value_msat {
+		if msg.contents.htlc_minimum_msat >= self.channel_value_satoshis * 1000 {
 			return Err(ChannelError::Close("Minimum htlc value is greater than channel value".to_string()));
 		}
 		self.counterparty_forwarding_info = Some(CounterpartyForwardingInfo {
@@ -4646,11 +4651,11 @@ impl<Signer: Sign> Writeable for Channel<Signer> {
 		self.counterparty_dust_limit_satoshis.write(writer)?;
 		self.holder_dust_limit_satoshis.write(writer)?;
 		self.counterparty_max_htlc_value_in_flight_msat.write(writer)?;
-		self.counterparty_selected_channel_reserve_satoshis.write(writer)?;
+		self.counterparty_selected_channel_reserve_satoshis.unwrap_or(0).write(writer)?;
 		self.counterparty_htlc_minimum_msat.write(writer)?;
 		self.holder_htlc_minimum_msat.write(writer)?;
 		self.counterparty_max_accepted_htlcs.write(writer)?;
-		self.minimum_depth.write(writer)?;
+		self.minimum_depth.unwrap_or(0).write(writer)?;
 
 		match &self.counterparty_forwarding_info {
 			Some(info) => {
@@ -4675,7 +4680,17 @@ impl<Signer: Sign> Writeable for Channel<Signer> {
 
 		self.channel_update_status.write(writer)?;
 
-		write_tlv_fields!(writer, {(0, self.announcement_sigs, option)});
+		write_tlv_fields!(writer, {
+			(0, self.announcement_sigs, option),
+			// minimum_depth and counterparty_selected_channel_reserve_satoshis used to have a
+			// default value instead of being Option<>al. Thus, to maintain compatibility we write
+			// them twice, once with their original default values above, and once as an option
+			// here. On the read side, old versions will simply ignore the odd-type entries here,
+			// and new versions map the default values to None and allow the TLV entries here to
+			// override that.
+			(1, self.minimum_depth, option),
+			(3, self.counterparty_selected_channel_reserve_satoshis, option),
+		});
 
 		Ok(())
 	}
@@ -4818,11 +4833,21 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel<Signer>
 		let counterparty_dust_limit_satoshis = Readable::read(reader)?;
 		let holder_dust_limit_satoshis = Readable::read(reader)?;
 		let counterparty_max_htlc_value_in_flight_msat = Readable::read(reader)?;
-		let counterparty_selected_channel_reserve_satoshis = Readable::read(reader)?;
+		let mut counterparty_selected_channel_reserve_satoshis = Some(Readable::read(reader)?);
+		if counterparty_selected_channel_reserve_satoshis == Some(0) {
+			// Versions up to 0.0.98 had counterparty_selected_channel_reserve_satoshis as a
+			// non-option, writing 0 for what we now consider None.
+			counterparty_selected_channel_reserve_satoshis = None;
+		}
 		let counterparty_htlc_minimum_msat = Readable::read(reader)?;
 		let holder_htlc_minimum_msat = Readable::read(reader)?;
 		let counterparty_max_accepted_htlcs = Readable::read(reader)?;
-		let minimum_depth = Readable::read(reader)?;
+		let mut minimum_depth = Some(Readable::read(reader)?);
+		if minimum_depth == Some(0) {
+			// Versions up to 0.0.98 had minimum_depth as a non-option, writing 0 for what we now
+			// consider None.
+			minimum_depth = None;
+		}
 
 		let counterparty_forwarding_info = match <u8 as Readable>::read(reader)? {
 			0 => None,
@@ -4848,7 +4873,11 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel<Signer>
 		let channel_update_status = Readable::read(reader)?;
 
 		let mut announcement_sigs = None;
-		read_tlv_fields!(reader, {(0, announcement_sigs, option)});
+		read_tlv_fields!(reader, {
+			(0, announcement_sigs, option),
+			(1, minimum_depth, option),
+			(3, counterparty_selected_channel_reserve_satoshis, option),
+		});
 
 		let mut secp_ctx = Secp256k1::new();
 		secp_ctx.seeded_randomize(&keys_source.get_secure_random_bytes());
@@ -5303,6 +5332,7 @@ mod tests {
 		config.channel_options.announced_channel = false;
 		let mut chan = Channel::<InMemorySigner>::new_outbound(&&feeest, &&keys_provider, counterparty_node_id, 10_000_000, 100000, 42, &config).unwrap(); // Nothing uses their network key in this test
 		chan.holder_dust_limit_satoshis = 546;
+		chan.counterparty_selected_channel_reserve_satoshis = Some(0); // Filled in in accept_channel
 
 		let funding_info = OutPoint{ txid: Txid::from_hex("8984484a580b825b9972d7adb15050b3ab624ccd731946b3eeddb92f4e7ef6be").unwrap(), index: 0 };