Merge pull request #4516 from wpaulino/funding-contribution-builder

Introduce FundingContributionBuilder API

Author: jkczyz

fuzz/src/chanmon_consistency.rs

@@ -1512,7 +1512,6 @@ pub fn do_test<Out: Output + MaybeSend + MaybeSync>(data: &[u8], out: Out) {
 	                  counterparty_node_id: &PublicKey,
 	                  channel_id: &ChannelId,
 	                  wallet: &TestWalletSource,
-	                  logger: Arc<dyn Logger + MaybeSend + MaybeSync>,
 	                  funding_feerate_sat_per_kw: FeeRate| {
 		// We conditionally splice out `MAX_STD_OUTPUT_DUST_LIMIT_SATOSHIS` only when the node
 		// has double the balance required to send a payment upon a `0xff` byte. We do this to
@@ -1532,12 +1531,7 @@ pub fn do_test<Out: Output + MaybeSend + MaybeSync>(data: &[u8], out: Out) {
 				value: Amount::from_sat(MAX_STD_OUTPUT_DUST_LIMIT_SATOSHIS),
 				script_pubkey: wallet.get_change_script().unwrap(),
 			}];
-			funding_template.splice_out_sync(
-				outputs,
-				feerate,
-				FeeRate::MAX,
-				&WalletSync::new(wallet, logger.clone()),
-			)
+			funding_template.splice_out(outputs, feerate, FeeRate::MAX)
 		});
 	};
 
@@ -2479,39 +2473,35 @@ pub fn do_test<Out: Output + MaybeSend + MaybeSync>(data: &[u8], out: Out) {
 				}
 				let cp_node_id = nodes[1].get_our_node_id();
 				let wallet = &wallets[0];
-				let logger = Arc::clone(&loggers[0]);
 				let feerate_sat_per_kw = fee_estimators[0].feerate_sat_per_kw();
-				splice_out(&nodes[0], &cp_node_id, &chan_a_id, wallet, logger, feerate_sat_per_kw);
+				splice_out(&nodes[0], &cp_node_id, &chan_a_id, wallet, feerate_sat_per_kw);
 			},
 			0xa5 => {
 				if !cfg!(splicing) {
 					test_return!();
 				}
 				let cp_node_id = nodes[0].get_our_node_id();
 				let wallet = &wallets[1];
-				let logger = Arc::clone(&loggers[1]);
 				let feerate_sat_per_kw = fee_estimators[1].feerate_sat_per_kw();
-				splice_out(&nodes[1], &cp_node_id, &chan_a_id, wallet, logger, feerate_sat_per_kw);
+				splice_out(&nodes[1], &cp_node_id, &chan_a_id, wallet, feerate_sat_per_kw);
 			},
 			0xa6 => {
 				if !cfg!(splicing) {
 					test_return!();
 				}
 				let cp_node_id = nodes[2].get_our_node_id();
 				let wallet = &wallets[1];
-				let logger = Arc::clone(&loggers[1]);
 				let feerate_sat_per_kw = fee_estimators[1].feerate_sat_per_kw();
-				splice_out(&nodes[1], &cp_node_id, &chan_b_id, wallet, logger, feerate_sat_per_kw);
+				splice_out(&nodes[1], &cp_node_id, &chan_b_id, wallet, feerate_sat_per_kw);
 			},
 			0xa7 => {
 				if !cfg!(splicing) {
 					test_return!();
 				}
 				let cp_node_id = nodes[1].get_our_node_id();
 				let wallet = &wallets[2];
-				let logger = Arc::clone(&loggers[2]);
 				let feerate_sat_per_kw = fee_estimators[2].feerate_sat_per_kw();
-				splice_out(&nodes[2], &cp_node_id, &chan_b_id, wallet, logger, feerate_sat_per_kw);
+				splice_out(&nodes[2], &cp_node_id, &chan_b_id, wallet, feerate_sat_per_kw);
 			},
 
 			// Sync node by 1 block to cover confirmation of a transaction.

fuzz/src/full_stack.rs

@@ -1083,13 +1083,9 @@ pub fn do_test(mut data: &[u8], logger: &Arc<dyn Logger + MaybeSend + MaybeSync>
 						value: Amount::from_sat(splice_out_sats),
 						script_pubkey: wallet.get_change_script().unwrap(),
 					}];
