Review comments: reserve check, etc

Author: optout21

lightning/src/ln/channel.rs

@@ -23,8 +23,6 @@ use bitcoin::secp256k1::constants::PUBLIC_KEY_SIZE;
 use bitcoin::secp256k1::{PublicKey,SecretKey};
 use bitcoin::secp256k1::{Secp256k1,ecdsa::Signature};
 use bitcoin::{secp256k1, sighash};
-#[cfg(splicing)]
-use bitcoin::TxIn;
 
 use crate::ln::types::ChannelId;
 use crate::types::payment::{PaymentPreimage, PaymentHash};
@@ -1185,11 +1183,12 @@ impl UnfundedChannelContext {
 #[derive(Clone)]
 pub(crate) struct PendingSpliceInfoPre {
 	pub our_funding_contribution: i64,
-	pub funding_feerate_perkw: u32,
-	pub locktime: u32,
-	/// The funding inputs we will be contributing to the splice.
-	/// TODO(splice): will be changed to TransactionU16LenLimited
-	pub our_funding_inputs: Vec<(TxIn, Transaction)>,
+	// TODO(splicing): Enable below fields
+	// pub funding_feerate_perkw: u32,
+	// pub locktime: u32,
+	// /// The funding inputs we will be contributing to the splice.
+	// /// TODO(splice): will be changed to TransactionU16LenLimited
+	// pub our_funding_inputs: Vec<(TxIn, Transaction)>,
 }
 
 #[cfg(splicing)]
@@ -3425,6 +3424,27 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
 		(context.holder_selected_channel_reserve_satoshis, context.counterparty_selected_channel_reserve_satoshis)
 	}
 
+	/// Check that a proposed channel value meets the channel reserve requirements or violates them (below reserve)
+	pub fn check_channel_value_meets_reserve_requirements(&self, proposed_channel_value: u64) -> Result<(), ChannelError> {
+		let holder_selected_channel_reserve_satoshis = get_v2_channel_reserve_satoshis(
+			proposed_channel_value, self.holder_dust_limit_satoshis);
+		if proposed_channel_value < holder_selected_channel_reserve_satoshis {
+			return Err(ChannelError::Warn(format!(
+				"Proposed channel value below reserve mandated by holder, {} vs {}",
+				proposed_channel_value, holder_selected_channel_reserve_satoshis,
+			)));
+		}
+		let counterparty_selected_channel_reserve_satoshis = get_v2_channel_reserve_satoshis(
+			proposed_channel_value, self.counterparty_dust_limit_satoshis);
+		if proposed_channel_value < counterparty_selected_channel_reserve_satoshis {
+			return Err(ChannelError::Warn(format!(
+				"Proposed channel value below reserve mandated by counterparty, {} vs {}",
+				proposed_channel_value, counterparty_selected_channel_reserve_satoshis,
+			)));
+		}
+		Ok(())
+	}
+
 	/// Get the commitment tx fee for the local's (i.e. our) next commitment transaction based on the
 	/// number of pending HTLCs that are on track to be in our next commitment tx.
 	///
