For splicing, don’t use contribute_funding_inputs() call, but provide…

… inputs upfront in splice_channel(). This is in preparation for rebase to dual funding latest

README.md

@@ -27,6 +27,7 @@ Objective, Restrictions:
 - Splice from only V2 channel is supported, channel ID is not changed
 - It is assumed that all extra inputs belong to the initiator (the full capacity increase is credited to the channel initiator)
 - Only a single pending splicing is supported at a time
+- Splice-in with contributions from the acceptor is not supported
 
 Up-to-date with main branch as of v0.0.123 (May 8, 475f736; originally branched off v0.0.115).
 
@@ -68,14 +69,14 @@ Client  LDK                                       Counterparty node (acceptor)
         Cycle back the channel to UnfundedOutboundV2
         splice_start() -- ChannelContext
         Start the splice, update capacity, state to NegotiatingFunding, reset funding transaction
-        event: SpliceAckedInputsContributionReady
-        contains the pre & post capacities, channel ID
-        ---
-event: SpliceAckedInputsContributionReady
-action by client:
-provide extra input(s) for new funding
----
-        contribute_funding_inputs() - ChannelManager API
+        //event: SpliceAckedInputsContributionReady
+        //contains the pre & post capacities, channel ID
+//        ---
+//event: SpliceAckedInputsContributionReady
+//action by client:
+//provide extra input(s) for new funding
+//---
+        //contribute_funding_inputs() - ChannelManager API
         begin_interactive_funding_tx_construction() - Channel
         begin_interactive_funding_tx_construction() - ChannelContext
         Splicing specific: Add the previous funding as an input to the new one.

lightning/src/ln/channel.rs

@@ -32,7 +32,7 @@ use crate::ln::types::{ChannelId, PaymentPreimage, PaymentHash};
 use crate::ln::channel_splice::{PendingSpliceInfoPre, PendingSpliceInfoPost};
 use crate::ln::features::{ChannelTypeFeatures, InitFeatures};
 #[cfg(any(dual_funding, splicing))]
-use crate::ln::interactivetxs::{AbortReason, ConstructedTransaction, estimate_input_weight, get_output_weight, HandleTxCompleteValue, InteractiveTxConstructor, InteractiveTxMessageSend, InteractiveTxSigningSession, TX_COMMON_FIELDS_WEIGHT};
+use crate::ln::interactivetxs::{ConstructedTransaction, estimate_input_weight, get_output_weight, HandleTxCompleteValue, InteractiveTxConstructor, InteractiveTxMessageSend, InteractiveTxSigningSession, TX_COMMON_FIELDS_WEIGHT};
 use crate::ln::msgs;
 use crate::ln::msgs::DecodeError;
 use crate::ln::script::{self, ShutdownScript};
@@ -3720,7 +3720,6 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider  {
 			}
 		}
 
