diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index c7011af43a9..38eed18a814 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -1302,30 +1302,51 @@ pub(crate) struct ChannelMonitorImpl { alternative_funding_confirmed: Option<(Txid, u32)>, } +// Returns a `&FundingScope` for the one we are currently observing/handling commitment transactions +// for on the chain. +macro_rules! get_confirmed_funding_scope { + ($self: expr) => { + $self + .alternative_funding_confirmed + .map(|(alternative_funding_txid, _)| { + $self + .pending_funding + .iter() + .find(|funding| funding.funding_txid() == alternative_funding_txid) + .expect("FundingScope for confirmed alternative funding must exist") + }) + .unwrap_or(&$self.funding) + }; +} + // Macro helper to access holder commitment HTLC data (including both non-dust and dust) while // holding mutable references to `self`. Unfortunately, if these were turned into helper functions, // we'd be unable to mutate `self` while holding an immutable iterator (specifically, returned from // a function) over `self`. #[rustfmt::skip] macro_rules! holder_commitment_htlcs { - ($self: expr, CURRENT) => { - $self.funding.current_holder_commitment_tx.nondust_htlcs().iter() + ($self: expr, CURRENT) => {{ + let funding = get_confirmed_funding_scope!($self); + funding.current_holder_commitment_tx.nondust_htlcs().iter() .chain($self.current_holder_htlc_data.dust_htlcs.iter().map(|(htlc, _)| htlc)) - }; + }}; ($self: expr, CURRENT_WITH_SOURCES) => {{ + let funding = get_confirmed_funding_scope!($self); holder_commitment_htlcs!( - &$self.funding.current_holder_commitment_tx, &$self.current_holder_htlc_data + &funding.current_holder_commitment_tx, &$self.current_holder_htlc_data ) }}; ($self: expr, PREV) => {{ - $self.funding.prev_holder_commitment_tx.as_ref().map(|tx| { + let funding = get_confirmed_funding_scope!($self); + funding.prev_holder_commitment_tx.as_ref().map(|tx| { let dust_htlcs = $self.prev_holder_htlc_data.as_ref().unwrap().dust_htlcs.iter() .map(|(htlc, _)| htlc); tx.nondust_htlcs().iter().chain(dust_htlcs) }) }}; ($self: expr, PREV_WITH_SOURCES) => {{ - $self.funding.prev_holder_commitment_tx.as_ref().map(|tx| { + let funding = get_confirmed_funding_scope!($self); + funding.prev_holder_commitment_tx.as_ref().map(|tx| { holder_commitment_htlcs!(tx, $self.prev_holder_htlc_data.as_ref().unwrap()) }) }}; @@ -2104,6 +2125,10 @@ impl ChannelMonitor { /// to the commitment transaction being revoked, this will return a signed transaction, but /// the signature will not be valid. /// + /// Note that due to splicing, this can also return an `Err` when the counterparty commitment + /// this transaction is attempting to claim is no longer valid because the corresponding funding + /// transaction was spliced. + /// /// [`EcdsaChannelSigner::sign_justice_revoked_output`]: crate::sign::ecdsa::EcdsaChannelSigner::sign_justice_revoked_output /// [`Persist`]: crate::chain::chainmonitor::Persist #[rustfmt::skip] @@ -2408,7 +2433,8 @@ impl ChannelMonitor { pub fn get_spendable_outputs(&self, tx: &Transaction, confirmation_height: u32) -> Vec { let inner = self.inner.lock().unwrap(); let current_height = inner.best_block.height; - let mut spendable_outputs = inner.get_spendable_outputs(tx); + let funding = get_confirmed_funding_scope!(inner); + let mut spendable_outputs = inner.get_spendable_outputs(&funding, tx); spendable_outputs.retain(|descriptor| { let mut conf_threshold = current_height.saturating_sub(ANTI_REORG_DELAY) + 1; if let SpendableOutputDescriptor::DelayedPaymentOutput(descriptor) = descriptor { @@ -3015,17 +3041,19 @@ impl ChannelMonitor { } } - let txid = confirmed_txid.unwrap(); - if Some(txid) == us.funding.current_counterparty_commitment_txid || Some(txid) == us.funding.prev_counterparty_commitment_txid { - walk_htlcs!(false, us.funding.counterparty_claimable_outpoints.get(&txid).unwrap().iter().filter_map(|(a, b)| { + let commitment_txid = confirmed_txid.unwrap(); + let funding_spent = get_confirmed_funding_scope!(us); + + if Some(commitment_txid) == funding_spent.current_counterparty_commitment_txid || Some(commitment_txid) == funding_spent.prev_counterparty_commitment_txid { + walk_htlcs!(false, funding_spent.counterparty_claimable_outpoints.get(&commitment_txid).unwrap().iter().filter_map(|(a, b)| { if let &Some(ref source) = b { Some((a, Some(&**source))) } else { None } })); - } else if txid == us.funding.current_holder_commitment_tx.trust().txid() { + } else if commitment_txid == funding_spent.current_holder_commitment_tx.trust().txid() { walk_htlcs!(true, holder_commitment_htlcs!(us, CURRENT_WITH_SOURCES)); - } else if let Some(prev_commitment_tx) = &us.funding.prev_holder_commitment_tx { - if txid == prev_commitment_tx.trust().txid() { + } else if let Some(prev_commitment_tx) = &funding_spent.prev_holder_commitment_tx { + if commitment_txid == prev_commitment_tx.trust().txid() { walk_htlcs!(true, holder_commitment_htlcs!(us, PREV_WITH_SOURCES).unwrap()); } } @@ -3562,12 +3590,13 @@ impl ChannelMonitorImpl { } else { return; }; + let funding_spent = get_confirmed_funding_scope!(self); // If the channel is force closed, try to claim the output from this preimage. // First check if a counterparty commitment transaction has been broadcasted: macro_rules! claim_htlcs { ($commitment_number: expr, $txid: expr, $htlcs: expr) => { - let (htlc_claim_reqs, _) = self.get_counterparty_output_claim_info($commitment_number, $txid, None, $htlcs, confirmed_spend_height); + let (htlc_claim_reqs, _) = self.get_counterparty_output_claim_info(funding_spent, $commitment_number, $txid, None, $htlcs, confirmed_spend_height); 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, @@ -3575,10 +3604,10 @@ impl ChannelMonitorImpl { ); } } - if let Some(txid) = self.funding.current_counterparty_commitment_txid { + if let Some(txid) = funding_spent.current_counterparty_commitment_txid { if txid == confirmed_spend_txid { if let Some(commitment_number) = self.counterparty_commitment_txn_on_chain.get(&txid) { - claim_htlcs!(*commitment_number, txid, self.funding.counterparty_claimable_outpoints.get(&txid)); + claim_htlcs!(*commitment_number, txid, funding_spent.counterparty_claimable_outpoints.get(&txid)); } else { debug_assert!(false); log_error!(logger, "Detected counterparty commitment tx on-chain without tracking commitment number"); @@ -3586,10 +3615,10 @@ impl ChannelMonitorImpl { return; } } - if let Some(txid) = self.funding.prev_counterparty_commitment_txid { + if let Some(txid) = funding_spent.prev_counterparty_commitment_txid { if txid == confirmed_spend_txid { if let Some(commitment_number) = self.counterparty_commitment_txn_on_chain.get(&txid) { - claim_htlcs!(*commitment_number, txid, self.funding.counterparty_claimable_outpoints.get(&txid)); + claim_htlcs!(*commitment_number, txid, funding_spent.counterparty_claimable_outpoints.get(&txid)); } else { debug_assert!(false); log_error!(logger, "Detected counterparty commitment tx on-chain without tracking commitment number"); @@ -3604,9 +3633,9 @@ impl ChannelMonitorImpl { // *we* sign a holder commitment transaction, not when e.g. a watchtower broadcasts one of our // holder commitment transactions. if self.broadcasted_holder_revokable_script.is_some() { - let holder_commitment_tx = if self.funding.current_holder_commitment_tx.trust().txid() == confirmed_spend_txid { - Some(&self.funding.current_holder_commitment_tx) - } else if let Some(prev_holder_commitment_tx) = &self.funding.prev_holder_commitment_tx { + let holder_commitment_tx = if funding_spent.current_holder_commitment_tx.trust().txid() == confirmed_spend_txid { + Some(&funding_spent.current_holder_commitment_tx) + } else if let Some(prev_holder_commitment_tx) = &funding_spent.prev_holder_commitment_tx { if prev_holder_commitment_tx.trust().txid() == confirmed_spend_txid { Some(prev_holder_commitment_tx) } else { @@ -3619,7 +3648,9 @@ impl ChannelMonitorImpl { // Assume that the broadcasted commitment transaction confirmed in the current best // block. Even if not, its a reasonable metric for the bump criteria on the HTLC // transactions. - let (claim_reqs, _) = self.get_broadcasted_holder_claims(&self.funding, holder_commitment_tx, self.best_block.height); + let (claim_reqs, _) = self.get_broadcasted_holder_claims( + funding_spent, 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, @@ -3633,14 +3664,7 @@ impl ChannelMonitorImpl { fn generate_claimable_outpoints_and_watch_outputs( &mut self, generate_monitor_event_with_reason: Option, ) -> (Vec, Vec) { - let funding = self.alternative_funding_confirmed - .map(|(alternative_funding_txid, _)| { - self.pending_funding - .iter() - .find(|funding| funding.funding_txid() == alternative_funding_txid) - .expect("FundingScope for confirmed alternative funding must exist") - }) - .unwrap_or(&self.funding); + let funding = get_confirmed_funding_scope!(self); let holder_commitment_tx = &funding.current_holder_commitment_tx; let funding_outp = HolderFundingOutput::build( holder_commitment_tx.clone(), @@ -3851,6 +3875,9 @@ impl ChannelMonitorImpl { self.outputs_to_watch.remove(&funding.funding_txid()); } if let Some((alternative_funding_txid, _)) = self.alternative_funding_confirmed.take() { + // In exceedingly rare cases, it's possible there was a reorg that caused a potential funding to + // be locked in that this `ChannelMonitor` has not yet seen. Thus, we avoid a runtime assertion + // and only assert in debug mode. debug_assert_eq!(alternative_funding_txid, new_funding_txid); } @@ -4164,8 +4191,14 @@ impl ChannelMonitorImpl { to_countersignatory_value, )| { let nondust_htlcs = vec![]; + // Since we're expected to only reach here during the initial persistence of a + // monitor (i.e., via [`Persist::persist_new_channel`]), we expect to only have + // one `FundingScope` present. + debug_assert!(self.pending_funding.is_empty()); + let channel_parameters = &self.funding.channel_parameters; let commitment_tx = self.build_counterparty_commitment_tx( + channel_parameters, INITIAL_COMMITMENT_NUMBER, &their_per_commitment_point, to_broadcaster_value, @@ -4183,11 +4216,12 @@ impl ChannelMonitorImpl { #[rustfmt::skip] fn build_counterparty_commitment_tx( - &self, commitment_number: u64, their_per_commitment_point: &PublicKey, - to_broadcaster_value: u64, to_countersignatory_value: u64, feerate_per_kw: u32, + &self, channel_parameters: &ChannelTransactionParameters, commitment_number: u64, + their_per_commitment_point: &PublicKey, to_broadcaster_value: u64, + to_countersignatory_value: u64, feerate_per_kw: u32, nondust_htlcs: Vec ) -> CommitmentTransaction { - let channel_parameters = &self.funding.channel_parameters.as_counterparty_broadcastable(); + let channel_parameters = &channel_parameters.as_counterparty_broadcastable(); CommitmentTransaction::new(commitment_number, their_per_commitment_point, to_broadcaster_value, to_countersignatory_value, feerate_per_kw, nondust_htlcs, channel_parameters, &self.onchain_tx_handler.secp_ctx) } @@ -4209,9 +4243,20 @@ impl ChannelMonitorImpl { htlc.transaction_output_index.map(|_| htlc).cloned() }).collect::>(); - let commitment_tx = self.build_counterparty_commitment_tx(commitment_number, - &their_per_commitment_point, to_broadcaster_value, - to_countersignatory_value, feerate_per_kw, nondust_htlcs); + // This monitor update variant is only applicable while there's a single + // `FundingScope` active, otherwise we expect to see + // `LatestCounterpartyCommitment` instead. + debug_assert!(self.pending_funding.is_empty()); + let channel_parameters = &self.funding.channel_parameters; + let commitment_tx = self.build_counterparty_commitment_tx( + channel_parameters, + commitment_number, + &their_per_commitment_point, + to_broadcaster_value, + to_countersignatory_value, + feerate_per_kw, + nondust_htlcs, + ); debug_assert_eq!(commitment_tx.trust().txid(), commitment_txid); @@ -4244,7 +4289,17 @@ impl ChannelMonitorImpl { let revokeable_redeemscript = chan_utils::get_revokeable_redeemscript(&revocation_pubkey, self.counterparty_commitment_params.on_counterparty_tx_csv, &delayed_key); - let channel_parameters = &self.funding.channel_parameters; + let commitment_txid = &justice_tx.input[input_idx].previous_output.txid; + // Since there may be multiple counterparty commitment transactions for the same commitment + // number due to splicing, we have to locate the matching `FundingScope::channel_parameters` + // to provide the signer. Since this is intended to be called during + // `Persist::update_persisted_channel`, the monitor should have already had the update + // applied. + let channel_parameters = core::iter::once(&self.funding) + .chain(&self.pending_funding) + .find(|funding| funding.counterparty_claimable_outpoints.contains_key(commitment_txid)) + .map(|funding| &funding.channel_parameters) + .ok_or(())?; let sig = self.onchain_tx_handler.signer.sign_justice_revoked_output( &channel_parameters, &justice_tx, input_idx, value, &per_commitment_key, &self.onchain_tx_handler.secp_ctx, @@ -4280,7 +4335,7 @@ impl ChannelMonitorImpl { /// Returns packages to claim the revoked output(s) and general information about the output that /// is to the counterparty in the commitment transaction. #[rustfmt::skip] - fn check_spend_counterparty_transaction(&mut self, tx: &Transaction, height: u32, block_hash: &BlockHash, logger: &L) + fn check_spend_counterparty_transaction(&mut self, commitment_txid: Txid, commitment_tx: &Transaction, height: u32, block_hash: &BlockHash, logger: &L) -> (Vec, CommitmentTxCounterpartyOutputInfo) where L::Target: Logger { // Most secp and related errors trying to create keys means we have no hope of constructing @@ -4288,8 +4343,8 @@ impl ChannelMonitorImpl { let mut claimable_outpoints = Vec::new(); let mut to_counterparty_output_info = None; - let commitment_txid = tx.compute_txid(); //TODO: This is gonna be a performance bottleneck for watchtowers! - let per_commitment_option = self.funding.counterparty_claimable_outpoints.get(&commitment_txid); + let funding_spent = get_confirmed_funding_scope!(self); + let per_commitment_option = funding_spent.counterparty_claimable_outpoints.get(&commitment_txid); macro_rules! ignore_error { ( $thing : expr ) => { @@ -4300,8 +4355,11 @@ impl ChannelMonitorImpl { }; } - let commitment_number = 0xffffffffffff - ((((tx.input[0].sequence.0 as u64 & 0xffffff) << 3*8) | (tx.lock_time.to_consensus_u32() as u64 & 0xffffff)) ^ self.commitment_transaction_number_obscure_factor); + let funding_txid_spent = commitment_tx.input[0].previous_output.txid; + let commitment_number = 0xffffffffffff - ((((commitment_tx.input[0].sequence.0 as u64 & 0xffffff) << 3*8) | (commitment_tx.lock_time.to_consensus_u32() as u64 & 0xffffff)) ^ self.commitment_transaction_number_obscure_factor); if commitment_number >= self.get_min_seen_secret() { + assert_eq!(funding_spent.funding_txid(), funding_txid_spent); + let secret = self.get_secret(commitment_number).unwrap(); let per_commitment_key = ignore_error!(SecretKey::from_slice(&secret)); let per_commitment_point = PublicKey::from_secret_key(&self.onchain_tx_handler.secp_ctx, &per_commitment_key); @@ -4312,13 +4370,12 @@ impl ChannelMonitorImpl { let revokeable_p2wsh = revokeable_redeemscript.to_p2wsh(); // First, process non-htlc outputs (to_holder & to_counterparty) - for (idx, outp) in tx.output.iter().enumerate() { + for (idx, outp) in commitment_tx.output.iter().enumerate() { if outp.script_pubkey == revokeable_p2wsh { let revk_outp = RevokedOutput::build( per_commitment_point, per_commitment_key, outp.value, - self.funding.channel_parameters.channel_type_features.supports_anchors_zero_fee_htlc_tx(), - self.funding.channel_parameters.clone(), - height, + funding_spent.channel_type_features().supports_anchors_zero_fee_htlc_tx(), + funding_spent.channel_parameters.clone(), height, ); let justice_package = PackageTemplate::build_package( commitment_txid, idx as u32, @@ -4335,15 +4392,14 @@ impl ChannelMonitorImpl { if let Some(per_commitment_claimable_data) = per_commitment_option { for (htlc, _) in per_commitment_claimable_data { if let Some(transaction_output_index) = htlc.transaction_output_index { - if transaction_output_index as usize >= tx.output.len() || - tx.output[transaction_output_index as usize].value != htlc.to_bitcoin_amount() { + if transaction_output_index as usize >= commitment_tx.output.len() || + commitment_tx.output[transaction_output_index as usize].value != htlc.to_bitcoin_amount() { // per_commitment_data is corrupt or our commitment signing key leaked! return (claimable_outpoints, to_counterparty_output_info); } let revk_htlc_outp = RevokedHTLCOutput::build( per_commitment_point, per_commitment_key, htlc.clone(), - self.funding.channel_parameters.clone(), - height, + funding_spent.channel_parameters.clone(), height, ); let counterparty_spendable_height = if htlc.offered { htlc.cltv_expiry @@ -4368,7 +4424,7 @@ impl ChannelMonitorImpl { self.counterparty_commitment_txn_on_chain.insert(commitment_txid, commitment_number); if let Some(per_commitment_claimable_data) = per_commitment_option { - fail_unbroadcast_htlcs!(self, "revoked_counterparty", commitment_txid, tx, height, + fail_unbroadcast_htlcs!(self, "revoked_counterparty", commitment_txid, commitment_tx, height, block_hash, per_commitment_claimable_data.iter().map(|(htlc, htlc_source)| (htlc, htlc_source.as_ref().map(|htlc_source| htlc_source.as_ref())) ), logger); @@ -4378,11 +4434,13 @@ impl ChannelMonitorImpl { // commitment transactions. Thus, we can only debug-assert here when not // fuzzing. debug_assert!(cfg!(fuzzing), "We should have per-commitment option for any recognized old commitment txn"); - fail_unbroadcast_htlcs!(self, "revoked counterparty", commitment_txid, tx, height, + fail_unbroadcast_htlcs!(self, "revoked counterparty", commitment_txid, commitment_tx, height, block_hash, [].iter().map(|reference| *reference), logger); } } } else if let Some(per_commitment_claimable_data) = per_commitment_option { + assert_eq!(funding_spent.funding_txid(), funding_txid_spent); + // While this isn't useful yet, there is a potential race where if a counterparty // revokes a state at the same time as the commitment transaction for that state is // confirmed, and the watchtower receives the block before the user, the user could @@ -4393,25 +4451,26 @@ impl ChannelMonitorImpl { self.counterparty_commitment_txn_on_chain.insert(commitment_txid, commitment_number); log_info!(logger, "Got broadcast of non-revoked counterparty commitment transaction {}", commitment_txid); - fail_unbroadcast_htlcs!(self, "counterparty", commitment_txid, tx, height, block_hash, + fail_unbroadcast_htlcs!(self, "counterparty", commitment_txid, commitment_tx, height, block_hash, per_commitment_claimable_data.iter().map(|(htlc, htlc_source)| (htlc, htlc_source.as_ref().map(|htlc_source| htlc_source.as_ref())) ), logger); let (htlc_claim_reqs, counterparty_output_info) = - self.get_counterparty_output_claim_info(commitment_number, commitment_txid, Some(tx), per_commitment_option, Some(height)); + self.get_counterparty_output_claim_info(funding_spent, commitment_number, commitment_txid, Some(commitment_tx), per_commitment_option, Some(height)); to_counterparty_output_info = counterparty_output_info; for req in htlc_claim_reqs { claimable_outpoints.push(req); } - } + (claimable_outpoints, to_counterparty_output_info) } /// Returns the HTLC claim package templates and the counterparty output info #[rustfmt::skip] fn get_counterparty_output_claim_info( - &self, commitment_number: u64, commitment_txid: Txid, tx: Option<&Transaction>, + &self, funding_spent: &FundingScope, commitment_number: u64, commitment_txid: Txid, + tx: Option<&Transaction>, per_commitment_option: Option<&Vec<(HTLCOutputInCommitment, Option>)>>, confirmation_height: Option, ) -> (Vec, CommitmentTxCounterpartyOutputInfo) { @@ -4473,7 +4532,7 @@ impl ChannelMonitorImpl { CounterpartyOfferedHTLCOutput::build( *per_commitment_point, preimage.unwrap(), htlc.clone(), - self.funding.channel_parameters.clone(), + funding_spent.channel_parameters.clone(), confirmation_height, ) ) @@ -4482,7 +4541,7 @@ impl ChannelMonitorImpl { CounterpartyReceivedHTLCOutput::build( *per_commitment_point, htlc.clone(), - self.funding.channel_parameters.clone(), + funding_spent.channel_parameters.clone(), confirmation_height, ) ) @@ -4508,6 +4567,9 @@ impl ChannelMonitorImpl { }; let per_commitment_point = PublicKey::from_secret_key(&self.onchain_tx_handler.secp_ctx, &per_commitment_key); + let funding_spent = get_confirmed_funding_scope!(self); + debug_assert!(funding_spent.counterparty_claimable_outpoints.contains_key(commitment_txid)); + let htlc_txid = tx.compute_txid(); let mut claimable_outpoints = vec![]; let mut outputs_to_watch = None; @@ -4526,7 +4588,7 @@ impl ChannelMonitorImpl { log_error!(logger, "Got broadcast of revoked counterparty HTLC transaction, spending {}:{}", htlc_txid, idx); let revk_outp = RevokedOutput::build( per_commitment_point, per_commitment_key, tx.output[idx].value, false, - self.funding.channel_parameters.clone(), + funding_spent.channel_parameters.clone(), height, ); let justice_package = PackageTemplate::build_package( @@ -4640,66 +4702,65 @@ impl ChannelMonitorImpl { /// Should not be used if check_spend_revoked_transaction succeeds. /// Returns None unless the transaction is definitely one of our commitment transactions. fn check_spend_holder_transaction( - &mut self, tx: &Transaction, height: u32, block_hash: &BlockHash, logger: &L, + &mut self, commitment_txid: Txid, commitment_tx: &Transaction, height: u32, + block_hash: &BlockHash, logger: &L, ) -> Option<(Vec, TransactionOutputs)> where L::Target: Logger, { - let commitment_txid = tx.compute_txid(); - let mut claim_requests = Vec::new(); - let mut watch_outputs = Vec::new(); - - macro_rules! append_onchain_update { - ($updates: expr, $to_watch: expr) => { - claim_requests = $updates.0; - self.broadcasted_holder_revokable_script = $updates.1; - watch_outputs.append(&mut $to_watch); - }; - } + let funding_spent = get_confirmed_funding_scope!(self); // HTLCs set may differ between last and previous holder commitment txn, in case of one them hitting chain, ensure we cancel all HTLCs backward - let mut is_holder_tx = false; - - if self.funding.current_holder_commitment_tx.trust().txid() == commitment_txid { - is_holder_tx = true; - log_info!(logger, "Got broadcast of latest holder commitment tx {}, searching for available HTLCs to claim", commitment_txid); - let holder_commitment_tx = &self.funding.current_holder_commitment_tx; - let res = - self.get_broadcasted_holder_claims(&self.funding, holder_commitment_tx, height); - let mut to_watch = self.get_broadcasted_holder_watch_outputs(holder_commitment_tx); - append_onchain_update!(res, to_watch); - fail_unbroadcast_htlcs!( - self, - "latest holder", - commitment_txid, - tx, - height, - block_hash, - holder_commitment_htlcs!(self, CURRENT_WITH_SOURCES), - logger - ); - } else if let Some(holder_commitment_tx) = &self.funding.prev_holder_commitment_tx { - if holder_commitment_tx.trust().txid() == commitment_txid { - is_holder_tx = true; - log_info!(logger, "Got broadcast of previous holder commitment tx {}, searching for available HTLCs to claim", commitment_txid); - let res = - self.get_broadcasted_holder_claims(&self.funding, holder_commitment_tx, height); - let mut to_watch = self.get_broadcasted_holder_watch_outputs(holder_commitment_tx); - append_onchain_update!(res, to_watch); + let holder_commitment_tx = Some((&funding_spent.current_holder_commitment_tx, true)) + .filter(|(current_holder_commitment_tx, _)| { + current_holder_commitment_tx.trust().txid() == commitment_txid + }) + .or_else(|| { + funding_spent + .prev_holder_commitment_tx + .as_ref() + .map(|prev_holder_commitment_tx| (prev_holder_commitment_tx, false)) + .filter(|(prev_holder_commitment_tx, _)| { + prev_holder_commitment_tx.trust().txid() == commitment_txid + }) + }); + + if let Some((holder_commitment_tx, current)) = holder_commitment_tx { + let funding_txid_spent = commitment_tx.input[0].previous_output.txid; + assert_eq!(funding_spent.funding_txid(), funding_txid_spent); + + let current_msg = if current { "latest holder" } else { "previous holder" }; + log_info!(logger, "Got broadcast of {current_msg} commitment tx {commitment_txid}, searching for available HTLCs to claim"); + + let (claim_requests, broadcasted_holder_revokable_script) = + self.get_broadcasted_holder_claims(funding_spent, holder_commitment_tx, height); + self.broadcasted_holder_revokable_script = broadcasted_holder_revokable_script; + let watch_outputs = self.get_broadcasted_holder_watch_outputs(holder_commitment_tx); + + if current { fail_unbroadcast_htlcs!( self, - "previous holder", + current_msg, commitment_txid, - tx, + commitment_tx, + height, + block_hash, + holder_commitment_htlcs!(self, CURRENT_WITH_SOURCES), + logger + ); + } else { + fail_unbroadcast_htlcs!( + self, + current_msg, + commitment_txid, + commitment_tx, height, block_hash, holder_commitment_htlcs!(self, PREV_WITH_SOURCES).unwrap(), logger ); } - } - if is_holder_tx { Some((claim_requests, (commitment_txid, watch_outputs))) } else { None @@ -4863,7 +4924,7 @@ impl ChannelMonitorImpl { 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, &self.destination_script, fee_estimator, logger, + height + 1, &broadcaster, conf_target, &self.destination_script, fee_estimator, logger, ); Vec::new() } else { Vec::new() } @@ -4946,13 +5007,13 @@ impl ChannelMonitorImpl { .iter() .find(|funding| funding.funding_txid() == txid) { - debug_assert!(self.alternative_funding_confirmed.is_none()); - debug_assert!( + assert!(self.alternative_funding_confirmed.is_none()); + assert!( !self.onchain_events_awaiting_threshold_conf.iter() .any(|e| matches!(e.event, OnchainEvent::AlternativeFundingConfirmation {})) ); - debug_assert!(self.funding_spend_confirmed.is_none()); - debug_assert!( + assert!(self.funding_spend_confirmed.is_none()); + assert!( !self.onchain_events_awaiting_threshold_conf.iter() .any(|e| matches!(e.event, OnchainEvent::FundingSpendConfirmation { .. })) ); @@ -5011,18 +5072,30 @@ impl ChannelMonitorImpl { // commitment transactions and HTLC transactions will all only ever have one input // (except for HTLC transactions for channels with anchor outputs), which is an easy // way to filter out any potential non-matching txn for lazy filters. - // - // TODO(splicing): Produce commitment claims for currently confirmed funding. - let prevout = &tx.input[0].previous_output; - let funding_outpoint = self.get_funding_txo(); - if prevout.txid == funding_outpoint.txid && prevout.vout == funding_outpoint.index as u32 { - let mut balance_spendable_csv = None; - log_info!(logger, "Channel {} closed by funding output spend in txid {}.", - &self.channel_id(), txid); + if let Some(funding_txid_spent) = core::iter::once(&self.funding) + .chain(self.pending_funding.iter()) + .find(|funding| { + let funding_outpoint = funding.funding_outpoint().into_bitcoin_outpoint(); + funding_outpoint == tx.input[0].previous_output + }) + .map(|funding| funding.funding_txid()) + { + assert_eq!( + funding_txid_spent, + self.alternative_funding_confirmed + .map(|(txid, _)| txid) + .unwrap_or_else(|| self.funding.funding_txid()) + ); + log_info!(logger, "Channel {} closed by funding output spend in txid {txid}", + self.channel_id()); self.funding_spend_seen = true; + + let mut balance_spendable_csv = None; let mut commitment_tx_to_counterparty_output = None; + + // Is it a commitment transaction? if (tx.input[0].sequence.0 >> 8*3) as u8 == 0x80 && (tx.lock_time.to_consensus_u32() >> 8*3) as u8 == 0x20 { - if let Some((mut new_outpoints, new_outputs)) = self.check_spend_holder_transaction(&tx, height, &block_hash, &logger) { + if let Some((mut new_outpoints, new_outputs)) = self.check_spend_holder_transaction(txid, &tx, height, &block_hash, &logger) { if !new_outputs.1.is_empty() { watch_outputs.push(new_outputs); } @@ -5037,7 +5110,7 @@ impl ChannelMonitorImpl { watch_outputs.push((txid, new_watch_outputs)); let (mut new_outpoints, counterparty_output_idx_sats) = - self.check_spend_counterparty_transaction(&tx, height, &block_hash, &logger); + self.check_spend_counterparty_transaction(txid, &tx, height, &block_hash, &logger); commitment_tx_to_counterparty_output = counterparty_output_idx_sats; claimable_outpoints.append(&mut new_outpoints); @@ -5050,6 +5123,7 @@ impl ChannelMonitorImpl { should_broadcast_commitment = false; } } + self.onchain_events_awaiting_threshold_conf.push(OnchainEventEntry { txid, transaction: Some((*tx).clone()), @@ -5060,6 +5134,7 @@ impl ChannelMonitorImpl { commitment_tx_to_counterparty_output, }, }); + // Now that we've detected a confirmed commitment transaction, attempt to cancel // pending claims for any commitments that were previously confirmed such that // we don't continue claiming inputs that no longer exist. @@ -5338,16 +5413,18 @@ impl ChannelMonitorImpl { self.onchain_events_awaiting_threshold_conf.retain(|ref entry| entry.height < height); // TODO: Replace with `take_if` once our MSRV is >= 1.80. + let mut should_broadcast_commitment = false; if let Some((_, conf_height)) = self.alternative_funding_confirmed.as_ref() { if *conf_height == height { self.alternative_funding_confirmed.take(); - if self.holder_tx_signed { + if self.holder_tx_signed || self.funding_spend_seen { // Cancel any previous claims that are no longer valid as they stemmed from a - // different funding transaction. We'll wait until we see a funding transaction - // confirm again before attempting to broadcast the new valid holder commitment. + // different funding transaction. let new_holder_commitment_txid = self.funding.current_holder_commitment_tx.trust().txid(); self.cancel_prev_commitment_claims(&logger, &new_holder_commitment_txid); + + should_broadcast_commitment = true; } } } @@ -5355,9 +5432,15 @@ impl ChannelMonitorImpl { 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, &self.destination_script, &bounded_fee_estimator, logger + height, &broadcaster, conf_target, &self.destination_script, &bounded_fee_estimator, logger ); + // Only attempt to broadcast the new commitment after the `block_disconnected` call above so that + // it doesn't get removed from the set of pending claims. + if should_broadcast_commitment { + self.queue_latest_holder_commitment_txn_for_broadcast(&broadcaster, &bounded_fee_estimator, logger); + } + self.best_block = BestBlock::new(header.prev_blockhash, height - 1); } @@ -5392,24 +5475,32 @@ impl ChannelMonitorImpl { debug_assert!(!self.onchain_events_awaiting_threshold_conf.iter().any(|ref entry| entry.txid == *txid)); // TODO: Replace with `take_if` once our MSRV is >= 1.80. + let mut should_broadcast_commitment = false; if let Some((alternative_funding_txid, _)) = self.alternative_funding_confirmed.as_ref() { if alternative_funding_txid == txid { self.alternative_funding_confirmed.take(); - if self.holder_tx_signed { + if self.holder_tx_signed || self.funding_spend_seen { // Cancel any previous claims that are no longer valid as they stemmed from a - // different funding transaction. We'll wait until we see a funding transaction - // confirm again before attempting to broadcast the new valid holder commitment. + // different funding transaction. let new_holder_commitment_txid = self.funding.current_holder_commitment_tx.trust().txid(); self.cancel_prev_commitment_claims(&logger, &new_holder_commitment_txid); + + should_broadcast_commitment = true; } } } let conf_target = self.closure_conf_target(); self.onchain_tx_handler.transaction_unconfirmed( - txid, broadcaster, conf_target, &self.destination_script, fee_estimator, logger + txid, &broadcaster, conf_target, &self.destination_script, fee_estimator, logger ); + + // Only attempt to broadcast the new commitment after the `transaction_unconfirmed` call above so + // that it doesn't get removed from the set of pending claims. + if should_broadcast_commitment { + self.queue_latest_holder_commitment_txn_for_broadcast(&broadcaster, fee_estimator, logger); + } } /// Filters a block's `txdata` for transactions spending watched outputs or for any child @@ -5537,6 +5628,8 @@ impl ChannelMonitorImpl { fn is_resolving_htlc_output( &mut self, tx: &Transaction, height: u32, block_hash: &BlockHash, logger: &WithChannelMonitor, ) where L::Target: Logger { + let funding_spent = get_confirmed_funding_scope!(self); + 'outer_loop: for input in &tx.input { let mut payment_data = None; let htlc_claim = HTLCClaim::from_witness(&input.witness); @@ -5598,7 +5691,7 @@ impl ChannelMonitorImpl { } macro_rules! scan_commitment { - ($htlcs: expr, $tx_info: expr, $holder_tx: expr) => { + ($funding_spent: expr, $htlcs: expr, $tx_info: expr, $holder_tx: expr) => { for (ref htlc_output, source_option) in $htlcs { if Some(input.previous_output.vout) == htlc_output.transaction_output_index { if let Some(ref source) = source_option { @@ -5610,12 +5703,12 @@ impl ChannelMonitorImpl { // resolve the source HTLC with the original sender. payment_data = Some(((*source).clone(), htlc_output.payment_hash, htlc_output.amount_msat)); } else if !$holder_tx { - if let Some(current_counterparty_commitment_txid) = &self.funding.current_counterparty_commitment_txid { - check_htlc_valid_counterparty!(htlc_output, self.funding.counterparty_claimable_outpoints.get(current_counterparty_commitment_txid).unwrap()); + if let Some(current_counterparty_commitment_txid) = &$funding_spent.current_counterparty_commitment_txid { + check_htlc_valid_counterparty!(htlc_output, $funding_spent.counterparty_claimable_outpoints.get(current_counterparty_commitment_txid).unwrap()); } if payment_data.is_none() { - if let Some(prev_counterparty_commitment_txid) = &self.funding.prev_counterparty_commitment_txid { - check_htlc_valid_counterparty!(htlc_output, self.funding.counterparty_claimable_outpoints.get(prev_counterparty_commitment_txid).unwrap()); + if let Some(prev_counterparty_commitment_txid) = &$funding_spent.prev_counterparty_commitment_txid { + check_htlc_valid_counterparty!(htlc_output, $funding_spent.counterparty_claimable_outpoints.get(prev_counterparty_commitment_txid).unwrap()); } } } @@ -5643,23 +5736,24 @@ impl ChannelMonitorImpl { } } - if input.previous_output.txid == self.funding.current_holder_commitment_tx.trust().txid() { + if input.previous_output.txid == funding_spent.current_holder_commitment_tx.trust().txid() { scan_commitment!( - holder_commitment_htlcs!(self, CURRENT_WITH_SOURCES), + funding_spent, holder_commitment_htlcs!(self, CURRENT_WITH_SOURCES), "our latest holder commitment tx", true ); } - if let Some(prev_holder_commitment_tx) = self.funding.prev_holder_commitment_tx.as_ref() { + if let Some(prev_holder_commitment_tx) = funding_spent.prev_holder_commitment_tx.as_ref() { if input.previous_output.txid == prev_holder_commitment_tx.trust().txid() { scan_commitment!( - holder_commitment_htlcs!(self, PREV_WITH_SOURCES).unwrap(), + funding_spent, holder_commitment_htlcs!(self, PREV_WITH_SOURCES).unwrap(), "our previous holder commitment tx", true ); } } - if let Some(ref htlc_outputs) = self.funding.counterparty_claimable_outpoints.get(&input.previous_output.txid) { - scan_commitment!(htlc_outputs.iter().map(|&(ref a, ref b)| (a, b.as_ref().map(|boxed| &**boxed))), - "counterparty commitment tx", false); + if let Some(ref htlc_outputs) = funding_spent.counterparty_claimable_outpoints.get(&input.previous_output.txid) { + let htlcs = htlc_outputs.iter() + .map(|&(ref a, ref b)| (a, b.as_ref().map(|boxed| &**boxed))); + scan_commitment!(funding_spent, htlcs, "counterparty commitment tx", false); } // Check that scan_commitment, above, decided there is some source worth relaying an @@ -5739,7 +5833,7 @@ impl ChannelMonitorImpl { } #[rustfmt::skip] - fn get_spendable_outputs(&self, tx: &Transaction) -> Vec { + fn get_spendable_outputs(&self, funding_spent: &FundingScope, tx: &Transaction) -> Vec { let mut spendable_outputs = Vec::new(); for (i, outp) in tx.output.iter().enumerate() { if outp.script_pubkey == self.destination_script { @@ -5758,8 +5852,8 @@ impl ChannelMonitorImpl { output: outp.clone(), revocation_pubkey: broadcasted_holder_revokable_script.2, channel_keys_id: self.channel_keys_id, - channel_value_satoshis: self.funding.channel_parameters.channel_value_satoshis, - channel_transaction_parameters: Some(self.funding.channel_parameters.clone()), + channel_value_satoshis: funding_spent.channel_parameters.channel_value_satoshis, + channel_transaction_parameters: Some(funding_spent.channel_parameters.clone()), })); } } @@ -5768,8 +5862,8 @@ impl ChannelMonitorImpl { outpoint: OutPoint { txid: tx.compute_txid(), index: i as u16 }, output: outp.clone(), channel_keys_id: self.channel_keys_id, - channel_value_satoshis: self.funding.channel_parameters.channel_value_satoshis, - channel_transaction_parameters: Some(self.funding.channel_parameters.clone()), + channel_value_satoshis: funding_spent.channel_parameters.channel_value_satoshis, + channel_transaction_parameters: Some(funding_spent.channel_parameters.clone()), })); } if self.shutdown_script.as_ref() == Some(&outp.script_pubkey) { @@ -5789,7 +5883,8 @@ impl ChannelMonitorImpl { fn check_tx_and_push_spendable_outputs( &mut self, tx: &Transaction, height: u32, block_hash: &BlockHash, logger: &WithChannelMonitor, ) where L::Target: Logger { - for spendable_output in self.get_spendable_outputs(tx) { + let funding_spent = get_confirmed_funding_scope!(self); + for spendable_output in self.get_spendable_outputs(funding_spent, tx) { let entry = OnchainEventEntry { txid: tx.compute_txid(), transaction: Some(tx.clone()), diff --git a/lightning/src/chain/onchaintx.rs b/lightning/src/chain/onchaintx.rs index 95df05f4fd3..67ee01971ee 100644 --- a/lightning/src/chain/onchaintx.rs +++ b/lightning/src/chain/onchaintx.rs @@ -1120,7 +1120,7 @@ impl OnchainTxHandler { pub(super) fn transaction_unconfirmed( &mut self, txid: &Txid, - broadcaster: B, + broadcaster: &B, conf_target: ConfirmationTarget, destination_script: &Script, fee_estimator: &LowerBoundedFeeEstimator, @@ -1146,7 +1146,7 @@ impl OnchainTxHandler { #[rustfmt::skip] pub(super) fn block_disconnected( - &mut self, height: u32, broadcaster: B, conf_target: ConfirmationTarget, + &mut self, height: u32, broadcaster: &B, conf_target: ConfirmationTarget, destination_script: &Script, fee_estimator: &LowerBoundedFeeEstimator, logger: &L, ) where B::Target: BroadcasterInterface,