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

Keypair Serde is Broken #18

Closed
punwai opened this issue Jul 22, 2022 · 6 comments · Fixed by #284
Closed

Keypair Serde is Broken #18

punwai opened this issue Jul 22, 2022 · 6 comments · Fixed by #284
Assignees
Labels
bug Something isn't working crypto

Comments

@punwai
Copy link
Contributor

punwai commented Jul 22, 2022

Description

Keypair serializes, but does not deserialize when trying to test it out in MystenLabs/narwhal#557. Does not break Narwhal nor Sui so continuing with MystenLabs/narwhal#557 anyways.

To replicate this issue, run the following test:

#[test]
fn test_kp_serde() {
    let kp = keys().pop().unwrap();
    let serialized = bincode::serialize(&kp).unwrap();
    let deserialized: Ed25519KeyPair = bincode::deserialize(&serialized).unwrap();
}

We get this error:

thread 'bls12377_tests::test_kp_serde' panicked at 'called `Result::unwrap()` on an `Err` value: Custom("invalid Base64 encoding")', crypto/src/tests/bls12377_tests.rs:375:75

Upon further digging, it looks like removing this line fixes the issue:

#[serde(tag = "type")] // REMOVE THIS LINE
pub struct Ed25519KeyPair {
    name: Ed25519PublicKey,
    secret: Ed25519PrivateKey,
}
@punwai punwai added the bug Something isn't working label Jul 22, 2022
@huitseeker
Copy link
Contributor

@punwai is this still active, and if so, do you have instructions to reproduce?

@huitseeker
Copy link
Contributor

@punwai ping

@punwai
Copy link
Contributor Author

punwai commented Aug 12, 2022

Yes, sorry, I forgot to ping you when I added the context @huitseeker, but I assume you've seen it now

@huitseeker
Copy link
Contributor

This is probably fixed in MystenLabs/narwhal#756

@huitseeker huitseeker transferred this issue from MystenLabs/narwhal Aug 23, 2022
@kchalkias
Copy link
Collaborator

@punwai has this been resolved?

@huitseeker huitseeker self-assigned this Nov 28, 2022
huitseeker added a commit to huitseeker/fastcrypto that referenced this issue Nov 28, 2022
@huitseeker
Copy link
Contributor

I have an upcoming fix, I'll test its compatibility w/Sui before opening a PR though.
https://github.com/huitseeker/fastcrypto/pull/new/test_keypair_serde

huitseeker added a commit to huitseeker/fastcrypto that referenced this issue Dec 8, 2022
huitseeker added a commit to huitseeker/fastcrypto that referenced this issue Dec 8, 2022
huitseeker added a commit that referenced this issue Dec 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working crypto
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants