Add 2428

Author: dunxen

_posts/2023-08-04-#2428.md

@@ -0,0 +1,32 @@
+---
+layout: pr
+date: 2023-08-04
+title: "Wait to create a channel until after accepting."
+pr: 2428
+authors: [waterson]
+components: ["channel", "ChannelManager"]
+host: dunxen
+status: upcoming
+commit:
+---
+
+
+## Notes
+
+* LDK allows a user to manually accept inbound channels from other peers when the corresponding `UserConfig::manually_accept_inbound_channels` field is set to `true`.
+  This results in an `Event::OpenChannelRequest` being triggered by a new inbound request for a channel via receiving an `OpenChannel` message. At this point, the
+  user needs to manually call `ChannelManager::accept_inbound_channel` or similar before an `AcceptChannel` will be sent back to the peer.
+* This feature gives the user an opportunity to make a decision on accepting or rejecting the channel with their own logic. Importantly, it also allows a user
+  to assign their own custom identity to the inbound channel using the `user_channel_id` field, but currently there's a bit of a problem with that...
+* Currently, LDK's `OpenChannel` message handler will _always_ attempt to construct an `InboundV1Channel` object before the `OpenChannelRequest` event is even triggered.
+  Consequently, cryptographic material is immediately requested from signer objects using a randomly generated `user_channel_id` and not one the user selected.
+* This PR fixes this bug by deferring construction of an `InboundV1Channel` object until after the user actually manually accepts the channel and has the opportunity
+  to provide a custom identity.
+
+## Questions
+1. Did you review the PR? [Concept ACK, approach ACK, tested ACK, or NACK](https://github.com/lightningdevkit/rust-lightning/blob/master/CONTRIBUTING.md#peer-review)?i
+2. What is an important use of a custom `user_channel_id` field when it comes to requesting cryptographic material? Hint: Take a look at [`SignerProvider::generate_channel_keys_id`](https://docs.rs/lightning/0.0.116/lightning/sign/trait.SignerProvider.html#tymethod.generate_channel_keys_id)
+3. What object is introduced to represent inbound channels awaiting a user to accept or reject it? Why would or wouldn't you consider this object a channel at this point?
+4. A new map, `inbound_channel_request_by_id` is introduced for `PeerState`. How do we treat and limit the entries of this map when it comes to denial-of-service mitigations?
+5. Why should we or why should we not still ignore any contributions that inbound 0conf channel requests make to the overall unfunded channel counts?
+6. Which fields/methods in `ChannelContext` are made obsolete by this PR?