Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix ChannelManager::accept_inbound_channel error handling #2953

Merged
merged 2 commits into from
Mar 21, 2024
Merged

Fix ChannelManager::accept_inbound_channel error handling #2953

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1cb6d2f
Fix ChannelManager::accept_inbound_channel error handling
dunxen Mar 20, 2024
3206d1f
Test ChannelManager::accept_inbound_channel errors
dunxen Mar 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
107 changes: 58 additions & 49 deletions lightning/src/ln/channelmanager.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
dunxen marked this conversation as resolved.
Show resolved Hide resolved
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,
Expand Down
33 changes: 33 additions & 0 deletions lightning/src/ln/functional_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Loading