Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Keep track on which side initiated a mutual close #3042

Merged
merged 3 commits into from
Mar 21, 2025
Merged

Conversation

pm47
Copy link
Member

@pm47 pm47 commented Mar 20, 2025

We currently lose this information once we transition to SHUTDOWN (in NORMAL, we can compute it based on which shutdown is present).

I tried two approaches in two successive commits:

  • Replace Option[ClosingFeerates] by Option[CloseInitiated] (which itself stores an Option[ClosingFeerates]. This approach is very close to what we have currently, but ambiguous in the non-initiator case (there is no difference between "no close in progress" and "a close as non-initiator"). This ambiguity makes the backward compat code simpler.
  • Introduce a proper CloseStatus. This approach is a bit more involved, but also unambiguous and probably easier for the reader. The CloseStatus is optional in NORMAL state, mandatory in SHUTDOWN state.

@pm47 pm47 requested a review from t-bast March 20, 2025 14:59
@pm47 pm47 marked this pull request as draft March 20, 2025 15:02
@pm47 pm47 removed the request for review from t-bast March 20, 2025 15:02
This change is a little bit more involved, but is also cleaner and
easier to understand for the reader, as the initiator status is
explicit.

We also solve an ambiguity where we would bump the fee of a pending
mutual close that we didn't initiate.
@pm47 pm47 force-pushed the close-initiator branch from 7e9e03d to fdd3ff3 Compare March 20, 2025 15:14
@pm47 pm47 marked this pull request as ready for review March 20, 2025 15:19
@pm47 pm47 requested a review from t-bast March 20, 2025 15:19
Copy link
Member

@t-bast t-bast left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, this is simple enough and we can later add more data to the CloseStatus if needed.

@pm47 pm47 merged commit 9210dff into master Mar 21, 2025
1 check passed
@pm47 pm47 deleted the close-initiator branch March 21, 2025 12:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants