Introduce `FundingTransactionReadyForSignatures` event #3889

dunxen · 2025-06-24T12:12:50Z

Cherry-picked from #3735 as it is relevant to splicing and will unblock testing after #3736 lands.

The FundingTransactionReadyForSignatures event requests witnesses from the client for their contributed inputs to an interactively constructed transaction.

The client calls ChannelManager::funding_transaction_signed to provide the witnesses to LDK.

ldk-reviews-bot · 2025-06-24T12:12: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/events/mod.rs

lightning/src/ln/channelmanager.rs

lightning/src/ln/interactivetxs.rs

codecov · 2025-06-25T11:08:53Z

Codecov Report

❌ Patch coverage is 75.91241% with 132 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.88%. Comparing base (0a23664) to head (59c763e).
⚠️ Report is 13 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning/src/ln/channelmanager.rs	35.05%	63 Missing ⚠️
lightning/src/ln/interactivetxs.rs	86.48%	44 Missing and 11 partials ⚠️
lightning/src/ln/channel.rs	68.18%	8 Missing and 6 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3889      +/-   ##
==========================================
- Coverage   89.01%   88.88%   -0.14%     
==========================================
  Files         174      174              
  Lines      124395   125030     +635     
  Branches   124395   125030     +635     
==========================================
+ Hits       110730   111132     +402     
- Misses      11187    11396     +209     
- Partials     2478     2502      +24

