Add user configurable csv delay encumbering channel refund output,

within reasonable lower or upper bound

Add our_to_self_delay in Channel, to cache user config field at
channel construction.

Author: Antoine Riard

src/ln/channel.rs

@@ -317,7 +317,7 @@ pub(super) struct Channel {
 	their_htlc_minimum_msat: u64,
 	our_htlc_minimum_msat: u64,
 	their_to_self_delay: u16,
-	//implied by BREAKDOWN_TIMEOUT: our_to_self_delay: u16,
+	our_to_self_delay: u16,
 	#[cfg(test)]
 	pub their_max_accepted_htlcs: u16,
 	#[cfg(not(test))]
@@ -413,6 +413,9 @@ impl Channel {
 		if push_msat > channel_value_satoshis * 1000 {
 			return Err(APIError::APIMisuseError{err: "push value > channel value"});
 		}
+		if config.own_channel_config.our_to_self_delay < BREAKDOWN_TIMEOUT {
+			return Err(APIError::APIMisuseError{err: "Configured with an unreasonable our_to_self_delay putting user funds at risks"});
+		}
 
 
 		let background_feerate = fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Background);
@@ -481,6 +484,7 @@ impl Channel {
 			their_htlc_minimum_msat: 0,
 			our_htlc_minimum_msat: Channel::derive_our_htlc_minimum_msat(feerate),
 			their_to_self_delay: 0,
+			our_to_self_delay: config.own_channel_config.our_to_self_delay,
 			their_max_accepted_htlcs: 0,
 			minimum_depth: 0, // Filled in in accept_channel
 
@@ -518,6 +522,10 @@ impl Channel {
 		let chan_keys = keys_provider.get_channel_keys(true);
 		let mut local_config = (*config).channel_options.clone();
 
+		if config.own_channel_config.our_to_self_delay < BREAKDOWN_TIMEOUT {
+			return Err(ChannelError::Close("Configured with an unreasonable our_to_self_delay putting user funds at risks"));
+		}
+
 		// Check sanity of message fields:
 		if msg.funding_satoshis >= MAX_FUNDING_SATOSHIS {
 			return Err(ChannelError::Close("funding value > 2^24"));
@@ -539,7 +547,7 @@ impl Channel {
 		}
 		Channel::check_remote_fee(fee_estimator, msg.feerate_per_kw)?;
 
-		if msg.to_self_delay > MAX_LOCAL_BREAKDOWN_TIMEOUT {
+		if msg.to_self_delay > config.peer_channel_config_limits.their_to_self_delay || msg.to_self_delay > MAX_LOCAL_BREAKDOWN_TIMEOUT {
 			return Err(ChannelError::Close("They wanted our payments to be delayed by a needlessly long period"));
 		}
 		if msg.max_accepted_htlcs < 1 {
@@ -671,6 +679,7 @@ impl Channel {
 			their_htlc_minimum_msat: msg.htlc_minimum_msat,
 			our_htlc_minimum_msat: Channel::derive_our_htlc_minimum_msat(msg.feerate_per_kw as u64),
 			their_to_self_delay: msg.to_self_delay,
+			our_to_self_delay: config.own_channel_config.our_to_self_delay,
 			their_max_accepted_htlcs: msg.max_accepted_htlcs,
 			minimum_depth: config.own_channel_config.minimum_depth,
 
@@ -1359,7 +1368,7 @@ impl Channel {
 		if msg.htlc_minimum_msat >= (self.channel_value_satoshis - msg.channel_reserve_satoshis) * 1000 {
 			return Err(ChannelError::Close("Minimum htlc value is full channel value"));
 		}
-		if msg.to_self_delay > MAX_LOCAL_BREAKDOWN_TIMEOUT {
+		if msg.to_self_delay > config.peer_channel_config_limits.their_to_self_delay || msg.to_self_delay > MAX_LOCAL_BREAKDOWN_TIMEOUT {
 			return Err(ChannelError::Close("They wanted our payments to be delayed by a needlessly long period"));
 		}
 		if msg.max_accepted_htlcs < 1 {
@@ -3021,7 +3030,7 @@ impl Channel {
 			channel_reserve_satoshis: Channel::get_our_channel_reserve_satoshis(self.channel_value_satoshis),
 			htlc_minimum_msat: self.our_htlc_minimum_msat,
 			feerate_per_kw: fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Background) as u32,
-			to_self_delay: BREAKDOWN_TIMEOUT,
+			to_self_delay: self.our_to_self_delay,
 			max_accepted_htlcs: OUR_MAX_HTLCS,
 			funding_pubkey: PublicKey::from_secret_key(&self.secp_ctx, &self.local_keys.funding_key),
 			revocation_basepoint: PublicKey::from_secret_key(&self.secp_ctx, &self.local_keys.revocation_base_key),
@@ -3054,7 +3063,7 @@ impl Channel {
 			channel_reserve_satoshis: Channel::get_our_channel_reserve_satoshis(self.channel_value_satoshis),
 			htlc_minimum_msat: self.our_htlc_minimum_msat,
 			minimum_depth: self.minimum_depth,
-			to_self_delay: BREAKDOWN_TIMEOUT,
+			to_self_delay: self.our_to_self_delay,
 			max_accepted_htlcs: OUR_MAX_HTLCS,
 			funding_pubkey: PublicKey::from_secret_key(&self.secp_ctx, &self.local_keys.funding_key),
 			revocation_basepoint: PublicKey::from_secret_key(&self.secp_ctx, &self.local_keys.revocation_base_key),
@@ -3703,6 +3712,7 @@ impl Writeable for Channel {
 		self.their_htlc_minimum_msat.write(writer)?;
 		self.our_htlc_minimum_msat.write(writer)?;
 		self.their_to_self_delay.write(writer)?;
+		self.our_to_self_delay.write(writer)?;
 		self.their_max_accepted_htlcs.write(writer)?;
 		self.minimum_depth.write(writer)?;
 
@@ -3864,6 +3874,7 @@ impl<R : ::std::io::Read> ReadableArgs<R, Arc<Logger>> for Channel {
 		let their_htlc_minimum_msat = Readable::read(reader)?;
 		let our_htlc_minimum_msat = Readable::read(reader)?;
 		let their_to_self_delay = Readable::read(reader)?;
+		let our_to_self_delay = Readable::read(reader)?;
 		let their_max_accepted_htlcs = Readable::read(reader)?;
 		let minimum_depth = Readable::read(reader)?;
 
@@ -3941,6 +3952,7 @@ impl<R : ::std::io::Read> ReadableArgs<R, Arc<Logger>> for Channel {
 			their_htlc_minimum_msat,
 			our_htlc_minimum_msat,
 			their_to_self_delay,
+			our_to_self_delay,
 			their_max_accepted_htlcs,
 			minimum_depth,
 

src/util/config.rs

@@ -1,6 +1,8 @@
 //! Various user-configurable channel limits and settings which ChannelManager
 //! applies for you.
 
+use ln::channelmanager::{BREAKDOWN_TIMEOUT, MAX_LOCAL_BREAKDOWN_TIMEOUT};
+
 /// Top-level config which holds ChannelHandshakeLimits and ChannelConfig.
 #[derive(Clone, Debug)]
 pub struct UserConfig {
@@ -30,13 +32,25 @@ pub struct ChannelHandshakeConfig {
 	/// Applied only for inbound channels (see ChannelHandshakeLimits::max_minimum_depth for the
 	/// equivalent limit applied to outbound channels).
 	pub minimum_depth: u32,
+	/// Set to the amount of time we require our counterparty to wait to claim their money.
+	///
+	/// It's one of the main parameter of our security model. We (or one of our watchtowers) MUST
+	/// be online to check for peer having broadcast a revoked transaction to steal us funds.
+	/// Default is BREAKDOWN_TIMEOUT, we enforce it as a minimum at channel opening so you can
+	/// tweak config to ask for more security, not less.
+	///
+	/// Meanwhile, asking for a too high delay, we bother peer to freeze funds for nothing in
+	/// case of an honest unilateral channel close, which implicitly decrease the economic value of
+	/// our channel.
+	pub our_to_self_delay: u16,
 }
 
 impl ChannelHandshakeConfig {
 	/// Provides sane defaults for `ChannelHandshakeConfig`
 	pub fn new() -> ChannelHandshakeConfig {
 		ChannelHandshakeConfig {
 			minimum_depth: 6,
+			our_to_self_delay: BREAKDOWN_TIMEOUT,
 		}
 	}
 }
@@ -88,6 +102,13 @@ pub struct ChannelHandshakeLimits {
 	/// Defaults to true to make the default that no announced channels are possible (which is
 	/// appropriate for any nodes which are not online very reliably).
 	pub force_announced_channel_preference: bool,
+	/// Set to the amount of time we're willing to wait to claim money back to us.
+	///
+	/// Not checking this value would be also a security issue, as our peer would be able to set it to
+	/// max relative lock-time (a year) and us losing money by useless locked funds.
+	/// Default is MAX_LOCAL_BREAKDOWN_TIMEOUT, we enforce it as a maximum at channel opening offer
+	/// so you can tweak config to reduce the loss of having useless locked funds (if you peer accept)
+	pub their_to_self_delay: u16
 }
 
 impl ChannelHandshakeLimits {
@@ -107,6 +128,7 @@ impl ChannelHandshakeLimits {
 			max_dust_limit_satoshis: <u64>::max_value(),
 			max_minimum_depth: 144,
 			force_announced_channel_preference: true,
+			their_to_self_delay: MAX_LOCAL_BREAKDOWN_TIMEOUT,
 		}
 	}
 }