Fix ChannelManager::accept_inbound_channel error handling
authorDuncan Dean <git@dunxen.dev>
Wed, 20 Mar 2024 17:31:39 +0000 (19:31 +0200)
committerDuncan Dean <git@dunxen.dev>
Thu, 21 Mar 2024 07:25:16 +0000 (09:25 +0200)
lightning/src/ln/channelmanager.rs

index 53cdf17ecba1aacf5efc6b448bca3bcc8d942742..825d450713482eee1e0dc27f88a6b83a5f1820bb 100644 (file)
@@ -6106,73 +6106,82 @@ where
                // happening and return an error. N.B. that we create channel with an outbound SCID of zero so
                // that we can delay allocating the SCID until after we're sure that the checks below will
                // succeed.
-               let mut channel = match peer_state.inbound_channel_request_by_id.remove(temporary_channel_id) {
+               let res = match peer_state.inbound_channel_request_by_id.remove(temporary_channel_id) {
                        Some(unaccepted_channel) => {
                                let best_block_height = self.best_block.read().unwrap().height;
                                InboundV1Channel::new(&self.fee_estimator, &self.entropy_source, &self.signer_provider,
                                        counterparty_node_id.clone(), &self.channel_type_features(), &peer_state.latest_features,
                                        &unaccepted_channel.open_channel_msg, user_channel_id, &self.default_configuration, best_block_height,
-                                       &self.logger, accept_0conf).map_err(|e| {
-                                               let err_str = e.to_string();
-                                               log_error!(logger, "{}", err_str);
-
-                                               APIError::ChannelUnavailable { err: err_str }
-                                       })
-                               }
+                                       &self.logger, accept_0conf).map_err(|err| MsgHandleErrInternal::from_chan_no_close(err, *temporary_channel_id))
+                       },
                        _ => {
                                let err_str = "No such channel awaiting to be accepted.".to_owned();
                                log_error!(logger, "{}", err_str);
 
-                               Err(APIError::APIMisuseError { err: err_str })
+                               return Err(APIError::APIMisuseError { err: err_str });
                        }
-               }?;
+               };
 
