Handle sign_counterparty_commitment failing during outb funding

If sign_counterparty_commitment fails (i.e. because the signer is
temporarily disconnected), this really indicates that we should
retry the message sending which required the signature later,
rather than force-closing the channel (which probably won't even
work if the signer is missing).

Here we add initial handling of sign_counterparty_commitment
failing during outbound channel funding, setting a new flag in
`ChannelContext` which indicates we should retry sending the
`funding_created` later. We don't yet add any ability to do that
retry.

diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
index b0c318a91e48bc5e60fad2e8f5a2a8318333640d..f48145f9f5c818a5556238deba2e670de1e6354c 100644 (file)
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -707,6 +707,10 @@ pub(super) struct ChannelContext<SP: Deref> where SP::Target: SignerProvider {
        /// This flag is set in such a case. Note that we don't need to persist this as we'll end up
        /// setting it again as a side-effect of [`Channel::channel_reestablish`].
        signer_pending_commitment_update: bool,
+       /// Similar to [`Self::signer_pending_commitment_update`] but we're waiting to send either a
+       /// [`msgs::FundingCreated`] or [`msgs::FundingSigned`] depending on if this channel is
+       /// outbound or inbound.
+       signer_pending_funding: bool,
 
        // pending_update_fee is filled when sending and receiving update_fee.
        //
@@ -5722,6 +5726,7 @@ impl<SP: Deref> OutboundV1Channel<SP> where SP::Target: SignerProvider {
                                monitor_pending_finalized_fulfills: Vec::new(),
 
                                signer_pending_commitment_update: false,
+                               signer_pending_funding: false,
 
                                #[cfg(debug_assertions)]
                                holder_max_commitment_tx_output: Mutex::new((channel_value_satoshis * 1000 - push_msat, push_msat)),
@@ -5802,15 +5807,14 @@ impl<SP: Deref> OutboundV1Channel<SP> where SP::Target: SignerProvider {
                })
        }
 