Flag	Coverage Δ
fuzzing	`22.09% <3.96%> (-0.13%)`	⬇️
tests	`88.71% <75.91%> (-0.13%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

ldk-reviews-bot · 2025-06-26T12:13:27Z

🔔 1st Reminder

Hey @wpaulino @optout21! 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-26T12:13:29Z

🔔 1st Reminder

Hey @wpaulino @optout21! 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

Looks good!

lightning/src/ln/channel.rs

ldk-reviews-bot · 2025-06-28T12:14:21Z

🔔 2nd Reminder

Hey @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-30T12:15:10Z

🔔 3rd Reminder

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

jkczyz

Not sure if there was a problem rebasing, but some comments that had been marked resolved weren't fixed.

lightning/src/events/mod.rs

lightning/src/ln/channelmanager.rs

lightning/src/events/mod.rs

dunxen · 2025-07-01T08:08:04Z

Not sure if there was a problem rebasing, but some comments that had been marked resolved weren't fixed.

Yeah, they got lost on a rebase and somehow lost the commit. Rebased to get the one CI fix in. Fixing.

ldk-reviews-bot · 2025-07-02T12:15:51Z

🔔 4th Reminder

Hey @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-07-05T00:01:03Z

🔔 5th Reminder

Hey @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-07-07T00:01:47Z

🔔 6th Reminder

Hey @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/events/mod.rs

lightning/src/ln/channel.rs

lightning/src/ln/channelmanager.rs

dunxen

Ok, it should be ready now.

lightning/src/ln/channel.rs

lightning/src/ln/channelmanager.rs

lightning/src/ln/interactivetxs.rs

lightning/src/ln/channelmanager.rs

lightning/src/ln/channel.rs

ldk-reviews-bot · 2025-08-07T08:38:59Z

🔔 1st Reminder

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

jkczyz

No major concerns. Happy to leave these to a follow-up if @wpaulino is good.

lightning/src/ln/channelmanager.rs

jkczyz · 2025-08-07T15:09:10Z

lightning/src/ln/channel.rs

+			self.funding.funding_transaction = funding_tx_opt.clone();
+			self.context.channel_state =
+				ChannelState::AwaitingChannelReady(AwaitingChannelReadyFlags::new());


Could you add a TODO(splicing) to use the pending FundingScope and set the channel_state back to ChannelReady?

I'm going to fix it in the new few days anyway so I wouldn't bother

lightning/src/ln/channel.rs

wpaulino · 2025-08-07T23:15:10Z

lightning/src/ln/channel.rs

+		}
+
+		if funding_tx_opt.is_some() {
+			self.funding.funding_transaction = funding_tx_opt.clone();


We don't need to clone, we can consume it here and return a reference back

If we return the shared reference Option<&Transaction> from FundedChannel's funding.funding_transaction back then we'll have it exist at the same time as the exclusive reference &mut FundedChannel at the call site which isn't legal.

Since we'd have:

match channel.funding_transaction_signed(vec![]) { Ok((Some(tx_signatures), funding_tx_opt)) => { if let Some(funding_tx) = funding_tx_opt { // funding_tx is &Transaction from FundedChannel and channel is &mut FundedChannel self.broadcast_interactive_funding(channel, funding_tx); } pending_msg_events.push(MessageSendEvent::SendTxSignatures { node_id: counterparty_node_id, msg: tx_signatures, }); }, ... }

wpaulino · 2025-08-07T23:17:24Z

lightning/src/ln/channel.rs

+			self.funding.funding_transaction = funding_tx_opt.clone();
+			self.context.channel_state =
+				ChannelState::AwaitingChannelReady(AwaitingChannelReadyFlags::new());


I'm going to fix it in the new few days anyway so I wouldn't bother

lightning/src/ln/channel.rs

lightning/src/ln/interactivetxs.rs

dunxen

Just a comment on the clone avoidance.

dunxen · 2025-08-08T12:01:09Z

lightning/src/ln/channel.rs

+		}
+
+		if funding_tx_opt.is_some() {
+			self.funding.funding_transaction = funding_tx_opt.clone();


If we return the shared reference Option<&Transaction> from FundedChannel's funding.funding_transaction back then we'll have it exist at the same time as the exclusive reference &mut FundedChannel at the call site which isn't legal.

Since we'd have:

match channel.funding_transaction_signed(vec![]) { Ok((Some(tx_signatures), funding_tx_opt)) => { if let Some(funding_tx) = funding_tx_opt { // funding_tx is &Transaction from FundedChannel and channel is &mut FundedChannel self.broadcast_interactive_funding(channel, funding_tx); } pending_msg_events.push(MessageSendEvent::SendTxSignatures { node_id: counterparty_node_id, msg: tx_signatures, }); }, ... }

The `FundingTransactionReadyForSignatures` event requests witnesses from the client for their contributed inputs to an interactively constructed transaction. The client calls `ChannelManager::funding_transaction_signed` to provide the witnesses to LDK. The `handle_channel_resumption` method handles resumption from both a channel re-establish and a monitor update. When the corresponding monitor update for the commitment_signed message completes, we will push the event here. We can thus only ever provide holder signatures after a monitor update has completed. We can also get rid of the reestablish code involved with `monitor_pending_tx_signatures` and remove that field too.

…hecks In a following commit, We'll use the contained scriptPubKeys to validate P2WPKH and P2TR key path spends and to assist in checking that signatures in provided holder witnesses use SIGHASH_ALL to prevent funds being frozen or held ransom.

LDK checks the following: * Each input spends an output that is one of P2WPKH, P2WSH, or P2TR. These were already checked by LDK when the inputs to be contributed were provided. * All signatures use the `SIGHASH_ALL` sighash type. * P2WPKH and P2TR key path spends are valid (verifies signatures) NOTE: * When checking P2WSH spends, LDK tries to decode 70-72 byte witness elements as ECDSA signatures with a sighash flag. If the internal DER-decoding fails, then LDK just assumes it wasn't a signature and carries with checks. If the element can be decoded as an ECDSA signature, the the sighash flag must be `SIGHASH_ALL`. * When checking P2TR script-path spends, LDK assumes all elements of exactly 65 bytes with the last byte matching any valid sighash flag byte are schnorr signatures and checks that the sighash type is `SIGHASH_ALL`. If the last byte is not any valid sighash flag, the element is assumed not to be a signature and is ignored. Elements of 64 bytes are not checked because if they were schnorr signatures then they would implicitly be `SIGHASH_DEFAULT` which is an alias of `SIGHASH_ALL`.

dunxen · 2025-08-08T19:25:52Z

Rebased to get #3997 in and CI passing

jkczyz

LGTM. Let's handle any nits in a follow-up so we don't need to wait on CI.

jkczyz · 2025-08-08T22:30:30Z

lightning/src/ln/interactivetxs.rs

@@ -516,6 +537,161 @@ impl InteractiveTxSigningSession {
 			output: outputs.iter().cloned().map(|output| output.into_tx_out()).collect(),
 		}
 	}
+
+	pub fn verify_interactive_tx_signatures(


nit: no longer needs to be pub.

dunxen added Dual-funding Splicing labels Jun 24, 2025

dunxen requested review from wpaulino, optout21 and jkczyz June 24, 2025 12:13

jkczyz reviewed Jun 24, 2025

View reviewed changes

dunxen mentioned this pull request Jun 25, 2025

Contribute funding inputs on accepting dual-funded channel #3735

Draft

dunxen force-pushed the 2025-06-readyforsigningevent branch from 165d59d to 8ca6d79 Compare June 25, 2025 10:59

optout21 reviewed Jun 26, 2025

View reviewed changes

dunxen force-pushed the 2025-06-readyforsigningevent branch from 890633d to a1de384 Compare June 27, 2025 07:20

jkczyz reviewed Jun 27, 2025

View reviewed changes

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

dunxen force-pushed the 2025-06-readyforsigningevent branch 2 times, most recently from 7df5779 to c8f981c Compare June 30, 2025 10:29

jkczyz reviewed Jun 30, 2025

View reviewed changes

dunxen force-pushed the 2025-06-readyforsigningevent branch 2 times, most recently from c15f426 to ff1489d Compare July 1, 2025 09:18

dunxen force-pushed the 2025-06-readyforsigningevent branch from ff1489d to 0a586e6 Compare July 4, 2025 07:05

wpaulino reviewed Jul 7, 2025

View reviewed changes

dunxen force-pushed the 2025-06-readyforsigningevent branch 2 times, most recently from a9e1a3a to 83e78d6 Compare July 14, 2025 09:34

dunxen force-pushed the 2025-06-readyforsigningevent branch from fdf8e21 to 1fe02a7 Compare August 4, 2025 11:39

wpaulino reviewed Aug 4, 2025

View reviewed changes

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

wpaulino reviewed Aug 4, 2025

View reviewed changes

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

dunxen force-pushed the 2025-06-readyforsigningevent branch 2 times, most recently from fa4b4b1 to 60e8c7d Compare August 5, 2025 08:38

dunxen requested review from jkczyz and wpaulino August 5, 2025 08:38

dunxen force-pushed the 2025-06-readyforsigningevent branch from 60e8c7d to 91ce15f Compare August 5, 2025 08:40

dunxen commented Aug 5, 2025

View reviewed changes

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

wpaulino reviewed Aug 5, 2025

View reviewed changes

dunxen force-pushed the 2025-06-readyforsigningevent branch from 91ce15f to 04e66f1 Compare August 6, 2025 10:11

dunxen requested a review from wpaulino August 6, 2025 10:13

dunxen force-pushed the 2025-06-readyforsigningevent branch 2 times, most recently from 381a990 to abba0a7 Compare August 6, 2025 17:09

wpaulino reviewed Aug 7, 2025

View reviewed changes

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

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

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

dunxen force-pushed the 2025-06-readyforsigningevent branch from abba0a7 to c91e213 Compare August 7, 2025 08:03

dunxen force-pushed the 2025-06-readyforsigningevent branch from c91e213 to 882c313 Compare August 7, 2025 08:54

dunxen requested a review from wpaulino August 7, 2025 10:36

jkczyz reviewed Aug 7, 2025

View reviewed changes

wpaulino reviewed Aug 7, 2025

View reviewed changes

dunxen commented Aug 8, 2025

View reviewed changes

dunxen force-pushed the 2025-06-readyforsigningevent branch from 882c313 to 59c763e Compare August 8, 2025 12:48

dunxen added 3 commits August 8, 2025 21:23

dunxen force-pushed the 2025-06-readyforsigningevent branch from 59c763e to 9cd4091 Compare August 8, 2025 19:24

jkczyz approved these changes Aug 8, 2025

View reviewed changes

wpaulino approved these changes Aug 8, 2025

View reviewed changes

Introduce FundingTransactionReadyForSignatures event #3889

Are you sure you want to change the base?

Introduce FundingTransactionReadyForSignatures event #3889

Conversation

dunxen commented Jun 24, 2025

Uh oh!

ldk-reviews-bot commented Jun 24, 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!

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

ldk-reviews-bot commented Jun 26, 2025

Uh oh!

ldk-reviews-bot commented Jun 26, 2025

Uh oh!

optout21 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ldk-reviews-bot commented Jun 28, 2025

Uh oh!

ldk-reviews-bot commented Jun 30, 2025

Uh oh!

jkczyz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dunxen commented Jul 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ldk-reviews-bot commented Jul 2, 2025

Uh oh!

ldk-reviews-bot commented Jul 5, 2025

Uh oh!

ldk-reviews-bot commented Jul 7, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dunxen 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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ldk-reviews-bot commented Aug 7, 2025

Uh oh!

jkczyz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jkczyz Aug 7, 2025

Introduce `FundingTransactionReadyForSignatures` event #3889

Introduce `FundingTransactionReadyForSignatures` event #3889

ldk-reviews-bot commented Jun 24, 2025 •

edited

Loading

codecov bot commented Jun 25, 2025 •

edited

Loading

dunxen commented Jul 1, 2025 •

edited

Loading