From 60b962a18ebcf494340ddc001870f8160c625968 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Thu, 18 Mar 2021 20:32:30 -0400 Subject: [PATCH] Move ChannelManager to Channel's new block data API This also moves the scanning of the block for commitment transactions into channel, unifying the error path. --- lightning/src/ln/channel.rs | 118 +++++++++++++---------------- lightning/src/ln/channelmanager.rs | 48 ++++-------- 2 files changed, 68 insertions(+), 98 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 1279f310d..0f09fd330 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -7,7 +7,6 @@ // You may not use this file except in accordance with one or both of these // licenses. -use bitcoin::blockdata::block::BlockHeader; use bitcoin::blockdata::script::{Script,Builder}; use bitcoin::blockdata::transaction::{TxIn, TxOut, Transaction, SigHashType}; use bitcoin::blockdata::opcodes; @@ -3502,47 +3501,63 @@ impl Channel { self.network_sync == UpdateStatus::DisabledMarked } - pub fn transactions_confirmed(&mut self, block_hash: &BlockHash, height: u32, txdata: &TransactionData) -> Result<(), msgs::ErrorMessage> { + /// When a transaction is confirmed, we check whether it is or spends the funding transaction + /// In the first case, we store the confirmation height and calculating the short channel id. + /// In the second, we simply return an Err indicating we need to be force-closed now. + pub fn transactions_confirmed(&mut self, block_hash: &BlockHash, height: u32, txdata: &TransactionData, logger: &L) + -> Result<(), msgs::ErrorMessage> where L::Target: Logger { let non_shutdown_state = self.channel_state & (!MULTI_STATE_FLAGS); - if non_shutdown_state & !(ChannelState::TheirFundingLocked as u32) == ChannelState::FundingSent as u32 { - for &(index_in_block, tx) in txdata.iter() { - let funding_txo = self.get_funding_txo().unwrap(); - if tx.txid() == funding_txo.txid { - let txo_idx = funding_txo.index as usize; - if txo_idx >= tx.output.len() || tx.output[txo_idx].script_pubkey != self.get_funding_redeemscript().to_v0_p2wsh() || - tx.output[txo_idx].value != self.channel_value_satoshis { - if self.is_outbound() { - // If we generated the funding transaction and it doesn't match what it - // should, the client is really broken and we should just panic and - // tell them off. That said, because hash collisions happen with high - // probability in fuzztarget mode, if we're fuzzing we just close the - // channel and move on. - #[cfg(not(feature = "fuzztarget"))] - panic!("Client called ChannelManager::funding_transaction_generated with bogus transaction!"); + for &(index_in_block, tx) in txdata.iter() { + if let Some(funding_txo) = self.get_funding_txo() { + // If we haven't yet sent a funding_locked, but are in FundingSent (ignoring + // whether they've sent a funding_locked or not), check if we should send one. + if non_shutdown_state & !(ChannelState::TheirFundingLocked as u32) == ChannelState::FundingSent as u32 { + if tx.txid() == funding_txo.txid { + let txo_idx = funding_txo.index as usize; + if txo_idx >= tx.output.len() || tx.output[txo_idx].script_pubkey != self.get_funding_redeemscript().to_v0_p2wsh() || + tx.output[txo_idx].value != self.channel_value_satoshis { + if self.is_outbound() { + // If we generated the funding transaction and it doesn't match what it + // should, the client is really broken and we should just panic and + // tell them off. That said, because hash collisions happen with high + // probability in fuzztarget mode, if we're fuzzing we just close the + // channel and move on. + #[cfg(not(feature = "fuzztarget"))] + panic!("Client called ChannelManager::funding_transaction_generated with bogus transaction!"); + } + self.channel_state = ChannelState::ShutdownComplete as u32; + self.update_time_counter += 1; + return Err(msgs::ErrorMessage { + channel_id: self.channel_id(), + data: "funding tx had wrong script/value or output index".to_owned() + }); + } else { + if self.is_outbound() { + for input in tx.input.iter() { + if input.witness.is_empty() { + // We generated a malleable funding transaction, implying we've + // just exposed ourselves to funds loss to our counterparty. + #[cfg(not(feature = "fuzztarget"))] + panic!("Client called ChannelManager::funding_transaction_generated with bogus transaction!"); + } + } + } + self.funding_tx_confirmation_height = height as u64; + self.funding_tx_confirmed_in = Some(*block_hash); + self.short_channel_id = match scid_from_parts(height as u64, index_in_block as u64, txo_idx as u64) { + Ok(scid) => Some(scid), + Err(_) => panic!("Block was bogus - either height was > 16 million, had > 16 million transactions, or had > 65k outputs"), + } } - self.channel_state = ChannelState::ShutdownComplete as u32; - self.update_time_counter += 1; + } + } + for inp in tx.input.iter() { + if inp.previous_output == funding_txo.into_bitcoin_outpoint() { + log_trace!(logger, "Detected channel-closing tx {} spending {}:{}, closing channel {}", tx.txid(), inp.previous_output.txid, inp.previous_output.vout, log_bytes!(self.channel_id())); return Err(msgs::ErrorMessage { channel_id: self.channel_id(), - data: "funding tx had wrong script/value".to_owned() + data: "Commitment or closing transaction was confirmed on chain.".to_owned() }); - } else { - if self.is_outbound() { - for input in tx.input.iter() { - if input.witness.is_empty() { - // We generated a malleable funding transaction, implying we've - // just exposed ourselves to funds loss to our counterparty. - #[cfg(not(feature = "fuzztarget"))] - panic!("Client called ChannelManager::funding_transaction_generated with bogus transaction!"); - } - } - } - self.funding_tx_confirmation_height = height as u64; - self.funding_tx_confirmed_in = Some(*block_hash); - self.short_channel_id = match scid_from_parts(height as u64, index_in_block as u64, txo_idx as u64) { - Ok(scid) => Some(scid), - Err(_) => panic!("Block was bogus - either height was > 16 million, had > 16 million transactions, or had > 65k outputs"), - } } } } @@ -3635,35 +3650,6 @@ impl Channel { Ok((None, timed_out_htlcs)) } - /// When we receive a new block, we (a) check whether the block contains the funding - /// transaction (which would start us counting blocks until we send the funding_signed), and - /// (b) check the height of the block against outbound holding cell HTLCs in case we need to - /// give up on them prematurely and time them out. Everything else (e.g. commitment - /// transaction broadcasts, channel closure detection, HTLC transaction broadcasting, etc) is - /// handled by the ChannelMonitor. - /// - /// If we return Err, the channel may have been closed, at which point the standard - /// requirements apply - no calls may be made except those explicitly stated to be allowed - /// post-shutdown. - /// Only returns an ErrorAction of DisconnectPeer, if Err. - /// - /// May return some HTLCs (and their payment_hash) which have timed out and should be failed - /// back. - pub fn block_connected(&mut self, header: &BlockHeader, txdata: &TransactionData, height: u32) -> Result<(Option, Vec<(HTLCSource, PaymentHash)>), msgs::ErrorMessage> { - self.transactions_confirmed(&header.block_hash(), height, txdata)?; - self.update_best_block(height, header.time) - } - - /// Called by channelmanager based on chain blocks being disconnected. - /// Returns true if we need to close the channel now due to funding transaction - /// unconfirmation/reorg. - pub fn block_disconnected(&mut self, header: &BlockHeader, new_height: u32) -> bool { - if self.update_best_block(new_height, header.time).is_err() { - return true; - } - false - } - // Methods to get unprompted messages to send to the remote end (or where we already returned // something in the handler for the message that prompted this message): diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 14fed3c40..acdc06488 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -3334,7 +3334,8 @@ impl ChannelMana let short_to_id = &mut channel_state.short_to_id; let pending_msg_events = &mut channel_state.pending_msg_events; channel_state.by_id.retain(|_, channel| { - let res = channel.block_connected(header, txdata, height); + let res = channel.transactions_confirmed(&block_hash, height, txdata, &self.logger) + .and_then(|_| channel.update_best_block(height, header.time)); if let Ok((chan_res, mut timed_out_pending_htlcs)) = res { for (source, payment_hash) in timed_out_pending_htlcs.drain(..) { let chan_update = self.get_channel_update(&channel).map(|u| u.encode_with_len()).unwrap(); // Cannot add/recv HTLCs before we have a short_id so unwrap is safe @@ -3360,38 +3361,23 @@ impl ChannelMana short_to_id.insert(channel.get_short_channel_id().unwrap(), channel.channel_id()); } } else if let Err(e) = res { + if let Some(short_id) = channel.get_short_channel_id() { + short_to_id.remove(&short_id); + } + // It looks like our counterparty went on-chain or funding transaction was + // reorged out of the main chain. Close the channel. + failed_channels.push(channel.force_shutdown(true)); + if let Ok(update) = self.get_channel_update(&channel) { + pending_msg_events.push(events::MessageSendEvent::BroadcastChannelUpdate { + msg: update + }); + } pending_msg_events.push(events::MessageSendEvent::HandleError { node_id: channel.get_counterparty_node_id(), action: msgs::ErrorAction::SendErrorMessage { msg: e }, }); return false; } - if let Some(funding_txo) = channel.get_funding_txo() { - for &(_, tx) in txdata.iter() { - for inp in tx.input.iter() { - if inp.previous_output == funding_txo.into_bitcoin_outpoint() { - log_trace!(self.logger, "Detected channel-closing tx {} spending {}:{}, closing channel {}", tx.txid(), inp.previous_output.txid, inp.previous_output.vout, log_bytes!(channel.channel_id())); - if let Some(short_id) = channel.get_short_channel_id() { - short_to_id.remove(&short_id); - } - // It looks like our counterparty went on-chain. Close the channel. - failed_channels.push(channel.force_shutdown(true)); - if let Ok(update) = self.get_channel_update(&channel) { - pending_msg_events.push(events::MessageSendEvent::BroadcastChannelUpdate { - msg: update - }); - } - pending_msg_events.push(events::MessageSendEvent::HandleError { - node_id: channel.get_counterparty_node_id(), - action: msgs::ErrorAction::SendErrorMessage { - msg: msgs::ErrorMessage { channel_id: channel.channel_id(), data: "Commitment or closing transaction was confirmed on chain.".to_owned() } - }, - }); - return false; - } - } - } - } true }); @@ -3456,8 +3442,8 @@ impl ChannelMana let channel_state = &mut *channel_lock; let short_to_id = &mut channel_state.short_to_id; let pending_msg_events = &mut channel_state.pending_msg_events; - channel_state.by_id.retain(|channel_id, v| { - if v.block_disconnected(header, new_height) { + channel_state.by_id.retain(|_, v| { + if let Err(err_msg) = v.update_best_block(new_height, header.time) { if let Some(short_id) = v.get_short_channel_id() { short_to_id.remove(&short_id); } @@ -3469,9 +3455,7 @@ impl ChannelMana } pending_msg_events.push(events::MessageSendEvent::HandleError { node_id: v.get_counterparty_node_id(), - action: msgs::ErrorAction::SendErrorMessage { - msg: msgs::ErrorMessage { channel_id: *channel_id, data: "Funding transaction was un-confirmed.".to_owned() } - }, + action: msgs::ErrorAction::SendErrorMessage { msg: err_msg }, }); false } else { -- 2.39.5