Include NegotiationFailureReason in InteractiveTxMsgError

[wpaulino]

Each SpliceNegotiationFailed event originating from an InteractiveTxMsgError needs a NegotiationFailureReason, so it makes sense to track it in the same place. In most cases, the NegotiationFailureReason included uses the NegotiationError variant, but other cases may require their own specific variant, such as LocallyCanceled after calling ChannelManager::cancel_funding_contributed.

Fixes#4490 (comment)

[ldk-reviews-bot]

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

[ldk-claude-review-bot]

I've thoroughly reviewed the entire PR diff and explored the relevant codebase context. This is a clean refactoring that moves NegotiationFailureReason into InteractiveTxMsgError rather than passing it as a separate parameter to handle_interactive_tx_msg_err.

Key verification points:

• The into_parts() fallback via reason_from_channel_error produces the same output as the old inline format!("{:?}", err.err) — both format the ChannelError with its Debug impl.
• The one call site that previously passed Some(LocallyCanceled) (the cancel_funding_contributed handler) now has the reason embedded in both branches of cancel_funding_contributed() in channel.rs.
• All other call sites previously passed None, so the fallback behavior is unchanged.
• All 6 call sites of handle_interactive_tx_msg_err are updated to the new 4-parameter signature.
• The borrow pattern in into_parts() (borrowing err in the .map() closure then moving it into the return tuple) is correct — the borrow is released when .map() completes.

No issues found.

[codecov]

Codecov Report

❌ Patch coverage is 70.37037% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.11%. Comparing base (8882edd) to head (946ee09).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning/src/ln/channel.rs	67.34%	16 Missing ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #4609 +/- ##
==========================================
- Coverage 86.12% 86.11% -0.01% 
==========================================
Files 157 157 Lines 108824 108841 +17 Branches 108824 108841 +17 ==========================================
+ Hits 93721 93725 +4 - Misses 12487 12497 +10 - Partials 2616 2619 +3 

Flag	Coverage Δ
tests	86.11% <70.37%> (-0.01%)	⬇️

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]

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

[TheBlueMatt]

thanks

[ldk-reviews-bot]

👋 The first review has been submitted!

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

[wpaulino]

Each SpliceNegotiationFailed event originating from an InteractiveTxMsgError needs a NegotiationFailureReason, so it makes sense to track it in the same place. In most cases, the NegotiationFailureReason included uses the NegotiationError variant, but other cases may require their own specific variant, such as LocallyCanceled after calling ChannelManager::cancel_funding_contributed.

Fixes#4490 (comment)

[ldk-reviews-bot]

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

[ldk-claude-review-bot]

I've thoroughly reviewed the entire PR diff and explored the relevant codebase context. This is a clean refactoring that moves NegotiationFailureReason into InteractiveTxMsgError rather than passing it as a separate parameter to handle_interactive_tx_msg_err.

Key verification points:

• The into_parts() fallback via reason_from_channel_error produces the same output as the old inline format!("{:?}", err.err) — both format the ChannelError with its Debug impl.
• The one call site that previously passed Some(LocallyCanceled) (the cancel_funding_contributed handler) now has the reason embedded in both branches of cancel_funding_contributed() in channel.rs.
• All other call sites previously passed None, so the fallback behavior is unchanged.
• All 6 call sites of handle_interactive_tx_msg_err are updated to the new 4-parameter signature.
• The borrow pattern in into_parts() (borrowing err in the .map() closure then moving it into the return tuple) is correct — the borrow is released when .map() completes.

No issues found.

[codecov]

Codecov Report

❌ Patch coverage is 70.37037% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.11%. Comparing base (8882edd) to head (946ee09).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning/src/ln/channel.rs	67.34%	16 Missing ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #4609 +/- ##
==========================================
- Coverage 86.12% 86.11% -0.01% 
==========================================
Files 157 157 Lines 108824 108841 +17 Branches 108824 108841 +17 ==========================================
+ Hits 93721 93725 +4 - Misses 12487 12497 +10 - Partials 2616 2619 +3 

Flag	Coverage Δ
tests	86.11% <70.37%> (-0.01%)	⬇️

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]

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

[TheBlueMatt]

thanks

[ldk-reviews-bot]

