Skip to content

Add user_channel_id to accept_inbound_channel method #1381

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

Merged
merged 1 commit into from
Mar 25, 2022
Merged

Add user_channel_id to accept_inbound_channel method #1381

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 4 additions & 3 deletions lightning/src/ln/channel.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4745,7 +4745,7 @@ impl<Signer: Sign> Channel<Signer> {
/// should be sent back to the counterparty node.
///
/// [`msgs::AcceptChannel`]: crate::ln::msgs::AcceptChannel
pub fn accept_inbound_channel(&mut self) -> msgs::AcceptChannel {
pub fn accept_inbound_channel(&mut self, user_id: u64) -> msgs::AcceptChannel {
if self.is_outbound() {
panic!("Tried to send accept_channel for an outbound channel?");
}
Expand All @@ -4759,6 +4759,7 @@ impl<Signer: Sign> Channel<Signer> {
panic!("The inbound channel has already been accepted");
}

self.user_id = user_id;
self.inbound_awaiting_accept = false;

self.generate_accept_channel_message()
Expand Down Expand Up @@ -6420,7 +6421,7 @@ mod tests {
let mut node_b_chan = Channel::<EnforcingSigner>::new_from_req(&&feeest, &&keys_provider, node_b_node_id, &InitFeatures::known(), &open_channel_msg, 7, &config, 0, &&logger, 42).unwrap();

// Node B --> Node A: accept channel, explicitly setting B's dust limit.
let mut accept_channel_msg = node_b_chan.accept_inbound_channel();
let mut accept_channel_msg = node_b_chan.accept_inbound_channel(0);
accept_channel_msg.dust_limit_satoshis = 546;
node_a_chan.accept_channel(&accept_channel_msg, &config.peer_channel_config_limits, &InitFeatures::known()).unwrap();
node_a_chan.holder_dust_limit_satoshis = 1560;
Expand Down Expand Up @@ -6538,7 +6539,7 @@ mod tests {
let mut node_b_chan = Channel::<EnforcingSigner>::new_from_req(&&feeest, &&keys_provider, node_b_node_id, &InitFeatures::known(), &open_channel_msg, 7, &config, 0, &&logger, 42).unwrap();

// Node B --> Node A: accept channel
let accept_channel_msg = node_b_chan.accept_inbound_channel();
let accept_channel_msg = node_b_chan.accept_inbound_channel(0);
node_a_chan.accept_channel(&accept_channel_msg, &config.peer_channel_config_limits, &InitFeatures::known()).unwrap();

// Node A --> Node B: funding created
Expand Down
13 changes: 9 additions & 4 deletions lightning/src/ln/channelmanager.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4287,8 +4287,13 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
///
/// The `temporary_channel_id` parameter indicates which inbound channel should be accepted.
///
/// [`Event::OpenChannelRequest`]: crate::util::events::Event::OpenChannelRequest
pub fn accept_inbound_channel(&self, temporary_channel_id: &[u8; 32]) -> Result<(), APIError> {
/// For inbound channels, the `user_channel_id` parameter will be provided back in
/// [`Event::ChannelClosed::user_channel_id`] to allow tracking of which events correspond
/// with which `accept_inbound_channel` call.
///
/// [`Event::OpenChannelRequest`]: events::Event::OpenChannelRequest
/// [`Event::ChannelClosed::user_channel_id`]: events::Event::ChannelClosed::user_channel_id
pub fn accept_inbound_channel(&self, temporary_channel_id: &[u8; 32], user_channel_id: u64) -> Result<(), APIError> {
let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(&self.total_consistency_lock, &self.persistence_notifier);

let mut channel_state_lock = self.channel_state.lock().unwrap();
Expand All @@ -4300,7 +4305,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
}
channel_state.pending_msg_events.push(events::MessageSendEvent::SendAcceptChannel {
node_id: channel.get().get_counterparty_node_id(),
msg: channel.get_mut().accept_inbound_channel(),
msg: channel.get_mut().accept_inbound_channel(user_channel_id),
});
}
hash_map::Entry::Vacant(_) => {
Expand Down Expand Up @@ -4341,7 +4346,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
if !self.default_configuration.manually_accept_inbound_channels {
channel_state.pending_msg_events.push(events::MessageSendEvent::SendAcceptChannel {
node_id: counterparty_node_id.clone(),
msg: channel.accept_inbound_channel(),
msg: channel.accept_inbound_channel(0),
Copy link
Collaborator

@TheBlueMatt TheBlueMatt Mar 25, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, this now overwrites the user id for outbound channels - can either make the channel.rs function an Option again or re-set it by fetching the channel's ID here.

Copy link
Contributor Author

@shamardy shamardy Mar 25, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not clear to me how this can happen since accept_inbound_channel doesn't allow outbound channels modifications

rust-lightning/lightning/src/ln/channel.rs

Lines 4748 to 4751 in edd4bab

pub fn accept_inbound_channel(&mut self, user_id: u64) -> msgs::AcceptChannel {
if self.is_outbound() {
panic!("Tried to send accept_channel for an outbound channel?");
}

But I guess it's better to use get_user_id anyways.

Copy link
Collaborator

@TheBlueMatt TheBlueMatt Mar 25, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh! I'm sorry, yes. So this comment is in internal_open_channel, not accept_inbound_channel, but I clearly wasn't thinking, that is also for inbound channels so no problem.

});
} else {
let mut pending_events = self.pending_events.lock().unwrap();
Expand Down
23 changes: 18 additions & 5 deletions lightning/src/ln/functional_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8110,7 +8110,7 @@ fn test_manually_accept_inbound_channel_request() {
let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, Some(manually_accept_conf.clone())]);
let nodes = create_network(2, &node_cfgs, &node_chanmgrs);

nodes[0].node.create_channel(nodes[1].node.get_our_node_id(), 100000, 10001, 42, Some(manually_accept_conf)).unwrap();
let temp_channel_id = nodes[0].node.create_channel(nodes[1].node.get_our_node_id(), 100000, 10001, 42, Some(manually_accept_conf)).unwrap();
let res = 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(), InitFeatures::known(), &res);
Expand All @@ -8122,7 +8122,7 @@ fn test_manually_accept_inbound_channel_request() {
let events = nodes[1].node.get_and_clear_pending_events();
match events[0] {
Event::OpenChannelRequest { temporary_channel_id, .. } => {
nodes[1].node.accept_inbound_channel(&temporary_channel_id).unwrap();
nodes[1].node.accept_inbound_channel(&temporary_channel_id, 23).unwrap();
}
_ => panic!("Unexpected event"),
}
Expand All @@ -8136,6 +8136,19 @@ fn test_manually_accept_inbound_channel_request() {
}
_ => panic!("Unexpected event"),
}

nodes[1].node.force_close_channel(&temp_channel_id).unwrap();

let close_msg_ev = nodes[1].node.get_and_clear_pending_msg_events();
assert_eq!(close_msg_ev.len(), 1);

let events = nodes[1].node.get_and_clear_pending_events();
match events[0] {
Event::ChannelClosed { user_channel_id, .. } => {
assert_eq!(user_channel_id, 23);
}
_ => panic!("Unexpected event"),
}
}

#[test]
Expand Down Expand Up @@ -8259,8 +8272,8 @@ fn test_can_not_accept_inbound_channel_twice() {
let events = nodes[1].node.get_and_clear_pending_events();
match events[0] {
Event::OpenChannelRequest { temporary_channel_id, .. } => {
nodes[1].node.accept_inbound_channel(&temporary_channel_id).unwrap();
let api_res = nodes[1].node.accept_inbound_channel(&temporary_channel_id);
nodes[1].node.accept_inbound_channel(&temporary_channel_id, 0).unwrap();
let api_res = nodes[1].node.accept_inbound_channel(&temporary_channel_id, 0);
match api_res {
Err(APIError::APIMisuseError { err }) => {
assert_eq!(err, "The channel isn't currently awaiting to be accepted.");
Expand Down Expand Up @@ -8292,7 +8305,7 @@ fn test_can_not_accept_unknown_inbound_channel() {
let node = create_network(1, &node_cfg, &node_chanmgr)[0].node;

let unknown_channel_id = [0; 32];
let api_res = node.accept_inbound_channel(&unknown_channel_id);
let api_res = node.accept_inbound_channel(&unknown_channel_id, 0);
match api_res {
Err(APIError::ChannelUnavailable { err }) => {
assert_eq!(err, "Can't accept a channel that doesn't exist");
Expand Down
10 changes: 7 additions & 3 deletions lightning/src/util/events.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -365,11 +365,15 @@ pub enum Event {
/// The channel_id of the channel which has been closed. Note that on-chain transactions
/// resolving the channel are likely still awaiting confirmation.
channel_id: [u8; 32],
/// The `user_channel_id` value passed in to [`ChannelManager::create_channel`], or 0 for
/// an inbound channel. This will always be zero for objects serialized with LDK versions
/// prior to 0.0.102.
/// The `user_channel_id` value passed in to [`ChannelManager::create_channel`] for outbound
/// channels, or to [`ChannelManager::accept_inbound_channel`] for inbound channels if
/// [`UserConfig::manually_accept_inbound_channels`] config flag is set to true. Otherwise
/// `user_channel_id` will be 0 for an inbound channel.
/// This will always be zero for objects serialized with LDK versions prior to 0.0.102.
///
/// [`ChannelManager::create_channel`]: crate::ln::channelmanager::ChannelManager::create_channel
/// [`ChannelManager::accept_inbound_channel`]: crate::ln::channelmanager::ChannelManager::accept_inbound_channel
/// [`UserConfig::manually_accept_inbound_channels`]: crate::util::config::UserConfig::manually_accept_inbound_channels
user_channel_id: u64,
/// The reason the channel was closed.
reason: ClosureReason
Expand Down