From ef6738dcc4b91f35c38ef2265cd6c570d34deca7 Mon Sep 17 00:00:00 2001 From: Chris Waterson Date: Sun, 10 Sep 2023 14:00:02 -0700 Subject: [PATCH] Update for channel phase changes Moves the following methods (which, once unwound, don't actually depend on anything but the ChannelContext) into ChannelContext: - Channel::build_commitment_no_state_update - Channel::send_commitment_no_state_update - Channel::get_last_commitment_update_for_send - Channel::signer_maybe_unblocked Modifies `signer_unblocked` to use the phase rather than the channel. --- lightning/src/ln/channel.rs | 360 +++++++++++----------- lightning/src/ln/channelmanager.rs | 11 +- lightning/src/ln/functional_test_utils.rs | 12 +- 3 files changed, 189 insertions(+), 194 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 70dba6d35f5..e14153292b7 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -2085,6 +2085,178 @@ impl ChannelContext where SP::Target: SignerProvider { } } } + + pub fn build_commitment_no_state_update(&self, logger: &L) + -> (Vec<(HTLCOutputInCommitment, Option<&HTLCSource>)>, CommitmentTransaction) + where L::Target: Logger + { + let counterparty_keys = self.build_remote_transaction_keys(); + let commitment_stats = self.build_commitment_transaction(self.cur_counterparty_commitment_transaction_number, &counterparty_keys, false, true, logger); + let counterparty_commitment_tx = commitment_stats.tx; + + #[cfg(any(test, fuzzing))] + { + if !self.is_outbound() { + let projected_commit_tx_info = self.next_remote_commitment_tx_fee_info_cached.lock().unwrap().take(); + *self.next_local_commitment_tx_fee_info_cached.lock().unwrap() = None; + if let Some(info) = projected_commit_tx_info { + let total_pending_htlcs = self.pending_inbound_htlcs.len() + self.pending_outbound_htlcs.len(); + if info.total_pending_htlcs == total_pending_htlcs + && info.next_holder_htlc_id == self.next_holder_htlc_id + && info.next_counterparty_htlc_id == self.next_counterparty_htlc_id + && info.feerate == self.feerate_per_kw { + let actual_fee = commit_tx_fee_msat(self.feerate_per_kw, commitment_stats.num_nondust_htlcs, self.get_channel_type()); + assert_eq!(actual_fee, info.fee); + } + } + } + } + + (commitment_stats.htlcs_included, counterparty_commitment_tx) + } + + /// Only fails in case of signer rejection. Used for channel_reestablish commitment_signed + /// generation when we shouldn't change HTLC/channel state. + pub fn send_commitment_no_state_update(&self, logger: &L) -> Result<(msgs::CommitmentSigned, (Txid, Vec<(HTLCOutputInCommitment, Option<&HTLCSource>)>)), ChannelError> where L::Target: Logger { + // Get the fee tests from `build_commitment_no_state_update` + #[cfg(any(test, fuzzing))] + self.build_commitment_no_state_update(logger); + + let counterparty_keys = self.build_remote_transaction_keys(); + let commitment_stats = self.build_commitment_transaction(self.cur_counterparty_commitment_transaction_number, &counterparty_keys, false, true, logger); + let counterparty_commitment_txid = commitment_stats.tx.trust().txid(); + + match &self.holder_signer { + ChannelSignerType::Ecdsa(ecdsa) => { + let (signature, htlc_signatures); + + { + let mut htlcs = Vec::with_capacity(commitment_stats.htlcs_included.len()); + for &(ref htlc, _) in commitment_stats.htlcs_included.iter() { + htlcs.push(htlc); + } + + let res = ecdsa.sign_counterparty_commitment(&commitment_stats.tx, commitment_stats.preimages, &self.secp_ctx) + .map_err(|_| ChannelError::Ignore("Failed to get signatures for new commitment_signed".to_owned()))?; + signature = res.0; + htlc_signatures = res.1; + + log_trace!(logger, "Signed remote commitment tx {} (txid {}) with redeemscript {} -> {} in channel {}", + encode::serialize_hex(&commitment_stats.tx.trust().built_transaction().transaction), + &counterparty_commitment_txid, encode::serialize_hex(&self.get_funding_redeemscript()), + log_bytes!(signature.serialize_compact()[..]), &self.channel_id()); + + for (ref htlc_sig, ref htlc) in htlc_signatures.iter().zip(htlcs) { + log_trace!(logger, "Signed remote HTLC tx {} with redeemscript {} with pubkey {} -> {} in channel {}", + encode::serialize_hex(&chan_utils::build_htlc_transaction(&counterparty_commitment_txid, commitment_stats.feerate_per_kw, self.get_holder_selected_contest_delay(), htlc, &self.channel_type, &counterparty_keys.broadcaster_delayed_payment_key, &counterparty_keys.revocation_key)), + encode::serialize_hex(&chan_utils::get_htlc_redeemscript(&htlc, &self.channel_type, &counterparty_keys)), + log_bytes!(counterparty_keys.broadcaster_htlc_key.serialize()), + log_bytes!(htlc_sig.serialize_compact()[..]), &self.channel_id()); + } + } + + Ok((msgs::CommitmentSigned { + channel_id: self.channel_id, + signature, + htlc_signatures, + #[cfg(taproot)] + partial_signature_with_nonce: None, + }, (counterparty_commitment_txid, commitment_stats.htlcs_included))) + } + } + } + + /// Gets the last commitment update for immediate sending to our peer. + pub fn get_last_commitment_update_for_send(&mut self, logger: &L) -> Result where L::Target: Logger { + let mut update_add_htlcs = Vec::new(); + let mut update_fulfill_htlcs = Vec::new(); + let mut update_fail_htlcs = Vec::new(); + let mut update_fail_malformed_htlcs = Vec::new(); + + for htlc in self.pending_outbound_htlcs.iter() { + if let &OutboundHTLCState::LocalAnnounced(ref onion_packet) = &htlc.state { + update_add_htlcs.push(msgs::UpdateAddHTLC { + channel_id: self.channel_id(), + htlc_id: htlc.htlc_id, + amount_msat: htlc.amount_msat, + payment_hash: htlc.payment_hash, + cltv_expiry: htlc.cltv_expiry, + onion_routing_packet: (**onion_packet).clone(), + skimmed_fee_msat: htlc.skimmed_fee_msat, + }); + } + } + + for htlc in self.pending_inbound_htlcs.iter() { + if let &InboundHTLCState::LocalRemoved(ref reason) = &htlc.state { + match reason { + &InboundHTLCRemovalReason::FailRelay(ref err_packet) => { + update_fail_htlcs.push(msgs::UpdateFailHTLC { + channel_id: self.channel_id(), + htlc_id: htlc.htlc_id, + reason: err_packet.clone() + }); + }, + &InboundHTLCRemovalReason::FailMalformed((ref sha256_of_onion, ref failure_code)) => { + update_fail_malformed_htlcs.push(msgs::UpdateFailMalformedHTLC { + channel_id: self.channel_id(), + htlc_id: htlc.htlc_id, + sha256_of_onion: sha256_of_onion.clone(), + failure_code: failure_code.clone(), + }); + }, + &InboundHTLCRemovalReason::Fulfill(ref payment_preimage) => { + update_fulfill_htlcs.push(msgs::UpdateFulfillHTLC { + channel_id: self.channel_id(), + htlc_id: htlc.htlc_id, + payment_preimage: payment_preimage.clone(), + }); + }, + } + } + } + + let update_fee = if self.is_outbound() && self.pending_update_fee.is_some() { + Some(msgs::UpdateFee { + channel_id: self.channel_id(), + feerate_per_kw: self.pending_update_fee.unwrap().0, + }) + } else { None }; + + log_trace!(logger, "Regenerated latest commitment update in channel {} with{} {} update_adds, {} update_fulfills, {} update_fails, and {} update_fail_malformeds", + &self.channel_id(), if update_fee.is_some() { " update_fee," } else { "" }, + update_add_htlcs.len(), update_fulfill_htlcs.len(), update_fail_htlcs.len(), update_fail_malformed_htlcs.len()); + let commitment_signed = if let Ok(update) = self.send_commitment_no_state_update(logger).map(|(cu, _)| cu) { + self.signer_pending_commitment_update = false; + update + } else { + self.signer_pending_commitment_update = true; + return Err(()); + }; + Ok(msgs::CommitmentUpdate { + update_add_htlcs, update_fulfill_htlcs, update_fail_htlcs, update_fail_malformed_htlcs, update_fee, + commitment_signed, + }) + } + + /// Indicates that the signer may have some signatures for us, so we should retry if we're + /// blocked. + pub fn signer_maybe_unblocked(&mut self, logger: &L) -> SignerResumeUpdates where L::Target: Logger { + let commitment_update = if self.signer_pending_commitment_update { + self.get_last_commitment_update_for_send(logger).ok() + } else { None }; + let funding_signed = if self.signer_pending_funding && !self.is_outbound() { + self.get_funding_signed_msg(logger).1 + } else { None }; + let funding_created = if self.signer_pending_funding && self.is_outbound() { + self.get_funding_created_msg(logger) + } else { None }; + SignerResumeUpdates { + commitment_update, + funding_signed, + funding_created, + } + } } // Internal utility functions for channels @@ -3598,8 +3770,8 @@ impl Channel where /// If our balance is too low to cover the cost of the next commitment transaction at the /// new feerate, the update is cancelled. /// - /// You MUST call [`Self::send_commitment_no_state_update`] prior to any other calls on this - /// [`Channel`] if `force_holding_cell` is false. + /// You MUST call [`ChannelContext::send_commitment_no_state_update`] prior to any other calls on + /// this [`Channel`] if `force_holding_cell` is false. fn send_update_fee( &mut self, feerate_per_kw: u32, mut force_holding_cell: bool, fee_estimator: &LowerBoundedFeeEstimator, logger: &L @@ -3830,7 +4002,7 @@ impl Channel where Some(self.get_last_revoke_and_ack()) } else { None }; let commitment_update = if self.context.monitor_pending_commitment_signed { - self.get_last_commitment_update_for_send(logger).ok() + self.context.get_last_commitment_update_for_send(logger).ok() } else { None }; if commitment_update.is_some() { self.mark_awaiting_response(); @@ -3883,25 +4055,6 @@ impl Channel where Ok(()) } - /// Indicates that the signer may have some signatures for us, so we should retry if we're - /// blocked. - pub fn signer_maybe_unblocked(&mut self, logger: &L) -> SignerResumeUpdates where L::Target: Logger { - let commitment_update = if self.context.signer_pending_commitment_update { - self.get_last_commitment_update_for_send(logger).ok() - } else { None }; - let funding_signed = if self.context.signer_pending_funding && !self.context.is_outbound() { - self.context.get_funding_signed_msg(logger).1 - } else { None }; - let funding_created = if self.context.signer_pending_funding && self.context.is_outbound() { - self.context.get_funding_created_msg(logger) - } else { None }; - SignerResumeUpdates { - commitment_update, - funding_signed, - funding_created, - } - } - fn get_last_revoke_and_ack(&self) -> msgs::RevokeAndACK { let next_per_commitment_point = self.context.holder_signer.as_ref().get_per_commitment_point(self.context.cur_holder_commitment_transaction_number, &self.context.secp_ctx); let per_commitment_secret = self.context.holder_signer.as_ref().release_commitment_secret(self.context.cur_holder_commitment_transaction_number + 2); @@ -3914,79 +4067,6 @@ impl Channel where } } - /// Gets the last commitment update for immediate sending to our peer. - fn get_last_commitment_update_for_send(&mut self, logger: &L) -> Result where L::Target: Logger { - let mut update_add_htlcs = Vec::new(); - let mut update_fulfill_htlcs = Vec::new(); - let mut update_fail_htlcs = Vec::new(); - let mut update_fail_malformed_htlcs = Vec::new(); - - for htlc in self.context.pending_outbound_htlcs.iter() { - if let &OutboundHTLCState::LocalAnnounced(ref onion_packet) = &htlc.state { - update_add_htlcs.push(msgs::UpdateAddHTLC { - channel_id: self.context.channel_id(), - htlc_id: htlc.htlc_id, - amount_msat: htlc.amount_msat, - payment_hash: htlc.payment_hash, - cltv_expiry: htlc.cltv_expiry, - onion_routing_packet: (**onion_packet).clone(), - skimmed_fee_msat: htlc.skimmed_fee_msat, - }); - } - } - - for htlc in self.context.pending_inbound_htlcs.iter() { - if let &InboundHTLCState::LocalRemoved(ref reason) = &htlc.state { - match reason { - &InboundHTLCRemovalReason::FailRelay(ref err_packet) => { - update_fail_htlcs.push(msgs::UpdateFailHTLC { - channel_id: self.context.channel_id(), - htlc_id: htlc.htlc_id, - reason: err_packet.clone() - }); - }, - &InboundHTLCRemovalReason::FailMalformed((ref sha256_of_onion, ref failure_code)) => { - update_fail_malformed_htlcs.push(msgs::UpdateFailMalformedHTLC { - channel_id: self.context.channel_id(), - htlc_id: htlc.htlc_id, - sha256_of_onion: sha256_of_onion.clone(), - failure_code: failure_code.clone(), - }); - }, - &InboundHTLCRemovalReason::Fulfill(ref payment_preimage) => { - update_fulfill_htlcs.push(msgs::UpdateFulfillHTLC { - channel_id: self.context.channel_id(), - htlc_id: htlc.htlc_id, - payment_preimage: payment_preimage.clone(), - }); - }, - } - } - } - - let update_fee = if self.context.is_outbound() && self.context.pending_update_fee.is_some() { - Some(msgs::UpdateFee { - channel_id: self.context.channel_id(), - feerate_per_kw: self.context.pending_update_fee.unwrap().0, - }) - } else { None }; - - log_trace!(logger, "Regenerated latest commitment update in channel {} with{} {} update_adds, {} update_fulfills, {} update_fails, and {} update_fail_malformeds", - &self.context.channel_id(), if update_fee.is_some() { " update_fee," } else { "" }, - update_add_htlcs.len(), update_fulfill_htlcs.len(), update_fail_htlcs.len(), update_fail_malformed_htlcs.len()); - let commitment_signed = if let Ok(update) = self.send_commitment_no_state_update(logger).map(|(cu, _)| cu) { - self.context.signer_pending_commitment_update = false; - update - } else { - self.context.signer_pending_commitment_update = true; - return Err(()); - }; - Ok(msgs::CommitmentUpdate { - update_add_htlcs, update_fulfill_htlcs, update_fail_htlcs, update_fail_malformed_htlcs, update_fee, - commitment_signed, - }) - } - /// Gets the `Shutdown` message we should send our peer on reconnect, if any. pub fn get_outbound_shutdown(&self) -> Option { if self.context.channel_state & (ChannelState::LocalShutdownSent as u32) != 0 { @@ -4164,7 +4244,7 @@ impl Channel where Ok(ReestablishResponses { channel_ready, shutdown_msg, announcement_sigs, raa: required_revoke, - commitment_update: self.get_last_commitment_update_for_send(logger).ok(), + commitment_update: self.context.get_last_commitment_update_for_send(logger).ok(), order: self.context.resend_order.clone(), }) } @@ -5319,8 +5399,8 @@ impl Channel where /// we may not yet have sent the previous commitment update messages and will need to /// regenerate them. /// - /// You MUST call [`Self::send_commitment_no_state_update`] prior to calling any other methods - /// on this [`Channel`] if `force_holding_cell` is false. + /// You MUST call [`ChannelContext::send_commitment_no_state_update`] prior to calling any other + /// methods on this [`Channel`] if `force_holding_cell` is false. /// /// `Err`s will only be [`ChannelError::Ignore`]. fn send_htlc( @@ -5445,7 +5525,7 @@ impl Channel where self.context.resend_order = RAACommitmentOrder::RevokeAndACKFirst; let (mut htlcs_ref, counterparty_commitment_tx) = - self.build_commitment_no_state_update(logger); + self.context.build_commitment_no_state_update(logger); let counterparty_commitment_txid = counterparty_commitment_tx.trust().txid(); let htlcs: Vec<(HTLCOutputInCommitment, Option>)> = htlcs_ref.drain(..).map(|(htlc, htlc_source)| (htlc, htlc_source.map(|source_ref| Box::new(source_ref.clone())))).collect(); @@ -5471,91 +5551,11 @@ impl Channel where monitor_update } - fn build_commitment_no_state_update(&self, logger: &L) - -> (Vec<(HTLCOutputInCommitment, Option<&HTLCSource>)>, CommitmentTransaction) - where L::Target: Logger - { - let counterparty_keys = self.context.build_remote_transaction_keys(); - let commitment_stats = self.context.build_commitment_transaction(self.context.cur_counterparty_commitment_transaction_number, &counterparty_keys, false, true, logger); - let counterparty_commitment_tx = commitment_stats.tx; - - #[cfg(any(test, fuzzing))] - { - if !self.context.is_outbound() { - let projected_commit_tx_info = self.context.next_remote_commitment_tx_fee_info_cached.lock().unwrap().take(); - *self.context.next_local_commitment_tx_fee_info_cached.lock().unwrap() = None; - if let Some(info) = projected_commit_tx_info { - let total_pending_htlcs = self.context.pending_inbound_htlcs.len() + self.context.pending_outbound_htlcs.len(); - if info.total_pending_htlcs == total_pending_htlcs - && info.next_holder_htlc_id == self.context.next_holder_htlc_id - && info.next_counterparty_htlc_id == self.context.next_counterparty_htlc_id - && info.feerate == self.context.feerate_per_kw { - let actual_fee = commit_tx_fee_msat(self.context.feerate_per_kw, commitment_stats.num_nondust_htlcs, self.context.get_channel_type()); - assert_eq!(actual_fee, info.fee); - } - } - } - } - - (commitment_stats.htlcs_included, counterparty_commitment_tx) - } - - /// Only fails in case of signer rejection. Used for channel_reestablish commitment_signed - /// generation when we shouldn't change HTLC/channel state. - fn send_commitment_no_state_update(&self, logger: &L) -> Result<(msgs::CommitmentSigned, (Txid, Vec<(HTLCOutputInCommitment, Option<&HTLCSource>)>)), ChannelError> where L::Target: Logger { - // Get the fee tests from `build_commitment_no_state_update` - #[cfg(any(test, fuzzing))] - self.build_commitment_no_state_update(logger); - - let counterparty_keys = self.context.build_remote_transaction_keys(); - let commitment_stats = self.context.build_commitment_transaction(self.context.cur_counterparty_commitment_transaction_number, &counterparty_keys, false, true, logger); - let counterparty_commitment_txid = commitment_stats.tx.trust().txid(); - - match &self.context.holder_signer { - ChannelSignerType::Ecdsa(ecdsa) => { - let (signature, htlc_signatures); - - { - let mut htlcs = Vec::with_capacity(commitment_stats.htlcs_included.len()); - for &(ref htlc, _) in commitment_stats.htlcs_included.iter() { - htlcs.push(htlc); - } - - let res = ecdsa.sign_counterparty_commitment(&commitment_stats.tx, commitment_stats.preimages, &self.context.secp_ctx) - .map_err(|_| ChannelError::Ignore("Failed to get signatures for new commitment_signed".to_owned()))?; - signature = res.0; - htlc_signatures = res.1; - - log_trace!(logger, "Signed remote commitment tx {} (txid {}) with redeemscript {} -> {} in channel {}", - encode::serialize_hex(&commitment_stats.tx.trust().built_transaction().transaction), - &counterparty_commitment_txid, encode::serialize_hex(&self.context.get_funding_redeemscript()), - log_bytes!(signature.serialize_compact()[..]), &self.context.channel_id()); - - for (ref htlc_sig, ref htlc) in htlc_signatures.iter().zip(htlcs) { - log_trace!(logger, "Signed remote HTLC tx {} with redeemscript {} with pubkey {} -> {} in channel {}", - encode::serialize_hex(&chan_utils::build_htlc_transaction(&counterparty_commitment_txid, commitment_stats.feerate_per_kw, self.context.get_holder_selected_contest_delay(), htlc, &self.context.channel_type, &counterparty_keys.broadcaster_delayed_payment_key, &counterparty_keys.revocation_key)), - encode::serialize_hex(&chan_utils::get_htlc_redeemscript(&htlc, &self.context.channel_type, &counterparty_keys)), - log_bytes!(counterparty_keys.broadcaster_htlc_key.serialize()), - log_bytes!(htlc_sig.serialize_compact()[..]), &self.context.channel_id()); - } - } - - Ok((msgs::CommitmentSigned { - channel_id: self.context.channel_id, - signature, - htlc_signatures, - #[cfg(taproot)] - partial_signature_with_nonce: None, - }, (counterparty_commitment_txid, commitment_stats.htlcs_included))) - } - } - } - /// Adds a pending outbound HTLC to this channel, and builds a new remote commitment /// transaction and generates the corresponding [`ChannelMonitorUpdate`] in one go. /// /// Shorthand for calling [`Self::send_htlc`] followed by a commitment update, see docs on - /// [`Self::send_htlc`] and [`Self::build_commitment_no_state_update`] for more info. + /// [`Self::send_htlc`] and [`ChannelContext::build_commitment_no_state_update`] for more info. pub fn send_htlc_and_commit( &mut self, amount_msat: u64, payment_hash: PaymentHash, cltv_expiry: u32, source: HTLCSource, onion_routing_packet: msgs::OnionPacket, skimmed_fee_msat: Option, diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index f039a073ef5..140adf0e756 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -6615,23 +6615,24 @@ where pub fn signer_unblocked(&self, channel_opt: Option<(PublicKey, ChannelId)>) { let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self); - let unblock_chan = |chan: &mut Channel, pending_msg_events: &mut Vec| { - let msgs = chan.signer_maybe_unblocked(&self.logger); + let unblock_chan = |phase: &mut ChannelPhase, pending_msg_events: &mut Vec| { + let msgs = phase.context_mut().signer_maybe_unblocked(&self.logger); + let node_id = phase.context().get_counterparty_node_id(); if let Some(updates) = msgs.commitment_update { pending_msg_events.push(events::MessageSendEvent::UpdateHTLCs { - node_id: chan.context.get_counterparty_node_id(), + node_id, updates, }); } if let Some(msg) = msgs.funding_signed { pending_msg_events.push(events::MessageSendEvent::SendFundingSigned { - node_id: chan.context.get_counterparty_node_id(), + node_id, msg, }); } if let Some(msg) = msgs.funding_created { pending_msg_events.push(events::MessageSendEvent::SendFundingCreated { - node_id: chan.context.get_counterparty_node_id(), + node_id, msg, }); } diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index e57bf400590..896d1dab83d 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -427,16 +427,10 @@ impl<'a, 'b, 'c> Node<'a, 'b, 'c> { let per_peer_state = self.node.per_peer_state.read().unwrap(); let chan_lock = per_peer_state.get(peer_id).unwrap().lock().unwrap(); let signer = (|| { - if let Some(local_chan) = chan_lock.channel_by_id.get(chan_id) { - return local_chan.get_signer(); + match chan_lock.channel_by_id.get(chan_id) { + Some(phase) => phase.context().get_signer(), + None => panic!("Couldn't find a channel with id {}", chan_id), } - if let Some(local_chan) = chan_lock.inbound_v1_channel_by_id.get(chan_id) { - return local_chan.context.get_signer(); - } - if let Some(local_chan) = chan_lock.outbound_v1_channel_by_id.get(chan_id) { - return local_chan.context.get_signer(); - } - panic!("Couldn't find a channel with id {}", chan_id); })(); log_debug!(self.logger, "Setting channel signer for {} as available={}", chan_id, available); signer.as_ecdsa().unwrap().set_available(available);