Add Shared Input support in interactive TX construction #3842

optout21 · 2025-06-10T15:40:49Z

In interactive TX construction, add support for shared input:

if we expect a shared input, make sure that the remote node provides it
if we provide a shared input, make sure that it's accounted appropriately

Additionally, the prevtx field of the TxAddInput message is changed to Optional, as it should not be set for the shared input (it cannot, as the full funding transaction is not stored on the acceptor side) (spec discussion: lightning/bolts#1160 (comment))

To be used by splicing, see #3736 .

Note: this PR does not include splicing negotiation.

ldk-reviews-bot · 2025-06-10T15:40:52Z

👋 Thanks for assigning @wpaulino 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.

lightning/src/ln/msgs.rs

wpaulino · 2025-06-11T23:35:30Z

lightning/src/ln/msgs.rs

-	/// malleable.
-	pub prevtx: TransactionU16LenLimited,
+	/// malleable. Omitted for shared input.
+	pub prevtx: Option<TransactionU16LenLimited>,


We could make this an enum along with shared_input_txid, but I'm not sure what's a good name to call it

Probably not a good suggestion, but how about just TxInputOrigin with Txid and PrevTx variants?

I think it's a good idea to keep the message struct field names close to the names in the spec.

Discussed this offline with @wpaulino. While ideally we could use an enum to represent the correct semantics, in practice we don't want to fail parsing if they are incorrect. Otherwise, we'd disconnect rather than simply abandoning the negotiation.

lightning/src/ln/interactivetxs.rs

ldk-reviews-bot · 2025-06-14T00:00:19Z

🔔 1st Reminder

Hey @dunxen @jkczyz! 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-14T00:00:21Z

🔔 1st Reminder

Hey @dunxen @jkczyz! 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-16T00:01:15Z

🔔 2nd Reminder

Hey @dunxen @jkczyz! 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-16T00:01:16Z

🔔 2nd Reminder

Hey @dunxen @jkczyz! 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.

optout21 · 2025-06-16T12:45:46Z

Great, can you push those changes here then and rework the shared funding input work to follow a similar approach?

I managed to do it: first I refactored the shared output support as discussed, then added shared input support in a similar style. Although this PR does not include spicing, I also checked the changes with splicing (shared input is used there).

lightning/src/ln/interactivetxs.rs

lightning/src/ln/channel.rs

lightning/src/ln/interactivetxs.rs

wpaulino · 2025-06-16T18:16:44Z

lightning/src/ln/interactivetxs.rs

+		total_input_satoshis = total_input_satoshis.saturating_add(shared_input);
+		if is_initiator {
+			our_funding_inputs_weight =
+				our_funding_inputs_weight.saturating_add(P2WSH_INPUT_WEIGHT_LOWER_BOUND);


This needs to be the weight of the full witness (see FUNDING_TRANSACTION_WITNESS_WEIGHT) plus the base input weight

That's included in P2WPKH_INPUT_WEIGHT_LOWER_BOUND, I think:

/// Lower bound for P2WPKH input weight pub(crate) const P2WPKH_INPUT_WEIGHT_LOWER_BOUND: u64 = BASE_INPUT_WEIGHT + EMPTY_SCRIPT_SIG_WEIGHT + P2WPKH_WITNESS_WEIGHT;

wpaulino · 2025-06-16T18:20:12Z

lightning/src/ln/interactivetxs.rs

+	// If there is a shared input, account for it,
+	// and for the initiator also consider the fee
+	if let Some(shared_input) = shared_input {
+		total_input_satoshis = total_input_satoshis.saturating_add(shared_input);


Is this supposed to be the full channel value of the shared input, or just the locally owned portion of it?

The locally owned portion, as this calculation is for ensuring that the contribution is enough to cover the (locally owned) outputs and fees.

I have some doubts about the possible rounding errors due to the msat->sat conversion.

optout21 · 2025-06-17T03:49:12Z

@wpaulino thanks for the comments. I will process them (I've re-requested review accidentally, please ignore that).

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

🔔 3rd Reminder

Hey @dunxen @jkczyz @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-18T00:01:40Z

🔔 3rd Reminder

Hey @dunxen @jkczyz @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-19T03:11:00Z

🔔 1st Reminder

Hey @dunxen @jkczyz @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.

lightning/src/ln/msgs.rs

This simplifies tracking separately the expected and actual shared output. In the initiator case, we can just provide the shared output separately, instead of including it within other outputs, and marking which one is the output. We can use the same field for the intended shared output in the initiator case, and the expected one in the acceptor case.

codecov · 2025-06-19T08:28:59Z

Codecov Report

Attention: Patch coverage is 86.65710% with 93 lines in your changes missing coverage. Please review.

Project coverage is 90.92%. Comparing base (30ab411) to head (96236df).
Report is 58 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning/src/ln/interactivetxs.rs	91.99%	41 Missing and 8 partials ⚠️
lightning/src/ln/msgs.rs	40.00%	34 Missing and 8 partials ⚠️
lightning/src/ln/channel.rs	81.81%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3842      +/-   ##
==========================================
+ Coverage   89.95%   90.92%   +0.97%     
==========================================
  Files         164      164              
  Lines      131981   142768   +10787     
  Branches   131981   142768   +10787     
==========================================
+ Hits       118718   129812   +11094     
+ Misses      10586    10350     -236     
+ Partials     2677     2606      -71

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

lightning/src/ln/interactivetxs.rs

Co-authored-by: graphite-app[bot] <96075541+graphite-app[bot]@users.noreply.github.com>

optout21 · 2025-06-19T14:47:16Z

Review comments addressed. Keeping the fixup commits separately for now, but in the end I will just squash into a single commit.

optout21 mentioned this pull request Jun 10, 2025

[Splicing] Tx negotiation during splicing #3736

Open

optout21 force-pushed the shared-input branch from 660dd6b to f63c8a4 Compare June 11, 2025 18:53

optout21 marked this pull request as ready for review June 11, 2025 19:03

optout21 requested review from jkczyz, wpaulino and dunxen June 11, 2025 19:03

wpaulino reviewed Jun 12, 2025

View reviewed changes

optout21 force-pushed the shared-input branch from f63c8a4 to 9151a72 Compare June 16, 2025 12:43

optout21 force-pushed the shared-input branch 5 times, most recently from 166e364 to b2e0da8 Compare June 16, 2025 13:47

wpaulino reviewed Jun 16, 2025

View reviewed changes

optout21 requested a review from wpaulino June 17, 2025 03:10

optout21 force-pushed the shared-input branch from 967dfa3 to 5fb333d Compare June 19, 2025 08:12

graphite-app bot reviewed Jun 19, 2025

View reviewed changes

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

optout21 force-pushed the shared-input branch from 5fb333d to 151a2ec Compare June 19, 2025 08:19

graphite-app bot reviewed Jun 19, 2025

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 Outdated Show resolved Hide resolved

optout21 added 2 commits June 19, 2025 12:52

fix Use SharedOwnedOutput for shared_funding_output

c96b44f

Add Shared Input support

31416e8

optout21 force-pushed the shared-input branch from cafc821 to 96236df Compare June 19, 2025 10:59

optout21 added 2 commits June 19, 2025 13:55

fix Make transaction field optional in SingleOwnedInput

51e1e7c

fix Define serialization on Option<TransactionU16LenLimited>

fa69c27

optout21 force-pushed the shared-input branch from 96236df to 21e5e36 Compare June 19, 2025 11:57

optout21 added 3 commits June 19, 2025 14:23

fix Minor changes, error enum, comments

356bd80

fix Include vout check as well in OutPoint check for shared input

e938971

fix Don’t carry the tx after input received

bb322a8

optout21 force-pushed the shared-input branch from 21e5e36 to bb322a8 Compare June 19, 2025 12:24

graphite-app bot reviewed Jun 19, 2025

View reviewed changes

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

optout21 and others added 4 commits June 19, 2025 15:50

fix Adjust error code MissingPrevTx

eeaa9b7

Co-authored-by: graphite-app[bot] <96075541+graphite-app[bot]@users.noreply.github.com>

fix SharedOwnedInput has no prev_output

7e2b4ba

fix Use SharedOwnedInput for shared_funding_input

5f5e3b7

fix Simplify prev txout existence check

2560fce

Add Shared Input support in interactive TX construction #3842

Are you sure you want to change the base?

Add Shared Input support in interactive TX construction #3842

Conversation

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

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ldk-reviews-bot commented Jun 14, 2025

Uh oh!

ldk-reviews-bot commented Jun 14, 2025

Uh oh!

ldk-reviews-bot commented Jun 16, 2025

Uh oh!

ldk-reviews-bot commented Jun 16, 2025

Uh oh!

optout21 commented Jun 16, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

optout21 commented Jun 17, 2025

Uh oh!

ldk-reviews-bot commented Jun 18, 2025

Uh oh!

ldk-reviews-bot commented Jun 18, 2025

Uh oh!

ldk-reviews-bot commented Jun 19, 2025

Uh oh!

Uh oh!

codecov bot commented Jun 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

optout21 commented Jun 19, 2025

Uh oh!

Uh oh!

optout21 commented Jun 10, 2025 •

edited

Loading

ldk-reviews-bot commented Jun 10, 2025 •

edited

Loading

codecov bot commented Jun 19, 2025 •

edited

Loading