Merge pull request #2953 from dunxen/2024-03-fix2941
authorGursharan Singh <3442979+G8XSU@users.noreply.github.com>
Thu, 21 Mar 2024 17:55:19 +0000 (10:55 -0700)
committerGitHub <noreply@github.com>
Thu, 21 Mar 2024 17:55:19 +0000 (10:55 -0700)
Fix `ChannelManager::accept_inbound_channel` error handling

lightning/src/ln/channelmanager.rs
lightning/src/ln/functional_tests.rs

index 546b317ee26ddf65b04ab64488c53520353c2ba0..bddcd4c9b8117b7884228161757028d118dc7af0 100644 (file)
@@ -6112,73 +6112,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,
index 8c8be0b837bb05fc6cd49fbdf6a3b8b97209e72a..af6eee9cf85249bb2e2110bce91034ba1ef5b5d1 100644 (file)
@@ -10983,3 +10983,36 @@ fn test_funding_and_commitment_tx_confirm_same_block() {
        do_test_funding_and_commitment_tx_confirm_same_block(false);
        do_test_funding_and_commitment_tx_confirm_same_block(true);
 }
+
+#[test]
+fn test_accept_inbound_channel_errors_queued() {
+       // For manually accepted inbound channels, tests that a close error is correctly handled
+       // and the channel fails for the initiator.
+       let mut config0 = test_default_channel_config();
+       let mut config1 = config0.clone();
+       config1.channel_handshake_limits.their_to_self_delay = 1000;
+       config1.manually_accept_inbound_channels = true;
+       config0.channel_handshake_config.our_to_self_delay = 2000;
+
+       let chanmon_cfgs = create_chanmon_cfgs(2);
+       let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
+       let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(config0), Some(config1)]);
+       let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
+
+       nodes[0].node.create_channel(nodes[1].node.get_our_node_id(), 100_000, 0, 42, None, None).unwrap();
+       let open_channel_msg = get_event_msg!(nodes[0], MessageSendEvent::SendOpenChannel, nodes[1].node.get_our_node_id());
+
+       nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), &open_channel_msg);
+       let events = nodes[1].node.get_and_clear_pending_events();
+       match events[0] {
+               Event::OpenChannelRequest { temporary_channel_id, .. } => {
+                       match nodes[1].node.accept_inbound_channel(&temporary_channel_id, &nodes[0].node.get_our_node_id(), 23) {
+                               Err(APIError::ChannelUnavailable { err: _ }) => (),
+                               _ => panic!(),
+                       }
+               }
+               _ => panic!("Unexpected event"),
+       }
+       assert_eq!(get_err_msg(&nodes[1], &nodes[0].node.get_our_node_id()).channel_id,
+               open_channel_msg.common_fields.temporary_channel_id);
+}