Add commit_upfront_shutdown_pubkey to ChannelHandshakeConfig

Author: matchacactus

lightning/src/ln/chanmon_update_fail_tests.rs

@@ -2461,7 +2461,7 @@ fn test_temporary_error_during_shutdown() {
 	// Test that temporary failures when updating the monitor's shutdown script delay cooperative
 	// close.
 	let mut config = test_default_channel_config();
-	config.channel_options.commit_upfront_shutdown_pubkey = false;
+	config.own_channel_config.commit_upfront_shutdown_pubkey = false;
 
 	let chanmon_cfgs = create_chanmon_cfgs(2);
 	let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
@@ -2516,7 +2516,7 @@ fn test_permanent_error_during_sending_shutdown() {
 	// Test that permanent failures when updating the monitor's shutdown script result in a force
 	// close when initiating a cooperative close.
 	let mut config = test_default_channel_config();
-	config.channel_options.commit_upfront_shutdown_pubkey = false;
+	config.own_channel_config.commit_upfront_shutdown_pubkey = false;
 
 	let chanmon_cfgs = create_chanmon_cfgs(2);
 	let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
@@ -2537,7 +2537,7 @@ fn test_permanent_error_during_handling_shutdown() {
 	// Test that permanent failures when updating the monitor's shutdown script result in a force
 	// close when handling a cooperative close.
 	let mut config = test_default_channel_config();
-	config.channel_options.commit_upfront_shutdown_pubkey = false;
+	config.own_channel_config.commit_upfront_shutdown_pubkey = false;
 
 	let chanmon_cfgs = create_chanmon_cfgs(2);
 	let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);

lightning/src/ln/channel.rs

@@ -425,6 +425,7 @@ pub(super) struct Channel<Signer: Sign> {
 	pub(crate) config: ChannelConfig,
 	#[cfg(not(any(test, feature = "_test_utils")))]
 	config: ChannelConfig,
+	commit_upfront_shutdown_pubkey: bool,
 
 	user_id: u64,
 
@@ -751,7 +752,7 @@ impl<Signer: Sign> Channel<Signer> {
 		let mut secp_ctx = Secp256k1::new();
 		secp_ctx.seeded_randomize(&keys_provider.get_secure_random_bytes());
 
-		let shutdown_scriptpubkey = if config.channel_options.commit_upfront_shutdown_pubkey {
+		let shutdown_scriptpubkey = if config.own_channel_config.commit_upfront_shutdown_pubkey {
 			Some(keys_provider.get_shutdown_scriptpubkey())
 		} else { None };
 
@@ -764,6 +765,7 @@ impl<Signer: Sign> Channel<Signer> {
 		Ok(Channel {
 			user_id,
 			config: config.channel_options.clone(),
+			commit_upfront_shutdown_pubkey: config.own_channel_config.commit_upfront_shutdown_pubkey.clone(),
 
 			channel_id: keys_provider.get_secure_random_bytes(),
 			channel_state: ChannelState::OurInitSent as u32,
@@ -1046,7 +1048,7 @@ impl<Signer: Sign> Channel<Signer> {
 			}
 		} else { None };
 
-		let shutdown_scriptpubkey = if config.channel_options.commit_upfront_shutdown_pubkey {
+		let shutdown_scriptpubkey = if config.own_channel_config.commit_upfront_shutdown_pubkey {
 			Some(keys_provider.get_shutdown_scriptpubkey())
 		} else { None };
 
@@ -1062,7 +1064,7 @@ impl<Signer: Sign> Channel<Signer> {
 		let chan = Channel {
 			user_id,
 			config: local_config,
-
+			commit_upfront_shutdown_pubkey: config.own_channel_config.commit_upfront_shutdown_pubkey,
 			channel_id: msg.temporary_channel_id,
 			channel_state: (ChannelState::OurInitSent as u32) | (ChannelState::TheirInitSent as u32),
 			secp_ctx,
@@ -5191,7 +5193,7 @@ impl<Signer: Sign> Writeable for Channel<Signer> {
 		self.config.forwarding_fee_proportional_millionths.write(writer)?;
 		self.config.cltv_expiry_delta.write(writer)?;
 		self.config.announced_channel.write(writer)?;
-		self.config.commit_upfront_shutdown_pubkey.write(writer)?;
+		self.commit_upfront_shutdown_pubkey.write(writer)?;
 
 		self.channel_id.write(writer)?;
 		(self.channel_state | ChannelState::PeerDisconnected as u32).write(writer)?;
@@ -5434,6 +5436,7 @@ impl<Signer: Sign> Writeable for Channel<Signer> {
 			(9, self.target_closing_feerate_sats_per_kw, option),
 			(11, self.monitor_pending_finalized_fulfills, vec_type),
 			(13, self.channel_creation_height, required),
+			(15, self.commit_upfront_shutdown_pubkey, required),
 		});
 
 		Ok(())
@@ -5675,6 +5678,7 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<(&'a K, u32)> for Channel<Signer>
 		// only, so we default to that if none was written.
 		let mut channel_type = Some(ChannelTypeFeatures::only_static_remote_key());
 		let mut channel_creation_height = Some(serialized_height);
+		let mut commit_upfront_shutdown_pubkey = None;
 		read_tlv_fields!(reader, {
 			(0, announcement_sigs, option),
 			(1, minimum_depth, option),
@@ -5687,6 +5691,7 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<(&'a K, u32)> for Channel<Signer>
 			(9, target_closing_feerate_sats_per_kw, option),
 			(11, monitor_pending_finalized_fulfills, vec_type),
 			(13, channel_creation_height, option),
+			(15, commit_upfront_shutdown_pubkey, option),
 		});
 
 		let chan_features = channel_type.as_ref().unwrap();
@@ -5701,13 +5706,28 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<(&'a K, u32)> for Channel<Signer>
 			return Err(DecodeError::InvalidValue);
 		}
 
+		if commit_upfront_shutdown_pubkey.is_none() {
+			// commit_upfront_shutdown_pubkey has moved around a good bit, in version 1
+			// serialization, it was written out as a part of the explicit field list of the
+			// `ChannelConfig`. Then, it was written out as a field in the `ChannelConfig` itself.
+			// Now, it is written out explicitly as its own TLV (as the field has moved to
+			// `ChannelHandshakeConfig`).
+			// Thus, if its not in a TLV, we here pull it from the `ChannelConfig`, and if we can't
+			// find it at all, fail.
+			let legacy_commit_upfront_shutdown_pubkey = config.as_ref().unwrap().commit_upfront_shutdown_pubkey;
+			if let Some(val) = legacy_commit_upfront_shutdown_pubkey {
+				commit_upfront_shutdown_pubkey = Some(val);
+			} else {
+				return Err(DecodeError::InvalidValue);
+			}
+		}
+
 		let mut secp_ctx = Secp256k1::new();
 		secp_ctx.seeded_randomize(&keys_source.get_secure_random_bytes());
 
 		Ok(Channel {
 			user_id,
-
-			config: config.unwrap(),
+			commit_upfront_shutdown_pubkey: commit_upfront_shutdown_pubkey.unwrap(),			config: config.unwrap(),
 			channel_id,
 			channel_state,
 			secp_ctx,

lightning/src/ln/shutdown_tests.rs

@@ -404,7 +404,7 @@ fn test_upfront_shutdown_script() {
 	let mut config = UserConfig::default();
 	config.channel_options.announced_channel = true;
 	config.peer_channel_config_limits.force_announced_channel_preference = false;
-	config.channel_options.commit_upfront_shutdown_pubkey = false;
+	config.own_channel_config.commit_upfront_shutdown_pubkey = false;
 	let user_cfgs = [None, Some(config), None];
 	let chanmon_cfgs = create_chanmon_cfgs(3);
 	let node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
@@ -569,7 +569,7 @@ fn test_segwit_v0_shutdown_script() {
 	let mut config = UserConfig::default();
 	config.channel_options.announced_channel = true;
 	config.peer_channel_config_limits.force_announced_channel_preference = false;
-	config.channel_options.commit_upfront_shutdown_pubkey = false;
+	config.own_channel_config.commit_upfront_shutdown_pubkey = false;
 	let user_cfgs = [None, Some(config), None];
 	let chanmon_cfgs = create_chanmon_cfgs(3);
 	let node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
@@ -604,7 +604,7 @@ fn test_anysegwit_shutdown_script() {
 	let mut config = UserConfig::default();
 	config.channel_options.announced_channel = true;
 	config.peer_channel_config_limits.force_announced_channel_preference = false;
-	config.channel_options.commit_upfront_shutdown_pubkey = false;
+	config.own_channel_config.commit_upfront_shutdown_pubkey = false;
 	let user_cfgs = [None, Some(config), None];
 	let chanmon_cfgs = create_chanmon_cfgs(3);
 	let node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
@@ -639,7 +639,7 @@ fn test_unsupported_anysegwit_shutdown_script() {
 	let mut config = UserConfig::default();
 	config.channel_options.announced_channel = true;
 	config.peer_channel_config_limits.force_announced_channel_preference = false;
-	config.channel_options.commit_upfront_shutdown_pubkey = false;
+	config.own_channel_config.commit_upfront_shutdown_pubkey = false;
 	let user_cfgs = [None, Some(config), None];
 	let chanmon_cfgs = create_chanmon_cfgs(3);
 	let mut node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
@@ -681,7 +681,7 @@ fn test_invalid_shutdown_script() {
 	let mut config = UserConfig::default();
 	config.channel_options.announced_channel = true;
 	config.peer_channel_config_limits.force_announced_channel_preference = false;
-	config.channel_options.commit_upfront_shutdown_pubkey = false;
+	config.own_channel_config.commit_upfront_shutdown_pubkey = false;
 	let user_cfgs = [None, Some(config), None];
 	let chanmon_cfgs = create_chanmon_cfgs(3);
 	let node_cfgs = create_node_cfgs(3, &chanmon_cfgs);

lightning/src/util/config.rs

@@ -47,6 +47,18 @@ pub struct ChannelHandshakeConfig {
 	/// Default value: 1. If the value is less than 1, it is ignored and set to 1, as is required
 	/// by the protocol.
 	pub our_htlc_minimum_msat: u64,
+	/// When set, we commit to an upfront shutdown_pubkey at channel open. If our counterparty
+	/// supports it, they will then enforce the mutual-close output to us matches what we provided
+	/// at intialization, preventing us from closing to an alternate pubkey.
+	///
+	/// This is set to true by default to provide a slight increase in security, though ultimately
+	/// any attacker who is able to take control of a channel can just as easily send the funds via
+	/// lightning payments, so we never require that our counterparties support this option.
+	///
+	/// This cannot be changed after a channel has been initialized.
+	///
+	/// Default value: true.
+	pub commit_upfront_shutdown_pubkey: bool
 }
 
 impl Default for ChannelHandshakeConfig {
@@ -55,6 +67,7 @@ impl Default for ChannelHandshakeConfig {
 			minimum_depth: 6,
 			our_to_self_delay: BREAKDOWN_TIMEOUT,
 			our_htlc_minimum_msat: 1,
+			commit_upfront_shutdown_pubkey: true
 		}
 	}
 }
@@ -195,18 +208,8 @@ pub struct ChannelConfig {
 	///
 	/// Default value: false.
 	pub announced_channel: bool,
-	/// When set, we commit to an upfront shutdown_pubkey at channel open. If our counterparty
-	/// supports it, they will then enforce the mutual-close output to us matches what we provided
-	/// at intialization, preventing us from closing to an alternate pubkey.
-	///
-	/// This is set to true by default to provide a slight increase in security, though ultimately
-	/// any attacker who is able to take control of a channel can just as easily send the funds via
-	/// lightning payments, so we never require that our counterparties support this option.
-	///
-	/// This cannot be changed after a channel has been initialized.
-	///
-	/// Default value: true.
-	pub commit_upfront_shutdown_pubkey: bool,
+	/// This value is moved to ChannelHandshakeConfig, optional here for old serialiization
+	pub(crate) commit_upfront_shutdown_pubkey: Option<bool>,
 	/// Limit our total exposure to in-flight HTLCs which are burned to fees as they are too
 	/// small to claim on-chain.
 	///
@@ -256,7 +259,7 @@ impl Default for ChannelConfig {
 			forwarding_fee_base_msat: 1000,
 			cltv_expiry_delta: 6 * 12, // 6 blocks/hour * 12 hours
 			announced_channel: false,
-			commit_upfront_shutdown_pubkey: true,
+			commit_upfront_shutdown_pubkey: None,
 			max_dust_htlc_exposure_msat: 5_000_000,
 			force_close_avoidance_max_fee_satoshis: 1000,
 		}
@@ -269,7 +272,7 @@ impl_writeable_tlv_based!(ChannelConfig, {
 	(2, cltv_expiry_delta, required),
 	(3, force_close_avoidance_max_fee_satoshis, (default_value, 1000)),
 	(4, announced_channel, required),
-	(6, commit_upfront_shutdown_pubkey, required),
+	(6, commit_upfront_shutdown_pubkey, option),
 	(8, forwarding_fee_base_msat, required),
 });