Introduce interactive signing state flags for funded states.
This PR includes some deferred follow-ups extracted from #3423 and introduces new state flags to track interactive signing along with persistence of the minimum information needed from a signing session to reconstruct it.
A top-level state flag was avoided so that this work is compatible with splicing as well as V2 channel establishment (dual-funding).
👋 Thanks for assigning @jkczyz 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.
Did you want to include test coverage for restarts here?
Not yet. Tracked in #3636. Will need to be able to contribute inputs first to test a useful order of message exchange + restart.
Codecov Report
Attention: Patch coverage is 64.55026% with 67 lines in your changes missing coverage. Please review.
Project coverage is 90.01%. Comparing base (
89f5217) to head (fb00cb6). Report is 19 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@
## main #3637 +/- ##
==========================================
+ Coverage 89.37% 90.01% +0.63%
==========================================
Files 157 157
Lines 124095 129376 +5281
Branches 124095 129376 +5281
==========================================
+ Hits 110915 116452 +5537
+ Misses 10469 10187 -282
- Partials 2711 2737 +26
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
@dunxen re-request when this is ready for review again, feel free to squash as well
🔔 1st Reminder
Hey @TheBlueMatt @jkczyz @wpaulino! This PR has been waiting for your review. Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.
Taking myself off since @wpaulino and @jkczyz are on this one. Aside from my first comment I don't have any more high-level feedback.
🔔 1st Reminder
Hey @jkczyz @wpaulino! This PR has been waiting for your review. Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.
🔔 2nd Reminder
Hey @jkczyz @wpaulino! This PR has been waiting for your review. Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.
Just pushed the change for full persistence of InteractiveTxConstructor. Still addressing other feedback.
@dunxen let us know when this is ready for another round
@wpaulino, @jkczyz, sorry for the delay. This is ready for another round.
🔔 1st Reminder
Hey @jkczyz @wpaulino! This PR has been waiting for your review. Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.
🔔 2nd Reminder
Hey @jkczyz @wpaulino! This PR has been waiting for your review. Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.
🔔 1st Reminder
Hey @jkczyz @wpaulino! This PR has been waiting for your review. Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.
🔔 1st Reminder
Hey @jkczyz @wpaulino! This PR has been waiting for your review. Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.
🔔 2nd Reminder
Hey @jkczyz @wpaulino! This PR has been waiting for your review. Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.
🔔 2nd Reminder
Hey @jkczyz @wpaulino! This PR has been waiting for your review. Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.
Looks like there's some breakages in the
full_stackfuzz test.
Thanks, will address those. Honestly haven't paid much attention as fuzz had been broken for a while on main I believe.
Hmm... when possible please avoid rebasing unless necessary. Otherwise, it makes it difficult to view new changes related to the PR. If needed, doing two separate pushes would be ideal.
Thanks, will address those. Honestly haven't paid much attention as fuzz had been broken for a while on main I believe.
Locally, it still should work. CI has just been timing out (unfortunately) but will catch actual failures, AFAICT.
Locally, it still should work. CI has just been timing out (unfortunately) but will catch actual failures, AFAICT.
Oh yeah I see this was just related to the new bool in config.
Hmm... when possible please avoid rebasing unless necessary.
Sorry, bad habit I do sometimes. I'll keep it in mind again.
Changes:
git diff-tree -U1 4f6192f 21a25fb3f
diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
index b43e47e68..a60f3f293 100644
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -5849,4 +5849,3 @@ impl<SP: Deref> FundedChannel<SP> where
- let need_channel_ready = self.check_get_channel_ready(0, logger).is_some();
- self.monitor_updating_paused(false, false, need_channel_ready, Vec::new(), Vec::new(), Vec::new());
+ self.monitor_updating_paused(false, false, false, Vec::new(), Vec::new(), Vec::new());
@@ -6496,3 +6495,3 @@ impl<SP: Deref> FundedChannel<SP> where
if !matches!(self.context.channel_state, ChannelState::FundingNegotiated(flags) if flags.is_interactive_signing()) {
- return Err(ChannelError::close("Received tx_signatures in strange state!".to_owned()));
+ return Err(ChannelError::Ignore("Ignoring tx_signatures received outside of interactive signing".to_owned()));
}
🔔 1st 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.
Changes:
git diff-tree -U1 7d19a76 21d3c78
diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index ecec47387..93dd16e7d 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -11989,3 +11989,3 @@ where
fn handle_open_channel_v2(&self, counterparty_node_id: PublicKey, msg: &msgs::OpenChannelV2) {
- if !self.node_features().supports_dual_fund() {
+ if !self.init_features().supports_dual_fund() {
let _: Result<(), _> = handle_error!(self, Err(MsgHandleErrInternal::send_err_msg_no_close(
🔔 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.
🔔 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.
🔔 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.
🔔 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.
🔔 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.