Add serde support

[shekohex]

Closes #14

[drewstone]

CC @survived if you're able to help merge this

[tbraun96]

Yes, please merge. We need it too.

[survived]

This PR implies you're transmitting Incoming<Msg> and Outgoing<Msg> over a wire, I'm not sure it's a right thing to do. Incoming and outgoing structs contain information such as sender/recipient and whether it's p2p or broadcast message. However, this information should rather be implied from specific delivery implementation.

For instance, if delivery is implemented as p2p network, sender ID would be implied from IP address of sender (or rather, from which specific channel this message came from).

Also, while parties send Outgoing<Msg>, they expect to receive Incoming<Msg> which just simply have different set of fields, so you can't just serialize and send Outgoing and then receive and deserialize Incoming, it just won't work.

Would you elaborate on why you need serde traits on incoming and outgoing?

[tbraun96]

This PR implies you're transmitting Incoming<Msg> and Outgoing<Msg> over a wire, I'm not sure it's a right thing to do. Incoming and outgoing structs contain information such as sender/recipient and whether it's p2p or broadcast message. However, this information should rather be implied from specific delivery implementation.

For instance, if delivery is implemented as p2p network, sender ID would be implied from IP address of sender (or rather, from which specific channel this message came from).

Also, while parties send Outgoing<Msg>, they expect to receive Incoming<Msg> which just simply have different set of fields, so you can't just serialize and send Outgoing and then receive and deserialize Incoming, it just won't work.

Would you elaborate on why you need serde traits on incoming and outgoing?

The use of IP address won't work in our use case, because, we have generalized a Network trait that does not care about the underlying method of data transmission (e.g., we might use in-memory channels for tests). Thus, IP addresses are not guaranteed. Furthermore, using IP addresses in mapping in modern networks is a poor practice due to the nature of NATs and the possibility of nodes transiting between subnets intra-protocol.

To this effect, since we have generalized the underlying method of transmission and have forwarding to/from the network source and the channels in memory that receive/transmit these payloads, we do need to be able to serialize these incoming/outgoing structs otherwise our codebase will get ugly in a hurry and we'd be forced to maintain our own fork going forward.

As such, this PR does not necessarily imply that these structs will literally be transferred over the wire, but they may be depending on our chosen network implementation.

[shekohex]

This PR implies you're transmitting Incoming<Msg> and Outgoing<Msg> over a wire. I'm not sure it's the right thing to do.

It doesn't have to be sent over a networking wire; we have a wrapper over that already that handles the inner message $M$.

Would you elaborate on why you need serde traits on Incoming and Outgoing?

Currently, we have one channel for outgoing and incoming messages. We split this single channel into different channels with different kinds of message types. They all share the single channel under the hood (did I just explain multiplexing?). We use an enum to unify all these kinds of channels, which requires all messages to be serializable.

From your explanation, if you're saying we shouldn't send these messages as-is and instead send the inner message $M$, that's fine, but we'd still need $M$ to be serializable, right?

Thank you!

[survived]

The use of IP address won't work in our use case

IP addresses was an oversimplified example. Delivery implementation shouldn't rely on them. Instead, in case of p2p network, it should establish secure p2p channels by performing a cryptographic handshake. Then, identifying the channel from which message is received should give information about the sender.

To this effect, since we have generalized the underlying method of transmission and have forwarding to/from the network source and the channels in memory that receive/transmit these payloads

I'd say that generalized network framework should leave a functionality of identifying sender (and figuring out other metadata) up to the implementation. Having an option of sending Incoming<M> and Outgoing<M> over the wire means that, for instance, sender field has to be trusted. That's not a secure implementation of delivery, if malicious party can pretend being any other party by changing sender field of transmitted message.

As such, this PR does not necessarily imply that these structs will literally be transferred over the wire, but they may be depending on our chosen network implementation.

Yes, it doesn't force anyone to use serde, but having it in public API could suggest one that it's fine to send incomings/outgoings as is. If we're merging this PR, I need to identify a legit use-case for serialization, and update documentation to be clear when serialization should and should not be used.

[survived]

From your explanation, if you're saying we shouldn't send these messages as-is and instead send the inner message $M$, that's fine, but we'd still need $M$ to be serializable, right?

Yes, M certainly need to be serializable.

[survived]

If it helps, I can give you an example of how we implemented delivery (it's not opensourced tho). We have a relay server that relays messages between parties. Parties know each other (signing) public keys in advance. In order to do any communications, they first connect to delivery server and perform cryptographic handshakes and establish encryption keys. Then, when they send a p2p message, they send encrypted M, own public key, and a signature. When other party receives p2p message, it verifies the signature, then it looks up sender public key and determines which signer index is associated with the sender. Then it just decrypts M and reassembles Incoming<M> from the information it already knows.

Any other secure implementation of delivery I can think of (no matter if it uses p2p network/relay server/blockchain/..) should be using similar approach, it would likely replace signer index by signer identity (public key) and transmit the signature as well. Or other valid approach I can imagine would be implying sender ID from the secure channel which produced the message. Either approach doesn't need incoming/outgoing to be serializable.

[shekohex]

Thank you, @survived, for your explanation. That really helps.

[shekohex]

Closing for now as not needed.