Skip to content
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

EC point serialization #141

Merged
merged 13 commits into from
Mar 14, 2024
Merged

EC point serialization #141

merged 13 commits into from
Mar 14, 2024

Conversation

davidnevadoc
Copy link
Contributor

@davidnevadoc davidnevadoc commented Feb 25, 2024

This PR follows up the serialization issue that was brought up in #109.

This PR applies the proposed solution for serialization of EC defined over prime fields with 0, 1 or 2 spare bits. (Bls12-381 has its own standard and has been left out).

Here is a summary with the main changes:

For the serialization of elliptic curve points defined over prime fields:

  • Keep using LE.
  • Keep using the hash-to-curve sgn0 definition.

Change in flags:

  • Sign in 7th bit for 0, 1, 2 spare bits.

  • Identity in 6th bit for 0, 2 bits.
    ⚠️ This changes the serialization of compressed Bn254 points. The flag bits are now flipped.

  • Uncompressed format does not use flags. The spare bits must be set to 0.

Flag bits, according to the number of spare bits.

1 Spare bit.

Compressed format

| ---- | ------------ |
| sign | x-coordinate |
| ---- | ------------ |
  1 bit
sign x-coordinate
Identity 0 0
Non-identity $P$ $sgn0(P)$ $P.x$

Uncompressed format

| ---- | ------------ | ---- | ------------ |
|   0  | x-coordinate |   0  | y-coordinate |
| ---- | ------------ | ---- | ------------ |
  1 bit                 1 bit
0 x-coordinate 0 y-coordinate
Identity 0 0 0 0
Non-identity $P$ 0 $P.x$ 0 $P.y$
  • Flag bits are unused
  • b must be != 0.

2 Spare bits.

Compressed format

| ---- | ------ | ------------ |
| sign | ident  | x-coordinate |
| ---- | ------ | ------------ |
 1 bit  1 bit  
sign ident x-coordinate
Identity 0 1 0
Non-identity $P$ $sgn0(P)$ 0 $P.x$

Uncompressed format
Spare bits are set to 0.

| ------ | ----  | ------------ | ------ | ---- | ------------ |
| 0      |  0    | y-coordinate | 0      | 0    | x-coordinate |
| ------ | ----  | ------------ | ------ | ---- | ------------ |
  1 bit    1 bit                  1 bit    1 bit
0 0 x-coordinate 0 0 y-coordinate
Identity 0 0 0 0 0 0
Non-identity $P$ 0 0 $P.x$ 0 0 $P.y$

0 Spare bits.

Add an extra byte in the compressed format to hold the flags. Then follow the 2 spare bit flag format.

Compressed format

| ---- | ----- | ------ | ------------ |
| sign | ident | 000000 | x-coordinate |
| ---- | ----- | ------ | ------------ |
 1 bit  1 bit       6 bit   

Same as 2 spare bits, with padding:

sign identity 000000 x-coordinate
Identity 0 1 000000 0
Non-identity $P$ $sgn0(P)$ 0 000000 $P.x$

Uncompressed format

| ------------ | ------------ |
| x-coordinate | y-coordinate |
| ------------ | ------------ |
x-coordinate y-coordinate
Identity 0 0
Non-identity $P$ $P.x$ $P.y$

@davidnevadoc davidnevadoc marked this pull request as ready for review February 28, 2024 08:46
@kilic kilic self-requested a review February 28, 2024 11:13
#[macro_export]
macro_rules! new_curve_impl {
(($($privacy:tt)*),
$name:ident,
$name_affine:ident,
$flags_extra_byte:expr,
$spare_bits:expr,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think we can drive $spare_bits from PrimeField::NUM_BITS?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea!
Done in 45e3366

src/derive/curve.rs Outdated Show resolved Hide resolved
src/derive/curve.rs Outdated Show resolved Hide resolved
src/derive/curve.rs Outdated Show resolved Hide resolved
src/derive/curve.rs Outdated Show resolved Hide resolved
src/derive/curve.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@duguorong009 duguorong009 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@davidnevadoc
I left some comments where there could be errors.
Pls check the comments.

Copy link
Contributor

@duguorong009 duguorong009 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe that the issue of mismatch still exists.
In case of 2 spare bits uncompressed format, the proposal says the MSB of P.x stores the identity flag.
Current implementation does it differently - reads/writes the identity flag in MSB of P.y. @davidnevadoc
Maybe, should we update the proposal doc?

src/derive/curve.rs Outdated Show resolved Hide resolved
src/derive/curve.rs Outdated Show resolved Hide resolved
@davidnevadoc davidnevadoc marked this pull request as draft February 29, 2024 09:28
src/derive/curve.rs Outdated Show resolved Hide resolved
src/derive/curve.rs Show resolved Hide resolved
src/derive/curve.rs Outdated Show resolved Hide resolved
src/derive/curve.rs Outdated Show resolved Hide resolved
src/derive/curve.rs Outdated Show resolved Hide resolved
src/derive/curve.rs Show resolved Hide resolved
@davidnevadoc davidnevadoc force-pushed the nev@ec-format branch 2 times, most recently from 4501c29 to 45e3366 Compare March 6, 2024 18:09
add: uncompressed identity test

fix: uncompressed serialization

fix: cleanup

fix: review comments

add: compute spare bits from NUM_BITS

fix: strict flag decoding

fix: imports

add: check for the bits in 0 spare bits case
@davidnevadoc davidnevadoc marked this pull request as ready for review March 7, 2024 16:52
@davidnevadoc davidnevadoc requested a review from ed255 March 7, 2024 17:02
src/derive/curve.rs Outdated Show resolved Hide resolved
src/derive/curve.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@duguorong009 duguorong009 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Seen that there are NO flag bits in uncompressed format.
Pls update the PR description & HackMD doc as is. @davidnevadoc

Copy link
Member

@ed255 ed255 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Great work!

@davidnevadoc davidnevadoc added this pull request to the merge queue Mar 14, 2024
Merged via the queue into main with commit 1662f88 Mar 14, 2024
12 checks passed
gilescope pushed a commit to gilescope/halo2curves that referenced this pull request May 14, 2024
* refactor!: Compressed format for EC points

 - The bit flags for sign and identity have been fliped for
 curves with 2 spare bits: Bn256, Pluto and Eris.

* fix: bn test vectors

* fix: update new_impl_curve calls

* refactor! Uncompressed format for EC points

* add: unchecked sered test

* fix: review comments

add: uncompressed identity test

fix: uncompressed serialization

fix: cleanup

fix: review comments

add: compute spare bits from NUM_BITS

fix: strict flag decoding

fix: imports

add: check for the bits in 0 spare bits case

* fix: fip FLAG_BITS

* fix: make to_bytes constant time

* add: Econding format doc

* fix: typos

* chore: update encoding spec with explicit byte/bit position and endianess

* chore: Remove flags from uncompressed format

Modified the docs accordingly and rearranged x,y order.

---------

Co-authored-by: Eduard S <eduardsanou@posteo.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants