Skip to content

Inbound channel requests cryptographic material before being accepted & assigned identity #2429

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.

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

Closed
waterson opened this issue Jul 18, 2023 · 4 comments

Comments

@waterson
Copy link
Contributor

When the channel manager has been configured to "manually accept" incoming channels, the OpenChannelRequest event is sent to the event handler. Besides giving the user an opportunity to accept or reject the channel, it additionally allows the user to assign their own identity to a channel via the user_channel_id. This is also presumably a reasonable place for user code to create any internal structures that it needs that are related to the channel.

Currently, handler for the open_channel Lightning protocol message unilaterally creates the IncomingV1Channel object using a randomly generated user_channel_id: in particular, it does this /before/ sending the OpenChannelRequest. This has the side effect of immediately requesting cryptographic material via the Signer interfaces using the randomly generated user_channel_id (e.g., when calling SignerProvider::generate_channel_keys_id).

It seems like it might make sense to instead defer creation of the IncomingV1Channel object until after user code has accepted the channel and provided an identity for it. See for example #2428 for one possible approach.

@dunxen
Copy link
Contributor

dunxen commented Jul 18, 2023

Thanks! I'm going to track this in #2422 too and hopefully we also get some input on this issue when I get to that soon, but I don't think it changes anything with that refactor.

@dunxen
Copy link
Contributor

dunxen commented Jul 19, 2023

@waterson
So I've been having a look and thinking that it is probably better to just have a separate map for this as you proposed in #2428. Since we don't even have an actual channel context at that stage it's more like a "pending inbound open channel request" and not an actual channel so should not be part of the new ChannelPhase enum I am planning. If you're happy to continue with that PR then I think it's a good start. :) Sorry for the back and forth!

@dunxen dunxen added this to the 0.0.117 milestone Jul 19, 2023
@waterson
Copy link
Contributor Author

Sounds good... I've since made some changes to the PR to cleanup if the user rejects, and to fix some of the tests. A few questions on that.

@TheBlueMatt
Copy link
Collaborator

Fixed in #2428.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants