Implement V2 channel establishment #2302

dunxen · 2023-05-16T10:20:00Z

This PR aims to do the following:

Introduce V2 prefunded channel types with state specific to dual-funding.
Add functionality to ChannelManager to create and accept dual-funded channels.
- Can set config contribute_to_dual_funded_channels if the user would like to contribute inputs to incoming dual-funded channels. In this case, these channels will need to be accepted manually, indicated by a new OpenChannelV2Request event so that inputs can be provided. If the above flag is false (default), then no inputs will be contributed and the channel is accepted on the user's behalf gated by all the commonly enforced rules. Other policies and functionality to automate funding of incoming channels may be introduced in the future.
Introduce dual-funded channel feature bits

lightning/src/ln/channel.rs

lightning/src/util/config.rs

lightning/src/ln/channelmanager.rs

codecov-commenter · 2023-05-25T17:35:57Z

Codecov Report

Attention: Patch coverage is 75.64808% with 310 lines in your changes missing coverage. Please review.

Project coverage is 89.89%. Comparing base (7326334) to head (14ca0af).

Files	Patch %	Lines
lightning/src/ln/interactivetxs.rs	72.96%	205 Missing and 8 partials ⚠️
lightning/src/ln/channelmanager.rs	88.34%	29 Missing and 7 partials ⚠️
lightning/src/ln/channel.rs	78.32%	29 Missing and 2 partials ⚠️
lightning/src/ln/functional_test_utils.rs	6.25%	30 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2302      +/-   ##
==========================================
- Coverage   89.90%   89.89%   -0.01%     
==========================================
  Files         117      117              
  Lines       97415    98059     +644     
  Branches    97415    98059     +644     
==========================================
+ Hits        87581    88152     +571     
- Misses       7272     7341      +69     
- Partials     2562     2566       +4

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

TheBlueMatt · 2023-07-08T20:22:07Z

This isn't blocked on anything anymore, right? Is there anything you want early feedback on here?

dunxen · 2023-07-08T21:22:18Z

This isn't blocked on anything anymore, right? Is there anything you want early feedback on here?

Technically blocked on interactive txs. But yeah, a few things I need to update before some conceptual/approach review here.

dunxen · 2023-07-10T15:31:23Z

Also now blocked on the 2077 fixups.

ariard · 2023-07-12T00:10:27Z

Technically blocked on interactive txs. But yeah, a few things I need to update before some conceptual/approach review here.

If this is blocked on review interactive txs on the spec-side, I can do a round here. Though dunno which PRs it’s blocked on.

dunxen · 2023-07-12T10:11:39Z

If this is blocked on review interactive txs on the spec-side, I can do a round here. Though dunno which PRs it’s blocked on.

Oh sorry, I meant the interactive txs implementation. Jurvis had two sketches up in his fork and will be putting a PR together as per the first action item here: https://gist.github.com/jurvis/98215abd6fd392a3f2f0ded03c5c6fff.
I'll mark this as blocked on that, but it will be at least ready for some conceptual review after I rebase the changes in #2382.

ariard · 2023-07-15T01:46:15Z

Jurvis had two sketches up in his fork and will be putting a PR together as per the first action item here: https://gist.github.com/jurvis/98215abd6fd392a3f2f0ded03c5c6fff.

@jurvis If you have a branch available for early conceptual review, happy to have a look and see how it scores compared to the specification, the ongoing changes in the mempool and LDK current interfaces :)

jurvis · 2023-07-15T23:07:54Z

@ariard we just finalised an approach for the TX constructor we are happy with so I’m working to put together a proof of concept rn for review. Should be ready in the next couple of days!

ariard · 2023-07-19T01:49:20Z

Should be ready in the next couple of days!

Many thanks, left a couple of review feedback on the new poc PR. If Duncan or you can maintain a dual-funding / splicing issue like Wilmer is doing for anchor output, this is very welcome to coordinate review and pin feedback backlog. I’ll have a look on the gist too, have to catchup with spec first.

