Channel Establishment for V3 Channels #3792

carlaKC · 2025-05-22T15:43:09Z

This PR updates the channel establishment flow to allow and validate V3 channels (behind test flag).

Useful for the commits that follow where we add more downgrade tests.

ldk-reviews-bot · 2025-05-22T15:43:13Z

👋 Thanks for assigning @tankyleo as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

TheBlueMatt

Cool, basically all LGTM.

TheBlueMatt · 2025-05-22T18:02:13Z

lightning/src/ln/channelmanager.rs

@@ -16153,22 +16154,28 @@ mod tests {
 	}

 	#[test]
-	fn test_anchors_zero_fee_htlc_tx_fallback() {
+	fn test_anchors_zero_fee_htlc_tx_downgrade() {


Congrats, you touched it, now you get to move it out of channelmanager into some other test-specific file that isn't 15000 lines of code 😂

Done this in a follow up in #3797 so that move + format can be reviewed separately.

TheBlueMatt · 2025-05-22T18:04:58Z

lightning/src/util/config.rs

+	/// back to a `anchors_zero_fee_htlc` (if [`Self::negotiate_anchors_zero_fee_htlc_tx`]
+	/// is set) or `static_remote_key` channel.
+	///
+	/// *Implies [`Self::negotiate_anchors_zero_fee_htlc_tx`].*


These are in conflict - one says we'll fall back if its set, the other says that its implied (ie always set) if we set this flag.

ldk-reviews-bot · 2025-05-22T18:13:18Z

👋 The first review has been submitted!

Do you think this PR is ready for a second reviewer? If so, click here to assign a second reviewer.

ldk-reviews-bot · 2025-05-22T18:13:31Z

✅ Added second reviewer: @valentinewallace

wpaulino

LGTM after Matt's comments are addressed

carlaKC · 2025-05-23T18:36:06Z

Removed conflicting docs statement + opened followup for test separation (felt wrong to do move+format in the same PR, happy to include if we want it in here).

ldk-reviews-bot · 2025-05-26T00:00:59Z

🔔 1st Reminder

Hey @TheBlueMatt @wpaulino! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2025-05-26T00:01:04Z

🔔 1st Reminder

Hey @TheBlueMatt @wpaulino! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

tankyleo

Sorry for the delay, here's a first pass :)

lightning/src/ln/channel.rs

lightning/src/ln/channelmanager.rs

lightning/src/ln/channel.rs

ldk-reviews-bot · 2025-05-28T00:01:29Z

🔔 2nd Reminder

Hey @TheBlueMatt @wpaulino! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2025-05-28T00:01:39Z

🔔 2nd Reminder

Hey @TheBlueMatt @wpaulino! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

tankyleo

Another pass :)

lightning/src/ln/channel.rs

lightning/src/ln/channelmanager.rs

lightning/src/ln/channel.rs

lightning/src/util/config.rs

lightning/src/ln/channel.rs

lightning/src/ln/channelmanager.rs

Rather than duplicating our channel type preference ordering in downgrade logic, make a modified version of the remote peer's supported features and remove our current channel type from it to get the next preferred channel type.

carlaKC · 2025-05-29T15:53:46Z

Major change in push is using get_initial_channel_type in channel type downgrades to DRY up the code a bit.
Otherwise addressed nits + added some tests in fixups (full diff).

tankyleo

Thank you for the downgrade tests ! Just three nits at this point :)

lightning/src/ln/channel.rs

lightning/src/ln/channelmanager.rs

tankyleo · 2025-05-29T19:33:57Z

lightning/src/ln/channelmanager.rs

+		initiator_cfg .channel_handshake_config.negotiate_anchor_zero_fee_commitments = true;
+		initiator_cfg .channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = true;
+
+		let mut receiver_cfg = test_default_channel_config();
+		receiver_cfg.channel_handshake_config.negotiate_anchor_zero_fee_commitments = true;
+		receiver_cfg.manually_accept_inbound_channels = true;


Do we also want to run a version of this test where local does not support zero fee htlc (instead of remote) ?

I might have been unclear, I meant a diff similar to this one ? Or do you think it's not necessary ?

Thank you thank you :)

let start_type = ChannelTypeFeatures::anchors_zero_fee_commitments(); let end_type = ChannelTypeFeatures::only_static_remote_key(); do_test_channel_type_downgrade(initiator_cfg, receiver_cfg, start_type, vec![end_type]); + + let mut initiator_cfg = test_default_channel_config(); + initiator_cfg.channel_handshake_config.negotiate_anchor_zero_fee_commitments = true; + + let mut receiver_cfg = test_default_channel_config(); + receiver_cfg.channel_handshake_config.negotiate_anchor_zero_fee_commitments = true; + receiver_cfg.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = true; + receiver_cfg.manually_accept_inbound_channels = true; + + let start_type = ChannelTypeFeatures::anchors_zero_fee_commitments(); + let end_type = ChannelTypeFeatures::only_static_remote_key(); + do_test_channel_type_downgrade(initiator_cfg, receiver_cfg, start_type, vec![end_type]); }

To allow testing along the way in this PR, turn on negotiation of zero fee channels. Co-authored-by: Matt Corallo <git@bluematt.me>

Sender: MUST set `feerate_per_kw` to zero Receiver: MUST fail the channel if `feerate_per_kw` != 0 Co-authored-by: Matt Corallo <git@bluematt.me>

Like anchor channels, these channels require that the user reserves a UTXO to bump the channel. If we automatically accept this channel type and the user does not have such reserve available, they are at risk of losing funds because they cannot fee bump the channel.

carlaKC · 2025-05-30T20:26:33Z

Added test + fixed minor nit.

ldk-reviews-bot · 2025-05-31T00:00:17Z

🔔 3rd Reminder

Hey @TheBlueMatt @wpaulino! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2025-05-31T00:00:25Z

🔔 3rd Reminder

Hey @TheBlueMatt @wpaulino! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2025-06-02T00:01:32Z

🔔 4th Reminder

Hey @TheBlueMatt @wpaulino! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2025-06-02T00:01:39Z

🔔 4th Reminder

Hey @TheBlueMatt @wpaulino! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ln/refactor: add helper test function for channel type downgrade

a3a4af9

Useful for the commits that follow where we add more downgrade tests.

carlaKC requested a review from TheBlueMatt May 22, 2025 15:43

carlaKC mentioned this pull request May 22, 2025

Zero Fee Commitments #3789

Open

36 tasks

TheBlueMatt reviewed May 22, 2025

View reviewed changes

ldk-reviews-bot requested a review from valentinewallace May 22, 2025 18:13

wpaulino requested review from wpaulino and removed request for valentinewallace May 22, 2025 18:17

wpaulino reviewed May 23, 2025

View reviewed changes

carlaKC force-pushed the 3789-channelestablishment branch from 3a36506 to b9ec89c Compare May 23, 2025 18:30

carlaKC mentioned this pull request May 23, 2025

rustfmt: move channel acceptance tests into own file and format #3797

Open

carlaKC requested review from wpaulino and TheBlueMatt May 23, 2025 20:54

tankyleo reviewed May 27, 2025

View reviewed changes

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

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

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

tankyleo reviewed May 28, 2025

View reviewed changes

carlaKC added 4 commits May 28, 2025 09:16

f Add ability to assert on downgraded type in test helper

93dbf0a

f Allow test helper to use separate config for channel opener/recipient

93df503

f Allow channel downgrade tests to cover several iterations

7077f5c

carlaKC force-pushed the 3789-channelestablishment branch from b9ec89c to 892d87a Compare May 29, 2025 15:50

carlaKC requested a review from tankyleo May 29, 2025 15:53

tankyleo reviewed May 29, 2025

View reviewed changes

carlaKC and others added 15 commits May 30, 2025 15:43

f Add coverage for no channel downgrade rejection

147877a

ln+util: add test option to enable negotiation of zero fee channels

8aa4c23

To allow testing along the way in this PR, turn on negotiation of zero fee channels. Co-authored-by: Matt Corallo <git@bluematt.me>

f Update wording in static remote key assumed comment + style fixup

b5612b7

f Remove unnecessary indentation for cfg[test]

bd5facf

f Add test coverage for skipping anchors when not negotiated

6fe04ad

f Add assertion that we can't downgrade to anchor_zero_fee_commitments

4db1ffd

f Remove unnecessary spaces

cdf060d

ln/refactor: add helper function for channel type tests

17c88ef

f Stricter assertion on default channel type in test

6ce402a

ln: do not set or accept non-zero feerates in open_channel for v3

ac648a5

Sender: MUST set `feerate_per_kw` to zero Receiver: MUST fail the channel if `feerate_per_kw` != 0 Co-authored-by: Matt Corallo <git@bluematt.me>

f Add coverage for both anchor types supported, zero fee commits chosen

dc84c74

ln/refactor: add helper function for manual acceptance tests

f0ed7da

f Standardize naming of manual acceptance test for zero fee comms

15d1c5c

ln: set max_htlcs based on the channel type

e56e32a

carlaKC force-pushed the 3789-channelestablishment branch from 892d87a to e56e32a Compare May 30, 2025 20:23

Channel Establishment for V3 Channels #3792

Are you sure you want to change the base?

Channel Establishment for V3 Channels #3792

Uh oh!

Conversation

carlaKC commented May 22, 2025

Uh oh!

ldk-reviews-bot commented May 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TheBlueMatt left a comment

Choose a reason for hiding this comment

Uh oh!

TheBlueMatt May 22, 2025

Choose a reason for hiding this comment

Uh oh!

carlaKC May 23, 2025

Choose a reason for hiding this comment

Uh oh!

TheBlueMatt May 22, 2025

Choose a reason for hiding this comment

Uh oh!

ldk-reviews-bot commented May 22, 2025

Uh oh!

ldk-reviews-bot commented May 22, 2025

Uh oh!

wpaulino left a comment

Choose a reason for hiding this comment

Uh oh!

carlaKC commented May 23, 2025

Uh oh!

ldk-reviews-bot commented May 26, 2025

Uh oh!

ldk-reviews-bot commented May 26, 2025

Uh oh!

tankyleo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ldk-reviews-bot commented May 28, 2025

Uh oh!

ldk-reviews-bot commented May 28, 2025

Uh oh!

tankyleo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

carlaKC commented May 29, 2025

Uh oh!

tankyleo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

tankyleo May 29, 2025

Choose a reason for hiding this comment

Uh oh!

tankyleo May 30, 2025

Choose a reason for hiding this comment

Uh oh!

carlaKC commented May 30, 2025

Uh oh!

ldk-reviews-bot commented May 31, 2025

Uh oh!

ldk-reviews-bot commented May 31, 2025

Uh oh!

ldk-reviews-bot commented Jun 2, 2025

Uh oh!

ldk-reviews-bot commented Jun 2, 2025

Uh oh!

Uh oh!

ldk-reviews-bot commented May 22, 2025 •

edited

Loading