Skip to content

Shutdown retransmission clarifications #199

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

Closed
rustyrussell wants to merge 2 commits into from
Closed

Shutdown retransmission clarifications #199

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
55ac90c
BOLT 2: don't require revoke_and_ack rexmit if it's bogus.
rustyrussell Jul 3, 2017
975fcdf
BOLT 2: don't require shutdown re-xmit if we're doing closing_signd r…
rustyrussell Jul 3, 2017
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
17 changes: 11 additions & 6 deletions 02-peer-protocol.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -980,7 +980,7 @@ last `commitment_signed` message the receiving node has sent, it
SHOULD fail the channel.

If `next_remote_revocation_number` is equal to the commitment number of
the last `revoke_and_ack` the receiving node has sent, it MUST re-send
the last `revoke_and_ack` the receiving node has sent and the receiving node has not already received a `closing_signed`, it MUST re-send
Copy link
Copy Markdown
Collaborator

@pm47 pm47 Jul 22, 2017

Choose a reason for hiding this comment

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

As you state in the rationale below, the sender won't send a closing_signed until it has sent the last revoke_and_ack.

Thus I don't see how this scenario is possible?

Copy link
Copy Markdown
Collaborator Author

@rustyrussell rustyrussell Jul 24, 2017

Choose a reason for hiding this comment

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

Yes, exactly. It shouldn't happen, thus you don't need to handle it.

Copy link
Copy Markdown
Collaborator

@pm47 pm47 Jul 24, 2017

Choose a reason for hiding this comment

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

Should we mention it though? There are an infinity of other scenarii that shouldn't happen, that we don't specify. Why is this specific one more important than the others? I might be missing something but to me it isn't really different from a node sending, say, two funding_locked in a row. That sounds silly I know but my concern is consistency.

How about stating somewhere else more generally what a node should do when the counterparty violates the order of messages of the protocol, eg. always fail the channel? I think that would be a reasonable thing, since AFAICT we were careful to specify when to ignore messages.

the `revoke_and_ack`, otherwise if `next_remote_revocation_number` is not
equal to one greater than the commitment number of the last `revoke_and_ack` the
receiving node has sent (or equal to zero if none have been sent), it SHOULD fail the channel.
Expand All @@ -992,9 +992,9 @@ transaction is broadcast by the other side at any time. This is
particularly important if a node does not simply retransmit the exact
same `update_` messages as previously sent.

On reconnection if the node has sent a previous `shutdown` it MUST
retransmit it, and if the node has sent a previous `closing_signed` it
MUST then retransmit the last `closing_signed`.
On reconnection if the node has sent a previous `closing_signed` it
Copy link
Copy Markdown
Collaborator

@Roasbeef Roasbeef Jul 10, 2017

Choose a reason for hiding this comment

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

Why do we even require sending an identical closing_signed message? I don't see a reason to make nodes worry about an additional layer of persistence.

It seems that upon re-connect, the closing negotiation should just start from scratch (similar to a failed funding negotiation). It also could be the case the the fee preferences of either side have changed significantly since the last connection.

Copy link
Copy Markdown
Collaborator Author

@rustyrussell rustyrussell Jul 29, 2017

Choose a reason for hiding this comment

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

Done, added new patch.

MUST then retransmit the last `closing_signed`, otherwise if the node
has sent a previous `shutdown` it MUST retransmit it.

### Rationale

Expand All @@ -1011,8 +1011,8 @@ polite to retransmit before disconnecting again, but it's not a MUST
because there are also occasions where a node can simply forget the
channel altogether.

There is similarly no acknowledgment for `closing_signed`, or
`shutdown`, so they are also retransmitted on reconnection.
`closing_signed` can only be sent after `shutdown`, so only one needs
to be retransmitted on reconnection.

The handling of updates is similarly atomic: if the commit is not
acknowledged (or wasn't sent) the updates are re-sent. However, we
Expand All @@ -1023,6 +1023,11 @@ write to disk by the sender upon each transmission, whereas the scheme
here encourages a single persistent write to disk for each
`commitment_signed` sent or received.

We should never be asked to retransmit `revoke_and_ack` if we've
received a `closing_signed`, since that implies we've completed
shutdown which can only happen once the `revoke_and_ack` was received
by the remote node.

Note that the `next_local_commitment_number` starts at 1 since
commitment number 0 is created during opening.
`next_remote_revocation_number` will be 0 until the
Expand Down