From 8945f6239e5463359f33d0dc5de2d94fbbcf7ddb Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Thu, 25 Jan 2024 09:29:16 -0800 Subject: [PATCH] Rework get_latest_holder_commitment_txn to broadcast for users This method is meant to be used as a last resort when a user is forced to broadcast the current state, even if it is stale, in an attempt to claim their funds in the channel. Previously, we'd return the commitment and HTLC transactions such that they broadcast them themselves. Doing so required a different code path, one which was not tested, to obtain these transactions than our usual path when force closing. It's not worth maintaining both, and it's much simpler for us to broadcast instead. --- lightning/src/chain/channelmonitor.rs | 58 +++++---------------------- 1 file changed, 10 insertions(+), 48 deletions(-) diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index ce27c8baba9..c1013ec71b8 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -1551,11 +1551,18 @@ impl ChannelMonitor { /// close channel with their commitment transaction after a substantial amount of time. Best /// may be to contact the other node operator out-of-band to coordinate other options available /// to you. - pub fn get_latest_holder_commitment_txn(&self, logger: &L) -> Result, ()> - where L::Target: Logger { + pub fn broadcast_latest_holder_commitment_txn( + &self, broadcaster: &B, fee_estimator: &F, logger: &L + ) + where + B::Target: BroadcasterInterface, + F::Target: FeeEstimator, + L::Target: Logger + { let mut inner = self.inner.lock().unwrap(); + let fee_estimator = LowerBoundedFeeEstimator::new(&**fee_estimator); let logger = WithChannelMonitor::from_impl(logger, &*inner); - inner.get_latest_holder_commitment_txn(&logger) + inner.queue_latest_holder_commitment_txn_for_broadcast(broadcaster, &fee_estimator, &logger); } /// Unsafe test-only version of get_latest_holder_commitment_txn used by our test framework @@ -3461,51 +3468,6 @@ impl ChannelMonitorImpl { } } - fn get_latest_holder_commitment_txn( - &mut self, logger: &WithChannelMonitor, - ) -> Result, ()> where L::Target: Logger { - log_debug!(logger, "Getting signed latest holder commitment transaction!"); - self.holder_tx_signed = true; - let commitment_tx = self.onchain_tx_handler.get_maybe_signed_holder_tx(&self.funding_redeemscript); - if commitment_tx.input.iter().any(|input| input.witness.is_empty()) { - return Err(()); - } - let txid = commitment_tx.txid(); - let mut holder_transactions = vec![commitment_tx]; - // When anchor outputs are present, the HTLC transactions are only valid once the commitment - // transaction confirms. - if self.onchain_tx_handler.channel_type_features().supports_anchors_zero_fee_htlc_tx() { - return Ok(holder_transactions); - } - for htlc in self.current_holder_commitment_tx.htlc_outputs.iter() { - if let Some(vout) = htlc.0.transaction_output_index { - let preimage = if !htlc.0.offered { - if let Some(preimage) = self.payment_preimages.get(&htlc.0.payment_hash) { Some(preimage.clone()) } else { - // We can't build an HTLC-Success transaction without the preimage - continue; - } - } else if htlc.0.cltv_expiry > self.best_block.height() + 1 { - // Don't broadcast HTLC-Timeout transactions immediately as they don't meet the - // current locktime requirements on-chain. We will broadcast them in - // `block_confirmed` when `should_broadcast_holder_commitment_txn` returns true. - // Note that we add + 1 as transactions are broadcastable when they can be - // confirmed in the next block. - continue; - } else { None }; - if let Some(htlc_tx) = self.onchain_tx_handler.get_maybe_signed_htlc_tx( - &::bitcoin::OutPoint { txid, vout }, &preimage - ) { - if !htlc_tx.input.iter().any(|input| input.witness.is_empty()) { - holder_transactions.push(htlc_tx); - } - } - } - } - // We throw away the generated waiting_first_conf data as we aren't (yet) confirmed and we don't actually know what the caller wants to do. - // The data will be re-generated and tracked in check_spend_holder_transaction if we get a confirmation. - Ok(holder_transactions) - } - #[cfg(any(test,feature = "unsafe_revoked_tx_signing"))] /// Note that this includes possibly-locktimed-in-the-future transactions! fn unsafe_get_latest_holder_commitment_txn(