Kirk baird patch 01 #79

kirk-baird · 2019-09-16T01:16:41Z

What was wrong?

Updating the BLS hash_to_G2 implementation to match the standard and ethereum/consensus-specs#1398

How was it fixed?

Implement hash_to_curve for G2 which uses Boneh & Wahby method.

There are three parts to this:

hash_to_field - convert a message to a point in the finite field.
map_to_curve - SWU Map to 3-Isogenous Curve + 3-Isogeny Map to BLs12-318 G2
clear_cofactor - ensure that the point is in the correct subfield.

Note: This is a Work In Progress PR to show progress and may not always be passing tests.

Cute Animal Picture

Signed-off-by: Kirk Baird <[email protected]>

pipermerriam

I know you have this marked as a WIP so think of these as just some idle thoughts as I perused the PR. Thanks for doing this work.

py_ecc/bls/hash.py

pipermerriam · 2019-09-16T02:08:05Z

py_ecc/bls/hash.py

+        okm = okm + previous
+
+    # Return first `length` bytes.
+    return okm[:length]


Since we discard the end of this can we exit early from the loop above once the condition okm >= length is satisfied?

Yep, though okm >= length after n iterators of the loop.

The loop could be changed to the condition while len(okm) < length
Then we would not have to calculate n. However since it is following RFC5869 and that is the way it's mentioned there I think it'd be better to leave it with n.

py_ecc/bls/utils.py

py_ecc/fields/optimized_field_elements.py

py_ecc/optimized_bls12_381/constants.py

py_ecc/optimized_bls12_381/optimized_swu.py

Signed-off-by: Kirk Baird <[email protected]>

kirk-baird · 2019-10-14T02:07:52Z

Update: Sorry I've been off this for awhile I've been working on a couple of other things + DevCon but I'll finish implementing it from now.

Signed-off-by: Kirk Baird <[email protected]>

kirk-baird · 2019-10-16T11:13:27Z

Although this branch should now be usable there are a few things left todo:

Increase test coverage of hash_to_curve
Optimise clear_cofactor if it is allowed as per patent
This should be constant time therefore solving Constant time hashing to the curve #73 but worth bench marking to check
Update links once bls standard drafts are finalised
PopVerify and PopProve functions

Also worth noting that compression/decompression will need to be updated (currently looks like they will be made to match zcash).

py_ecc/fields/optimized_field_elements.py

pipermerriam · 2019-10-16T15:01:58Z

py_ecc/optimized_bls12_381/constants.py

+ISO_3_B = FQ2([1012, 1012])
+ISO_3_Z = FQ2([1, 1])
+P_MINUS_9_DIV_16 = 1001205140483106588246484290269935788605945006208159541241399033561623546780709821462541004956387089373434649096260670658193992783731681621012512651314777238193313314641988297376025498093520728838658813979860931248214124593092835  # noqa: E501
+SQRT_I = 1028732146235106349975324479215795277384839936929757896155643118032610843298655225875571310552543014690878354869257  # noqa: E501


I have some discomfort from having all of these constants floating around. Is there a formal/canonical spec that we can include a link to so that it's easy to reference/cross-check these constants are indeed the values they are supposed to be?

There are soo many constants! Unfortunately they are necessary for the isogeny mapping.

Most of them come from
BLS General Constants: https://tools.ietf.org/html/draft-irtf-cfrg-hash-to-curve-04#section-8.7
ISO constants: https://tools.ietf.org/html/draft-irtf-cfrg-hash-to-curve-04#appendix-C.2

py_ecc/optimized_bls12_381/constants.py

Signed-off-by: Kirk Baird <[email protected]>

kirk-baird · 2019-11-04T00:02:36Z

Update:

Version 5 of the hash-to-curve spec is out.

I will update the code soon to match the new version. This is expected to be the last breaking change so hopefully won't be too long before we'll be able to merge.

hwwhww

Great work!

py_ecc/bls/api.py

py_ecc/bls/utils.py

py_ecc/bls/api.py

Signed-off-by: Kirk Baird <[email protected]>

kirk-baird · 2019-11-19T03:01:58Z

I happy for this to be merged now :)

Some final notes:

