From c0fbe4237bb14342be73dcda086b96cd4a5874e4 Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Thu, 5 Jun 2025 09:22:15 +0200 Subject: [PATCH 1/9] Drop `rustfmt::skip` from `shutdown`/`closing_signed` related methods --- lightning/src/ln/channel.rs | 520 ++++++++++++++++++++--------- lightning/src/ln/channelmanager.rs | 98 ++++-- 2 files changed, 431 insertions(+), 187 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 3861a7052f1..2d09a600095 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -3482,37 +3482,50 @@ where } /// shutdown state returns the state of the channel in its various stages of shutdown - #[rustfmt::skip] pub fn shutdown_state(&self) -> ChannelShutdownState { match self.channel_state { - ChannelState::AwaitingChannelReady(_)|ChannelState::ChannelReady(_) => - if self.channel_state.is_local_shutdown_sent() && !self.channel_state.is_remote_shutdown_sent() { + ChannelState::AwaitingChannelReady(_) | ChannelState::ChannelReady(_) => { + if self.channel_state.is_local_shutdown_sent() + && !self.channel_state.is_remote_shutdown_sent() + { ChannelShutdownState::ShutdownInitiated - } else if (self.channel_state.is_local_shutdown_sent() || self.channel_state.is_remote_shutdown_sent()) && !self.closing_negotiation_ready() { + } else if (self.channel_state.is_local_shutdown_sent() + || self.channel_state.is_remote_shutdown_sent()) + && !self.closing_negotiation_ready() + { ChannelShutdownState::ResolvingHTLCs - } else if (self.channel_state.is_local_shutdown_sent() || self.channel_state.is_remote_shutdown_sent()) && self.closing_negotiation_ready() { + } else if (self.channel_state.is_local_shutdown_sent() + || self.channel_state.is_remote_shutdown_sent()) + && self.closing_negotiation_ready() + { ChannelShutdownState::NegotiatingClosingFee } else { ChannelShutdownState::NotShuttingDown - }, + } + }, ChannelState::ShutdownComplete => ChannelShutdownState::ShutdownComplete, _ => ChannelShutdownState::NotShuttingDown, } } - #[rustfmt::skip] fn closing_negotiation_ready(&self) -> bool { let is_ready_to_close = match self.channel_state { - ChannelState::AwaitingChannelReady(flags) => - flags & FundedStateFlags::ALL == FundedStateFlags::LOCAL_SHUTDOWN_SENT | FundedStateFlags::REMOTE_SHUTDOWN_SENT, - ChannelState::ChannelReady(flags) => - flags == FundedStateFlags::LOCAL_SHUTDOWN_SENT | FundedStateFlags::REMOTE_SHUTDOWN_SENT, + ChannelState::AwaitingChannelReady(flags) => { + flags & FundedStateFlags::ALL + == FundedStateFlags::LOCAL_SHUTDOWN_SENT + | FundedStateFlags::REMOTE_SHUTDOWN_SENT + }, + ChannelState::ChannelReady(flags) => { + flags + == FundedStateFlags::LOCAL_SHUTDOWN_SENT + | FundedStateFlags::REMOTE_SHUTDOWN_SENT + }, _ => false, }; - self.pending_inbound_htlcs.is_empty() && - self.pending_outbound_htlcs.is_empty() && - self.pending_update_fee.is_none() && - is_ready_to_close + self.pending_inbound_htlcs.is_empty() + && self.pending_outbound_htlcs.is_empty() + && self.pending_update_fee.is_none() + && is_ready_to_close } /// Returns true if this channel is currently available for use. This is a superset of @@ -5203,8 +5216,9 @@ where /// 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). - #[rustfmt::skip] - pub fn force_shutdown(&mut self, funding: &FundingScope, should_broadcast: bool, closure_reason: ClosureReason) -> ShutdownResult { + pub fn force_shutdown( + &mut self, funding: &FundingScope, should_broadcast: bool, closure_reason: ClosureReason, + ) -> ShutdownResult { // 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 @@ -5218,9 +5232,14 @@ where for htlc_update in self.holding_cell_htlc_updates.drain(..) { match htlc_update { HTLCUpdateAwaitingACK::AddHTLC { source, payment_hash, .. } => { - dropped_outbound_htlcs.push((source, payment_hash, counterparty_node_id, self.channel_id)); + dropped_outbound_htlcs.push(( + source, + payment_hash, + counterparty_node_id, + self.channel_id, + )); }, - _ => {} + _ => {}, } } let monitor_update = if let Some(funding_txo) = funding.get_funding_txo() { @@ -5233,13 +5252,24 @@ where // See test_duplicate_chan_id and test_pre_lockin_no_chan_closed_update for more. if !self.channel_state.is_pre_funded_state() { self.latest_monitor_update_id += 1; - Some((self.get_counterparty_node_id(), funding_txo, self.channel_id(), ChannelMonitorUpdate { - update_id: self.latest_monitor_update_id, - updates: vec![ChannelMonitorUpdateStep::ChannelForceClosed { should_broadcast }], - channel_id: Some(self.channel_id()), - })) - } else { None } - } else { None }; + Some(( + self.get_counterparty_node_id(), + funding_txo, + self.channel_id(), + ChannelMonitorUpdate { + update_id: self.latest_monitor_update_id, + updates: vec![ChannelMonitorUpdateStep::ChannelForceClosed { + should_broadcast, + }], + channel_id: Some(self.channel_id()), + }, + )) + } else { + None + } + } else { + None + }; let unbroadcasted_batch_funding_txid = self.unbroadcasted_batch_funding_txid(funding); let unbroadcasted_funding_tx = self.unbroadcasted_funding(funding); @@ -5794,10 +5824,10 @@ where } #[inline] - #[rustfmt::skip] - fn get_closing_transaction_weight(&self, a_scriptpubkey: Option<&Script>, b_scriptpubkey: Option<&Script>) -> u64 { - let mut ret = - (4 + // version + fn get_closing_transaction_weight( + &self, a_scriptpubkey: Option<&Script>, b_scriptpubkey: Option<&Script>, + ) -> u64 { + let mut ret = (4 + // version 1 + // input count 36 + // prevout 1 + // script length (0) @@ -5809,28 +5839,34 @@ where 1 + // witness element count 4 + // 4 element lengths (2 sigs, multisig dummy, and witness script) self.funding.get_funding_redeemscript().len() as u64 + // funding witness script - 2*(1 + 71); // two signatures + sighash type flags + 2*(1 + 71); // two signatures + sighash type flags if let Some(spk) = a_scriptpubkey { ret += ((8+1) + // output values and script length - spk.len() as u64) * 4; // scriptpubkey and witness multiplier + spk.len() as u64) + * 4; // scriptpubkey and witness multiplier } if let Some(spk) = b_scriptpubkey { ret += ((8+1) + // output values and script length - spk.len() as u64) * 4; // scriptpubkey and witness multiplier + spk.len() as u64) + * 4; // scriptpubkey and witness multiplier } ret } #[inline] - #[rustfmt::skip] - fn build_closing_transaction(&self, proposed_total_fee_satoshis: u64, skip_remote_output: bool) -> Result<(ClosingTransaction, u64), ChannelError> { + fn build_closing_transaction( + &self, proposed_total_fee_satoshis: u64, skip_remote_output: bool, + ) -> Result<(ClosingTransaction, u64), ChannelError> { assert!(self.context.pending_inbound_htlcs.is_empty()); assert!(self.context.pending_outbound_htlcs.is_empty()); assert!(self.context.pending_update_fee.is_none()); let mut total_fee_satoshis = proposed_total_fee_satoshis; - let mut value_to_holder: i64 = (self.funding.value_to_self_msat as i64) / 1000 - if self.funding.is_outbound() { total_fee_satoshis as i64 } else { 0 }; - let mut value_to_counterparty: i64 = ((self.funding.get_value_satoshis() * 1000 - self.funding.value_to_self_msat) as i64 / 1000) - if self.funding.is_outbound() { 0 } else { total_fee_satoshis as i64 }; + let mut value_to_holder: i64 = (self.funding.value_to_self_msat as i64) / 1000 + - if self.funding.is_outbound() { total_fee_satoshis as i64 } else { 0 }; + let mut value_to_counterparty: i64 = + ((self.funding.get_value_satoshis() * 1000 - self.funding.value_to_self_msat) as i64 + / 1000) - if self.funding.is_outbound() { 0 } else { total_fee_satoshis as i64 }; if value_to_holder < 0 { assert!(self.funding.is_outbound()); @@ -5842,15 +5878,23 @@ where debug_assert!(value_to_counterparty >= 0); if value_to_counterparty < 0 { - return Err(ChannelError::close(format!("Value to counterparty below 0: {}", value_to_counterparty))) + return Err(ChannelError::close(format!( + "Value to counterparty below 0: {}", + value_to_counterparty + ))); } - if skip_remote_output || value_to_counterparty as u64 <= self.context.holder_dust_limit_satoshis { + if skip_remote_output + || value_to_counterparty as u64 <= self.context.holder_dust_limit_satoshis + { value_to_counterparty = 0; } debug_assert!(value_to_holder >= 0); if value_to_holder < 0 { - return Err(ChannelError::close(format!("Value to holder below 0: {}", value_to_holder))) + return Err(ChannelError::close(format!( + "Value to holder below 0: {}", + value_to_holder + ))); } if value_to_holder as u64 <= self.context.holder_dust_limit_satoshis { value_to_holder = 0; @@ -5858,10 +5902,17 @@ where assert!(self.context.shutdown_scriptpubkey.is_some()); let holder_shutdown_script = self.get_closing_scriptpubkey(); - let counterparty_shutdown_script = self.context.counterparty_shutdown_scriptpubkey.clone().unwrap(); + let counterparty_shutdown_script = + self.context.counterparty_shutdown_scriptpubkey.clone().unwrap(); let funding_outpoint = self.funding_outpoint().into_bitcoin_outpoint(); - let closing_transaction = ClosingTransaction::new(value_to_holder as u64, value_to_counterparty as u64, holder_shutdown_script, counterparty_shutdown_script, funding_outpoint); + let closing_transaction = ClosingTransaction::new( + value_to_holder as u64, + value_to_counterparty as u64, + holder_shutdown_script, + counterparty_shutdown_script, + funding_outpoint, + ); Ok((closing_transaction, total_fee_satoshis)) } @@ -8014,21 +8065,27 @@ where /// Calculates and returns our minimum and maximum closing transaction fee amounts, in whole /// satoshis. The amounts remain consistent unless a peer disconnects/reconnects or we restart, /// at which point they will be recalculated. - #[rustfmt::skip] - fn calculate_closing_fee_limits(&mut self, fee_estimator: &LowerBoundedFeeEstimator) - -> (u64, u64) - where F::Target: FeeEstimator + fn calculate_closing_fee_limits( + &mut self, fee_estimator: &LowerBoundedFeeEstimator, + ) -> (u64, u64) + where + F::Target: FeeEstimator, { - if let Some((min, max)) = self.context.closing_fee_limits { return (min, max); } + if let Some((min, max)) = self.context.closing_fee_limits { + return (min, max); + } // Propose a range from our current Background feerate to our Normal feerate plus our // force_close_avoidance_max_fee_satoshis. // If we fail to come to consensus, we'll have to force-close. - let mut proposed_feerate = fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::ChannelCloseMinimum); + let mut proposed_feerate = + fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::ChannelCloseMinimum); // Use NonAnchorChannelFee because this should be an estimate for a channel close // that we don't expect to need fee bumping - let normal_feerate = fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::NonAnchorChannelFee); - let mut proposed_max_feerate = if self.funding.is_outbound() { normal_feerate } else { u32::max_value() }; + let normal_feerate = + fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::NonAnchorChannelFee); + let mut proposed_max_feerate = + if self.funding.is_outbound() { normal_feerate } else { u32::max_value() }; // The spec requires that (when the channel does not have anchors) we only send absolute // channel fees no greater than the absolute channel fee on the current commitment @@ -8037,7 +8094,11 @@ where // some force-closure by old nodes, but we wanted to close the channel anyway. if let Some(target_feerate) = self.context.target_closing_feerate_sats_per_kw { - let min_feerate = if self.funding.is_outbound() { target_feerate } else { cmp::min(self.context.feerate_per_kw, target_feerate) }; + let min_feerate = if self.funding.is_outbound() { + target_feerate + } else { + cmp::min(self.context.feerate_per_kw, target_feerate) + }; proposed_feerate = cmp::max(proposed_feerate, min_feerate); proposed_max_feerate = cmp::max(proposed_max_feerate, min_feerate); } @@ -8049,19 +8110,26 @@ where // come to consensus with our counterparty on appropriate fees, however it should be a // relatively rare case. We can revisit this later, though note that in order to determine // if the funders' output is dust we have to know the absolute fee we're going to use. - let tx_weight = self.get_closing_transaction_weight(Some(&self.get_closing_scriptpubkey()), Some(self.context.counterparty_shutdown_scriptpubkey.as_ref().unwrap())); + let tx_weight = self.get_closing_transaction_weight( + Some(&self.get_closing_scriptpubkey()), + Some(self.context.counterparty_shutdown_scriptpubkey.as_ref().unwrap()), + ); let proposed_total_fee_satoshis = proposed_feerate as u64 * tx_weight / 1000; let proposed_max_total_fee_satoshis = if self.funding.is_outbound() { - // We always add force_close_avoidance_max_fee_satoshis to our normal - // feerate-calculated fee, but allow the max to be overridden if we're using a - // target feerate-calculated fee. - cmp::max(normal_feerate as u64 * tx_weight / 1000 + self.context.config.options.force_close_avoidance_max_fee_satoshis, - proposed_max_feerate as u64 * tx_weight / 1000) - } else { - self.funding.get_value_satoshis() - (self.funding.value_to_self_msat + 999) / 1000 - }; + // We always add force_close_avoidance_max_fee_satoshis to our normal + // feerate-calculated fee, but allow the max to be overridden if we're using a + // target feerate-calculated fee. + cmp::max( + normal_feerate as u64 * tx_weight / 1000 + + self.context.config.options.force_close_avoidance_max_fee_satoshis, + proposed_max_feerate as u64 * tx_weight / 1000, + ) + } else { + self.funding.get_value_satoshis() - (self.funding.value_to_self_msat + 999) / 1000 + }; - self.context.closing_fee_limits = Some((proposed_total_fee_satoshis, proposed_max_total_fee_satoshis)); + self.context.closing_fee_limits = + Some((proposed_total_fee_satoshis, proposed_max_total_fee_satoshis)); self.context.closing_fee_limits.clone().unwrap() } @@ -8089,11 +8157,15 @@ where Ok(()) } - #[rustfmt::skip] pub fn maybe_propose_closing_signed( - &mut self, fee_estimator: &LowerBoundedFeeEstimator, logger: &L) - -> Result<(Option, Option, Option), ChannelError> - where F::Target: FeeEstimator, L::Target: Logger + &mut self, fee_estimator: &LowerBoundedFeeEstimator, logger: &L, + ) -> Result< + (Option, Option, Option), + ChannelError, + > + where + F::Target: FeeEstimator, + L::Target: Logger, { // If we're waiting on a monitor persistence, that implies we're also waiting to send some // message to our counterparty (probably a `revoke_and_ack`). In such a case, we shouldn't @@ -8119,11 +8191,19 @@ where let (our_min_fee, our_max_fee) = self.calculate_closing_fee_limits(fee_estimator); assert!(self.context.shutdown_scriptpubkey.is_some()); - let (closing_tx, total_fee_satoshis) = self.build_closing_transaction(our_min_fee, false)?; + let (closing_tx, total_fee_satoshis) = + self.build_closing_transaction(our_min_fee, false)?; log_trace!(logger, "Proposing initial closing_signed for our counterparty with a fee range of {}-{} sat (with initial proposal {} sats)", our_min_fee, our_max_fee, total_fee_satoshis); - let closing_signed = self.get_closing_signed_msg(&closing_tx, false, total_fee_satoshis, our_min_fee, our_max_fee, logger); + let closing_signed = self.get_closing_signed_msg( + &closing_tx, + false, + total_fee_satoshis, + our_min_fee, + our_max_fee, + logger, + ); Ok((closing_signed, None, None)) } @@ -8165,23 +8245,30 @@ where } } - #[rustfmt::skip] pub fn shutdown( - &mut self, signer_provider: &SP, their_features: &InitFeatures, msg: &msgs::Shutdown - ) -> Result<(Option, Option, Vec<(HTLCSource, PaymentHash)>), ChannelError> - { + &mut self, signer_provider: &SP, their_features: &InitFeatures, msg: &msgs::Shutdown, + ) -> Result< + (Option, Option, Vec<(HTLCSource, PaymentHash)>), + ChannelError, + > { if self.context.channel_state.is_peer_disconnected() { - return Err(ChannelError::close("Peer sent shutdown when we needed a channel_reestablish".to_owned())); + return Err(ChannelError::close( + "Peer sent shutdown when we needed a channel_reestablish".to_owned(), + )); } if self.context.channel_state.is_pre_funded_state() { // Spec says we should fail the connection, not the channel, but that's nonsense, there // are plenty of reasons you may want to fail a channel pre-funding, and spec says you // can do that via error message without getting a connection fail anyway... - return Err(ChannelError::close("Peer sent shutdown pre-funding generation".to_owned())); + return Err(ChannelError::close( + "Peer sent shutdown pre-funding generation".to_owned(), + )); } for htlc in self.context.pending_inbound_htlcs.iter() { if let InboundHTLCState::RemoteAnnounced(_) = htlc.state { - return Err(ChannelError::close("Got shutdown with remote pending HTLCs".to_owned())); + return Err(ChannelError::close( + "Got shutdown with remote pending HTLCs".to_owned(), + )); } } assert!(!matches!(self.context.channel_state, ChannelState::ShutdownComplete)); @@ -8191,11 +8278,16 @@ where || self.context.channel_state.is_remote_stfu_sent() || self.context.channel_state.is_quiescent() { - return Err(ChannelError::WarnAndDisconnect("Got shutdown request while quiescent".to_owned())); + return Err(ChannelError::WarnAndDisconnect( + "Got shutdown request while quiescent".to_owned(), + )); } if !script::is_bolt2_compliant(&msg.scriptpubkey, their_features) { - return Err(ChannelError::Warn(format!("Got a nonstandard scriptpubkey ({}) from remote peer", msg.scriptpubkey.to_hex_string()))); + return Err(ChannelError::Warn(format!( + "Got a nonstandard scriptpubkey ({}) from remote peer", + msg.scriptpubkey.to_hex_string() + ))); } if self.context.counterparty_shutdown_scriptpubkey.is_some() { @@ -8217,10 +8309,17 @@ where assert!(send_shutdown); let shutdown_scriptpubkey = match signer_provider.get_shutdown_scriptpubkey() { Ok(scriptpubkey) => scriptpubkey, - Err(_) => return Err(ChannelError::close("Failed to get shutdown scriptpubkey".to_owned())), + Err(_) => { + return Err(ChannelError::close( + "Failed to get shutdown scriptpubkey".to_owned(), + )) + }, }; if !shutdown_scriptpubkey.is_compatible(their_features) { - return Err(ChannelError::close(format!("Provided a scriptpubkey format not accepted by peer: {}", shutdown_scriptpubkey))); + return Err(ChannelError::close(format!( + "Provided a scriptpubkey format not accepted by peer: {}", + shutdown_scriptpubkey + ))); } self.context.shutdown_scriptpubkey = Some(shutdown_scriptpubkey); true @@ -8248,27 +8347,30 @@ where }; self.monitor_updating_paused(false, false, false, Vec::new(), Vec::new(), Vec::new()); self.push_ret_blockable_mon_update(monitor_update) - } else { None }; + } else { + None + }; let shutdown = if send_shutdown { Some(msgs::Shutdown { channel_id: self.context.channel_id, scriptpubkey: self.get_closing_scriptpubkey(), }) - } else { None }; + } else { + None + }; // We can't send our shutdown until we've committed all of our pending HTLCs, but the // remote side is unlikely to accept any new HTLCs, so we go ahead and "free" any holding // cell HTLCs and return them to fail the payment. self.context.holding_cell_update_fee = None; - let mut dropped_outbound_htlcs = Vec::with_capacity(self.context.holding_cell_htlc_updates.len()); - self.context.holding_cell_htlc_updates.retain(|htlc_update| { - match htlc_update { - &HTLCUpdateAwaitingACK::AddHTLC { ref payment_hash, ref source, .. } => { - dropped_outbound_htlcs.push((source.clone(), payment_hash.clone())); - false - }, - _ => true - } + let mut dropped_outbound_htlcs = + Vec::with_capacity(self.context.holding_cell_htlc_updates.len()); + self.context.holding_cell_htlc_updates.retain(|htlc_update| match htlc_update { + &HTLCUpdateAwaitingACK::AddHTLC { ref payment_hash, ref source, .. } => { + dropped_outbound_htlcs.push((source.clone(), payment_hash.clone())); + false + }, + _ => true, }); self.context.channel_state.set_local_shutdown_sent(); @@ -8302,18 +8404,24 @@ where tx } - #[rustfmt::skip] fn get_closing_signed_msg( - &mut self, closing_tx: &ClosingTransaction, skip_remote_output: bool, - fee_satoshis: u64, min_fee_satoshis: u64, max_fee_satoshis: u64, logger: &L + &mut self, closing_tx: &ClosingTransaction, skip_remote_output: bool, fee_satoshis: u64, + min_fee_satoshis: u64, max_fee_satoshis: u64, logger: &L, ) -> Option - where L::Target: Logger + where + L::Target: Logger, { let sig = match &self.context.holder_signer { - ChannelSignerType::Ecdsa(ecdsa) => ecdsa.sign_closing_transaction(&self.funding.channel_transaction_parameters, closing_tx, &self.context.secp_ctx).ok(), + ChannelSignerType::Ecdsa(ecdsa) => ecdsa + .sign_closing_transaction( + &self.funding.channel_transaction_parameters, + closing_tx, + &self.context.secp_ctx, + ) + .ok(), // TODO (taproot|arik) #[cfg(taproot)] - _ => todo!() + _ => todo!(), }; if sig.is_none() { log_trace!(logger, "Closing transaction signature unavailable, waiting on signer"); @@ -8322,7 +8430,8 @@ where self.context.signer_pending_closing = false; } let fee_range = msgs::ClosingSignedFeeRange { min_fee_satoshis, max_fee_satoshis }; - self.context.last_sent_closing_fee = Some((fee_satoshis, skip_remote_output, fee_range.clone(), sig.clone())); + self.context.last_sent_closing_fee = + Some((fee_satoshis, skip_remote_output, fee_range.clone(), sig.clone())); sig.map(|signature| msgs::ClosingSigned { channel_id: self.context.channel_id, fee_satoshis, @@ -8331,7 +8440,6 @@ where }) } - #[rustfmt::skip] fn shutdown_result_coop_close(&self) -> ShutdownResult { let closure_reason = if self.initiated_shutdown() { ClosureReason::LocallyInitiatedCooperativeClosure @@ -8342,7 +8450,9 @@ where closure_reason, monitor_update: None, dropped_outbound_htlcs: Vec::new(), - unbroadcasted_batch_funding_txid: self.context.unbroadcasted_batch_funding_txid(&self.funding), + unbroadcasted_batch_funding_txid: self + .context + .unbroadcasted_batch_funding_txid(&self.funding), channel_id: self.context.channel_id, user_channel_id: self.context.user_id, channel_capacity_satoshis: self.funding.get_value_satoshis(), @@ -8354,26 +8464,44 @@ where } } - #[rustfmt::skip] pub fn closing_signed( - &mut self, fee_estimator: &LowerBoundedFeeEstimator, msg: &msgs::ClosingSigned, logger: &L) - -> Result<(Option, Option, Option), ChannelError> - where F::Target: FeeEstimator, L::Target: Logger + &mut self, fee_estimator: &LowerBoundedFeeEstimator, msg: &msgs::ClosingSigned, + logger: &L, + ) -> Result< + (Option, Option, Option), + ChannelError, + > + where + F::Target: FeeEstimator, + L::Target: Logger, { if self.is_shutdown_pending_signature() { return Err(ChannelError::Warn(String::from("Remote end sent us a closing_signed while fully shutdown and just waiting on the final closing signature"))); } if !self.context.channel_state.is_both_sides_shutdown() { - return Err(ChannelError::close("Remote end sent us a closing_signed before both sides provided a shutdown".to_owned())); + return Err(ChannelError::close( + "Remote end sent us a closing_signed before both sides provided a shutdown" + .to_owned(), + )); } if self.context.channel_state.is_peer_disconnected() { - return Err(ChannelError::close("Peer sent closing_signed when we needed a channel_reestablish".to_owned())); + return Err(ChannelError::close( + "Peer sent closing_signed when we needed a channel_reestablish".to_owned(), + )); } - if !self.context.pending_inbound_htlcs.is_empty() || !self.context.pending_outbound_htlcs.is_empty() { - return Err(ChannelError::close("Remote end sent us a closing_signed while there were still pending HTLCs".to_owned())); + if !self.context.pending_inbound_htlcs.is_empty() + || !self.context.pending_outbound_htlcs.is_empty() + { + return Err(ChannelError::close( + "Remote end sent us a closing_signed while there were still pending HTLCs" + .to_owned(), + )); } - if msg.fee_satoshis > TOTAL_BITCOIN_SUPPLY_SATOSHIS { // this is required to stop potential overflow in build_closing_transaction - return Err(ChannelError::close("Remote tried to send us a closing tx with > 21 million BTC fee".to_owned())); + if msg.fee_satoshis > TOTAL_BITCOIN_SUPPLY_SATOSHIS { + // this is required to stop potential overflow in build_closing_transaction + return Err(ChannelError::close( + "Remote tried to send us a closing tx with > 21 million BTC fee".to_owned(), + )); } if self.funding.is_outbound() && self.context.last_sent_closing_fee.is_none() { @@ -8387,26 +8515,45 @@ where let funding_redeemscript = self.funding.get_funding_redeemscript(); let mut skip_remote_output = false; - let (mut closing_tx, used_total_fee) = self.build_closing_transaction(msg.fee_satoshis, skip_remote_output)?; + let (mut closing_tx, used_total_fee) = + self.build_closing_transaction(msg.fee_satoshis, skip_remote_output)?; if used_total_fee != msg.fee_satoshis { return Err(ChannelError::close(format!("Remote sent us a closing_signed with a fee other than the value they can claim. Fee in message: {}. Actual closing tx fee: {}", msg.fee_satoshis, used_total_fee))); } - let sighash = closing_tx.trust().get_sighash_all(&funding_redeemscript, self.funding.get_value_satoshis()); + let sighash = closing_tx + .trust() + .get_sighash_all(&funding_redeemscript, self.funding.get_value_satoshis()); - match self.context.secp_ctx.verify_ecdsa(&sighash, &msg.signature, &self.funding.get_counterparty_pubkeys().funding_pubkey) { + match self.context.secp_ctx.verify_ecdsa( + &sighash, + &msg.signature, + &self.funding.get_counterparty_pubkeys().funding_pubkey, + ) { Ok(_) => {}, Err(_e) => { // The remote end may have decided to revoke their output due to inconsistent dust // limits, so check for that case by re-checking the signature here. skip_remote_output = true; - closing_tx = self.build_closing_transaction(msg.fee_satoshis, skip_remote_output)?.0; - let sighash = closing_tx.trust().get_sighash_all(&funding_redeemscript, self.funding.get_value_satoshis()); - secp_check!(self.context.secp_ctx.verify_ecdsa(&sighash, &msg.signature, self.funding.counterparty_funding_pubkey()), "Invalid closing tx signature from peer".to_owned()); + closing_tx = + self.build_closing_transaction(msg.fee_satoshis, skip_remote_output)?.0; + let sighash = closing_tx + .trust() + .get_sighash_all(&funding_redeemscript, self.funding.get_value_satoshis()); + secp_check!( + self.context.secp_ctx.verify_ecdsa( + &sighash, + &msg.signature, + self.funding.counterparty_funding_pubkey() + ), + "Invalid closing tx signature from peer".to_owned() + ); }, }; for outp in closing_tx.trust().built_transaction().output.iter() { - if !outp.script_pubkey.is_witness_program() && outp.value < Amount::from_sat(MAX_STD_OUTPUT_DUST_LIMIT_SATOSHIS) { + if !outp.script_pubkey.is_witness_program() + && outp.value < Amount::from_sat(MAX_STD_OUTPUT_DUST_LIMIT_SATOSHIS) + { return Err(ChannelError::close("Remote sent us a closing_signed with a dust output. Always use segwit closing scripts!".to_owned())); } } @@ -8415,7 +8562,8 @@ where if let Some((last_fee, _, _, Some(sig))) = self.context.last_sent_closing_fee { if last_fee == msg.fee_satoshis { let shutdown_result = self.shutdown_result_coop_close(); - let tx = self.build_signed_closing_transaction(&mut closing_tx, &msg.signature, &sig); + let tx = + self.build_signed_closing_transaction(&mut closing_tx, &msg.signature, &sig); self.context.channel_state = ChannelState::ShutdownComplete; self.context.update_time_counter += 1; return Ok((None, Some(tx), Some(shutdown_result))); @@ -8433,26 +8581,40 @@ where self.build_closing_transaction($new_fee, skip_remote_output)? }; - let closing_signed = self.get_closing_signed_msg(&closing_tx, skip_remote_output, used_fee, our_min_fee, our_max_fee, logger); + let closing_signed = self.get_closing_signed_msg( + &closing_tx, + skip_remote_output, + used_fee, + our_min_fee, + our_max_fee, + logger, + ); let (signed_tx, shutdown_result) = if $new_fee == msg.fee_satoshis { - let shutdown_result = closing_signed.as_ref() - .map(|_| self.shutdown_result_coop_close()); + let shutdown_result = + closing_signed.as_ref().map(|_| self.shutdown_result_coop_close()); if closing_signed.is_some() { self.context.channel_state = ChannelState::ShutdownComplete; } self.context.update_time_counter += 1; self.context.last_received_closing_sig = Some(msg.signature.clone()); - let tx = closing_signed.as_ref().map(|ClosingSigned { signature, .. }| - self.build_signed_closing_transaction(&closing_tx, &msg.signature, signature)); + let tx = closing_signed.as_ref().map(|ClosingSigned { signature, .. }| { + self.build_signed_closing_transaction( + &closing_tx, + &msg.signature, + signature, + ) + }); (tx, shutdown_result) } else { (None, None) }; return Ok((closing_signed, signed_tx, shutdown_result)) - } + }; } - if let Some(msgs::ClosingSignedFeeRange { min_fee_satoshis, max_fee_satoshis }) = msg.fee_range { + if let Some(msgs::ClosingSignedFeeRange { min_fee_satoshis, max_fee_satoshis }) = + msg.fee_range + { if msg.fee_satoshis < min_fee_satoshis || msg.fee_satoshis > max_fee_satoshis { return Err(ChannelError::close(format!("Peer sent a bogus closing_signed - suggested fee of {} sat was not in their desired range of {} sat - {} sat", msg.fee_satoshis, min_fee_satoshis, max_fee_satoshis))); } @@ -8466,7 +8628,11 @@ where if !self.funding.is_outbound() { // They have to pay, so pick the highest fee in the overlapping range. // We should never set an upper bound aside from their full balance - debug_assert_eq!(our_max_fee, self.funding.get_value_satoshis() - (self.funding.value_to_self_msat + 999) / 1000); + debug_assert_eq!( + our_max_fee, + self.funding.get_value_satoshis() + - (self.funding.value_to_self_msat + 999) / 1000 + ); propose_fee!(cmp::min(max_fee_satoshis, our_max_fee)); } else { if msg.fee_satoshis < our_min_fee || msg.fee_satoshis > our_max_fee { @@ -9871,33 +10037,48 @@ where /// Begins the shutdown process, getting a message for the remote peer and returning all /// holding cell HTLCs for payment failure. - #[rustfmt::skip] - pub fn get_shutdown(&mut self, signer_provider: &SP, their_features: &InitFeatures, - target_feerate_sats_per_kw: Option, override_shutdown_script: Option) - -> Result<(msgs::Shutdown, Option, Vec<(HTLCSource, PaymentHash)>), APIError> - { + pub fn get_shutdown( + &mut self, signer_provider: &SP, their_features: &InitFeatures, + target_feerate_sats_per_kw: Option, override_shutdown_script: Option, + ) -> Result< + (msgs::Shutdown, Option, Vec<(HTLCSource, PaymentHash)>), + APIError, + > { if self.context.channel_state.is_local_stfu_sent() || self.context.channel_state.is_remote_stfu_sent() || self.context.channel_state.is_quiescent() { - return Err(APIError::APIMisuseError { err: "Cannot begin shutdown while quiescent".to_owned() }); + return Err(APIError::APIMisuseError { + err: "Cannot begin shutdown while quiescent".to_owned(), + }); } for htlc in self.context.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()}); + return Err(APIError::APIMisuseError { + err: "Cannot begin shutdown with pending HTLCs. Process pending events first" + .to_owned(), + }); } } if self.context.channel_state.is_local_shutdown_sent() { - return Err(APIError::APIMisuseError{err: "Shutdown already in progress".to_owned()}); - } - else if self.context.channel_state.is_remote_shutdown_sent() { - return Err(APIError::ChannelUnavailable{err: "Shutdown already in progress by remote".to_owned()}); + return Err(APIError::APIMisuseError { + err: "Shutdown already in progress".to_owned(), + }); + } else if self.context.channel_state.is_remote_shutdown_sent() { + return Err(APIError::ChannelUnavailable { + err: "Shutdown already in progress by remote".to_owned(), + }); } if self.context.shutdown_scriptpubkey.is_some() && override_shutdown_script.is_some() { - return Err(APIError::APIMisuseError{err: "Cannot override shutdown script for a channel with one already set".to_owned()}); + return Err(APIError::APIMisuseError { + err: "Cannot override shutdown script for a channel with one already set" + .to_owned(), + }); } assert!(!matches!(self.context.channel_state, ChannelState::ShutdownComplete)); - if self.context.channel_state.is_peer_disconnected() || self.context.channel_state.is_monitor_update_in_progress() { + if self.context.channel_state.is_peer_disconnected() + || self.context.channel_state.is_monitor_update_in_progress() + { 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()}); } @@ -9911,12 +10092,18 @@ where // otherwise, use the shutdown scriptpubkey provided by the signer match signer_provider.get_shutdown_scriptpubkey() { Ok(scriptpubkey) => scriptpubkey, - Err(_) => return Err(APIError::ChannelUnavailable{err: "Failed to get shutdown scriptpubkey".to_owned()}), + Err(_) => { + return Err(APIError::ChannelUnavailable { + err: "Failed to get shutdown scriptpubkey".to_owned(), + }) + }, } }, }; if !shutdown_scriptpubkey.is_compatible(their_features) { - return Err(APIError::IncompatibleShutdownScript { script: shutdown_scriptpubkey.clone() }); + return Err(APIError::IncompatibleShutdownScript { + script: shutdown_scriptpubkey.clone(), + }); } self.context.shutdown_scriptpubkey = Some(shutdown_scriptpubkey); true @@ -9943,7 +10130,9 @@ where }; self.monitor_updating_paused(false, false, false, Vec::new(), Vec::new(), Vec::new()); self.push_ret_blockable_mon_update(monitor_update) - } else { None }; + } else { + None + }; let shutdown = msgs::Shutdown { channel_id: self.context.channel_id, scriptpubkey: self.get_closing_scriptpubkey(), @@ -9952,19 +10141,20 @@ where // Go ahead and drop holding cell updates as we'd rather fail payments than wait to send // our shutdown until we've committed all of the pending changes. self.context.holding_cell_update_fee = None; - let mut dropped_outbound_htlcs = Vec::with_capacity(self.context.holding_cell_htlc_updates.len()); - self.context.holding_cell_htlc_updates.retain(|htlc_update| { - match htlc_update { - &HTLCUpdateAwaitingACK::AddHTLC { ref payment_hash, ref source, .. } => { - dropped_outbound_htlcs.push((source.clone(), payment_hash.clone())); - false - }, - _ => true - } + let mut dropped_outbound_htlcs = + Vec::with_capacity(self.context.holding_cell_htlc_updates.len()); + self.context.holding_cell_htlc_updates.retain(|htlc_update| match htlc_update { + &HTLCUpdateAwaitingACK::AddHTLC { ref payment_hash, ref source, .. } => { + dropped_outbound_htlcs.push((source.clone(), payment_hash.clone())); + false + }, + _ => true, }); - debug_assert!(!self.is_shutdown() || monitor_update.is_none(), - "we can't both complete shutdown and return a monitor update"); + debug_assert!( + !self.is_shutdown() || monitor_update.is_none(), + "we can't both complete shutdown and return a monitor update" + ); Ok((shutdown, monitor_update, dropped_outbound_htlcs)) } @@ -12432,26 +12622,40 @@ mod tests { } #[test] - #[rustfmt::skip] fn upfront_shutdown_script_incompatibility() { let mut features = channelmanager::provided_init_features(&UserConfig::default()); features.clear_shutdown_anysegwit(); let non_v0_segwit_shutdown_script = ShutdownScript::new_witness_program( &WitnessProgram::new(WitnessVersion::V16, &[0, 40]).unwrap(), - ).unwrap(); + ) + .unwrap(); let seed = [42; 32]; let network = Network::Testnet; let keys_provider = TestKeysInterface::new(&seed, network); - keys_provider.expect(OnGetShutdownScriptpubkey { - returns: non_v0_segwit_shutdown_script.clone(), - }); + keys_provider + .expect(OnGetShutdownScriptpubkey { returns: non_v0_segwit_shutdown_script.clone() }); let logger = TestLogger::new(); let secp_ctx = Secp256k1::new(); - let node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap()); + let node_id = + PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap()); let config = UserConfig::default(); - match OutboundV1Channel::<&TestKeysInterface>::new(&LowerBoundedFeeEstimator::new(&TestFeeEstimator::new(253)), &&keys_provider, &&keys_provider, node_id, &features, 10000000, 100000, 42, &config, 0, 42, None, &logger) { + match OutboundV1Channel::<&TestKeysInterface>::new( + &LowerBoundedFeeEstimator::new(&TestFeeEstimator::new(253)), + &&keys_provider, + &&keys_provider, + node_id, + &features, + 10000000, + 100000, + 42, + &config, + 0, + 42, + None, + &logger, + ) { Err(APIError::IncompatibleShutdownScript { script }) => { assert_eq!(script.into_inner(), non_v0_segwit_shutdown_script.into_inner()); }, diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 773fda9d769..458aeb2a2d4 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -9181,32 +9181,45 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ } } - #[rustfmt::skip] - fn internal_shutdown(&self, counterparty_node_id: &PublicKey, msg: &msgs::Shutdown) -> Result<(), MsgHandleErrInternal> { + fn internal_shutdown( + &self, counterparty_node_id: &PublicKey, msg: &msgs::Shutdown, + ) -> Result<(), MsgHandleErrInternal> { let mut dropped_htlcs: Vec<(HTLCSource, PaymentHash)> = Vec::new(); let mut finish_shutdown = None; { let per_peer_state = self.per_peer_state.read().unwrap(); - let peer_state_mutex = per_peer_state.get(counterparty_node_id) - .ok_or_else(|| { - debug_assert!(false); - MsgHandleErrInternal::send_err_msg_no_close(format!("Can't find a peer matching the passed counterparty node_id {}", counterparty_node_id), msg.channel_id) - })?; + let peer_state_mutex = per_peer_state.get(counterparty_node_id).ok_or_else(|| { + debug_assert!(false); + MsgHandleErrInternal::send_err_msg_no_close( + format!( + "Can't find a peer matching the passed counterparty node_id {}", + counterparty_node_id + ), + msg.channel_id, + ) + })?; let mut peer_state_lock = peer_state_mutex.lock().unwrap(); let peer_state = &mut *peer_state_lock; - if let hash_map::Entry::Occupied(mut chan_entry) = peer_state.channel_by_id.entry(msg.channel_id.clone()) { + if let hash_map::Entry::Occupied(mut chan_entry) = + peer_state.channel_by_id.entry(msg.channel_id.clone()) + { match chan_entry.get_mut().as_funded_mut() { Some(chan) => { if !chan.received_shutdown() { - let logger = WithChannelContext::from(&self.logger, &chan.context, None); + let logger = + WithChannelContext::from(&self.logger, &chan.context, None); log_info!(logger, "Received a shutdown message from our counterparty for channel {}{}.", msg.channel_id, if chan.sent_shutdown() { " after we initiated shutdown" } else { "" }); } let funding_txo_opt = chan.funding.get_funding_txo(); - let (shutdown, monitor_update_opt, htlcs) = try_channel_entry!(self, peer_state, - chan.shutdown(&self.signer_provider, &peer_state.latest_features, &msg), chan_entry); + let (shutdown, monitor_update_opt, htlcs) = try_channel_entry!( + self, + peer_state, + chan.shutdown(&self.signer_provider, &peer_state.latest_features, &msg), + chan_entry + ); dropped_htlcs = htlcs; if let Some(msg) = shutdown { @@ -9220,24 +9233,41 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ } // Update the monitor with the shutdown script if necessary. if let Some(monitor_update) = monitor_update_opt { - handle_new_monitor_update!(self, funding_txo_opt.unwrap(), monitor_update, - peer_state_lock, peer_state, per_peer_state, chan); + handle_new_monitor_update!( + self, + funding_txo_opt.unwrap(), + monitor_update, + peer_state_lock, + peer_state, + per_peer_state, + chan + ); } }, None => { - let logger = WithChannelContext::from(&self.logger, chan_entry.get().context(), None); + let logger = WithChannelContext::from( + &self.logger, + chan_entry.get().context(), + None, + ); log_error!(logger, "Immediately closing unfunded channel {} as peer asked to cooperatively shut it down (which is unnecessary)", &msg.channel_id); - let mut close_res = chan_entry.get_mut().force_shutdown(false, ClosureReason::CounterpartyCoopClosedUnfundedChannel); + let mut close_res = chan_entry.get_mut().force_shutdown( + false, + ClosureReason::CounterpartyCoopClosedUnfundedChannel, + ); remove_channel_entry!(self, peer_state, chan_entry, close_res); finish_shutdown = Some(close_res); }, } } else { - return Err(MsgHandleErrInternal::send_err_msg_no_close(format!("Got a message for a channel from the wrong node! No such channel for the passed counterparty_node_id {}", counterparty_node_id), msg.channel_id)) + return Err(MsgHandleErrInternal::send_err_msg_no_close(format!("Got a message for a channel from the wrong node! No such channel for the passed counterparty_node_id {}", counterparty_node_id), msg.channel_id)); } } for htlc_source in dropped_htlcs.drain(..) { - let receiver = HTLCHandlingFailureType::Forward { node_id: Some(counterparty_node_id.clone()), channel_id: msg.channel_id }; + let receiver = HTLCHandlingFailureType::Forward { + node_id: Some(counterparty_node_id.clone()), + channel_id: msg.channel_id, + }; let reason = HTLCFailReason::from_failure_code(LocalHTLCFailureReason::ChannelClosed); self.fail_htlc_backwards_internal(&htlc_source.0, &htlc_source.1, &reason, receiver); } @@ -9248,14 +9278,20 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ Ok(()) } - #[rustfmt::skip] - fn internal_closing_signed(&self, counterparty_node_id: &PublicKey, msg: &msgs::ClosingSigned) -> Result<(), MsgHandleErrInternal> { + fn internal_closing_signed( + &self, counterparty_node_id: &PublicKey, msg: &msgs::ClosingSigned, + ) -> Result<(), MsgHandleErrInternal> { let per_peer_state = self.per_peer_state.read().unwrap(); - let peer_state_mutex = per_peer_state.get(counterparty_node_id) - .ok_or_else(|| { - debug_assert!(false); - MsgHandleErrInternal::send_err_msg_no_close(format!("Can't find a peer matching the passed counterparty node_id {}", counterparty_node_id), msg.channel_id) - })?; + let peer_state_mutex = per_peer_state.get(counterparty_node_id).ok_or_else(|| { + debug_assert!(false); + MsgHandleErrInternal::send_err_msg_no_close( + format!( + "Can't find a peer matching the passed counterparty node_id {}", + counterparty_node_id + ), + msg.channel_id, + ) + })?; let (tx, chan_option, shutdown_result) = { let mut peer_state_lock = peer_state_mutex.lock().unwrap(); let peer_state = &mut *peer_state_lock; @@ -9294,15 +9330,19 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ }; if let Some(broadcast_tx) = tx { let channel_id = chan_option.as_ref().map(|channel| channel.context().channel_id()); - log_info!(WithContext::from(&self.logger, Some(*counterparty_node_id), channel_id, None), "Broadcasting {}", log_tx!(broadcast_tx)); + log_info!( + WithContext::from(&self.logger, Some(*counterparty_node_id), channel_id, None), + "Broadcasting {}", + log_tx!(broadcast_tx) + ); self.tx_broadcaster.broadcast_transactions(&[&broadcast_tx]); } if let Some(chan) = chan_option.as_ref().and_then(Channel::as_funded) { if let Ok(update) = self.get_channel_update_for_broadcast(chan) { - let mut pending_broadcast_messages = self.pending_broadcast_messages.lock().unwrap(); - pending_broadcast_messages.push(MessageSendEvent::BroadcastChannelUpdate { - msg: update - }); + let mut pending_broadcast_messages = + self.pending_broadcast_messages.lock().unwrap(); + pending_broadcast_messages + .push(MessageSendEvent::BroadcastChannelUpdate { msg: update }); } } mem::drop(per_peer_state); From 09f57a9a9afee142c9abe45d865db23874133a18 Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Mon, 16 Jun 2025 10:58:22 +0200 Subject: [PATCH 2/9] Post-`fmt` cleanup --- lightning/src/ln/channel.rs | 41 +++++++++++++++++-------------------- 1 file changed, 19 insertions(+), 22 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 2d09a600095..7fdfd4264b2 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -5252,18 +5252,14 @@ where // See test_duplicate_chan_id and test_pre_lockin_no_chan_closed_update for more. if !self.channel_state.is_pre_funded_state() { self.latest_monitor_update_id += 1; - Some(( - self.get_counterparty_node_id(), - funding_txo, - self.channel_id(), - ChannelMonitorUpdate { - update_id: self.latest_monitor_update_id, - updates: vec![ChannelMonitorUpdateStep::ChannelForceClosed { - should_broadcast, - }], - channel_id: Some(self.channel_id()), - }, - )) + let update = ChannelMonitorUpdate { + update_id: self.latest_monitor_update_id, + updates: vec![ChannelMonitorUpdateStep::ChannelForceClosed { + should_broadcast, + }], + channel_id: Some(self.channel_id()), + }; + Some((self.get_counterparty_node_id(), funding_txo, self.channel_id(), update)) } else { None } @@ -8539,14 +8535,12 @@ where let sighash = closing_tx .trust() .get_sighash_all(&funding_redeemscript, self.funding.get_value_satoshis()); - secp_check!( - self.context.secp_ctx.verify_ecdsa( - &sighash, - &msg.signature, - self.funding.counterparty_funding_pubkey() - ), - "Invalid closing tx signature from peer".to_owned() + let res = self.context.secp_ctx.verify_ecdsa( + &sighash, + &msg.signature, + self.funding.counterparty_funding_pubkey(), ); + secp_check!(res, "Invalid closing tx signature from peer".to_owned()); }, }; @@ -12635,14 +12629,16 @@ mod tests { let keys_provider = TestKeysInterface::new(&seed, network); keys_provider .expect(OnGetShutdownScriptpubkey { returns: non_v0_segwit_shutdown_script.clone() }); + let fee_estimator = TestFeeEstimator::new(253); + let bounded_fee_estimator = LowerBoundedFeeEstimator::new(&fee_estimator); let logger = TestLogger::new(); let secp_ctx = Secp256k1::new(); let node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap()); let config = UserConfig::default(); - match OutboundV1Channel::<&TestKeysInterface>::new( - &LowerBoundedFeeEstimator::new(&TestFeeEstimator::new(253)), + let res = OutboundV1Channel::new( + &bounded_fee_estimator, &&keys_provider, &&keys_provider, node_id, @@ -12655,7 +12651,8 @@ mod tests { 42, None, &logger, - ) { + ); + match res { Err(APIError::IncompatibleShutdownScript { script }) => { assert_eq!(script.into_inner(), non_v0_segwit_shutdown_script.into_inner()); }, From 262abf89c08d108dd582f52ba063c5751e4c5306 Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Tue, 5 Nov 2024 10:33:07 +0100 Subject: [PATCH 3/9] Add `SimpleClose` feature for bits 60/61 We define the feature bits that will be set in `Init` and `Node` contexts. --- lightning-types/src/features.rs | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/lightning-types/src/features.rs b/lightning-types/src/features.rs index 37e011a7f4e..aca4bb6e5a9 100644 --- a/lightning-types/src/features.rs +++ b/lightning-types/src/features.rs @@ -51,6 +51,8 @@ //! (see [BOLT-2](https://github.com/lightning/bolts/blob/master/02-peer-protocol.md) for more information). //! - `DualFund` - requires/supports V2 channel establishment //! (see [BOLT-2](https://github.com/lightning/bolts/blob/master/02-peer-protocol.md#channel-establishment-v2) for more information). +//! - `SimpleClose` - requires/supports simplified closing negotiation +//! (see [BOLT-2](https://github.com/lightning/bolts/blob/master/02-peer-protocol.md#closing-negotiation-closing_complete-and-closing_sig) for more information). //! - `OnionMessages` - requires/supports forwarding onion messages //! (see [BOLT-7](https://github.com/lightning/bolts/pull/759/files) for more information). // TODO: update link @@ -161,7 +163,7 @@ mod sealed { // Byte 6 ZeroConf, // Byte 7 - Trampoline, + Trampoline | SimpleClose, ] ); define_context!( @@ -182,7 +184,7 @@ mod sealed { // Byte 6 ZeroConf | Keysend, // Byte 7 - Trampoline, + Trampoline | SimpleClose, // Byte 8 - 31 ,,,,,,,,,,,,,,,,,,,,,,,, // Byte 32 @@ -660,9 +662,20 @@ mod sealed { supports_trampoline_routing, requires_trampoline_routing ); - // By default, allocate enough bytes to cover up to Trampoline. Update this as new features are + define_feature!( + 61, + SimpleClose, + [InitContext, NodeContext], + "Feature flags for simplified closing negotiation.", + set_simple_close_optional, + set_simple_close_required, + clear_simple_close, + supports_simple_close, + requires_simple_close + ); + // By default, allocate enough bytes to cover up to SimpleClose. Update this as new features are // added which we expect to appear commonly across contexts. - pub(super) const MIN_FEATURES_ALLOCATION_BYTES: usize = (57 + 7) / 8; + pub(super) const MIN_FEATURES_ALLOCATION_BYTES: usize = (61 + 7) / 8; define_feature!( 259, DnsResolver, @@ -1355,6 +1368,7 @@ mod tests { init_features.set_scid_privacy_optional(); init_features.set_zero_conf_optional(); init_features.set_quiescence_optional(); + init_features.set_simple_close_optional(); assert!(init_features.initial_routing_sync()); assert!(!init_features.supports_upfront_shutdown_script()); @@ -1370,7 +1384,8 @@ mod tests { // - onion_messages // - option_channel_type | option_scid_alias // - option_zeroconf - assert_eq!(node_features.flags.len(), 7); + // - option_simple_close + assert_eq!(node_features.flags.len(), 8); assert_eq!(node_features.flags[0], 0b00000001); assert_eq!(node_features.flags[1], 0b01010001); assert_eq!(node_features.flags[2], 0b10001010); @@ -1378,6 +1393,7 @@ mod tests { assert_eq!(node_features.flags[4], 0b10001000); assert_eq!(node_features.flags[5], 0b10100000); assert_eq!(node_features.flags[6], 0b00001000); + assert_eq!(node_features.flags[7], 0b00100000); } // Check that cleared flags are kept blank when converting back: From 694160f223033ab10e981abaf247e578adbdb131 Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Tue, 1 Apr 2025 11:37:28 +0200 Subject: [PATCH 4/9] Enable `option_simple_close` feature by default .. although behind a `cfg` gate for now --- Cargo.toml | 1 + ci/ci-tests.sh | 2 ++ lightning/src/ln/channelmanager.rs | 4 +++- lightning/src/ln/peer_handler.rs | 2 ++ 4 files changed, 8 insertions(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 5f19f395a09..3891b11a2b4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -67,4 +67,5 @@ check-cfg = [ "cfg(require_route_graph_test)", "cfg(splicing)", "cfg(async_payments)", + "cfg(simple_close)", ] diff --git a/ci/ci-tests.sh b/ci/ci-tests.sh index d426dbec745..2ab512e2d3e 100755 --- a/ci/ci-tests.sh +++ b/ci/ci-tests.sh @@ -155,4 +155,6 @@ RUSTFLAGS="--cfg=splicing" cargo test --verbose --color always -p lightning [ "$CI_MINIMIZE_DISK_USAGE" != "" ] && cargo clean RUSTFLAGS="--cfg=async_payments" cargo test --verbose --color always -p lightning [ "$CI_MINIMIZE_DISK_USAGE" != "" ] && cargo clean +RUSTFLAGS="--cfg=simple_close" cargo test --verbose --color always -p lightning +[ "$CI_MINIMIZE_DISK_USAGE" != "" ] && cargo clean RUSTFLAGS="--cfg=lsps1_service" cargo test --verbose --color always -p lightning-liquidity diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 458aeb2a2d4..d55e4f95442 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -13379,7 +13379,7 @@ pub(crate) fn provided_channel_type_features(config: &UserConfig) -> ChannelType /// [`ChannelManager`]. pub fn provided_init_features(config: &UserConfig) -> InitFeatures { // Note that if new features are added here which other peers may (eventually) require, we - // should also add the corresponding (optional) bit to the [`ChannelMessageHandler`] impl for + // should also add the corresponding (optional) bit to the [`BaseMessageHandler`] impl for // [`ErroringMessageHandler`]. let mut features = InitFeatures::empty(); features.set_data_loss_protect_required(); @@ -13395,6 +13395,8 @@ pub fn provided_init_features(config: &UserConfig) -> InitFeatures { features.set_zero_conf_optional(); features.set_route_blinding_optional(); features.set_provide_storage_optional(); + #[cfg(simple_close)] + features.set_simple_close_optional(); if config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx { features.set_anchors_zero_fee_htlc_tx_optional(); } diff --git a/lightning/src/ln/peer_handler.rs b/lightning/src/ln/peer_handler.rs index a989d172687..87a685ad599 100644 --- a/lightning/src/ln/peer_handler.rs +++ b/lightning/src/ln/peer_handler.rs @@ -271,6 +271,8 @@ impl BaseMessageHandler for ErroringMessageHandler { features.set_scid_privacy_optional(); features.set_zero_conf_optional(); features.set_route_blinding_optional(); + #[cfg(simple_close)] + features.set_simple_close_optional(); features } From 692105cc5d2104b4443f5706f5c8c8e60dfd83a7 Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Tue, 5 Nov 2024 11:13:28 +0100 Subject: [PATCH 5/9] Add `ClosingComplete` and `ClosingSig` message types + serialization --- lightning-net-tokio/src/lib.rs | 4 ++ lightning/src/ln/channelmanager.rs | 28 ++++++++++++ lightning/src/ln/msgs.rs | 70 ++++++++++++++++++++++++++++++ lightning/src/ln/peer_handler.rs | 18 +++++++- lightning/src/ln/wire.rs | 28 ++++++++++++ lightning/src/util/test_utils.rs | 8 ++++ 6 files changed, 155 insertions(+), 1 deletion(-) diff --git a/lightning-net-tokio/src/lib.rs b/lightning-net-tokio/src/lib.rs index ec26ae8c461..e7d29912be7 100644 --- a/lightning-net-tokio/src/lib.rs +++ b/lightning-net-tokio/src/lib.rs @@ -716,6 +716,10 @@ mod tests { fn handle_channel_ready(&self, _their_node_id: PublicKey, _msg: &ChannelReady) {} fn handle_shutdown(&self, _their_node_id: PublicKey, _msg: &Shutdown) {} fn handle_closing_signed(&self, _their_node_id: PublicKey, _msg: &ClosingSigned) {} + #[cfg(simple_close)] + fn handle_closing_complete(&self, _their_node_id: PublicKey, _msg: ClosingComplete) {} + #[cfg(simple_close)] + fn handle_closing_sig(&self, _their_node_id: PublicKey, _msg: ClosingSig) {} fn handle_update_add_htlc(&self, _their_node_id: PublicKey, _msg: &UpdateAddHTLC) {} fn handle_update_fulfill_htlc(&self, _their_node_id: PublicKey, _msg: &UpdateFulfillHTLC) {} fn handle_update_fail_htlc(&self, _their_node_id: PublicKey, _msg: &UpdateFailHTLC) {} diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index d55e4f95442..e47356c7e9b 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -9352,6 +9352,20 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ Ok(()) } + #[cfg(simple_close)] + fn internal_closing_complete( + &self, _counterparty_node_id: PublicKey, _msg: msgs::ClosingComplete, + ) -> Result<(), MsgHandleErrInternal> { + unimplemented!("Handling ClosingComplete is not implemented"); + } + + #[cfg(simple_close)] + fn internal_closing_sig( + &self, _counterparty_node_id: PublicKey, _msg: msgs::ClosingSig, + ) -> Result<(), MsgHandleErrInternal> { + unimplemented!("Handling ClosingSig is not implemented"); + } + #[rustfmt::skip] fn internal_update_add_htlc(&self, counterparty_node_id: &PublicKey, msg: &msgs::UpdateAddHTLC) -> Result<(), MsgHandleErrInternal> { //TODO: BOLT 4 points out a specific attack where a peer may re-send an onion packet and @@ -12650,6 +12664,20 @@ where let _ = handle_error!(self, res, counterparty_node_id); } + #[cfg(simple_close)] + fn handle_closing_complete(&self, counterparty_node_id: PublicKey, msg: msgs::ClosingComplete) { + let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self); + let res = self.internal_closing_complete(counterparty_node_id, msg); + let _ = handle_error!(self, res, counterparty_node_id); + } + + #[cfg(simple_close)] + fn handle_closing_sig(&self, counterparty_node_id: PublicKey, msg: msgs::ClosingSig) { + let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self); + let res = self.internal_closing_sig(counterparty_node_id, msg); + let _ = handle_error!(self, res, counterparty_node_id); + } + fn handle_update_add_htlc(&self, counterparty_node_id: PublicKey, msg: &msgs::UpdateAddHTLC) { // Note that we never need to persist the updated ChannelManager for an inbound // update_add_htlc message - the message itself doesn't change our channel state only the diff --git a/lightning/src/ln/msgs.rs b/lightning/src/ln/msgs.rs index 2b18901591e..35612f814ef 100644 --- a/lightning/src/ln/msgs.rs +++ b/lightning/src/ln/msgs.rs @@ -684,6 +684,52 @@ pub struct ClosingSigned { pub fee_range: Option, } +/// A [`closing_complete`] message to be sent to or received from a peer. +/// +/// [`closing_complete`]: https://github.com/lightning/bolts/blob/master/02-peer-protocol.md#closing-negotiation-closing_complete-and-closing_sig +#[derive(Clone, Debug, Hash, PartialEq, Eq)] +pub struct ClosingComplete { + /// The channel ID. + pub channel_id: ChannelId, + /// The destination of the closer's funds on closing. + pub closer_scriptpubkey: ScriptBuf, + /// The destination of the closee's funds on closing. + pub closee_scriptpubkey: ScriptBuf, + /// The proposed total fee for the closing transaction. + pub fee_satoshis: u64, + /// The locktime of the closing transaction. + pub locktime: u32, + /// A signature on the closing transaction omitting the `closee` output. + pub closer_output_only: Option, + /// A signature on the closing transaction omitting the `closer` output. + pub closee_output_only: Option, + /// A signature on the closing transaction covering both `closer` and `closee` outputs. + pub closer_and_closee_outputs: Option, +} + +/// A [`closing_sig`] message to be sent to or received from a peer. +/// +/// [`closing_sig`]: https://github.com/lightning/bolts/blob/master/02-peer-protocol.md#closing-negotiation-closing_complete-and-closing_sig +#[derive(Clone, Debug, Hash, PartialEq, Eq)] +pub struct ClosingSig { + /// The channel ID. + pub channel_id: ChannelId, + /// The destination of the closer's funds on closing. + pub closer_scriptpubkey: ScriptBuf, + /// The destination of the closee's funds on closing. + pub closee_scriptpubkey: ScriptBuf, + /// The proposed total fee for the closing transaction. + pub fee_satoshis: u64, + /// The locktime of the closing transaction. + pub locktime: u32, + /// A signature on the closing transaction omitting the `closee` output. + pub closer_output_only: Option, + /// A signature on the closing transaction omitting the `closer` output. + pub closee_output_only: Option, + /// A signature on the closing transaction covering both `closer` and `closee` outputs. + pub closer_and_closee_outputs: Option, +} + /// A [`start_batch`] message to be sent to group together multiple channel messages as a single /// logical message. /// @@ -1914,6 +1960,12 @@ pub trait ChannelMessageHandler: BaseMessageHandler { fn handle_shutdown(&self, their_node_id: PublicKey, msg: &Shutdown); /// Handle an incoming `closing_signed` message from the given peer. fn handle_closing_signed(&self, their_node_id: PublicKey, msg: &ClosingSigned); + /// Handle an incoming `closing_complete` message from the given peer. + #[cfg(simple_close)] + fn handle_closing_complete(&self, their_node_id: PublicKey, msg: ClosingComplete); + /// Handle an incoming `closing_sig` message from the given peer. + #[cfg(simple_close)] + fn handle_closing_sig(&self, their_node_id: PublicKey, msg: ClosingSig); // Quiescence /// Handle an incoming `stfu` message from the given peer. @@ -2744,6 +2796,24 @@ impl_writeable_msg!(ClosingSigned, { (1, fee_range, option) } ); +impl_writeable_msg!(ClosingComplete, + { channel_id, closer_scriptpubkey, closee_scriptpubkey, fee_satoshis, locktime }, + { + (1, closer_output_only, option), + (2, closee_output_only, option), + (3, closer_and_closee_outputs, option) + } +); + +impl_writeable_msg!(ClosingSig, + { channel_id, closer_scriptpubkey, closee_scriptpubkey, fee_satoshis, locktime }, + { + (1, closer_output_only, option), + (2, closee_output_only, option), + (3, closer_and_closee_outputs, option) + } +); + impl_writeable!(ClosingSignedFeeRange, { min_fee_satoshis, max_fee_satoshis diff --git a/lightning/src/ln/peer_handler.rs b/lightning/src/ln/peer_handler.rs index 87a685ad599..6f0163fa213 100644 --- a/lightning/src/ln/peer_handler.rs +++ b/lightning/src/ln/peer_handler.rs @@ -306,6 +306,14 @@ impl ChannelMessageHandler for ErroringMessageHandler { fn handle_closing_signed(&self, their_node_id: PublicKey, msg: &msgs::ClosingSigned) { ErroringMessageHandler::push_error(self, their_node_id, msg.channel_id); } + #[cfg(simple_close)] + fn handle_closing_complete(&self, their_node_id: PublicKey, msg: msgs::ClosingComplete) { + ErroringMessageHandler::push_error(self, their_node_id, msg.channel_id); + } + #[cfg(simple_close)] + fn handle_closing_sig(&self, their_node_id: PublicKey, msg: msgs::ClosingSig) { + ErroringMessageHandler::push_error(self, their_node_id, msg.channel_id); + } fn handle_stfu(&self, their_node_id: PublicKey, msg: &msgs::Stfu) { ErroringMessageHandler::push_error(&self, their_node_id, msg.channel_id); } @@ -451,7 +459,7 @@ pub struct MessageHandler { self.message_handler.chan_handler.handle_closing_signed(their_node_id, &msg); }, + #[cfg(simple_close)] + wire::Message::ClosingComplete(msg) => { + self.message_handler.chan_handler.handle_closing_complete(their_node_id, msg); + }, + #[cfg(simple_close)] + wire::Message::ClosingSig(msg) => { + self.message_handler.chan_handler.handle_closing_sig(their_node_id, msg); + }, // Commitment messages: wire::Message::UpdateAddHTLC(msg) => { diff --git a/lightning/src/ln/wire.rs b/lightning/src/ln/wire.rs index 2dc54f852b5..32b1792a566 100644 --- a/lightning/src/ln/wire.rs +++ b/lightning/src/ln/wire.rs @@ -81,6 +81,10 @@ pub(crate) enum Message { ChannelReady(msgs::ChannelReady), Shutdown(msgs::Shutdown), ClosingSigned(msgs::ClosingSigned), + #[cfg(simple_close)] + ClosingComplete(msgs::ClosingComplete), + #[cfg(simple_close)] + ClosingSig(msgs::ClosingSig), OnionMessage(msgs::OnionMessage), StartBatch(msgs::StartBatch), UpdateAddHTLC(msgs::UpdateAddHTLC), @@ -142,6 +146,10 @@ impl Writeable for Message { &Message::ChannelReady(ref msg) => msg.write(writer), &Message::Shutdown(ref msg) => msg.write(writer), &Message::ClosingSigned(ref msg) => msg.write(writer), + #[cfg(simple_close)] + &Message::ClosingComplete(ref msg) => msg.write(writer), + #[cfg(simple_close)] + &Message::ClosingSig(ref msg) => msg.write(writer), &Message::OnionMessage(ref msg) => msg.write(writer), &Message::StartBatch(ref msg) => msg.write(writer), &Message::UpdateAddHTLC(ref msg) => msg.write(writer), @@ -203,6 +211,10 @@ impl Type for Message { &Message::ChannelReady(ref msg) => msg.type_id(), &Message::Shutdown(ref msg) => msg.type_id(), &Message::ClosingSigned(ref msg) => msg.type_id(), + #[cfg(simple_close)] + &Message::ClosingComplete(ref msg) => msg.type_id(), + #[cfg(simple_close)] + &Message::ClosingSig(ref msg) => msg.type_id(), &Message::OnionMessage(ref msg) => msg.type_id(), &Message::StartBatch(ref msg) => msg.type_id(), &Message::UpdateAddHTLC(ref msg) => msg.type_id(), @@ -350,6 +362,14 @@ where msgs::ClosingSigned::TYPE => { Ok(Message::ClosingSigned(LengthReadable::read_from_fixed_length_buffer(buffer)?)) }, + #[cfg(simple_close)] + msgs::ClosingComplete::TYPE => { + Ok(Message::ClosingComplete(LengthReadable::read_from_fixed_length_buffer(buffer)?)) + }, + #[cfg(simple_close)] + msgs::ClosingSig::TYPE => { + Ok(Message::ClosingSig(LengthReadable::read_from_fixed_length_buffer(buffer)?)) + }, msgs::OnionMessage::TYPE => { Ok(Message::OnionMessage(LengthReadable::read_from_fixed_length_buffer(buffer)?)) }, @@ -535,6 +555,14 @@ impl Encode for msgs::ClosingSigned { const TYPE: u16 = 39; } +impl Encode for msgs::ClosingComplete { + const TYPE: u16 = 40; +} + +impl Encode for msgs::ClosingSig { + const TYPE: u16 = 41; +} + impl Encode for msgs::OpenChannelV2 { const TYPE: u16 = 64; } diff --git a/lightning/src/util/test_utils.rs b/lightning/src/util/test_utils.rs index b26794a30c6..fecdb830fe0 100644 --- a/lightning/src/util/test_utils.rs +++ b/lightning/src/util/test_utils.rs @@ -1024,6 +1024,14 @@ impl msgs::ChannelMessageHandler for TestChannelMessageHandler { fn handle_closing_signed(&self, _their_node_id: PublicKey, msg: &msgs::ClosingSigned) { self.received_msg(wire::Message::ClosingSigned(msg.clone())); } + #[cfg(simple_close)] + fn handle_closing_complete(&self, _their_node_id: PublicKey, msg: msgs::ClosingComplete) { + self.received_msg(wire::Message::ClosingComplete(msg)); + } + #[cfg(simple_close)] + fn handle_closing_sig(&self, _their_node_id: PublicKey, msg: msgs::ClosingSig) { + self.received_msg(wire::Message::ClosingSig(msg)); + } fn handle_stfu(&self, _their_node_id: PublicKey, msg: &msgs::Stfu) { self.received_msg(wire::Message::Stfu(msg.clone())); } From 29fc4f28d70e02e519d32864054b63630a108fff Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Tue, 5 Nov 2024 11:21:05 +0100 Subject: [PATCH 6/9] Add `MessageSendEvent` for `ClosingComplete` and `ClosingSig` --- lightning/src/ln/channelmanager.rs | 2 ++ lightning/src/ln/functional_test_utils.rs | 6 ++++++ lightning/src/ln/msgs.rs | 14 ++++++++++++++ lightning/src/ln/peer_handler.rs | 12 ++++++++++++ 4 files changed, 34 insertions(+) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index e47356c7e9b..b420feeca8c 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -11712,6 +11712,8 @@ where &MessageSendEvent::UpdateHTLCs { .. } => false, &MessageSendEvent::SendRevokeAndACK { .. } => false, &MessageSendEvent::SendClosingSigned { .. } => false, + &MessageSendEvent::SendClosingComplete { .. } => false, + &MessageSendEvent::SendClosingSig { .. } => false, &MessageSendEvent::SendShutdown { .. } => false, &MessageSendEvent::SendChannelReestablish { .. } => false, &MessageSendEvent::HandleError { .. } => false, diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index ce794cd7cb5..79219857b0e 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -937,6 +937,12 @@ pub fn remove_first_msg_event_to_node(msg_node_id: &PublicKey, msg_events: &mut MessageSendEvent::SendClosingSigned { node_id, .. } => { node_id == msg_node_id }, + MessageSendEvent::SendClosingComplete { node_id, .. } => { + node_id == msg_node_id + }, + MessageSendEvent::SendClosingSig { node_id, .. } => { + node_id == msg_node_id + }, MessageSendEvent::SendShutdown { node_id, .. } => { node_id == msg_node_id }, diff --git a/lightning/src/ln/msgs.rs b/lightning/src/ln/msgs.rs index 35612f814ef..bda4a41c97b 100644 --- a/lightning/src/ln/msgs.rs +++ b/lightning/src/ln/msgs.rs @@ -1776,6 +1776,20 @@ pub enum MessageSendEvent { /// The message which should be sent. msg: ClosingSigned, }, + /// Used to indicate that a `closing_complete` message should be sent to the peer with the given `node_id`. + SendClosingComplete { + /// The node_id of the node which should receive this message + node_id: PublicKey, + /// The message which should be sent. + msg: ClosingComplete, + }, + /// Used to indicate that a `closing_sig` message should be sent to the peer with the given `node_id`. + SendClosingSig { + /// The node_id of the node which should receive this message + node_id: PublicKey, + /// The message which should be sent. + msg: ClosingSig, + }, /// Used to indicate that a shutdown message should be sent to the peer with the given node_id. SendShutdown { /// The node_id of the node which should receive this message diff --git a/lightning/src/ln/peer_handler.rs b/lightning/src/ln/peer_handler.rs index 6f0163fa213..93bab4516dc 100644 --- a/lightning/src/ln/peer_handler.rs +++ b/lightning/src/ln/peer_handler.rs @@ -2559,6 +2559,18 @@ impl { + log_debug!(WithContext::from(&self.logger, Some(*node_id), Some(msg.channel_id), None), "Handling SendClosingComplete event in peer_handler for node {} for channel {}", + log_pubkey!(node_id), + &msg.channel_id); + self.enqueue_message(&mut *get_peer_for_forwarding!(node_id)?, msg); + }, + MessageSendEvent::SendClosingSig { ref node_id, ref msg } => { + log_debug!(WithContext::from(&self.logger, Some(*node_id), Some(msg.channel_id), None), "Handling SendClosingSig event in peer_handler for node {} for channel {}", + log_pubkey!(node_id), + &msg.channel_id); + self.enqueue_message(&mut *get_peer_for_forwarding!(node_id)?, msg); + }, MessageSendEvent::SendShutdown { ref node_id, ref msg } => { log_debug!(WithContext::from(&self.logger, Some(*node_id), Some(msg.channel_id), None), "Handling Shutdown event in peer_handler for node {} for channel {}", log_pubkey!(node_id), From db456bedb27384eac0540d7d533a6e3b1e4bdc8f Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Mon, 31 Mar 2025 10:52:57 +0200 Subject: [PATCH 7/9] Add `fuzz` targets for `ClosingComplete` and `ClosingSig` messages --- fuzz/src/bin/gen_target.sh | 2 + fuzz/src/bin/msg_closing_complete_target.rs | 120 +++++++++++++++++++ fuzz/src/bin/msg_closing_sig_target.rs | 120 +++++++++++++++++++ fuzz/src/msg_targets/gen_target.sh | 2 + fuzz/src/msg_targets/mod.rs | 2 + fuzz/src/msg_targets/msg_closing_complete.rs | 27 +++++ fuzz/src/msg_targets/msg_closing_sig.rs | 27 +++++ fuzz/targets.h | 2 + 8 files changed, 302 insertions(+) create mode 100644 fuzz/src/bin/msg_closing_complete_target.rs create mode 100644 fuzz/src/bin/msg_closing_sig_target.rs create mode 100644 fuzz/src/msg_targets/msg_closing_complete.rs create mode 100644 fuzz/src/msg_targets/msg_closing_sig.rs diff --git a/fuzz/src/bin/gen_target.sh b/fuzz/src/bin/gen_target.sh index 5672363ddd3..af7b68bda74 100755 --- a/fuzz/src/bin/gen_target.sh +++ b/fuzz/src/bin/gen_target.sh @@ -31,6 +31,8 @@ GEN_TEST msg_accept_channel msg_targets:: GEN_TEST msg_announcement_signatures msg_targets:: GEN_TEST msg_channel_reestablish msg_targets:: GEN_TEST msg_closing_signed msg_targets:: +GEN_TEST msg_closing_complete msg_targets:: +GEN_TEST msg_closing_sig msg_targets:: GEN_TEST msg_commitment_signed msg_targets:: GEN_TEST msg_decoded_onion_error_packet msg_targets:: GEN_TEST msg_funding_created msg_targets:: diff --git a/fuzz/src/bin/msg_closing_complete_target.rs b/fuzz/src/bin/msg_closing_complete_target.rs new file mode 100644 index 00000000000..d097e0f6b81 --- /dev/null +++ b/fuzz/src/bin/msg_closing_complete_target.rs @@ -0,0 +1,120 @@ +// This file is Copyright its original authors, visible in version control +// history. +// +// This file is licensed under the Apache License, Version 2.0 or the MIT license +// , at your option. +// You may not use this file except in accordance with one or both of these +// licenses. + +// This file is auto-generated by gen_target.sh based on target_template.txt +// To modify it, modify target_template.txt and run gen_target.sh instead. + +#![cfg_attr(feature = "libfuzzer_fuzz", no_main)] +#![cfg_attr(rustfmt, rustfmt_skip)] + +#[cfg(not(fuzzing))] +compile_error!("Fuzz targets need cfg=fuzzing"); + +#[cfg(not(hashes_fuzz))] +compile_error!("Fuzz targets need cfg=hashes_fuzz"); + +#[cfg(not(secp256k1_fuzz))] +compile_error!("Fuzz targets need cfg=secp256k1_fuzz"); + +extern crate lightning_fuzz; +use lightning_fuzz::msg_targets::msg_closing_complete::*; + +#[cfg(feature = "afl")] +#[macro_use] extern crate afl; +#[cfg(feature = "afl")] +fn main() { + fuzz!(|data| { + msg_closing_complete_run(data.as_ptr(), data.len()); + }); +} + +#[cfg(feature = "honggfuzz")] +#[macro_use] extern crate honggfuzz; +#[cfg(feature = "honggfuzz")] +fn main() { + loop { + fuzz!(|data| { + msg_closing_complete_run(data.as_ptr(), data.len()); + }); + } +} + +#[cfg(feature = "libfuzzer_fuzz")] +#[macro_use] extern crate libfuzzer_sys; +#[cfg(feature = "libfuzzer_fuzz")] +fuzz_target!(|data: &[u8]| { + msg_closing_complete_run(data.as_ptr(), data.len()); +}); + +#[cfg(feature = "stdin_fuzz")] +fn main() { + use std::io::Read; + + let mut data = Vec::with_capacity(8192); + std::io::stdin().read_to_end(&mut data).unwrap(); + msg_closing_complete_run(data.as_ptr(), data.len()); +} + +#[test] +fn run_test_cases() { + use std::fs; + use std::io::Read; + use lightning_fuzz::utils::test_logger::StringBuffer; + + use std::sync::{atomic, Arc}; + { + let data: Vec = vec![0]; + msg_closing_complete_run(data.as_ptr(), data.len()); + } + let mut threads = Vec::new(); + let threads_running = Arc::new(atomic::AtomicUsize::new(0)); + if let Ok(tests) = fs::read_dir("test_cases/msg_closing_complete") { + for test in tests { + let mut data: Vec = Vec::new(); + let path = test.unwrap().path(); + fs::File::open(&path).unwrap().read_to_end(&mut data).unwrap(); + threads_running.fetch_add(1, atomic::Ordering::AcqRel); + + let thread_count_ref = Arc::clone(&threads_running); + let main_thread_ref = std::thread::current(); + threads.push((path.file_name().unwrap().to_str().unwrap().to_string(), + std::thread::spawn(move || { + let string_logger = StringBuffer::new(); + + let panic_logger = string_logger.clone(); + let res = if ::std::panic::catch_unwind(move || { + msg_closing_complete_test(&data, panic_logger); + }).is_err() { + Some(string_logger.into_string()) + } else { None }; + thread_count_ref.fetch_sub(1, atomic::Ordering::AcqRel); + main_thread_ref.unpark(); + res + }) + )); + while threads_running.load(atomic::Ordering::Acquire) > 32 { + std::thread::park(); + } + } + } + let mut failed_outputs = Vec::new(); + for (test, thread) in threads.drain(..) { + if let Some(output) = thread.join().unwrap() { + println!("\nOutput of {}:\n{}\n", test, output); + failed_outputs.push(test); + } + } + if !failed_outputs.is_empty() { + println!("Test cases which failed: "); + for case in failed_outputs { + println!("{}", case); + } + panic!(); + } +} diff --git a/fuzz/src/bin/msg_closing_sig_target.rs b/fuzz/src/bin/msg_closing_sig_target.rs new file mode 100644 index 00000000000..67150cef167 --- /dev/null +++ b/fuzz/src/bin/msg_closing_sig_target.rs @@ -0,0 +1,120 @@ +// This file is Copyright its original authors, visible in version control +// history. +// +// This file is licensed under the Apache License, Version 2.0 or the MIT license +// , at your option. +// You may not use this file except in accordance with one or both of these +// licenses. + +// This file is auto-generated by gen_target.sh based on target_template.txt +// To modify it, modify target_template.txt and run gen_target.sh instead. + +#![cfg_attr(feature = "libfuzzer_fuzz", no_main)] +#![cfg_attr(rustfmt, rustfmt_skip)] + +#[cfg(not(fuzzing))] +compile_error!("Fuzz targets need cfg=fuzzing"); + +#[cfg(not(hashes_fuzz))] +compile_error!("Fuzz targets need cfg=hashes_fuzz"); + +#[cfg(not(secp256k1_fuzz))] +compile_error!("Fuzz targets need cfg=secp256k1_fuzz"); + +extern crate lightning_fuzz; +use lightning_fuzz::msg_targets::msg_closing_sig::*; + +#[cfg(feature = "afl")] +#[macro_use] extern crate afl; +#[cfg(feature = "afl")] +fn main() { + fuzz!(|data| { + msg_closing_sig_run(data.as_ptr(), data.len()); + }); +} + +#[cfg(feature = "honggfuzz")] +#[macro_use] extern crate honggfuzz; +#[cfg(feature = "honggfuzz")] +fn main() { + loop { + fuzz!(|data| { + msg_closing_sig_run(data.as_ptr(), data.len()); + }); + } +} + +#[cfg(feature = "libfuzzer_fuzz")] +#[macro_use] extern crate libfuzzer_sys; +#[cfg(feature = "libfuzzer_fuzz")] +fuzz_target!(|data: &[u8]| { + msg_closing_sig_run(data.as_ptr(), data.len()); +}); + +#[cfg(feature = "stdin_fuzz")] +fn main() { + use std::io::Read; + + let mut data = Vec::with_capacity(8192); + std::io::stdin().read_to_end(&mut data).unwrap(); + msg_closing_sig_run(data.as_ptr(), data.len()); +} + +#[test] +fn run_test_cases() { + use std::fs; + use std::io::Read; + use lightning_fuzz::utils::test_logger::StringBuffer; + + use std::sync::{atomic, Arc}; + { + let data: Vec = vec![0]; + msg_closing_sig_run(data.as_ptr(), data.len()); + } + let mut threads = Vec::new(); + let threads_running = Arc::new(atomic::AtomicUsize::new(0)); + if let Ok(tests) = fs::read_dir("test_cases/msg_closing_sig") { + for test in tests { + let mut data: Vec = Vec::new(); + let path = test.unwrap().path(); + fs::File::open(&path).unwrap().read_to_end(&mut data).unwrap(); + threads_running.fetch_add(1, atomic::Ordering::AcqRel); + + let thread_count_ref = Arc::clone(&threads_running); + let main_thread_ref = std::thread::current(); + threads.push((path.file_name().unwrap().to_str().unwrap().to_string(), + std::thread::spawn(move || { + let string_logger = StringBuffer::new(); + + let panic_logger = string_logger.clone(); + let res = if ::std::panic::catch_unwind(move || { + msg_closing_sig_test(&data, panic_logger); + }).is_err() { + Some(string_logger.into_string()) + } else { None }; + thread_count_ref.fetch_sub(1, atomic::Ordering::AcqRel); + main_thread_ref.unpark(); + res + }) + )); + while threads_running.load(atomic::Ordering::Acquire) > 32 { + std::thread::park(); + } + } + } + let mut failed_outputs = Vec::new(); + for (test, thread) in threads.drain(..) { + if let Some(output) = thread.join().unwrap() { + println!("\nOutput of {}:\n{}\n", test, output); + failed_outputs.push(test); + } + } + if !failed_outputs.is_empty() { + println!("Test cases which failed: "); + for case in failed_outputs { + println!("{}", case); + } + panic!(); + } +} diff --git a/fuzz/src/msg_targets/gen_target.sh b/fuzz/src/msg_targets/gen_target.sh index c8cc45926a1..ada61010b77 100755 --- a/fuzz/src/msg_targets/gen_target.sh +++ b/fuzz/src/msg_targets/gen_target.sh @@ -17,6 +17,8 @@ GEN_TEST() { GEN_TEST lightning::ln::msgs::AcceptChannel test_msg_simple "" GEN_TEST lightning::ln::msgs::AnnouncementSignatures test_msg_simple "" GEN_TEST lightning::ln::msgs::ClosingSigned test_msg_simple "" +GEN_TEST lightning::ln::msgs::ClosingComplete test_msg_simple "" +GEN_TEST lightning::ln::msgs::ClosingSig test_msg_simple "" GEN_TEST lightning::ln::msgs::CommitmentSigned test_msg_simple "" GEN_TEST lightning::ln::msgs::FundingCreated test_msg_simple "" GEN_TEST lightning::ln::msgs::ChannelReady test_msg_simple "" diff --git a/fuzz/src/msg_targets/mod.rs b/fuzz/src/msg_targets/mod.rs index 6f5f8120ca0..f9f56191b0a 100644 --- a/fuzz/src/msg_targets/mod.rs +++ b/fuzz/src/msg_targets/mod.rs @@ -3,6 +3,8 @@ mod utils; pub mod msg_accept_channel; pub mod msg_announcement_signatures; pub mod msg_closing_signed; +pub mod msg_closing_complete; +pub mod msg_closing_sig; pub mod msg_commitment_signed; pub mod msg_funding_created; pub mod msg_channel_ready; diff --git a/fuzz/src/msg_targets/msg_closing_complete.rs b/fuzz/src/msg_targets/msg_closing_complete.rs new file mode 100644 index 00000000000..922a3f3c67d --- /dev/null +++ b/fuzz/src/msg_targets/msg_closing_complete.rs @@ -0,0 +1,27 @@ +// This file is Copyright its original authors, visible in version control +// history. +// +// This file is licensed under the Apache License, Version 2.0 or the MIT license +// , at your option. +// You may not use this file except in accordance with one or both of these +// licenses. + +// This file is auto-generated by gen_target.sh based on msg_target_template.txt +// To modify it, modify msg_target_template.txt and run gen_target.sh instead. + +#![cfg_attr(rustfmt, rustfmt_skip)] + +use crate::msg_targets::utils::VecWriter; +use crate::utils::test_logger; + +#[inline] +pub fn msg_closing_complete_test(data: &[u8], _out: Out) { + test_msg_simple!(lightning::ln::msgs::ClosingComplete, data); +} + +#[no_mangle] +pub extern "C" fn msg_closing_complete_run(data: *const u8, datalen: usize) { + let data = unsafe { std::slice::from_raw_parts(data, datalen) }; + test_msg_simple!(lightning::ln::msgs::ClosingComplete, data); +} diff --git a/fuzz/src/msg_targets/msg_closing_sig.rs b/fuzz/src/msg_targets/msg_closing_sig.rs new file mode 100644 index 00000000000..d1854a784bf --- /dev/null +++ b/fuzz/src/msg_targets/msg_closing_sig.rs @@ -0,0 +1,27 @@ +// This file is Copyright its original authors, visible in version control +// history. +// +// This file is licensed under the Apache License, Version 2.0 or the MIT license +// , at your option. +// You may not use this file except in accordance with one or both of these +// licenses. + +// This file is auto-generated by gen_target.sh based on msg_target_template.txt +// To modify it, modify msg_target_template.txt and run gen_target.sh instead. + +#![cfg_attr(rustfmt, rustfmt_skip)] + +use crate::msg_targets::utils::VecWriter; +use crate::utils::test_logger; + +#[inline] +pub fn msg_closing_sig_test(data: &[u8], _out: Out) { + test_msg_simple!(lightning::ln::msgs::ClosingSig, data); +} + +#[no_mangle] +pub extern "C" fn msg_closing_sig_run(data: *const u8, datalen: usize) { + let data = unsafe { std::slice::from_raw_parts(data, datalen) }; + test_msg_simple!(lightning::ln::msgs::ClosingSig, data); +} diff --git a/fuzz/targets.h b/fuzz/targets.h index 3a3928b4f4d..25fa3d84458 100644 --- a/fuzz/targets.h +++ b/fuzz/targets.h @@ -23,6 +23,8 @@ void msg_accept_channel_run(const unsigned char* data, size_t data_len); void msg_announcement_signatures_run(const unsigned char* data, size_t data_len); void msg_channel_reestablish_run(const unsigned char* data, size_t data_len); void msg_closing_signed_run(const unsigned char* data, size_t data_len); +void msg_closing_complete_run(const unsigned char* data, size_t data_len); +void msg_closing_sig_run(const unsigned char* data, size_t data_len); void msg_commitment_signed_run(const unsigned char* data, size_t data_len); void msg_decoded_onion_error_packet_run(const unsigned char* data, size_t data_len); void msg_funding_created_run(const unsigned char* data, size_t data_len); From 22f3641e4e2f6c773ee4cc9cd4ddc9b1e98171a1 Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Tue, 1 Apr 2025 13:28:19 +0200 Subject: [PATCH 8/9] Allow to construct `ShutdownScript` from `OP_RETURN` script We add a new `ShutdownScript::new_op_return` constructor and extend the `is_bolt_compliant` check to enforce the new rules introduced by `option_simple_close`. --- lightning/src/ln/script.rs | 131 ++++++++++++++++++++++++++++++++++--- 1 file changed, 123 insertions(+), 8 deletions(-) diff --git a/lightning/src/ln/script.rs b/lightning/src/ln/script.rs index 96deae93876..07de4d0b182 100644 --- a/lightning/src/ln/script.rs +++ b/lightning/src/ln/script.rs @@ -1,8 +1,9 @@ //! Abstractions for scripts used in the Lightning Network. +use bitcoin::blockdata::script::Instruction; use bitcoin::hashes::Hash; -use bitcoin::opcodes::all::OP_PUSHBYTES_0 as SEGWIT_V0; -use bitcoin::script::{Script, ScriptBuf}; +use bitcoin::opcodes::all::{OP_PUSHBYTES_0 as SEGWIT_V0, OP_RETURN}; +use bitcoin::script::{PushBytes, Script, ScriptBuf}; use bitcoin::secp256k1::PublicKey; use bitcoin::{WPubkeyHash, WScriptHash, WitnessProgram}; @@ -75,6 +76,20 @@ impl ShutdownScript { Self(ShutdownScriptImpl::Bolt2(ScriptBuf::new_p2wsh(script_hash))) } + /// Generates an `OP_RETURN` script pubkey from the given `data` bytes. + /// + /// This is only needed and valid for channels supporting `option_simple_close`. Please refer + /// to [BOLT-2] for more information. + /// + /// # Errors + /// + /// This function may return an error if `data` is not [BOLT-2] compliant. + /// + /// [BOLT-2]: https://github.com/lightning/bolts/blob/master/02-peer-protocol.md#closing-negotiation-closing_complete-and-closing_sig + pub fn new_op_return>(data: T) -> Result { + Self::try_from(ScriptBuf::new_op_return(data)) + } + /// Generates a witness script pubkey from the given segwit version and program. /// /// Note for version-zero witness scripts you must use [`ShutdownScript::new_p2wpkh`] or @@ -104,7 +119,8 @@ impl ShutdownScript { /// Returns whether the shutdown script is compatible with the features as defined by BOLT #2. /// - /// Specifically, checks for compliance with feature `option_shutdown_anysegwit`. + /// Specifically, checks for compliance with feature `option_shutdown_anysegwit` and/or + /// `option_simple_close`. pub fn is_compatible(&self, features: &InitFeatures) -> bool { match &self.0 { ShutdownScriptImpl::Legacy(_) => true, @@ -116,10 +132,47 @@ impl ShutdownScript { /// Check if a given script is compliant with BOLT 2's shutdown script requirements for the given /// counterparty features. pub(crate) fn is_bolt2_compliant(script: &Script, features: &InitFeatures) -> bool { + // BOLT2: + // 1. `OP_0` `20` 20-bytes (version 0 pay to witness pubkey hash), OR + // 2. `OP_0` `32` 32-bytes (version 0 pay to witness script hash), OR if script.is_p2pkh() || script.is_p2sh() || script.is_p2wpkh() || script.is_p2wsh() { true - } else if features.supports_shutdown_anysegwit() { - script.is_witness_program() && script.as_bytes()[0] != SEGWIT_V0.to_u8() + } else if features.supports_shutdown_anysegwit() && script.is_witness_program() { + // 3. if (and only if) `option_shutdown_anysegwit` is negotiated: + // * `OP_1` through `OP_16` inclusive, followed by a single push of 2 to 40 bytes + // (witness program versions 1 through 16) + script.as_bytes()[0] != SEGWIT_V0.to_u8() + } else if features.supports_simple_close() && script.is_op_return() { + // 4. if (and only if) `option_simple_close` is negotiated: + let mut instruction_iter = script.instructions(); + if let Some(Ok(Instruction::Op(opcode))) = instruction_iter.next() { + // * `OP_RETURN` followed by one of: + if opcode != OP_RETURN { + return false; + } + + match instruction_iter.next() { + Some(Ok(Instruction::PushBytes(bytes))) => { + // * `6` to `75` inclusive followed by exactly that many bytes + if (6..=75).contains(&bytes.len()) { + return instruction_iter.next().is_none(); + } + + // `rust-bitcoin` interprets `OP_PUSHDATA1` as `Instruction::PushBytes`, having + // us land here in this case, too. + // + // * `76` followed by `76` to `80` followed by exactly that many bytes + if (76..=80).contains(&bytes.len()) { + return instruction_iter.next().is_none(); + } + + false + }, + _ => false, + } + } else { + false + } } else { false } @@ -142,7 +195,7 @@ impl TryFrom<(ScriptBuf, &InitFeatures)> for ShutdownScript { type Error = InvalidShutdownScript; fn try_from((script, features): (ScriptBuf, &InitFeatures)) -> Result { - if is_bolt2_compliant(&script, features) && script.is_witness_program() { + if is_bolt2_compliant(&script, features) { Ok(Self(ShutdownScriptImpl::Bolt2(script))) } else { Err(InvalidShutdownScript { script }) @@ -175,7 +228,7 @@ mod shutdown_script_tests { use super::ShutdownScript; use bitcoin::opcodes; - use bitcoin::script::{Builder, ScriptBuf}; + use bitcoin::script::{Builder, PushBytes, ScriptBuf}; use bitcoin::secp256k1::Secp256k1; use bitcoin::secp256k1::{PublicKey, SecretKey}; use bitcoin::{WitnessProgram, WitnessVersion}; @@ -210,6 +263,13 @@ mod shutdown_script_tests { features } + #[cfg(simple_close)] + fn simple_close_features() -> InitFeatures { + let mut features = InitFeatures::empty(); + features.set_simple_close_optional(); + features + } + #[test] fn generates_p2wpkh_from_pubkey() { let pubkey = pubkey(); @@ -246,6 +306,42 @@ mod shutdown_script_tests { assert!(ShutdownScript::try_from(p2wsh_script).is_ok()); } + #[cfg(simple_close)] + #[test] + fn generates_op_return_from_data() { + let data = [6; 6]; + let op_return_script = ScriptBuf::new_op_return(&data); + let shutdown_script = ShutdownScript::new_op_return(&data).unwrap(); + assert!(shutdown_script.is_compatible(&simple_close_features())); + assert!(!shutdown_script.is_compatible(&InitFeatures::empty())); + assert_eq!(shutdown_script.into_inner(), op_return_script); + assert!(ShutdownScript::try_from(op_return_script).is_ok()); + + let assert_pushdata_script_compat = |len| { + let mut pushdata_vec = Builder::new() + .push_opcode(opcodes::all::OP_RETURN) + .push_opcode(opcodes::all::OP_PUSHDATA1) + .into_bytes(); + pushdata_vec.push(len as u8); + pushdata_vec.extend_from_slice(&vec![1u8; len]); + let pushdata_script = ScriptBuf::from_bytes(pushdata_vec); + let pushdata_shutdown_script = ShutdownScript::try_from(pushdata_script).unwrap(); + assert!(pushdata_shutdown_script.is_compatible(&simple_close_features())); + assert!(!pushdata_shutdown_script.is_compatible(&InitFeatures::empty())); + }; + + // For `option_simple_close` we assert compatibility with `OP_PUSHDATA1` scripts for the + // intended length range of 76 to 80 bytes. + assert_pushdata_script_compat(76); + assert_pushdata_script_compat(80); + + // While the `option_simple_close` spec prescribes the use of `OP_PUSHBYTES_0` up to 75 + // bytes, we follow Postel's law and accept if our counterparty would create an + // `OP_PUSHDATA1` script for less than 76 bytes of payload. + assert_pushdata_script_compat(75); + assert_pushdata_script_compat(6); + } + #[test] fn generates_segwit_from_non_v0_witness_program() { let witness_program = WitnessProgram::new(WitnessVersion::V16, &[0; 40]).unwrap(); @@ -258,7 +354,26 @@ mod shutdown_script_tests { #[test] fn fails_from_unsupported_script() { - let op_return = ScriptBuf::new_op_return(&[0; 42]); + // For `option_simple_close` we assert we fail when: + // + // - The first byte of the OP_RETURN data (interpreted as u8 int) is not equal to the + // remaining number of bytes (i.e., `[5; 6]` would succeed here). + let op_return = ScriptBuf::new_op_return(&[5; 5]); assert!(ShutdownScript::try_from(op_return).is_err()); + + // - The OP_RETURN data will fail if it's longer than 80 bytes. + let mut pushdata_vec = Builder::new() + .push_opcode(opcodes::all::OP_RETURN) + .push_opcode(opcodes::all::OP_PUSHDATA1) + .into_bytes(); + pushdata_vec.push(81); + pushdata_vec.extend_from_slice(&[1u8; 81]); + let pushdata_script = ScriptBuf::from_bytes(pushdata_vec); + assert!(ShutdownScript::try_from(pushdata_script).is_err()); + + // - In `ShutdownScript::new_op_return` the OP_RETURN data is longer than 80 bytes. + let big_buffer = &[1u8; 81][..]; + let push_bytes: &PushBytes = big_buffer.try_into().unwrap(); + assert!(ShutdownScript::new_op_return(&push_bytes).is_err()); } } From 0293a3c80d3c34cb6dbc2853441c8404d7133314 Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Wed, 18 Jun 2025 09:37:08 +0200 Subject: [PATCH 9/9] Refactor: Switch `Into` to `From` .. as Rust advises to only ever implement `From`, and let the compiler derive `Into` automatically. --- lightning/src/ln/script.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lightning/src/ln/script.rs b/lightning/src/ln/script.rs index 07de4d0b182..6bbbf3bf86e 100644 --- a/lightning/src/ln/script.rs +++ b/lightning/src/ln/script.rs @@ -203,9 +203,9 @@ impl TryFrom<(ScriptBuf, &InitFeatures)> for ShutdownScript { } } -impl Into for ShutdownScript { - fn into(self) -> ScriptBuf { - match self.0 { +impl From for ScriptBuf { + fn from(value: ShutdownScript) -> Self { + match value.0 { ShutdownScriptImpl::Legacy(pubkey) => { ScriptBuf::new_p2wpkh(&WPubkeyHash::hash(&pubkey.serialize())) },