Bls12_381 Basic Fp functionality #3
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Added datatype BigUint: vector of u32 limbs - Implements addition for BigUint - Tests for addition in main.sw (this is a script)
- Added subtraction for biguint - With tests - Introduced trait Zero, so we can check for "zero-being" - Added tests for addition and structured them
… subtraction is covered. - i >= 0 instead of i > 0 - test added for 2 elements of len 1 - added code for edge case 0 - 0 = 0 - test added for 0 - 0 = 0
- Added the schoolbook mult from the swayPractice repo that works for 2 tests (not extensively tested though) - Added data that can be used for a test case, with the values that would be used in the first iteration of Karatsuba
- A function to try out how Karatsuba would work splitting 1 time. Assumes a perfect scenario where x and y have the same length which is a multiple of 2. - Working on going through the code step by step to see if it is doing the expected thing, according to the test data above the testcase that is called "biguint_schoolbook_mult"
…ing any checks and some unsafe unwrapping.
- Length of x should be a multiple of 2 - At 2 places an unwrap was being done, it is now asserted first that this should be possible
…into modularArithmetic This takes into account the changes made to 3-karatsub-mult.
- a very naive implementation with 2 tests. Where modular reduction is done, the result is wrong by 1 - can't figure out why yet. - added a comment about the "zero" definition for BigUint which we should get straight Note that I added an equals function in the big_uint and named the mod function biguint_mod2, this is mainly because the compiler wouldn't stop giving issues. I think 'forc clean' doesn't actually clean everything :(
…19 and elements represented radix 51. - Radix-51 representation - Zero element - Carry_propagate function - Corresponding tests and test_helpers functions
The a+a test reveals that reduction is probably not correct - should be cheched.
…ome situations) - Added extra test for mod that represents this situation - Both addition tests are now working
- Added scalar mult function - With corresponding tests
…on implementation from the go implementation - Made the implementation constant (following the ref impl) - Replaced the * 19 by the function times19 - Replaced the mod_25519 call in the addition function with the propagate_carry call - Fixed the addition test that was using a test element too big
…ltiply_64 - Replaced *19 with times19 from the NaCl impl - Added tests and organized tests into which ones work if all ran at the same time and the mult test that...: - Mult tests only work when ran 1 at a time -> open issue
- Some functions added or moved to util.sw - Importing choice into the testing contract gives an Internal compiler error
…nal compiler error:)
…oved soon. - Updated authors in tests-bls
- Converted tests_fp and tests_fp2 to use a local node with custom gas limit - What functions & tests work - Fp2 mult gives result 0 atm
- Script is updated for debugging fp2 mult
- Added test for double in g1 to main.sw (but doesn't seem to terminate) - Added sum_of_products_6. (Can't be tested yet) - Removed unnecessary comments
- Updated uses of eq to ct_eq for Fp - Tests in script expanded. This is still meant as a temporary way to debug stuff.
- ct_eq, is_zero for Fp2 and Fp6 - Adjustments for other functions that can now use Choice instead of bool
- Added bitwise_or and conditional_select for u8 - Added ct_eq for G1Affine
- g1: added several functions, but compilation is really slow - trait from in choice.sw can use capital letter because it doesn't shadow a trait from U128 anymore - Removed outdated comments - script in main.sw is updated with some new tests. Again, this is temporary and can be stripped down, but it is handy for debugging and testing some sub functions
- Added lexicographically_largest in fp and fp2 - Added functions in scalar and g1 - Moved subtract_wrap - main.sw script is continuously updated for debugging and testing - meant as a scratchpad
…pto impl). This runs into the error: Err` value: ValidationError(TransactionGasLimit - Added mul_by_nonresidue for fp6 which will be tested indirectly.
- Adjusted some of the contract testing to new version - Added a few comments according to the performance of this new version
There's some assembly in here because Sway doesn't support underflow natively. This is usually leveraged to avoid writing if else.
… test for lexicographically_largest_fp will still give an error.
Mikerah
requested changes
Sep 23, 2022
Contributor
Mikerah
left a comment
Mikerah
left a comment
There was a problem hiding this comment.
Some feedback:
- Perhaps the test folder structure should change. For example, maybe we should have an overarching test folder in which tests_bls12_381 can be inside of
- Some of the commits are for the ed25519 implementation. Cherry pick these commits and put them in a separate branch
- Since this PR has many commits, it makes sense to git squash logical batches of commits together in order to have a cleaner git history and makes it easier to identify problems later down the road
- Add more comments to each function across all the files. As Sway is a new language and crypto code already is quite terse, I think it's best to have a lot of documentation to aid newcomers and to remind ourselves why we did a particular thing
- It would be good to know if assembly versions of the computational heavy operations were more efficient than pure Sway.
|
|
||
| To run tests for edwards25519 folder: | ||
| ``` | ||
| cd test_contract |
|
|
||
| To run tests for bls folder: | ||
| ``` | ||
| cd tests-bls |
| @@ -0,0 +1,204 @@ | |||
| library choice; | |||
Contributor
There was a problem hiding this comment.
Since most of this code is from the Dalek-Cryptography subtle library, we should reproduce the license at the beginning of the file since they released the code under the 3-clause BSD license.
| @@ -0,0 +1,204 @@ | |||
| library choice; | |||
|
|
|||
Contributor
There was a problem hiding this comment.
I would maybe move this file to a utils folder since having a constant-time boolean implementation will be important for other cryptography primitives that we may implement in sway.
Contributor
Author
|
Working on this in branch |
Contributor
Author
|
PR created in new branch that works on the feedback given here. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR contains working add, sub, mul, square for Fp.
For this, we worked on fp.sw, choice.sw, util.sw.
There is code in the other files fp2, fp6 and g1, but very little can run at the same time and a lot cannot run at all. We're waiting for this GitHub issue to be resolved.
To run tests on the function in Fp:
This will currently run the tests in
tests_bls12_381/tests/tests_fp/mod.rs(since thefpmodule is uncommented intests_bls12_381/tests/harness.rs.