-
Notifications
You must be signed in to change notification settings - Fork 57
Add QM31
implementation
#149
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
base: main
Are you sure you want to change the base?
Conversation
/// Each of this limbs can be represented by 36 bits. | ||
#[repr(transparent)] | ||
#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] | ||
pub struct QM31([u64; 4]); |
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.
why are we using u64s here ?
and why a limb would be 36 bits ?
Shouldn't we also introduce CM31 as an intermediate type ?
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.
why are we using u64s here
So that we can hold 36 bits.
and why a limb would be 36 bits ?
This is how it was implemented in Cairo VM. This is the commit which introduced it. The comments suggests QM31 is composted of 4 u64 limbs which represent 36 bit values, so 144 bits in total.
Shouldn't we also introduce CM31 as an intermediate type ?
This is implementation of a QM31. Instead of having two CM31, we have the four coordinates.
This PR migrates what was implemented in the Cairo VM. Is there anything you think should be changed?
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.
a felt in M31 is 31 bits i don't get why here it'd be 36, am i missing something ?
This PR migrates what was implemented in the Cairo VM.
I understand that but it feels weird to have the quartic EF and not the base field imo. Fine for now but i think the rest should be added in a follow up PR
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.
Found this comment from one of the commits that implemented this feature in Cairo VM. It explains why is was decided to use u64 as m31 representation. Seems it uses 36 bits for a m31 for an efficient Felt representation.
As you suggested, I think it's better to u32 instead of u64. I'll make this change. Sorry about the inconvenience.
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.
Change the representation to u32 coordinates 020c0dc.
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.
you changed the coordinates to be [u32; 4]
but 32*4 = 128.
We need 144 bits.
Which is 4,5 u32. So we need [u35; 5]
.
Or am I missing something?
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.
I mess up with the docs in cairo VM. This firstly was implemented in the cairo VM and since it only operates with felts, a qm31 was always being packed into a Felt. For efficiency reasons (check this lambdaclass/cairo-vm#1944 (comment)), related to the multiplication operation, it's better to represent every M31 with 36 bits instead of 31 (as it should). This mens that, only in the felt representation, a QM31 is 144 bits long.
This implementation treats every M31 as 31 bit value. So only when packing a QM31 into a felt a M31 be represented with 36 bits now. This is why we still check that, when unpacking from a Felt, the value is no longer than 144 bits.
/// [QM31] constant that's equal to 0. | ||
pub const ZERO: Self = Self([0, 0, 0, 0]); | ||
|
||
pub fn inner(&self) -> [u32; 4] { |
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.
pub fn inner(&self) -> [u32; 4] { | |
pub fn to_coordinates(&self) -> [u32; 4] { |
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.
Done in: edd3fa5
/// This method is convinient for performing multications and inversions, | ||
/// in which operating with the coordinates could result in overflows if | ||
/// the 32 bit representation was used. | ||
fn inner_u64(&self) -> [u64; 4] { |
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.
fn inner_u64(&self) -> [u64; 4] { | |
fn to_coordinates_u64(&self) -> [u64; 4] { |
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.
Done in: edd3fa5
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.
convinient -> convenient
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.
I didn't realize that renaming to to_coordinates_u64
would cause a clippy error. Maybe we can rename it to as_coordinates_u64
(or as_u64_coordinates
) since it's private
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.
Yes, I ended up doing that
@FrancoGiachetta We still need top level ( |
Hi @tdelabro! Added the requested top level docs. |
@@ -1,3 +1,20 @@ | |||
//! A Cairo-like QM31 type. | |||
//! | |||
//! The QM31 type represents a Degree-4 extension the Mersenne 31 Field. This extension can be |
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.
in the Mersenne 31 field
?
"in" missing + field don't have to be capitalized
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.
Fix here: 7f3d6ef
//! The QM31 type represents a Degree-4 extension the Mersenne 31 Field. This extension can be | ||
//! represented as two components of the Complex extension the Mersenne 31 Field as follows ((a, b), (c, d)), where | ||
//! a, b, c and d represent a value from the Mersenne 31 Field, denotated as M31. | ||
//! If anly a M31 was used by the verifier, then a 31 bit value wouldn't be enough to provide security |
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.
"If any" typo
"a M31" is it "a" like the value you named "a" or a regular english word?
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.
It refers to the value a named. This is to avoid repeating "a value from the Mersenne 31 field". The first typo is already fixed in this commit: b65e34b
//! A Cairo-like QM31 type. | ||
//! | ||
//! The QM31 type represents a Degree-4 extension in the Mersenne 31 field. This extension can be | ||
//! represented as two components of the Complex extension the Mersenne 31 field as follows ((a, b), (c, d)), where |
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.
of the Complex extension the Mersenne 31 field
It doesn't seem very clear, maybe a word is missing here?
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.
Yes, I forgot to add an "in". Fix here: 6bad58f
//! The QM31 type represents a Degree-4 extension in the Mersenne 31 field. This extension can be | ||
//! represented as two components of the Complex extension the Mersenne 31 field as follows ((a, b), (c, d)), where | ||
//! a, b, c and d represent a value from the Mersenne 31 field, denotated as M31. | ||
//! If only a M31 was used by the verifier, then a 31 bit value wouldn't be enough to provide security |
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.
//! If only a M31 was used by the verifier, then a 31 bit value wouldn't be enough to provide security | |
//! If only an M31 was used by the verifier, then a 31 bit value wouldn't be enough to provide security |
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.
Done here: 6bad58f
//! If only a M31 was used by the verifier, then a 31 bit value wouldn't be enough to provide security | ||
//! to the verification. A QM31 not only provides an efficient arithmetic field, since it is composed of four M31 values, but | ||
//! also allows for a more secure level of verification as it offers a 124 bit value. By using this extension, | ||
//! the verifier is able to generate challeges with a proper level of randomness, ensuring the security of the protocol. |
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.
the verifier is able
What verifier? The Cairo one?
//! the verifier is able to generate challeges with a proper level of randomness, ensuring the security of the protocol. | |
//! the verifier is able to generate challenges with a proper level of randomness, ensuring the security of the protocol. |
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.
A verifier in general. A 31 bit value is not enough to provided security since the range of random values is too short and vulnerable to brute-force attacks
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.
fixed typo here: 6bad58f. Also changed the verifier
for a verifier
as the comment aims to a verifier in general.
//! While the Cairo language's representation of QM31 consists of four `BoundedInt`s | ||
//! simulating a 31 bit value, this implementation uses four 32 bit values. |
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.
I would add the corresponding link to the cairo repo, at the end of this paragraph.
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.
Done here: 6bad58f
//! The conversion to a Felt is done by using the four elements of the struct, refered to as coordinates, as bytes and then | ||
//! parsed as a little endian value. For a more efficient Felt representation, each coordinate is stored as a 36 bit. Hence, |
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.
The conversion to a Felt is done by using the four elements of the struct, refered to as coordinates, as bytes
It doesn't seem very clear. I would briefly explain what a coordinate is and then describe the Felt conversion. That way you don't have to define coordinates in the middle of the description of the conversion.
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.
Done here: 035a87c
Summary
The purpose of this PR is to move QM31's implementation from Cairo VM to types-rs. It adds a wrapper over
FieldElement<Stark252PrimeField>
which allows a safer usage of QM31 operations.The implemented set of operations only corresponds to the ones in Cairo VM.
Pull Request type
Please add the labels corresponding to the type of changes your PR introduces:
What is the current behavior?
QM31 is not implemented in types-rs
Resolves: #NA
What is the new behavior?
Add
QM31Felt
abstraction for performing QM31 operation in a safe manner.Does this introduce a breaking change?
No
Other information