X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=blobdiff_plain;f=lightning%2Fsrc%2Fchain%2Fchannelmonitor.rs;h=b5f6bce228a3886ee17d337ddcac2f103ba32cd7;hb=refs%2Fheads%2F2021-07-detect-htlcs-on-local-commitment;hp=42c3e13a34feeff236a1b19712470808f0b19f69;hpb=f06f9d11365360dc2add96acd7916673ea9ce383;p=rust-lightning diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index 42c3e13a..b5f6bce2 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -53,9 +53,9 @@ use util::events::Event; use prelude::*; use core::{cmp, mem}; -use std::io::Error; +use io::{self, Error}; use core::ops::Deref; -use std::sync::Mutex; +use sync::Mutex; /// An update generated by the underlying Channel itself which contains some new information the /// ChannelMonitor should be made aware of. @@ -88,7 +88,7 @@ pub struct ChannelMonitorUpdate { pub const CLOSED_CHANNEL_UPDATE_ID: u64 = core::u64::MAX; impl Writeable for ChannelMonitorUpdate { - fn write(&self, w: &mut W) -> Result<(), ::std::io::Error> { + fn write(&self, w: &mut W) -> Result<(), io::Error> { write_ver_prefix!(w, SERIALIZATION_VERSION, MIN_SERIALIZATION_VERSION); self.update_id.write(w)?; (self.updates.len() as u64).write(w)?; @@ -100,7 +100,7 @@ impl Writeable for ChannelMonitorUpdate { } } impl Readable for ChannelMonitorUpdate { - fn read(r: &mut R) -> Result { + fn read(r: &mut R) -> Result { let _ver = read_ver_prefix!(r, SERIALIZATION_VERSION); let update_id: u64 = Readable::read(r)?; let len: u64 = Readable::read(r)?; @@ -199,10 +199,12 @@ pub enum MonitorEvent { pub struct HTLCUpdate { pub(crate) payment_hash: PaymentHash, pub(crate) payment_preimage: Option, - pub(crate) source: HTLCSource + pub(crate) source: HTLCSource, + pub(crate) onchain_value_satoshis: Option, } impl_writeable_tlv_based!(HTLCUpdate, { (0, payment_hash, required), + (1, onchain_value_satoshis, option), (2, source, required), (4, payment_preimage, option), }); @@ -293,7 +295,7 @@ struct CounterpartyCommitmentTransaction { } impl Writeable for CounterpartyCommitmentTransaction { - fn write(&self, w: &mut W) -> Result<(), ::std::io::Error> { + fn write(&self, w: &mut W) -> Result<(), io::Error> { w.write_all(&byte_utils::be64_to_array(self.per_htlc.len() as u64))?; for (ref txid, ref htlcs) in self.per_htlc.iter() { w.write_all(&txid[..])?; @@ -311,7 +313,7 @@ impl Writeable for CounterpartyCommitmentTransaction { } } impl Readable for CounterpartyCommitmentTransaction { - fn read(r: &mut R) -> Result { + fn read(r: &mut R) -> Result { let counterparty_commitment_transaction = { let per_htlc_len: u64 = Readable::read(r)?; let mut per_htlc = HashMap::with_capacity(cmp::min(per_htlc_len as usize, MAX_ALLOC_SIZE / 64)); @@ -385,6 +387,7 @@ enum OnchainEvent { HTLCUpdate { source: HTLCSource, payment_hash: PaymentHash, + onchain_value_satoshis: Option, }, MaturingOutput { descriptor: SpendableOutputDescriptor, @@ -400,6 +403,7 @@ impl_writeable_tlv_based!(OnchainEventEntry, { impl_writeable_tlv_based_enum!(OnchainEvent, (0, HTLCUpdate) => { (0, source, required), + (1, onchain_value_satoshis, option), (2, payment_hash, required), }, (1, MaturingOutput) => { @@ -506,6 +510,9 @@ pub(crate) struct ChannelMonitorImpl { on_holder_tx_csv: u16, commitment_secrets: CounterpartyCommitmentSecrets, + /// The set of outpoints in each counterparty commitment transaction. We always need at least + /// the payment hash from `HTLCOutputInCommitment` to claim even a revoked commitment + /// transaction broadcast as we need to be able to construct the witness script in all cases. counterparty_claimable_outpoints: HashMap>)>>, /// We cannot identify HTLC-Success or HTLC-Timeout transactions by themselves on the chain. /// Nor can we figure out their commitment numbers without the commitment transaction they are @@ -1197,6 +1204,81 @@ impl ChannelMonitor { } } +/// Compares a broadcasted commitment transaction's HTLCs with those in the latest state, +/// failing any HTLCs which didn't make it into the broadcasted commitment transaction back +/// after ANTI_REORG_DELAY blocks. +/// +/// We always compare against the set of HTLCs in counterparty commitment transactions, as those +/// are the commitment transactions which are generated by us. The off-chain state machine in +/// `Channel` will automatically resolve any HTLCs which were never included in a commitment +/// transaction when it detects channel closure, but it is up to us to ensure any HTLCs which were +/// included in a remote commitment transaction are failed back if they are not present in the +/// broadcasted commitment transaction. +/// +/// Specifically, the removal process for HTLCs in `Channel` is always based on the counterparty +/// sending a `revoke_and_ack`, which causes us to clear `prev_counterparty_commitment_txid`. Thus, +/// as long as we examine both the current counterparty commitment transaction and, if it hasn't +/// been revoked yet, the previous one, we we will never "forget" to resolve an HTLC. +macro_rules! fail_unbroadcast_htlcs { + ($self: expr, $commitment_tx_type: expr, $commitment_tx_conf_height: expr, $confirmed_htlcs_list: expr, $logger: expr) => { { + macro_rules! check_htlc_fails { + ($txid: expr, $commitment_tx: expr) => { + if let Some(ref latest_outpoints) = $self.counterparty_claimable_outpoints.get($txid) { + for &(ref htlc, ref source_option) in latest_outpoints.iter() { + if let &Some(ref source) = source_option { + // Check if the HTLC is present in the commitment transaction that was + // broadcast, but not if it was below the dust limit, which we should + // fail backwards immediately as there is no way for us to learn the + // payment_preimage. + // Note that if the dust limit were allowed to change between + // commitment transactions we'd want to be check whether *any* + // broadcastable commitment transaction has the HTLC in it, but it + // cannot currently change after channel initialization, so we don't + // need to here. + let confirmed_htlcs_iter: &mut Iterator)> = &mut $confirmed_htlcs_list; + let mut matched_htlc = false; + for (ref broadcast_htlc, ref broadcast_source) in confirmed_htlcs_iter { + if broadcast_htlc.transaction_output_index.is_some() && Some(&**source) == *broadcast_source { + matched_htlc = true; + break; + } + } + if matched_htlc { continue; } + $self.onchain_events_awaiting_threshold_conf.retain(|ref entry| { + if entry.height != $commitment_tx_conf_height { return true; } + match entry.event { + OnchainEvent::HTLCUpdate { source: ref update_source, .. } => { + *update_source != **source + }, + _ => true, + } + }); + let entry = OnchainEventEntry { + txid: *$txid, + height: $commitment_tx_conf_height, + event: OnchainEvent::HTLCUpdate { + source: (**source).clone(), + payment_hash: htlc.payment_hash.clone(), + onchain_value_satoshis: Some(htlc.amount_msat / 1000), + }, + }; + log_trace!($logger, "Failing HTLC with payment_hash {} from {} counterparty commitment tx due to broadcast of {} commitment transaction, waiting for confirmation (at height {})", + log_bytes!(htlc.payment_hash.0), $commitment_tx, $commitment_tx_type, entry.confirmation_threshold()); + $self.onchain_events_awaiting_threshold_conf.push(entry); + } + } + } + } + } + if let Some(ref txid) = $self.current_counterparty_commitment_txid { + check_htlc_fails!(txid, "current"); + } + if let Some(ref txid) = $self.prev_counterparty_commitment_txid { + check_htlc_fails!(txid, "previous"); + } + } } +} + impl ChannelMonitorImpl { /// Inserts a revocation secret into this channel monitor. Prunes old preimages if neither /// needed by holder commitment transactions HTCLs nor by counterparty ones. Unless we haven't already seen @@ -1554,42 +1636,7 @@ impl ChannelMonitorImpl { } self.counterparty_commitment_txn_on_chain.insert(commitment_txid, commitment_number); - macro_rules! check_htlc_fails { - ($txid: expr, $commitment_tx: expr) => { - if let Some(ref outpoints) = self.counterparty_claimable_outpoints.get($txid) { - for &(ref htlc, ref source_option) in outpoints.iter() { - if let &Some(ref source) = source_option { - self.onchain_events_awaiting_threshold_conf.retain(|ref entry| { - if entry.height != height { return true; } - match entry.event { - OnchainEvent::HTLCUpdate { source: ref update_source, .. } => { - *update_source != **source - }, - _ => true, - } - }); - let entry = OnchainEventEntry { - txid: *$txid, - height, - event: OnchainEvent::HTLCUpdate { - source: (**source).clone(), - payment_hash: htlc.payment_hash.clone(), - }, - }; - log_info!(logger, "Failing HTLC with payment_hash {} from {} counterparty commitment tx due to broadcast of revoked counterparty commitment transaction, waiting for confirmation (at height {})", log_bytes!(htlc.payment_hash.0), $commitment_tx, entry.confirmation_threshold()); - self.onchain_events_awaiting_threshold_conf.push(entry); - } - } - } - } - } - if let Some(ref txid) = self.current_counterparty_commitment_txid { - check_htlc_fails!(txid, "current"); - } - if let Some(ref txid) = self.prev_counterparty_commitment_txid { - check_htlc_fails!(txid, "counterparty"); - } - // No need to check holder commitment txn, symmetric HTLCSource must be present as per-htlc data on counterparty commitment tx + fail_unbroadcast_htlcs!(self, "revoked counterparty", height, [].iter().map(|a| *a), logger); } } else if let Some(per_commitment_data) = per_commitment_option { // While this isn't useful yet, there is a potential race where if a counterparty @@ -1605,55 +1652,7 @@ 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); - - macro_rules! check_htlc_fails { - ($txid: expr, $commitment_tx: expr, $id: tt) => { - if let Some(ref latest_outpoints) = self.counterparty_claimable_outpoints.get($txid) { - $id: for &(ref htlc, ref source_option) in latest_outpoints.iter() { - if let &Some(ref source) = source_option { - // Check if the HTLC is present in the commitment transaction that was - // broadcast, but not if it was below the dust limit, which we should - // fail backwards immediately as there is no way for us to learn the - // payment_preimage. - // Note that if the dust limit were allowed to change between - // commitment transactions we'd want to be check whether *any* - // broadcastable commitment transaction has the HTLC in it, but it - // cannot currently change after channel initialization, so we don't - // need to here. - for &(ref broadcast_htlc, ref broadcast_source) in per_commitment_data.iter() { - if broadcast_htlc.transaction_output_index.is_some() && Some(source) == broadcast_source.as_ref() { - continue $id; - } - } - log_trace!(logger, "Failing HTLC with payment_hash {} from {} counterparty commitment tx due to broadcast of counterparty commitment transaction", log_bytes!(htlc.payment_hash.0), $commitment_tx); - self.onchain_events_awaiting_threshold_conf.retain(|ref entry| { - if entry.height != height { return true; } - match entry.event { - OnchainEvent::HTLCUpdate { source: ref update_source, .. } => { - *update_source != **source - }, - _ => true, - } - }); - self.onchain_events_awaiting_threshold_conf.push(OnchainEventEntry { - txid: *$txid, - height, - event: OnchainEvent::HTLCUpdate { - source: (**source).clone(), - payment_hash: htlc.payment_hash.clone(), - }, - }); - } - } - } - } - } - if let Some(ref txid) = self.current_counterparty_commitment_txid { - check_htlc_fails!(txid, "current", 'current_loop); - } - if let Some(ref txid) = self.prev_counterparty_commitment_txid { - check_htlc_fails!(txid, "previous", 'prev_loop); - } + fail_unbroadcast_htlcs!(self, "counterparty", height, per_commitment_data.iter().map(|(a, b)| (a, b.as_ref().map(|b| b.as_ref()))), logger); let htlc_claim_reqs = self.get_counterparty_htlc_output_claim_reqs(commitment_number, commitment_txid, Some(tx)); for req in htlc_claim_reqs { @@ -1779,27 +1778,6 @@ impl ChannelMonitorImpl { let mut claim_requests = Vec::new(); let mut watch_outputs = Vec::new(); - macro_rules! wait_threshold_conf { - ($source: expr, $commitment_tx: expr, $payment_hash: expr) => { - self.onchain_events_awaiting_threshold_conf.retain(|ref entry| { - if entry.height != height { return true; } - match entry.event { - OnchainEvent::HTLCUpdate { source: ref update_source, .. } => { - *update_source != $source - }, - _ => true, - } - }); - let entry = OnchainEventEntry { - txid: commitment_txid, - height, - event: OnchainEvent::HTLCUpdate { source: $source, payment_hash: $payment_hash }, - }; - log_trace!(logger, "Failing HTLC with payment_hash {} from {} holder commitment tx due to broadcast of transaction, waiting confirmation (at height{})", log_bytes!($payment_hash.0), $commitment_tx, entry.confirmation_threshold()); - self.onchain_events_awaiting_threshold_conf.push(entry); - } - } - macro_rules! append_onchain_update { ($updates: expr, $to_watch: expr) => { claim_requests = $updates.0; @@ -1817,6 +1795,7 @@ impl ChannelMonitorImpl { let res = self.get_broadcasted_holder_claims(&self.current_holder_commitment_tx, height); let mut to_watch = self.get_broadcasted_holder_watch_outputs(&self.current_holder_commitment_tx, tx); append_onchain_update!(res, to_watch); + fail_unbroadcast_htlcs!(self, "latest holder", height, self.current_holder_commitment_tx.htlc_outputs.iter().map(|(a, _, c)| (a, c.as_ref())), logger); } else if let &Some(ref holder_tx) = &self.prev_holder_signed_commitment_tx { if holder_tx.txid == commitment_txid { is_holder_tx = true; @@ -1824,26 +1803,11 @@ impl ChannelMonitorImpl { let res = self.get_broadcasted_holder_claims(holder_tx, height); let mut to_watch = self.get_broadcasted_holder_watch_outputs(holder_tx, tx); append_onchain_update!(res, to_watch); - } - } - - macro_rules! fail_dust_htlcs_after_threshold_conf { - ($holder_tx: expr) => { - for &(ref htlc, _, ref source) in &$holder_tx.htlc_outputs { - if htlc.transaction_output_index.is_none() { - if let &Some(ref source) = source { - wait_threshold_conf!(source.clone(), "lastest", htlc.payment_hash.clone()); - } - } - } + fail_unbroadcast_htlcs!(self, "previous holder", height, holder_tx.htlc_outputs.iter().map(|(a, _, c)| (a, c.as_ref())), logger); } } if is_holder_tx { - fail_dust_htlcs_after_threshold_conf!(self.current_holder_commitment_tx); - if let &Some(ref holder_tx) = &self.prev_holder_signed_commitment_tx { - fail_dust_htlcs_after_threshold_conf!(holder_tx); - } } (claim_requests, (commitment_txid, watch_outputs)) @@ -2090,7 +2054,7 @@ impl ChannelMonitorImpl { // Produce actionable events from on-chain events having reached their threshold. for entry in onchain_events_reaching_threshold_conf.drain(..) { match entry.event { - OnchainEvent::HTLCUpdate { ref source, payment_hash } => { + OnchainEvent::HTLCUpdate { ref source, payment_hash, onchain_value_satoshis } => { // Check for duplicate HTLC resolutions. #[cfg(debug_assertions)] { @@ -2109,9 +2073,10 @@ impl ChannelMonitorImpl { log_debug!(logger, "HTLC {} failure update has got enough confirmations to be passed upstream", log_bytes!(payment_hash.0)); self.pending_monitor_events.push(MonitorEvent::HTLCEvent(HTLCUpdate { - payment_hash: payment_hash, + payment_hash, payment_preimage: None, source: source.clone(), + onchain_value_satoshis, })); }, OnchainEvent::MaturingOutput { descriptor } => { @@ -2328,7 +2293,7 @@ impl ChannelMonitorImpl { if pending_htlc.payment_hash == $htlc_output.payment_hash && pending_htlc.amount_msat == $htlc_output.amount_msat { if let &Some(ref source) = pending_source { log_claim!("revoked counterparty commitment tx", false, pending_htlc, true); - payment_data = Some(((**source).clone(), $htlc_output.payment_hash)); + payment_data = Some(((**source).clone(), $htlc_output.payment_hash, $htlc_output.amount_msat)); break; } } @@ -2348,7 +2313,7 @@ impl ChannelMonitorImpl { // transaction. This implies we either learned a preimage, the HTLC // has timed out, or we screwed up. In any case, we should now // resolve the source HTLC with the original sender. - payment_data = Some(((*source).clone(), htlc_output.payment_hash)); + payment_data = Some(((*source).clone(), htlc_output.payment_hash, htlc_output.amount_msat)); } else if !$holder_tx { check_htlc_valid_counterparty!(self.current_counterparty_commitment_txid, htlc_output); if payment_data.is_none() { @@ -2381,7 +2346,7 @@ impl ChannelMonitorImpl { // Check that scan_commitment, above, decided there is some source worth relaying an // HTLC resolution backwards to and figure out whether we learned a preimage from it. - if let Some((source, payment_hash)) = payment_data { + if let Some((source, payment_hash, amount_msat)) = payment_data { let mut payment_preimage = PaymentPreimage([0; 32]); if accepted_preimage_claim { if !self.pending_monitor_events.iter().any( @@ -2390,7 +2355,8 @@ impl ChannelMonitorImpl { self.pending_monitor_events.push(MonitorEvent::HTLCEvent(HTLCUpdate { source, payment_preimage: Some(payment_preimage), - payment_hash + payment_hash, + onchain_value_satoshis: Some(amount_msat / 1000), })); } } else if offered_preimage_claim { @@ -2402,7 +2368,8 @@ impl ChannelMonitorImpl { self.pending_monitor_events.push(MonitorEvent::HTLCEvent(HTLCUpdate { source, payment_preimage: Some(payment_preimage), - payment_hash + payment_hash, + onchain_value_satoshis: Some(amount_msat / 1000), })); } } else { @@ -2418,7 +2385,10 @@ impl ChannelMonitorImpl { let entry = OnchainEventEntry { txid: tx.txid(), height, - event: OnchainEvent::HTLCUpdate { source: source, payment_hash: payment_hash }, + event: OnchainEvent::HTLCUpdate { + source, payment_hash, + onchain_value_satoshis: Some(amount_msat / 1000), + }, }; log_info!(logger, "Failing HTLC with payment_hash {} timeout by a spend tx, waiting for confirmation (at height {})", log_bytes!(payment_hash.0), entry.confirmation_threshold()); self.onchain_events_awaiting_threshold_conf.push(entry); @@ -2451,7 +2421,8 @@ impl ChannelMonitorImpl { output: outp.clone(), }); break; - } else if let Some(ref broadcasted_holder_revokable_script) = self.broadcasted_holder_revokable_script { + } + if let Some(ref broadcasted_holder_revokable_script) = self.broadcasted_holder_revokable_script { if broadcasted_holder_revokable_script.0 == outp.script_pubkey { spendable_output = Some(SpendableOutputDescriptor::DelayedPaymentOutput(DelayedPaymentOutputDescriptor { outpoint: OutPoint { txid: tx.txid(), index: i as u16 }, @@ -2464,7 +2435,8 @@ impl ChannelMonitorImpl { })); break; } - } else if self.counterparty_payment_script == outp.script_pubkey { + } + if self.counterparty_payment_script == outp.script_pubkey { spendable_output = Some(SpendableOutputDescriptor::StaticPaymentOutput(StaticPaymentOutputDescriptor { outpoint: OutPoint { txid: tx.txid(), index: i as u16 }, output: outp.clone(), @@ -2472,11 +2444,13 @@ impl ChannelMonitorImpl { channel_value_satoshis: self.channel_value_satoshis, })); break; - } else if outp.script_pubkey == self.shutdown_script { + } + if outp.script_pubkey == self.shutdown_script { spendable_output = Some(SpendableOutputDescriptor::StaticOutput { outpoint: OutPoint { txid: tx.txid(), index: i as u16 }, output: outp.clone(), }); + break; } } if let Some(spendable_output) = spendable_output { @@ -2581,7 +2555,7 @@ const MAX_ALLOC_SIZE: usize = 64*1024; impl<'a, Signer: Sign, K: KeysInterface> ReadableArgs<&'a K> for (BlockHash, ChannelMonitor) { - fn read(reader: &mut R, keys_manager: &'a K) -> Result { + fn read(reader: &mut R, keys_manager: &'a K) -> Result { macro_rules! unwrap_obj { ($key: expr) => { match $key { @@ -2843,7 +2817,7 @@ mod tests { use util::test_utils::{TestLogger, TestBroadcaster, TestFeeEstimator}; use bitcoin::secp256k1::key::{SecretKey,PublicKey}; use bitcoin::secp256k1::Secp256k1; - use std::sync::{Arc, Mutex}; + use sync::{Arc, Mutex}; use chain::keysinterface::InMemorySigner; use prelude::*; @@ -2852,7 +2826,7 @@ mod tests { let secp_ctx = Secp256k1::new(); let logger = Arc::new(TestLogger::new()); let broadcaster = Arc::new(TestBroadcaster{txn_broadcasted: Mutex::new(Vec::new()), blocks: Arc::new(Mutex::new(Vec::new()))}); - let fee_estimator = Arc::new(TestFeeEstimator { sat_per_kw: 253 }); + let fee_estimator = Arc::new(TestFeeEstimator { sat_per_kw: Mutex::new(253) }); let dummy_key = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap()); let dummy_tx = Transaction { version: 0, lock_time: 0, input: Vec::new(), output: Vec::new() };