feat: make encryption per partition instead of per message #107
Conversation
ChaoticTempest
force-pushed
the
phuong/feat/less-encryption
branch
2 times, most recently
from
fd38ec2 to
df8b908
Compare
| })?; | ||
| let SignedMessage::<Vec<u8>> { msg, sig, from } = serde_json::from_slice(&message)?; | ||
| let Self { msg, sig, from } = bincode::deserialize(&msg).unwrap(); |
There was a problem hiding this comment.
I would be careful with this change.
Unlike JSON, bincode is not self-describing. In other words, it does not encode field information. Instead, you must use the exact same struct for serialization and deserialization. If anything in the serialized type changes, for example you add a new field to a variant of MpcMessage, this may break deserialization. This manifests as either an explicit deserialization error or, if it cannot detect a mismatchmaybe, it just silently deserializes invalid data.
For non-distributed systems, this is usually not a problem.
But you have to consider that nodes will not update all at the same time. Hence, there will be times where different versions of a struct like MpcMessage are used in the system. You should be able to handle this gracefully.
Typically, one either uses a format that allows dynamic changes, like JSON or protobuf. Then you can add new fields or make old fields optional, without breaking anything. If you use something like bincode, however, it requires manual versioning, which can be a bit painful.
If you are looking for an easy JSON replacement but with binary encoding, take a look at CBOR, it is supported with serde: https://github.com/enarx/ciborium
Long term, it might make sense to think about using something like protobuf.
There was a problem hiding this comment.
I see, yeah that's a good point. I'll change it to CBOR in this PR
(It might also make sense to have the format change in a separate PR from "make encryption per partition instead of per message" .)
the reason why it's a part of this PR is because changing from Partition = Vec<Encrypted> to Partition = Encrypted breaks some things due to how json serialization works, like how serializing a bundle of messages would more than increase its size up to 10x what it was before. This was easy to check before when we encrypted per message and summed them up before partitioning them, but since partition now happens before encryption, things aren't as well
ChaoticTempest
force-pushed
the
phuong/feat/less-encryption
branch
from
df8b908 to
eb18367
Compare
ChaoticTempest
force-pushed
the
phuong/feat/less-encryption
branch
from
ea05873 to
e69ab91
Compare
|
Moved to ciborium for cbor encoding. Also added unit tests for serialization changes. So our |
| use crate::actions::sign::SignAction; | ||
| use crate::actions::wait::WaitAction; | ||
|
|
||
| pub fn spawn() -> ClusterSpawner { |
There was a problem hiding this comment.
Are these deletions relevant? Are they intentional?
There was a problem hiding this comment.
these were mistakenly committed. Look at the folder structure
send(Partition = Vec<Ciphered>)tosend(Partition = Ciphered(Vec<Message>))is the effective change.MessageInboxsuch that when nodes call send to other nodes, there's no wait for the response due to all the decryption that is happening.SignMessage. json serialization is very inefficient, especially for byte vectors, sobincodeis used to get away from all that json overhead.bincodeis for the most part just raw byte serialization.