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

crypto: Fix the rust and go ideas of Ed25519 to match reality #140

Merged
merged 2 commits into from
May 18, 2021

Conversation

Yawning
Copy link
Contributor

@Yawning Yawning commented May 18, 2021

Part of #137

@Yawning Yawning requested review from kostko, pro-wh and ptrus as code owners May 18, 2021 12:40
@codecov
Copy link

codecov bot commented May 18, 2021

Codecov Report

Merging #140 (3ba7126) into main (ab02f6c) will increase coverage by 0.39%.
The diff coverage is 50.00%.

❗ Current head 3ba7126 differs from pull request most recent head adb9529. Consider uploading reports for the commit adb9529 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##             main     #140      +/-   ##
==========================================
+ Coverage   68.79%   69.19%   +0.39%     
==========================================
  Files          51       43       -8     
  Lines        2724     2545     -179     
==========================================
- Hits         1874     1761     -113     
+ Misses        831      784      -47     
+ Partials       19        0      -19     
Impacted Files Coverage Δ
runtime-sdk/src/crypto/signature/ed25519.rs 56.00% <50.00%> (+1.00%) ⬆️
client-sdk/go/types/crypto.go
client-sdk/go/types/transaction.go
client-sdk/go/testing/testing.go
client-sdk/go/types/address.go
client-sdk/go/types/token.go
client-sdk/go/crypto/signature/context.go
...ent-sdk/go/crypto/signature/secp256k1/secp256k1.go
client-sdk/go/crypto/signature/secp256k1/signer.go

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 ab02f6c...adb9529. Read the comment docs.

@Yawning Yawning force-pushed the yawning/feature/fix-ed25519 branch from a395839 to 1bd7bbe Compare May 18, 2021 12:48
This also bumps the oasis-core version because old versions did not
implement Ed25519 verification with the semantics that we want.
@Yawning Yawning force-pushed the yawning/feature/fix-ed25519 branch from 1bd7bbe to 5f31293 Compare May 18, 2021 12:57
The runtime-sdk uses a newer version of oasis-core, so bring it back
into sync.
@Yawning Yawning force-pushed the yawning/feature/fix-ed25519 branch from 5f31293 to adb9529 Compare May 18, 2021 13:10
@Yawning Yawning enabled auto-merge May 18, 2021 13:15
Ok(PublicKey(
ed25519_dalek::PublicKey::from_bytes(bytes).map_err(|_| Error::MalformedPublicKey)?,
))
// CorePublicKey::from doesn't support error checking.
Copy link
Member

Choose a reason for hiding this comment

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

Could implement TryFrom or from_bytes for the core public key and move this there.

@Yawning Yawning merged commit 0a5add6 into main May 18, 2021
@Yawning Yawning deleted the yawning/feature/fix-ed25519 branch May 18, 2021 16:13
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