I have based this from against the hash-to-curve standard version 5.
I've made some of the functions constant time such as sqrt_division_FQ2 and optimized_swu_G2. We could make some pretty decent speed ups if these are not made to be constant time (constant time hashing is not necessary for Ethereum).
The tests are based off input to the reference implementation

hwwhww

Awesome work! 🎉

I only tried to compare the code with the IETF spec instead of checking the math, and some nitpick.

py_ecc/bls/hash.py

py_ecc/bls/utils.py

py_ecc/fields/optimized_field_elements.py

py_ecc/optimized_bls12_381/constants.py

py_ecc/optimized_bls12_381/optimized_swu.py

hwwhww · 2019-11-19T07:59:25Z

py_ecc/optimized_bls12_381/optimized_swu.py

+        temp2 = temp1 ** 2 * v - u
+        if temp2 == FQ2.zero() and not valid_root:
+            valid_root = True
+            result = temp1


It seems okay to break here.

Suggested change

result = temp1

result = temp1

break

This is where I deliberate left it as a constant time implementation.
We could get some speed ups here if we were to remove those checks.
Ethereum does not need constant time hash to curve cause the messages are public so I'm happy to add the break statements in there if you would like?

Not sure if this instinct is right but maybe lets leave this as constant time since that seems like a safe default.

We can track this in an issue as an option for performance improvements if needed.

Sure I'll leave it as is and write-up an issue for it now.

py_ecc/bls/utils.py

Signed-off-by: Kirk Baird <[email protected]>

ChihChengLiang

Great works @kirk-baird!
This is my first pass of review.

py_ecc/bls/hash.py

py_ecc/bls/constants.py

py_ecc/optimized_bls12_381/constants.py

py_ecc/optimized_bls12_381/optimized_swu.py

Co-Authored-By: Chih Cheng Liang <[email protected]>

Signed-off-by: Kirk Baird <[email protected]>

py_ecc/optimized_bls12_381/optimized_swu.py

ChihChengLiang

A question of Jacobian coordinates

py_ecc/optimized_bls12_381/optimized_swu.py

tests/test_bls_hash_to_G2.py

py_ecc/optimized_bls12_381/optimized_swu.py

Signed-off-by: Kirk Baird <[email protected]>

ChihChengLiang

My final review and some nitpick on reusing variables.

py_ecc/optimized_bls12_381/optimized_swu.py

Signed-off-by: Kirk Baird <[email protected]>

kirk-baird · 2019-11-26T07:11:15Z

My final review and some nitpick on reusing variables.

Looks good, I'll add all of these!

* repin flake8, bump tox to >=4.0.0 as that's where whitelist was deprecated, misc updates

kirk-baird added 3 commits September 9, 2019 16:15

Write hkdf extract and expand and add tests from RFC

4d69c2d

Signed-off-by: Kirk Baird <[email protected]>

Begin updating hash to curve to match bls standard

13bfbb8

Signed-off-by: Kirk Baird <[email protected]>

WIP commit implementing hash_to_curve_G2

7a360ac

Signed-off-by: Kirk Baird <[email protected]>

pipermerriam reviewed Sep 16, 2019

View reviewed changes

kirk-baird added 2 commits September 16, 2019 13:25

Minor updates and corrections

b18a3a8

Signed-off-by: Kirk Baird <[email protected]>

Change byte types for hkdf

bb021c5

Signed-off-by: Kirk Baird <[email protected]>

kirk-baird added 5 commits October 14, 2019 21:25

Minor Updates

5206225

Signed-off-by: Kirk Baird <[email protected]>

Add clear_cofactor

4e7068c

Signed-off-by: Kirk Baird <[email protected]>

fix multiplication by cofactor

dafa3e6

Signed-off-by: Kirk Baird <[email protected]>

flake8

51c56ea

Signed-off-by: Kirk Baird <[email protected]>

linting

3c2118f

Signed-off-by: Kirk Baird <[email protected]>

pipermerriam reviewed Oct 16, 2019

View reviewed changes

Increase test coverage for hash_to_cruve

678fb2e

Signed-off-by: Kirk Baird <[email protected]>

kirk-baird mentioned this pull request Oct 17, 2019

Verify Signatures are in the correct Sub-group #80

Closed

Add check that signature is in the correct subgroup

260f7df

Signed-off-by: Kirk Baird <[email protected]>

hwwhww reviewed Nov 5, 2019

