Dual funding extension: begin_interactive_funding_tx_construction by optout21 · Pull Request #3443 · lightningdevkit/rust-lightning

optout21 · 2024-12-05T09:10:35Z

Add these two methods to the current dual funding implementation:
begin_interactive_funding_tx_construction()
calculate_change_output_value()

These changes are in the pipeline for dual funding implementation, and needed for dual funding negotiation during splicing, in #3444 . This PR doesn't contain the splicing-specific bits.

Additionally, as a result of a review finding:

From the OutputOwned enum, we have removed the SharedControlFullyOwned option, as Shared can covers it as well. It can cover any balance spit combination, including the case when the full amount is at one counterparty.

dunxen · 2024-12-05T10:53:25Z

Makes sense to get this this in so it can unblock you as #2302 will still take time. Is this much different from what is in #2302/any of the splicing PRs?

optout21 · 2024-12-05T17:56:11Z

~~TODO: I need to run a comparison against current version of dual funding WIP~~

optout21 · 2024-12-06T15:21:59Z

Did the following:

break up maybe_add_funding_change_output() into two: one part for determining if a change needs to be added, and the other to actually add it
moved need_to_add_funding_change_output() into interactivetxs.rs
added unit tests for need_to_add_funding_change_output()

lightning/src/ln/interactivetxs.rs

lightning/src/ln/channel.rs

jkczyz

Could you update the commit message with the rationale for the change? (i.e., specifically how it will be used in splicing)

lightning/src/ln/channel.rs

optout21 · 2024-12-11T16:55:53Z

Review comments addressed, all resolved from my side (left a few open for visibility)

codecov · 2024-12-11T19:46:55Z

Codecov Report

Attention: Patch coverage is 58.33333% with 105 lines in your changes missing coverage. Please review.

Project coverage is 90.43%. Comparing base (830ffa0) to head (f1a8602).
Report is 39 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning/src/ln/channel.rs	11.49%	77 Missing ⚠️
lightning/src/ln/interactivetxs.rs	83.03%	28 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3443      +/-   ##
==========================================
+ Coverage   89.26%   90.43%   +1.16%     
==========================================
  Files         155      155              
  Lines      119968   130672   +10704     
  Branches   119968   130672   +10704     
==========================================
+ Hits       107092   118174   +11082     
+ Misses      10265     9924     -341     
+ Partials     2611     2574      -37

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

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

optout21 · 2025-01-06T10:05:52Z

Rebased.

jkczyz

I'm good with this now. Should squash into three commits, IMO.

main change
removal of SharedControlFullyOwned
bug fix

lightning/src/ln/interactivetxs.rs

optout21 · 2025-03-27T23:51:53Z

Squashed. Kept the SharedControlFullyOwned removal in a separate commit (the proposed 3rd commit is closely related to it, not separate).

lightning/src/ln/channel.rs

lightning/src/ln/interactivetxs.rs

lightning/src/ln/channel.rs

wpaulino · 2025-04-01T21:31:13Z

LGTM after squash

optout21 · 2025-04-02T07:16:34Z

LGTM after squash

Squashed (fix commit into the first one, kept 2nd separate)

lightning/src/ln/interactivetxs.rs

This method is needed by both V2 channel open and splicing. Auxiliary changes: - New method `calculate_change_output_value()` for determining if a change output is needed, and with what value. - In `interactivetxs.rs` adjust the visibility of `SharedOwnedOutput` and `OutputOwned` structs (were not used before). - In `DualFundingChannelContext` add a new field for the counterparty contribution, `their_funding_satoshis`.

The Shared option can cover it as well.