wpaulino · 2023-07-19T16:46:09Z

If Duncan or you can maintain a dual-funding / splicing issue like Wilmer is doing for anchor output, this is very welcome to coordinate review and pin feedback backlog.

@ariard see #1621.

dunxen · 2023-07-19T18:32:12Z

@ariard see #1621.

Just added this PR there and will continue to keep fleshing it out.

jkczyz

Mostly a high-level pass.

jkczyz · 2024-05-08T21:37:03Z

lightning/src/ln/channel.rs

+	/// The current interactive transaction construction session under negotiation.
+	#[cfg(dual_funding)]
+	interactive_tx_constructor: Option<InteractiveTxConstructor>,


Rather than putting this in the context, should this go only in the relevant ChannelPhase variants?

This could always still be populated in an ordinary Channel too because of splicing. In fact we keep it around after promoting inbound and outbound V2 channels to Channel since we still need to complete the interactive signing at that point.

I could triplicate this in InboundV2Channel, OutboundV2Channel, and Channel if that makes more sense, and then the V1 unfunded channels wouldn't have access to it. I'm mostly indifferent to that. So if it's preferred I'll do that! :)

lightning/src/events/mod.rs

lightning/src/ln/channelmanager.rs

jkczyz · 2024-05-08T22:16:05Z

lightning/src/ln/channelmanager.rs

+					ChannelPhase::UnfundedInboundV2(ref mut channel) => {
+						channel.tx_add_input(msg).into_msg_send_event(counterparty_node_id)
+					},
+					ChannelPhase::UnfundedOutboundV2(ref mut channel) => {
+						channel.tx_add_input(msg).into_msg_send_event(counterparty_node_id)
+					},
+					_ => try_chan_phase_entry!(self, Err(ChannelError::Warn(
+						"Got a tx_add_input message with no interactive transaction construction expected or in-progress"
+						.into())), chan_phase_entry)


Related to my other comment about putting the InteractiveTxConstructor in the relevant phase variants, would that remove the need for the InteractivelyFunded trait? Seems the check here is essentially being repeated there. And here the context is available so you don't need HasChannelContext if all the code in InteractivelyFunded were inlined in these methods. Just feels like this could be simplified.

Yeah, those methods were inlined in the respective variants in the previous iteration. We had quite a bit of duplication before. I'll look into simplifying without re-introducing the duplication.

I was thinking something like this:

diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 1f65d064d..185daec8d 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -1243,6 +1243,19 @@ impl<'a, SP: Deref> ChannelPhase<SP> where ChannelPhase::UnfundedInboundV2(ref mut chan) => &mut chan.context, } } + + #[cfg(any(dual_funding, splicing))] + pub fn interactive_tx_constructor_mut(&'a mut self) -> Result<Option<&'a mut InteractiveTxConstructor>, ()> { + match self { + ChannelPhase::Funded(ref mut chan) => Ok(chan.interactive_tx_constructor.as_mut()), + ChannelPhase::UnfundedOutboundV1(ref mut chan) => Err(()), + ChannelPhase::UnfundedInboundV1(ref mut chan) => Err(()), + #[cfg(any(dual_funding, splicing))] + ChannelPhase::UnfundedOutboundV2(ref mut chan) => Ok(chan.interactive_tx_constructor.as_mut()), + #[cfg(any(dual_funding, splicing))] + ChannelPhase::UnfundedInboundV2(ref mut chan) => Ok(chan.interactive_tx_constructor.as_mut()), + } + } } /// Contains all state common to unfunded inbound/outbound channels. @@ -1652,17 +1665,6 @@ pub(super) trait InteractivelyFunded<SP: Deref> where SP::Target: SignerProvider Ok(msg) } - fn tx_add_input(&mut self, msg: &msgs::TxAddInput) -> InteractiveTxMessageSendResult { - InteractiveTxMessageSendResult(match self.interactive_tx_constructor_mut() { - Some(ref mut tx_constructor) => tx_constructor.handle_tx_add_input(msg).map_err( - |reason| reason.into_tx_abort_msg(self.context_mut().channel_id())), - None => Err(msgs::TxAbort { - channel_id: self.context_mut().channel_id(), - data: "We do not have an interactive transaction negotiation in progress".to_string().into_bytes() - }), - }) - } - fn tx_add_output(&mut self, msg: &msgs::TxAddOutput)-> InteractiveTxMessageSendResult { InteractiveTxMessageSendResult(match self.interactive_tx_constructor_mut() { Some(ref mut tx_constructor) => tx_constructor.handle_tx_add_output(msg).map_err( diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 7f1000ab9..077e022cb 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -53,7 +53,7 @@ use crate::ln::features::{Bolt12InvoiceFeatures, ChannelFeatures, ChannelTypeFea #[cfg(any(feature = "_test_utils", test))] use crate::ln::features::Bolt11InvoiceFeatures; #[cfg(any(dual_funding, splicing))] -use crate::ln::interactivetxs::InteractiveTxMessageSend; +use crate::ln::interactivetxs::{InteractiveTxMessageSend, InteractiveTxMessageSendResult}; use crate::routing::router::{BlindedTail, InFlightHtlcs, Path, Payee, PaymentParameters, Route, RouteParameters, Router}; use crate::ln::onion_payment::{check_incoming_htlc_cltv, create_recv_pending_htlc_info, create_fwd_pending_htlc_info, decode_incoming_update_add_htlc_onion, InboundHTLCErr, NextPacketDetails}; use crate::ln::msgs; @@ -7864,18 +7864,23 @@ where match peer_state.channel_by_id.entry(msg.channel_id) { hash_map::Entry::Occupied(mut chan_phase_entry) => { let channel_phase = chan_phase_entry.get_mut(); - let msg_send_event = match channel_phase { - ChannelPhase::UnfundedInboundV2(ref mut channel) => { - channel.tx_add_input(msg).into_msg_send_event(counterparty_node_id) - }, - ChannelPhase::UnfundedOutboundV2(ref mut channel) => { - channel.tx_add_input(msg).into_msg_send_event(counterparty_node_id) + let msg_send_event = match channel_phase.interactive_tx_constructor_mut() { + Ok(Some(tx_constructor)) => { + tx_constructor + .handle_tx_add_input(msg) + .map_err(|reason| reason.into_tx_abort_msg(channel_phase.context_mut().channel_id())) }, - _ => try_chan_phase_entry!(self, Err(ChannelError::Warn( + Ok(None) => Err(msgs::TxAbort { + channel_id: channel_phase.context_mut().channel_id(), + data: "We do not have an interactive transaction negotiation in progress".to_string().into_bytes(), + }), + Err(()) => try_chan_phase_entry!(self, Err(ChannelError::Warn( "Got a tx_add_input message with no interactive transaction construction expected or in-progress" .into())), chan_phase_entry) }; - peer_state.pending_msg_events.push(msg_send_event); + peer_state.pending_msg_events.push( + InteractiveTxMessageSendResult(msg_send_event).into_msg_send_event(counterparty_node_id) + ); Ok(()) }, hash_map::Entry::Vacant(_) => {

Though I admit returning a Result in ChannelPhase::interactive_tx_constructor_mut might not be ideal. What you have is probably a bit safer, now that I think about it more. And it may be needed for begin_interactive_funding_tx_construction, anyway.

lightning/src/events/mod.rs

lightning/src/ln/channel.rs

dunxen · 2024-05-15T17:07:34Z

Just resolved some conflicts and addressed a few comments. Still busy addressing Jeff's other initial review comments.

jkczyz · 2024-05-23T23:16:53Z

lightning/src/ln/channelmanager.rs

