-
Notifications
You must be signed in to change notification settings - Fork 119
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(all): add versioning of serializable types on tfhe-rs 0.6 #1151
base: integration/versioning
Are you sure you want to change the base?
Conversation
I don't think this is a breaking change for serialization/deserialization, since this PR only adds an optional set of methods on every types. i.e. messages serialized directly will still be deserializable after this PR. To use the versioning you need to use the |
baba1c2
to
a31dd7b
Compare
a31dd7b
to
25cf708
Compare
25cf708
to
db95bef
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor comments that have nothing to do with the code to start
51694f7
to
be733d5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some additional non code comments, then we'll get to the meat of the proc macro review (though I'm far from a pro on that)
I like how little code it ends up requiring in TFHE-rs (though I know there is no upgrade implementations for now but it is looking very promising)
#[derive(serde::Serialize, serde::Deserialize, Versionize)] | ||
#[versionize(SerializableLweCiphertextModulusVersions)] | ||
/// Actual serialized modulus to be able to carry the UnsignedInteger bitwidth information | ||
pub struct SerializableLweCiphertextModulus { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this can be rename to just "SerializaleCiphertextModulus", was likely a leftover from something older
[package] | ||
name = "tfhe-versionable" | ||
version = "0.1.0" | ||
edition = "2021" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
make sure there are enough keys here to be able to publish the crate, I have had the issue with the ZK crate, so the set of metadata in the zk crate should be a good indication of minimal metadata
[package] | ||
name = "tfhe-versionable-derive" | ||
version = "0.1.0" | ||
edition = "2021" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same thing for publication
be733d5
to
3444fa1
Compare
Still in the process of reviewing, I have a doubt on the fact that the derive crate is in the tfhe-versionable crate dir, I wonder if when doing cargo package/publish the code from the derive crate is going to be included in the tar ball that cargo uploads (and is not going to use it to build tfhe-versionable as it goind to download the proper the-versionable-derive crate) |
Maybe I can just move it up a level into |
I tried with It may still be worth to move the crate up a level as its a crate not a module |
3444fa1
to
b317723
Compare
b317723
to
c4c6b85
Compare
will need a rebase the action fix has been merged in release/0.6.x as it was needed |
} | ||
|
||
/// This derives the `Versionize` and `Unversionize` trait for the target type. This macro | ||
/// has a mandatory parameter, which |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This sentence seems unfinished ?
if let Some(target) = &self.from { | ||
quote! { #target::unversionize(#arg_name).into() } | ||
} else if let Some(target) = &self.try_from { | ||
quote! { #target::unversionize(#arg_name).try_into().unwrap() } // TODO: handle errors |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this be handled now ?
self.orig_type.variants.len() | ||
} | ||
|
||
/// Returns the latest version of the original type, which is the last variant in the enum |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does the macro test that variants are correctly ordered ? to catch errors like enum ThingVersion { V0(..), V2(...), V1(..)}
?
closes: zama-ai/tfhe-rs-internal#538
PR content/description
This PR adds data versioning to serialized types for backward compatibility between tfhe-rs versions. This is done using a new crate,
tfhe-versionable
, that adds a set of derive macros. These macro derive a pair of traits (Versionize
/Unversionize
) that add conversion functions between a type and its "versioned" representation. The versioned representation of a type is an enum where each variant is a version of the type.Before serialization, the type is wrapped into the latest variant of the enum in the
versionize
method of theVersionize
trait. To be able to use it after deserialization, the enum is converted into the target type with theunversionize
method of theUnversionize
trait. To make this work, we have to define for each older version of a type anupgrade
method that is able to transform versionVn
intoVn+1
. The generatedunversionize
method will chain calls ofupgrade
enough times to get to the latest version.For a given type that has to be versioned, there are 3 macro that should be used:
Versionize
: used on the main type, that is used elsewhere in the code. Will derive theVersionize
/Unversionize
traitsVersion
: used on a previous version of the type.Versionize
also automatically deriveVersion
for the latest version.VersionsDispatch
: used on the enum with all the versions. Each variant should deriveVersion
, except the last one that derivesVersionize
a fourth proc macro
NotVersioned
can be used on a type that should not be versioned. TheVersionize
/Unversionize
traits will be implemented usingSelf
as versioned representation of the type. This is used for built-in types.Here is an example of the workflow:
The proc macro are used to handle the versioning recursivity. If we see a type definition as a tree where each type is a node and its children are the types of its attributes, the version of a given type is made to be independent of the version of its children. That way, if we update a type we don't have to manually update the version of all the type that recursively use it.
The macros handle:
Internals
Internally, the
Version
proc macro will generate for each version of the type a pair of associated types. Each associated types will have the same shape as the type that the macro is derived on except that their fields will be replaced by their versioned representation. The difference between the two types is that one is defined using references and the other using owned data. This allows to try to avoid copies as much as possible.For example for this type:
the macro will generate these types:
MyStructVersion
will be used for versioning if possible, andMyStructVersionOwned
for unversioning and for versioning if it is not possible to use a reference. The macro also generates conversion methods between a type and itsVersion
associated types. It also implements aVersion
trait that allows easier access to these generated types in other macro.Similarly, the
VersionsDispatch
macro will generate for the dispatch enum two associated enums, one with references and one with owned data. These enums will be used as the versioned representation for the type. They are the result and parameters of theversionize
andunversionize
methods and can be serialized/deserialized:Finally, the
Versionize
macro will use the generated enums.versionize
is just a conversion betweenMyStruct
and the latest variant ofMyStructVersionsDispatch
andunversionize
is a conversion betweenMyStructVersionDispatchOwned
andMyStruct
(slightly more complicated because of the chained calls toupgrade
)TODO
Versionize is currently implemented for the shortint ciphertext and all its subtypes.
Check-list: