Try to reword Dilation specification #33
Conversation
We want this repository to be agnostic of any site generators or similar. Information related to rendering the markdown as web page should be stored externally.
It doesn't seem to be reference anywhere and also looks like a prototype/sketch.
Closes #30. Those files also contained a couple of paragraphs – mainly the general introduction – that will have to be rewritten in a client-agnostic style.
piegamesde
force-pushed
the
dilation-reword
branch
from
05d7f48 to
2ffbf5a
Compare
Rewrote the existing topics. Tried to minimize the discussion of any hypotheticals and future work, as this should be done outside of the document itself. Added privacy sections discussing file size metadata and IP addresses.
- Deprecate the `current_cli_version` key - Improved wording, added type information
Slowly make the naming more consistent.
semantic newlines; document what the implementation does; versions are integers
update (just) the Dilation abilities section
At the moment it is written more as the documentation of the Python implementation. In the end, we want it to be a standalone document that helps others implement that protocol too, without having to know any of the Python internals or Twisted.
piegamesde
force-pushed
the
dilation-reword
branch
from
2ffbf5a to
68c6494
Compare
piegamesde
changed the title
There was a problem hiding this comment.
I still see benefits in keeping layers distinct, or at least describing them that way. Naming aside, grouping the different concerns I think helps implementers -- knowing that the L4 layer doesn't have to care about anything to do with channel-selection, encryption, deserialization, framing etc. Basically all it is concerned with is the high-level messages and their acknowledgement.
We don't have to strictly follow what's already there, but some separation of concerns is a big benefit I think. For example, you have a "todo" about ordering of subchannel vs seq-num serialization, but that doesn't matter because you've already gotten "one whole message" from L2 so it's irrelevant what order the bytes are in.
Have you read the latest draft of the python specification? Also it would be good to link that in this document.
(Some inline comments too of course :)
dilation-protocol.md
Outdated
| Additionally, the protocol takes care of automatically re-establishing such a channel after a connection interruption, | ||
| including re-sending all messages that got lost. | ||
| Furthermore, it has the concept of sub-channels built in, | ||
| and will take care of the sub-channel management and data multiplexing too. |
There was a problem hiding this comment.
This "kind of" gets at the "durability" aspect, but I feel that should be stated more clearly.
What you get from the subchannels is a stream of data that will definitely be delivered to the other side even changing networks, etc, until one side gives up or both sides are disconnected from the mailbox for "too long".
dilation-protocol.md
Outdated
| It is additionally used by the Dilation implementation to exchange internal coordination messages. | ||
|
|
||
| The usual Transit connection is replaced by a Dilation connection, that is managed by the Dilation implementation. | ||
| Compared to the Transit connection, it has additional features like resilience against connection interruptions and sub-channels. |
There was a problem hiding this comment.
I guess this hints at durability too, but similar to above doesn't quite state that?
There was a problem hiding this comment.
Hm, what would you like to hear here?
dilation-protocol.md
Outdated
| * 0x04 CLOSE, 4-byte subchannel-id, 4-byte seqnum | ||
| * 0x05 ACK, 4-byte response-seqnum | ||
|
|
||
| TODO it would make more sense to have the seqnum before the subchannel-id, processing wise ~piegames |
There was a problem hiding this comment.
Why? Also you have to deserialize all of this to process it, so I can't see why order matters at all.
There was a problem hiding this comment.
I agree that it mostly doesn't matter, this is just me being pedantic.
|
|
||
| ### Flow Control | ||
|
|
||
| TODO review and potentially remove? ~piegames |
There was a problem hiding this comment.
I would say: absolutely not, flow-control is pretty central to streaming applications.
There was a problem hiding this comment.
Basically, as far as I can see flow control is something for the Dilation implementation to handle, and requires no protocol support at all. So if you remove the Python specifics from the paragraph, it will go something like "Implementations might offer some flow-control functionality or quality of service for the sub-channels, so that a single very busy channel does not starve the others. This is not a requirement though, as it significantly increases implementation complexity and most applications do just fine without".
There was a problem hiding this comment.
No, it would still say the stuff about pausing ALL channels whenever a single one needs to provide backpressure. That is, that implementations should not attempt to provide their own subchannel flow-control, and instead rely on TCP's. (Competing flow-controllers on the same channel do not play nicely)
There was a problem hiding this comment.
Oh I see, I think I was confusing flow control and quality of service. (Tbf not entirely my fault, because the paragraph also talks about the one while actually describing the latter.)
So is the takeaway here "don't do your own flow control but feel free to implement QoS"?
There was a problem hiding this comment.
The original (https://github.com/magic-wormhole/magic-wormhole/blob/master/docs/dilation-protocol.md#flow-control) doesn't mention QoS at all, and I wouldn't mention it here.
There was a problem hiding this comment.
I know but it talks about QoS: "e.g. a chat conversation sharing a wormhole with file-transfer might prefer the IM text to take priority". Therefore why not talk about that explicitly?
There was a problem hiding this comment.
I guess that's certainly a use-case for QoS -- but what do you think is worth mentioning here? There's nothing to be done by implementers -- they have to pause "the TCP stream" to achieve backpressure.
If anything, this would be an interesting point for application developers: mixing high-bandwidth subchannels with low-bandwith ones may perform poorly under bad network conditions. But I think all they can do here is "not do that"?
That is, I don't believe that sentence is supposed to imply to Dilation implementers that they should attempt to implement some sort of QoS system. I don't recall details on how that'll interact (or not) with TCP windowing etc. (Usually QoS is done per-TCP-stream, e.g. prioritizing one whole stream over another).
dilation-protocol.md
Outdated
| This involves sending a `reconnect` message and according hints over the Mailbox connection, | ||
| the same way as for initially establishing the connection. | ||
| Once a new connection is established and the handshake completes, | ||
| both sides re-transmit their outbound messages that were lost due to the connection interruption. |
There was a problem hiding this comment.
Careful here: these are L4 messages. If you instead re-transmitted L2 messages, the crypto may break (and another reason to use distinct keys for different generations).
There was a problem hiding this comment.
(I see you resolved this, but I don't seen changes .. maybe your workflow is different from mine).
This should also say how a peer determines which messages were lost (or potentially lost).
There was a problem hiding this comment.
Oops I'm sorry.
I usually keep track of open discussions as my TODO list (at least for things that are easily fixable), but I forgot to push my changes after doing so.
There was a problem hiding this comment.
Unresolving because of "This should also say how a peer determines which messages were lost"
|
Thanks for the review
I don't like the concept of "layer" because to me it invokes feeling of an "OSI layers" stack, which is only true for parts of it. I'd rather think of them as interdependent components, and treat them as such. But you are right, the new wording could make the separation a bit more clear again. One thing to note is that sometimes the boundaries are a bit fuzzy: For example while Python has L5 stacked on top of L4, one could also write the code that mostly does both at once, but delegates parts of it to a "subchannel manager". (Basically the reverse from what Python does.)
Not really no, these changes here are mostly a rebase from my work in last autumn. I think it has changes over there that I should incorporate here? (If possible, I'd like to delegate that too to a future PR) |
|
Yes, they're like the OSI stack: a "higher layer" can depend on a lower one, but NOT vice-versa. This is a good thing. So they're not "inter"-dependent. It's fine if you want to assign different names, I suppose, but for network protocol implementers the OSI stack is very familiar and talking about "layers" is pretty common in lots of Internet protocols. It also lets you forget / compartmentalize complexity: I can forget about framing, encryption, ACKs, re-sends, etc etc at the higher layers and just think about the logical flow of in-order messages. It also helps with state-machine diagrams and separation. |
At the moment it is written more as the documentation of the Python implementation. In the end, we want it to be a standalone document that helps others implement that protocol too, without having to know any of the Python internals or Twisted.