Skip to content

Conversation

@gogoex
Copy link
Collaborator

@gogoex gogoex commented Aug 21, 2022

For issue #42

  • Reviewed the original implementation of Scalar and G1Point and made necessary changes
  • Added tests for each exposed methods of Scalar and G1Point
  • Added Elements class that represents a vector of Scalar and G1Point classes
  • Added operator overloads to Scalar, G1Point and Elements classes to improve readability of code
  • Confirmed that Scalar, G1Point and Elements provide enough functionalities to implement a range proof by adding a test that implemented a range proof

@aguycalled
Copy link
Collaborator

aguycalled commented Aug 22, 2022

One of the tests fails for me:

% ./src/test/test_navcoin 
Running 587 test cases...
test/blsct/arith/integration_tests.cpp:30: error: in "test_integration_invert_zero": check x.Pow(zero) == x.Pow(zero.Invert()) has failed

*** 1 failure is detected in the test module "Navcoin Core Test Suite"

Added some log:

    std::cout << strprintf("%s\n", HexStr(zero.GetVch()));
    std::cout << strprintf("%s\n", HexStr(zero.Invert().GetVch()));

Output:

0000000000000000000000000000000000000000000000000000000000000000
72af98d20d0c60cebe24f97a434290039b9673f3e0b1bb7a981cf9380e9f5524

@mxaddict mxaddict mentioned this pull request Aug 22, 2022
@gogoex
Copy link
Collaborator Author

gogoex commented Aug 23, 2022

@aguycalled The test was originally created to test FirstNPow for a case where y is 0:

    // (64)
    auto hhp = hh * Scalars::FirstNPow(n, y.Invert());

but now i think the test is not necessary since y is in Z^*_p and non-zero.

Also since multiplying any element by its invese results in unit element 1 and multiplying any element by 0 results in 0, I think the inverse of 0 in Z_p is undefined. It probably just happes to return 0 on x86_64 implementation.

So I removed the test and added another test case for Scalars::FirstNPow to test if it always return alist that starts with 1 regardless of the value of base value instead.

Can you try again?

@aguycalled
Copy link
Collaborator

@aguycalled The test was originally created to test FirstNPow for a case where y is 0:

    // (64)
    auto hhp = hh * Scalars::FirstNPow(n, y.Invert());

but now i think the test is not necessary since y is in Z^*_p and non-zero.

Also since multiplying any element by its invese results in unit element 1 and multiplying any element by 0 results in 0, I think the inverse of 0 in Z_p is undefined. It probably just happes to return 0 on x86_64 implementation.

So I removed the test and added another test case for Scalars::FirstNPow to test if it always return alist that starts with 1 regardless of the value of base value instead.

Can you try again?

All the tests pass for me now 👍

% ./src/test/test_navcoin                       
Running 586 test cases...

*** No errors detected

Maybe, Scalar::Invert() should throw an exception if the value is zero.

@mxaddict
Copy link
Collaborator

I think this would be a good idea, since we are not supposed to use a zero value in Scalar right?

@gogoex
Copy link
Collaborator Author

gogoex commented Aug 23, 2022

sure. now calling Invert on Scalar(0) throws an exception.

@aguycalled aguycalled closed this Aug 25, 2022
@aguycalled aguycalled reopened this Aug 25, 2022
Copy link
Collaborator

@aguycalled aguycalled left a comment

Choose a reason for hiding this comment

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

Approving and merging. Pending solving the CI issues in a different task #61.

@aguycalled aguycalled merged commit 575880a into nav-io:master Aug 25, 2022
mxaddict added a commit to mxaddict/navio-core that referenced this pull request Aug 29, 2022
* Added the new classes Scalar and G1Point

* Added scalar.{h,cpp} and g1point.{h,cpp} to make file

* Implemented more of the operators

* Implemented more functions

* Added new << >> logic for bitwise ops

* implemented Scalar::{Rand,GetString,Hash}

* Updated the arith scalar logic

* Added more G1Point arith logic, math...

* Added hashMapTo

* Removed Scalar::hashAndMap

* Updates that I forgot to push

* Updated G1Point::Rand()

* Update the mulVec func in G1Point to throw and exception if gVec and sVec size don't match

* Removed the test code that was accidentally added

* Implemented mulVec

* Removed some unused header calls

* Added return statement for G1Point::Double

* add g1point/scalar and their tests

* ignore .vscode

* add endianness parameter to G1Point::MapToG1. add g1point_hash_and_map test

* add mulvec_mcl test

* add mulvec tests

* add scalars and g1points classes

* add MapToG1(string&)

* extracted common parts of Scalars and G1Points to Elements. add slicing functions to Elements

* wip

* wip

* drop G1point::operator^

* drop the 2nd type artument from Elements

* add skeleton elements tests

* add elements tests

* support non-zero rand generatation

* wip

* add Elements::FirstNPowers

* add .gitignore to exclude dynamically generated files on macos. add Elements::RandomVector

* wip

* add Scalar::Pow and Scalar::GetBits

* add range proof test (not tested)

* add type parameter where required

* calculate t1, t2 properly

* fix typo

* add Elements::RepeatN test

* add comment

* add integration_tests and move such tests there

* introduce std::is_same in elements.h

* add gg^z == gg^(ones * z)

* wip

* add working h^mu gg^l test

* add Scalars::FirstNInvPow

* add multiply inv power seq for G1Points

* wip

* wip

* drop FirstNInvPow. add working 65_g_part_ts_only test

* wip

* add working range proof test

* wip

* make the range proof procedure a function

* integrate inner product argument to range proof

* wip

* wip

* wip

* wip

* drop unused variable

* separate elements implementation to cpp

* clean up code

* wip

* wip

* ignore .vscode

add endianness parameter to G1Point::MapToG1. add g1point_hash_and_map test

add mulvec_mcl test

add mulvec tests

add scalars and g1points classes

add MapToG1(string&)

extracted common parts of Scalars and G1Points to Elements. add slicing functions to Elements

wip

wip

drop G1point::operator^

drop the 2nd type artument from Elements

add skeleton elements tests

add elements tests

support non-zero rand generatation

wip

add Elements::FirstNPowers

add .gitignore to exclude dynamically generated files on macos. add Elements::RandomVector

wip

add Scalar::Pow and Scalar::GetBits

add range proof test (not tested)

add type parameter where required

calculate t1, t2 properly

fix typo

add Elements::RepeatN test

add comment

add integration_tests and move such tests there

introduce std::is_same in elements.h

add gg^z == gg^(ones * z)

wip

add working h^mu gg^l test

add Scalars::FirstNInvPow

add multiply inv power seq for G1Points

wip

wip

drop FirstNInvPow. add working 65_g_part_ts_only test

wip

add working range proof test

wip

make the range proof procedure a function

integrate inner product argument to range proof

wip

drop unused variable

wip

wip

wip

separate elements implementation to cpp

clean up code

wip

wip

* squashed commits

* fix typo

* migrate indirect MulVec to Elements

* fix build issues in clang

* fix spelling errors

* add newly introduced boost depdendency to lint-includes.py

* remove local depdendent functions

* wip

* remove trailing spaces

* Added BLS and MCL libs/includes to kernel build

* Updated copy constructor and assignment operators

* Added clean steps to clean-local for mcl and bls libs

* FORCE mcl and bls libs to build using main project CC and CXX values

* replace test_integration_invert_zero by additional test case of Scalars::FirstNPow

* use fixture test suite to provide name for bls/arith tests

* throw exception when Scalar(0) is inverted

* include bls/arith headers in setup_common.h

* add g1point and mcl_initializer dependency to test_util source

* fix test_util.include

* add blsct/arith/scalar depedency to libtest_util

* Disabled LLVM use in MCL compilation, this disabled ASM

* Added NM to android host configs for depends builds

* Added ABI=32 for i386 builds of GMP in depends

* fix undefined sanitizer error

Co-authored-by: mxaddict <mxaddict@codedmaster.com>
aguycalled pushed a commit that referenced this pull request Aug 29, 2022
* Added the new classes Scalar and G1Point

* Added scalar.{h,cpp} and g1point.{h,cpp} to make file

* Implemented more of the operators

* Implemented more functions

* Added new << >> logic for bitwise ops

* implemented Scalar::{Rand,GetString,Hash}

* Updated the arith scalar logic

