Release tx_signatures after async monitor update completes by wpaulino · Pull Request #4472 · lightningdevkit/rust-lightning

wpaulino · 2026-03-09T18:04:31Z

In 83b2d3e, we reworked ChannelManager::funding_transaction_signed such that it would also for a user to cancel a splice up until they send commitment_signed. Previously, we would would only emit Event::FundingTransactionReadyForSigning when both nodes exchanged commitment_signed and the corresponding monitor update completed. With the event now being generated immediately after the nodes exchange tx_complete, we now need to handle the monitor update not having completed by the time we are ready to send tx_signatures. Unfortunately, we also did not have test coverage, allowing this to go unnoticed until being caught by the fuzzer due to a debug assertion. Doing so avoids a potential funds-loss scenario if the funding transaction confirms without the counterparty's signature for our commitment being durably persisted.

ldk-reviews-bot · 2026-03-09T18:04:33Z

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

codecov · 2026-03-09T20:49:14Z

Codecov Report

❌ Patch coverage is 80.95238% with 32 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.18%. Comparing base (066859b) to head (ea204f6).
⚠️ Report is 2767 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning/src/ln/channel.rs	78.84%	19 Missing and 3 partials ⚠️
lightning/src/ln/channelmanager.rs	80.00%	6 Missing and 4 partials ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #4472 +/- ##
==========================================
+ Coverage 84.55% 86.18% +1.62% 
==========================================
Files 135 161 +26 Lines 76569 107532 +30963 Branches 76569 107532 +30963 ==========================================
+ Hits 64745 92679 +27934 - Misses 9783 12233 +2450 - Partials 2041 2620 +579

Flag	Coverage Δ
tests	`86.18% <80.95%> (?)`

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 · 2026-03-11T18:05:00Z

🔔 1st Reminder

Hey @TheBlueMatt! 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 · 2026-03-14T00:01:35Z

🔔 2nd Reminder

Hey @TheBlueMatt! 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-claude-review-bot · 2026-03-14T21:15:48Z

ldk-reviews-bot · 2026-03-16T00:02:44Z

🔔 3rd Reminder

Hey @TheBlueMatt! 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 · 2026-03-16T17:43:42Z

✅ Added second reviewer: @tankyleo

TheBlueMatt

Aside from what claude noted I didn't see any issues.

TheBlueMatt · 2026-03-17T00:49:19Z

Needs rebase now.

ldk-reviews-bot · 2026-03-18T18:08:18Z

🔔 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

Looks good though consider these comments to encapsulate some logic in InteractiveTxSigningSession.

In 83b2d3e, we reworked `ChannelManager::funding_transaction_signed` such that it would also for a user to cancel a splice up until they send `commitment_signed`. Previously, we would would only emit `Event::FundingTransactionReadyForSigning` when both nodes exchanged `commitment_signed` and the corresponding monitor update completed. With the event now being generated immediately after the nodes exchange `tx_complete`, we now need to handle the monitor update not having completed by the time we are ready to send `tx_signatures`. Unfortunately, we also did not have test coverage, allowing this to go unnoticed until being caught by the fuzzer due to a debug assertion. Doing so avoids a potential funds-loss scenario if the funding transaction confirms without the counterparty's signature for our commitment being durably persisted.

ldk-claude-review-bot · 2026-03-19T18:01:47Z