+					ChannelPhase::UnfundedInboundV2(ref mut channel) => {
+						channel.tx_add_input(msg).into_msg_send_event(counterparty_node_id)
+					},
+					ChannelPhase::UnfundedOutboundV2(ref mut channel) => {
+						channel.tx_add_input(msg).into_msg_send_event(counterparty_node_id)
+					},
+					_ => try_chan_phase_entry!(self, Err(ChannelError::Warn(
+						"Got a tx_add_input message with no interactive transaction construction expected or in-progress"
+						.into())), chan_phase_entry)


I was thinking something like this:

diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 1f65d064d..185daec8d 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -1243,6 +1243,19 @@ impl<'a, SP: Deref> ChannelPhase<SP> where ChannelPhase::UnfundedInboundV2(ref mut chan) => &mut chan.context, } } + + #[cfg(any(dual_funding, splicing))] + pub fn interactive_tx_constructor_mut(&'a mut self) -> Result<Option<&'a mut InteractiveTxConstructor>, ()> { + match self { + ChannelPhase::Funded(ref mut chan) => Ok(chan.interactive_tx_constructor.as_mut()), + ChannelPhase::UnfundedOutboundV1(ref mut chan) => Err(()), + ChannelPhase::UnfundedInboundV1(ref mut chan) => Err(()), + #[cfg(any(dual_funding, splicing))] + ChannelPhase::UnfundedOutboundV2(ref mut chan) => Ok(chan.interactive_tx_constructor.as_mut()), + #[cfg(any(dual_funding, splicing))] + ChannelPhase::UnfundedInboundV2(ref mut chan) => Ok(chan.interactive_tx_constructor.as_mut()), + } + } } /// Contains all state common to unfunded inbound/outbound channels. @@ -1652,17 +1665,6 @@ pub(super) trait InteractivelyFunded<SP: Deref> where SP::Target: SignerProvider Ok(msg) } - fn tx_add_input(&mut self, msg: &msgs::TxAddInput) -> InteractiveTxMessageSendResult { - InteractiveTxMessageSendResult(match self.interactive_tx_constructor_mut() { - Some(ref mut tx_constructor) => tx_constructor.handle_tx_add_input(msg).map_err( - |reason| reason.into_tx_abort_msg(self.context_mut().channel_id())), - None => Err(msgs::TxAbort { - channel_id: self.context_mut().channel_id(), - data: "We do not have an interactive transaction negotiation in progress".to_string().into_bytes() - }), - }) - } - fn tx_add_output(&mut self, msg: &msgs::TxAddOutput)-> InteractiveTxMessageSendResult { InteractiveTxMessageSendResult(match self.interactive_tx_constructor_mut() { Some(ref mut tx_constructor) => tx_constructor.handle_tx_add_output(msg).map_err( diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 7f1000ab9..077e022cb 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -53,7 +53,7 @@ use crate::ln::features::{Bolt12InvoiceFeatures, ChannelFeatures, ChannelTypeFea #[cfg(any(feature = "_test_utils", test))] use crate::ln::features::Bolt11InvoiceFeatures; #[cfg(any(dual_funding, splicing))] -use crate::ln::interactivetxs::InteractiveTxMessageSend; +use crate::ln::interactivetxs::{InteractiveTxMessageSend, InteractiveTxMessageSendResult}; use crate::routing::router::{BlindedTail, InFlightHtlcs, Path, Payee, PaymentParameters, Route, RouteParameters, Router}; use crate::ln::onion_payment::{check_incoming_htlc_cltv, create_recv_pending_htlc_info, create_fwd_pending_htlc_info, decode_incoming_update_add_htlc_onion, InboundHTLCErr, NextPacketDetails}; use crate::ln::msgs; @@ -7864,18 +7864,23 @@ where match peer_state.channel_by_id.entry(msg.channel_id) { hash_map::Entry::Occupied(mut chan_phase_entry) => { let channel_phase = chan_phase_entry.get_mut(); - let msg_send_event = match channel_phase { - ChannelPhase::UnfundedInboundV2(ref mut channel) => { - channel.tx_add_input(msg).into_msg_send_event(counterparty_node_id) - }, - ChannelPhase::UnfundedOutboundV2(ref mut channel) => { - channel.tx_add_input(msg).into_msg_send_event(counterparty_node_id) + let msg_send_event = match channel_phase.interactive_tx_constructor_mut() { + Ok(Some(tx_constructor)) => { + tx_constructor + .handle_tx_add_input(msg) + .map_err(|reason| reason.into_tx_abort_msg(channel_phase.context_mut().channel_id())) }, - _ => try_chan_phase_entry!(self, Err(ChannelError::Warn( + Ok(None) => Err(msgs::TxAbort { + channel_id: channel_phase.context_mut().channel_id(), + data: "We do not have an interactive transaction negotiation in progress".to_string().into_bytes(), + }), + Err(()) => try_chan_phase_entry!(self, Err(ChannelError::Warn( "Got a tx_add_input message with no interactive transaction construction expected or in-progress" .into())), chan_phase_entry) }; - peer_state.pending_msg_events.push(msg_send_event); + peer_state.pending_msg_events.push( + InteractiveTxMessageSendResult(msg_send_event).into_msg_send_event(counterparty_node_id) + ); Ok(()) }, hash_map::Entry::Vacant(_) => {