…ific extra input (on top of lightningdevkit#3443) lightningdevkit#3516

optout21 · 2025-04-04T11:57:21Z

'Conversation 155' -- thanks for the patience, @jkczyz & @wpaulino !

…ific extra input (on top of lightningdevkit#3443) lightningdevkit#3516

optout21 requested a review from dunxen December 5, 2024 09:10

This was referenced Dec 5, 2024

[Early Draft][Splicing] Partial, perform interactive transaction negotiation for new funding #3444

Closed

Dual-funded channels and Splicing Project Tracking #1621

Closed

jkczyz self-requested a review December 5, 2024 15:22

optout21 force-pushed the splicing-dual-reqs2 branch 2 times, most recently from 4f7e41b to 0448cf9 Compare December 6, 2024 15:20

optout21 changed the title ~~[Draft] Dual funding extension needed for Splicing, begin_interactive_funding_tx_construction~~ Dual funding extension needed for Splicing, begin_interactive_funding_tx_construction Dec 6, 2024

optout21 marked this pull request as ready for review December 6, 2024 15:23

optout21 changed the title ~~Dual funding extension needed for Splicing, begin_interactive_funding_tx_construction~~ Dual funding extension needed also for splicing: begin_interactive_funding_tx_construction Dec 6, 2024

optout21 force-pushed the splicing-dual-reqs2 branch from 0448cf9 to 0e47819 Compare December 6, 2024 15:43

dunxen reviewed Dec 6, 2024

View reviewed changes

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

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

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

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

optout21 requested a review from dunxen December 6, 2024 16:39

optout21 force-pushed the splicing-dual-reqs2 branch from 48e0518 to 9e44d2b Compare December 6, 2024 16:41

dunxen reviewed Dec 6, 2024

View reviewed changes

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

optout21 force-pushed the splicing-dual-reqs2 branch from 9e44d2b to b7e7a34 Compare December 9, 2024 07:20

optout21 requested a review from dunxen December 9, 2024 07:22

jkczyz reviewed Dec 9, 2024

View reviewed changes

optout21 force-pushed the splicing-dual-reqs2 branch from 70fde13 to b1ae668 Compare December 11, 2024 16:49

optout21 force-pushed the splicing-dual-reqs2 branch from b1ae668 to 86f9680 Compare December 11, 2024 16:56

optout21 requested a review from jkczyz December 11, 2024 16:57

optout21 force-pushed the splicing-dual-reqs2 branch from 86f9680 to a379f1d Compare December 11, 2024 19:37

optout21 force-pushed the splicing-dual-reqs2 branch from a379f1d to 9256a47 Compare December 14, 2024 12:40

optout21 force-pushed the splicing-dual-reqs2 branch from 9256a47 to 5194458 Compare January 6, 2025 09:57

optout21 added the Splicing label Jan 6, 2025

optout21 requested a review from jkczyz March 27, 2025 05:21

jkczyz reviewed Mar 27, 2025

View reviewed changes

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

optout21 force-pushed the splicing-dual-reqs2 branch from fc58dcc to 5fc91e0 Compare March 27, 2025 23:48

jkczyz previously approved these changes Mar 31, 2025

View reviewed changes

wpaulino reviewed Mar 31, 2025

View reviewed changes

optout21 dismissed jkczyz’s stale review via c75bbfb April 1, 2025 04:12

optout21 force-pushed the splicing-dual-reqs2 branch from 5fc91e0 to c75bbfb Compare April 1, 2025 04:12

optout21 requested a review from wpaulino April 1, 2025 04:21

wpaulino removed their request for review April 1, 2025 21:31

optout21 force-pushed the splicing-dual-reqs2 branch from c75bbfb to 65dcc0e Compare April 2, 2025 07:14

optout21 requested review from jkczyz and wpaulino April 2, 2025 07:16

jkczyz previously approved these changes Apr 2, 2025

View reviewed changes

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

optout21 added 2 commits April 2, 2025 17:46

Remove OutputOwned::SharedControlFullyOwned option

f1a8602

The Shared option can cover it as well.

optout21 dismissed jkczyz’s stale review via f1a8602 April 2, 2025 15:47

optout21 force-pushed the splicing-dual-reqs2 branch from 65dcc0e to f1a8602 Compare April 2, 2025 15:47

optout21 added a commit to optout21/rust-lightning that referenced this pull request Apr 2, 2025

Extend begin_interactive_funding_tx_construction() with splicing-spec…

192b773

…ific extra input (on top of lightningdevkit#3443) lightningdevkit#3516

jkczyz approved these changes Apr 2, 2025

View reviewed changes

wpaulino approved these changes Apr 2, 2025

View reviewed changes

wpaulino merged commit 31c97f4 into lightningdevkit:main Apr 2, 2025
25 of 27 checks passed

optout21 added a commit to optout21/rust-lightning that referenced this pull request Apr 3, 2025

Extend begin_interactive_funding_tx_construction() with splicing-spec…

7a0479c

…ific extra input (on top of lightningdevkit#3443) lightningdevkit#3516

optout21 added a commit to optout21/rust-lightning that referenced this pull request Apr 3, 2025

Extend begin_interactive_funding_tx_construction() with splicing-spec…

90f3723

…ific extra input (on top of lightningdevkit#3443) lightningdevkit#3516

optout21 added a commit to optout21/rust-lightning that referenced this pull request Apr 4, 2025

Extend begin_interactive_funding_tx_construction() with splicing-spec…

1fe60f1

…ific extra input (on top of lightningdevkit#3443) lightningdevkit#3516

optout21 added a commit to optout21/rust-lightning that referenced this pull request Apr 4, 2025

Extend begin_interactive_funding_tx_construction() with splicing-spec…

3e99e27

…ific extra input (on top of lightningdevkit#3443) lightningdevkit#3516

optout21 added a commit to optout21/rust-lightning that referenced this pull request Apr 7, 2025

Extend begin_interactive_funding_tx_construction() with splicing-spec…

b33fe68

…ific extra input (on top of lightningdevkit#3443) lightningdevkit#3516

Conversation

optout21 commented Dec 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dunxen commented Dec 5, 2024

Uh oh!

optout21 commented Dec 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

optout21 commented Dec 6, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jkczyz left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

Uh oh!

optout21 commented Dec 11, 2024

Uh oh!

codecov bot commented Dec 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

optout21 commented Jan 6, 2025

Uh oh!

jkczyz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

optout21 commented Mar 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wpaulino commented Apr 1, 2025

Uh oh!

optout21 commented Apr 2, 2025

Uh oh!

Uh oh!

Uh oh!

optout21 commented Apr 4, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

optout21 commented Dec 5, 2024 •

edited

Loading

optout21 commented Dec 5, 2024 •

edited

Loading

jkczyz left a comment •

edited

Loading

codecov bot commented Dec 11, 2024 •

edited

Loading

optout21 commented Mar 27, 2025 •

edited

Loading