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.

Sign up for GitHub

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

Jump to bottom

BOLT 2: quiescence protocol (feature 34/35) option_quiesce #869

Merged
merged 3 commits into from
Jun 17, 2024
Merged

BOLT 2: quiescence protocol (feature 34/35) option_quiesce #869

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
dc8728a
BOLT 2: quiescence protocol.
rustyrussell Jun 11, 2024
b474755
require quiescence sessions to be single-use
ProofOfKeags May 20, 2024
ccaa4e6
BOLT 2: add requirement for disconnect if quiescence takes too long w…
rustyrussell Jun 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions 02-peer-protocol.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1465,6 +1465,9 @@ The receiver of `stfu`:
- SHOULD NOT send any more update messages.
t-bast marked this conversation as resolved.
Show resolved Hide resolved
- MUST reply with `stfu` once it can do so.

Copy link

Choose a reason for hiding this comment

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

Maybe you should add, "must now consider the channel as quiescent here as well" ?

Copy link
Contributor

Choose a reason for hiding this comment

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

You can't do that because quiescence is only achieved once both sides have sent it.

Copy link

Choose a reason for hiding this comment

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

Yeah both sides have sent it, if a peer receives stfu and has sent one before, the peer must now consider the channel as quiescent (This is already stated in the spec, line 527)

The otherwise branch(L528-530), where the peer receives but has not sent one and so replies, I think it should also be stated that peer must now consider the channel as quiescent as well, just as in the other scenario.

Copy link
Contributor

Choose a reason for hiding this comment

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

It's redundant as that case is already covered by the requirements on the sender

Copy link

Choose a reason for hiding this comment

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

I do not think it is. I could not find anywhere in the sender perspective where it is stated that the channel should now be considered as quiescent when stfu is sent. Rather, the channel is considered as quiescing in that case, line 522. I think when stfu is sent and received that is when the channel is considered quiescent?

Copy link
Contributor

Choose a reason for hiding this comment

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

Good catch. I will add this to the sender requirements.

Copy link

Choose a reason for hiding this comment

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

I think we need to add it when the sender has received a reply so it might be more appropriate here, but maybe there is a better way to structure it.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand. Is it quiescent only when you've send AND received the message? Or only when you send it?

Both nodes:
- MUST disconnect after 60 seconds of quiescence if the HTLCs are pending.
Comment on lines +1468 to +1469
Copy link
Contributor

Choose a reason for hiding this comment

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

I know this has already been merged but I do not believe this is a MUST. This is up to node policy and can be initiated by either side. Especially considering that time is both relative and difficult in distributed systems this ought to be a SHOULD

Copy link
Contributor

Choose a reason for hiding this comment

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

What is "pending" here? I believe it is a different definition than the one above.


Upon disconnection:
- the channel is no longer considered quiescent.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is that the only condition under which a channel can get out of being considered quiescent?

I guess future protocols that need a quiescent channel end with stopping the channel to be considered quiescent. But maybe this could also be done with a separate message or an additional explicit field in the stfu that resets the request to be quiescent.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Indeed. For now we only use this for channel upgrade: other changes may have different needs.

Copy link
Contributor

Choose a reason for hiding this comment

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

I probably missed something but why is a disconnect (of the connection) necessary for a channel upgrade? Do the peers after upgrade need to execute the routines in connection reestablishment?

Copy link

Choose a reason for hiding this comment

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

Whether we add a new message that explicitly ends quiescence as suggested by @ProofOfKeags or leave it to be described in each downstream protocol, I think we need to say you can exit quiescence without sending a disconnect.

I do not think all downstream protocols should be forced to trigger a disconnect to leave quiescence.

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe this is addressed.


Expand Down