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.

Sign up for GitHub

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

Jump to bottom

Add ed25519 support #29

Merged
merged 1 commit into from
May 23, 2024
Merged

Add ed25519 support #29

merged 1 commit into from
May 23, 2024

Conversation

johannww
Copy link
Contributor

This is related to fabric pull request #3343 regarding adding ed25519 support. Since the bccsp folder was moved to fabric-lib-go, I am moving the related changes here.

@johannww johannww requested a review from a team as a code owner February 21, 2024 15:04
@johannww johannww mentioned this pull request Feb 21, 2024
Merged
@bestbeforetoday
Copy link
Member

@hyperledger/fabric-core-maintainers Can somebody review this change?

@yacovm
Copy link

yacovm commented Apr 29, 2024

LGTM, maybe @adecaro can also review this, given he is the author of BCCSP?

@denyeart
Copy link
Contributor

@Mergifyio rebase

@johannww @denyeart
Add ed25519 support
8846300 
related to fabric pull request #3343,
regarding adding ed25519 support. Since the
bccsp folder was moved to fabric-lib-go,
I am moving the related changes here.

Signed-off-by: Johann Westphall <[email protected]>
@mergify Mergify
Copy link

mergify bot commented Apr 30, 2024

rebase

✅ Branch has been successfully rebased

@bestbeforetoday
Copy link
Member

bestbeforetoday commented May 14, 2024 •
edited
Loading

How many weeks do we wait for additional reviews before we just go with the ones we've already had and merge this? That might give the opportunity for beta users to actually use it and give us more confidence that all is good.

Just a reminder that this change was started more than two years ago, and we already missed getting it into Fabric v2.5. Please let's get it into v3.

@denyeart
Copy link
Contributor

Agree it has been too long, I'm expecting this will be in for v3.
I do think we want one of the project crypto experts to review before merging though, that would either be @adecaro or @ale-linux

@adecaro
Copy link

adecaro commented May 22, 2024

This looks good to me overall. I just want to check a few more details before approving the PR.
In the meantime, I have a question: Is this enough to then use X509 certificates with ed25519 keys for endorsement?

@adecaro
Copy link

adecaro commented May 23, 2024

LGTM, thanks :)

@yacovm yacovm merged commit 25edd1e into hyperledger:main May 23, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yacovm yacovm yacovm approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@johannww @bestbeforetoday @yacovm @denyeart @adecaro