-					let wallet_sync = WalletSync::new(&wallet, Arc::clone(&logger));
-					if let Ok(contribution) = funding_template.splice_out_sync(
-						outputs,
-						feerate,
-						FeeRate::MAX,
-						&wallet_sync,
-					) {
+					if let Ok(contribution) =
+						funding_template.splice_out(outputs, feerate, FeeRate::MAX)
+					{
 						let _ = channelmanager.funding_contributed(
 							&chan_id,
 							&counterparty,
@@ -1890,8 +1886,8 @@ fn splice_seed() -> Vec<u8> {
 	// CommitmentSigned message with proper signature (r=f7, s=01...) and funding_txid TLV
 	// signature r encodes sighash first byte f7, s follows the pattern from funding_created
 	// TLV type 1 (odd/optional) for funding_txid as per impl_writeable_msg!(CommitmentSigned, ...)
-	// Note: txid is encoded in reverse byte order (Bitcoin standard), so to get display 0000...0033, encode 3300...0000
-	ext_from_hex("0084 c000000000000000000000000000000000000000000000000000000000000000 00000000000000000000000000000000000000000000000000000000000000f7 0100000000000000000000000000000000000000000000000000000000000000 0000 01 20 3300000000000000000000000000000000000000000000000000000000000000 03000000000000000000000000000000", &mut test);
+	// Note: txid is encoded in reverse byte order (Bitcoin standard), so to get display 0000...0031, encode 3100...0000
+	ext_from_hex("0084 c000000000000000000000000000000000000000000000000000000000000000 00000000000000000000000000000000000000000000000000000000000000f7 0100000000000000000000000000000000000000000000000000000000000000 0000 01 20 3100000000000000000000000000000000000000000000000000000000000000 03000000000000000000000000000000", &mut test);
 
 	// After commitment_signed exchange, we need to exchange tx_signatures.
 	// Message type IDs: TxSignatures = 71 (0x0047)
@@ -1904,19 +1900,19 @@ fn splice_seed() -> Vec<u8> {
 	// inbound read from peer id 0 of len 150 (134 message + 16 MAC)
 	ext_from_hex("030096", &mut test);
 	// TxSignatures message with shared_input_signature TLV (type 0)
-	// txid must match the splice funding txid (0x33 in reverse byte order)
+	// txid must match the splice funding txid (0x31 in reverse byte order)
 	// shared_input_signature: 64-byte fuzz signature for the shared input
-	ext_from_hex("0047 c000000000000000000000000000000000000000000000000000000000000000 3300000000000000000000000000000000000000000000000000000000000000 0000 00 40 00000000000000000000000000000000000000000000000000000000000000dc 0100000000000000000000000000000000000000000000000000000000000000 03000000000000000000000000000000", &mut test);
+	ext_from_hex("0047 c000000000000000000000000000000000000000000000000000000000000000 3100000000000000000000000000000000000000000000000000000000000000 0000 00 40 00000000000000000000000000000000000000000000000000000000000000dc 0100000000000000000000000000000000000000000000000000000000000000 03000000000000000000000000000000", &mut test);
 
 	// Connect a block with the splice funding transaction to confirm it
 	// The splice funding tx: version(4) + input_count(1) + txid(32) + vout(4) + script_len(1) + sequence(4)
 	//                       + output_count(1) + value(8) + script_len(1) + script(34) + locktime(4) = 94 bytes = 0x5e
 	// Transaction structure from FundingTransactionReadyForSigning:
 	// - Input: spending c000...00:0 with sequence 0xfffffffd
-	// - Output: 115536 sats to OP_0 PUSH32 6e00...00
+	// - Output: 115538 sats to OP_0 PUSH32 6e00...00
 	// - Locktime: 13
 	ext_from_hex("0c005e", &mut test);
-	ext_from_hex("02000000 01 c000000000000000000000000000000000000000000000000000000000000000 00000000 00 fdffffff 01 50c3010000000000 22 00206e00000000000000000000000000000000000000000000000000000000000000 0d000000", &mut test);
+	ext_from_hex("02000000 01 c000000000000000000000000000000000000000000000000000000000000000 00000000 00 fdffffff 01 52c3010000000000 22 00206e00000000000000000000000000000000000000000000000000000000000000 0d000000", &mut test);
 
 	// Connect additional blocks to reach minimum_depth confirmations
 	for _ in 0..5 {
@@ -1933,8 +1929,8 @@ fn splice_seed() -> Vec<u8> {
 	// inbound read from peer id 0 of len 82 (66 message + 16 MAC)
 	ext_from_hex("030052", &mut test);
 	// SpliceLocked message (type 77 = 0x004d): channel_id + splice_txid + mac
-	// splice_txid must match the splice funding txid (0x33 in reverse byte order)
-	ext_from_hex("004d c000000000000000000000000000000000000000000000000000000000000000 3300000000000000000000000000000000000000000000000000000000000000 03000000000000000000000000000000", &mut test);
+	// splice_txid must match the splice funding txid (0x31 in reverse byte order)
+	ext_from_hex("004d c000000000000000000000000000000000000000000000000000000000000000 3100000000000000000000000000000000000000000000000000000000000000 03000000000000000000000000000000", &mut test);
 
 	test
 }
@@ -2064,6 +2060,6 @@ mod tests {
 
 		// Splice locked
 		assert_eq!(log_entries.get(&("lightning::ln::peer_handler".to_string(), "Handling SendSpliceLocked event in peer_handler for node 030000000000000000000000000000000000000000000000000000000000000002 for channel c000000000000000000000000000000000000000000000000000000000000000".to_string())), Some(&1));
-		assert_eq!(log_entries.get(&("lightning::ln::channel".to_string(), "Promoting splice funding txid 0000000000000000000000000000000000000000000000000000000000000033".to_string())), Some(&1));
+		assert_eq!(log_entries.get(&("lightning::ln::channel".to_string(), "Promoting splice funding txid 0000000000000000000000000000000000000000000000000000000000000031".to_string())), Some(&1));
 	}
 }

lightning/src/ln/channel.rs

@@ -12360,7 +12360,25 @@ where
 			);
 			let min_rbf_feerate = prev_feerate.map(min_rbf_feerate);
 			let prior = if pending_splice.last_funding_feerate_sat_per_1000_weight.is_some() {
-				self.build_prior_contribution()
+				if let Some(prior) = self
+					.pending_splice
+					.as_ref()
+					.and_then(|pending_splice| pending_splice.contributions.last())
+				{
+					let holder_balance = self
+						.get_holder_counterparty_balances_floor_incl_fee(&self.funding)
+						.map(|(h, _)| h)
+						.map_err(|e| APIError::ChannelUnavailable {
+							err: format!(
+								"Channel {} cannot be spliced at this time: {}",
+								self.context.channel_id(),
+								e
+							),
+						})?;
+					Some(PriorContribution::new(prior.clone(), holder_balance))
+				} else {
+					None
+				}
 			} else {
 				None
 			};
@@ -12382,21 +12400,6 @@ where
 		Ok(FundingTemplate::new(Some(shared_input), min_rbf_feerate, prior_contribution))
 	}
 
-	/// Clones the prior contribution and fetches the holder balance for deferred feerate
-	/// adjustment.
-	fn build_prior_contribution(&self) -> Option<PriorContribution> {
-		debug_assert!(
-			self.pending_splice.is_some(),
-			"build_prior_contribution requires pending_splice"
-		);
-		let prior = self.pending_splice.as_ref()?.contributions.last()?;
-		let holder_balance = self
-			.get_holder_counterparty_balances_floor_incl_fee(&self.funding)
-			.map(|(h, _)| h)
-			.ok();
-		Some(PriorContribution::new(prior.clone(), holder_balance))
-	}
-
 	/// Returns whether this channel can ever RBF, independent of splice state.
 	fn is_rbf_compatible(&self) -> Result<(), String> {
 		if self.context.minimum_depth(&self.funding) == Some(0) {
@@ -12568,14 +12571,12 @@ where
 			};
 		}
 
-		if let Err(e) = contribution.validate().and_then(|()| {
-			// For splice-out, our_funding_contribution is adjusted to cover fees if there
-			// aren't any inputs.
-			let our_funding_contribution = contribution.net_value();
+		let our_funding_contribution = contribution.net_value();
+
+		if let Err(e) =
 			self.validate_splice_contributions(our_funding_contribution, SignedAmount::ZERO)
-		}) {
+		{
 			log_error!(logger, "Channel {} cannot be funded: {}", self.context.channel_id(), e);
-
 			return Err(QuiescentError::FailSplice(self.splice_funding_failed_for(contribution)));
 		}
 
@@ -14138,13 +14139,11 @@ where
 					// funding_contributed and quiescence, reducing the holder's
 					// balance. If invalid, disconnect and return the contribution so
 					// the user can reclaim their inputs.
-					if let Err(e) = contribution.validate().and_then(|()| {
-						let our_funding_contribution = contribution.net_value();
-						self.validate_splice_contributions(
-							our_funding_contribution,
-							SignedAmount::ZERO,
-						)
-					}) {
+					let our_funding_contribution = contribution.net_value();
+					if let Err(e) = self.validate_splice_contributions(
+						our_funding_contribution,
+						SignedAmount::ZERO,
+					) {
 						let failed = self.splice_funding_failed_for(contribution);
 						return Err((
 							ChannelError::WarnAndDisconnect(format!(

lightning/src/ln/channelmanager.rs

@@ -6733,20 +6733,32 @@ impl<
 	/// The splice initiator is responsible for paying fees for common fields, shared inputs, and
 	/// shared outputs along with any contributed inputs and outputs. When building a
 	/// [`FundingContribution`], fees are estimated at `min_feerate` assuming initiator
-	/// responsibility and must be covered by the supplied inputs for splice-in or the channel
-	/// balance for splice-out. If the counterparty also initiates a splice and wins the
-	/// tie-break, they become the initiator and choose the feerate. The fee is then
-	/// re-estimated at the counterparty's feerate for only our contributed inputs and outputs,
-	/// which may be higher or lower than the original estimate. The contribution is dropped and
-	/// the splice proceeds without it when:
+	/// responsibility. Contributions fall into two cases:
+	/// - **input-backed contributions**: when wallet inputs are selected, those inputs pay for both
+	///   the requested value added to the channel and any explicit withdrawal outputs. For
+	///   example, a 60,000 sat input might add 50,000 sat to the channel, pay a 2,000 sat fee,
+	///   and return 8,000 sat as change. A later RBF first tries to preserve that 50,000 sat
+	///   value added and cover any higher fee or newly requested withdrawal from the original
+	///   10,000 sat fee buffer (2,000 sat fee + 8,000 sat change). If that buffer is not enough,
+	///   the prior contribution cannot be reused without selecting new wallet inputs.
+	/// - **input-less contributions**: when no wallet inputs are selected, fees and explicit
+	///   withdrawal outputs are paid from the channel balance. For example, a pure splice-out that
+	///   withdraws 20,000 sat from a 100,000 sat holder balance leaves up to 80,000 sat available
+	///   for fees. A later RBF keeps the 20,000 sat withdrawal only while that remaining balance
+	///   can still cover the re-estimated fee.
+	///
+	/// If the counterparty also initiates a splice and wins the tie-break, they become the
+	/// initiator and choose the feerate. The fee is then re-estimated at the counterparty's
+	/// feerate for only our contributed inputs and outputs, which may be higher or lower than the
+	/// original estimate. The contribution is dropped and the splice proceeds without it when:
 	/// - the counterparty's feerate is below `min_feerate`
 	/// - the counterparty's feerate is above `max_feerate` and the re-estimated fee exceeds the
 	///   original fee estimate
 	/// - the re-estimated fee exceeds the *fee buffer* regardless of `max_feerate`
 	///
 	/// The fee buffer is the maximum fee that can be accommodated:
-	/// - **splice-in**: the selected inputs' value minus the contributed amount
-	/// - **splice-out**: the channel balance minus the withdrawal outputs
+	/// - **input-backed contributions**: the original fee plus any change output value
+	/// - **input-less contributions**: the channel balance minus the withdrawal outputs
 	///
 	/// # Events
 	///