f - Defer resolve_queued_contribution until after validation

validate_splice_init and validate_tx_init_rbf check preconditions
including quiescence. Move them before resolve_queued_contribution
so that a misbehaving peer sending splice_init or tx_init_rbf before
quiescence is rejected early. This also moves validate_splice_contributions
and FundingScope construction into the callers since they depend on the
resolved contribution.

Have validate_tx_init_rbf return the last candidate's funding pubkeys
so the caller can construct FundingScope without re-accessing
pending_splice.

Add a debug_assert in quiescent_negotiation_err that Abort errors only
occur when the channel is quiescent, since exit_quiescence requires it.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: jkczyz

lightning/src/ln/channel.rs

@@ -12686,9 +12686,7 @@ where
 	}
 
 	/// Checks during handling splice_init
-	pub fn validate_splice_init(
-		&self, msg: &msgs::SpliceInit, our_funding_contribution: SignedAmount,
-	) -> Result<FundingScope, ChannelError> {
+	pub fn validate_splice_init(&self, msg: &msgs::SpliceInit) -> Result<(), ChannelError> {
 		if self.holder_commitment_point.current_point().is_none() {
 			return Err(ChannelError::WarnAndDisconnect(format!(
 				"Channel {} commitment point needs to be advanced once before spliced",
@@ -12725,32 +12723,7 @@ where
 			)));
 		}
 
-		self.validate_splice_contributions(our_funding_contribution, their_funding_contribution)
-			.map_err(|e| ChannelError::WarnAndDisconnect(e))?;
-
-		// Rotate the pubkeys using the prev_funding_txid as a tweak
-		let prev_funding_txid = self.funding.get_funding_txid();
-		let funding_pubkey = match prev_funding_txid {
-			None => {
-				debug_assert!(false);
-				self.funding.get_holder_pubkeys().funding_pubkey
-			},
-			Some(prev_funding_txid) => self
-				.context
-				.holder_signer
-				.new_funding_pubkey(prev_funding_txid, &self.context.secp_ctx),
-		};
-		let mut new_keys = self.funding.get_holder_pubkeys().clone();
-		new_keys.funding_pubkey = funding_pubkey;
-
-		Ok(FundingScope::for_splice(
-			&self.funding,
-			&self.context,
-			our_funding_contribution,
-			their_funding_contribution,
-			msg.funding_pubkey,
-			new_keys,
-		))
+		Ok(())
 	}
 
 	fn validate_splice_contributions(
@@ -12891,18 +12864,45 @@ where
 		&mut self, msg: &msgs::SpliceInit, entropy_source: &ES, holder_node_id: &PublicKey,
 		logger: &L,
 	) -> Result<msgs::SpliceAck, InteractiveTxMsgError> {
+		self.validate_splice_init(msg).map_err(|e| self.quiescent_negotiation_err(e))?;
+
 		let feerate = FeeRate::from_sat_per_kwu(msg.funding_feerate_per_kw as u64);
-		let (our_funding_contribution, holder_balance) = self
+		let (queued_net_value, holder_balance) = self
 			.resolve_queued_contribution(feerate, logger)
 			.map_err(|e| self.quiescent_negotiation_err(e))?;
 
-		let splice_funding = self
-			.validate_splice_init(msg, our_funding_contribution.unwrap_or(SignedAmount::ZERO))
-			.map_err(|e| self.quiescent_negotiation_err(e))?;
+		let our_funding_contribution = queued_net_value.unwrap_or(SignedAmount::ZERO);
+		let their_funding_contribution = SignedAmount::from_sat(msg.funding_contribution_satoshis);
+		self.validate_splice_contributions(our_funding_contribution, their_funding_contribution)
+			.map_err(|e| self.quiescent_negotiation_err(ChannelError::WarnAndDisconnect(e)))?;
+
+		// Rotate the pubkeys using the prev_funding_txid as a tweak
+		let prev_funding_txid = self.funding.get_funding_txid();
+		let funding_pubkey = match prev_funding_txid {
+			None => {
+				debug_assert!(false);
+				self.funding.get_holder_pubkeys().funding_pubkey
+			},
+			Some(prev_funding_txid) => self
+				.context
+				.holder_signer
+				.new_funding_pubkey(prev_funding_txid, &self.context.secp_ctx),
+		};
+		let mut holder_pubkeys = self.funding.get_holder_pubkeys().clone();
+		holder_pubkeys.funding_pubkey = funding_pubkey;
+
+		let splice_funding = FundingScope::for_splice(
+			&self.funding,
+			&self.context,
+			our_funding_contribution,
+			their_funding_contribution,
+			msg.funding_pubkey,
+			holder_pubkeys,
+		);
 
 		// Adjust for the feerate and clone so we can store it for future RBF re-use.
 		let (adjusted_contribution, our_funding_inputs, our_funding_outputs) =
-			if our_funding_contribution.is_some() {
+			if queued_net_value.is_some() {
 				let adjusted_contribution = self
 					.take_queued_funding_contribution()
 					.expect("queued_funding_contribution was Some")
@@ -12913,7 +12913,6 @@ where
 			} else {
 				(None, Default::default(), Default::default())
 			};
-		let our_funding_contribution = our_funding_contribution.unwrap_or(SignedAmount::ZERO);
 
 		log_info!(
 			logger,
@@ -12956,9 +12955,8 @@ where
 
 	/// Checks during handling tx_init_rbf for an existing splice
 	fn validate_tx_init_rbf<F: FeeEstimator>(
-		&self, msg: &msgs::TxInitRbf, our_funding_contribution: SignedAmount,
-		fee_estimator: &LowerBoundedFeeEstimator<F>,
-	) -> Result<FundingScope, ChannelError> {
+		&self, msg: &msgs::TxInitRbf, fee_estimator: &LowerBoundedFeeEstimator<F>,
+	) -> Result<(ChannelPublicKeys, PublicKey), ChannelError> {
 		if self.holder_commitment_point.current_point().is_none() {
 			return Err(ChannelError::WarnAndDisconnect(format!(
 				"Channel {} commitment point needs to be advanced once before RBF",
@@ -13024,33 +13022,22 @@ where
 			return Err(ChannelError::Abort(AbortReason::InsufficientRbfFeerate));
 		}
 
-		let their_funding_contribution = match msg.funding_output_contribution {
-			Some(value) => SignedAmount::from_sat(value),
-			None => SignedAmount::ZERO,
-		};
-
-		self.validate_splice_contributions(our_funding_contribution, their_funding_contribution)
-			.map_err(|e| ChannelError::WarnAndDisconnect(e))?;
-
 		// Reuse funding pubkeys from the last negotiated candidate since all RBF candidates
 		// for the same splice share the same funding output script.
-		let holder_pubkeys = last_candidate.get_holder_pubkeys().clone();
-		let counterparty_funding_pubkey = *last_candidate.counterparty_funding_pubkey();
-
-		Ok(FundingScope::for_splice(
-			&self.funding,
-			&self.context,
-			our_funding_contribution,
-			their_funding_contribution,
-			counterparty_funding_pubkey,
-			holder_pubkeys,
+		Ok((
+			last_candidate.get_holder_pubkeys().clone(),
+			*last_candidate.counterparty_funding_pubkey(),
 		))
 	}
 
 	pub(crate) fn tx_init_rbf<ES: EntropySource, F: FeeEstimator, L: Logger>(
 		&mut self, msg: &msgs::TxInitRbf, entropy_source: &ES, holder_node_id: &PublicKey,
 		fee_estimator: &LowerBoundedFeeEstimator<F>, logger: &L,
 	) -> Result<msgs::TxAckRbf, InteractiveTxMsgError> {
+		let (holder_pubkeys, counterparty_funding_pubkey) = self
+			.validate_tx_init_rbf(msg, fee_estimator)
+			.map_err(|e| self.quiescent_negotiation_err(e))?;
+
 		let feerate = FeeRate::from_sat_per_kwu(msg.feerate_sat_per_1000_weight as u64);
 		let (queued_net_value, holder_balance) = self
 			.resolve_queued_contribution(feerate, logger)
@@ -13079,14 +13066,23 @@ where
 		};
 
 		let our_funding_contribution = queued_net_value.or(prior_net_value);
+		let our_funding_contribution = our_funding_contribution.unwrap_or(SignedAmount::ZERO);
 
-		let rbf_funding = self
-			.validate_tx_init_rbf(
-				msg,
-				our_funding_contribution.unwrap_or(SignedAmount::ZERO),
-				fee_estimator,
-			)
-			.map_err(|e| self.quiescent_negotiation_err(e))?;
+		let their_funding_contribution = match msg.funding_output_contribution {
+			Some(value) => SignedAmount::from_sat(value),
+			None => SignedAmount::ZERO,
+		};
+		self.validate_splice_contributions(our_funding_contribution, their_funding_contribution)
+			.map_err(|e| self.quiescent_negotiation_err(ChannelError::WarnAndDisconnect(e)))?;
+
+		let rbf_funding = FundingScope::for_splice(
+			&self.funding,
+			&self.context,
+			our_funding_contribution,
+			their_funding_contribution,
+			counterparty_funding_pubkey,
+			holder_pubkeys,
+		);
 
 		// Consume the appropriate contribution source.
 		let (our_funding_inputs, our_funding_outputs) = if queued_net_value.is_some() {
@@ -13123,8 +13119,6 @@ where
 			Default::default()
 		};
 
-		let our_funding_contribution = our_funding_contribution.unwrap_or(SignedAmount::ZERO);
-
 		log_info!(
 			logger,
 			"Starting RBF funding negotiation for channel {} after receiving tx_init_rbf; channel value: {} sats",
@@ -14256,8 +14250,12 @@ where
 	}
 
 	fn quiescent_negotiation_err(&mut self, err: ChannelError) -> InteractiveTxMsgError {
-		let exited_quiescence =
-			if matches!(err, ChannelError::Abort(_)) { self.exit_quiescence() } else { false };
+		let exited_quiescence = if matches!(err, ChannelError::Abort(_)) {
+			debug_assert!(self.context.channel_state.is_quiescent());
+			self.exit_quiescence()
+		} else {
+			false
+		};
 		InteractiveTxMsgError { err, splice_funding_failed: None, exited_quiescence }
 	}
 

lightning/src/ln/splicing_tests.rs

@@ -6583,6 +6583,96 @@ fn test_splice_revalidation_at_quiescence() {
 	expect_splice_failed_events(&nodes[0], &channel_id, contribution);
 }
 
+#[test]
+fn test_splice_init_before_quiescence_sends_warning() {
+	// A misbehaving peer sends splice_init before quiescence is established. The receiver
+	// should send a warning and disconnect.
+	let chanmon_cfgs = create_chanmon_cfgs(2);
+	let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
+	let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
+	let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
+
+	let node_id_1 = nodes[1].node.get_our_node_id();
+
+	let initial_channel_value_sat = 100_000;
+	let (_, _, channel_id, _) =
+		create_announced_chan_between_nodes_with_value(&nodes, 0, 1, initial_channel_value_sat, 0);
+
+	// Node 0 initiates quiescence.
+	nodes[0].node.maybe_propose_quiescence(&node_id_1, &channel_id).unwrap();
+	let _stfu = get_event_msg!(nodes[0], MessageSendEvent::SendStfu, node_id_1);
+
+	// Misbehaving node 1 sends splice_init before completing the STFU handshake.
+	let funding_pubkey =
+		PublicKey::from_secret_key(&Secp256k1::new(), &SecretKey::from_slice(&[42; 32]).unwrap());
+	let splice_init = msgs::SpliceInit {
+		channel_id,
+		funding_contribution_satoshis: 50_000,
+		funding_feerate_per_kw: FEERATE_FLOOR_SATS_PER_KW,
+		locktime: 0,
+		funding_pubkey,
+		require_confirmed_inputs: None,
+	};
+	nodes[0].node.handle_splice_init(node_id_1, &splice_init);
+
+	// Node 0 should send a warning and disconnect.
+	let msg_events = nodes[0].node.get_and_clear_pending_msg_events();
+	assert_eq!(msg_events.len(), 1);
+	match &msg_events[0] {
+		MessageSendEvent::HandleError { node_id, .. } => assert_eq!(*node_id, node_id_1),
+		other => panic!("Expected HandleError, got {:?}", other),
+	}
+}
+
+#[test]
+fn test_tx_init_rbf_before_quiescence_sends_warning() {
+	// A misbehaving peer sends tx_init_rbf before quiescence is established. The receiver
+	// should send a warning and disconnect.
+	let chanmon_cfgs = create_chanmon_cfgs(2);
+	let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
+	let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
+	let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
+
+	let node_id_1 = nodes[1].node.get_our_node_id();
+
+	let initial_channel_value_sat = 100_000;
+	let (_, _, channel_id, _) =
+		create_announced_chan_between_nodes_with_value(&nodes, 0, 1, initial_channel_value_sat, 0);
+
+	let added_value = Amount::from_sat(50_000);
+	provide_utxo_reserves(&nodes, 2, added_value * 2);
+
+	// Complete a splice-in so there's a pending splice to RBF.
+	let funding_contribution = do_initiate_splice_in(&nodes[0], &nodes[1], channel_id, added_value);
+	let (_splice_tx, _new_funding_script) =
+		splice_channel(&nodes[0], &nodes[1], channel_id, funding_contribution);
+
+	// Node 0 initiates quiescence.
+	nodes[0].node.maybe_propose_quiescence(&node_id_1, &channel_id).unwrap();
+	let _stfu = get_event_msg!(nodes[0], MessageSendEvent::SendStfu, node_id_1);
+
+	// Misbehaving node 1 sends tx_init_rbf before completing the STFU handshake.
+	let tx_init_rbf = msgs::TxInitRbf {
+		channel_id,
+		locktime: 0,
+		feerate_sat_per_1000_weight: FEERATE_FLOOR_SATS_PER_KW + 25,
+		funding_output_contribution: Some(added_value.to_sat() as i64),
+	};
+	nodes[0].node.handle_tx_init_rbf(node_id_1, &tx_init_rbf);
+
+	// Node 0 should send a warning and disconnect.
+	let msg_events = nodes[0].node.get_and_clear_pending_msg_events();
+	assert_eq!(msg_events.len(), 1);
+	match &msg_events[0] {
+		MessageSendEvent::HandleError { node_id, .. } => assert_eq!(*node_id, node_id_1),
+		other => panic!("Expected HandleError, got {:?}", other),
+	}
+
+	// Clean up events from the splice setup.
+	nodes[0].node.get_and_clear_pending_events();
+	nodes[1].node.get_and_clear_pending_events();
+}
+
 #[test]
 fn test_splice_rbf_rejects_low_feerate_after_several_attempts() {
 	// After several RBF attempts, the counterparty's RBF feerate must be high enough to