Skip to content
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

feat(eddsa): Added EDDSA crypto module #112

Merged
merged 15 commits into from
Mar 10, 2024
Merged

feat(eddsa): Added EDDSA crypto module #112

merged 15 commits into from
Mar 10, 2024

Conversation

hanish520
Copy link
Contributor

Added EDDSA(25519 curve) crypto module to the framework.
The current design for eddsa module is messy, this PR is a starting point to take this activity.
Need to discuss with others.

Added EDDSA(25519 curve) crypto module to the framework.
Current design for eddsa module is messy, this PR is a
starting point to take this activity.
Need to discuss with others.
@hanish520 hanish520 requested a review from meling February 25, 2024 00:25
hanish520 and others added 6 commits February 25, 2024 01:30
This is an attempt at making the MultiSignature type be generic
so that impl can be switched at the top level instead of nested.
While converting the quorum signature from proto, initialization
of the multi signature is causing type conversion failures in the
crypto modules.
Copy link

codecov bot commented Mar 9, 2024

Codecov Report

Attention: Patch coverage is 76.44231% with 49 lines in your changes are missing coverage. Please review.

Project coverage is 62.52%. Comparing base (d23946a) to head (8928cbc).

Files Patch % Lines
crypto/eddsa/eddsa.go 73.68% 18 Missing and 7 partials ⚠️
crypto/keygen/keygen.go 52.77% 13 Missing and 4 partials ⚠️
crypto/multisignature.go 89.18% 4 Missing ⚠️
internal/testutil/testutil.go 57.14% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #112      +/-   ##
==========================================
+ Coverage   61.67%   62.52%   +0.85%     
==========================================
  Files          77       79       +2     
  Lines        7433     7576     +143     
==========================================
+ Hits         4584     4737     +153     
+ Misses       2548     2537      -11     
- Partials      301      302       +1     
Flag Coverage Δ
unittests 62.52% <76.44%> (+0.85%) ⬆️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@meling meling left a comment

Choose a reason for hiding this comment

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

A few small adjustments should do it.

internal/orchestration/orchestration_test.go Outdated Show resolved Hide resolved
internal/proto/hotstuffpb/convert.go Outdated Show resolved Hide resolved
crypto/eddsa/eddsa.go Show resolved Hide resolved
internal/orchestration/orchestration_test.go Outdated Show resolved Hide resolved
crypto/multi_signature.go Outdated Show resolved Hide resolved
crypto/multi_signature.go Outdated Show resolved Hide resolved
crypto/multi_signature.go Outdated Show resolved Hide resolved
crypto/keygen/keygen.go Show resolved Hide resolved
hanish520 and others added 8 commits March 10, 2024 01:13
Addressed review comments except for the comment in keygen.go
file, as it would add a switch case in the orchestration layer.
Tried to add more cases, this mock layer is not very flexible
to add more negative cases.
We now use ed25519.PublicKey directly instead of []byte.
The ed25519.PublicKey is a type alias for []byte, but it is best
to use the correct type.
@meling meling merged commit 301532c into master Mar 10, 2024
6 checks passed
@meling meling deleted the eddsa branch March 10, 2024 11:08
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.

2 participants