f: Build splice funding scopes after validate_splice_contributions

Author: tankyleo

lightning/src/ln/channel.rs

@@ -10960,18 +10960,9 @@ where
 		// Note: post-splice channel value is not yet known at this point, counterparty contribution is not known
 		// (Cannot test for miminum required post-splice channel value)
 		let their_funding_contribution = SignedAmount::ZERO;
-		let counterparty_public_key = self
-			.funding
-			.channel_transaction_parameters
-			.counterparty_parameters
-			.as_ref()
-			.expect("counterparty_parameters should be set")
-			.pubkeys
-			.funding_pubkey;
 		self.validate_splice_contributions(
 			adjusted_funding_contribution,
 			their_funding_contribution,
-			counterparty_public_key,
 		)
 		.map_err(|err| APIError::APIMisuseError { err })?;
 
@@ -11068,19 +11059,22 @@ where
 			)));
 		}
 
-		self.validate_splice_contributions(
+		self.validate_splice_contributions(our_funding_contribution, their_funding_contribution)
+			.map_err(|e| ChannelError::WarnAndDisconnect(e))?;
+
+		Ok(FundingScope::for_splice(
+			&self.funding,
+			&self.context,
 			our_funding_contribution,
 			their_funding_contribution,
 			msg.funding_pubkey,
-		)
-		.map_err(|e| ChannelError::WarnAndDisconnect(e))
+		))
 	}
 
 	#[cfg(splicing)]
 	fn validate_splice_contributions(
 		&self, our_funding_contribution: SignedAmount, their_funding_contribution: SignedAmount,
-		counterparty_funding_pubkey: PublicKey,
-	) -> Result<FundingScope, String> {
+	) -> Result<(), String> {
 		if our_funding_contribution.abs() > SignedAmount::MAX_MONEY {
 			return Err(format!(
 				"Channel {} cannot be spliced; our {} contribution exceeds the total bitcoin supply",
@@ -11097,77 +11091,76 @@ where
 			));
 		}
 
-		// Sanity check all funding contributions here; we need to do this before building a `FundingScope`
+		let (holder_balance_remaining, counterparty_balance_remaining) =
+			self.get_holder_counterparty_balances_floor_incl_fee(&self.funding).map_err(|e| {
+				format!("Channel {} cannot be spliced; {}", self.context.channel_id(), e)
+			})?;
 
-		let our_channel_balance = Amount::from_sat(self.funding.get_value_to_self_msat() / 1000);
-		AddSigned::checked_add_signed(
-			our_channel_balance.to_sat(),
+		let post_channel_value = self.funding.compute_post_splice_value(
 			our_funding_contribution.to_sat(),
-		)
-		.ok_or(format!(
-				"Channel {} cannot be spliced out; our {} contribution exhausts our channel balance: {}",
-				self.context.channel_id(),
-				our_funding_contribution,
-				our_channel_balance,
-			))?;
-
-		let their_channel_balance = Amount::from_sat(
-			self.funding.get_value_satoshis() - self.funding.get_value_to_self_msat() / 1000,
-		);
-		AddSigned::checked_add_signed(
-			their_channel_balance.to_sat(),
 			their_funding_contribution.to_sat(),
-		)
-		.ok_or(format!(
-				"Channel {} cannot be spliced out; their {} contribution exhausts their channel balance: {}",
-				self.context.channel_id(),
-				their_funding_contribution,
-				their_channel_balance,
-			))?;
-
-		let splice_funding = FundingScope::for_splice(
-			&self.funding,
-			&self.context,
-			our_funding_contribution,
-			their_funding_contribution,
-			counterparty_funding_pubkey,
 		);
-
-		let (holder_balance_remaining, counterparty_balance_remaining) =
-			self.get_holder_counterparty_balances_floor_incl_fee(&splice_funding).map_err(|e| {
-				format!("Channel {} cannot be spliced; {}", self.context.channel_id(), e)
-			})?;
+		let counterparty_selected_channel_reserve = Amount::from_sat(
+			get_v2_channel_reserve_satoshis(post_channel_value, MIN_CHAN_DUST_LIMIT_SATOSHIS),
+		);
+		let holder_selected_channel_reserve = Amount::from_sat(get_v2_channel_reserve_satoshis(
+			post_channel_value,
+			self.context.counterparty_dust_limit_satoshis,
+		));
 
 		// We allow parties to draw from their previous reserve, as long as they satisfy their v2 reserve
 
 		if our_funding_contribution != SignedAmount::ZERO {
-			let counterparty_selected_channel_reserve_satoshis = splice_funding
-				.counterparty_selected_channel_reserve_satoshis
-				.expect("counterparty_selected_channel_reserve_satoshis should be set");
-			holder_balance_remaining
-				.checked_sub(Amount::from_sat(counterparty_selected_channel_reserve_satoshis))
+			let post_splice_holder_balance = Amount::from_sat(
+				AddSigned::checked_add_signed(
+					holder_balance_remaining.to_sat(),
+					our_funding_contribution.to_sat(),
+				)
+				.ok_or(format!(
+					"Channel {} cannot be {}; our remaining balance {} does not cover our negative funding contribution {}",
+					self.context.channel_id(),
+					if our_funding_contribution.is_positive() { "spliced in" } else { "spliced out" },
+					holder_balance_remaining,
+					our_funding_contribution,
+				))?,
+			);
+
+			post_splice_holder_balance.checked_sub(counterparty_selected_channel_reserve)
 				.ok_or(format!(
-						"Channel {} cannot be {}; We cannot afford the new counterparty mandated reserve {} vs {}",
+						"Channel {} cannot be {}; Our post-splice channel balance {} is smaller than their selected v2 reserve {}",
 						self.context.channel_id(),
 						if our_funding_contribution.is_positive() { "spliced in" } else { "spliced out" },
-						holder_balance_remaining, counterparty_selected_channel_reserve_satoshis,
+						post_splice_holder_balance,
+						counterparty_selected_channel_reserve,
 					))?;
 		}
 
 		if their_funding_contribution != SignedAmount::ZERO {
-			let holder_selected_channel_reserve_satoshis =
-				splice_funding.holder_selected_channel_reserve_satoshis;
-			counterparty_balance_remaining
-				.checked_sub(Amount::from_sat(holder_selected_channel_reserve_satoshis))
+			let post_splice_counterparty_balance = Amount::from_sat(
+				AddSigned::checked_add_signed(
+					counterparty_balance_remaining.to_sat(),
+					their_funding_contribution.to_sat(),
+				)
+				.ok_or(format!(
+					"Channel {} cannot be {}; their remaining balance {} does not cover their negative funding contribution {}",
+					self.context.channel_id(),
+					if their_funding_contribution.is_positive() { "spliced in" } else { "spliced out" },
+					counterparty_balance_remaining,
+					their_funding_contribution,
+				))?,
+			);
+
+			post_splice_counterparty_balance.checked_sub(holder_selected_channel_reserve)
 				.ok_or(format!(
-						"Channel {} cannot be {}; They cannot afford the new holder mandated reserve {} vs {}",
+						"Channel {} cannot be {}; Their post-splice channel balance {} is smaller than our selected v2 reserve {}",
 						self.context.channel_id(),
 						if their_funding_contribution.is_positive() { "spliced in" } else { "spliced out" },
-						counterparty_balance_remaining, holder_selected_channel_reserve_satoshis,
+						post_splice_counterparty_balance,
+						holder_selected_channel_reserve,
 					))?;
 		}
 
-		Ok(splice_funding)
+		Ok(())
 	}
 
 	#[cfg(splicing)]
@@ -11320,12 +11313,16 @@ where
 
 		let our_funding_contribution = funding_negotiation_context.our_funding_contribution;
 		let their_funding_contribution = SignedAmount::from_sat(msg.funding_contribution_satoshis);
-		self.validate_splice_contributions(
+		self.validate_splice_contributions(our_funding_contribution, their_funding_contribution)
+			.map_err(|e| ChannelError::WarnAndDisconnect(e))?;
+
+		Ok(FundingScope::for_splice(
+			&self.funding,
+			&self.context,
 			our_funding_contribution,
 			their_funding_contribution,
 			msg.funding_pubkey,
-		)
-		.map_err(|e| ChannelError::WarnAndDisconnect(e))
+		))
 	}
 
 	#[cfg(splicing)]