-
 		let total_input_satoshis: u64 = funding_inputs_with_extra.iter().map(|input| input.1.as_transaction().output[input.0.previous_output.vout as usize].value).sum();
 		if total_input_satoshis < dual_funding_context.our_funding_satoshis {
 			return Err(APIError::APIMisuseError {
@@ -3754,39 +3753,11 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider  {
 		Ok(msg)
 	}
 
-	#[cfg(any(dual_funding, splicing))]
-	fn get_tx_abort_msg_from_abort_reason(&self, reason: AbortReason) -> msgs::TxAbort {
-		let msg = match reason {
-			AbortReason::InvalidStateTransition => "State transition was invalid",
-			AbortReason::UnexpectedCounterpartyMessage => "Unexpected message",
-			AbortReason::ReceivedTooManyTxAddInputs => "Too many `tx_add_input`s received",
-			AbortReason::ReceivedTooManyTxAddOutputs => "Too many `tx_add_output`s received",
-			AbortReason::IncorrectInputSequenceValue => "Input has a sequence value greater than 0xFFFFFFFD",
-			AbortReason::IncorrectSerialIdParity => "Parity for `serial_id` was incorrect",
-			AbortReason::SerialIdUnknown => "The `serial_id` is unknown",
-			AbortReason::DuplicateSerialId => "The `serial_id` already exists",
-			AbortReason::PrevTxOutInvalid => "Invalid previous transaction output",
-			AbortReason::ExceededMaximumSatsAllowed => "Output amount exceeded total bitcoin supply",
-			AbortReason::ExceededNumberOfInputsOrOutputs => "Too many inputs or outputs",
-			AbortReason::TransactionTooLarge => "Transaction weight is too large",
-			AbortReason::BelowDustLimit => "Output amount is below the dust limit",
-			AbortReason::InvalidOutputScript => "The output script is non-standard",
-			AbortReason::InsufficientFees => "Insufficient fees paid",
-			AbortReason::OutputsValueExceedsInputsValue => "Total value of outputs exceeds total value of inputs",
-			AbortReason::InvalidTx => "The transaction is invalid",
-		}.to_string();
-
-		msgs::TxAbort {
-			channel_id: self.channel_id(),
-			data: msg.into_bytes(),
-		}
-	}
-
 	#[cfg(any(dual_funding, splicing))]
 	pub fn tx_add_input(&mut self, msg: &msgs::TxAddInput) -> Result<InteractiveTxMessageSend, msgs::TxAbort> {
 		match self.interactive_tx_constructor {
 			Some(ref mut tx_constructor) => tx_constructor.handle_tx_add_input(msg).map_err(
-				|reason| self.get_tx_abort_msg_from_abort_reason(reason)),
+				|reason| reason.into_tx_abort_msg(self.channel_id)),
 			None => Err(msgs::TxAbort {
 				channel_id: self.channel_id(),
 				data: "We do not have an interactive transaction negotiation in progress".to_string().into_bytes()
@@ -3798,7 +3769,7 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider  {
 	pub fn tx_add_output(&mut self, msg: &msgs::TxAddOutput)-> Result<InteractiveTxMessageSend, msgs::TxAbort> {
 		match self.interactive_tx_constructor {
 			Some(ref mut tx_constructor) => tx_constructor.handle_tx_add_output(msg).map_err(
-				|reason| self.get_tx_abort_msg_from_abort_reason(reason)),
+				|reason| reason.into_tx_abort_msg(self.channel_id)),
 			None => Err(msgs::TxAbort {
 				channel_id: self.channel_id(),
 				data: "We do not have an interactive transaction negotiation in progress".to_string().into_bytes()
@@ -3810,7 +3781,7 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider  {
 	pub fn tx_remove_input(&mut self, msg: &msgs::TxRemoveInput)-> Result<InteractiveTxMessageSend, msgs::TxAbort> {
 		match self.interactive_tx_constructor {
 			Some(ref mut tx_constructor) => tx_constructor.handle_tx_remove_input(msg).map_err(
-				|reason| self.get_tx_abort_msg_from_abort_reason(reason)),
+				|reason| reason.into_tx_abort_msg(self.channel_id)),
 			None => Err(msgs::TxAbort {
 				channel_id: self.channel_id(),
 				data: "We do not have an interactive transaction negotiation in progress".to_string().into_bytes()
@@ -3822,7 +3793,7 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider  {
 	pub fn tx_remove_output(&mut self, msg: &msgs::TxRemoveOutput)-> Result<InteractiveTxMessageSend, msgs::TxAbort> {
 		match self.interactive_tx_constructor {
 			Some(ref mut tx_constructor) => tx_constructor.handle_tx_remove_output(msg).map_err(
-				|reason| self.get_tx_abort_msg_from_abort_reason(reason)),
+				|reason| reason.into_tx_abort_msg(self.channel_id)),
 			None => Err(msgs::TxAbort {
 				channel_id: self.channel_id(),
 				data: "We do not have an interactive transaction negotiation in progress".to_string().into_bytes()
@@ -3835,7 +3806,7 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider  {
 	-> Result<HandleTxCompleteValue, msgs::TxAbort> {
 		match self.interactive_tx_constructor {
 			Some(ref mut tx_constructor) => tx_constructor.handle_tx_complete(msg).map_err(
-				|reason| self.get_tx_abort_msg_from_abort_reason(reason)),
+				|reason| reason.into_tx_abort_msg(self.channel_id)),
 			None => Err(msgs::TxAbort {
 				channel_id: self.channel_id(),
 				data: "We do not have an interactive transaction negotiation in progress".to_string().into_bytes()

lightning/src/ln/channel_splice.rs

@@ -26,6 +26,8 @@ pub(crate) struct PendingSpliceInfoPre {
 	pub post_channel_id: Option<ChannelId>,
 	pub funding_feerate_perkw: u32,
 	pub locktime: u32,
+	/// The funding inputs we will be contributing to the splice.
+	pub our_funding_inputs: Vec<(TxIn, TransactionU16LenLimited)>,
 }
 
 /// Info about a pending splice, used in the post-splice channel
@@ -48,13 +50,15 @@ pub(crate) struct PendingSpliceInfoPost {
 impl PendingSpliceInfoPre {
 	pub(crate) fn new(relative_satoshis: i64, pre_channel_value: u64,
 		post_channel_id: Option<ChannelId>, funding_feerate_perkw: u32, locktime: u32,
+		our_funding_inputs: Vec<(TxIn, TransactionU16LenLimited)>,
 	) -> Self {
 		let post_channel_value = Self::add_checked(pre_channel_value, relative_satoshis);
 		Self {
 			post_channel_value,
 			post_channel_id,
 			funding_feerate_perkw,
 			locktime,
+			our_funding_inputs,
 		}
 	}
 

lightning/src/ln/channelmanager.rs

@@ -3491,7 +3491,12 @@ where
 	///   <------- splice_signed_ack ---    send signature on funding tx. In future this should be tx_signatures
 	///   [new funding tx can be broadcast]
 	#[cfg(splicing)]
-	pub fn splice_channel(&self, channel_id: &ChannelId, their_network_key: &PublicKey, relative_satoshis: i64, funding_feerate_perkw: u32, locktime: u32) -> Result<(), APIError> {
+	pub fn splice_channel(
+		&self, channel_id: &ChannelId, their_network_key: &PublicKey, relative_satoshis: i64,
+		funding_inputs: Vec<(TxIn, Transaction)>, funding_feerate_perkw: u32, locktime: u32
+	) -> Result<(), APIError> {
+		let funding_inputs = Self::length_limit_holder_input_prev_txs(funding_inputs)?;
+
 		let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self);
 		// We want to make sure the lock is actually acquired by PersistenceNotifierGuard.
 		debug_assert!(&self.total_consistency_lock.try_write().is_err());
@@ -3526,7 +3531,7 @@ where
 						return Err(APIError::ChannelUnavailable { err: format!("Channel has already a splice pending, channel id {}", channel_id) });
 					}
 
-					chan.context.pending_splice_pre = Some(PendingSpliceInfoPre::new(relative_satoshis, pre_channel_value, None, funding_feerate_perkw, locktime));
+					chan.context.pending_splice_pre = Some(PendingSpliceInfoPre::new(relative_satoshis, pre_channel_value, None, funding_feerate_perkw, locktime, funding_inputs));
 
 					// Check channel id
 					let post_splice_v2_channel_id = chan.context.generate_v2_channel_id_from_revocation_basepoints();
@@ -7334,6 +7339,17 @@ where
 			funding_satoshis)
 	}
 
+	#[cfg(any(dual_funding, splicing))]
+	fn length_limit_holder_input_prev_txs(funding_inputs: Vec<(TxIn, Transaction)>) -> Result<Vec<(TxIn, TransactionU16LenLimited)>, APIError> {
+		funding_inputs.into_iter().map(|(txin, tx)| {
+			match TransactionU16LenLimited::new(tx) {
+				Ok(tx) => Ok((txin, tx)),
+				Err(err) => Err(err)
+			}
+		}).collect::<Result<Vec<(TxIn, TransactionU16LenLimited)>, ()>>()
+		.map_err(|_| APIError::APIMisuseError { err: "One or more transactions had a serialized length exceeding 65535 bytes".into() })
+	}
+
 	// TODO(dual_funding): Remove param _-prefix once #[cfg(dual_funding)] is dropped.
 	fn do_accept_inbound_channel(
 		&self, temporary_channel_id: &ChannelId, counterparty_node_id: &PublicKey, accept_0conf: bool,
@@ -9313,7 +9329,7 @@ where
 				if let ChannelPhase::UnfundedInboundV2(post_chan) = chan_entry.get_mut() {
 					let pre_channel_value = post_chan.context.get_value_satoshis();
 
-					post_chan.context.pending_splice_pre = Some(PendingSpliceInfoPre::new(msg.relative_satoshis, pre_channel_value, Some(post_chan_id), msg.funding_feerate_perkw, msg.locktime));
+					post_chan.context.pending_splice_pre = Some(PendingSpliceInfoPre::new(msg.relative_satoshis, pre_channel_value, Some(post_chan_id), msg.funding_feerate_perkw, msg.locktime, Vec::new()));
 
 					// Apply start of splice changed in the state (update state, capacity funding tx, ...)
 					post_chan.context.splice_start(false, msg.relative_satoshis, &self.logger)
@@ -9421,8 +9437,8 @@ where
 			hash_map::Entry::Vacant(_) => return Err(MsgHandleErrInternal::send_err_msg_no_close("Internal consistency error".to_string(), post_chan_id)),
 			hash_map::Entry::Occupied(mut chan_entry) => {
 				if let ChannelPhase::UnfundedOutboundV2(post_chan) = chan_entry.get_mut() {
-					let pre_channel_value = post_chan.context.get_value_satoshis();
-					let post_channel_value = PendingSpliceInfoPre::add_checked(pre_channel_value, msg.relative_satoshis);
+					// let pre_channel_value = post_chan.context.get_value_satoshis();
+					// let post_channel_value = PendingSpliceInfoPre::add_checked(pre_channel_value, msg.relative_satoshis);
 
 					// Update pre-splice info with the new channel ID of the post channel
 					post_chan.context.pending_splice_pre.as_mut().unwrap().post_channel_id = Some(post_chan_id);
@@ -9431,6 +9447,7 @@ where
 					post_chan.context.splice_start(true, msg.relative_satoshis, &self.logger)
 						.map_err(|ce| MsgHandleErrInternal::send_err_msg_no_close(ce.to_string(), post_chan_id))?;
 
+					/* Node: SpliceAckedInputsContributionReady event is no longer used
 					// Prepare SpliceAckedInputsContributionReady event
 					let mut pending_events = self.pending_events.lock().unwrap();
 					pending_events.push_back((events::Event::SpliceAckedInputsContributionReady {
@@ -9441,6 +9458,22 @@ where
 						holder_funding_satoshis: if post_channel_value < pre_channel_value { 0 } else { post_channel_value.saturating_sub(pre_channel_value) },
 						counterparty_funding_satoshis: 0,
 					} , None));
+					*/
+					let tx_msg_opt = post_chan.begin_interactive_funding_tx_construction(&self.signer_provider,
+						&self.entropy_source, self.get_our_node_id(), pending_splice.our_funding_inputs)
+						.map_err(|e| MsgHandleErrInternal::from_chan_no_close(
+							ChannelError::Close(format!("V2 channel rejected due to sender error {:?}", e)), post_chan_id))?;
+					if let Some(tx_msg) = tx_msg_opt {
+						let msg_send_event = match tx_msg {
+							InteractiveTxMessageSend::TxAddInput(msg) => events::MessageSendEvent::SendTxAddInput {
+								node_id: *counterparty_node_id, msg },
+							InteractiveTxMessageSend::TxAddOutput(msg) => events::MessageSendEvent::SendTxAddOutput {
+								node_id: *counterparty_node_id, msg },
+							InteractiveTxMessageSend::TxComplete(msg) => events::MessageSendEvent::SendTxComplete {
+								node_id: *counterparty_node_id, msg },
+						};
+						peer_state.pending_msg_events.push(msg_send_event);
+					}
 				} else {
 					return Err(MsgHandleErrInternal::send_err_msg_no_close("Internal consistency error".to_string(), post_chan_id));
 				}

lightning/src/ln/functional_tests_splice.rs

@@ -680,7 +680,7 @@ fn test_v1_splice_in() {
 	let locktime = 0; // TODO
 
 	// Initiate splice-in (on node0)
-	let res_error = initiator_node.node.splice_channel(&channel_id2, &acceptor_node.node.get_our_node_id(), splice_in_sats as i64, funding_feerate_perkw, locktime);
+	let res_error = initiator_node.node.splice_channel(&channel_id2, &acceptor_node.node.get_our_node_id(), splice_in_sats as i64, Vec::new(), funding_feerate_perkw, locktime);
 	assert!(res_error.is_err());
 	assert_eq!(format!("{:?}", res_error.err().unwrap())[..53].to_string(), "Misuse error: Channel ID would change during splicing".to_string());
 
@@ -907,7 +907,9 @@ fn test_v2_splice_in() {
 	let locktime = 0; // TODO
 
 	// Initiate splice-in (on node0)
-	let _res = initiator_node.node.splice_channel(&channel_id1, &acceptor_node.node.get_our_node_id(), splice_in_sats as i64, funding_feerate_perkw, locktime).unwrap();
+	let extra_splice_funding_input_sats = 35_000;
+	let funding_inputs = vec![create_custom_dual_funding_input_with_pubkey(&initiator_node, extra_splice_funding_input_sats, &custom_input_pubkey)];
+	let _res = initiator_node.node.splice_channel(&channel_id1, &acceptor_node.node.get_our_node_id(), splice_in_sats as i64, funding_inputs, funding_feerate_perkw, locktime).unwrap();
 	// Extract the splice message from node0 to node1
 	let splice_msg = get_event_msg!(initiator_node, MessageSendEvent::SendSplice, acceptor_node.node.get_our_node_id());
 
@@ -930,19 +932,7 @@ fn test_v2_splice_in() {
 
 	let _res = initiator_node.node.handle_splice_ack(&acceptor_node.node.get_our_node_id(), &splice_ack_msg);
 
-	// Note: SpliceAckedInputsContributionReady emitted
-	let events = initiator_node.node.get_and_clear_pending_events();
-	assert_eq!(events.len(), 1);
-	match events[0] {
-		Event::SpliceAckedInputsContributionReady { channel_id, pre_channel_value_satoshis, post_channel_value_satoshis, holder_funding_satoshis, counterparty_funding_satoshis, .. } => {
-			assert_eq!(channel_id.to_string(), expected_funded_channel_id);
-			assert_eq!(pre_channel_value_satoshis, channel_value_sat);
-			assert_eq!(post_channel_value_satoshis, post_splice_channel_value);
-			assert_eq!(holder_funding_satoshis, post_splice_channel_value - channel_value_sat);
-			assert_eq!(counterparty_funding_satoshis, 0);
-		},
-		_ => panic!("SpliceAckedInputsContributionReady event missing {:?}", events[0]),
-	};
+	// Note: SpliceAckedInputsContributionReady event is no longer used
 
 	// check that capacity has been updated, channel is not usable, and funding tx is unset
 	assert_eq!(initiator_node.node.list_channels().len(), 1);
@@ -956,11 +946,8 @@ fn test_v2_splice_in() {
 		assert!(channel.funding_txo.is_none());
 		assert_eq!(channel.confirmations.unwrap(), 0);
 	}
-
-	let extra_splice_funding_input_sats = 35_000;
-	let funding_inputs = vec![create_custom_dual_funding_input_with_pubkey(&initiator_node, extra_splice_funding_input_sats, &custom_input_pubkey)];
 
-	let _res = initiator_node.node.contribute_funding_inputs(&channel_id1, &acceptor_node.node.get_our_node_id(), funding_inputs).unwrap();
+	// Note: contribute_funding_inputs() call is no longer used
 
 	// Initiator_node will generate first TxAddInput message
 	let tx_add_input_msg = get_event_msg!(&initiator_node, MessageSendEvent::SendTxAddInput, acceptor_node.node.get_our_node_id());
@@ -1467,7 +1454,9 @@ fn test_v2_payment_splice_in_payment() {
 	let locktime = 0; // TODO
 
 	// Initiate splice-in (on node0)
-	let _res = initiator_node.node.splice_channel(&channel_id1, &acceptor_node.node.get_our_node_id(), splice_in_sats as i64, funding_feerate_perkw, locktime).unwrap();
+	let extra_splice_funding_input_sats = 35_000;
+	let funding_inputs = vec![create_custom_dual_funding_input_with_pubkey(&initiator_node, extra_splice_funding_input_sats, &custom_input_pubkey)];
+	let _res = initiator_node.node.splice_channel(&channel_id1, &acceptor_node.node.get_our_node_id(), splice_in_sats as i64, funding_inputs, funding_feerate_perkw, locktime).unwrap();
 	// Extract the splice message from node0 to node1
 	let splice_msg = get_event_msg!(initiator_node, MessageSendEvent::SendSplice, acceptor_node.node.get_our_node_id());
 
@@ -1490,19 +1479,7 @@ fn test_v2_payment_splice_in_payment() {
 
 	let _res = initiator_node.node.handle_splice_ack(&acceptor_node.node.get_our_node_id(), &splice_ack_msg);
 
-	// Note: SpliceAckedInputsContributionReady emitted
-	let events = initiator_node.node.get_and_clear_pending_events();
-	assert_eq!(events.len(), 1);
-	match events[0] {
-		Event::SpliceAckedInputsContributionReady { channel_id, pre_channel_value_satoshis, post_channel_value_satoshis, holder_funding_satoshis, counterparty_funding_satoshis, .. } => {
-			assert_eq!(channel_id.to_string(), expected_funded_channel_id);
-			assert_eq!(pre_channel_value_satoshis, channel_value_sat);
-			assert_eq!(post_channel_value_satoshis, post_splice_channel_value);
-			assert_eq!(holder_funding_satoshis, post_splice_channel_value);
-			assert_eq!(counterparty_funding_satoshis, 0);
-		},
-		_ => panic!("SpliceAckedInputsContributionReady event missing {:?}", events[0]),
-	};
+	// Note: SpliceAckedInputsContributionReady event no longer used
 
 	// check that capacity has been updated, channel is not usable, and funding tx is unset
 	assert_eq!(initiator_node.node.list_channels().len(), 1);
@@ -1516,11 +1493,8 @@ fn test_v2_payment_splice_in_payment() {
 		assert!(channel.funding_txo.is_none());
 		assert_eq!(channel.confirmations.unwrap(), 0);
 	}
-
-	let extra_splice_funding_input_sats = 35_000;
-	let funding_inputs = vec![create_custom_dual_funding_input_with_pubkey(&initiator_node, extra_splice_funding_input_sats, &custom_input_pubkey)];
 
-	let _res = initiator_node.node.contribute_funding_inputs(&channel_id1, &acceptor_node.node.get_our_node_id(), funding_inputs).unwrap();
+	// Note: contribute_funding_inputs() call is no longer used
 
 	// Initiator_node will generate first TxAddInput message
 	let tx_add_input_msg = get_event_msg!(&initiator_node, MessageSendEvent::SendTxAddInput, acceptor_node.node.get_our_node_id());