-
Notifications
You must be signed in to change notification settings - Fork 32
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
BLS12-381 Crypto Primitives #154
Conversation
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.
please add integration tests.
need to do the following:
add test contract to the tests/unit/test_contracts
that calls every bls function
add this test to the 'tests/integration/contracts.hpp.in'
add integration test to the 'tests/integration' that is using this contract to check that all actions work properly
} | ||
} | ||
|
||
int32_t bls_g1_add(const uint8_t* op1, const uint8_t* op2, uint8_t* res) |
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.
what about something like this? that way you can enforce structure size:
using some_type = uint8_t[144];
int32_t bls_g1_add(const some_type& op1, const some_type& op2, some_type& res)
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.
Added bls types to enforce correct operand sizes in: mschoenebeck@9d1424d
Edit: Updated https://github.com/mschoenebeck/aggsigtest/blob/main/aggsigtest.cpp as well.
Added integration tests for all bls primitives in: mschoenebeck@7a6dfcb |
The integration tests are not passing because the cdt/.github/workflows/build.yaml Lines 70 to 80 in 3f701e2
If we'd like to see the tests pass here (and continue to pass post merge), we need to switch the leap-dev.deb over to one from leap's BLS PR. Sadly since it's a fork, referencing it by branch won't work, but I'm pretty sure using
target: 90071617799738820350538f632ca5d5fd54e5e8 will work (this is the head of AntelopeIO/leap#1071 at the moment) Once AntelopeIO/leap#1071 is merged in to leap's main, the above will need to become target: main until 5.0 is cut, at which point it can then be changed to target: 5 |
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.
Overall this looks good to me. There are some small tweaks the ENF will do including getting the CI to fully pass. But we are good as it is to merge this into the bls_integraton
branch.
This is the corresponding
cdt
pull request to 1071 ofleap
. Enable the following types and host functions for smart contracts.Added the following types to
cdt
:Added the following host functions to
cdt
(according to https://eips.ethereum.org/EIPS/eip-2537):Check out mschoenebeck/aggsigtest for a sample contract using those new host functions.