View reviewed changes

py_ecc/bls/api.py Show resolved Hide resolved

py_ecc/bls/utils.py Show resolved Hide resolved

py_ecc/bls/api.py Outdated Show resolved Hide resolved

py_ecc/bls/api.py Outdated Show resolved Hide resolved

kirk-baird added 3 commits November 6, 2019 15:47

Minor touchups

e43c635

Signed-off-by: Kirk Baird <[email protected]>

Update to hash-to-curve version 5

c2c2197

Signed-off-by: Kirk Baird <[email protected]>

Fix typo

a361378

Signed-off-by: Kirk Baird <[email protected]>

kirk-baird changed the title ~~[WIP] Kirk baird patch 01~~ Kirk baird patch 01 Nov 19, 2019

hwwhww reviewed Nov 19, 2019

View reviewed changes

Minor fixes and updates base on review

8cf3eeb

Signed-off-by: Kirk Baird <[email protected]>

ChihChengLiang reviewed Nov 20, 2019

View reviewed changes

kirk-baird and others added 2 commits November 21, 2019 10:40

Update py_ecc/bls/constants.py

7665714

Co-Authored-By: Chih Cheng Liang <[email protected]>

Add additional comments for review

12a3247

Signed-off-by: Kirk Baird <[email protected]>

ChihChengLiang reviewed Nov 21, 2019

View reviewed changes

py_ecc/optimized_bls12_381/optimized_swu.py Outdated Show resolved Hide resolved

ChihChengLiang reviewed Nov 21, 2019

View reviewed changes

py_ecc/optimized_bls12_381/optimized_swu.py Show resolved Hide resolved

tests/test_bls_hash_to_G2.py Show resolved Hide resolved

ChihChengLiang reviewed Nov 21, 2019

View reviewed changes

kirk-baird added 2 commits November 25, 2019 10:07

Fiz minor bug and update for loop style

6103a28

Signed-off-by: Kirk Baird <[email protected]>

Ensure test output is on curve

4face13

Signed-off-by: Kirk Baird <[email protected]>

ChihChengLiang approved these changes Nov 26, 2019

View reviewed changes

Optimisations

6f37f1a

Signed-off-by: Kirk Baird <[email protected]>

hwwhww changed the base branch from master to bls_standard December 9, 2019 06:44

hwwhww merged commit 5b75be0 into ethereum:bls_standard Dec 9, 2019

pacrob added a commit to pacrob/py_ecc that referenced this pull request Oct 29, 2023

repin flake8 and misc updates (ethereum#79)

68d37fa

* repin flake8, bump tox to >=4.0.0 as that's where whitelist was deprecated, misc updates

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Kirk baird patch 01 #79

Kirk baird patch 01 #79

kirk-baird commented Sep 16, 2019

pipermerriam left a comment

pipermerriam Sep 16, 2019

kirk-baird Sep 16, 2019

kirk-baird commented Oct 14, 2019

kirk-baird commented Oct 16, 2019

pipermerriam Oct 16, 2019

kirk-baird Oct 16, 2019

kirk-baird commented Nov 4, 2019

hwwhww left a comment

kirk-baird commented Nov 19, 2019

hwwhww left a comment •

edited

Loading

hwwhww Nov 19, 2019

kirk-baird Nov 19, 2019

pipermerriam Nov 20, 2019

kirk-baird Nov 21, 2019

ChihChengLiang left a comment

ChihChengLiang left a comment

ChihChengLiang left a comment

kirk-baird commented Nov 26, 2019

Kirk baird patch 01 #79

Kirk baird patch 01 #79

Conversation

kirk-baird commented Sep 16, 2019

What was wrong?

How was it fixed?

Cute Animal Picture

pipermerriam left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kirk-baird commented Oct 14, 2019

kirk-baird commented Oct 16, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kirk-baird commented Nov 4, 2019

hwwhww left a comment

Choose a reason for hiding this comment

kirk-baird commented Nov 19, 2019

hwwhww left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ChihChengLiang left a comment

Choose a reason for hiding this comment

ChihChengLiang left a comment

Choose a reason for hiding this comment

ChihChengLiang left a comment

Choose a reason for hiding this comment

kirk-baird commented Nov 26, 2019

hwwhww left a comment •

edited

Loading