-
Notifications
You must be signed in to change notification settings - Fork 224
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
Replace signatory
with ed25519-dalek
and k256
crates
#522
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,13 @@ | ||
use super::compute_prefix; | ||
use crate::public_key::PublicKey; | ||
use crate::{ | ||
error, | ||
public_key::{Ed25519, PublicKey}, | ||
Error, | ||
}; | ||
use anomaly::format_err; | ||
use once_cell::sync::Lazy; | ||
use prost_amino_derive::Message; | ||
use signatory::ed25519::PUBLIC_KEY_SIZE; | ||
use std::convert::TryFrom; | ||
|
||
// Note:On the golang side this is generic in the sense that it could everything that implements | ||
// github.com/tendermint/tendermint/crypto.PubKey | ||
|
@@ -24,13 +29,15 @@ pub struct PubKeyResponse { | |
#[amino_name = "tendermint/remotesigner/PubKeyRequest"] | ||
pub struct PubKeyRequest {} | ||
|
||
impl From<PubKeyResponse> for PublicKey { | ||
impl TryFrom<PubKeyResponse> for PublicKey { | ||
type Error = Error; | ||
|
||
// This does not check if the underlying pub_key_ed25519 has the right size. | ||
// The caller needs to make sure that this is actually the case. | ||
fn from(response: PubKeyResponse) -> PublicKey { | ||
let mut public_key = [0u8; PUBLIC_KEY_SIZE]; | ||
public_key.copy_from_slice(response.pub_key_ed25519.as_ref()); | ||
PublicKey::Ed25519(signatory::ed25519::PublicKey::new(public_key)) | ||
fn try_from(response: PubKeyResponse) -> Result<PublicKey, Error> { | ||
Ed25519::from_bytes(&response.pub_key_ed25519) | ||
.map(Into::into) | ||
.map_err(|_| format_err!(error::Kind::InvalidKey, "malformed Ed25519 key").into()) | ||
} | ||
} | ||
|
||
|
@@ -49,7 +56,9 @@ impl From<PublicKey> for PubKeyResponse { | |
#[cfg(test)] | ||
mod tests { | ||
use super::*; | ||
use ed25519_dalek::PUBLIC_KEY_LENGTH; | ||
use prost_amino::Message; | ||
use std::convert::TryInto; | ||
|
||
#[test] | ||
fn test_empty_pubkey_msg() { | ||
|
@@ -140,21 +149,21 @@ mod tests { | |
|
||
#[test] | ||
fn test_into() { | ||
let raw_pk: [u8; PUBLIC_KEY_SIZE] = [ | ||
0x79, 0xce, 0xd, 0xe0, 0x43, 0x33, 0x4a, 0xec, 0xe0, 0x8b, 0x7b, 0xb5, 0x61, 0xbc, | ||
0xe7, 0xc1, 0xd4, 0x69, 0xc3, 0x44, 0x26, 0xec, 0xef, 0xc0, 0x72, 0xa, 0x52, 0x4d, | ||
0x37, 0x32, 0xef, 0xed, | ||
let raw_pk: [u8; PUBLIC_KEY_LENGTH] = [ | ||
0xaf, 0xf3, 0x94, 0xc5, 0xb7, 0x5c, 0xfb, 0xd, 0xd9, 0x28, 0xe5, 0x8a, 0x92, 0xdd, | ||
0x76, 0x55, 0x2b, 0x2e, 0x8d, 0x19, 0x6f, 0xe9, 0x12, 0x14, 0x50, 0x80, 0x6b, 0xd0, | ||
0xd9, 0x3f, 0xd0, 0xcb, | ||
Comment on lines
-143
to
+155
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ed(wards)25519 point decompression was failing for this? (the old implementation didn't attempt to decompress points) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK, I've tested against a few freshly generated pub-keys and they all pass! I think that |
||
]; | ||
let want = PublicKey::Ed25519(signatory::ed25519::PublicKey::new(raw_pk)); | ||
let want = PublicKey::Ed25519(Ed25519::from_bytes(&raw_pk).unwrap()); | ||
let pk = PubKeyResponse { | ||
pub_key_ed25519: vec![ | ||
0x79, 0xce, 0xd, 0xe0, 0x43, 0x33, 0x4a, 0xec, 0xe0, 0x8b, 0x7b, 0xb5, 0x61, 0xbc, | ||
0xe7, 0xc1, 0xd4, 0x69, 0xc3, 0x44, 0x26, 0xec, 0xef, 0xc0, 0x72, 0xa, 0x52, 0x4d, | ||
0x37, 0x32, 0xef, 0xed, | ||
0xaf, 0xf3, 0x94, 0xc5, 0xb7, 0x5c, 0xfb, 0xd, 0xd9, 0x28, 0xe5, 0x8a, 0x92, 0xdd, | ||
0x76, 0x55, 0x2b, 0x2e, 0x8d, 0x19, 0x6f, 0xe9, 0x12, 0x14, 0x50, 0x80, 0x6b, 0xd0, | ||
0xd9, 0x3f, 0xd0, 0xcb, | ||
], | ||
}; | ||
let orig = pk.clone(); | ||
let got: PublicKey = pk.into(); | ||
let got: PublicKey = pk.try_into().unwrap(); | ||
|
||
assert_eq!(got, want); | ||
|
||
|
@@ -170,6 +179,6 @@ mod tests { | |
pub_key_ed25519: vec![], | ||
}; | ||
// we expect this to panic: | ||
let _got: PublicKey = empty_msg.into(); | ||
let _got: PublicKey = empty_msg.try_into().unwrap(); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,7 @@ | |
//! blockchain networks, including chain information types, secret connections, | ||
//! and remote procedure calls (JSONRPC). | ||
|
||
#![cfg_attr(docsrs, feature(doc_cfg))] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should add this to all our other crates too. |
||
#![deny( | ||
warnings, | ||
missing_docs, | ||
|
@@ -50,17 +51,17 @@ pub mod vote; | |
#[cfg(test)] | ||
mod test; | ||
|
||
pub use crate::genesis::Genesis; | ||
pub use crate::{ | ||
block::Block, | ||
error::{Error, Kind}, | ||
genesis::Genesis, | ||
hash::Hash, | ||
moniker::Moniker, | ||
private_key::PrivateKey, | ||
public_key::{PublicKey, TendermintKey}, | ||
signature::Signature, | ||
time::Time, | ||
timeout::Timeout, | ||
version::Version, | ||
vote::Vote, | ||
}; | ||
pub use private_key::PrivateKey; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't look into k256 but tendermint-rs doesn't really use secp256k1. Hence this is not critical from tendermint's POV.