From: Matt Corallo Date: Sat, 8 May 2021 20:20:24 +0000 (+0000) Subject: Make the result of Channel::force_shutdown a struct, not a tuple X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=commitdiff_plain;h=edbbe135a89ed4ab4e2ac7387e72cdcc444eee94;p=rust-lightning Make the result of Channel::force_shutdown a struct, not a tuple In the next commit we will add a second field of identical type (but different semantics) to the result, so really need the values to be named. --- diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 413dc8313..35b658031 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -296,6 +296,12 @@ pub struct CounterpartyForwardingInfo { pub cltv_expiry_delta: u16, } +/// The result of a call to force_shutdown +pub(super) struct ForceShutdownResult { + pub(super) monitor_update: Option<(OutPoint, ChannelMonitorUpdate)>, + pub(super) outbound_htlcs_failed: Vec<(HTLCSource, PaymentHash)>, +} + // TODO: We should refactor this to be an Inbound/OutboundChannel until initial setup handshaking // has been completed, and then turn into a Channel to get compiler-time enforcement of things like // calling channel_id() before we're set up or things like get_outbound_funding_signed on an @@ -4281,7 +4287,7 @@ impl Channel { /// those explicitly stated to be allowed after shutdown completes, eg some simple getters). /// Also returns the list of payment_hashes for channels which we can safely fail backwards /// immediately (others we will have to allow to time out). - pub fn force_shutdown(&mut self, should_broadcast: bool) -> (Option<(OutPoint, ChannelMonitorUpdate)>, Vec<(HTLCSource, PaymentHash)>) { + pub fn force_shutdown(&mut self, should_broadcast: bool) -> ForceShutdownResult { // Note that we MUST only generate a monitor update that indicates force-closure - we're // called during initialization prior to the chain_monitor in the encompassing ChannelManager // being fully configured in some cases. Thus, its likely any monitor events we generate will @@ -4318,7 +4324,7 @@ impl Channel { self.channel_state = ChannelState::ShutdownComplete as u32; self.update_time_counter += 1; - (monitor_update, dropped_outbound_htlcs) + ForceShutdownResult { monitor_update, outbound_htlcs_failed: dropped_outbound_htlcs } } } diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index a9093b567..1b16fcb2b 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -45,7 +45,7 @@ use chain::transaction::{OutPoint, TransactionData}; // construct one themselves. use ln::{PaymentHash, PaymentPreimage, PaymentSecret}; pub use ln::channel::CounterpartyForwardingInfo; -use ln::channel::{Channel, ChannelError}; +use ln::channel::{Channel, ChannelError, ForceShutdownResult}; use ln::features::{InitFeatures, NodeFeatures}; use routing::router::{Route, RouteHop}; use ln::msgs; @@ -197,8 +197,6 @@ pub(super) enum HTLCFailReason { } } -type ShutdownResult = (Option<(OutPoint, ChannelMonitorUpdate)>, Vec<(HTLCSource, PaymentHash)>); - /// Error type returned across the channel_state mutex boundary. When an Err is generated for a /// Channel, we generally end up with a ChannelError::Close for which we have to close the channel /// immediately (ie with no further calls on it made). Thus, this step happens inside a @@ -207,7 +205,7 @@ type ShutdownResult = (Option<(OutPoint, ChannelMonitorUpdate)>, Vec<(HTLCSource struct MsgHandleErrInternal { err: msgs::LightningError, - shutdown_finish: Option<(ShutdownResult, Option)>, + shutdown_finish: Option<(ForceShutdownResult, Option)>, } impl MsgHandleErrInternal { #[inline] @@ -240,7 +238,7 @@ impl MsgHandleErrInternal { Self { err, shutdown_finish: None } } #[inline] - fn from_finish_shutdown(err: String, channel_id: [u8; 32], shutdown_res: ShutdownResult, channel_update: Option) -> Self { + fn from_finish_shutdown(err: String, channel_id: [u8; 32], shutdown_res: ForceShutdownResult, channel_update: Option) -> Self { Self { err: LightningError { err: err.clone(), @@ -1066,13 +1064,12 @@ impl ChannelMana } #[inline] - fn finish_force_close_channel(&self, shutdown_res: ShutdownResult) { - let (monitor_update_option, mut failed_htlcs) = shutdown_res; - log_trace!(self.logger, "Finishing force-closure of channel {} HTLCs to fail", failed_htlcs.len()); - for htlc_source in failed_htlcs.drain(..) { + fn finish_force_close_channel(&self, mut shutdown_res: ForceShutdownResult) { + log_trace!(self.logger, "Finishing force-closure of channel {} HTLCs to fail", shutdown_res.outbound_htlcs_failed.len()); + for htlc_source in shutdown_res.outbound_htlcs_failed.drain(..) { self.fail_htlc_backwards_internal(self.channel_state.lock().unwrap(), htlc_source.0, &htlc_source.1, HTLCFailReason::Reason { failure_code: 0x4000 | 8, data: Vec::new() }); } - if let Some((funding_txo, monitor_update)) = monitor_update_option { + if let Some((funding_txo, monitor_update)) = shutdown_res.monitor_update { // There isn't anything we can do if we get an update failure - we're already // force-closing. The monitor update on the required in-memory copy should broadcast // the latest local state, which is the best we can do anyway. Thus, it is safe to @@ -2669,8 +2666,8 @@ impl ChannelMana // We do not do a force-close here as that would generate a monitor update for // a monitor that we didn't manage to store (and that we don't care about - we // don't respond with the funding_signed so the channel can never go on chain). - let (_monitor_update, failed_htlcs) = chan.force_shutdown(true); - assert!(failed_htlcs.is_empty()); + let shutdown_res = chan.force_shutdown(true); + assert!(shutdown_res.outbound_htlcs_failed.is_empty()); return Err(MsgHandleErrInternal::send_err_msg_no_close("ChannelMonitor storage failure".to_owned(), funding_msg.channel_id)); }, ChannelMonitorUpdateErr::TemporaryFailure => { @@ -3382,7 +3379,7 @@ impl ChannelMana /// Handle a list of channel failures during a block_connected or block_disconnected call, /// pushing the channel monitor update (if any) to the background events queue and removing the /// Channel object. - fn handle_init_event_channel_failures(&self, mut failed_channels: Vec) { + fn handle_init_event_channel_failures(&self, mut failed_channels: Vec) { for mut failure in failed_channels.drain(..) { // Either a commitment transactions has been confirmed on-chain or // Channel::block_disconnected detected that the funding transaction has been @@ -3391,7 +3388,7 @@ impl ChannelMana // Channel::force_shutdown tries to make us do) as we may still be in initialization, // so we track the update internally and handle it when the user next calls // timer_tick_occurred, guaranteeing we're running normally. - if let Some((funding_txo, update)) = failure.0.take() { + if let Some((funding_txo, update)) = failure.monitor_update.take() { assert_eq!(update.updates.len(), 1); if let ChannelMonitorUpdateStep::ChannelForceClosed { should_broadcast } = update.updates[0] { assert!(should_broadcast); @@ -4578,8 +4575,8 @@ impl<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> channel.get_cur_counterparty_commitment_transaction_number() > monitor.get_cur_counterparty_commitment_number() || channel.get_latest_monitor_update_id() < monitor.get_latest_update_id() { // But if the channel is behind of the monitor, close the channel: - let (_, mut new_failed_htlcs) = channel.force_shutdown(true); - failed_htlcs.append(&mut new_failed_htlcs); + let mut shutdown_res = channel.force_shutdown(true); + failed_htlcs.append(&mut shutdown_res.outbound_htlcs_failed); monitor.broadcast_latest_holder_commitment_txn(&args.tx_broadcaster, &args.logger); } else { if let Some(short_channel_id) = channel.get_short_channel_id() {