X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=blobdiff_plain;f=lightning%2Fsrc%2Fchain%2Fchannelmonitor.rs;h=36c1eac1ab0e5a1db91618fb46b65e1e255e052b;hb=abf4e79dcda4c97dd80dd00233de32507184543a;hp=2be7f4cf1f52ebe0d49e8224514b05e07d2d69d0;hpb=36e6023633ada2029bc5b6fbe37d96eb2eb6bd8e;p=rust-lightning diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index 2be7f4cf..36c1eac1 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -42,7 +42,7 @@ use crate::chain; use crate::chain::{BestBlock, WatchedOutput}; use crate::chain::chaininterface::{BroadcasterInterface, FeeEstimator, LowerBoundedFeeEstimator}; use crate::chain::transaction::{OutPoint, TransactionData}; -use crate::chain::keysinterface::{SpendableOutputDescriptor, StaticPaymentOutputDescriptor, DelayedPaymentOutputDescriptor, Sign, KeysInterface}; +use crate::chain::keysinterface::{SpendableOutputDescriptor, StaticPaymentOutputDescriptor, DelayedPaymentOutputDescriptor, Sign, SignerProvider, EntropySource}; #[cfg(anchors)] use crate::chain::onchaintx::ClaimEvent; use crate::chain::onchaintx::OnchainTxHandler; @@ -53,7 +53,7 @@ use crate::util::ser::{Readable, ReadableArgs, MaybeReadable, Writer, Writeable, use crate::util::byte_utils; use crate::util::events::Event; #[cfg(anchors)] -use crate::util::events::{AnchorDescriptor, BumpTransactionEvent}; +use crate::util::events::{AnchorDescriptor, HTLCDescriptor, BumpTransactionEvent}; use crate::prelude::*; use core::{cmp, mem}; @@ -647,6 +647,7 @@ struct IrrevocablyResolvedHTLC { /// was not present in the confirmed commitment transaction), HTLC-Success, or HTLC-Timeout /// transaction. resolving_txid: Option, // Added as optional, but always filled in, in 0.0.110 + resolving_tx: Option, /// Only set if the HTLC claim was ours using a payment preimage payment_preimage: Option, } @@ -662,6 +663,7 @@ impl Writeable for IrrevocablyResolvedHTLC { (0, mapped_commitment_tx_output_idx, required), (1, self.resolving_txid, option), (2, self.payment_preimage, option), + (3, self.resolving_tx, option), }); Ok(()) } @@ -672,15 +674,18 @@ impl Readable for IrrevocablyResolvedHTLC { let mut mapped_commitment_tx_output_idx = 0; let mut resolving_txid = None; let mut payment_preimage = None; + let mut resolving_tx = None; read_tlv_fields!(reader, { (0, mapped_commitment_tx_output_idx, required), (1, resolving_txid, option), (2, payment_preimage, option), + (3, resolving_tx, option), }); Ok(Self { commitment_tx_output_idx: if mapped_commitment_tx_output_idx == u32::max_value() { None } else { Some(mapped_commitment_tx_output_idx) }, resolving_txid, payment_preimage, + resolving_tx, }) } } @@ -1526,6 +1531,7 @@ impl ChannelMonitorImpl { if let Some(v) = htlc.transaction_output_index { v } else { return None; }; let mut htlc_spend_txid_opt = None; + let mut htlc_spend_tx_opt = None; let mut holder_timeout_spend_pending = None; let mut htlc_spend_pending = None; let mut holder_delayed_output_pending = None; @@ -1534,7 +1540,9 @@ impl ChannelMonitorImpl { OnchainEvent::HTLCUpdate { commitment_tx_output_idx, htlc_value_satoshis, .. } if commitment_tx_output_idx == Some(htlc_commitment_tx_output_idx) => { debug_assert!(htlc_spend_txid_opt.is_none()); - htlc_spend_txid_opt = event.transaction.as_ref().map(|tx| tx.txid()); + htlc_spend_txid_opt = Some(&event.txid); + debug_assert!(htlc_spend_tx_opt.is_none()); + htlc_spend_tx_opt = event.transaction.as_ref(); debug_assert!(holder_timeout_spend_pending.is_none()); debug_assert_eq!(htlc_value_satoshis.unwrap(), htlc.amount_msat / 1000); holder_timeout_spend_pending = Some(event.confirmation_threshold()); @@ -1542,7 +1550,9 @@ impl ChannelMonitorImpl { OnchainEvent::HTLCSpendConfirmation { commitment_tx_output_idx, preimage, .. } if commitment_tx_output_idx == htlc_commitment_tx_output_idx => { debug_assert!(htlc_spend_txid_opt.is_none()); - htlc_spend_txid_opt = event.transaction.as_ref().map(|tx| tx.txid()); + htlc_spend_txid_opt = Some(&event.txid); + debug_assert!(htlc_spend_tx_opt.is_none()); + htlc_spend_tx_opt = event.transaction.as_ref(); debug_assert!(htlc_spend_pending.is_none()); htlc_spend_pending = Some((event.confirmation_threshold(), preimage.is_some())); }, @@ -1558,19 +1568,32 @@ impl ChannelMonitorImpl { let htlc_resolved = self.htlcs_resolved_on_chain.iter() .find(|v| if v.commitment_tx_output_idx == Some(htlc_commitment_tx_output_idx) { debug_assert!(htlc_spend_txid_opt.is_none()); - htlc_spend_txid_opt = v.resolving_txid; + htlc_spend_txid_opt = v.resolving_txid.as_ref(); + debug_assert!(htlc_spend_tx_opt.is_none()); + htlc_spend_tx_opt = v.resolving_tx.as_ref(); true } else { false }); debug_assert!(holder_timeout_spend_pending.is_some() as u8 + htlc_spend_pending.is_some() as u8 + htlc_resolved.is_some() as u8 <= 1); + let htlc_commitment_outpoint = BitcoinOutPoint::new(confirmed_txid.unwrap(), htlc_commitment_tx_output_idx); let htlc_output_to_spend = if let Some(txid) = htlc_spend_txid_opt { - debug_assert!( - self.onchain_tx_handler.channel_transaction_parameters.opt_anchors.is_none(), - "This code needs updating for anchors"); - BitcoinOutPoint::new(txid, 0) + // Because HTLC transactions either only have 1 input and 1 output (pre-anchors) or + // are signed with SIGHASH_SINGLE|ANYONECANPAY under BIP-0143 (post-anchors), we can + // locate the correct output by ensuring its adjacent input spends the HTLC output + // in the commitment. + if let Some(ref tx) = htlc_spend_tx_opt { + let htlc_input_idx_opt = tx.input.iter().enumerate() + .find(|(_, input)| input.previous_output == htlc_commitment_outpoint) + .map(|(idx, _)| idx as u32); + debug_assert!(htlc_input_idx_opt.is_some()); + BitcoinOutPoint::new(*txid, htlc_input_idx_opt.unwrap_or(0)) + } else { + debug_assert!(!self.onchain_tx_handler.opt_anchors()); + BitcoinOutPoint::new(*txid, 0) + } } else { - BitcoinOutPoint::new(confirmed_txid.unwrap(), htlc_commitment_tx_output_idx) + htlc_commitment_outpoint }; let htlc_output_spend_pending = self.onchain_tx_handler.is_output_spend_pending(&htlc_output_to_spend); @@ -1594,8 +1617,7 @@ impl ChannelMonitorImpl { } = &event.event { if event.transaction.as_ref().map(|tx| tx.input.iter().any(|inp| { if let Some(htlc_spend_txid) = htlc_spend_txid_opt { - Some(tx.txid()) == htlc_spend_txid_opt || - inp.previous_output.txid == htlc_spend_txid + tx.txid() == *htlc_spend_txid || inp.previous_output.txid == *htlc_spend_txid } else { Some(inp.previous_output.txid) == confirmed_txid && inp.previous_output.vout == htlc_commitment_tx_output_idx @@ -2303,6 +2325,17 @@ impl ChannelMonitorImpl { log_trace!(logger, "Updating ChannelMonitor: channel force closed, should broadcast: {}", should_broadcast); self.lockdown_from_offchain = true; if *should_broadcast { + // There's no need to broadcast our commitment transaction if we've seen one + // confirmed (even with 1 confirmation) as it'll be rejected as + // duplicate/conflicting. + let detected_funding_spend = self.funding_spend_confirmed.is_some() || + self.onchain_events_awaiting_threshold_conf.iter().find(|event| match event.event { + OnchainEvent::FundingSpendConfirmation { .. } => true, + _ => false, + }).is_some(); + if detected_funding_spend { + continue; + } self.broadcast_latest_holder_commitment_txn(broadcaster, logger); // If the channel supports anchor outputs, we'll need to emit an external // event to be consumed such that a child transaction is broadcast with a @@ -2403,6 +2436,27 @@ impl ChannelMonitorImpl { pending_htlcs, })); }, + ClaimEvent::BumpHTLC { + target_feerate_sat_per_1000_weight, htlcs, + } => { + let mut htlc_descriptors = Vec::with_capacity(htlcs.len()); + for htlc in htlcs { + htlc_descriptors.push(HTLCDescriptor { + channel_keys_id: self.channel_keys_id, + channel_value_satoshis: self.channel_value_satoshis, + channel_parameters: self.onchain_tx_handler.channel_transaction_parameters.clone(), + commitment_txid: htlc.commitment_txid, + per_commitment_number: htlc.per_commitment_number, + htlc: htlc.htlc, + preimage: htlc.preimage, + counterparty_sig: htlc.counterparty_sig, + }); + } + ret.push(Event::BumpTransaction(BumpTransactionEvent::HTLCResolution { + target_feerate_sat_per_1000_weight, + htlc_descriptors, + })); + } } } ret @@ -2623,31 +2677,49 @@ impl ChannelMonitorImpl { } /// Attempts to claim a counterparty HTLC-Success/HTLC-Timeout's outputs using the revocation key - fn check_spend_counterparty_htlc(&mut self, tx: &Transaction, commitment_number: u64, height: u32, logger: &L) -> (Vec, Option) where L::Target: Logger { - let htlc_txid = tx.txid(); - if tx.input.len() != 1 || tx.output.len() != 1 || tx.input[0].witness.len() != 5 { - return (Vec::new(), None) - } - - macro_rules! ignore_error { - ( $thing : expr ) => { - match $thing { - Ok(a) => a, - Err(_) => return (Vec::new(), None) - } - }; - } - + fn check_spend_counterparty_htlc( + &mut self, tx: &Transaction, commitment_number: u64, commitment_txid: &Txid, height: u32, logger: &L + ) -> (Vec, Option) where L::Target: Logger { let secret = if let Some(secret) = self.get_secret(commitment_number) { secret } else { return (Vec::new(), None); }; - let per_commitment_key = ignore_error!(SecretKey::from_slice(&secret)); + let per_commitment_key = match SecretKey::from_slice(&secret) { + Ok(key) => key, + Err(_) => return (Vec::new(), None) + }; let per_commitment_point = PublicKey::from_secret_key(&self.secp_ctx, &per_commitment_key); - log_error!(logger, "Got broadcast of revoked counterparty HTLC transaction, spending {}:{}", htlc_txid, 0); - let revk_outp = RevokedOutput::build(per_commitment_point, self.counterparty_commitment_params.counterparty_delayed_payment_base_key, self.counterparty_commitment_params.counterparty_htlc_base_key, per_commitment_key, tx.output[0].value, self.counterparty_commitment_params.on_counterparty_tx_csv); - let justice_package = PackageTemplate::build_package(htlc_txid, 0, PackageSolvingData::RevokedOutput(revk_outp), height + self.counterparty_commitment_params.on_counterparty_tx_csv as u32, true, height); - let claimable_outpoints = vec!(justice_package); - let outputs = vec![(0, tx.output[0].clone())]; - (claimable_outpoints, Some((htlc_txid, outputs))) + let htlc_txid = tx.txid(); + let mut claimable_outpoints = vec![]; + let mut outputs_to_watch = None; + // Previously, we would only claim HTLCs from revoked HTLC transactions if they had 1 input + // with a witness of 5 elements and 1 output. This wasn't enough for anchor outputs, as the + // counterparty can now aggregate multiple HTLCs into a single transaction thanks to + // `SIGHASH_SINGLE` remote signatures, leading us to not claim any HTLCs upon seeing a + // confirmed revoked HTLC transaction (for more details, see + // https://lists.linuxfoundation.org/pipermail/lightning-dev/2022-April/003561.html). + // + // We make sure we're not vulnerable to this case by checking all inputs of the transaction, + // and claim those which spend the commitment transaction, have a witness of 5 elements, and + // have a corresponding output at the same index within the transaction. + for (idx, input) in tx.input.iter().enumerate() { + if input.previous_output.txid == *commitment_txid && input.witness.len() == 5 && tx.output.get(idx).is_some() { + log_error!(logger, "Got broadcast of revoked counterparty HTLC transaction, spending {}:{}", htlc_txid, idx); + let revk_outp = RevokedOutput::build( + per_commitment_point, self.counterparty_commitment_params.counterparty_delayed_payment_base_key, + self.counterparty_commitment_params.counterparty_htlc_base_key, per_commitment_key, + tx.output[idx].value, self.counterparty_commitment_params.on_counterparty_tx_csv + ); + let justice_package = PackageTemplate::build_package( + htlc_txid, idx as u32, PackageSolvingData::RevokedOutput(revk_outp), + height + self.counterparty_commitment_params.on_counterparty_tx_csv as u32, true, height + ); + claimable_outpoints.push(justice_package); + if outputs_to_watch.is_none() { + outputs_to_watch = Some((htlc_txid, vec![])); + } + outputs_to_watch.as_mut().unwrap().1.push((idx as u32, tx.output[idx].clone())); + } + } + (claimable_outpoints, outputs_to_watch) } // Returns (1) `PackageTemplate`s that can be given to the OnchainTxHandler, so that the handler can @@ -2661,18 +2733,28 @@ impl ChannelMonitorImpl { for &(ref htlc, _, _) in holder_tx.htlc_outputs.iter() { if let Some(transaction_output_index) = htlc.transaction_output_index { - let htlc_output = if htlc.offered { - HolderHTLCOutput::build_offered(htlc.amount_msat, htlc.cltv_expiry) + let (htlc_output, aggregable) = if htlc.offered { + let htlc_output = HolderHTLCOutput::build_offered( + htlc.amount_msat, htlc.cltv_expiry, self.onchain_tx_handler.opt_anchors() + ); + (htlc_output, false) + } else { + let payment_preimage = if let Some(preimage) = self.payment_preimages.get(&htlc.payment_hash) { + preimage.clone() } else { - let payment_preimage = if let Some(preimage) = self.payment_preimages.get(&htlc.payment_hash) { - preimage.clone() - } else { - // We can't build an HTLC-Success transaction without the preimage - continue; - }; - HolderHTLCOutput::build_accepted(payment_preimage, htlc.amount_msat) + // We can't build an HTLC-Success transaction without the preimage + continue; }; - let htlc_package = PackageTemplate::build_package(holder_tx.txid, transaction_output_index, PackageSolvingData::HolderHTLCOutput(htlc_output), htlc.cltv_expiry, false, conf_height); + let htlc_output = HolderHTLCOutput::build_accepted( + payment_preimage, htlc.amount_msat, self.onchain_tx_handler.opt_anchors() + ); + (htlc_output, self.onchain_tx_handler.opt_anchors()) + }; + let htlc_package = PackageTemplate::build_package( + holder_tx.txid, transaction_output_index, + PackageSolvingData::HolderHTLCOutput(htlc_output), + htlc.cltv_expiry, aggregable, conf_height + ); claim_requests.push(htlc_package); } } @@ -2905,9 +2987,9 @@ impl ChannelMonitorImpl { if tx.input.len() == 1 { // Assuming our keys were not leaked (in which case we're screwed no matter what), - // commitment transactions and HTLC transactions will all only ever have one input, - // which is an easy way to filter out any potential non-matching txn for lazy - // filters. + // 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. let prevout = &tx.input[0].previous_output; if prevout.txid == self.funding_info.0.txid && prevout.vout == self.funding_info.0.index as u32 { let mut balance_spendable_csv = None; @@ -2945,22 +3027,33 @@ impl ChannelMonitorImpl { commitment_tx_to_counterparty_output, }, }); - } else { - if let Some(&commitment_number) = self.counterparty_commitment_txn_on_chain.get(&prevout.txid) { - let (mut new_outpoints, new_outputs_option) = self.check_spend_counterparty_htlc(&tx, commitment_number, height, &logger); + } + } + if tx.input.len() >= 1 { + // While all commitment transactions have one input, HTLC transactions may have more + // if the HTLC was present in an anchor channel. HTLCs can also be resolved in a few + // other ways which can have more than one output. + for tx_input in &tx.input { + let commitment_txid = tx_input.previous_output.txid; + if let Some(&commitment_number) = self.counterparty_commitment_txn_on_chain.get(&commitment_txid) { + let (mut new_outpoints, new_outputs_option) = self.check_spend_counterparty_htlc( + &tx, commitment_number, &commitment_txid, height, &logger + ); claimable_outpoints.append(&mut new_outpoints); if let Some(new_outputs) = new_outputs_option { watch_outputs.push(new_outputs); } + // Since there may be multiple HTLCs for this channel (all spending the + // same commitment tx) being claimed by the counterparty within the same + // transaction, and `check_spend_counterparty_htlc` already checks all the + // ones relevant to this channel, we can safely break from our loop. + break; } } - } - // While all commitment/HTLC-Success/HTLC-Timeout transactions have one input, HTLCs - // can also be resolved in a few other ways which can have more than one output. Thus, - // we call is_resolving_htlc_output here outside of the tx.input.len() == 1 check. - self.is_resolving_htlc_output(&tx, height, &block_hash, &logger); + self.is_resolving_htlc_output(&tx, height, &block_hash, &logger); - self.is_paying_spendable_output(&tx, height, &block_hash, &logger); + self.is_paying_spendable_output(&tx, height, &block_hash, &logger); + } } if height > self.best_block.height() { @@ -3074,7 +3167,9 @@ impl ChannelMonitorImpl { htlc_value_satoshis, })); self.htlcs_resolved_on_chain.push(IrrevocablyResolvedHTLC { - commitment_tx_output_idx, resolving_txid: Some(entry.txid), + commitment_tx_output_idx, + resolving_txid: Some(entry.txid), + resolving_tx: entry.transaction, payment_preimage: None, }); }, @@ -3087,7 +3182,9 @@ impl ChannelMonitorImpl { }, OnchainEvent::HTLCSpendConfirmation { commitment_tx_output_idx, preimage, .. } => { self.htlcs_resolved_on_chain.push(IrrevocablyResolvedHTLC { - commitment_tx_output_idx: Some(commitment_tx_output_idx), resolving_txid: Some(entry.txid), + commitment_tx_output_idx: Some(commitment_tx_output_idx), + resolving_txid: Some(entry.txid), + resolving_tx: entry.transaction, payment_preimage: preimage, }); }, @@ -3607,9 +3704,9 @@ where const MAX_ALLOC_SIZE: usize = 64*1024; -impl<'a, K: KeysInterface> ReadableArgs<&'a K> - for (BlockHash, ChannelMonitor) { - fn read(reader: &mut R, keys_manager: &'a K) -> Result { +impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP)> + for (BlockHash, ChannelMonitor) { + fn read(reader: &mut R, args: (&'a ES, &'b SP)) -> Result { macro_rules! unwrap_obj { ($key: expr) => { match $key { @@ -3619,6 +3716,8 @@ impl<'a, K: KeysInterface> ReadableArgs<&'a K> } } + let (entropy_source, signer_provider) = args; + let _ver = read_ver_prefix!(reader, SERIALIZATION_VERSION); let latest_update_id: u64 = Readable::read(reader)?; @@ -3792,8 +3891,8 @@ impl<'a, K: KeysInterface> ReadableArgs<&'a K> return Err(DecodeError::InvalidValue); } } - let onchain_tx_handler: OnchainTxHandler = ReadableArgs::read( - reader, (keys_manager, channel_value_satoshis, channel_keys_id) + let onchain_tx_handler: OnchainTxHandler = ReadableArgs::read( + reader, (entropy_source, signer_provider, channel_value_satoshis, channel_keys_id) )?; let lockdown_from_offchain = Readable::read(reader)?; @@ -3833,7 +3932,7 @@ impl<'a, K: KeysInterface> ReadableArgs<&'a K> }); let mut secp_ctx = Secp256k1::new(); - secp_ctx.seeded_randomize(&keys_manager.get_secure_random_bytes()); + secp_ctx.seeded_randomize(&entropy_source.get_secure_random_bytes()); Ok((best_block.block_hash(), ChannelMonitor::from_impl(ChannelMonitorImpl { latest_update_id, @@ -3922,7 +4021,7 @@ mod tests { use crate::ln::{PaymentPreimage, PaymentHash}; use crate::ln::chan_utils; use crate::ln::chan_utils::{HTLCOutputInCommitment, ChannelPublicKeys, ChannelTransactionParameters, HolderCommitmentTransaction, CounterpartyChannelTransactionParameters}; - use crate::ln::channelmanager::{self, PaymentSendFailure, PaymentId}; + use crate::ln::channelmanager::{PaymentSendFailure, PaymentId}; use crate::ln::functional_test_utils::*; use crate::ln::script::ShutdownScript; use crate::util::errors::APIError; @@ -3950,10 +4049,8 @@ mod tests { let node_cfgs = create_node_cfgs(3, &chanmon_cfgs); let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, None, None]); let nodes = create_network(3, &node_cfgs, &node_chanmgrs); - let channel = create_announced_chan_between_nodes( - &nodes, 0, 1, channelmanager::provided_init_features(), channelmanager::provided_init_features()); - create_announced_chan_between_nodes( - &nodes, 1, 2, channelmanager::provided_init_features(), channelmanager::provided_init_features()); + let channel = create_announced_chan_between_nodes(&nodes, 0, 1); + create_announced_chan_between_nodes(&nodes, 1, 2); // Rebalance somewhat send_payment(&nodes[0], &[&nodes[1]], 10_000_000); @@ -3982,7 +4079,7 @@ mod tests { let (_, pre_update_monitor) = <(BlockHash, ChannelMonitor)>::read( &mut io::Cursor::new(&get_monitor!(nodes[1], channel.2).encode()), - &nodes[1].keys_manager.backing).unwrap(); + (&nodes[1].keys_manager.backing, &nodes[1].keys_manager.backing)).unwrap(); // If the ChannelManager tries to update the channel, however, the ChainMonitor will pass // the update through to the ChannelMonitor which will refuse it (as the channel is closed).