Message flows and endpoint state management (Was: ANNOUNCE)

[martinduke]

Fixes #556
Fixes #557
Fixes #583
Fixes #584

Also eliminates text from the relay section that is redundant, because it applies to endpoints whether or not they are relays.

[ianswett]

I still find some of the ANNOUNCE flow overly complex, but this does a good job of describing how you might use the defined messages.

[afrind]

Individual Review:

Thanks for tackling this. There's a lot going on here.

Moving the error codes is helpful but seems like a great candidate for a standalone editorial PR.

There are some normative changes to relay handling not related to ANNOUNCE which should also be in a separate PR.

There's a subtle deletion of the "exact" namespace matching behavior. This is less problematic in the face of structured namespaces, but I forget if that was something we discussed and had consensus on?

[martinduke]

Individual Review:

Thanks for tackling this. There's a lot going on here.

Moving the error codes is helpful but seems like a great candidate for a standalone editorial PR.

This PR is already heavily editing the relay section for redundant and poorly located text. This falls into that theme; it is not specific to relays.

There are some normative changes to relay handling not related to ANNOUNCE which should also be in a separate PR.

Again, a huge rewrite of the Relay section is in scope for this.

There's a subtle deletion of the "exact" namespace matching behavior. This is less problematic in the face of structured namespaces, but I forget if that was something we discussed and had consensus on?

I don't see how structured namespaces can work with exact matching.

[afrind]

This looks better. A bunch of editorial comments/suggestions. The substantive comment is about superset.

[martinduke]

The flexibility built into this architecture can lead to routing loops.

Take relays A, B, and C, which are connected to each other.

C sends SUBSCRIBE_ANNOUNCES for namespace Foo to B, but not A. B sends ("forwards") SUBSCRIBE_ANNOUNCES to A for Foo.

Then A gets an ANNOUNCE for Foo from Upstream. it sends it to B, which passes it on to C. C forwards that ANNOUNCE unsolicited to A. Now A has a backup path to Foo, so even if its upstream session dies, it will not send UNANNOUNCE.

Even without SUBSCRIBE_ANNOUNCE, there could be this chain of events.

One way out of this would be to require SUBSCRIBE_ANNOUNCES to send an ANNOUNCE.

[afrind]

Minor nits, lgtm.

[martinduke]

Minor nits, lgtm.

The SUBSCRIBE_DONE thing is intended! It's part of the logic this protocol describes.

[afrind]

The SUBSCRIBE_DONE thing is intended! It's part of the logic this protocol describes.

"Still there" was supposed to refer to a non-normative should on line 1411. Removing the SUBSCRIBE_DONE ack to UNSUBSCRIBE seems fine to me.

[martinduke]

Deleted #577 from the list of issues, since that is now solved by #656

[martinduke]

The SUBSCRIBE_DONE thing is intended! It's part of the logic this protocol describes.

"Still there" was supposed to refer to a non-normative should on line 1411. Removing the SUBSCRIBE_DONE ack to UNSUBSCRIBE seems fine to me.

Ah, got it, fixed.

[martinduke]

I removed the "subscriber" and "publisher" definitions I just added, because these terms are already well defined in Section 1.2

[ianswett]

I and the chairs acknowledge that this might not be perfect, but it clarifies the behavior in a meaningful way.