-               if accept_0conf {
-                       // This should have been correctly configured by the call to InboundV1Channel::new.
-                       debug_assert!(channel.context.minimum_depth().unwrap() == 0);
-               } else if channel.context.get_channel_type().requires_zero_conf() {
-                       let send_msg_err_event = events::MessageSendEvent::HandleError {
-                               node_id: channel.context.get_counterparty_node_id(),
-                               action: msgs::ErrorAction::SendErrorMessage{
-                                       msg: msgs::ErrorMessage { channel_id: temporary_channel_id.clone(), data: "No zero confirmation channels accepted".to_owned(), }
+               match res {
+                       Err(err) => {
+                               mem::drop(peer_state_lock);
+                               mem::drop(per_peer_state);
+                               match handle_error!(self, Result::<(), MsgHandleErrInternal>::Err(err), *counterparty_node_id) {
+                                       Ok(_) => unreachable!("`handle_error` only returns Err as we've passed in an Err"),
+                                       Err(e) => {
+                                               return Err(APIError::ChannelUnavailable { err: e.err });
+                                       },
                                }
-                       };
-                       peer_state.pending_msg_events.push(send_msg_err_event);
-                       let err_str = "Please use accept_inbound_channel_from_trusted_peer_0conf to accept channels with zero confirmations.".to_owned();
-                       log_error!(logger, "{}", err_str);
+                       }
+                       Ok(mut channel) => {
+                               if accept_0conf {
+                                       // This should have been correctly configured by the call to InboundV1Channel::new.
+                                       debug_assert!(channel.context.minimum_depth().unwrap() == 0);
+                               } else if channel.context.get_channel_type().requires_zero_conf() {
+                                       let send_msg_err_event = events::MessageSendEvent::HandleError {
+                                               node_id: channel.context.get_counterparty_node_id(),
+                                               action: msgs::ErrorAction::SendErrorMessage{
+                                                       msg: msgs::ErrorMessage { channel_id: temporary_channel_id.clone(), data: "No zero confirmation channels accepted".to_owned(), }
+                                               }
+                                       };
+                                       peer_state.pending_msg_events.push(send_msg_err_event);
+                                       let err_str = "Please use accept_inbound_channel_from_trusted_peer_0conf to accept channels with zero confirmations.".to_owned();
+                                       log_error!(logger, "{}", err_str);
 
-                       return Err(APIError::APIMisuseError { err: err_str });
-               } else {
-                       // If this peer already has some channels, a new channel won't increase our number of peers
-                       // with unfunded channels, so as long as we aren't over the maximum number of unfunded
-                       // channels per-peer we can accept channels from a peer with existing ones.
-                       if is_only_peer_channel && peers_without_funded_channels >= MAX_UNFUNDED_CHANNEL_PEERS {
-                               let send_msg_err_event = events::MessageSendEvent::HandleError {
-                                       node_id: channel.context.get_counterparty_node_id(),
-                                       action: msgs::ErrorAction::SendErrorMessage{
-                                               msg: msgs::ErrorMessage { channel_id: temporary_channel_id.clone(), data: "Have too many peers with unfunded channels, not accepting new ones".to_owned(), }
-                                       }
-                               };
-                               peer_state.pending_msg_events.push(send_msg_err_event);
-                               let err_str = "Too many peers with unfunded channels, refusing to accept new ones".to_owned();
-                               log_error!(logger, "{}", err_str);
+                                       return Err(APIError::APIMisuseError { err: err_str });
+                               } else {
+                                       // If this peer already has some channels, a new channel won't increase our number of peers
+                                       // with unfunded channels, so as long as we aren't over the maximum number of unfunded
+                                       // channels per-peer we can accept channels from a peer with existing ones.
+                                       if is_only_peer_channel && peers_without_funded_channels >= MAX_UNFUNDED_CHANNEL_PEERS {
+                                               let send_msg_err_event = events::MessageSendEvent::HandleError {
+                                                       node_id: channel.context.get_counterparty_node_id(),
+                                                       action: msgs::ErrorAction::SendErrorMessage{
+                                                               msg: msgs::ErrorMessage { channel_id: temporary_channel_id.clone(), data: "Have too many peers with unfunded channels, not accepting new ones".to_owned(), }
+                                                       }
+                                               };
+                                               peer_state.pending_msg_events.push(send_msg_err_event);
+                                               let err_str = "Too many peers with unfunded channels, refusing to accept new ones".to_owned();
+                                               log_error!(logger, "{}", err_str);
 
-                               return Err(APIError::APIMisuseError { err: err_str });
-                       }
-               }
+                                               return Err(APIError::APIMisuseError { err: err_str });
+                                       }
+                               }
 
-               // Now that we know we have a channel, assign an outbound SCID alias.
-               let outbound_scid_alias = self.create_and_insert_outbound_scid_alias();
-               channel.context.set_outbound_scid_alias(outbound_scid_alias);
+                               // Now that we know we have a channel, assign an outbound SCID alias.
+                               let outbound_scid_alias = self.create_and_insert_outbound_scid_alias();
+                               channel.context.set_outbound_scid_alias(outbound_scid_alias);
 
-               peer_state.pending_msg_events.push(events::MessageSendEvent::SendAcceptChannel {
-                       node_id: channel.context.get_counterparty_node_id(),
-                       msg: channel.accept_inbound_channel(),
-               });
+                               peer_state.pending_msg_events.push(events::MessageSendEvent::SendAcceptChannel {
+                                       node_id: channel.context.get_counterparty_node_id(),
+                                       msg: channel.accept_inbound_channel(),
+                               });
 
-               peer_state.channel_by_id.insert(temporary_channel_id.clone(), ChannelPhase::UnfundedInboundV1(channel));
+                               peer_state.channel_by_id.insert(temporary_channel_id.clone(), ChannelPhase::UnfundedInboundV1(channel));
 
-               Ok(())
+                               Ok(())
+                       },
+               }
        }
 
        /// Gets the number of peers which match the given filter and do not have any funded, outbound,