Though I admit returning a Result in ChannelPhase::interactive_tx_constructor_mut might not be ideal. What you have is probably a bit safer, now that I think about it more. And it may be needed for begin_interactive_funding_tx_construction, anyway.

lightning/src/ln/channel.rs

lightning/src/ln/channelmanager.rs

lightning/src/ln/channel.rs

dunxen mentioned this pull request May 16, 2023

Dual-funded channels and Splicing Project Tracking #1621

Open

10 tasks

ariard reviewed May 16, 2023

View reviewed changes

lightning/src/ln/channel.rs Outdated Show resolved Hide resolved

lightning/src/util/config.rs Outdated Show resolved Hide resolved

lightning/src/ln/channelmanager.rs Outdated Show resolved Hide resolved

dunxen force-pushed the 2023-05-v2channelestablish branch 3 times, most recently from 34b5cef to 5bd6e42 Compare May 22, 2023 14:55

dunxen changed the title ~~WIP: Implement V2 channel establishment~~ Implement V2 channel establishment May 24, 2023

dunxen added Dual-funding Review club labels May 24, 2023

dunxen force-pushed the 2023-05-v2channelestablish branch from 5bd6e42 to cc88119 Compare May 25, 2023 16:02

dunxen force-pushed the 2023-05-v2channelestablish branch from cc88119 to 6de5a2d Compare May 26, 2023 08:32

dunxen mentioned this pull request May 31, 2023

Add infra to block ChannelMonitorUpdates on forwarded claims #2167

Merged

dunxen removed the Review club label Jun 5, 2023

dunxen force-pushed the 2023-05-v2channelestablish branch from 6de5a2d to d155b0e Compare June 30, 2023 13:00

dunxen force-pushed the 2023-05-v2channelestablish branch from d155b0e to c861c67 Compare July 11, 2023 09:36

dunxen force-pushed the 2023-05-v2channelestablish branch from c861c67 to 335ab6f Compare July 12, 2023 14:23

dunxen mentioned this pull request Aug 14, 2023

Use a single peer state map for all channel phases in peer state #2495

Merged

dunxen force-pushed the 2023-05-v2channelestablish branch from 335ab6f to 6099dbf Compare August 15, 2023 20:05

dunxen force-pushed the 2023-05-v2channelestablish branch from 6099dbf to e50d576 Compare August 22, 2023 21:22

dunxen mentioned this pull request Aug 23, 2023

