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 55505b97f24a9de513f01e11176867dc37383572..5af38393e1739a668dc028df81af6ff556e7bc46 100644 (file)
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -757,6 +757,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.
        //
@@ -5847,6 +5851,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)),
@@ -5928,15 +5933,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)
                        }
                }
        }
@@ -5949,7 +5953,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, is_batch_funding: bool, 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!");
                }
@@ -5965,15 +5969,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:
@@ -5992,20 +5987,27 @@ impl<SP: Deref> OutboundV1Channel<SP> where SP::Target: SignerProvider {
                self.context.funding_transaction = Some(funding_transaction);
                self.context.is_batch_funding = Some(()).filter(|_| is_batch_funding);
 
+               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 {
@@ -6503,6 +6505,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)),
@@ -7596,6 +7599,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,
@@ -7868,7 +7872,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, false, &&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();
@@ -7995,7 +7999,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, false, &&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();
@@ -8183,7 +8187,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, false, &&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();
@@ -9255,7 +9259,7 @@ mod tests {
                        &&logger,
                ).map_err(|_| ()).unwrap();
                let (mut node_b_chan, funding_signed_msg, _) = node_b_chan.funding_created(
-                       &funding_created_msg,
+                       &funding_created_msg.unwrap(),
                        best_block,
                        &&keys_provider,
                        &&logger,

diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index c4efd895796e67d1f3c1ecc46e5559b277c5cc9b..892e64eea95382e4a2a112e3d569f9e4ad277d7a 100644 (file)
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -3803,7 +3803,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.channel_by_id.remove(temporary_channel_id) {
+               let (chan, msg_opt) = match peer_state.channel_by_id.remove(temporary_channel_id) {
                        Some(ChannelPhase::UnfundedOutboundV1(chan)) => {
                                let funding_txo = find_funding_output(&chan, &funding_transaction)?;
 
@@ -3842,10 +3842,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 555c8a4e9e8b7ab3eb5e61c8055b1f8eefd215e3..9b33d8029ecc9accc99164d45c528face92f9836 100644 (file)
--- a/lightning/src/ln/functional_tests.rs
+++ b/lightning/src/ln/functional_tests.rs
@@ -9045,7 +9045,7 @@ fn test_duplicate_chan_id() {
                }
        };
        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);