Fix Recovery Pseudocode

[ianswett]

Fixes #2906

[martinthomson]

When do you use parentheses?

[ianswett]

It appears most conditionals have parentheses currently, except for 1177. Are you suggesting to add them there or something else?

[martinthomson]

Pick one. Be consistent.

[ianswett]

SG, done. I also fixed some missing ':'

[martinduke]

endpoint is server or has received hs ack or an ack of an 1-RTT packet (not a 1-RTT packet, this could be an ack of 0RTT)

[martinduke]

Sorry, just add "has received HS ack" and we're good. There's a case where the server flight doesn't fit in a cwnd and the extra text may save us a crypto timeout.

[janaiyengar]

You'll need to rebase to get the changes from #2912 in here.

[janaiyengar]

This looks good to me, module rebasing and working out the conflicts.

[janaiyengar]

Chatting about this at the editors meeting, @martinthomson pointed out that this isn't quite right. Here's my reformulation that I believe is the right condition:
"endpoint is server || until the handshake is confirmed || has received Handshake ACK"

[martinthomson]

The prose I would use here is:

An endpoint always keeps the timer running if it has an ack-eliciting packet that has not been acknowledged.

A client keeps the timer running during the handshake until it has confirmed that the server is no longer limited in how much it can send. The server needs to acknowledge a Handshake packet or a 1-RTT packet from the client to demonstrate this.

[janaiyengar]

Suggestion: if !(ack-eliciting packet in flight || (endpoint is client && has not received handshake ACK && has not received 1-RTT ACK))

[ianswett]

As a code reviewer, I'd request a method like PeerAwaitingAddressValidation() and say

if (no ack-eliciting packets in flight &&
!PeerAwaitingAddressValidation()):

[janaiyengar]

Thanks for the change -- looks MUCH better now.

[martinthomson]

I shouldn't have commented on the changeset. Here's my alternative to Jana's:

# Whether the peer has provided evidence that their
# path validation is complete during the handshake.
PeerCompletedAddressValidation():
  # Assume that clients always validate the server implicitly.
  if endpoint is server:
    return true
  # Otherwise path validation is completed when a
  # protected packet is received, so look for an ACK.
  return has received Handshake ACK ||
           has received 1-RTT ACK
...

  if (no ack-eliciting packets in flight &&
      PeerCompletedAddressValidation()):