Provide destination_script from ChannelMonitor to OnchainTxHandler

The `ChannelMonitor` and `OnchainTxHandler` have historically been tied
together, often tracking some of the same state twice. As we introduce
support for splices in the `ChannelMonitor`, we'd like to avoid leaking
some of those details to the `OnchainTxHandler`. Ultimately, the
`OnchainTxHandler` should stand on its own and support claiming funds
from multiple `ChannelMonitor`s, allowing us to save on fees by batching
aggregatable claims across multiple in-flight closing channels.

This commit removes the internal usage of
`OnchainTxHandler::destination_script` and deprecates it, opting to
instead provide it as a function argument wherever needed.

Author: wpaulino

lightning/src/chain/channelmonitor.rs

@@ -2131,12 +2131,14 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
 		L::Target: Logger,
 	{
 		let fee_estimator = LowerBoundedFeeEstimator::new(fee_estimator);
-		let mut inner = self.inner.lock().unwrap();
+		let mut lock = self.inner.lock().unwrap();
+		let inner = &mut *lock;
 		let logger = WithChannelMonitor::from_impl(logger, &*inner, None);
 		let current_height = inner.best_block.height;
 		let conf_target = inner.closure_conf_target();
 		inner.onchain_tx_handler.rebroadcast_pending_claims(
-			current_height, FeerateStrategy::HighestOfPreviousOrNew, &broadcaster, conf_target, &fee_estimator, &logger,
+			current_height, FeerateStrategy::HighestOfPreviousOrNew, &broadcaster, conf_target,
+			&inner.destination_script, &fee_estimator, &logger,
 		);
 	}
 
@@ -2157,12 +2159,14 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
 		L::Target: Logger,
 	{
 		let fee_estimator = LowerBoundedFeeEstimator::new(fee_estimator);
-		let mut inner = self.inner.lock().unwrap();
+		let mut lock = self.inner.lock().unwrap();
+		let inner = &mut *lock;
 		let logger = WithChannelMonitor::from_impl(logger, &*inner, None);
 		let current_height = inner.best_block.height;
 		let conf_target = inner.closure_conf_target();
 		inner.onchain_tx_handler.rebroadcast_pending_claims(
-			current_height, FeerateStrategy::RetryPrevious, &broadcaster, conf_target, &fee_estimator, &logger,
+			current_height, FeerateStrategy::RetryPrevious, &broadcaster, conf_target,
+			&inner.destination_script, &fee_estimator, &logger,
 		);
 	}
 
@@ -3212,7 +3216,10 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 			($commitment_number: expr, $txid: expr, $htlcs: expr) => {
 				let (htlc_claim_reqs, _) = self.get_counterparty_output_claim_info($commitment_number, $txid, None, $htlcs);
 				let conf_target = self.closure_conf_target();
-				self.onchain_tx_handler.update_claims_view_from_requests(htlc_claim_reqs, self.best_block.height, self.best_block.height, broadcaster, conf_target, fee_estimator, logger);
+				self.onchain_tx_handler.update_claims_view_from_requests(
+					htlc_claim_reqs, self.best_block.height, self.best_block.height, broadcaster,
+					conf_target, &self.destination_script, fee_estimator, logger,
+				);
 			}
 		}
 		if let Some(txid) = self.funding.current_counterparty_commitment_txid {
@@ -3261,7 +3268,10 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 				// transactions.
 				let (claim_reqs, _) = self.get_broadcasted_holder_claims(&holder_commitment.tx, self.best_block.height);
 				let conf_target = self.closure_conf_target();
-				self.onchain_tx_handler.update_claims_view_from_requests(claim_reqs, self.best_block.height, self.best_block.height, broadcaster, conf_target, fee_estimator, logger);
+				self.onchain_tx_handler.update_claims_view_from_requests(
+					claim_reqs, self.best_block.height, self.best_block.height, broadcaster,
+					conf_target, &self.destination_script, fee_estimator, logger,
+				);
 			}
 		}
 	}
@@ -3324,7 +3334,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 		let conf_target = self.closure_conf_target();
 		self.onchain_tx_handler.update_claims_view_from_requests(
 			claimable_outpoints, self.best_block.height, self.best_block.height, broadcaster,
-			conf_target, fee_estimator, logger,
+			conf_target, &self.destination_script, fee_estimator, logger,
 		);
 	}
 
