Drop creating new funding pubkey, new signer (from review)

Author: optout21

lightning/src/ln/channel.rs

@@ -3846,25 +3846,13 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
 
 	/// Get the splice message that can be sent during splice initiation.
 	#[cfg(splicing)]
-	pub fn get_splice_init(&self, our_funding_contribution_satoshis: i64, signer_provider: &SP,
+	pub fn get_splice_init(&self, our_funding_contribution_satoshis: i64,
 		funding_feerate_perkw: u32, locktime: u32,
 	) -> msgs::SpliceInit {
-		// 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.
+		// Reuse the existing funding pubkey, in spite of the channel value changing
+		// (though at this point we don't know the new value yet, due tue the optional counterparty contribution)
 		// Note that channel_keys_id is supposed NOT to change
-		let funding_pubkey = {
-			// TODO: Funding pubkey generation requires the post channel value, but that is not known yet,
-			// the acceptor contribution is missing. There is a need for a way to generate a new funding pubkey,
-			// not based on the channel value
-			let pre_channel_value = self.channel_value_satoshis;
-			let placeholder_counterparty_contribution = 0;
-			let incomplete_post_splice_channel_value = PendingSpliceInfoPre::compute_post_value(pre_channel_value,
-				our_funding_contribution_satoshis, placeholder_counterparty_contribution);
-			let holder_signer = signer_provider.derive_channel_signer(incomplete_post_splice_channel_value, self.channel_keys_id);
-			holder_signer.pubkeys().funding_pubkey
-		};
-
+		let funding_pubkey = self.get_holder_pubkeys().funding_pubkey.clone();
 		msgs::SpliceInit {
 			channel_id: self.channel_id,
 			funding_contribution_satoshis: our_funding_contribution_satoshis,
@@ -3880,7 +3868,7 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
 	pub fn get_splice_ack(&mut self, our_funding_contribution_satoshis: i64) -> Result<msgs::SpliceAck, ChannelError> {
 		// TODO(splicing): checks
 
-		// Note: at this point keys are already updated
+		// Reuse the existing funding pubkey, in spite of the channel value changing
 		let funding_pubkey = self.get_holder_pubkeys().funding_pubkey;
 		Ok(msgs::SpliceAck {
 			channel_id: self.channel_id,

lightning/src/ln/channelmanager.rs

@@ -4160,7 +4160,7 @@ where
 						our_funding_contribution: our_funding_contribution_satoshis,
 					});
 
-					let msg = chan.context.get_splice_init(our_funding_contribution_satoshis, &self.signer_provider, funding_feerate_perkw, locktime);
+					let msg = chan.context.get_splice_init(our_funding_contribution_satoshis, funding_feerate_perkw, locktime);
 
 					peer_state.pending_msg_events.push(events::MessageSendEvent::SendSpliceInit {
 						node_id: *counterparty_node_id,

lightning/src/ln/functional_tests_splice.rs

@@ -63,6 +63,12 @@ fn test_v1_splice_in() {
 		MessageSendEvent::SendOpenChannel,
 		acceptor_node.node.get_our_node_id()
 	);
+	let expected_initiator_funding_key =
+		"03c21e841cbc0b48197d060c71e116c185fa0ac281b7d0aa5924f535154437ca3b";
+	assert_eq!(
+		open_channel_message.common_fields.funding_pubkey.to_string(),
+		expected_initiator_funding_key
+	);
 
 	let _res = acceptor_node
 		.node
@@ -73,6 +79,15 @@ fn test_v1_splice_in() {
 		MessageSendEvent::SendAcceptChannel,
 		initiator_node.node.get_our_node_id()
 	);
+	let expected_acceptor_funding_key = "039481c28b904cbe12681e79937373fc76245c1b29871028ae60ba3152162c319b";
+	assert_eq!(accept_channel_message.common_fields.funding_pubkey.to_string(), expected_acceptor_funding_key);
+	let expected_acceptor_funding_key =
+		"039481c28b904cbe12681e79937373fc76245c1b29871028ae60ba3152162c319b";
+	assert_eq!(
+		accept_channel_message.common_fields.funding_pubkey.to_string(),
+		expected_acceptor_funding_key
+	);
+
 	let _res = initiator_node.node.handle_accept_channel(
 		acceptor_node.node.get_our_node_id(),
 		&accept_channel_message.clone(),
@@ -229,20 +244,28 @@ fn test_v1_splice_in() {
 		)
 		.unwrap();
 	// Extract the splice message from node0 to node1
-	let splice_msg = get_event_msg!(
+	let splice_init_msg = get_event_msg!(
 		initiator_node,
 		MessageSendEvent::SendSpliceInit,
 		acceptor_node.node.get_our_node_id()
 	);
+	assert_eq!(splice_init_msg.funding_contribution_satoshis, splice_in_sats as i64);
+	assert_eq!(splice_init_msg.funding_feerate_perkw, funding_feerate_perkw);
+	assert_eq!(splice_init_msg.funding_pubkey.to_string(), expected_initiator_funding_key);
+	assert!(splice_init_msg.require_confirmed_inputs.is_none());
 
-	let _res =
-		acceptor_node.node.handle_splice_init(initiator_node.node.get_our_node_id(), &splice_msg);
+	let _res = acceptor_node
+		.node
+		.handle_splice_init(initiator_node.node.get_our_node_id(), &splice_init_msg);
 	// Extract the splice_ack message from node1 to node0
 	let splice_ack_msg = get_event_msg!(
 		acceptor_node,
 		MessageSendEvent::SendSpliceAck,
 		initiator_node.node.get_our_node_id()
 	);
+	assert_eq!(splice_ack_msg.funding_contribution_satoshis, 0);
+	assert_eq!(splice_ack_msg.funding_pubkey.to_string(), expected_acceptor_funding_key);
+	assert!(splice_ack_msg.require_confirmed_inputs.is_none());
 
 	// still pre-splice channel: capacity not updated, channel usable, and funding tx set
 	assert_eq!(acceptor_node.node.list_channels().len(), 1);