Skip to content
This repository has been archived by the owner on Mar 1, 2023. It is now read-only.

Use stdlib when possible #14

Merged
merged 2 commits into from
Dec 1, 2022
Merged

Conversation

fasmat
Copy link
Member

@fasmat fasmat commented Nov 29, 2022

This updates ed25519 to use the optimised stdlib implementation when it can and only use our own code for Sign2, Verify2 and ExtractPublicKey since those are not available in the standard library.

Benchmarks on Macbook M1:

BenchmarkPublicKeyExtraction-6              6666            176173 ns/op              32 B/op          1 allocs/op

BenchmarkSigningExt-6                      36211             33105 ns/op              64 B/op          1 allocs/op
BenchmarkSigning-6                         54192             22148 ns/op               0 B/op          0 allocs/op

BenchmarkVerificationExt-6                 13506             89068 ns/op               0 B/op          0 allocs/op
BenchmarkVerification-6                    25872             46401 ns/op               0 B/op          0 allocs/op

@fasmat fasmat requested a review from pigmej November 29, 2022 11:39
@fasmat fasmat self-assigned this Nov 29, 2022
@pigmej pigmej requested a review from dshulyak November 29, 2022 12:30
@fasmat
Copy link
Member Author

fasmat commented Nov 29, 2022

These are the benchmarks on my machine for the Sign and Verify functions on the master branch:

BenchmarkSigning-6                         36337             32931 ns/op              64 B/op          1 allocs/op
BenchmarkVerification-6                    13368             90031 ns/op               0 B/op          0 allocs/op

Copy link
Member

@pigmej pigmej left a comment

Choose a reason for hiding this comment

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

So from my side it makes sense. I cannot assess all the side effects, though. Checked both m1 and amd64.

Having separate opinion from @dshulyak is required.

@dshulyak
Copy link

dshulyak commented Dec 1, 2022

i don't understand why spacemesh library needs to wrap any other lib
if i had to do it, i would create a lib named ed25519extract (or similar) and leave only sign/extract/verify functions that work with key extraction

but if there is some preference doing it this way i don't mind

@fasmat
Copy link
Member Author

fasmat commented Dec 1, 2022

I see one reason to do this: If we change/remove the API of this package we break existing software using it.

This would be a "soft" migration away from our implementation of ed25519 where we don't break any existing software using this library, while at the same time allowing users of it a soft transistion to crypto/ed25519 similar how the transition from golang.org/x/net/context to context was handled. The former is now also just a wrapper around the later.

@fasmat fasmat merged commit e3bb34d into develop Dec 1, 2022
@fasmat fasmat deleted the use-stdlib-functions-when-possible branch December 1, 2022 18:42
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants