Merge pull request #4060 from wpaulino/splice-misc-fixes

Fix minor miscellaneous splicing issues

Author: TheBlueMatt

Cargo.toml

@@ -65,7 +65,6 @@ check-cfg = [
     "cfg(ldk_test_vectors)",
     "cfg(taproot)",
     "cfg(require_route_graph_test)",
-    "cfg(splicing)",
     "cfg(simple_close)",
     "cfg(peer_storage)",
 ]

ci/ci-tests.sh

@@ -151,8 +151,6 @@ fi
 echo -e "\n\nTest cfg-flag builds"
 RUSTFLAGS="--cfg=taproot" cargo test --verbose --color always -p lightning
 [ "$CI_MINIMIZE_DISK_USAGE" != "" ] && cargo clean
-RUSTFLAGS="--cfg=splicing" cargo test --verbose --color always -p lightning
-[ "$CI_MINIMIZE_DISK_USAGE" != "" ] && cargo clean
 RUSTFLAGS="--cfg=async_payments" cargo test --verbose --color always -p lightning
 [ "$CI_MINIMIZE_DISK_USAGE" != "" ] && cargo clean
 RUSTFLAGS="--cfg=simple_close" cargo test --verbose --color always -p lightning

lightning-net-tokio/src/lib.rs

@@ -742,11 +742,8 @@ mod tests {
 		fn handle_open_channel_v2(&self, _their_node_id: PublicKey, _msg: &OpenChannelV2) {}
 		fn handle_accept_channel_v2(&self, _their_node_id: PublicKey, _msg: &AcceptChannelV2) {}
 		fn handle_stfu(&self, _their_node_id: PublicKey, _msg: &Stfu) {}
-		#[cfg(splicing)]
 		fn handle_splice_init(&self, _their_node_id: PublicKey, _msg: &SpliceInit) {}
-		#[cfg(splicing)]
 		fn handle_splice_ack(&self, _their_node_id: PublicKey, _msg: &SpliceAck) {}
-		#[cfg(splicing)]
 		fn handle_splice_locked(&self, _their_node_id: PublicKey, _msg: &SpliceLocked) {}
 		fn handle_tx_add_input(&self, _their_node_id: PublicKey, _msg: &TxAddInput) {}
 		fn handle_tx_add_output(&self, _their_node_id: PublicKey, _msg: &TxAddOutput) {}

lightning/src/chain/channelmonitor.rs

@@ -3399,26 +3399,35 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 
 		// Prune HTLCs from the previous counterparty commitment tx so we don't generate failure/fulfill
 		// events for now-revoked/fulfilled HTLCs.
-		if let Some(txid) = self.funding.prev_counterparty_commitment_txid.take() {
-			if self.funding.current_counterparty_commitment_txid.unwrap() != txid {
-				let cur_claimables = self.funding.counterparty_claimable_outpoints.get(
-					&self.funding.current_counterparty_commitment_txid.unwrap()).unwrap();
-				for (_, ref source_opt) in self.funding.counterparty_claimable_outpoints.get(&txid).unwrap() {
-					if let Some(source) = source_opt {
-						if !cur_claimables.iter()
-							.any(|(_, cur_source_opt)| cur_source_opt == source_opt)
-						{
-							self.counterparty_fulfilled_htlcs.remove(&SentHTLCId::from_source(source));
+		let mut removed_fulfilled_htlcs = false;
+		let prune_htlc_sources = |funding: &mut FundingScope| {
+			if let Some(txid) = funding.prev_counterparty_commitment_txid.take() {
+				if funding.current_counterparty_commitment_txid.unwrap() != txid {
+					let cur_claimables = funding.counterparty_claimable_outpoints.get(
+						&funding.current_counterparty_commitment_txid.unwrap()).unwrap();
+					// We only need to remove fulfilled HTLCs once for the first `FundingScope` we
+					// come across since all `FundingScope`s share the same set of HTLC sources.
+					if !removed_fulfilled_htlcs {
+						for (_, ref source_opt) in funding.counterparty_claimable_outpoints.get(&txid).unwrap() {
+							if let Some(source) = source_opt {
+								if !cur_claimables.iter()
+									.any(|(_, cur_source_opt)| cur_source_opt == source_opt)
+								{
+									self.counterparty_fulfilled_htlcs.remove(&SentHTLCId::from_source(source));
+								}
+							}
 						}
+						removed_fulfilled_htlcs = true;
 					}
+					for &mut (_, ref mut source_opt) in funding.counterparty_claimable_outpoints.get_mut(&txid).unwrap() {
+						*source_opt = None;
+					}
+				} else {
+					assert!(cfg!(fuzzing), "Commitment txids are unique outside of fuzzing, where hashes can collide");
 				}
-				for &mut (_, ref mut source_opt) in self.funding.counterparty_claimable_outpoints.get_mut(&txid).unwrap() {
-					*source_opt = None;
-				}
-			} else {
-				assert!(cfg!(fuzzing), "Commitment txids are unique outside of fuzzing, where hashes can collide");
 			}
-		}
+		};
+		core::iter::once(&mut self.funding).chain(&mut self.pending_funding).for_each(prune_htlc_sources);
 
 		if !self.payment_preimages.is_empty() {
 			let min_idx = self.get_min_seen_secret();
@@ -4049,6 +4058,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 
 		mem::swap(&mut self.funding, &mut new_funding);
 		self.onchain_tx_handler.update_after_renegotiated_funding_locked(
+			self.funding.channel_parameters.clone(),
 			self.funding.current_holder_commitment_tx.clone(),
 			self.funding.prev_holder_commitment_tx.clone(),
 		);
@@ -5164,6 +5174,10 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 			let txid = tx.compute_txid();
 			log_trace!(logger, "Transaction {} confirmed in block {}", txid , block_hash);
 			// If a transaction has already been confirmed, ensure we don't bother processing it duplicatively.
+			if self.alternative_funding_confirmed.map(|(alternative_funding_txid, _)| alternative_funding_txid == txid).unwrap_or(false) {
+				log_debug!(logger, "Skipping redundant processing of funding-spend tx {} as it was previously confirmed", txid);
+				continue 'tx_iter;
+			}
 			if Some(txid) == self.funding_spend_confirmed {
 				log_debug!(logger, "Skipping redundant processing of funding-spend tx {} as it was previously confirmed", txid);
 				continue 'tx_iter;

lightning/src/chain/onchaintx.rs

@@ -1236,11 +1236,14 @@ impl<ChannelSigner: EcdsaChannelSigner> OnchainTxHandler<ChannelSigner> {
 		self.prev_holder_commitment = Some(replace(&mut self.holder_commitment, tx));
 	}
 
-	/// Replaces the current/prev holder commitment transactions spending the currently confirmed
-	/// funding outpoint with those spending the new funding outpoint.
+	/// Replaces all the data pertaining to the currently locked funding transaction after a new
+	/// funding transaction has been renegotiated and locked.
 	pub(crate) fn update_after_renegotiated_funding_locked(
-		&mut self, current: HolderCommitmentTransaction, prev: Option<HolderCommitmentTransaction>,
+		&mut self, channel_parameters: ChannelTransactionParameters,
+		current: HolderCommitmentTransaction, prev: Option<HolderCommitmentTransaction>,
 	) {
+		self.channel_value_satoshis = channel_parameters.channel_value_satoshis;
+		self.channel_transaction_parameters = channel_parameters;
 		self.holder_commitment = current;
 		self.prev_holder_commitment = prev;
 	}