From 1e922aeb0c0ad63af46f98b85c733df576009802 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Mon, 26 Jul 2021 14:04:44 -0400 Subject: [PATCH] Generate shutdown script at channel close When a shutdown script is omitted from open_channel or accept_channel, it must be provided when sending shutdown. Generate the shutdown script at channel closing time in this case rather at channel opening. This requires producing a ChannelMonitorUpdate with the shutdown script since it is no longer known at ChannelMonitor creation. --- lightning/src/chain/channelmonitor.rs | 14 ++++- lightning/src/ln/channel.rs | 81 +++++++++++++++++++++------ lightning/src/ln/channelmanager.rs | 16 +++++- lightning/src/ln/functional_tests.rs | 15 +++-- 4 files changed, 101 insertions(+), 25 deletions(-) diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index d57d50c61..4bad06cde 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -434,6 +434,9 @@ pub(crate) enum ChannelMonitorUpdateStep { /// think we've fallen behind! should_broadcast: bool, }, + ShutdownScript { + scriptpubkey: Script, + }, } impl_writeable_tlv_based_enum!(ChannelMonitorUpdateStep, @@ -457,6 +460,9 @@ impl_writeable_tlv_based_enum!(ChannelMonitorUpdateStep, (4, ChannelForceClosed) => { (0, should_broadcast, required), }, + (5, ShutdownScript) => { + (0, scriptpubkey, required), + }, ;); /// A ChannelMonitor handles chain events (blocks connected and disconnected) and generates @@ -1431,7 +1437,13 @@ impl ChannelMonitorImpl { // shouldn't print the scary warning above. log_info!(logger, "Channel off-chain state closed after we broadcasted our latest commitment transaction."); } - } + }, + ChannelMonitorUpdateStep::ShutdownScript { scriptpubkey } => { + log_trace!(logger, "Updating ChannelMonitor with shutdown script"); + if let Some(shutdown_script) = self.shutdown_script.replace(scriptpubkey.clone()) { + panic!("Attempted to replace shutdown script {} with {}", shutdown_script, scriptpubkey); + } + }, } } self.latest_update_id = updates.update_id; diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index f6313c075..b22f3756b 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -595,6 +595,12 @@ impl Channel { let mut secp_ctx = Secp256k1::new(); secp_ctx.seeded_randomize(&keys_provider.get_secure_random_bytes()); + let shutdown_scriptpubkey = if config.channel_options.commit_upfront_shutdown_pubkey { + Some(keys_provider.get_shutdown_scriptpubkey()) + } else { + None + }; + Ok(Channel { user_id, config: config.channel_options.clone(), @@ -607,7 +613,7 @@ impl Channel { latest_monitor_update_id: 0, holder_signer, - shutdown_scriptpubkey: Some(keys_provider.get_shutdown_scriptpubkey()), + shutdown_scriptpubkey, destination_script: keys_provider.get_destination_script(), cur_holder_commitment_transaction_number: INITIAL_COMMITMENT_NUMBER, @@ -840,6 +846,12 @@ impl Channel { let mut secp_ctx = Secp256k1::new(); secp_ctx.seeded_randomize(&keys_provider.get_secure_random_bytes()); + let shutdown_scriptpubkey = if config.channel_options.commit_upfront_shutdown_pubkey { + Some(keys_provider.get_shutdown_scriptpubkey()) + } else { + None + }; + let chan = Channel { user_id, config: local_config, @@ -851,7 +863,7 @@ impl Channel { latest_monitor_update_id: 0, holder_signer, - shutdown_scriptpubkey: Some(keys_provider.get_shutdown_scriptpubkey()), + shutdown_scriptpubkey, destination_script: keys_provider.get_destination_script(), cur_holder_commitment_transaction_number: INITIAL_COMMITMENT_NUMBER, @@ -1196,6 +1208,7 @@ impl Channel { }, ())); } + assert!(self.shutdown_scriptpubkey.is_some()); if value_to_self as u64 > self.holder_dust_limit_satoshis { txouts.push((TxOut { script_pubkey: self.get_closing_scriptpubkey(), @@ -3082,6 +3095,7 @@ impl Channel { self.channel_state &= !(ChannelState::PeerDisconnected as u32); let shutdown_msg = if self.channel_state & (ChannelState::LocalShutdownSent as u32) != 0 { + assert!(self.shutdown_scriptpubkey.is_some()); Some(msgs::Shutdown { channel_id: self.channel_id, scriptpubkey: self.get_closing_scriptpubkey(), @@ -3193,6 +3207,7 @@ impl Channel { if self.feerate_per_kw > proposed_feerate { proposed_feerate = self.feerate_per_kw; } + assert!(self.shutdown_scriptpubkey.is_some()); let tx_weight = self.get_closing_transaction_weight(Some(&self.get_closing_scriptpubkey()), Some(self.counterparty_shutdown_scriptpubkey.as_ref().unwrap())); let proposed_total_fee_satoshis = proposed_feerate as u64 * tx_weight / 1000; @@ -3211,8 +3226,12 @@ impl Channel { }) } - pub fn shutdown(&mut self, fee_estimator: &F, their_features: &InitFeatures, msg: &msgs::Shutdown) -> Result<(Option, Option, Vec<(HTLCSource, PaymentHash)>), ChannelError> - where F::Target: FeeEstimator + pub fn shutdown( + &mut self, fee_estimator: &F, keys_provider: &K, their_features: &InitFeatures, msg: &msgs::Shutdown + ) -> Result<(Option, Option, Option, Vec<(HTLCSource, PaymentHash)>), ChannelError> + where + F::Target: FeeEstimator, + K::Target: KeysInterface { if self.channel_state & (ChannelState::PeerDisconnected as u32) == ChannelState::PeerDisconnected as u32 { return Err(ChannelError::Close("Peer sent shutdown when we needed a channel_reestablish".to_owned())); @@ -3262,23 +3281,36 @@ impl Channel { _ => true } }); + // If we have any LocalAnnounced updates we'll probably just get back a update_fail_htlc // immediately after the commitment dance, but we can send a Shutdown cause we won't send // any further commitment updates after we set LocalShutdownSent. - - let shutdown = if (self.channel_state & ChannelState::LocalShutdownSent as u32) == ChannelState::LocalShutdownSent as u32 { - None - } else { + let send_shutdown = (self.channel_state & ChannelState::LocalShutdownSent as u32) != ChannelState::LocalShutdownSent as u32; + let monitor_update = match self.shutdown_scriptpubkey { + Some(_) => None, + None => { + assert!(send_shutdown); + self.shutdown_scriptpubkey = Some(keys_provider.get_shutdown_scriptpubkey()); + self.latest_monitor_update_id += 1; + Some(ChannelMonitorUpdate { + update_id: self.latest_monitor_update_id, + updates: vec![ChannelMonitorUpdateStep::ShutdownScript { + scriptpubkey: self.get_closing_scriptpubkey(), + }], + }) + }, + }; + let shutdown = if send_shutdown { Some(msgs::Shutdown { channel_id: self.channel_id, scriptpubkey: self.get_closing_scriptpubkey(), }) - }; + } else { None }; self.channel_state |= ChannelState::LocalShutdownSent as u32; self.update_time_counter += 1; - Ok((shutdown, self.maybe_propose_first_closing_signed(fee_estimator), dropped_outbound_htlcs)) + Ok((shutdown, self.maybe_propose_first_closing_signed(fee_estimator), monitor_update, dropped_outbound_htlcs)) } fn build_signed_closing_transaction(&self, tx: &mut Transaction, counterparty_sig: &Signature, sig: &Signature) { @@ -3353,6 +3385,7 @@ impl Channel { macro_rules! propose_new_feerate { ($new_feerate: expr) => { + assert!(self.shutdown_scriptpubkey.is_some()); let tx_weight = self.get_closing_transaction_weight(Some(&self.get_closing_scriptpubkey()), Some(self.counterparty_shutdown_scriptpubkey.as_ref().unwrap())); let (closing_tx, used_total_fee) = self.build_closing_transaction($new_feerate as u64 * tx_weight / 1000, false); let sig = self.holder_signer @@ -3851,7 +3884,10 @@ impl Channel { htlc_basepoint: keys.htlc_basepoint, first_per_commitment_point, channel_flags: if self.config.announced_channel {1} else {0}, - shutdown_scriptpubkey: OptionalField::Present(if self.config.commit_upfront_shutdown_pubkey { self.get_closing_scriptpubkey() } else { Builder::new().into_script() }) + shutdown_scriptpubkey: OptionalField::Present(match &self.shutdown_scriptpubkey { + Some(script) => script.clone().into_inner(), + None => Builder::new().into_script(), + }), } } @@ -3884,7 +3920,10 @@ impl Channel { delayed_payment_basepoint: keys.delayed_payment_basepoint, htlc_basepoint: keys.htlc_basepoint, first_per_commitment_point, - shutdown_scriptpubkey: OptionalField::Present(if self.config.commit_upfront_shutdown_pubkey { self.get_closing_scriptpubkey() } else { Builder::new().into_script() }) + shutdown_scriptpubkey: OptionalField::Present(match &self.shutdown_scriptpubkey { + Some(script) => script.clone().into_inner(), + None => Builder::new().into_script(), + }), } } @@ -4392,7 +4431,8 @@ impl Channel { /// Begins the shutdown process, getting a message for the remote peer and returning all /// holding cell HTLCs for payment failure. - pub fn get_shutdown(&mut self) -> Result<(msgs::Shutdown, Vec<(HTLCSource, PaymentHash)>), APIError> { + pub fn get_shutdown(&mut self, keys_provider: &K) -> Result<(msgs::Shutdown, Option, Vec<(HTLCSource, PaymentHash)>), APIError> + where K::Target: KeysInterface { for htlc in self.pending_outbound_htlcs.iter() { if let OutboundHTLCState::LocalAnnounced(_) = htlc.state { return Err(APIError::APIMisuseError{err: "Cannot begin shutdown with pending HTLCs. Process pending events first".to_owned()}); @@ -4411,7 +4451,16 @@ impl Channel { return Err(APIError::ChannelUnavailable{err: "Cannot begin shutdown while peer is disconnected or we're waiting on a monitor update, maybe force-close instead?".to_owned()}); } - let closing_script = self.get_closing_scriptpubkey(); + let monitor_update = if self.shutdown_scriptpubkey.is_none() { + self.shutdown_scriptpubkey = Some(keys_provider.get_shutdown_scriptpubkey()); + self.latest_monitor_update_id += 1; + Some(ChannelMonitorUpdate { + update_id: self.latest_monitor_update_id, + updates: vec![ChannelMonitorUpdateStep::ShutdownScript { + scriptpubkey: self.get_closing_scriptpubkey(), + }], + }) + } else { None }; // From here on out, we may not fail! if self.channel_state < ChannelState::FundingSent as u32 { @@ -4437,8 +4486,8 @@ impl Channel { Ok((msgs::Shutdown { channel_id: self.channel_id, - scriptpubkey: closing_script, - }, dropped_outbound_htlcs)) + scriptpubkey: self.get_closing_scriptpubkey(), + }, monitor_update, dropped_outbound_htlcs)) } /// Gets the latest commitment transaction and any dependent transactions for relay (forcing diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index f4caa3edf..0ce337f60 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -1256,11 +1256,17 @@ impl ChannelMana let channel_state = &mut *channel_state_lock; match channel_state.by_id.entry(channel_id.clone()) { hash_map::Entry::Occupied(mut chan_entry) => { - let (shutdown_msg, failed_htlcs) = chan_entry.get_mut().get_shutdown()?; + let (shutdown_msg, monitor_update, failed_htlcs) = chan_entry.get_mut().get_shutdown(&self.keys_manager)?; channel_state.pending_msg_events.push(events::MessageSendEvent::SendShutdown { node_id: chan_entry.get().get_counterparty_node_id(), msg: shutdown_msg }); + if let Some(monitor_update) = monitor_update { + if let Err(_) = self.chain_monitor.update_channel(chan_entry.get().get_funding_txo().unwrap(), monitor_update) { + // TODO: How should this be handled? + unimplemented!(); + } + } if chan_entry.get().is_shutdown() { if let Some(short_id) = chan_entry.get().get_short_channel_id() { channel_state.short_to_id.remove(&short_id); @@ -3159,7 +3165,7 @@ impl ChannelMana if chan_entry.get().get_counterparty_node_id() != *counterparty_node_id { return Err(MsgHandleErrInternal::send_err_msg_no_close("Got a message for a channel from the wrong node!".to_owned(), msg.channel_id)); } - let (shutdown, closing_signed, dropped_htlcs) = try_chan_entry!(self, chan_entry.get_mut().shutdown(&self.fee_estimator, &their_features, &msg), channel_state, chan_entry); + let (shutdown, closing_signed, monitor_update, dropped_htlcs) = try_chan_entry!(self, chan_entry.get_mut().shutdown(&self.fee_estimator, &self.keys_manager, &their_features, &msg), channel_state, chan_entry); if let Some(msg) = shutdown { channel_state.pending_msg_events.push(events::MessageSendEvent::SendShutdown { node_id: counterparty_node_id.clone(), @@ -3172,6 +3178,12 @@ impl ChannelMana msg, }); } + if let Some(monitor_update) = monitor_update { + if let Err(_) = self.chain_monitor.update_channel(chan_entry.get().get_funding_txo().unwrap(), monitor_update) { + // TODO: How should this be handled? + unimplemented!(); + } + } if chan_entry.get().is_shutdown() { if let Some(short_id) = chan_entry.get().get_short_channel_id() { channel_state.short_to_id.remove(&short_id); diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index 9c71e070b..21f871fa8 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -7474,9 +7474,9 @@ fn test_upfront_shutdown_script() { let flags_no = InitFeatures::known().clear_upfront_shutdown_script(); let chan = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1000000, 1000000, flags_no, flags.clone()); nodes[0].node.close_channel(&OutPoint { txid: chan.3.txid(), index: 0 }.to_channel_id()).unwrap(); - let mut node_1_shutdown = get_event_msg!(nodes[0], MessageSendEvent::SendShutdown, nodes[1].node.get_our_node_id()); - node_1_shutdown.scriptpubkey = Builder::new().push_opcode(opcodes::all::OP_RETURN).into_script().to_p2sh(); + let node_1_shutdown = get_event_msg!(nodes[0], MessageSendEvent::SendShutdown, nodes[1].node.get_our_node_id()); nodes[1].node.handle_shutdown(&nodes[0].node.get_our_node_id(), &InitFeatures::known(), &node_1_shutdown); + check_added_monitors!(nodes[1], 1); let events = nodes[1].node.get_and_clear_pending_msg_events(); assert_eq!(events.len(), 1); match events[0] { @@ -7488,8 +7488,8 @@ fn test_upfront_shutdown_script() { // channel smoothly, opt-out is from channel initiator here let chan = create_announced_chan_between_nodes_with_value(&nodes, 1, 0, 1000000, 1000000, flags.clone(), flags.clone()); nodes[1].node.close_channel(&OutPoint { txid: chan.3.txid(), index: 0 }.to_channel_id()).unwrap(); - let mut node_0_shutdown = get_event_msg!(nodes[1], MessageSendEvent::SendShutdown, nodes[0].node.get_our_node_id()); - node_0_shutdown.scriptpubkey = Builder::new().push_opcode(opcodes::all::OP_RETURN).into_script().to_p2sh(); + check_added_monitors!(nodes[1], 1); + let node_0_shutdown = get_event_msg!(nodes[1], MessageSendEvent::SendShutdown, nodes[0].node.get_our_node_id()); nodes[0].node.handle_shutdown(&nodes[1].node.get_our_node_id(), &InitFeatures::known(), &node_0_shutdown); let events = nodes[0].node.get_and_clear_pending_msg_events(); assert_eq!(events.len(), 1); @@ -7502,8 +7502,8 @@ fn test_upfront_shutdown_script() { //// channel smoothly let chan = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1000000, 1000000, flags.clone(), flags.clone()); nodes[1].node.close_channel(&OutPoint { txid: chan.3.txid(), index: 0 }.to_channel_id()).unwrap(); - let mut node_0_shutdown = get_event_msg!(nodes[1], MessageSendEvent::SendShutdown, nodes[0].node.get_our_node_id()); - node_0_shutdown.scriptpubkey = Builder::new().push_opcode(opcodes::all::OP_RETURN).into_script().to_p2sh(); + check_added_monitors!(nodes[1], 1); + let node_0_shutdown = get_event_msg!(nodes[1], MessageSendEvent::SendShutdown, nodes[0].node.get_our_node_id()); nodes[0].node.handle_shutdown(&nodes[1].node.get_our_node_id(), &InitFeatures::known(), &node_0_shutdown); let events = nodes[0].node.get_and_clear_pending_msg_events(); assert_eq!(events.len(), 2); @@ -7607,6 +7607,7 @@ fn test_segwit_v0_shutdown_script() { let chan = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1000000, 1000000, InitFeatures::known(), InitFeatures::known()); nodes[1].node.close_channel(&OutPoint { txid: chan.3.txid(), index: 0 }.to_channel_id()).unwrap(); + check_added_monitors!(nodes[1], 1); // Use a segwit v0 script supported even without opt_shutdown_anysegwit let mut node_0_shutdown = get_event_msg!(nodes[1], MessageSendEvent::SendShutdown, nodes[0].node.get_our_node_id()); @@ -7677,6 +7678,7 @@ fn test_unsupported_anysegwit_shutdown_script() { let node_features = InitFeatures::known().clear_shutdown_anysegwit(); let chan = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1000000, 1000000, InitFeatures::known(), node_features.clone()); nodes[1].node.close_channel(&OutPoint { txid: chan.3.txid(), index: 0 }.to_channel_id()).unwrap(); + check_added_monitors!(nodes[1], 1); // Use a non-v0 segwit script supported by opt_shutdown_anysegwit let mut node_0_shutdown = get_event_msg!(nodes[1], MessageSendEvent::SendShutdown, nodes[0].node.get_our_node_id()); @@ -7711,6 +7713,7 @@ fn test_invalid_shutdown_script() { let chan = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1000000, 1000000, InitFeatures::known(), InitFeatures::known()); nodes[1].node.close_channel(&OutPoint { txid: chan.3.txid(), index: 0 }.to_channel_id()).unwrap(); + check_added_monitors!(nodes[1], 1); // Use a segwit v0 script with an unsupported witness program let mut node_0_shutdown = get_event_msg!(nodes[1], MessageSendEvent::SendShutdown, nodes[0].node.get_our_node_id()); -- 2.39.5