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 73.02780% with 359 lines in your changes missing coverage. Please review.

Project coverage is 89.70%. Comparing base (07f3380) to head (365aac8).

Files	Patch %	Lines
lightning/src/ln/interactivetxs.rs	72.96%	205 Missing and 8 partials ⚠️
lightning/src/ln/channelmanager.rs	76.58%	78 Missing and 7 partials ⚠️
lightning/src/ln/channel.rs	78.91%	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.70%   -0.20%     
==========================================
  Files         121      121              
  Lines       99170   100033     +863     
  Branches    99170   100033     +863     
==========================================
+ Hits        89159    89735     +576     
- Misses       7408     7687     +279     
- Partials     2603     2611       +8

☔ 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.

TheBlueMatt

So sorry this took a while to get back to.

lightning/src/ln/msgs.rs

TheBlueMatt · 2024-06-05T20:38:02Z

lightning/src/ln/channel.rs

@@ -3415,6 +3417,9 @@ pub(super) struct Channel<SP: Deref> where SP::Target: SignerProvider {
 	pub context: ChannelContext<SP>,
 	#[cfg(any(dual_funding, splicing))]
 	pub dual_funding_channel_context: Option<DualFundingChannelContext>,
+	/// The current interactive transaction construction session under negotiation.
+	#[cfg(any(dual_funding, splicing))]
+	interactive_tx_constructor: Option<InteractiveTxConstructor>,


Is this just for funding-RBF? Can we come up with a way to shove it in the AwaitingChannelReadyFlags? Or do we intend to reuse this for splicing when we get there (and is that gonna work? Is there a way to have multiple splicing/RBFs in-flight at once?)

It's for splicing too. There can only be one transaction under construction at any one time on a channel.

Ah, alright. Is there any reason to think we'd want to be able to detect if its splicing/RBF in the future?

There may be. I'll find out after we get a concrete splicing PR up :)

lightning/src/events/mod.rs

lightning/src/ln/channel.rs

TheBlueMatt · 2024-06-05T21:11:58Z

lightning/src/ln/channel.rs

+			self.dual_funding_context_mut().funding_feerate_sat_per_1000_weight,
+			holder_node_id, self.context().counterparty_node_id, self.is_initiator(),
+			self.dual_funding_context_mut().funding_tx_locktime,
+			self.dual_funding_context_mut().our_funding_inputs.clone(), funding_outputs,


It looks like this is the only place our_funding_inputs is used. Instead of taking it by ref via the dual_funding_context, maybe we should instead take it as an arg to this method?

So there's at least one case where our_funding_inputs needs to be "stored" before calling the begin_interactive_funding_tx_construction method, and that's for the initiator as inputs are provided upfront when creating the channel and then the begin_interactive_funding_tx_construction method is only called after the initiator receives an accept_channel2 message.

I think one way of doing this would be to make our_funding_inputs an Option and then just take() it here via the ref and in other places to avoid the clone.

lightning/src/ln/channelmanager.rs

lightning/src/ln/channel.rs

lightning/src/ln/channelmanager.rs

dunxen · 2024-06-07T13:24:21Z

Pushed some fixes for feedback. Working on remaining ones.

TheBlueMatt

Needs rebase on #3123

TheBlueMatt · 2024-06-13T15:03:00Z

lightning/src/ln/channelmanager.rs

+		&self, channel_id: &ChannelId, counterparty_node_id: &PublicKey, transaction: Transaction
+	) -> Result<(), APIError> {
+		let witnesses: Vec<_> = transaction.input.into_iter().filter_map(|input| {
+			if input.witness.is_empty() { None } else { Some(input.witness) }


We should fail if any inputs are non-segwit, right (and document it)? Otherwise the funding tx is malleable and we could lose funds.

We do those checks upfront (or should) and during negotiation. This is just to extract the witnesses.

I'll have another look through and add some test cases at the channelmanager level.

TheBlueMatt · 2024-06-13T15:06:28Z

lightning/src/events/mod.rs

@@ -1174,7 +1200,7 @@ pub enum Event {
 		/// The channel value of the requested channel.
 		funding_satoshis: u64,
 		/// Our starting balance in the channel if the request is accepted, in milli-satoshi.
-		push_msat: u64,
+		acceptor_funds: InboundChannelFunds,


What's the plan for landing this? We have public API changes (and associated documentation) here, but most of the logic is still cfg-flagged. Its not the end of the world but definitely a bit awkward...Does it make sense to land v2 establishment without the ability to fund the opening transaction so we can avoid all public API changes and then land the ability to negotiate in a separate PR? That would also neatly split this PR, though it would be some work to split it apart, I think?

So, first being allowed to accept V2 channels without contributing (and then introducing the public API in the next PR for contributing)?

It's not too much work, and it does seem safer to land that a bit earlier than everything here.

Do you plan on doing this in a new PR and rebasing this one or just doing it in this one?

I'm thinking new PR to slim this one down. Then I'll rebase this one.

EDIT: The split seems to be going smoothly for now.

dunxen · 2024-06-20T16:36:16Z

Split out #3137 which should be easier to review and lower risk since it involves no contribution of funds.

This PR will be rebased on that.

dunxen mentioned this pull request May 16, 2023

Dual-funded channels and Splicing Project Tracking #1621

Open

6 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

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

dunxen force-pushed the 2023-05-v2channelestablish branch 4 times, most recently from 7b66c45 to e0c9bbb Compare June 4, 2024 18:24

TheBlueMatt reviewed Jun 5, 2024

View reviewed changes

dunxen force-pushed the 2023-05-v2channelestablish branch from e0c9bbb to a5afd20 Compare June 7, 2024 13:23

dunxen force-pushed the 2023-05-v2channelestablish branch 3 times, most recently from a4730f8 to 6aa5161 Compare June 13, 2024 11:50

dunxen mentioned this pull request Jun 13, 2024

Move accept_channel checks into ChannelContext::do_accept_channel_checks #3123

Merged

dunxen force-pushed the 2023-05-v2channelestablish branch from 6aa5161 to 18ec344 Compare June 13, 2024 13:15

TheBlueMatt reviewed Jun 13, 2024

View reviewed changes

dunxen force-pushed the 2023-05-v2channelestablish branch from 18ec344 to 365aac8 Compare June 14, 2024 09:10

dunxen added 6 commits June 19, 2024 10:54

Add interactive tx constructor to ChannelContext

6451e2b

Implement support for accepting V2 channels

9fafba9

Handle initial commitment_signed for V2 channels

dac491f

Handle interactive signing sessions

7e8c3dd

Handle re-establishment next_funding_txid

8e4879f

Add option_dual_fund feature

9088894

dunxen force-pushed the 2023-05-v2channelestablish branch from 365aac8 to 9088894 Compare June 19, 2024 08:54

dunxen mentioned this pull request Jun 20, 2024

Implement accepting dual-funded channels without contributing #3137

Open

optout21 mentioned this pull request Aug 27, 2024

[Early Draft] Splicing prototype #3274

Draft

dunxen marked this pull request as draft August 27, 2024 11:04

dunxen added the blocked on dependent pr label Aug 27, 2024

optout21 mentioned this pull request Sep 6, 2024

Basic Splicing #3298

Open

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

Loading

codecov-commenter commented May 25, 2023 •

edited

Loading

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

TheBlueMatt left a comment

TheBlueMatt Jun 5, 2024

dunxen Jun 6, 2024

TheBlueMatt Jun 6, 2024

dunxen Jun 7, 2024

TheBlueMatt Jun 5, 2024

dunxen Jun 13, 2024 •

edited

Loading

dunxen commented Jun 7, 2024

TheBlueMatt left a comment

TheBlueMatt Jun 13, 2024

dunxen Jun 13, 2024

TheBlueMatt Jun 13, 2024

dunxen Jun 13, 2024

TheBlueMatt Jun 17, 2024

dunxen Jun 18, 2024 •

edited

Loading

dunxen commented Jun 20, 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 Loading

codecov-commenter commented May 25, 2023 • edited Loading

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

TheBlueMatt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dunxen Jun 13, 2024 • edited Loading

Choose a reason for hiding this comment

dunxen commented Jun 7, 2024

TheBlueMatt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dunxen Jun 18, 2024 • edited Loading

Choose a reason for hiding this comment

dunxen commented Jun 20, 2024

dunxen commented May 16, 2023 •

edited

Loading

codecov-commenter commented May 25, 2023 •

edited

Loading

dunxen Jun 13, 2024 •

edited

Loading

dunxen Jun 18, 2024 •

edited

Loading