Handle sign_counterparty_commitment failing during inb funding
authorMatt Corallo <git@bluematt.me>
Tue, 5 Sep 2023 21:13:07 +0000 (21:13 +0000)
committerMatt Corallo <git@bluematt.me>
Tue, 5 Sep 2023 22:09:49 +0000 (22:09 +0000)
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 inbound channel funding, setting a flag in
`ChannelContext` which indicates we should retry sending the
`funding_signed` later. We don't yet add any ability to do that
retry.

lightning/src/ln/channel.rs
lightning/src/ln/channelmanager.rs

index f48145f9f5c818a5556238deba2e670de1e6354c..bbaa751f4a7a2f5780dd7cc022d3e42f05d65310 100644 (file)
@@ -6521,7 +6521,7 @@ impl<SP: Deref> InboundV1Channel<SP> where SP::Target: SignerProvider {
                self.generate_accept_channel_message()
        }
 
-       fn funding_created_signature<L: Deref>(&mut self, sig: &Signature, logger: &L) -> Result<(CommitmentTransaction, CommitmentTransaction, Signature), ChannelError> where L::Target: Logger {
+       fn funding_created_signature<L: Deref>(&mut self, sig: &Signature, logger: &L) -> Result<(CommitmentTransaction, CommitmentTransaction, Option<Signature>), ChannelError> where L::Target: Logger {
                let funding_script = self.context.get_funding_redeemscript();
 
                let keys = self.context.build_holder_transaction_keys(self.context.cur_holder_commitment_transaction_number);
@@ -6550,7 +6550,7 @@ impl<SP: Deref> InboundV1Channel<SP> where SP::Target: SignerProvider {
                        // TODO (arik): move match into calling method for Taproot
                        ChannelSignerType::Ecdsa(ecdsa) => {
                                let counterparty_signature = 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;
+                                       .map(|(sig, _)| sig).ok();
 
                                // We sign "counterparty" commitment transaction, allowing them to broadcast the tx if they wish.
                                Ok((counterparty_initial_commitment_tx, initial_commitment_tx, counterparty_signature))
@@ -6560,7 +6560,7 @@ impl<SP: Deref> InboundV1Channel<SP> where SP::Target: SignerProvider {
 
        pub fn funding_created<L: Deref>(
                mut self, msg: &msgs::FundingCreated, best_block: BestBlock, signer_provider: &SP, logger: &L
-       ) -> Result<(Channel<SP>, msgs::FundingSigned, ChannelMonitor<<SP::Target as SignerProvider>::Signer>), (Self, ChannelError)>
+       ) -> Result<(Channel<SP>, Option<msgs::FundingSigned>, ChannelMonitor<<SP::Target as SignerProvider>::Signer>), (Self, ChannelError)>
        where
                L::Target: Logger
        {
@@ -6585,7 +6585,7 @@ impl<SP: Deref> InboundV1Channel<SP> where SP::Target: SignerProvider {
                // funding_created_signature may fail.
                self.context.holder_signer.as_mut().provide_channel_parameters(&self.context.channel_transaction_parameters);
 
-               let (counterparty_initial_commitment_tx, initial_commitment_tx, signature) = match self.funding_created_signature(&msg.signature, logger) {
+               let (counterparty_initial_commitment_tx, initial_commitment_tx, sig_opt) = match self.funding_created_signature(&msg.signature, logger) {
                        Ok(res) => res,
                        Err(ChannelError::Close(e)) => {
                                self.context.channel_transaction_parameters.funding_outpoint = None;
@@ -6649,12 +6649,19 @@ impl<SP: Deref> InboundV1Channel<SP> where SP::Target: SignerProvider {
                let need_channel_ready = channel.check_get_channel_ready(0).is_some();
                channel.monitor_updating_paused(false, false, need_channel_ready, Vec::new(), Vec::new(), Vec::new());
 
-               Ok((channel, msgs::FundingSigned {
-                       channel_id,
-                       signature,
-                       #[cfg(taproot)]
-                       partial_signature_with_nonce: None,
-               }, channel_monitor))
+               let funding_signed = if let Some(signature) = sig_opt {
+                       Some(msgs::FundingSigned {
+                               channel_id,
+                               signature,
+                               #[cfg(taproot)]
+                               partial_signature_with_nonce: None,
+                       })
+               } else {
+                       channel.context.signer_pending_funding = true;
+                       None
+               };
+
+               Ok((channel, funding_signed, channel_monitor))
        }
 }
 
@@ -7737,7 +7744,7 @@ mod tests {
                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();
+               let _ = node_a_chan.funding_signed(&funding_signed_msg.unwrap(), best_block, &&keys_provider, &&logger).unwrap();
 
                // Put some inbound and outbound HTLCs in A's channel.
                let htlc_amount_msat = 11_092_000; // put an amount below A's effective dust limit but above B's.
@@ -7864,7 +7871,7 @@ mod tests {
                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();
+               let _ = node_a_chan.funding_signed(&funding_signed_msg.unwrap(), best_block, &&keys_provider, &&logger).unwrap();
 
                // Now disconnect the two nodes and check that the commitment point in
                // Node B's channel_reestablish message is sane.
@@ -8052,7 +8059,7 @@ mod tests {
                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();
+               let _ = node_a_chan.funding_signed(&funding_signed_msg.unwrap(), best_block, &&keys_provider, &&logger).unwrap();
 
                // Make sure that receiving a channel update will update the Channel as expected.
                let update = ChannelUpdate {
index d84bca1795015d9e7715c8eeae0ab7ea968f2452..bcfca4cc246625f6b57f455e22cfce69d31cf1de 100644 (file)
@@ -5626,7 +5626,7 @@ where
 
                let mut peer_state_lock = peer_state_mutex.lock().unwrap();
                let peer_state = &mut *peer_state_lock;
-               let (chan, funding_msg, monitor) =
+               let (chan, funding_msg_opt, monitor) =
                        match peer_state.inbound_v1_channel_by_id.remove(&msg.temporary_channel_id) {
                                Some(inbound_chan) => {
                                        match inbound_chan.funding_created(msg, best_block, &self.signer_provider, &self.logger) {
@@ -5646,16 +5646,19 @@ where
                                None => 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.temporary_channel_id))
                        };
 
-               match peer_state.channel_by_id.entry(funding_msg.channel_id) {
+               match peer_state.channel_by_id.entry(chan.context.channel_id()) {
                        hash_map::Entry::Occupied(_) => {
-                               Err(MsgHandleErrInternal::send_err_msg_no_close("Already had channel with the new channel_id".to_owned(), funding_msg.channel_id))
+                               Err(MsgHandleErrInternal::send_err_msg_no_close(
+                                       "Already had channel with the new channel_id".to_owned(),
+                                       chan.context.channel_id()
+                               ))
                        },
                        hash_map::Entry::Vacant(e) => {
                                match self.id_to_peer.lock().unwrap().entry(chan.context.channel_id()) {
                                        hash_map::Entry::Occupied(_) => {
                                                return Err(MsgHandleErrInternal::send_err_msg_no_close(
                                                        "The funding_created message had the same funding_txid as an existing channel - funding is not possible".to_owned(),
-                                                       funding_msg.channel_id))
+                                                       chan.context.channel_id()))
                                        },
                                        hash_map::Entry::Vacant(i_e) => {
                                                i_e.insert(chan.context.get_counterparty_node_id());
@@ -5666,11 +5669,13 @@ where
                                // hasn't persisted to disk yet - we can't lose money on a transaction that we haven't
                                // accepted payment from yet. We do, however, need to wait to send our channel_ready
                                // until we have persisted our monitor.
-                               let new_channel_id = funding_msg.channel_id;
-                               peer_state.pending_msg_events.push(events::MessageSendEvent::SendFundingSigned {
-                                       node_id: counterparty_node_id.clone(),
-                                       msg: funding_msg,
-                               });
+                               let new_channel_id = chan.context.channel_id();
+                               if let Some(msg) = funding_msg_opt {
+                                       peer_state.pending_msg_events.push(events::MessageSendEvent::SendFundingSigned {
+                                               node_id: counterparty_node_id.clone(),
+                                               msg,
+                                       });
+                               }
 
                                let monitor_res = self.chain_monitor.watch_channel(monitor.get_funding_txo().0, monitor);