Introduce new ChannelId struct #2485

Merged

4 tasks

jkczyz reviewed May 8, 2024

View reviewed changes

optout21 reviewed May 15, 2024

View reviewed changes

lightning/src/events/mod.rs Outdated Show resolved Hide resolved

optout21 reviewed May 15, 2024

View reviewed changes

lightning/src/events/mod.rs Show resolved Hide resolved

optout21 reviewed May 15, 2024

View reviewed changes

lightning/src/events/mod.rs Outdated Show resolved Hide resolved

optout21 reviewed May 15, 2024

View reviewed changes

lightning/src/ln/channel.rs Show resolved Hide resolved

dunxen force-pushed the 2023-05-v2channelestablish branch 6 times, most recently from 953aa81 to 312429f Compare May 21, 2024 05:46

dunxen mentioned this pull request May 21, 2024

Interactive TX negotiation tracks shared outputs #2989

Open

dunxen force-pushed the 2023-05-v2channelestablish branch 3 times, most recently from b1b9d9d to ad11211 Compare May 23, 2024 17:30

jkczyz reviewed May 23, 2024

View reviewed changes

dunxen force-pushed the 2023-05-v2channelestablish branch 6 times, most recently from 5bb4f00 to ee08373 Compare May 30, 2024 11:45

dunxen added 6 commits June 3, 2024 20:32

Add interactive tx constructor to ChannelContext

c218186

Implement support for accepting V2 channels

ddf27af

Handle initial commitment_signed for V2 channels

5b4de38

Handle interactive signing sessions

97e661f

Handle re-establishment next_funding_txid

dfc7514

Add option_dual_fund feature

14ca0af

dunxen force-pushed the 2023-05-v2channelestablish branch from ee08373 to 14ca0af Compare June 3, 2024 19:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement V2 channel establishment #2302

Implement V2 channel establishment #2302

dunxen commented May 16, 2023 •

edited

codecov-commenter commented May 25, 2023 •

edited

TheBlueMatt commented Jul 8, 2023

dunxen commented Jul 8, 2023

dunxen commented Jul 10, 2023

ariard commented Jul 12, 2023

dunxen commented Jul 12, 2023

ariard commented Jul 15, 2023

jurvis commented Jul 15, 2023

ariard commented Jul 19, 2023

wpaulino commented Jul 19, 2023

dunxen commented Jul 19, 2023

jkczyz left a comment

jkczyz May 8, 2024

dunxen May 16, 2024

jkczyz May 8, 2024

dunxen May 15, 2024

jkczyz May 23, 2024

dunxen commented May 15, 2024

jkczyz May 23, 2024

Implement V2 channel establishment #2302

Are you sure you want to change the base?

Implement V2 channel establishment #2302

Conversation

dunxen commented May 16, 2023 • edited

codecov-commenter commented May 25, 2023 • edited

Codecov Report

TheBlueMatt commented Jul 8, 2023

dunxen commented Jul 8, 2023

dunxen commented Jul 10, 2023

ariard commented Jul 12, 2023

dunxen commented Jul 12, 2023

ariard commented Jul 15, 2023

jurvis commented Jul 15, 2023

ariard commented Jul 19, 2023

wpaulino commented Jul 19, 2023

dunxen commented Jul 19, 2023

jkczyz left a comment

Choose a reason for hiding this comment

jkczyz May 8, 2024

Choose a reason for hiding this comment

dunxen May 16, 2024

Choose a reason for hiding this comment

jkczyz May 8, 2024

Choose a reason for hiding this comment

dunxen May 15, 2024

Choose a reason for hiding this comment

jkczyz May 23, 2024

Choose a reason for hiding this comment

dunxen commented May 15, 2024

jkczyz May 23, 2024

Choose a reason for hiding this comment

dunxen commented May 16, 2023 •

edited

codecov-commenter commented May 25, 2023 •

edited