Waku Mail Client implementation part II

[kdeme]

The Waku (Status Whisper fork) mail client implementation was started here: status-im/nim-eth#141

This issue is about improving the implementation by addressing the many todos (some from review) in the code.

However, that work showed some pain points that could/should be improved but ideally this will require some specification changes:

• To be able to make use of the requestResponse mechanism of the DSL the p2pRequest packet and the p2pRequestComplete packet should have consecutive ids. While it would be possible to alter the DSL to support the current situation, the specification would benefit from having them consecutive, as it would be more clear for implementers that they are a request & response type of message.
• The p2pRequest wraps its data structure in a regular Whisper/Waku envelope. The p2pRequestComplete does not do this. It would be an improvement if both were wrapped in an envelope, this would make it more consistent and allow for the format to be agnostic for the Waku messaging protocol. Furthermore, if necessary, it would allow for encryption as with regular messages.
• The data structure of p2pRequestComplete contains a (custom) requestId. If the requestResponse system is used, we would have default access to such system. The les protocol already follows such practise: https://github.com/ethereum/devp2p/blob/master/caps/les.md#request-id . It would make sense to work in the same way here, especially if we move that data structure into an envelope, the requestId does not fit on that layer.
• Not a specification change, but rather missing of specification. It is not completely clear in case of P2P message (p2pRequest, p2pRequestComplete) which validation should be done on the envelope (in second case, there is no envelope..), if any at all.

[adambabik]

To be able to make use of the requestResponse mechanism of the DSL the p2pRequest packet and the p2pRequestComplete packet should have consecutive ids.

What do you mean by consecutive here? The easiest to make sure it's a request-response flow would be if they share the same ID, like in JSON-RPC. I agree that the current way of doing it is awful but it kinda follows the request-response mechanism. The request is an envelope and envelope's hash is provided in the response message.

The p2pRequest wraps its data structure in a regular Whisper/Waku envelope. The p2pRequestComplete does not do this. It would be an improvement if both were wrapped in an envelope, this would make it more consistent and allow for the format to be agnostic for the Waku messaging protocol. Furthermore, if necessary, it would allow for encryption as with regular messages.

I don't see much value in using envelopes in p2p messages. Also, when sending a payload wrapped in an envelope both parties must agree on the payload data structure anyway. Today we discussed a possibility to change to protobuf to unify the serialization/deserialization part.

The data structure of p2pRequestComplete contains a (custom) requestId. If the requestResponse system is used, we would have default access to such system. The les protocol already follows such practise

👍

Not a specification change, but rather missing of specification. It is not completely clear in case of P2P message (p2pRequest, p2pRequestComplete) which validation should be done on the envelope (in second case, there is no envelope..), if any at all.

I would like to hear some arguments why using an envelope is beneficial for p2p messages. the Envelope data structure is optimised for gossip protocol. If we want something closer to request-response flow, we should design another data structure with requestID and other necessary properties (LES can be a good example, maybe; JSON-RPC spec is another). In RLPx, we could have something like: [ reqPacketCode, reqID, packetSpecificDataStructure ] and [ respPacketCode, reqID, packetSpecificDataStruct ] for the request-response flow. It is trivial but it is a bit painful if we want to extend it with other common properties different from reqID.

[zah]

What do you mean by consecutive here? The easiest to make sure it's a request-response flow would be if they share the same ID, like in JSON-RPC. I agree that the current way of doing it is awful but it kinda follows the request-response mechanism. The request is an envelope and envelope's hash is provided in the response message.

The discussed ID here is the fixed numerical ID of the RLPx network message, not the changing run-time ID of each individual request. Let's elaborate with an example - in the LES protocol, the getBlockBodies request has a numeric ID of 0x4 and its response blockBodies has the ID 0x5. Both of them carry an additional reqID integer as part of the message. It's customary for the fixed message IDs to be assigned in consecutive order and for the run-time reqID to appear as the first parameter. We exploit such regularities to create high-level implementations of the RLPx protocols that are able to hide most of the complexity which you'll find in Geth. If we follow the established practices, we (and other teams) will be able to do the same with Waku as well. For the same reason, we should stick to using RLP as the serialisation format.

I'll let @kdeme comment on the question regarding the use of Envelope.

[kdeme]

Yes, regarding the use of the Envelope. It is not that I am bound to using only Whisper Envelopes, we can change it to another data structure that is better. I just suggested this one as it exists and is already used for the P2P request, but yes it is not really designed for these request responses.

I just wanted to point out that it would probably be better if P2P Request complete data gets wrapped in some structure (e.g. can be as simple as an RLP blob I guess, and then the same for the P2P request of course). Reasoning would be that currently the mail server implementation is "mostly" agnostic to the transport. Whisper/Waku does not know about its details. Except for the P2P request complete packet currently. If we make this consistent however, then it really doesn't matter how we encode inside the structure, we can do RLP for now, change to Protobuf later or something else, no impact on the Waku/Whisper code.
And vice versa when we want to swap Waku/Whisper for any other transport with whichever encoding used there.
And the same counts for any encryption scheme we decide on in that layer.

It would be either that, or move all the mailserver parts to be on the layer of Whisper to make it consistent (no Envelope in P2P request), but I believe this would be less beneficial.

[oskarth]

Thanks @zah, TIL :)

[adambabik]

That sounds good to me @kdeme.

It would be either that, or move all the mailserver parts to be on the layer of Whisper to make it consistent (no Envelope in P2P request), but I believe this would be less beneficial.

Agree. I would rather have a wrapping data structure but just something tailored for request-response rather than Envelope.

Happy to see a proposal about how the wrapping data structure for request-response flow could look like and how it would fit into RLPx.

[oskarth]

Also cc @iurimatias for client part