👋 The first review has been submitted!

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

[wpaulino]

Each SpliceNegotiationFailed event originating from an InteractiveTxMsgError needs a NegotiationFailureReason, so it makes sense to track it in the same place. In most cases, the NegotiationFailureReason included uses the NegotiationError variant, but other cases may require their own specific variant, such as LocallyCanceled after calling ChannelManager::cancel_funding_contributed.

Fixes#4490 (comment)

[ldk-reviews-bot]

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

[ldk-claude-review-bot]

I've thoroughly reviewed the entire PR diff and explored the relevant codebase context. This is a clean refactoring that moves NegotiationFailureReason into InteractiveTxMsgError rather than passing it as a separate parameter to handle_interactive_tx_msg_err.

Key verification points:

• The into_parts() fallback via reason_from_channel_error produces the same output as the old inline format!("{:?}", err.err) — both format the ChannelError with its Debug impl.
• The one call site that previously passed Some(LocallyCanceled) (the cancel_funding_contributed handler) now has the reason embedded in both branches of cancel_funding_contributed() in channel.rs.
• All other call sites previously passed None, so the fallback behavior is unchanged.
• All 6 call sites of handle_interactive_tx_msg_err are updated to the new 4-parameter signature.
• The borrow pattern in into_parts() (borrowing err in the .map() closure then moving it into the return tuple) is correct — the borrow is released when .map() completes.

No issues found.

[codecov]

Codecov Report

❌ Patch coverage is 70.37037% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.11%. Comparing base (8882edd) to head (946ee09).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning/src/ln/channel.rs	67.34%	16 Missing ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #4609 +/- ##
==========================================
- Coverage 86.12% 86.11% -0.01% 
==========================================
Files 157 157 Lines 108824 108841 +17 Branches 108824 108841 +17 ==========================================
+ Hits 93721 93725 +4 - Misses 12487 12497 +10 - Partials 2616 2619 +3 

Flag	Coverage Δ
tests	86.11% <70.37%> (-0.01%)	⬇️

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]

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

[TheBlueMatt]

thanks

[ldk-reviews-bot]

👋 The first review has been submitted!

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

[wpaulino]

Each SpliceNegotiationFailed event originating from an InteractiveTxMsgError needs a NegotiationFailureReason, so it makes sense to track it in the same place. In most cases, the NegotiationFailureReason included uses the NegotiationError variant, but other cases may require their own specific variant, such as LocallyCanceled after calling ChannelManager::cancel_funding_contributed.

Fixes#4490 (comment)

[ldk-reviews-bot]

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

[ldk-claude-review-bot]

I've thoroughly reviewed the entire PR diff and explored the relevant codebase context. This is a clean refactoring that moves NegotiationFailureReason into InteractiveTxMsgError rather than passing it as a separate parameter to handle_interactive_tx_msg_err.

Key verification points:

• The into_parts() fallback via reason_from_channel_error produces the same output as the old inline format!("{:?}", err.err) — both format the ChannelError with its Debug impl.
• The one call site that previously passed Some(LocallyCanceled) (the cancel_funding_contributed handler) now has the reason embedded in both branches of cancel_funding_contributed() in channel.rs.
• All other call sites previously passed None, so the fallback behavior is unchanged.
• All 6 call sites of handle_interactive_tx_msg_err are updated to the new 4-parameter signature.
• The borrow pattern in into_parts() (borrowing err in the .map() closure then moving it into the return tuple) is correct — the borrow is released when .map() completes.

No issues found.

[codecov]

Codecov Report

❌ Patch coverage is 70.37037% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.11%. Comparing base (8882edd) to head (946ee09).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning/src/ln/channel.rs	67.34%	16 Missing ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #4609 +/- ##
==========================================
- Coverage 86.12% 86.11% -0.01% 
==========================================
Files 157 157 Lines 108824 108841 +17 Branches 108824 108841 +17 ==========================================
+ Hits 93721 93725 +4 - Misses 12487 12497 +10 - Partials 2616 2619 +3 

Flag	Coverage Δ
tests	86.11% <70.37%> (-0.01%)	⬇️

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]

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

[TheBlueMatt]

thanks

[ldk-reviews-bot]