@@ -3828,10 +3848,6 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
 	pub fn get_splice_init(&self, our_funding_contribution_satoshis: i64, signer_provider: &SP,
 		funding_feerate_perkw: u32, locktime: u32,
 	) -> msgs::SpliceInit {
-		if !self.is_outbound() {
-			panic!("Tried to initiate a splice on an inbound channel!");
-		}
-
 		// At this point we are not committed to the new channel value yet, but the funding pubkey
 		// depends on the channel value, so we create here a new funding pubkey with the new
 		// channel value.
@@ -3861,10 +3877,6 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
 	/// Get the splice_ack message that can be sent in response to splice initiation.
 	#[cfg(splicing)]
 	pub fn get_splice_ack(&mut self, our_funding_contribution_satoshis: i64) -> Result<msgs::SpliceAck, ChannelError> {
-		if self.is_outbound() {
-			panic!("Tried to accept a splice on an outound channel!");
-		}
-
 		// TODO(splicing): checks
 
 		// Note: at this point keys are already updated
@@ -7458,7 +7470,7 @@ impl<SP: Deref> Channel<SP> where
 	#[cfg(splicing)]
 	pub fn splice_init<ES: Deref, L: Deref>(
 		&mut self, our_funding_contribution_satoshis: i64,
-		_signer_provider: &SP, _entropy_source: &ES, _holder_node_id: PublicKey, logger: &L
+		_signer_provider: &SP, _entropy_source: &ES, _holder_node_id: PublicKey, _logger: &L
 	)
 		-> Result<msgs::SpliceAck, ChannelError>
 		where ES::Target: EntropySource, L::Target: Logger

lightning/src/ln/channelmanager.rs

@@ -4011,7 +4011,7 @@ where
 	#[cfg(splicing)]
 	pub fn splice_channel(
 		&self, channel_id: &ChannelId, counterparty_node_id: &PublicKey, our_funding_contribution_satoshis: i64,
-		our_funding_inputs: Vec<(TxIn, Transaction)>, funding_feerate_perkw: u32, locktime: u32,
+		_our_funding_inputs: Vec<(TxIn, Transaction)>, funding_feerate_perkw: u32, locktime: u32,
 	) -> Result<(), APIError> {
 		let per_peer_state = self.per_peer_state.read().unwrap();
 
@@ -4027,7 +4027,7 @@ where
 				if let ChannelPhase::Funded(chan) = chan_phase_entry.get_mut() {
 					let pre_channel_value = chan.context.get_value_satoshis();
 					// Sanity check: capacity cannot decrease below 0
-					if our_funding_contribution_satoshis < 0 && -our_funding_contribution_satoshis > (pre_channel_value as i64) {
+					if (pre_channel_value as i64).saturating_add(our_funding_contribution_satoshis) < 0 {
 						return Err(APIError::APIMisuseError {
 							err: format!(
 								"Post-splicing channel value cannot be negative. It was {} - {}",
@@ -4054,7 +4054,6 @@ where
 
 					chan.context.pending_splice_pre = Some(PendingSpliceInfoPre {
 						our_funding_contribution: our_funding_contribution_satoshis,
-						funding_feerate_perkw, locktime, our_funding_inputs,
 					});
 
 					let msg = chan.context.get_splice_init(our_funding_contribution_satoshis, &self.signer_provider, funding_feerate_perkw, locktime);
@@ -9013,7 +9012,7 @@ where
 				if let ChannelPhase::Funded(chan) = chan_entry.get_mut() {
 					let pre_channel_value = chan.context.get_value_satoshis();
 					// Sanity check: capacity cannot decrease below 0
-					if msg.funding_contribution_satoshis < 0 && -msg.funding_contribution_satoshis > (pre_channel_value as i64) {
+					if (pre_channel_value as i64).saturating_add(msg.funding_contribution_satoshis) < 0 {
 						return Err(MsgHandleErrInternal::send_err_msg_no_close(format!(
 							"Post-splicing channel value cannot be negative. It was {} - {}", pre_channel_value, -msg.funding_contribution_satoshis,
 						), msg.channel_id));
@@ -9026,11 +9025,10 @@ where
 					}
 
 					let post_channel_value = PendingSpliceInfoPre::compute_post_value(pre_channel_value, msg.funding_contribution_satoshis, our_funding_contribution);
-					if post_channel_value < 1000 {
-						return Err(MsgHandleErrInternal::send_err_msg_no_close(format!(
-							"Post-splicing channel value must be at least 1000 satoshis. It was {}", post_channel_value,
-						), msg.channel_id));
-					}
+
+					// Check for reserve requirement, it will also be checked later at tx_complete
+					let _res = chan.context.check_channel_value_meets_reserve_requirements(post_channel_value)
+						.map_err(|e| MsgHandleErrInternal::from_chan_no_close(e, msg.channel_id))?;
 
 					// Check if a splice has been initiated already.
 					// Note: this could be handled more nicely, and support multiple outstanding splice's, the incoming splice_ack matters anyways.
@@ -9083,20 +9081,27 @@ where
 		let peer_state = &mut *peer_state_lock;
 
 		// Look for the channel
-		let _pending_splice = match peer_state.channel_by_id.entry(msg.channel_id) {
+		match peer_state.channel_by_id.entry(msg.channel_id) {
 			hash_map::Entry::Vacant(_) => return Err(MsgHandleErrInternal::send_err_msg_no_close(format!(
 					"Got a message for a channel from the wrong node! No such channel for the passed counterparty_node_id {}",
 					counterparty_node_id
 				), msg.channel_id)),
 			hash_map::Entry::Occupied(mut chan) => {
 				if let ChannelPhase::Funded(chan) = chan.get_mut() {
 					// check if splice is pending
-					if let Some(pending_splice) = &chan.context.pending_splice_pre {
+					let pending_splice = if let Some(pending_splice) = &chan.context.pending_splice_pre {
 						// Note: this is incomplete (their funding contribution is not set)
 						pending_splice.clone()
 					} else {
 						return Err(MsgHandleErrInternal::send_err_msg_no_close("Channel is not in pending splice".to_owned(), msg.channel_id));
-					}
+					};
+
+					let pre_channel_value = chan.context.get_value_satoshis();
+					let post_channel_value = PendingSpliceInfoPre::compute_post_value(pre_channel_value, pending_splice.our_funding_contribution, msg.funding_contribution_satoshis);
+
+					// Check for reserve requirement, it will also be checked later at tx_complete
+					let _res = chan.context.check_channel_value_meets_reserve_requirements(post_channel_value)
+						.map_err(|e| MsgHandleErrInternal::from_chan_no_close(e, msg.channel_id))?;
 				} else {
 					return Err(MsgHandleErrInternal::send_err_msg_no_close("Channel is not funded, cannot splice".to_owned(), msg.channel_id));
 				}