@@ -4216,7 +4226,9 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 			log_trace!(logger, "Best block re-orged, replaced with new block {} at height {}", block_hash, height);
 			self.onchain_events_awaiting_threshold_conf.retain(|ref entry| entry.height <= height);
 			let conf_target = self.closure_conf_target();
-			self.onchain_tx_handler.block_disconnected(height + 1, broadcaster, conf_target, fee_estimator, logger);
+			self.onchain_tx_handler.block_disconnected(
+				height + 1, broadcaster, conf_target, &self.destination_script, fee_estimator, logger,
+			);
 			Vec::new()
 		} else { Vec::new() }
 	}
@@ -4540,8 +4552,14 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 		}
 
 		let conf_target = self.closure_conf_target();
-		self.onchain_tx_handler.update_claims_view_from_requests(claimable_outpoints, conf_height, self.best_block.height, broadcaster, conf_target, fee_estimator, logger);
-		self.onchain_tx_handler.update_claims_view_from_matched_txn(&txn_matched, conf_height, conf_hash, self.best_block.height, broadcaster, conf_target, fee_estimator, logger);
+		self.onchain_tx_handler.update_claims_view_from_requests(
+			claimable_outpoints, conf_height, self.best_block.height, broadcaster, conf_target,
+			&self.destination_script, fee_estimator, logger,
+		);
+		self.onchain_tx_handler.update_claims_view_from_matched_txn(
+			&txn_matched, conf_height, conf_hash, self.best_block.height, broadcaster, conf_target,
+			&self.destination_script, fee_estimator, logger,
+		);
 
 		// Determine new outputs to watch by comparing against previously known outputs to watch,
 		// updating the latter in the process.
@@ -4581,7 +4599,9 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 
 		let bounded_fee_estimator = LowerBoundedFeeEstimator::new(fee_estimator);
 		let conf_target = self.closure_conf_target();
-		self.onchain_tx_handler.block_disconnected(height, broadcaster, conf_target, &bounded_fee_estimator, logger);
+		self.onchain_tx_handler.block_disconnected(
+			height, broadcaster, conf_target, &self.destination_script, &bounded_fee_estimator, logger
+		);
 
 		self.best_block = BestBlock::new(header.prev_blockhash, height - 1);
 	}
@@ -4616,7 +4636,9 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 		debug_assert!(!self.onchain_events_awaiting_threshold_conf.iter().any(|ref entry| entry.txid == *txid));
 
 		let conf_target = self.closure_conf_target();
-		self.onchain_tx_handler.transaction_unconfirmed(txid, broadcaster, conf_target, fee_estimator, logger);
+		self.onchain_tx_handler.transaction_unconfirmed(
+			txid, broadcaster, conf_target, &self.destination_script, fee_estimator, logger
+		);
 	}
 
 	/// Filters a block's `txdata` for transactions spending watched outputs or for any child

lightning/src/chain/onchaintx.rs

