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

runtime/common/crypto/signature: Switch Ed25519 to our consensus flavor #3946

Merged
merged 1 commit into from
May 17, 2021

Conversation

Yawning
Copy link
Contributor

@Yawning Yawning commented May 14, 2021

We currently use semantics that are close to ed25519-dalek's strict,
but we require using the cofactored verification equation. Implementing
verification ourselves lets us alter the behavior in the future as
we see fit, though "Just use Ristretto".

TODO:

@Yawning Yawning added the s:blocked Status: blocked on other work label May 14, 2021
@Yawning Yawning force-pushed the yawning/feature/rust-ed25519 branch from aabd9b8 to a795d3a Compare May 14, 2021 14:28
@codecov
Copy link

codecov bot commented May 14, 2021

Codecov Report

Merging #3946 (aa0544b) into master (42272a3) will increase coverage by 0.02%.
The diff coverage is 85.71%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3946      +/-   ##
==========================================
+ Coverage   66.99%   67.01%   +0.02%     
==========================================
  Files         406      406              
  Lines       41300    41351      +51     
==========================================
+ Hits        27667    27713      +46     
+ Misses       9681     9679       -2     
- Partials     3952     3959       +7     
Impacted Files Coverage Δ
go/staking/api/api.go 58.87% <40.00%> (ø)
...o/consensus/tendermint/apps/staking/state/state.go 63.44% <85.93%> (+2.14%) ⬆️
.../consensus/tendermint/apps/staking/transactions.go 67.85% <87.50%> (+0.26%) ⬆️
go/consensus/tendermint/apps/staking/staking.go 53.16% <100.00%> (+0.60%) ⬆️
go/staking/tests/tester.go 91.82% <100.00%> (+0.11%) ⬆️
go/consensus/tendermint/abci/state/state.go 54.54% <0.00%> (-9.10%) ⬇️
go/consensus/tendermint/apps/staking/auth.go 75.00% <0.00%> (-8.34%) ⬇️
go/common/grpc/policy/policy.go 62.12% <0.00%> (-7.58%) ⬇️
go/common/sgx/aesm/aesm.go 62.61% <0.00%> (-6.55%) ⬇️
go/worker/storage/service_external.go 48.57% <0.00%> (-5.72%) ⬇️
... and 27 more

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 ade6136...aa0544b. Read the comment docs.

@Yawning Yawning force-pushed the yawning/feature/rust-ed25519 branch 5 times, most recently from f88fd11 to 812ec65 Compare May 17, 2021 12:02
We currently use semantics that are close to ed25519-dalek's strict,
but we require using the cofactored verification equation.  Implementing
verification ourselves lets us alter the behavior in the future as
we see fit, though "Just use Ristretto".
@Yawning Yawning force-pushed the yawning/feature/rust-ed25519 branch from 812ec65 to aa0544b Compare May 17, 2021 12:07
@Yawning Yawning removed the s:blocked Status: blocked on other work label May 17, 2021
@Yawning
Copy link
Contributor Author

Yawning commented May 17, 2021

You know what makes writing Rust even less fun than it already is? Having to use CI as the compiler.

@Yawning Yawning marked this pull request as ready for review May 17, 2021 12:18
@Yawning Yawning enabled auto-merge May 17, 2021 12:19
@Yawning Yawning merged commit c287752 into master May 17, 2021
@Yawning Yawning deleted the yawning/feature/rust-ed25519 branch May 17, 2021 16: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.

None yet

2 participants