👋 The first review has been submitted!

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

[wpaulino]

Each SpliceNegotiationFailed event originating from an InteractiveTxMsgError needs a NegotiationFailureReason, so it makes sense to track it in the same place. In most cases, the NegotiationFailureReason included uses the NegotiationError variant, but other cases may require their own specific variant, such as LocallyCanceled after calling ChannelManager::cancel_funding_contributed.

Fixes#4490 (comment)

[ldk-reviews-bot]

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

[ldk-claude-review-bot]

I've thoroughly reviewed the entire PR diff and explored the relevant codebase context. This is a clean refactoring that moves NegotiationFailureReason into InteractiveTxMsgError rather than passing it as a separate parameter to handle_interactive_tx_msg_err.

Key verification points:

• The into_parts() fallback via reason_from_channel_error produces the same output as the old inline format!("{:?}", err.err) — both format the ChannelError with its Debug impl.
• The one call site that previously passed Some(LocallyCanceled) (the cancel_funding_contributed handler) now has the reason embedded in both branches of cancel_funding_contributed() in channel.rs.
• All other call sites previously passed None, so the fallback behavior is unchanged.
• All 6 call sites of handle_interactive_tx_msg_err are updated to the new 4-parameter signature.
• The borrow pattern in into_parts() (borrowing err in the .map() closure then moving it into the return tuple) is correct — the borrow is released when .map() completes.

No issues found.

[codecov]

Codecov Report

❌ Patch coverage is 70.37037% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.11%. Comparing base (8882edd) to head (946ee09).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning/src/ln/channel.rs	67.34%	16 Missing ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #4609 +/- ##
==========================================
- Coverage 86.12% 86.11% -0.01% 
==========================================
Files 157 157 Lines 108824 108841 +17 Branches 108824 108841 +17 ==========================================
+ Hits 93721 93725 +4 - Misses 12487 12497 +10 - Partials 2616 2619 +3 

Flag	Coverage Δ
tests	86.11% <70.37%> (-0.01%)	⬇️

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]

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

[TheBlueMatt]

thanks

[ldk-reviews-bot]

👋 The first review has been submitted!

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

[wpaulino]

Each SpliceNegotiationFailed event originating from an InteractiveTxMsgError needs a NegotiationFailureReason, so it makes sense to track it in the same place. In most cases, the NegotiationFailureReason included uses the NegotiationError variant, but other cases may require their own specific variant, such as LocallyCanceled after calling ChannelManager::cancel_funding_contributed.

Fixes#4490 (comment)

[ldk-reviews-bot]

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

[ldk-claude-review-bot]

I've thoroughly reviewed the entire PR diff and explored the relevant codebase context. This is a clean refactoring that moves NegotiationFailureReason into InteractiveTxMsgError rather than passing it as a separate parameter to handle_interactive_tx_msg_err.

Key verification points:

• The into_parts() fallback via reason_from_channel_error produces the same output as the old inline format!("{:?}", err.err) — both format the ChannelError with its Debug impl.
• The one call site that previously passed Some(LocallyCanceled) (the cancel_funding_contributed handler) now has the reason embedded in both branches of cancel_funding_contributed() in channel.rs.
• All other call sites previously passed None, so the fallback behavior is unchanged.
• All 6 call sites of handle_interactive_tx_msg_err are updated to the new 4-parameter signature.
• The borrow pattern in into_parts() (borrowing err in the .map() closure then moving it into the return tuple) is correct — the borrow is released when .map() completes.

No issues found.

[codecov]

Codecov Report

❌ Patch coverage is 70.37037% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.11%. Comparing base (8882edd) to head (946ee09).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning/src/ln/channel.rs	67.34%	16 Missing ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #4609 +/- ##
==========================================
- Coverage 86.12% 86.11% -0.01% 
==========================================
Files 157 157 Lines 108824 108841 +17 Branches 108824 108841 +17 ==========================================
+ Hits 93721 93725 +4 - Misses 12487 12497 +10 - Partials 2616 2619 +3 

Flag	Coverage Δ
tests	86.11% <70.37%> (-0.01%)	⬇️

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]

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

[TheBlueMatt]

thanks

[ldk-reviews-bot]

👋 The first review has been submitted!

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

