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

Rework channel_reestablish requirements #1051

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

t-bast
Copy link
Collaborator

@t-bast t-bast commented Jan 11, 2023

This is an alternative to #1049 which changes the requirements more in-depth. I recommend reviewing it commit-by-commit and using a markdown viewer.

I started from our code and rewrote the requirements based on that. I'm trying to more clearly express the fact that the only possible errors cases are:

  • we've lost data and our peer correctly proved it (by showing our valid future commitment secret): we absolutely must not broadcast our (revoked) commitment and must ask our peer to kindly broadcast theirs
  • we may have lost data, but our peer couldn't prove it: we should probably not broadcast our commitment, because it may or may not be revoked, but if our peer doesn't broadcast theirs either, we may be forced to eventually broadcast if we're confident that we haven't lost data (but that should be a manual step from the node operator)
  • our peer tried to make us think we lost data but they lied (by showing an invalid commitment secret): they're malicious, we should call their bluff: just send an error and wait for them to force-close (unless the node operator steps in to manually force close)
  • our peer seems to have lost data: they will realize it when receiving our channel_reestablish, we should wait for them to either send an error (if they want us to force-close), or reconnect and send a different channel_reestablish (if they've fixed their state backup)

This is harder to express in spec than in code, I may have gotten it wrong or missed a case, so please spend some time carefully reviewing this!

This should close #934 once and for all 🎉

This commit only contains cosmetic whitespace changes: statements weren't
 consistently aligned. I harmonized them as I find that
it helps readability,
especially with complex nested conditions.
It's hard to follow when requirements are scattered in too many sections.
Rework the `channel_reestablish` receiver requirements: we first handle
all error cases, then specify what should be retransmitted.
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.

Nodes shouldn't publish their commitment when receiving outdated channel_reestablish
1 participant