Split finalize_package into separate methods per malleability

Author: wpaulino

lightning/src/chain/onchaintx.rs

@@ -393,7 +393,7 @@ impl<ChannelSigner: Sign> OnchainTxHandler<ChannelSigner> {
 					cached_request.compute_package_output(predicted_weight, self.destination_script.dust_value().to_sat(), fee_estimator, logger) {
 				assert!(new_feerate != 0);
 
-				let transaction = cached_request.finalize_package(self, output_value, self.destination_script.clone(), logger).unwrap();
+				let transaction = cached_request.finalize_malleable_package(self, output_value, self.destination_script.clone(), logger).unwrap();
 				log_trace!(logger, "...with timer {} and feerate {}", new_timer.unwrap(), new_feerate);
 				assert!(predicted_weight >= transaction.weight());
 				return Some((new_timer, new_feerate, transaction))
@@ -402,7 +402,7 @@ impl<ChannelSigner: Sign> OnchainTxHandler<ChannelSigner> {
 			// Note: Currently, amounts of holder outputs spending witnesses aren't used
 			// as we can't malleate spending package to increase their feerate. This
 			// should change with the remaining anchor output patchset.
-			if let Some(transaction) = cached_request.finalize_package(self, 0, self.destination_script.clone(), logger) {
+			if let Some(transaction) = cached_request.finalize_untractable_package(self, logger) {
 				return Some((None, 0, transaction));
 			}
 		}

lightning/src/chain/package.rs

@@ -656,47 +656,46 @@ impl PackageTemplate {
 		};
 		transaction_weight + inputs_weight + witnesses_weight + output_weight
 	}
-	pub(crate) fn finalize_package<L: Deref, Signer: Sign>(&self, onchain_handler: &mut OnchainTxHandler<Signer>, value: u64, destination_script: Script, logger: &L) -> Option<Transaction>
-		where L::Target: Logger,
-	{
-		match self.malleability {
-			PackageMalleability::Malleable => {
-				let mut bumped_tx = Transaction {
-					version: 2,
-					lock_time: PackedLockTime::ZERO,
-					input: vec![],
-					output: vec![TxOut {
-						script_pubkey: destination_script,
-						value,
-					}],
-				};
-				for (outpoint, _) in self.inputs.iter() {
-					bumped_tx.input.push(TxIn {
-						previous_output: *outpoint,
-						script_sig: Script::new(),
-						sequence: Sequence::ENABLE_RBF_NO_LOCKTIME,
-						witness: Witness::new(),
-					});
-				}
-				for (i, (outpoint, out)) in self.inputs.iter().enumerate() {
-					log_debug!(logger, "Adding claiming input for outpoint {}:{}", outpoint.txid, outpoint.vout);
-					if !out.finalize_input(&mut bumped_tx, i, onchain_handler) { return None; }
-				}
-				log_debug!(logger, "Finalized transaction {} ready to broadcast", bumped_tx.txid());
-				return Some(bumped_tx);
-			},
-			PackageMalleability::Untractable => {
-				debug_assert_eq!(value, 0, "value is ignored for non-malleable packages, should be zero to ensure callsites are correct");
-				if let Some((outpoint, outp)) = self.inputs.first() {
-					if let Some(final_tx) = outp.get_finalized_tx(outpoint, onchain_handler) {
-						log_debug!(logger, "Adding claiming input for outpoint {}:{}", outpoint.txid, outpoint.vout);
-						log_debug!(logger, "Finalized transaction {} ready to broadcast", final_tx.txid());
-						return Some(final_tx);
-					}
-					return None;
-				} else { panic!("API Error: Package must not be inputs empty"); }
-			},
+	pub(crate) fn finalize_malleable_package<L: Deref, Signer: Sign>(
+		&self, onchain_handler: &mut OnchainTxHandler<Signer>, value: u64, destination_script: Script, logger: &L,
+	) -> Option<Transaction> where L::Target: Logger {
+		debug_assert!(self.is_malleable());
+		let mut bumped_tx = Transaction {
+			version: 2,
+			lock_time: PackedLockTime::ZERO,
+			input: vec![],
+			output: vec![TxOut {
+				script_pubkey: destination_script,
+				value,
+			}],
+		};
+		for (outpoint, _) in self.inputs.iter() {
+			bumped_tx.input.push(TxIn {
+				previous_output: *outpoint,
+				script_sig: Script::new(),
+				sequence: Sequence::ENABLE_RBF_NO_LOCKTIME,
+				witness: Witness::new(),
+			});
 		}
+		for (i, (outpoint, out)) in self.inputs.iter().enumerate() {
+			log_debug!(logger, "Adding claiming input for outpoint {}:{}", outpoint.txid, outpoint.vout);
+			if !out.finalize_input(&mut bumped_tx, i, onchain_handler) { return None; }
+		}
+		log_debug!(logger, "Finalized transaction {} ready to broadcast", bumped_tx.txid());
+		Some(bumped_tx)
+	}
+	pub(crate) fn finalize_untractable_package<L: Deref, Signer: Sign>(
+		&self, onchain_handler: &mut OnchainTxHandler<Signer>, logger: &L,
+	) -> Option<Transaction> where L::Target: Logger {
+		debug_assert!(!self.is_malleable());
+		if let Some((outpoint, outp)) = self.inputs.first() {
+			if let Some(final_tx) = outp.get_finalized_tx(outpoint, onchain_handler) {
+				log_debug!(logger, "Adding claiming input for outpoint {}:{}", outpoint.txid, outpoint.vout);
+				log_debug!(logger, "Finalized transaction {} ready to broadcast", final_tx.txid());
+				return Some(final_tx);
+			}
+			return None;
+		} else { panic!("API Error: Package must not be inputs empty"); }
 	}
 	/// In LN, output claimed are time-sensitive, which means we have to spend them before reaching some timelock expiration. At in-channel
 	/// output detection, we generate a first version of a claim tx and associate to it a height timer. A height timer is an absolute block