Combine InboundV2Channel and OutboundV2Channel
#3498
Conversation
There was a problem hiding this comment.
LGTM.
I've gone through each commit and it's pretty straightforward IMO.
For some more context for others: It made more sense to have these structs and phases combined as unfunded V2 channels are more "symmetric" than their V1 counterparts. Particularly, after the initial handshake, we go through the turn-based interactive tx construction, and then both exchange initial commitment_signed and tx_signatures.
EDIT: The incremental_mutants CI failures aren't newly introduced, so it's okay to ignore.
| ChannelPhase::UnfundedV2(chan) => { | ||
| // Outbound channels don't contribute to the unfunded count in the DoS context. | ||
| if chan.context.is_outbound() { | ||
| continue; | ||
| } |
| @@ -1684,63 +1684,53 @@ impl<SP: Deref> InitialRemoteCommitmentReceiver<SP> for Channel<SP> where SP::Ta | |||
| } | |||
| } | |||
|
|
|||
| pub(super) trait InteractivelyFunded<SP: Deref> where SP::Target: SignerProvider { | |||
There was a problem hiding this comment.
Won't we need it for channel splices post-funding as well?
There was a problem hiding this comment.
As far as I know, these won't use the funded Channel struct directly. @optout21 could probably comment on that a bit more.
There was a problem hiding this comment.
No, InteractivelyFunded was used only for OutboundV2Channel and InboundV2Channel, I saw no use in other place (for splicing).
There was a problem hiding this comment.
Something similar will still need to exist though to interactively fund the splices?
There was a problem hiding this comment.
A previous draft of the splicing prototype did this, IIRC. Instead, I think we'd have the splicing phase hold one or more PendingV2Channels instead (or something like that), so it would just call the methods on those directly.
Pending v2 channels will need to be broken up into separate phases for constructing and signing the funding transaction. To avoid increasing the number of phases, combine the InboundV2Channel and OutboundV2Channel types so that the can be used in one phase. Whether the channel is inbound or outbound can be inferred from the ChannelContext.
Now that InboundV2Channel and OutboundV2Channel have been combined into a PendingV2Channel, the InteractivelyFunded trait is no longer needed.
Now that InboundV2Channel and OutboundV2Channel have been combined into a PendingV2Channel, only one ChannelPhase variant is needed.
jkczyz
force-pushed
the
2024-12-combine-inbound-v2-phases
branch
from
6972cf3 to
d6637d7
Compare
|
Fixed internal docs. PTAL. |
|
The |
Pending v2 channels will need to be broken up into separate phases for constructing and signing the funding transaction; see #3423 (comment). To avoid increasing the number of phases, combine the
InboundV2ChannelandOutboundV2Channeltypes and correspondingChannelPhasevariants. Whether the channel is inbound or outbound can be inferred from theChannelContext. Also, remove theInteractivelyFundedtrait as it is now no longer needed.