[wpaulino]

Each SpliceNegotiationFailed event originating from an InteractiveTxMsgError needs a NegotiationFailureReason, so it makes sense to track it in the same place. In most cases, the NegotiationFailureReason included uses the NegotiationError variant, but other cases may require their own specific variant, such as LocallyCanceled after calling ChannelManager::cancel_funding_contributed.

Fixes#4490 (comment)

[ldk-reviews-bot]

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

[ldk-claude-review-bot]

I've thoroughly reviewed the entire PR diff and explored the relevant codebase context. This is a clean refactoring that moves NegotiationFailureReason into InteractiveTxMsgError rather than passing it as a separate parameter to handle_interactive_tx_msg_err.

Key verification points:

• The into_parts() fallback via reason_from_channel_error produces the same output as the old inline format!("{:?}", err.err) — both format the ChannelError with its Debug impl.
• The one call site that previously passed Some(LocallyCanceled) (the cancel_funding_contributed handler) now has the reason embedded in both branches of cancel_funding_contributed() in channel.rs.
• All other call sites previously passed None, so the fallback behavior is unchanged.
• All 6 call sites of handle_interactive_tx_msg_err are updated to the new 4-parameter signature.
• The borrow pattern in into_parts() (borrowing err in the .map() closure then moving it into the return tuple) is correct — the borrow is released when .map() completes.

No issues found.

[codecov]

Codecov Report

❌ Patch coverage is 70.37037% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.11%. Comparing base (8882edd) to head (946ee09).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning/src/ln/channel.rs	67.34%	16 Missing ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #4609 +/- ##
==========================================
- Coverage 86.12% 86.11% -0.01% 
==========================================
Files 157 157 Lines 108824 108841 +17 Branches 108824 108841 +17 ==========================================
+ Hits 93721 93725 +4 - Misses 12487 12497 +10 - Partials 2616 2619 +3 

Flag	Coverage Δ
tests	86.11% <70.37%> (-0.01%)	⬇️

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]

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

[TheBlueMatt]

thanks

[ldk-reviews-bot]

👋 The first review has been submitted!

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

[wpaulino]

Each SpliceNegotiationFailed event originating from an InteractiveTxMsgError needs a NegotiationFailureReason, so it makes sense to track it in the same place. In most cases, the NegotiationFailureReason included uses the NegotiationError variant, but other cases may require their own specific variant, such as LocallyCanceled after calling ChannelManager::cancel_funding_contributed.

Fixes#4490 (comment)

[ldk-reviews-bot]

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

[ldk-claude-review-bot]

I've thoroughly reviewed the entire PR diff and explored the relevant codebase context. This is a clean refactoring that moves NegotiationFailureReason into InteractiveTxMsgError rather than passing it as a separate parameter to handle_interactive_tx_msg_err.

Key verification points:

• The into_parts() fallback via reason_from_channel_error produces the same output as the old inline format!("{:?}", err.err) — both format the ChannelError with its Debug impl.
• The one call site that previously passed Some(LocallyCanceled) (the cancel_funding_contributed handler) now has the reason embedded in both branches of cancel_funding_contributed() in channel.rs.
• All other call sites previously passed None, so the fallback behavior is unchanged.
• All 6 call sites of handle_interactive_tx_msg_err are updated to the new 4-parameter signature.
• The borrow pattern in into_parts() (borrowing err in the .map() closure then moving it into the return tuple) is correct — the borrow is released when .map() completes.

No issues found.

[codecov]

Codecov Report

❌ Patch coverage is 70.37037% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.11%. Comparing base (8882edd) to head (946ee09).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning/src/ln/channel.rs	67.34%	16 Missing ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #4609 +/- ##
==========================================
- Coverage 86.12% 86.11% -0.01% 
==========================================
Files 157 157 Lines 108824 108841 +17 Branches 108824 108841 +17 ==========================================
+ Hits 93721 93725 +4 - Misses 12487 12497 +10 - Partials 2616 2619 +3 

Flag	Coverage Δ
tests	86.11% <70.37%> (-0.01%)	⬇️

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]

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

[TheBlueMatt]

thanks

[ldk-reviews-bot]

👋 The first review has been submitted!

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