-       /// If an Err is returned, it is a ChannelError::Close (for get_funding_created)
-       fn get_funding_created_signature<L: Deref>(&mut self, logger: &L) -> Result<Signature, ChannelError> where L::Target: Logger {
+       fn get_funding_created_signature<L: Deref>(&mut self, logger: &L) -> Result<Signature, ()> where L::Target: Logger {
                let counterparty_keys = self.context.build_remote_transaction_keys();
                let counterparty_initial_commitment_tx = self.context.build_commitment_transaction(self.context.cur_counterparty_commitment_transaction_number, &counterparty_keys, false, false, logger).tx;
                match &self.context.holder_signer {
                        // TODO (taproot|arik): move match into calling method for Taproot
                        ChannelSignerType::Ecdsa(ecdsa) => {
-                               Ok(ecdsa.sign_counterparty_commitment(&counterparty_initial_commitment_tx, Vec::new(), &self.context.secp_ctx)
-                                       .map_err(|_| ChannelError::Close("Failed to get signatures for new commitment_signed".to_owned()))?.0)
+                               ecdsa.sign_counterparty_commitment(&counterparty_initial_commitment_tx, Vec::new(), &self.context.secp_ctx)
+                                       .map(|(sig, _)| sig)
                        }
                }
        }
@@ -5823,7 +5827,7 @@ impl<SP: Deref> OutboundV1Channel<SP> where SP::Target: SignerProvider {
        /// Do NOT broadcast the funding transaction until after a successful funding_signed call!
        /// If an Err is returned, it is a ChannelError::Close.
        pub fn get_funding_created<L: Deref>(mut self, funding_transaction: Transaction, funding_txo: OutPoint, logger: &L)
-       -> Result<(Channel<SP>, msgs::FundingCreated), (Self, ChannelError)> where L::Target: Logger {
+       -> Result<(Channel<SP>, Option<msgs::FundingCreated>), (Self, ChannelError)> where L::Target: Logger {
                if !self.context.is_outbound() {
                        panic!("Tried to create outbound funding_created message on an inbound channel!");
                }
@@ -5839,15 +5843,6 @@ impl<SP: Deref> OutboundV1Channel<SP> where SP::Target: SignerProvider {
                self.context.channel_transaction_parameters.funding_outpoint = Some(funding_txo);
                self.context.holder_signer.as_mut().provide_channel_parameters(&self.context.channel_transaction_parameters);
 
-               let signature = match self.get_funding_created_signature(logger) {
-                       Ok(res) => res,
-                       Err(e) => {
-                               log_error!(logger, "Got bad signatures: {:?}!", e);
-                               self.context.channel_transaction_parameters.funding_outpoint = None;
-                               return Err((self, e));
-                       }
-               };
-
                let temporary_channel_id = self.context.channel_id;
 
                // Now that we're past error-generating stuff, update our local state:
@@ -5865,20 +5860,27 @@ impl<SP: Deref> OutboundV1Channel<SP> where SP::Target: SignerProvider {
 
                self.context.funding_transaction = Some(funding_transaction);
 
+               let funding_created = if let Ok(signature) = self.get_funding_created_signature(logger) {
+                       Some(msgs::FundingCreated {
+                               temporary_channel_id,
+                               funding_txid: funding_txo.txid,
+                               funding_output_index: funding_txo.index,
+                               signature,
+                               #[cfg(taproot)]
+                               partial_signature_with_nonce: None,
+                               #[cfg(taproot)]
+                               next_local_nonce: None,
+                       })
+               } else {
+                       self.context.signer_pending_funding = true;
+                       None
+               };
+
                let channel = Channel {
                        context: self.context,
                };
 
-               Ok((channel, msgs::FundingCreated {
-                       temporary_channel_id,
-                       funding_txid: funding_txo.txid,
-                       funding_output_index: funding_txo.index,
-                       signature,
-                       #[cfg(taproot)]
-                       partial_signature_with_nonce: None,
-                       #[cfg(taproot)]
-                       next_local_nonce: None,
-               }))
+               Ok((channel, funding_created))
        }
 
        fn get_initial_channel_type(config: &UserConfig, their_features: &InitFeatures) -> ChannelTypeFeatures {
@@ -6371,6 +6373,7 @@ impl<SP: Deref> InboundV1Channel<SP> where SP::Target: SignerProvider {
                                monitor_pending_finalized_fulfills: Vec::new(),
 
                                signer_pending_commitment_update: false,
+                               signer_pending_funding: false,
 
                                #[cfg(debug_assertions)]
                                holder_max_commitment_tx_output: Mutex::new((msg.push_msat, msg.funding_satoshis * 1000 - msg.push_msat)),
@@ -7459,6 +7462,7 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32, &'c Ch
                                monitor_pending_finalized_fulfills: monitor_pending_finalized_fulfills.unwrap(),
 
                                signer_pending_commitment_update: false,
+                               signer_pending_funding: false,
 
                                pending_update_fee,
                                holding_cell_update_fee,
@@ -7730,7 +7734,7 @@ mod tests {
                }]};
                let funding_outpoint = OutPoint{ txid: tx.txid(), index: 0 };
                let (mut node_a_chan, funding_created_msg) = node_a_chan.get_funding_created(tx.clone(), funding_outpoint, &&logger).map_err(|_| ()).unwrap();
-               let (_, funding_signed_msg, _) = node_b_chan.funding_created(&funding_created_msg, best_block, &&keys_provider, &&logger).map_err(|_| ()).unwrap();
+               let (_, funding_signed_msg, _) = node_b_chan.funding_created(&funding_created_msg.unwrap(), best_block, &&keys_provider, &&logger).map_err(|_| ()).unwrap();
 
                // Node B --> Node A: funding signed
                let _ = node_a_chan.funding_signed(&funding_signed_msg, best_block, &&keys_provider, &&logger).unwrap();
@@ -7857,7 +7861,7 @@ mod tests {
                }]};
                let funding_outpoint = OutPoint{ txid: tx.txid(), index: 0 };
                let (mut node_a_chan, funding_created_msg) = node_a_chan.get_funding_created(tx.clone(), funding_outpoint, &&logger).map_err(|_| ()).unwrap();
-               let (mut node_b_chan, funding_signed_msg, _) = node_b_chan.funding_created(&funding_created_msg, best_block, &&keys_provider, &&logger).map_err(|_| ()).unwrap();
+               let (mut node_b_chan, funding_signed_msg, _) = node_b_chan.funding_created(&funding_created_msg.unwrap(), best_block, &&keys_provider, &&logger).map_err(|_| ()).unwrap();
 
                // Node B --> Node A: funding signed
                let _ = node_a_chan.funding_signed(&funding_signed_msg, best_block, &&keys_provider, &&logger).unwrap();
@@ -8045,7 +8049,7 @@ mod tests {
                }]};
                let funding_outpoint = OutPoint{ txid: tx.txid(), index: 0 };
                let (mut node_a_chan, funding_created_msg) = node_a_chan.get_funding_created(tx.clone(), funding_outpoint, &&logger).map_err(|_| ()).unwrap();
-               let (_, funding_signed_msg, _) = node_b_chan.funding_created(&funding_created_msg, best_block, &&keys_provider, &&logger).map_err(|_| ()).unwrap();
+               let (_, funding_signed_msg, _) = node_b_chan.funding_created(&funding_created_msg.unwrap(), best_block, &&keys_provider, &&logger).map_err(|_| ()).unwrap();
 
                // Node B --> Node A: funding signed
                let _ = node_a_chan.funding_signed(&funding_signed_msg, best_block, &&keys_provider, &&logger).unwrap();

diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 017625954cca3c6a77673d7ec2eeb627ba6322e5..d84bca1795015d9e7715c8eeae0ab7ea968f2452 100644 (file)
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -3468,7 +3468,7 @@ where
 
                let mut peer_state_lock = peer_state_mutex.lock().unwrap();
                let peer_state = &mut *peer_state_lock;
-               let (chan, msg) = match peer_state.outbound_v1_channel_by_id.remove(&temporary_channel_id) {
+               let (chan, msg_opt) = match peer_state.outbound_v1_channel_by_id.remove(&temporary_channel_id) {
                        Some(chan) => {
                                let funding_txo = find_funding_output(&chan, &funding_transaction)?;
 
@@ -3502,10 +3502,12 @@ where
                        },
                };
 
-               peer_state.pending_msg_events.push(events::MessageSendEvent::SendFundingCreated {
-                       node_id: chan.context.get_counterparty_node_id(),
-                       msg,
-               });
+               if let Some(msg) = msg_opt {
+                       peer_state.pending_msg_events.push(events::MessageSendEvent::SendFundingCreated {
+                               node_id: chan.context.get_counterparty_node_id(),
+                               msg,
+                       });
+               }
                match peer_state.channel_by_id.entry(chan.context.channel_id()) {
                        hash_map::Entry::Occupied(_) => {
                                panic!("Generated duplicate funding txid?");

diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs
index a5a55bc2da34dcc7571369735c6227f98e2679bb..d88ee41b6c5e582f1afebe0e8cd0e90c416548ab 100644 (file)
--- a/lightning/src/ln/functional_tests.rs
+++ b/lightning/src/ln/functional_tests.rs
@@ -8936,7 +8936,7 @@ fn test_duplicate_chan_id() {
                as_chan.get_funding_created(tx.clone(), funding_outpoint, &&logger).map_err(|_| ()).unwrap()
        };
        check_added_monitors!(nodes[0], 0);
-       nodes[1].node.handle_funding_created(&nodes[0].node.get_our_node_id(), &funding_created);
+       nodes[1].node.handle_funding_created(&nodes[0].node.get_our_node_id(), &funding_created.unwrap());
        // At this point we'll look up if the channel_id is present and immediately fail the channel
        // without trying to persist the `ChannelMonitor`.
        check_added_monitors!(nodes[1], 0);