* Added more G1Point arith logic, math...

* Added hashMapTo

* Removed Scalar::hashAndMap

* Updates that I forgot to push

* Updated G1Point::Rand()

* Update the mulVec func in G1Point to throw and exception if gVec and sVec size don't match

* Removed the test code that was accidentally added

* Implemented mulVec

* Removed some unused header calls

* Added return statement for G1Point::Double

* add g1point/scalar and their tests

* ignore .vscode

* add endianness parameter to G1Point::MapToG1. add g1point_hash_and_map test

* add mulvec_mcl test

* add mulvec tests

* add scalars and g1points classes

* add MapToG1(string&)

* extracted common parts of Scalars and G1Points to Elements. add slicing functions to Elements

* wip

* wip

* drop G1point::operator^

* drop the 2nd type artument from Elements

* add skeleton elements tests

* add elements tests

* support non-zero rand generatation

* wip

* add Elements::FirstNPowers

* add .gitignore to exclude dynamically generated files on macos. add Elements::RandomVector

* wip

* add Scalar::Pow and Scalar::GetBits

* add range proof test (not tested)

* add type parameter where required

* calculate t1, t2 properly

* fix typo

* add Elements::RepeatN test

* add comment

* add integration_tests and move such tests there

* introduce std::is_same in elements.h

* add gg^z == gg^(ones * z)

* wip

* add working h^mu gg^l test

* add Scalars::FirstNInvPow

* add multiply inv power seq for G1Points

* wip

* wip

* drop FirstNInvPow. add working 65_g_part_ts_only test

* wip

* add working range proof test

* wip

* make the range proof procedure a function

* integrate inner product argument to range proof

* wip

* wip

* wip

* wip

* drop unused variable

* separate elements implementation to cpp

* clean up code

* wip

* wip

* ignore .vscode

add endianness parameter to G1Point::MapToG1. add g1point_hash_and_map test

add mulvec_mcl test

add mulvec tests

add scalars and g1points classes

add MapToG1(string&)

extracted common parts of Scalars and G1Points to Elements. add slicing functions to Elements

wip

wip

drop G1point::operator^

drop the 2nd type artument from Elements

add skeleton elements tests

add elements tests

support non-zero rand generatation

wip

add Elements::FirstNPowers

add .gitignore to exclude dynamically generated files on macos. add Elements::RandomVector

wip

add Scalar::Pow and Scalar::GetBits

add range proof test (not tested)

add type parameter where required

calculate t1, t2 properly

fix typo

add Elements::RepeatN test

add comment

add integration_tests and move such tests there

introduce std::is_same in elements.h

add gg^z == gg^(ones * z)

wip

add working h^mu gg^l test

add Scalars::FirstNInvPow

add multiply inv power seq for G1Points

wip

wip

drop FirstNInvPow. add working 65_g_part_ts_only test

wip

add working range proof test

wip

make the range proof procedure a function

integrate inner product argument to range proof

wip

drop unused variable

wip

wip

wip

separate elements implementation to cpp

clean up code

wip

wip

* squashed commits

* fix typo

* migrate indirect MulVec to Elements

* fix build issues in clang

* fix spelling errors

* add newly introduced boost depdendency to lint-includes.py

* remove local depdendent functions

* wip

* remove trailing spaces

* Added BLS and MCL libs/includes to kernel build

* Updated copy constructor and assignment operators

* Added clean steps to clean-local for mcl and bls libs

* FORCE mcl and bls libs to build using main project CC and CXX values

* replace test_integration_invert_zero by additional test case of Scalars::FirstNPow

* use fixture test suite to provide name for bls/arith tests

* throw exception when Scalar(0) is inverted

* include bls/arith headers in setup_common.h

* add g1point and mcl_initializer dependency to test_util source

* fix test_util.include

* add blsct/arith/scalar depedency to libtest_util

* Disabled LLVM use in MCL compilation, this disabled ASM

* Added NM to android host configs for depends builds

* Added ABI=32 for i386 builds of GMP in depends

* fix undefined sanitizer error

Co-authored-by: mxaddict <mxaddict@codedmaster.com>
aguycalled pushed a commit that referenced this pull request Aug 30, 2022
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.

3 participants