+ if self.is_awaiting_monitor_update() {
+ // Although the user may have already provided our `tx_signatures`, we must not send
+ // them if we're waiting for the monitor to durably persist the counterparty's signature
+ // for our initial commitment post-splice.
+ debug_assert!(self.context.monitor_pending_tx_signatures);


Bug: debug_assert!(self.context.monitor_pending_tx_signatures) is too strict for V2 channel opens (non-splice).
monitor_pending_tx_signatures is only set to true in splice_initial_commitment_signed (line 8006), but NOT in initial_commitment_signed_v2 (line 7847). Both call monitor_updating_paused() which sets is_monitor_update_in_progress().
For V2 dual-funded channel opens where the initial watch_channel() returns InProgress, if the counterparty sends tx_signatures while the monitor persist is still pending:
Debug builds: this debug_assert panics
Release builds: correctly returns early (holding back tx_signatures), BUT when the monitor update completes, monitor_updating_restored at line 9308-9317 skips the tx_signatures release because monitor_pending_tx_signatures is false. The tx_signatures become stuck until the next reconnection.
Consider adding self.context.monitor_pending_tx_signatures = true; in initial_commitment_signed_v2 (after the monitor_updating_paused call at line 7885), consistent with splice_initial_commitment_signed at line 8006.

Dual funding is not really supported yet, and we may even remove the incomplete work done thus far for the time being.

jkczyz · 2026-03-19T19:28:13Z

+ self.holder_tx_signatures
+ .as_ref()
+ .filter(|_| {
+ (self.has_received_commitment_signed && self.holder_sends_tx_signatures_first)


We don't want to release our signatures until we receive their commitment_signed , so I assume this stronger check previously was handle at the call sites / by the nature of when this was called?

jkczyz · 2026-03-19T19:30:30Z

+ .as_ref()
+ .filter(|_| {
+ (self.has_received_commitment_signed && self.holder_sends_tx_signatures_first)
+ || self.has_received_tx_signatures()


Does this imply we've received their commitment_signed? Should we have a stronger check / assertion here?

Not at this level but it is enforced in Channel::tx_signatures

wpaulino · 2026-03-09T18:04:31Z

In 83b2d3e, we reworked ChannelManager::funding_transaction_signed such that it would also for a user to cancel a splice up until they send commitment_signed. Previously, we would would only emit Event::FundingTransactionReadyForSigning when both nodes exchanged commitment_signed and the corresponding monitor update completed. With the event now being generated immediately after the nodes exchange tx_complete, we now need to handle the monitor update not having completed by the time we are ready to send tx_signatures. Unfortunately, we also did not have test coverage, allowing this to go unnoticed until being caught by the fuzzer due to a debug assertion. Doing so avoids a potential funds-loss scenario if the funding transaction confirms without the counterparty's signature for our commitment being durably persisted.

ldk-reviews-bot · 2026-03-09T18:04:33Z

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

codecov · 2026-03-09T20:49:14Z

Codecov Report

❌ Patch coverage is 80.95238% with 32 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.18%. Comparing base (066859b) to head (ea204f6).
⚠️ Report is 2767 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning/src/ln/channel.rs	78.84%	19 Missing and 3 partials ⚠️
lightning/src/ln/channelmanager.rs	80.00%	6 Missing and 4 partials ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #4472 +/- ##
==========================================
+ Coverage 84.55% 86.18% +1.62% 
==========================================
Files 135 161 +26 Lines 76569 107532 +30963 Branches 76569 107532 +30963 ==========================================
+ Hits 64745 92679 +27934 - Misses 9783 12233 +2450 - Partials 2041 2620 +579

Flag	Coverage Δ
tests	`86.18% <80.95%> (?)`

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 · 2026-03-11T18:05:00Z

🔔 1st Reminder

Hey @TheBlueMatt! 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 · 2026-03-14T00:01:35Z

🔔 2nd Reminder

Hey @TheBlueMatt! 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-claude-review-bot · 2026-03-14T21:15:48Z

ldk-reviews-bot · 2026-03-16T00:02:44Z

🔔 3rd Reminder

Hey @TheBlueMatt! 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 · 2026-03-16T17:43:42Z

✅ Added second reviewer: @tankyleo

TheBlueMatt

Aside from what claude noted I didn't see any issues.

TheBlueMatt · 2026-03-17T00:49:19Z

Needs rebase now.

ldk-reviews-bot · 2026-03-18T18:08:18Z

🔔 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

Looks good though consider these comments to encapsulate some logic in InteractiveTxSigningSession.

In 83b2d3e, we reworked `ChannelManager::funding_transaction_signed` such that it would also for a user to cancel a splice up until they send `commitment_signed`. Previously, we would would only emit `Event::FundingTransactionReadyForSigning` when both nodes exchanged `commitment_signed` and the corresponding monitor update completed. With the event now being generated immediately after the nodes exchange `tx_complete`, we now need to handle the monitor update not having completed by the time we are ready to send `tx_signatures`. Unfortunately, we also did not have test coverage, allowing this to go unnoticed until being caught by the fuzzer due to a debug assertion. Doing so avoids a potential funds-loss scenario if the funding transaction confirms without the counterparty's signature for our commitment being durably persisted.

ldk-claude-review-bot · 2026-03-19T18:01:47Z

+ if self.is_awaiting_monitor_update() {
+ // Although the user may have already provided our `tx_signatures`, we must not send
+ // them if we're waiting for the monitor to durably persist the counterparty's signature
+ // for our initial commitment post-splice.
+ debug_assert!(self.context.monitor_pending_tx_signatures);


Bug: debug_assert!(self.context.monitor_pending_tx_signatures) is too strict for V2 channel opens (non-splice).
monitor_pending_tx_signatures is only set to true in splice_initial_commitment_signed (line 8006), but NOT in initial_commitment_signed_v2 (line 7847). Both call monitor_updating_paused() which sets is_monitor_update_in_progress().
For V2 dual-funded channel opens where the initial watch_channel() returns InProgress, if the counterparty sends tx_signatures while the monitor persist is still pending:
Debug builds: this debug_assert panics
Release builds: correctly returns early (holding back tx_signatures), BUT when the monitor update completes, monitor_updating_restored at line 9308-9317 skips the tx_signatures release because monitor_pending_tx_signatures is false. The tx_signatures become stuck until the next reconnection.
Consider adding self.context.monitor_pending_tx_signatures = true; in initial_commitment_signed_v2 (after the monitor_updating_paused call at line 7885), consistent with splice_initial_commitment_signed at line 8006.

Dual funding is not really supported yet, and we may even remove the incomplete work done thus far for the time being.

jkczyz · 2026-03-19T19:28:13Z

+ self.holder_tx_signatures
+ .as_ref()
+ .filter(|_| {
+ (self.has_received_commitment_signed && self.holder_sends_tx_signatures_first)


We don't want to release our signatures until we receive their commitment_signed , so I assume this stronger check previously was handle at the call sites / by the nature of when this was called?

jkczyz · 2026-03-19T19:30:30Z

+ .as_ref()
+ .filter(|_| {
+ (self.has_received_commitment_signed && self.holder_sends_tx_signatures_first)
+ || self.has_received_tx_signatures()


Does this imply we've received their commitment_signed? Should we have a stronger check / assertion here?

Not at this level but it is enforced in Channel::tx_signatures

wpaulino · 2026-03-09T18:04:31Z

In 83b2d3e, we reworked ChannelManager::funding_transaction_signed such that it would also for a user to cancel a splice up until they send commitment_signed. Previously, we would would only emit Event::FundingTransactionReadyForSigning when both nodes exchanged commitment_signed and the corresponding monitor update completed. With the event now being generated immediately after the nodes exchange tx_complete, we now need to handle the monitor update not having completed by the time we are ready to send tx_signatures. Unfortunately, we also did not have test coverage, allowing this to go unnoticed until being caught by the fuzzer due to a debug assertion. Doing so avoids a potential funds-loss scenario if the funding transaction confirms without the counterparty's signature for our commitment being durably persisted.

ldk-reviews-bot · 2026-03-09T18:04:33Z

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

codecov · 2026-03-09T20:49:14Z

Codecov Report

❌ Patch coverage is 80.95238% with 32 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.18%. Comparing base (066859b) to head (ea204f6).
⚠️ Report is 2767 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning/src/ln/channel.rs	78.84%	19 Missing and 3 partials ⚠️
lightning/src/ln/channelmanager.rs	80.00%	6 Missing and 4 partials ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #4472 +/- ##
==========================================
+ Coverage 84.55% 86.18% +1.62% 
==========================================
Files 135 161 +26 Lines 76569 107532 +30963 Branches 76569 107532 +30963 ==========================================
+ Hits 64745 92679 +27934 - Misses 9783 12233 +2450 - Partials 2041 2620 +579

Flag	Coverage Δ
tests	`86.18% <80.95%> (?)`

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 · 2026-03-11T18:05:00Z

🔔 1st Reminder

Hey @TheBlueMatt! 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 · 2026-03-14T00:01:35Z

🔔 2nd Reminder

Hey @TheBlueMatt! 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-claude-review-bot · 2026-03-14T21:15:48Z

ldk-reviews-bot · 2026-03-16T00:02:44Z

🔔 3rd Reminder

Hey @TheBlueMatt! 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 · 2026-03-16T17:43:42Z

✅ Added second reviewer: @tankyleo

TheBlueMatt

Aside from what claude noted I didn't see any issues.

TheBlueMatt · 2026-03-17T00:49:19Z

Needs rebase now.

ldk-reviews-bot · 2026-03-18T18:08:18Z

🔔 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

Looks good though consider these comments to encapsulate some logic in InteractiveTxSigningSession.

In 83b2d3e, we reworked `ChannelManager::funding_transaction_signed` such that it would also for a user to cancel a splice up until they send `commitment_signed`. Previously, we would would only emit `Event::FundingTransactionReadyForSigning` when both nodes exchanged `commitment_signed` and the corresponding monitor update completed. With the event now being generated immediately after the nodes exchange `tx_complete`, we now need to handle the monitor update not having completed by the time we are ready to send `tx_signatures`. Unfortunately, we also did not have test coverage, allowing this to go unnoticed until being caught by the fuzzer due to a debug assertion. Doing so avoids a potential funds-loss scenario if the funding transaction confirms without the counterparty's signature for our commitment being durably persisted.

ldk-claude-review-bot · 2026-03-19T18:01:47Z

+ if self.is_awaiting_monitor_update() {
+ // Although the user may have already provided our `tx_signatures`, we must not send
+ // them if we're waiting for the monitor to durably persist the counterparty's signature
+ // for our initial commitment post-splice.
+ debug_assert!(self.context.monitor_pending_tx_signatures);


Bug: debug_assert!(self.context.monitor_pending_tx_signatures) is too strict for V2 channel opens (non-splice).
monitor_pending_tx_signatures is only set to true in splice_initial_commitment_signed (line 8006), but NOT in initial_commitment_signed_v2 (line 7847). Both call monitor_updating_paused() which sets is_monitor_update_in_progress().
For V2 dual-funded channel opens where the initial watch_channel() returns InProgress, if the counterparty sends tx_signatures while the monitor persist is still pending:
Debug builds: this debug_assert panics
Release builds: correctly returns early (holding back tx_signatures), BUT when the monitor update completes, monitor_updating_restored at line 9308-9317 skips the tx_signatures release because monitor_pending_tx_signatures is false. The tx_signatures become stuck until the next reconnection.
Consider adding self.context.monitor_pending_tx_signatures = true; in initial_commitment_signed_v2 (after the monitor_updating_paused call at line 7885), consistent with splice_initial_commitment_signed at line 8006.

Dual funding is not really supported yet, and we may even remove the incomplete work done thus far for the time being.

jkczyz · 2026-03-19T19:28:13Z

+ self.holder_tx_signatures
+ .as_ref()
+ .filter(|_| {
+ (self.has_received_commitment_signed && self.holder_sends_tx_signatures_first)


We don't want to release our signatures until we receive their commitment_signed , so I assume this stronger check previously was handle at the call sites / by the nature of when this was called?

jkczyz · 2026-03-19T19:30:30Z

+ .as_ref()
+ .filter(|_| {
+ (self.has_received_commitment_signed && self.holder_sends_tx_signatures_first)
+ || self.has_received_tx_signatures()


Does this imply we've received their commitment_signed? Should we have a stronger check / assertion here?

Not at this level but it is enforced in Channel::tx_signatures

wpaulino · 2026-03-09T18:04:31Z

In 83b2d3e, we reworked ChannelManager::funding_transaction_signed such that it would also for a user to cancel a splice up until they send commitment_signed. Previously, we would would only emit Event::FundingTransactionReadyForSigning when both nodes exchanged commitment_signed and the corresponding monitor update completed. With the event now being generated immediately after the nodes exchange tx_complete, we now need to handle the monitor update not having completed by the time we are ready to send tx_signatures. Unfortunately, we also did not have test coverage, allowing this to go unnoticed until being caught by the fuzzer due to a debug assertion. Doing so avoids a potential funds-loss scenario if the funding transaction confirms without the counterparty's signature for our commitment being durably persisted.

ldk-reviews-bot · 2026-03-09T18:04:33Z

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

codecov · 2026-03-09T20:49:14Z

Codecov Report

❌ Patch coverage is 80.95238% with 32 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.18%. Comparing base (066859b) to head (ea204f6).
⚠️ Report is 2767 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning/src/ln/channel.rs	78.84%	19 Missing and 3 partials ⚠️
lightning/src/ln/channelmanager.rs	80.00%	6 Missing and 4 partials ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #4472 +/- ##
==========================================
+ Coverage 84.55% 86.18% +1.62% 
==========================================
Files 135 161 +26 Lines 76569 107532 +30963 Branches 76569 107532 +30963 ==========================================
+ Hits 64745 92679 +27934 - Misses 9783 12233 +2450 - Partials 2041 2620 +579

Flag	Coverage Δ
tests	`86.18% <80.95%> (?)`

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 · 2026-03-11T18:05:00Z

🔔 1st Reminder

Hey @TheBlueMatt! 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 · 2026-03-14T00:01:35Z

🔔 2nd Reminder

Hey @TheBlueMatt! 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-claude-review-bot · 2026-03-14T21:15:48Z

ldk-reviews-bot · 2026-03-16T00:02:44Z

🔔 3rd Reminder

Hey @TheBlueMatt! 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 · 2026-03-16T17:43:42Z

✅ Added second reviewer: @tankyleo

TheBlueMatt

Aside from what claude noted I didn't see any issues.

TheBlueMatt · 2026-03-17T00:49:19Z

Needs rebase now.

ldk-reviews-bot · 2026-03-18T18:08:18Z

🔔 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

Looks good though consider these comments to encapsulate some logic in InteractiveTxSigningSession.

In 83b2d3e, we reworked `ChannelManager::funding_transaction_signed` such that it would also for a user to cancel a splice up until they send `commitment_signed`. Previously, we would would only emit `Event::FundingTransactionReadyForSigning` when both nodes exchanged `commitment_signed` and the corresponding monitor update completed. With the event now being generated immediately after the nodes exchange `tx_complete`, we now need to handle the monitor update not having completed by the time we are ready to send `tx_signatures`. Unfortunately, we also did not have test coverage, allowing this to go unnoticed until being caught by the fuzzer due to a debug assertion. Doing so avoids a potential funds-loss scenario if the funding transaction confirms without the counterparty's signature for our commitment being durably persisted.

ldk-claude-review-bot · 2026-03-19T18:01:47Z

+ if self.is_awaiting_monitor_update() {
+ // Although the user may have already provided our `tx_signatures`, we must not send
+ // them if we're waiting for the monitor to durably persist the counterparty's signature
+ // for our initial commitment post-splice.
+ debug_assert!(self.context.monitor_pending_tx_signatures);


Bug: debug_assert!(self.context.monitor_pending_tx_signatures) is too strict for V2 channel opens (non-splice).
monitor_pending_tx_signatures is only set to true in splice_initial_commitment_signed (line 8006), but NOT in initial_commitment_signed_v2 (line 7847). Both call monitor_updating_paused() which sets is_monitor_update_in_progress().
For V2 dual-funded channel opens where the initial watch_channel() returns InProgress, if the counterparty sends tx_signatures while the monitor persist is still pending:
Debug builds: this debug_assert panics
Release builds: correctly returns early (holding back tx_signatures), BUT when the monitor update completes, monitor_updating_restored at line 9308-9317 skips the tx_signatures release because monitor_pending_tx_signatures is false. The tx_signatures become stuck until the next reconnection.
Consider adding self.context.monitor_pending_tx_signatures = true; in initial_commitment_signed_v2 (after the monitor_updating_paused call at line 7885), consistent with splice_initial_commitment_signed at line 8006.

Dual funding is not really supported yet, and we may even remove the incomplete work done thus far for the time being.

jkczyz · 2026-03-19T19:28:13Z

+ self.holder_tx_signatures
+ .as_ref()
+ .filter(|_| {
+ (self.has_received_commitment_signed && self.holder_sends_tx_signatures_first)


We don't want to release our signatures until we receive their commitment_signed , so I assume this stronger check previously was handle at the call sites / by the nature of when this was called?

jkczyz · 2026-03-19T19:30:30Z

+ .as_ref()
+ .filter(|_| {
+ (self.has_received_commitment_signed && self.holder_sends_tx_signatures_first)
+ || self.has_received_tx_signatures()


Does this imply we've received their commitment_signed? Should we have a stronger check / assertion here?

Not at this level but it is enforced in Channel::tx_signatures

wpaulino · 2026-03-09T18:04:31Z

In 83b2d3e, we reworked ChannelManager::funding_transaction_signed such that it would also for a user to cancel a splice up until they send commitment_signed. Previously, we would would only emit Event::FundingTransactionReadyForSigning when both nodes exchanged commitment_signed and the corresponding monitor update completed. With the event now being generated immediately after the nodes exchange tx_complete, we now need to handle the monitor update not having completed by the time we are ready to send tx_signatures. Unfortunately, we also did not have test coverage, allowing this to go unnoticed until being caught by the fuzzer due to a debug assertion. Doing so avoids a potential funds-loss scenario if the funding transaction confirms without the counterparty's signature for our commitment being durably persisted.

ldk-reviews-bot · 2026-03-09T18:04:33Z

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

codecov · 2026-03-09T20:49:14Z

Codecov Report

❌ Patch coverage is 80.95238% with 32 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.18%. Comparing base (066859b) to head (ea204f6).
⚠️ Report is 2767 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning/src/ln/channel.rs	78.84%	19 Missing and 3 partials ⚠️
lightning/src/ln/channelmanager.rs	80.00%	6 Missing and 4 partials ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #4472 +/- ##
==========================================
+ Coverage 84.55% 86.18% +1.62% 
==========================================
Files 135 161 +26 Lines 76569 107532 +30963 Branches 76569 107532 +30963 ==========================================
+ Hits 64745 92679 +27934 - Misses 9783 12233 +2450 - Partials 2041 2620 +579

Flag	Coverage Δ
tests	`86.18% <80.95%> (?)`

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 · 2026-03-11T18:05:00Z

🔔 1st Reminder

Hey @TheBlueMatt! 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 · 2026-03-14T00:01:35Z

🔔 2nd Reminder

Hey @TheBlueMatt! 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-claude-review-bot · 2026-03-14T21:15:48Z

ldk-reviews-bot · 2026-03-16T00:02:44Z

🔔 3rd Reminder

Hey @TheBlueMatt! 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 · 2026-03-16T17:43:42Z

✅ Added second reviewer: @tankyleo

TheBlueMatt

Aside from what claude noted I didn't see any issues.

TheBlueMatt · 2026-03-17T00:49:19Z

Needs rebase now.

ldk-reviews-bot · 2026-03-18T18:08:18Z

🔔 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

Looks good though consider these comments to encapsulate some logic in InteractiveTxSigningSession.

In 83b2d3e, we reworked `ChannelManager::funding_transaction_signed` such that it would also for a user to cancel a splice up until they send `commitment_signed`. Previously, we would would only emit `Event::FundingTransactionReadyForSigning` when both nodes exchanged `commitment_signed` and the corresponding monitor update completed. With the event now being generated immediately after the nodes exchange `tx_complete`, we now need to handle the monitor update not having completed by the time we are ready to send `tx_signatures`. Unfortunately, we also did not have test coverage, allowing this to go unnoticed until being caught by the fuzzer due to a debug assertion. Doing so avoids a potential funds-loss scenario if the funding transaction confirms without the counterparty's signature for our commitment being durably persisted.

ldk-claude-review-bot · 2026-03-19T18:01:47Z

+ if self.is_awaiting_monitor_update() {
+ // Although the user may have already provided our `tx_signatures`, we must not send
+ // them if we're waiting for the monitor to durably persist the counterparty's signature
+ // for our initial commitment post-splice.
+ debug_assert!(self.context.monitor_pending_tx_signatures);


Bug: debug_assert!(self.context.monitor_pending_tx_signatures) is too strict for V2 channel opens (non-splice).
monitor_pending_tx_signatures is only set to true in splice_initial_commitment_signed (line 8006), but NOT in initial_commitment_signed_v2 (line 7847). Both call monitor_updating_paused() which sets is_monitor_update_in_progress().
For V2 dual-funded channel opens where the initial watch_channel() returns InProgress, if the counterparty sends tx_signatures while the monitor persist is still pending:
Debug builds: this debug_assert panics
Release builds: correctly returns early (holding back tx_signatures), BUT when the monitor update completes, monitor_updating_restored at line 9308-9317 skips the tx_signatures release because monitor_pending_tx_signatures is false. The tx_signatures become stuck until the next reconnection.
Consider adding self.context.monitor_pending_tx_signatures = true; in initial_commitment_signed_v2 (after the monitor_updating_paused call at line 7885), consistent with splice_initial_commitment_signed at line 8006.

Dual funding is not really supported yet, and we may even remove the incomplete work done thus far for the time being.

jkczyz · 2026-03-19T19:28:13Z

+ self.holder_tx_signatures
+ .as_ref()
+ .filter(|_| {
+ (self.has_received_commitment_signed && self.holder_sends_tx_signatures_first)


We don't want to release our signatures until we receive their commitment_signed , so I assume this stronger check previously was handle at the call sites / by the nature of when this was called?

jkczyz · 2026-03-19T19:30:30Z

+ .as_ref()
+ .filter(|_| {
+ (self.has_received_commitment_signed && self.holder_sends_tx_signatures_first)
+ || self.has_received_tx_signatures()


Does this imply we've received their commitment_signed? Should we have a stronger check / assertion here?

Not at this level but it is enforced in Channel::tx_signatures

wpaulino · 2026-03-09T18:04:31Z

In 83b2d3e, we reworked ChannelManager::funding_transaction_signed such that it would also for a user to cancel a splice up until they send commitment_signed. Previously, we would would only emit Event::FundingTransactionReadyForSigning when both nodes exchanged commitment_signed and the corresponding monitor update completed. With the event now being generated immediately after the nodes exchange tx_complete, we now need to handle the monitor update not having completed by the time we are ready to send tx_signatures. Unfortunately, we also did not have test coverage, allowing this to go unnoticed until being caught by the fuzzer due to a debug assertion. Doing so avoids a potential funds-loss scenario if the funding transaction confirms without the counterparty's signature for our commitment being durably persisted.

ldk-reviews-bot · 2026-03-09T18:04:33Z

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

codecov · 2026-03-09T20:49:14Z

Codecov Report

❌ Patch coverage is 80.95238% with 32 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.18%. Comparing base (066859b) to head (ea204f6).
⚠️ Report is 2767 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning/src/ln/channel.rs	78.84%	19 Missing and 3 partials ⚠️
lightning/src/ln/channelmanager.rs	80.00%	6 Missing and 4 partials ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #4472 +/- ##
==========================================
+ Coverage 84.55% 86.18% +1.62% 
==========================================
Files 135 161 +26 Lines 76569 107532 +30963 Branches 76569 107532 +30963 ==========================================
+ Hits 64745 92679 +27934 - Misses 9783 12233 +2450 - Partials 2041 2620 +579

Flag	Coverage Δ
tests	`86.18% <80.95%> (?)`

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 · 2026-03-11T18:05:00Z

🔔 1st Reminder

Hey @TheBlueMatt! 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 · 2026-03-14T00:01:35Z

🔔 2nd Reminder

Hey @TheBlueMatt! 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-claude-review-bot · 2026-03-14T21:15:48Z

ldk-reviews-bot · 2026-03-16T00:02:44Z

🔔 3rd Reminder

Hey @TheBlueMatt! 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 · 2026-03-16T17:43:42Z

✅ Added second reviewer: @tankyleo

TheBlueMatt

Aside from what claude noted I didn't see any issues.

TheBlueMatt · 2026-03-17T00:49:19Z

Needs rebase now.

ldk-reviews-bot · 2026-03-18T18:08:18Z

🔔 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

Looks good though consider these comments to encapsulate some logic in InteractiveTxSigningSession.

In 83b2d3e, we reworked `ChannelManager::funding_transaction_signed` such that it would also for a user to cancel a splice up until they send `commitment_signed`. Previously, we would would only emit `Event::FundingTransactionReadyForSigning` when both nodes exchanged `commitment_signed` and the corresponding monitor update completed. With the event now being generated immediately after the nodes exchange `tx_complete`, we now need to handle the monitor update not having completed by the time we are ready to send `tx_signatures`. Unfortunately, we also did not have test coverage, allowing this to go unnoticed until being caught by the fuzzer due to a debug assertion. Doing so avoids a potential funds-loss scenario if the funding transaction confirms without the counterparty's signature for our commitment being durably persisted.

ldk-claude-review-bot · 2026-03-19T18:01:47Z

+ if self.is_awaiting_monitor_update() {
+ // Although the user may have already provided our `tx_signatures`, we must not send
+ // them if we're waiting for the monitor to durably persist the counterparty's signature
+ // for our initial commitment post-splice.
+ debug_assert!(self.context.monitor_pending_tx_signatures);


Bug: debug_assert!(self.context.monitor_pending_tx_signatures) is too strict for V2 channel opens (non-splice).
monitor_pending_tx_signatures is only set to true in splice_initial_commitment_signed (line 8006), but NOT in initial_commitment_signed_v2 (line 7847). Both call monitor_updating_paused() which sets is_monitor_update_in_progress().
For V2 dual-funded channel opens where the initial watch_channel() returns InProgress, if the counterparty sends tx_signatures while the monitor persist is still pending:
Debug builds: this debug_assert panics
Release builds: correctly returns early (holding back tx_signatures), BUT when the monitor update completes, monitor_updating_restored at line 9308-9317 skips the tx_signatures release because monitor_pending_tx_signatures is false. The tx_signatures become stuck until the next reconnection.
Consider adding self.context.monitor_pending_tx_signatures = true; in initial_commitment_signed_v2 (after the monitor_updating_paused call at line 7885), consistent with splice_initial_commitment_signed at line 8006.

Dual funding is not really supported yet, and we may even remove the incomplete work done thus far for the time being.

jkczyz · 2026-03-19T19:28:13Z

+ self.holder_tx_signatures
+ .as_ref()
+ .filter(|_| {
+ (self.has_received_commitment_signed && self.holder_sends_tx_signatures_first)


We don't want to release our signatures until we receive their commitment_signed , so I assume this stronger check previously was handle at the call sites / by the nature of when this was called?

jkczyz · 2026-03-19T19:30:30Z

+ .as_ref()
+ .filter(|_| {
+ (self.has_received_commitment_signed && self.holder_sends_tx_signatures_first)
+ || self.has_received_tx_signatures()


Does this imply we've received their commitment_signed? Should we have a stronger check / assertion here?

Not at this level but it is enforced in Channel::tx_signatures

wpaulino · 2026-03-09T18:04:31Z

In 83b2d3e, we reworked ChannelManager::funding_transaction_signed such that it would also for a user to cancel a splice up until they send commitment_signed. Previously, we would would only emit Event::FundingTransactionReadyForSigning when both nodes exchanged commitment_signed and the corresponding monitor update completed. With the event now being generated immediately after the nodes exchange tx_complete, we now need to handle the monitor update not having completed by the time we are ready to send tx_signatures. Unfortunately, we also did not have test coverage, allowing this to go unnoticed until being caught by the fuzzer due to a debug assertion. Doing so avoids a potential funds-loss scenario if the funding transaction confirms without the counterparty's signature for our commitment being durably persisted.

ldk-reviews-bot · 2026-03-09T18:04:33Z

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

codecov · 2026-03-09T20:49:14Z

Codecov Report

❌ Patch coverage is 80.95238% with 32 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.18%. Comparing base (066859b) to head (ea204f6).
⚠️ Report is 2767 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning/src/ln/channel.rs	78.84%	19 Missing and 3 partials ⚠️
lightning/src/ln/channelmanager.rs	80.00%	6 Missing and 4 partials ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #4472 +/- ##
==========================================
+ Coverage 84.55% 86.18% +1.62% 
==========================================
Files 135 161 +26 Lines 76569 107532 +30963 Branches 76569 107532 +30963 ==========================================
+ Hits 64745 92679 +27934 - Misses 9783 12233 +2450 - Partials 2041 2620 +579

Flag	Coverage Δ
tests	`86.18% <80.95%> (?)`

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 · 2026-03-11T18:05:00Z

🔔 1st Reminder

Hey @TheBlueMatt! 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 · 2026-03-14T00:01:35Z

🔔 2nd Reminder

Hey @TheBlueMatt! 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-claude-review-bot · 2026-03-14T21:15:48Z

ldk-reviews-bot · 2026-03-16T00:02:44Z

🔔 3rd Reminder

Hey @TheBlueMatt! 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 · 2026-03-16T17:43:42Z

✅ Added second reviewer: @tankyleo

TheBlueMatt

Aside from what claude noted I didn't see any issues.

TheBlueMatt · 2026-03-17T00:49:19Z

Needs rebase now.

ldk-reviews-bot · 2026-03-18T18:08:18Z

🔔 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

Looks good though consider these comments to encapsulate some logic in InteractiveTxSigningSession.

In 83b2d3e, we reworked `ChannelManager::funding_transaction_signed` such that it would also for a user to cancel a splice up until they send `commitment_signed`. Previously, we would would only emit `Event::FundingTransactionReadyForSigning` when both nodes exchanged `commitment_signed` and the corresponding monitor update completed. With the event now being generated immediately after the nodes exchange `tx_complete`, we now need to handle the monitor update not having completed by the time we are ready to send `tx_signatures`. Unfortunately, we also did not have test coverage, allowing this to go unnoticed until being caught by the fuzzer due to a debug assertion. Doing so avoids a potential funds-loss scenario if the funding transaction confirms without the counterparty's signature for our commitment being durably persisted.

ldk-claude-review-bot · 2026-03-19T18:01:47Z

+ if self.is_awaiting_monitor_update() {
+ // Although the user may have already provided our `tx_signatures`, we must not send
+ // them if we're waiting for the monitor to durably persist the counterparty's signature
+ // for our initial commitment post-splice.
+ debug_assert!(self.context.monitor_pending_tx_signatures);


Bug: debug_assert!(self.context.monitor_pending_tx_signatures) is too strict for V2 channel opens (non-splice).
monitor_pending_tx_signatures is only set to true in splice_initial_commitment_signed (line 8006), but NOT in initial_commitment_signed_v2 (line 7847). Both call monitor_updating_paused() which sets is_monitor_update_in_progress().
For V2 dual-funded channel opens where the initial watch_channel() returns InProgress, if the counterparty sends tx_signatures while the monitor persist is still pending:
Debug builds: this debug_assert panics
Release builds: correctly returns early (holding back tx_signatures), BUT when the monitor update completes, monitor_updating_restored at line 9308-9317 skips the tx_signatures release because monitor_pending_tx_signatures is false. The tx_signatures become stuck until the next reconnection.
Consider adding self.context.monitor_pending_tx_signatures = true; in initial_commitment_signed_v2 (after the monitor_updating_paused call at line 7885), consistent with splice_initial_commitment_signed at line 8006.

Dual funding is not really supported yet, and we may even remove the incomplete work done thus far for the time being.

jkczyz · 2026-03-19T19:28:13Z

+ self.holder_tx_signatures
+ .as_ref()
+ .filter(|_| {
+ (self.has_received_commitment_signed && self.holder_sends_tx_signatures_first)


We don't want to release our signatures until we receive their commitment_signed , so I assume this stronger check previously was handle at the call sites / by the nature of when this was called?

jkczyz · 2026-03-19T19:30:30Z

+ .as_ref()
+ .filter(|_| {
+ (self.has_received_commitment_signed && self.holder_sends_tx_signatures_first)
+ || self.has_received_tx_signatures()


Does this imply we've received their commitment_signed? Should we have a stronger check / assertion here?

Not at this level but it is enforced in Channel::tx_signatures

wpaulino · 2026-03-09T18:04:31Z

In 83b2d3e, we reworked ChannelManager::funding_transaction_signed such that it would also for a user to cancel a splice up until they send commitment_signed. Previously, we would would only emit Event::FundingTransactionReadyForSigning when both nodes exchanged commitment_signed and the corresponding monitor update completed. With the event now being generated immediately after the nodes exchange tx_complete, we now need to handle the monitor update not having completed by the time we are ready to send tx_signatures. Unfortunately, we also did not have test coverage, allowing this to go unnoticed until being caught by the fuzzer due to a debug assertion. Doing so avoids a potential funds-loss scenario if the funding transaction confirms without the counterparty's signature for our commitment being durably persisted.

ldk-reviews-bot · 2026-03-09T18:04:33Z

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

codecov · 2026-03-09T20:49:14Z

Codecov Report

❌ Patch coverage is 80.95238% with 32 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.18%. Comparing base (066859b) to head (ea204f6).
⚠️ Report is 2767 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning/src/ln/channel.rs	78.84%	19 Missing and 3 partials ⚠️
lightning/src/ln/channelmanager.rs	80.00%	6 Missing and 4 partials ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #4472 +/- ##
==========================================
+ Coverage 84.55% 86.18% +1.62% 
==========================================
Files 135 161 +26 Lines 76569 107532 +30963 Branches 76569 107532 +30963 ==========================================
+ Hits 64745 92679 +27934 - Misses 9783 12233 +2450 - Partials 2041 2620 +579

Flag	Coverage Δ
tests	`86.18% <80.95%> (?)`

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 · 2026-03-11T18:05:00Z

🔔 1st Reminder

Hey @TheBlueMatt! 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 · 2026-03-14T00:01:35Z

🔔 2nd Reminder

Hey @TheBlueMatt! 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-claude-review-bot · 2026-03-14T21:15:48Z

ldk-reviews-bot · 2026-03-16T00:02:44Z

🔔 3rd Reminder

Hey @TheBlueMatt! 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 · 2026-03-16T17:43:42Z

✅ Added second reviewer: @tankyleo

TheBlueMatt

Aside from what claude noted I didn't see any issues.

TheBlueMatt · 2026-03-17T00:49:19Z

Needs rebase now.

ldk-reviews-bot · 2026-03-18T18:08:18Z

🔔 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

Looks good though consider these comments to encapsulate some logic in InteractiveTxSigningSession.

In 83b2d3e, we reworked `ChannelManager::funding_transaction_signed` such that it would also for a user to cancel a splice up until they send `commitment_signed`. Previously, we would would only emit `Event::FundingTransactionReadyForSigning` when both nodes exchanged `commitment_signed` and the corresponding monitor update completed. With the event now being generated immediately after the nodes exchange `tx_complete`, we now need to handle the monitor update not having completed by the time we are ready to send `tx_signatures`. Unfortunately, we also did not have test coverage, allowing this to go unnoticed until being caught by the fuzzer due to a debug assertion. Doing so avoids a potential funds-loss scenario if the funding transaction confirms without the counterparty's signature for our commitment being durably persisted.

ldk-claude-review-bot · 2026-03-19T18:01:47Z

+ if self.is_awaiting_monitor_update() {
+ // Although the user may have already provided our `tx_signatures`, we must not send
+ // them if we're waiting for the monitor to durably persist the counterparty's signature
+ // for our initial commitment post-splice.
+ debug_assert!(self.context.monitor_pending_tx_signatures);


Bug: debug_assert!(self.context.monitor_pending_tx_signatures) is too strict for V2 channel opens (non-splice).
monitor_pending_tx_signatures is only set to true in splice_initial_commitment_signed (line 8006), but NOT in initial_commitment_signed_v2 (line 7847). Both call monitor_updating_paused() which sets is_monitor_update_in_progress().
For V2 dual-funded channel opens where the initial watch_channel() returns InProgress, if the counterparty sends tx_signatures while the monitor persist is still pending:
Debug builds: this debug_assert panics
Release builds: correctly returns early (holding back tx_signatures), BUT when the monitor update completes, monitor_updating_restored at line 9308-9317 skips the tx_signatures release because monitor_pending_tx_signatures is false. The tx_signatures become stuck until the next reconnection.
Consider adding self.context.monitor_pending_tx_signatures = true; in initial_commitment_signed_v2 (after the monitor_updating_paused call at line 7885), consistent with splice_initial_commitment_signed at line 8006.

Dual funding is not really supported yet, and we may even remove the incomplete work done thus far for the time being.

jkczyz · 2026-03-19T19:28:13Z

+ self.holder_tx_signatures
+ .as_ref()
+ .filter(|_| {
+ (self.has_received_commitment_signed && self.holder_sends_tx_signatures_first)


We don't want to release our signatures until we receive their commitment_signed , so I assume this stronger check previously was handle at the call sites / by the nature of when this was called?

jkczyz · 2026-03-19T19:30:30Z

+ .as_ref()
+ .filter(|_| {
+ (self.has_received_commitment_signed && self.holder_sends_tx_signatures_first)
+ || self.has_received_tx_signatures()


Does this imply we've received their commitment_signed? Should we have a stronger check / assertion here?

Not at this level but it is enforced in Channel::tx_signatures

wpaulino added this to the 0.3 milestone Mar 9, 2026

wpaulino requested a review from TheBlueMatt March 9, 2026 18:04

wpaulino self-assigned this Mar 9, 2026

ldk-claude-review-bot reviewed Mar 14, 2026
View reviewed changes

Comment threadlightning/src/ln/channel.rs

ldk-claude-review-bot reviewed Mar 14, 2026
View reviewed changes

Comment threadlightning/src/ln/channel.rs Outdated

ldk-reviews-bot requested a review from tankyleo March 16, 2026 17:43

TheBlueMatt reviewed Mar 16, 2026
View reviewed changes

wpaulino force-pushed the release-tx-signatures-after-async-monitor-update-completion branch from c8c5dc0 to 86fa38cCompare March 16, 2026 18:07

wpaulino requested review from jkczyz and removed request for tankyleo March 16, 2026 18:07

ldk-claude-review-bot reviewed Mar 16, 2026
View reviewed changes

Comment threadlightning/src/ln/channel.rs Outdated

wpaulino force-pushed the release-tx-signatures-after-async-monitor-update-completion branch from 86fa38c to ec60d8eCompare March 16, 2026 18:39

wpaulino force-pushed the release-tx-signatures-after-async-monitor-update-completion branch from ec60d8e to f81bb39Compare March 17, 2026 17:07

joostjager mentioned this pull request Mar 19, 2026
fuzz: cover deferred writing in chanmon_consistency #4465
Merged

jkczyz reviewed Mar 19, 2026
View reviewed changes

Comment threadlightning/src/ln/channel.rs Outdated
Comment threadlightning/src/ln/channel.rs Outdated

wpaulino force-pushed the release-tx-signatures-after-async-monitor-update-completion branch from f81bb39 to ea204f6Compare March 19, 2026 17:45

wpaulino requested a review from jkczyz March 19, 2026 17:45

ldk-claude-review-bot reviewed Mar 19, 2026
View reviewed changes

jkczyz reviewed Mar 19, 2026
View reviewed changes

jkczyz mentioned this pull request Mar 19, 2026
Splicing / Dual Funding #4284
Open
50 tasks

jkczyz approved these changes Mar 19, 2026
View reviewed changes

wpaulino requested a review from TheBlueMatt March 19, 2026 20:51

TheBlueMatt approved these changes Mar 20, 2026
View reviewed changes

TheBlueMatt merged commit 20e240d into lightningdevkit:mainMar 20, 2026
21 of 22 checks passed

wpaulino deleted the release-tx-signatures-after-async-monitor-update-completion branch March 20, 2026 16:04

bolty-from-atl-bitlabBot mentioned this pull request Mar 27, 2026
Bitcoin Socratic Seminar #49 - Topic Suggestions atlantabitdevs/atlbit.dev#141
Closed

wpaulino mentioned this pull request Mar 30, 2026
Fuzz failure backlog #4504
Closed

notmandatory mentioned this pull request Apr 13, 2026
Socratic Seminar #42 NashBitDevs/nashbitdevs.org#132
Closed