@@ -232,7 +232,7 @@ pub(crate) enum FeerateStrategy {
 pub struct OnchainTxHandler<ChannelSigner: EcdsaChannelSigner> {
 	channel_value_satoshis: u64,
 	channel_keys_id: [u8; 32],
-	destination_script: ScriptBuf,
+	destination_script: ScriptBuf, // Deprecated as of 0.2.
 	holder_commitment: HolderCommitmentTransaction,
 	prev_holder_commitment: Option<HolderCommitmentTransaction>,
 
@@ -487,7 +487,8 @@ impl<ChannelSigner: EcdsaChannelSigner> OnchainTxHandler<ChannelSigner> {
 	/// connections, like on mobile.
 	pub(super) fn rebroadcast_pending_claims<B: Deref, F: Deref, L: Logger>(
 		&mut self, current_height: u32, feerate_strategy: FeerateStrategy, broadcaster: &B,
-		conf_target: ConfirmationTarget, fee_estimator: &LowerBoundedFeeEstimator<F>, logger: &L,
+		conf_target: ConfirmationTarget, destination_script: &Script,
+		fee_estimator: &LowerBoundedFeeEstimator<F>, logger: &L,
 	)
 	where
 		B::Target: BroadcasterInterface,
@@ -500,7 +501,10 @@ impl<ChannelSigner: EcdsaChannelSigner> OnchainTxHandler<ChannelSigner> {
 			bump_requests.push((*claim_id, request.clone()));
 		}
 		for (claim_id, request) in bump_requests {
-			self.generate_claim(current_height, &request, &feerate_strategy, conf_target, fee_estimator, logger)
+			self.generate_claim(
+				current_height, &request, &feerate_strategy, conf_target, destination_script,
+				fee_estimator, logger,
+			)
 				.map(|(_, new_feerate, claim)| {
 					let mut feerate_was_bumped = false;
 					if let Some(mut_request) = self.pending_claim_requests.get_mut(&claim_id) {
@@ -551,8 +555,9 @@ impl<ChannelSigner: EcdsaChannelSigner> OnchainTxHandler<ChannelSigner> {
 	/// Panics if there are signing errors, because signing operations in reaction to on-chain
 	/// events are not expected to fail, and if they do, we may lose funds.
 	fn generate_claim<F: Deref, L: Logger>(
-		&mut self, cur_height: u32, cached_request: &PackageTemplate, feerate_strategy: &FeerateStrategy,
-		conf_target: ConfirmationTarget, fee_estimator: &LowerBoundedFeeEstimator<F>, logger: &L,
+		&mut self, cur_height: u32, cached_request: &PackageTemplate,
+		feerate_strategy: &FeerateStrategy, conf_target: ConfirmationTarget,
+		destination_script: &Script, fee_estimator: &LowerBoundedFeeEstimator<F>, logger: &L,
 	) -> Option<(u32, u64, OnchainClaim)>
 	where F::Target: FeeEstimator,
 	{
@@ -617,15 +622,15 @@ impl<ChannelSigner: EcdsaChannelSigner> OnchainTxHandler<ChannelSigner> {
 				}
 			}
 
-			let predicted_weight = cached_request.package_weight(&self.destination_script);
+			let predicted_weight = cached_request.package_weight(destination_script);
 			if let Some((output_value, new_feerate)) = cached_request.compute_package_output(
-				predicted_weight, self.destination_script.minimal_non_dust().to_sat(),
+				predicted_weight, destination_script.minimal_non_dust().to_sat(),
 				feerate_strategy, conf_target, fee_estimator, logger,
 			) {
 				assert!(new_feerate != 0);
 
 				let transaction = cached_request.maybe_finalize_malleable_package(
-					cur_height, self, Amount::from_sat(output_value), self.destination_script.clone(), logger
+					cur_height, self, Amount::from_sat(output_value), destination_script.into(), logger
 				).unwrap();
 				assert!(predicted_weight >= transaction.0.weight().to_wu());
 				return Some((new_timer, new_feerate, OnchainClaim::Tx(transaction)));
@@ -727,7 +732,7 @@ impl<ChannelSigner: EcdsaChannelSigner> OnchainTxHandler<ChannelSigner> {
 	/// `cur_height`, however it must never be higher than `cur_height`.
 	pub(super) fn update_claims_view_from_requests<B: Deref, F: Deref, L: Logger>(
 		&mut self, mut requests: Vec<PackageTemplate>, conf_height: u32, cur_height: u32,
-		broadcaster: &B, conf_target: ConfirmationTarget,
+		broadcaster: &B, conf_target: ConfirmationTarget, destination_script: &Script,
 		fee_estimator: &LowerBoundedFeeEstimator<F>, logger: &L
 	) where
 		B::Target: BroadcasterInterface,
@@ -815,7 +820,8 @@ impl<ChannelSigner: EcdsaChannelSigner> OnchainTxHandler<ChannelSigner> {
 		// height timer expiration (i.e in how many blocks we're going to take action).
 		for mut req in preprocessed_requests {
 			if let Some((new_timer, new_feerate, claim)) = self.generate_claim(
-				cur_height, &req, &FeerateStrategy::ForceBump, conf_target, &*fee_estimator, &*logger,
+				cur_height, &req, &FeerateStrategy::ForceBump, conf_target, destination_script,
+				&*fee_estimator, &*logger,
 			) {
 				req.set_timer(new_timer);
 				req.set_feerate(new_feerate);
@@ -881,7 +887,7 @@ impl<ChannelSigner: EcdsaChannelSigner> OnchainTxHandler<ChannelSigner> {
 	pub(super) fn update_claims_view_from_matched_txn<B: Deref, F: Deref, L: Logger>(
 		&mut self, txn_matched: &[&Transaction], conf_height: u32, conf_hash: BlockHash,
 		cur_height: u32, broadcaster: &B, conf_target: ConfirmationTarget,
-		fee_estimator: &LowerBoundedFeeEstimator<F>, logger: &L
+		destination_script: &Script, fee_estimator: &LowerBoundedFeeEstimator<F>, logger: &L
 	) where
 		B::Target: BroadcasterInterface,
 		F::Target: FeeEstimator,
@@ -1032,7 +1038,8 @@ impl<ChannelSigner: EcdsaChannelSigner> OnchainTxHandler<ChannelSigner> {
 
 		for (claim_id, request) in bump_candidates.iter() {
 			if let Some((new_timer, new_feerate, bump_claim)) = self.generate_claim(
-				cur_height, &request, &FeerateStrategy::ForceBump, conf_target, &*fee_estimator, &*logger,
+				cur_height, &request, &FeerateStrategy::ForceBump, conf_target, destination_script,
+				&*fee_estimator, &*logger,
 			) {
 				match bump_claim {
 					OnchainClaim::Tx(bump_tx) => {
@@ -1068,6 +1075,7 @@ impl<ChannelSigner: EcdsaChannelSigner> OnchainTxHandler<ChannelSigner> {
 		txid: &Txid,
 		broadcaster: B,
 		conf_target: ConfirmationTarget,
+		destination_script: &Script,
 		fee_estimator: &LowerBoundedFeeEstimator<F>,
 		logger: &L,
 	) where
@@ -1083,13 +1091,15 @@ impl<ChannelSigner: EcdsaChannelSigner> OnchainTxHandler<ChannelSigner> {
 		}
 
 		if let Some(height) = height {
-			self.block_disconnected(height, broadcaster, conf_target, fee_estimator, logger);
+			self.block_disconnected(
+				height, broadcaster, conf_target, destination_script, fee_estimator, logger,
+			);
 		}
 	}
 
 	pub(super) fn block_disconnected<B: Deref, F: Deref, L: Logger>(
 		&mut self, height: u32, broadcaster: B, conf_target: ConfirmationTarget,
-		fee_estimator: &LowerBoundedFeeEstimator<F>, logger: &L,
+		destination_script: &Script, fee_estimator: &LowerBoundedFeeEstimator<F>, logger: &L,
 	)
 		where B::Target: BroadcasterInterface,
 			F::Target: FeeEstimator,
@@ -1122,7 +1132,8 @@ impl<ChannelSigner: EcdsaChannelSigner> OnchainTxHandler<ChannelSigner> {
 			// `height` is the height being disconnected, so our `current_height` is 1 lower.
 			let current_height = height - 1;
 			if let Some((new_timer, new_feerate, bump_claim)) = self.generate_claim(
-				current_height, &request, &FeerateStrategy::ForceBump, conf_target, fee_estimator, logger
+				current_height, &request, &FeerateStrategy::ForceBump, conf_target,
+				destination_script, fee_estimator, logger
 			) {
 				request.set_timer(new_timer);
 				request.set_feerate(new_feerate);
@@ -1375,10 +1386,11 @@ mod tests {
 			));
 		}
 		let holder_commit = HolderCommitmentTransaction::dummy(1000000, &mut htlcs);
+		let destination_script = ScriptBuf::new();
 		let mut tx_handler = OnchainTxHandler::new(
 			1000000,
 			[0; 32],
-			ScriptBuf::new(),
+			destination_script.clone(),
 			signer,
 			chan_params,
 			holder_commit,
@@ -1418,6 +1430,7 @@ mod tests {
 			1,
 			&&broadcaster,
 			ConfirmationTarget::UrgentOnChainSweep,
+			&destination_script,
 			&fee_estimator,
 			&logger,
 		);
@@ -1441,6 +1454,7 @@ mod tests {
 			2,
 			&&broadcaster,
 			ConfirmationTarget::UrgentOnChainSweep,
+			&destination_script,
 			&fee_estimator,
 			&logger,
 		);