[crypto] Add cross-testing of our BLS implementation with that of BLST

[huitseeker]

Please refer to individual commit messages.
Summary:

• cleans up a bit of dead code in crypto,
• introduces tests that use rapid to compare our implementation with that of BLST for various core BLS functions:
  • encoding / decoding scalars,
  • encoding / decoding points in G1 and G2,
  • signing
• in the process, creates a hybrid signature process for tests that relies on Relic for the map-to-curve, and finishes
  with our normal implementation. This is limited to the cipherSuite BLS_SIG_BLS12381G1_XMD:SHA-256_SSWU_RO_NUL_
  (extensible to other XMD:{SHA|BS) variants)

[codecov-commenter]

Codecov Report

Merging #934 (b110e51) into master (0e33b9e) will increase coverage by 0.03%.
The diff coverage is 54.54%.

@@            Coverage Diff             @@
##           master     #934      +/-   ##
==========================================
+ Coverage   53.14%   53.17%   +0.03%     
==========================================
  Files         321      321              
  Lines       21733    21730       -3     
==========================================
+ Hits        11550    11556       +6     
+ Misses       8618     8604      -14     
- Partials     1565     1570       +5     

Flag	Coverage Δ
unittests	53.17% <54.54%> (+0.03%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
engine/access/relay/engine.go	38.88% <ø> (-1.66%)	⬇️
engine/common/splitter/network/network.go	57.14% <42.85%> (+7.14%)	⬆️
cmd/bootstrap/cmd/key.go	71.01% <57.14%> (-0.87%)	⬇️
engine/common/splitter/engine.go	57.40% <100.00%> (-0.78%)	⬇️
cmd/util/ledger/migrations/storage_v4.go	41.56% <0.00%> (-0.61%)	⬇️
...sus/approvals/assignment_collector_statemachine.go	50.00% <0.00%> (+5.76%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5e6ef3d...b110e51. Read the comment docs.

[AlexHentschel]

Looks good. While the detailed cryptographic operations are beyond my background, I am sure Tarak and you have these details covered.

[tarakby]

Thank you for starting to play with the BLST library!
I also didn't know about the rapid package, we should be able to re-use it in many other tests!

I have made a suggestion branch that includes:

• updates to the deserialization functions to cover the malleability issue.
• minor comments updates.
• refactor TestMapToG1Relic.

Regarding the new test functions, I can group them into two categories:

1. test Relic map against the IRTF draft test vector

2. test consistency of flow-go/crypto and BLST

3. I think This is a useful test if we were to use relic hash to point mapping. This is unfortunately blocked by the PR to Relic. We can still use a similar test once I update our hash to point mapping to match the IRTF draft. Till these changes are made, I am wondering if the test should be merged to the repository, since it is testing a function that is not used in the package yet.

4. I have a similar feeling about the second type of tests. Since BLST isn't used in the package, we may need to wait before merging those tests. I can see the tests being very useful if we start the transition away from Relic and we want to make sure the updates are consistent with Relic. I think it's good to keep the tests on a feature branch and use them as a base to make more BLST work.

I'm curious to hear your thoughts about merging the tests!

[huitseeker]

I think This is a useful test if we were to use relic hash to point mapping. This is unfortunately blocked by the PR to Relic. We can still use a similar test once I update our hash to point mapping to match the IRTF draft. Till these changes are made, I am wondering if the test should be merged to the repository, since it is testing a function that is not used in the package yet.

The test checks that our signature function, minus the hash-to-curve component, is the same as the one in blst. I find that valuable, and there's an obvious transition (removing "minus the hash-to-curve component") that this test will easily undergo once we've solved the map-to-curve issue, one way or another (blst allows signing w/o the hash-to-field, so that we'll be able to splice in our kmac there).

I have a similar feeling about the second type of tests. Since BLST isn't used in the package, we may need to wait before merging those tests. I can see the tests being very useful if we start the transition away from Relic and we want to make sure the updates are consistent with Relic. I think it's good to keep the tests on a feature branch and use them as a base to make more BLST work.

A feature branch is meant to isolate wide-ranging changes to the code base that would impact the work of many other otherwise uninterested developers. While the work of transitioning our Relic-based library to anything else would be wide-ranging in terms of SLoC, it happens in one directory, in a half-dozen files, and -at least according to commit history- there isn't a very large number of impacted developers (and I would hope he would not be uninterested).

Moreover, even if we never transition to anything ever, we have to track both:

• our dependency, relic
• the fast-moving spec for BLS, well-tracked by blst.

These present tests help warn us about a change in either of them. They would similarly help if we were to choose integrating blst in the emulator or the SDK.

For future, transitional commits that would be moving the main arch of our relic library, we can certainly revisit the branch question.

[tarakby]

Thanks for the updates 🏄
I have made more comments on the updates, and made a suggestion PR to address them in order to merge soon.

All comments were addressed in my PR, but the one related to EP_ISOMAP.
As mentioned in my PR, I suggest to keep the current scope for this PR, and I'll submit a new PR to integrate the new Relic tool.