From ad44d7231d5555b6875fa33233471bc543603bb6 Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Mon, 17 Feb 2020 11:35:56 +0100 Subject: [PATCH 01/68] Add KEY_KIND_ID to the public trait This change is being introduced for the purpose of identifying a public key with it's identifier and algorithm "kind". --- primitives/core/src/crypto.rs | 9 ++++++++- primitives/core/src/ecdsa.rs | 1 + primitives/core/src/ed25519.rs | 7 +++++++ primitives/core/src/sr25519.rs | 7 +++++++ primitives/core/src/testing.rs | 35 ++++++++++++++++++++++++++++++++-- primitives/core/src/traits.rs | 14 +++++++++++++- 6 files changed, 69 insertions(+), 4 deletions(-) diff --git a/primitives/core/src/crypto.rs b/primitives/core/src/crypto.rs index 5ad5e1981379b..384d0b97e3551 100644 --- a/primitives/core/src/crypto.rs +++ b/primitives/core/src/crypto.rs @@ -38,6 +38,9 @@ use zeroize::Zeroize; pub use sp_std::ops::Deref; use sp_runtime_interface::pass_by::PassByInner; +/// Shorthand type for declaring the public key kind identifier +pub type KeyKindId = &'static str; + /// The root phrase for our publicly known keys. pub const DEV_PHRASE: &str = "bottom drive obey lake curtain smoke basket hold race lonely fit walk"; @@ -515,7 +518,11 @@ impl + AsRef<[u8]> + Default + Derive> Ss58Codec for T { } /// Trait suitable for typical cryptographic PKI key public type. -pub trait Public: AsRef<[u8]> + AsMut<[u8]> + Default + Derive + CryptoType + PartialEq + Eq + Clone + Send + Sync { +pub trait Public: + AsRef<[u8]> + AsMut<[u8]> + Default + Derive + CryptoType + PartialEq + Eq + Clone + Send + Sync +{ + /// Each implementation of Public should define its Kind identifier + const KEY_KIND_ID: &'static str = ""; /// A new instance from the given slice. /// /// NOTE: No checking goes on to ensure this is a real public key. Only use it if diff --git a/primitives/core/src/ecdsa.rs b/primitives/core/src/ecdsa.rs index 99db0e6b2d1a0..3a9da57fa0994 100644 --- a/primitives/core/src/ecdsa.rs +++ b/primitives/core/src/ecdsa.rs @@ -95,6 +95,7 @@ impl Public { } impl TraitPublic for Public { + const KEY_KIND_ID: &'static str = "sr25519"; /// A new instance from the given slice that should be 33 bytes long. /// /// NOTE: No checking goes on to ensure this is a real public key. Only use it if diff --git a/primitives/core/src/ed25519.rs b/primitives/core/src/ed25519.rs index f3aba47c20457..55f6426b57b7d 100644 --- a/primitives/core/src/ed25519.rs +++ b/primitives/core/src/ed25519.rs @@ -42,6 +42,11 @@ use crate::{crypto::{Public as TraitPublic, UncheckedFrom, CryptoType, Derive}}; use sp_runtime_interface::pass_by::PassByInner; use sp_std::ops::Deref; +/// A value which is passed along with the public key +/// to define which cryptographic algorithm that key +/// belongs to. +pub const ED25519_KIND_ID: &str = "ed25519"; + /// A secret seed. It's not called a "secret key" because ring doesn't expose the secret keys /// of the key pair (yeah, dumb); as such we're forced to remember the seed manually if we /// will need it later (such as for HDKD). @@ -365,6 +370,8 @@ impl Public { } impl TraitPublic for Public { + const KEY_KIND_ID: &'static str = ED25519_KIND_ID; + /// A new instance from the given slice that should be 32 bytes long. /// /// NOTE: No checking goes on to ensure this is a real public key. Only use it if diff --git a/primitives/core/src/sr25519.rs b/primitives/core/src/sr25519.rs index 3495f32872f16..8530d16a60e24 100644 --- a/primitives/core/src/sr25519.rs +++ b/primitives/core/src/sr25519.rs @@ -54,6 +54,11 @@ use sp_runtime_interface::pass_by::PassByInner; #[cfg(feature = "full_crypto")] const SIGNING_CTX: &[u8] = b"substrate"; +/// A value which is passed along with the public key +/// to define which cryptographic algorithm that key +/// belongs to. +pub const SR25519_KIND_ID: &str = "sr25519"; + /// An Schnorrkel/Ristretto x25519 ("sr25519") public key. #[cfg_attr(feature = "full_crypto", derive(Hash))] #[derive(PartialEq, Eq, PartialOrd, Ord, Clone, Copy, Encode, Decode, Default, PassByInner)] @@ -379,6 +384,8 @@ impl Public { } impl TraitPublic for Public { + const KEY_KIND_ID: &'static str = SR25519_KIND_ID; + /// A new instance from the given slice that should be 32 bytes long. /// /// NOTE: No checking goes on to ensure this is a real public key. Only use it if diff --git a/primitives/core/src/testing.rs b/primitives/core/src/testing.rs index 0def3454bc899..db1cad84647f1 100644 --- a/primitives/core/src/testing.rs +++ b/primitives/core/src/testing.rs @@ -16,9 +16,12 @@ //! Types that should only be used for testing! +use crate::crypto::{KeyKindId, KeyTypeId}; #[cfg(feature = "std")] -use crate::{ed25519, sr25519, crypto::{Public, Pair}}; -use crate::crypto::KeyTypeId; +use crate::{ + crypto::{Pair, Public}, + ed25519, sr25519, +}; /// Key type for generic Ed25519 key. pub const ED25519: KeyTypeId = KeyTypeId(*b"ed25"); @@ -131,6 +134,34 @@ impl crate::traits::BareCryptoStore for KeyStore { fn has_keys(&self, public_keys: &[(Vec, KeyTypeId)]) -> bool { public_keys.iter().all(|(k, t)| self.keys.get(&t).and_then(|s| s.get(k)).is_some()) } + + fn sign_with(&self, kind: KeyKindId, id: KeyTypeId, msg: &[u8]) -> Result, String> { + match kind { + "ed25519" => { + let ed_public_key = self + .ed25519_public_keys(id) + .pop() + .ok_or(String::from("ed25519 key not found"))?; + let key_pair: ed25519::Pair = self + .ed25519_key_pair(id, &ed_public_key) + .map(Into::into) + .ok_or(String::from("ed25519 pair not found"))?; + return Ok(<[u8; 64]>::from(key_pair.sign(msg)).to_vec()); + } + "sr25519" => { + let sr_public_key = self + .sr25519_public_keys(id) + .pop() + .ok_or(String::from("sr25519 key not found"))?; + let key_pair: sr25519::Pair = self + .sr25519_key_pair(id, &sr_public_key) + .map(Into::into) + .ok_or(String::from("sr25519 pair not found"))?; + return Ok(<[u8; 64]>::from(key_pair.sign(msg)).to_vec()); + } + _ => Err(String::from("Key kind invalid")), + } + } } /// Macro for exporting functions from wasm in with the expected signature for using it with the diff --git a/primitives/core/src/traits.rs b/primitives/core/src/traits.rs index bd02d39fb55a5..0018d30475677 100644 --- a/primitives/core/src/traits.rs +++ b/primitives/core/src/traits.rs @@ -16,7 +16,10 @@ //! Shareable Substrate traits. -use crate::{crypto::KeyTypeId, ed25519, sr25519}; +use crate::{ + crypto::{KeyKindId, KeyTypeId}, + ed25519, sr25519, +}; use std::{ fmt::{Debug, Display}, @@ -74,6 +77,15 @@ pub trait BareCryptoStore: Send + Sync { /// /// Returns `true` iff all private keys could be found. fn has_keys(&self, public_keys: &[(Vec, KeyTypeId)]) -> bool; + + /// Signs a message with the private key that matches + /// the public key passed. + fn sign_with( + &self, + kind: KeyKindId, + id: KeyTypeId, + msg: &[u8], + ) -> std::result::Result, String>; } /// A pointer to the key store. From 3199d1e27c2d9e1b05f62fd0b003cdbdae626f7c Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Mon, 17 Feb 2020 12:24:34 +0100 Subject: [PATCH 02/68] Use `sign_with` as implemented in BareCryptoStore --- primitives/io/src/lib.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/primitives/io/src/lib.rs b/primitives/io/src/lib.rs index ce8a546e86ff8..defa43de72efb 100644 --- a/primitives/io/src/lib.rs +++ b/primitives/io/src/lib.rs @@ -410,8 +410,9 @@ pub trait Crypto { self.extension::() .expect("No `keystore` associated for the current context!") .read() - .ed25519_key_pair(id, &pub_key) - .map(|k| k.sign(msg)) + .sign_with(ed25519::ED25519_KIND_ID, id, msg) + .map(|sig| ed25519::Signature::from_slice(sig.as_slice())) + .ok() } /// Verify an `ed25519` signature. @@ -462,8 +463,9 @@ pub trait Crypto { self.extension::() .expect("No `keystore` associated for the current context!") .read() - .sr25519_key_pair(id, &pub_key) - .map(|k| k.sign(msg)) + .sign_with(sr25519::SR25519_KIND_ID, id, msg) + .map(|sig| sr25519::Signature::from_slice(sig.as_slice())) + .ok() } /// Verify an `sr25519` signature. From 25f4a5ce1402033d59ab5ffa93ff439936cdbd86 Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Mon, 17 Feb 2020 14:20:27 +0100 Subject: [PATCH 03/68] Implement `sign_with` in sc_keystore --- client/keystore/src/lib.rs | 53 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 51 insertions(+), 2 deletions(-) diff --git a/client/keystore/src/lib.rs b/client/keystore/src/lib.rs index ef81c40c10f51..3c31a56fc0add 100644 --- a/client/keystore/src/lib.rs +++ b/client/keystore/src/lib.rs @@ -21,7 +21,8 @@ use std::{collections::HashMap, path::PathBuf, fs::{self, File}, io::{self, Write}, sync::Arc}; use sp_core::{ - crypto::{KeyTypeId, Pair as PairT, Public, IsWrappedBy, Protected}, traits::BareCryptoStore, + crypto::{IsWrappedBy, KeyKindId, KeyTypeId, Pair as PairT, Protected, Public}, + traits::BareCryptoStore, }; use sp_application_crypto::{AppKey, AppPublic, AppPair, ed25519, sr25519}; @@ -276,11 +277,59 @@ impl Store { buf.push(key_type + key.as_str()); Some(buf) } + + /// Signs a message with the private key that matches + /// the public key passed. + fn _sign_with( + &self, + kind: KeyKindId, + id: KeyTypeId, + msg: &[u8], + ) -> std::result::Result, String> { + match kind { + ed25519::ED25519_KIND_ID => { + let public_key = self + .ed25519_public_keys(id) + .pop() + .ok_or(String::from("ed25519 key not found"))?; + + let key_pair: ed25519::Pair = self + .key_pair_by_type::(&public_key, id) + .map(Into::into) + .map_err(|e| e.to_string())?; + return Ok(<[u8; 64]>::from(key_pair.sign(msg)).to_vec()); + } + sr25519::SR25519_KIND_ID => { + let public_key = self + .sr25519_public_keys(id) + .pop() + .ok_or(String::from("sr25519 key not found"))?; + let key_pair: sr25519::Pair = self + .key_pair_by_type::(&public_key, id) + .map(Into::into) + .map_err(|e| e.to_string())?; + return Ok(<[u8; 64]>::from(key_pair.sign(msg)).to_vec()); + } + _ => Err(String::from("Key kind invalid")), + } + } } impl BareCryptoStore for Store { + /// Signs a message with the private key that matches + /// the public key passed. + fn sign_with( + &self, + kind: KeyKindId, + id: KeyTypeId, + msg: &[u8], + ) -> std::result::Result, String> { + self._sign_with(kind, id, msg) + } + fn sr25519_public_keys(&self, key_type: KeyTypeId) -> Vec { - self.public_keys_by_type::(key_type).unwrap_or_default() + self.public_keys_by_type::(key_type) + .unwrap_or_default() } fn sr25519_generate_new( From 8cc6dfd370b0b4ecb25fcaaa4ef2a259da6e2c18 Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Mon, 17 Feb 2020 15:52:09 +0100 Subject: [PATCH 04/68] Fix inconsistencies, use *_KIND_ID in sp_core testing --- client/keystore/src/lib.rs | 4 +--- primitives/core/src/ecdsa.rs | 7 ++++++- primitives/core/src/testing.rs | 4 ++-- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/client/keystore/src/lib.rs b/client/keystore/src/lib.rs index 3c31a56fc0add..66a021d332e07 100644 --- a/client/keystore/src/lib.rs +++ b/client/keystore/src/lib.rs @@ -310,14 +310,12 @@ impl Store { .map_err(|e| e.to_string())?; return Ok(<[u8; 64]>::from(key_pair.sign(msg)).to_vec()); } - _ => Err(String::from("Key kind invalid")), + _ => Err(String::from("Key kind invalid")) } } } impl BareCryptoStore for Store { - /// Signs a message with the private key that matches - /// the public key passed. fn sign_with( &self, kind: KeyKindId, diff --git a/primitives/core/src/ecdsa.rs b/primitives/core/src/ecdsa.rs index 3a9da57fa0994..1c4fe2ea55784 100644 --- a/primitives/core/src/ecdsa.rs +++ b/primitives/core/src/ecdsa.rs @@ -40,6 +40,11 @@ use crate::crypto::{Public as TraitPublic, UncheckedFrom, CryptoType, Derive}; #[cfg(feature = "full_crypto")] use secp256k1::{PublicKey, SecretKey}; +/// A value which is passed along with the public key +/// to define which cryptographic algorithm that key +/// belongs to. +pub const ECDSA_KIND_ID: &str = "ecdsa"; + /// A secret seed (which is bytewise essentially equivalent to a SecretKey). /// /// We need it as a different type because `Seed` is expected to be AsRef<[u8]>. @@ -95,7 +100,7 @@ impl Public { } impl TraitPublic for Public { - const KEY_KIND_ID: &'static str = "sr25519"; + const KEY_KIND_ID: &'static str = ECDCA_KIND_ID; /// A new instance from the given slice that should be 33 bytes long. /// /// NOTE: No checking goes on to ensure this is a real public key. Only use it if diff --git a/primitives/core/src/testing.rs b/primitives/core/src/testing.rs index db1cad84647f1..822159633adb1 100644 --- a/primitives/core/src/testing.rs +++ b/primitives/core/src/testing.rs @@ -137,7 +137,7 @@ impl crate::traits::BareCryptoStore for KeyStore { fn sign_with(&self, kind: KeyKindId, id: KeyTypeId, msg: &[u8]) -> Result, String> { match kind { - "ed25519" => { + ed25519::ED25519_KIND_ID => { let ed_public_key = self .ed25519_public_keys(id) .pop() @@ -148,7 +148,7 @@ impl crate::traits::BareCryptoStore for KeyStore { .ok_or(String::from("ed25519 pair not found"))?; return Ok(<[u8; 64]>::from(key_pair.sign(msg)).to_vec()); } - "sr25519" => { + sr25519::SR25519_KIND_ID => { let sr_public_key = self .sr25519_public_keys(id) .pop() From 4895e80286781ae8ecd09f1c4f5a3b8579288e7a Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Tue, 18 Feb 2020 14:07:48 +0100 Subject: [PATCH 05/68] Rename KeyKindId to CryptoTypeId --- client/keystore/src/lib.rs | 10 +++++----- primitives/core/src/crypto.rs | 14 +++++++++----- primitives/core/src/ecdsa.rs | 6 +++--- primitives/core/src/ed25519.rs | 6 +++--- primitives/core/src/sr25519.rs | 6 +++--- primitives/core/src/testing.rs | 8 ++++---- primitives/core/src/traits.rs | 4 ++-- primitives/io/src/lib.rs | 4 ++-- 8 files changed, 31 insertions(+), 27 deletions(-) diff --git a/client/keystore/src/lib.rs b/client/keystore/src/lib.rs index 66a021d332e07..e764aff7168f5 100644 --- a/client/keystore/src/lib.rs +++ b/client/keystore/src/lib.rs @@ -21,7 +21,7 @@ use std::{collections::HashMap, path::PathBuf, fs::{self, File}, io::{self, Write}, sync::Arc}; use sp_core::{ - crypto::{IsWrappedBy, KeyKindId, KeyTypeId, Pair as PairT, Protected, Public}, + crypto::{IsWrappedBy, CryptoTypeId, KeyTypeId, Pair as PairT, Protected, Public}, traits::BareCryptoStore, }; @@ -282,12 +282,12 @@ impl Store { /// the public key passed. fn _sign_with( &self, - kind: KeyKindId, + kind: CryptoTypeId, id: KeyTypeId, msg: &[u8], ) -> std::result::Result, String> { match kind { - ed25519::ED25519_KIND_ID => { + ed25519::ED25519_CRYPTO_ID => { let public_key = self .ed25519_public_keys(id) .pop() @@ -299,7 +299,7 @@ impl Store { .map_err(|e| e.to_string())?; return Ok(<[u8; 64]>::from(key_pair.sign(msg)).to_vec()); } - sr25519::SR25519_KIND_ID => { + sr25519::SR25519_CRYPTO_ID => { let public_key = self .sr25519_public_keys(id) .pop() @@ -318,7 +318,7 @@ impl Store { impl BareCryptoStore for Store { fn sign_with( &self, - kind: KeyKindId, + kind: CryptoTypeId, id: KeyTypeId, msg: &[u8], ) -> std::result::Result, String> { diff --git a/primitives/core/src/crypto.rs b/primitives/core/src/crypto.rs index 384d0b97e3551..b1a288da2925e 100644 --- a/primitives/core/src/crypto.rs +++ b/primitives/core/src/crypto.rs @@ -38,9 +38,6 @@ use zeroize::Zeroize; pub use sp_std::ops::Deref; use sp_runtime_interface::pass_by::PassByInner; -/// Shorthand type for declaring the public key kind identifier -pub type KeyKindId = &'static str; - /// The root phrase for our publicly known keys. pub const DEV_PHRASE: &str = "bottom drive obey lake curtain smoke basket hold race lonely fit walk"; @@ -522,7 +519,8 @@ pub trait Public: AsRef<[u8]> + AsMut<[u8]> + Default + Derive + CryptoType + PartialEq + Eq + Clone + Send + Sync { /// Each implementation of Public should define its Kind identifier - const KEY_KIND_ID: &'static str = ""; + const CRYPTO_TYPE_ID: CryptoTypeId; + /// A new instance from the given slice. /// /// NOTE: No checking goes on to ensure this is a real public key. Only use it if @@ -664,6 +662,8 @@ mod dummy { impl Derive for Dummy {} impl Public for Dummy { + const CRYPTO_TYPE_ID: CryptoTypeId = CryptoTypeId("dummy"); + fn from_slice(_: &[u8]) -> Self { Self } #[cfg(feature = "std")] fn to_raw_vec(&self) -> Vec { vec![] } @@ -889,7 +889,7 @@ pub trait CryptoType { /// An identifier for a type of cryptographic key. /// -/// To avoid clashes with other modules when distributing your module publically, register your +/// To avoid clashes with other modules when distributing your module publicly, register your /// `KeyTypeId` on the list here by making a PR. /// /// Values whose first character is `_` are reserved for private use and won't conflict with any @@ -925,6 +925,10 @@ impl<'a> TryFrom<&'a str> for KeyTypeId { } } +/// An identifier for a specific cryptographic algorithm used by a key pair +#[derive(Copy, Clone, Default, PartialEq, Eq, PartialOrd, Ord, Hash, Encode, Decode)] +pub struct CryptoTypeId(pub &'static str); + /// Known key types; this also functions as a global registry of key types for projects wishing to /// avoid collisions with each other. /// diff --git a/primitives/core/src/ecdsa.rs b/primitives/core/src/ecdsa.rs index 1c4fe2ea55784..28e4ee5f78c1b 100644 --- a/primitives/core/src/ecdsa.rs +++ b/primitives/core/src/ecdsa.rs @@ -36,14 +36,14 @@ use crate::{hashing::blake2_256, crypto::{Pair as TraitPair, DeriveJunction, Sec use crate::crypto::Ss58Codec; #[cfg(feature = "std")] use serde::{de, Serializer, Serialize, Deserializer, Deserialize}; -use crate::crypto::{Public as TraitPublic, UncheckedFrom, CryptoType, Derive}; +use crate::crypto::{Public as TraitPublic, UncheckedFrom, CryptoType, Derive, CryptoTypeId}; #[cfg(feature = "full_crypto")] use secp256k1::{PublicKey, SecretKey}; /// A value which is passed along with the public key /// to define which cryptographic algorithm that key /// belongs to. -pub const ECDSA_KIND_ID: &str = "ecdsa"; +pub const ECDSA_CRYPTO_ID: CryptoTypeId = CryptoTypeId("ecdsa"); /// A secret seed (which is bytewise essentially equivalent to a SecretKey). /// @@ -100,7 +100,7 @@ impl Public { } impl TraitPublic for Public { - const KEY_KIND_ID: &'static str = ECDCA_KIND_ID; + const CRYPTO_TYPE_ID: CryptoTypeId = ECDSA_CRYPTO_ID; /// A new instance from the given slice that should be 33 bytes long. /// /// NOTE: No checking goes on to ensure this is a real public key. Only use it if diff --git a/primitives/core/src/ed25519.rs b/primitives/core/src/ed25519.rs index 55f6426b57b7d..f88341a7229c2 100644 --- a/primitives/core/src/ed25519.rs +++ b/primitives/core/src/ed25519.rs @@ -38,14 +38,14 @@ use crate::crypto::{Pair as TraitPair, DeriveJunction, SecretStringError}; use crate::crypto::Ss58Codec; #[cfg(feature = "std")] use serde::{de, Serializer, Serialize, Deserializer, Deserialize}; -use crate::{crypto::{Public as TraitPublic, UncheckedFrom, CryptoType, Derive}}; +use crate::crypto::{Public as TraitPublic, UncheckedFrom, CryptoType, Derive, CryptoTypeId}; use sp_runtime_interface::pass_by::PassByInner; use sp_std::ops::Deref; /// A value which is passed along with the public key /// to define which cryptographic algorithm that key /// belongs to. -pub const ED25519_KIND_ID: &str = "ed25519"; +pub const ED25519_CRYPTO_ID: CryptoTypeId = CryptoTypeId("ed25519"); /// A secret seed. It's not called a "secret key" because ring doesn't expose the secret keys /// of the key pair (yeah, dumb); as such we're forced to remember the seed manually if we @@ -370,7 +370,7 @@ impl Public { } impl TraitPublic for Public { - const KEY_KIND_ID: &'static str = ED25519_KIND_ID; + const CRYPTO_TYPE_ID: CryptoTypeId = ED25519_CRYPTO_ID; /// A new instance from the given slice that should be 32 bytes long. /// diff --git a/primitives/core/src/sr25519.rs b/primitives/core/src/sr25519.rs index 8530d16a60e24..408d16b210e3b 100644 --- a/primitives/core/src/sr25519.rs +++ b/primitives/core/src/sr25519.rs @@ -39,7 +39,7 @@ use crate::crypto::{ #[cfg(feature = "std")] use crate::crypto::Ss58Codec; -use crate::{crypto::{Public as TraitPublic, UncheckedFrom, CryptoType, Derive}}; +use crate::crypto::{Public as TraitPublic, UncheckedFrom, CryptoType, Derive, CryptoTypeId}; use crate::hash::{H256, H512}; use codec::{Encode, Decode}; use sp_std::ops::Deref; @@ -57,7 +57,7 @@ const SIGNING_CTX: &[u8] = b"substrate"; /// A value which is passed along with the public key /// to define which cryptographic algorithm that key /// belongs to. -pub const SR25519_KIND_ID: &str = "sr25519"; +pub const SR25519_CRYPTO_ID: CryptoTypeId = CryptoTypeId("sr25519"); /// An Schnorrkel/Ristretto x25519 ("sr25519") public key. #[cfg_attr(feature = "full_crypto", derive(Hash))] @@ -384,7 +384,7 @@ impl Public { } impl TraitPublic for Public { - const KEY_KIND_ID: &'static str = SR25519_KIND_ID; + const CRYPTO_TYPE_ID: CryptoTypeId = SR25519_CRYPTO_ID; /// A new instance from the given slice that should be 32 bytes long. /// diff --git a/primitives/core/src/testing.rs b/primitives/core/src/testing.rs index 822159633adb1..7178293567f9e 100644 --- a/primitives/core/src/testing.rs +++ b/primitives/core/src/testing.rs @@ -16,7 +16,7 @@ //! Types that should only be used for testing! -use crate::crypto::{KeyKindId, KeyTypeId}; +use crate::crypto::{CryptoTypeId, KeyTypeId}; #[cfg(feature = "std")] use crate::{ crypto::{Pair, Public}, @@ -135,9 +135,9 @@ impl crate::traits::BareCryptoStore for KeyStore { public_keys.iter().all(|(k, t)| self.keys.get(&t).and_then(|s| s.get(k)).is_some()) } - fn sign_with(&self, kind: KeyKindId, id: KeyTypeId, msg: &[u8]) -> Result, String> { + fn sign_with(&self, kind: CryptoTypeId, id: KeyTypeId, msg: &[u8]) -> Result, String> { match kind { - ed25519::ED25519_KIND_ID => { + ed25519::ED25519_CRYPTO_ID => { let ed_public_key = self .ed25519_public_keys(id) .pop() @@ -148,7 +148,7 @@ impl crate::traits::BareCryptoStore for KeyStore { .ok_or(String::from("ed25519 pair not found"))?; return Ok(<[u8; 64]>::from(key_pair.sign(msg)).to_vec()); } - sr25519::SR25519_KIND_ID => { + sr25519::SR25519_CRYPTO_ID => { let sr_public_key = self .sr25519_public_keys(id) .pop() diff --git a/primitives/core/src/traits.rs b/primitives/core/src/traits.rs index 0018d30475677..fef1d396399c4 100644 --- a/primitives/core/src/traits.rs +++ b/primitives/core/src/traits.rs @@ -17,7 +17,7 @@ //! Shareable Substrate traits. use crate::{ - crypto::{KeyKindId, KeyTypeId}, + crypto::{CryptoTypeId, KeyTypeId}, ed25519, sr25519, }; @@ -82,7 +82,7 @@ pub trait BareCryptoStore: Send + Sync { /// the public key passed. fn sign_with( &self, - kind: KeyKindId, + kind: CryptoTypeId, id: KeyTypeId, msg: &[u8], ) -> std::result::Result, String>; diff --git a/primitives/io/src/lib.rs b/primitives/io/src/lib.rs index defa43de72efb..6bd5f8ee407f7 100644 --- a/primitives/io/src/lib.rs +++ b/primitives/io/src/lib.rs @@ -410,7 +410,7 @@ pub trait Crypto { self.extension::() .expect("No `keystore` associated for the current context!") .read() - .sign_with(ed25519::ED25519_KIND_ID, id, msg) + .sign_with(ed25519::ED25519_CRYPTO_ID, id, msg) .map(|sig| ed25519::Signature::from_slice(sig.as_slice())) .ok() } @@ -463,7 +463,7 @@ pub trait Crypto { self.extension::() .expect("No `keystore` associated for the current context!") .read() - .sign_with(sr25519::SR25519_KIND_ID, id, msg) + .sign_with(sr25519::SR25519_CRYPTO_ID, id, msg) .map(|sig| sr25519::Signature::from_slice(sig.as_slice())) .ok() } From 42f426d8ceab22ca15c2947e8f3a2f1c244e71e7 Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Wed, 19 Feb 2020 14:08:32 +0100 Subject: [PATCH 06/68] Remove pair-returning functions from BareCryptoStore trait --- client/keystore/src/lib.rs | 8 -------- primitives/core/src/testing.rs | 34 ++++++++++++++++++---------------- primitives/core/src/traits.rs | 6 ------ 3 files changed, 18 insertions(+), 30 deletions(-) diff --git a/client/keystore/src/lib.rs b/client/keystore/src/lib.rs index e764aff7168f5..d3c5679d607ce 100644 --- a/client/keystore/src/lib.rs +++ b/client/keystore/src/lib.rs @@ -343,10 +343,6 @@ impl BareCryptoStore for Store { Ok(pair.public()) } - fn sr25519_key_pair(&self, id: KeyTypeId, pub_key: &sr25519::Public) -> Option { - self.key_pair_by_type::(pub_key, id).ok() - } - fn ed25519_public_keys(&self, key_type: KeyTypeId) -> Vec { self.public_keys_by_type::(key_type).unwrap_or_default() } @@ -364,10 +360,6 @@ impl BareCryptoStore for Store { Ok(pair.public()) } - fn ed25519_key_pair(&self, id: KeyTypeId, pub_key: &ed25519::Public) -> Option { - self.key_pair_by_type::(pub_key, id).ok() - } - fn insert_unknown(&mut self, key_type: KeyTypeId, suri: &str, public: &[u8]) -> std::result::Result<(), ()> { diff --git a/primitives/core/src/testing.rs b/primitives/core/src/testing.rs index 7178293567f9e..77502b7504324 100644 --- a/primitives/core/src/testing.rs +++ b/primitives/core/src/testing.rs @@ -42,6 +42,24 @@ impl KeyStore { pub fn new() -> crate::traits::BareCryptoStorePtr { std::sync::Arc::new(parking_lot::RwLock::new(Self::default())) } + + fn sr25519_key_pair(&self, id: KeyTypeId, pub_key: &sr25519::Public) -> Option { + self.keys.get(&id) + .and_then(|inner| + inner.get(pub_key.as_slice()) + .map(|s| sr25519::Pair::from_string(s, None).expect("`sr25519` seed slice is valid")) + ) + } + + + fn ed25519_key_pair(&self, id: KeyTypeId, pub_key: &ed25519::Public) -> Option { + self.keys.get(&id) + .and_then(|inner| + inner.get(pub_key.as_slice()) + .map(|s| ed25519::Pair::from_string(s, None).expect("`ed25519` seed slice is valid")) + ) + } + } #[cfg(feature = "std")] @@ -76,14 +94,6 @@ impl crate::traits::BareCryptoStore for KeyStore { } } - fn sr25519_key_pair(&self, id: KeyTypeId, pub_key: &sr25519::Public) -> Option { - self.keys.get(&id) - .and_then(|inner| - inner.get(pub_key.as_slice()) - .map(|s| sr25519::Pair::from_string(s, None).expect("`sr25519` seed slice is valid")) - ) - } - fn ed25519_public_keys(&self, id: KeyTypeId) -> Vec { self.keys.get(&id) .map(|keys| @@ -114,14 +124,6 @@ impl crate::traits::BareCryptoStore for KeyStore { } } - fn ed25519_key_pair(&self, id: KeyTypeId, pub_key: &ed25519::Public) -> Option { - self.keys.get(&id) - .and_then(|inner| - inner.get(pub_key.as_slice()) - .map(|s| ed25519::Pair::from_string(s, None).expect("`ed25519` seed slice is valid")) - ) - } - fn insert_unknown(&mut self, id: KeyTypeId, suri: &str, public: &[u8]) -> Result<(), ()> { self.keys.entry(id).or_default().insert(public.to_owned(), suri.to_string()); Ok(()) diff --git a/primitives/core/src/traits.rs b/primitives/core/src/traits.rs index fef1d396399c4..5b73c800fbbb5 100644 --- a/primitives/core/src/traits.rs +++ b/primitives/core/src/traits.rs @@ -43,9 +43,6 @@ pub trait BareCryptoStore: Send + Sync { id: KeyTypeId, seed: Option<&str>, ) -> Result; - /// Returns the sr25519 key pair for the given key type and public key combination. - fn sr25519_key_pair(&self, id: KeyTypeId, pub_key: &sr25519::Public) -> Option; - /// Returns all ed25519 public keys for the given key type. fn ed25519_public_keys(&self, id: KeyTypeId) -> Vec; /// Generate a new ed25519 key pair for the given key type and an optional seed. @@ -59,9 +56,6 @@ pub trait BareCryptoStore: Send + Sync { seed: Option<&str>, ) -> Result; - /// Returns the ed25519 key pair for the given key type and public key combination. - fn ed25519_key_pair(&self, id: KeyTypeId, pub_key: &ed25519::Public) -> Option; - /// Insert a new key. This doesn't require any known of the crypto; but a public key must be /// manually provided. /// From 35555ff51b8fba8019a1a33b044249dbc990becd Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Wed, 19 Feb 2020 14:09:05 +0100 Subject: [PATCH 07/68] Define CryptoTypeId in app-crypto macros --- primitives/application-crypto/src/lib.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/primitives/application-crypto/src/lib.rs b/primitives/application-crypto/src/lib.rs index b7c9ccaa9821a..42b67033a6594 100644 --- a/primitives/application-crypto/src/lib.rs +++ b/primitives/application-crypto/src/lib.rs @@ -25,7 +25,7 @@ pub use sp_core::{self, crypto::{CryptoType, Public, Derive, IsWrappedBy, Wraps} #[doc(hidden)] #[cfg(feature = "full_crypto")] pub use sp_core::crypto::{SecretStringError, DeriveJunction, Ss58Codec, Pair}; -pub use sp_core::{crypto::{KeyTypeId, key_types}}; +pub use sp_core::crypto::{CryptoTypeId, KeyTypeId, key_types}; #[doc(hidden)] pub use codec; @@ -271,6 +271,8 @@ macro_rules! app_crypto_public_common { } impl $crate::Public for Public { + const CRYPTO_TYPE_ID: $crate::CryptoTypeId = $crate::CryptoTypeId(stringify!($key_type)); + fn from_slice(x: &[u8]) -> Self { Self(<$public>::from_slice(x)) } } From d7623c70c90fc0175ae55d53ffed88c764c59ebd Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Wed, 19 Feb 2020 14:11:52 +0100 Subject: [PATCH 08/68] Add functions to get keys supported by keystore --- client/keystore/src/lib.rs | 39 +++++++++++++++++++++++++++++++++- primitives/core/src/crypto.rs | 5 ++++- primitives/core/src/testing.rs | 34 ++++++++++++++++++++++++++++- primitives/core/src/traits.rs | 8 +++++-- 4 files changed, 81 insertions(+), 5 deletions(-) diff --git a/client/keystore/src/lib.rs b/client/keystore/src/lib.rs index d3c5679d607ce..0a0c4c0f22ea7 100644 --- a/client/keystore/src/lib.rs +++ b/client/keystore/src/lib.rs @@ -21,7 +21,7 @@ use std::{collections::HashMap, path::PathBuf, fs::{self, File}, io::{self, Write}, sync::Arc}; use sp_core::{ - crypto::{IsWrappedBy, CryptoTypeId, KeyTypeId, Pair as PairT, Protected, Public}, + crypto::{IsWrappedBy, CryptoTypeId, CryptoTypePublicPair, KeyTypeId, Pair as PairT, Protected, Public}, traits::BareCryptoStore, }; @@ -316,6 +316,43 @@ impl Store { } impl BareCryptoStore for Store { + fn get_supported_keys(&self, id: KeyTypeId, keys: Vec) -> std::result::Result, String> { + let ed25519_existing_keys: Vec> = self.public_keys_by_type::(id) + .map(|keys| keys.iter().map(|k| k.to_raw_vec()).collect()) + .map_err(|e| e.to_string())?; + let sr25519_existing_keys: Vec> = self.public_keys_by_type::(id) + .map(|keys| keys.iter().map(|k| k.to_raw_vec()).collect()) + .map_err(|e| e.to_string())?; + + Ok(keys.iter().filter_map(|k| { + match k.0 { + sr25519::SR25519_CRYPTO_ID if sr25519_existing_keys.contains(&k.1.to_vec()) => Some(k), + ed25519::ED25519_CRYPTO_ID if ed25519_existing_keys.contains(&k.1.to_vec()) => Some(k), + _ => None + } + }).cloned().collect::>()) + } + + fn get_keys(&self, id: KeyTypeId) -> std::result::Result, String> { + let ed25519_existing_keys: Vec> = self.public_keys_by_type::(id) + .map(|keys| keys.iter().map(|k| k.to_raw_vec()).collect()) + .map_err(|e| e.to_string())?; + let sr25519_existing_keys: Vec> = self.public_keys_by_type::(id) + .map(|keys| keys.iter().map(|k| k.to_raw_vec()).collect()) + .map_err(|e| e.to_string())?; + + let mut keys: Vec = vec![]; + keys.extend(sr25519_existing_keys.iter() + .cloned() + .map(|k| (sr25519::SR25519_CRYPTO_ID, k))); + keys.extend(ed25519_existing_keys.iter() + .cloned() + .map(|k| (ed25519::ED25519_CRYPTO_ID, k)) + .collect::>()); + Ok(keys) + + } + fn sign_with( &self, kind: CryptoTypeId, diff --git a/primitives/core/src/crypto.rs b/primitives/core/src/crypto.rs index b1a288da2925e..4790e410b7b31 100644 --- a/primitives/core/src/crypto.rs +++ b/primitives/core/src/crypto.rs @@ -926,9 +926,12 @@ impl<'a> TryFrom<&'a str> for KeyTypeId { } /// An identifier for a specific cryptographic algorithm used by a key pair -#[derive(Copy, Clone, Default, PartialEq, Eq, PartialOrd, Ord, Hash, Encode, Decode)] +#[derive(Copy, Clone, Default, PartialEq, Eq, PartialOrd, Ord, Hash)] pub struct CryptoTypeId(pub &'static str); +/// A type alias of CryptoTypeId & a public key +pub type CryptoTypePublicPair = (CryptoTypeId, Vec); + /// Known key types; this also functions as a global registry of key types for projects wishing to /// avoid collisions with each other. /// diff --git a/primitives/core/src/testing.rs b/primitives/core/src/testing.rs index 77502b7504324..1538a5c5dd991 100644 --- a/primitives/core/src/testing.rs +++ b/primitives/core/src/testing.rs @@ -16,7 +16,7 @@ //! Types that should only be used for testing! -use crate::crypto::{CryptoTypeId, KeyTypeId}; +use crate::crypto::{CryptoTypeId, KeyTypeId, CryptoTypePublicPair}; #[cfg(feature = "std")] use crate::{ crypto::{Pair, Public}, @@ -137,6 +137,38 @@ impl crate::traits::BareCryptoStore for KeyStore { public_keys.iter().all(|(k, t)| self.keys.get(&t).and_then(|s| s.get(k)).is_some()) } + fn get_supported_keys(&self, id: KeyTypeId, keys: Vec) -> Result, String> { + let ed25519_existing_keys: Vec> = self.ed25519_public_keys(id).iter() + .map(|k| k.to_raw_vec()).collect(); + let sr25519_existing_keys: Vec> = self.sr25519_public_keys(id).iter() + .map(|k| k.to_raw_vec()).collect(); + + Ok(keys.iter().filter_map(|k| { + match k.0 { + sr25519::SR25519_CRYPTO_ID if sr25519_existing_keys.contains(&k.1.to_vec()) => Some(k), + ed25519::ED25519_CRYPTO_ID if ed25519_existing_keys.contains(&k.1.to_vec()) => Some(k), + _ => None + } + }).cloned().collect::>()) + } + + fn get_keys(&self, id: KeyTypeId) -> Result, String> { + let ed25519_existing_keys: Vec> = self.ed25519_public_keys(id).iter() + .map(|k| k.to_raw_vec()).collect(); + let sr25519_existing_keys: Vec> = self.sr25519_public_keys(id).iter() + .map(|k| k.to_raw_vec()).collect(); + + let mut keys: Vec = vec![]; + keys.extend(sr25519_existing_keys.iter() + .cloned() + .map(|k| (sr25519::SR25519_CRYPTO_ID, k))); + keys.extend(ed25519_existing_keys.iter() + .cloned() + .map(|k| (ed25519::ED25519_CRYPTO_ID, k)) + .collect::>()); + Ok(keys) + } + fn sign_with(&self, kind: CryptoTypeId, id: KeyTypeId, msg: &[u8]) -> Result, String> { match kind { ed25519::ED25519_CRYPTO_ID => { diff --git a/primitives/core/src/traits.rs b/primitives/core/src/traits.rs index 5b73c800fbbb5..cb80962f96f0a 100644 --- a/primitives/core/src/traits.rs +++ b/primitives/core/src/traits.rs @@ -17,7 +17,7 @@ //! Shareable Substrate traits. use crate::{ - crypto::{CryptoTypeId, KeyTypeId}, + crypto::{CryptoTypeId, KeyTypeId, CryptoTypePublicPair}, ed25519, sr25519, }; @@ -66,7 +66,11 @@ pub trait BareCryptoStore: Send + Sync { /// Get the password for this store. fn password(&self) -> Option<&str>; - + /// Provided a list of (CryptoTypeId,[u8]) pairs, this would return + /// a filtered list of public keys which are supported by the keystore. + fn get_supported_keys(&self, id: KeyTypeId, keys: Vec) -> Result, String>; + /// Get a list of public keys the signer supports. + fn get_keys(&self, id: KeyTypeId) -> Result, String>; /// Checks if the private keys for the given public key and key type combinations exist. /// /// Returns `true` iff all private keys could be found. From 01af73ed092191d87eae979bfe283d240fc55702 Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Wed, 19 Feb 2020 16:27:16 +0100 Subject: [PATCH 09/68] Fix sign_with signature to include CryptoTypePublicPair --- client/keystore/src/lib.rs | 25 +++++++++---------------- primitives/core/src/testing.rs | 23 ++++++++++------------- primitives/core/src/traits.rs | 4 ++-- primitives/io/src/lib.rs | 6 +++--- 4 files changed, 24 insertions(+), 34 deletions(-) diff --git a/client/keystore/src/lib.rs b/client/keystore/src/lib.rs index 0a0c4c0f22ea7..d2f5235cb18a4 100644 --- a/client/keystore/src/lib.rs +++ b/client/keystore/src/lib.rs @@ -21,7 +21,7 @@ use std::{collections::HashMap, path::PathBuf, fs::{self, File}, io::{self, Write}, sync::Arc}; use sp_core::{ - crypto::{IsWrappedBy, CryptoTypeId, CryptoTypePublicPair, KeyTypeId, Pair as PairT, Protected, Public}, + crypto::{IsWrappedBy, CryptoTypePublicPair, KeyTypeId, Pair as PairT, Protected, Public}, traits::BareCryptoStore, }; @@ -282,30 +282,23 @@ impl Store { /// the public key passed. fn _sign_with( &self, - kind: CryptoTypeId, id: KeyTypeId, + key: &CryptoTypePublicPair, msg: &[u8], ) -> std::result::Result, String> { - match kind { + match key.0 { ed25519::ED25519_CRYPTO_ID => { - let public_key = self - .ed25519_public_keys(id) - .pop() - .ok_or(String::from("ed25519 key not found"))?; - + let pub_key = ed25519::Public::from_slice(key.1.as_slice()); let key_pair: ed25519::Pair = self - .key_pair_by_type::(&public_key, id) + .key_pair_by_type::(&pub_key, id) .map(Into::into) .map_err(|e| e.to_string())?; return Ok(<[u8; 64]>::from(key_pair.sign(msg)).to_vec()); } sr25519::SR25519_CRYPTO_ID => { - let public_key = self - .sr25519_public_keys(id) - .pop() - .ok_or(String::from("sr25519 key not found"))?; + let pub_key = sr25519::Public::from_slice(key.1.as_slice()); let key_pair: sr25519::Pair = self - .key_pair_by_type::(&public_key, id) + .key_pair_by_type::(&pub_key, id) .map(Into::into) .map_err(|e| e.to_string())?; return Ok(<[u8; 64]>::from(key_pair.sign(msg)).to_vec()); @@ -355,11 +348,11 @@ impl BareCryptoStore for Store { fn sign_with( &self, - kind: CryptoTypeId, id: KeyTypeId, + key: &CryptoTypePublicPair, msg: &[u8], ) -> std::result::Result, String> { - self._sign_with(kind, id, msg) + self._sign_with(id, key, msg) } fn sr25519_public_keys(&self, key_type: KeyTypeId) -> Vec { diff --git a/primitives/core/src/testing.rs b/primitives/core/src/testing.rs index 1538a5c5dd991..63d02b0d27456 100644 --- a/primitives/core/src/testing.rs +++ b/primitives/core/src/testing.rs @@ -16,7 +16,7 @@ //! Types that should only be used for testing! -use crate::crypto::{CryptoTypeId, KeyTypeId, CryptoTypePublicPair}; +use crate::crypto::{KeyTypeId, CryptoTypePublicPair}; #[cfg(feature = "std")] use crate::{ crypto::{Pair, Public}, @@ -169,26 +169,23 @@ impl crate::traits::BareCryptoStore for KeyStore { Ok(keys) } - fn sign_with(&self, kind: CryptoTypeId, id: KeyTypeId, msg: &[u8]) -> Result, String> { - match kind { + fn sign_with( + &self, + id: KeyTypeId, + key: &CryptoTypePublicPair, + msg: &[u8] + ) -> Result, String> { + match key.0 { ed25519::ED25519_CRYPTO_ID => { - let ed_public_key = self - .ed25519_public_keys(id) - .pop() - .ok_or(String::from("ed25519 key not found"))?; let key_pair: ed25519::Pair = self - .ed25519_key_pair(id, &ed_public_key) + .ed25519_key_pair(id, &ed25519::Public::from_slice(key.1.as_slice())) .map(Into::into) .ok_or(String::from("ed25519 pair not found"))?; return Ok(<[u8; 64]>::from(key_pair.sign(msg)).to_vec()); } sr25519::SR25519_CRYPTO_ID => { - let sr_public_key = self - .sr25519_public_keys(id) - .pop() - .ok_or(String::from("sr25519 key not found"))?; let key_pair: sr25519::Pair = self - .sr25519_key_pair(id, &sr_public_key) + .sr25519_key_pair(id, &sr25519::Public::from_slice(key.1.as_slice())) .map(Into::into) .ok_or(String::from("sr25519 pair not found"))?; return Ok(<[u8; 64]>::from(key_pair.sign(msg)).to_vec()); diff --git a/primitives/core/src/traits.rs b/primitives/core/src/traits.rs index cb80962f96f0a..4d336a6e3051e 100644 --- a/primitives/core/src/traits.rs +++ b/primitives/core/src/traits.rs @@ -17,7 +17,7 @@ //! Shareable Substrate traits. use crate::{ - crypto::{CryptoTypeId, KeyTypeId, CryptoTypePublicPair}, + crypto::{KeyTypeId, CryptoTypePublicPair}, ed25519, sr25519, }; @@ -80,8 +80,8 @@ pub trait BareCryptoStore: Send + Sync { /// the public key passed. fn sign_with( &self, - kind: CryptoTypeId, id: KeyTypeId, + key: &CryptoTypePublicPair, msg: &[u8], ) -> std::result::Result, String>; } diff --git a/primitives/io/src/lib.rs b/primitives/io/src/lib.rs index 6bd5f8ee407f7..3f161f1071c74 100644 --- a/primitives/io/src/lib.rs +++ b/primitives/io/src/lib.rs @@ -33,7 +33,7 @@ use sp_std::ops::Deref; #[cfg(feature = "std")] use sp_core::{ - crypto::Pair, + crypto::{Pair, Public}, traits::{KeystoreExt, CallInWasmExt}, offchain::{OffchainExt, TransactionPoolExt}, hexdisplay::HexDisplay, @@ -410,7 +410,7 @@ pub trait Crypto { self.extension::() .expect("No `keystore` associated for the current context!") .read() - .sign_with(ed25519::ED25519_CRYPTO_ID, id, msg) + .sign_with(id, &(ed25519::ED25519_CRYPTO_ID, pub_key.to_raw_vec()), msg) .map(|sig| ed25519::Signature::from_slice(sig.as_slice())) .ok() } @@ -463,7 +463,7 @@ pub trait Crypto { self.extension::() .expect("No `keystore` associated for the current context!") .read() - .sign_with(sr25519::SR25519_CRYPTO_ID, id, msg) + .sign_with(id, &(sr25519::SR25519_CRYPTO_ID, pub_key.to_raw_vec()), msg) .map(|sig| sr25519::Signature::from_slice(sig.as_slice())) .ok() } From 97775db164416a34fad52722043f654b00d3386c Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Wed, 19 Feb 2020 16:28:13 +0100 Subject: [PATCH 10/68] Add `sign_with_any` and `sign_with_all` --- primitives/core/src/traits.rs | 36 +++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/primitives/core/src/traits.rs b/primitives/core/src/traits.rs index 4d336a6e3051e..a663ba8a7ace1 100644 --- a/primitives/core/src/traits.rs +++ b/primitives/core/src/traits.rs @@ -84,6 +84,42 @@ pub trait BareCryptoStore: Send + Sync { key: &CryptoTypePublicPair, msg: &[u8], ) -> std::result::Result, String>; + + /// Given a list of public keys, find the first supported key and + /// sign the provided message with that key. + /// + /// Returns a tuple of the used key and the signature + fn sign_with_any( + &self, + id: KeyTypeId, + keys: Vec, + msg: &[u8] + ) -> Result<(CryptoTypePublicPair, Vec), String> { + if keys.len() == 1 { + return self.sign_with(id, &keys[0], msg).map(|s| (keys[0].clone(), s)); + } else { + for k in self.get_supported_keys(id, keys)? { + if let Ok(sign) = self.sign_with(id, &k, msg) { + return Ok((k, sign)); + } + } + } + Err("Could not sign with any of the given keys".to_owned()) + } + + /// Provided a list of public keys, sign a message with + /// each key given that the key is supported. + /// + /// Returns a list of `Option`s each representing the signature of each key. + /// None is return for non-supported keys. + fn sign_with_all( + &self, + id: KeyTypeId, + keys: Vec, + msg: &[u8] + ) -> Vec>> { + keys.iter().map(|k| self.sign_with(id, k, msg).ok()).collect() + } } /// A pointer to the key store. From 1ee13ac8da4c06959996aeabe5c706c337079931 Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Wed, 19 Feb 2020 17:09:25 +0100 Subject: [PATCH 11/68] Use keystore.sign_with in auth_discovery --- client/authority-discovery/src/error.rs | 2 ++ client/authority-discovery/src/lib.rs | 37 ++++++++++++------------- 2 files changed, 20 insertions(+), 19 deletions(-) diff --git a/client/authority-discovery/src/error.rs b/client/authority-discovery/src/error.rs index d62281c0c2829..e5b974a12cac5 100644 --- a/client/authority-discovery/src/error.rs +++ b/client/authority-discovery/src/error.rs @@ -46,4 +46,6 @@ pub enum Error { EncodingDecodingScale(codec::Error), /// Failed to parse a libp2p multi address. ParsingMultiaddress(libp2p::core::multiaddr::Error), + /// Failed to map signature to public key + PublicKeyToSignatureMaping, } diff --git a/client/authority-discovery/src/lib.rs b/client/authority-discovery/src/lib.rs index 6260ac9a85b12..fae45add408b9 100644 --- a/client/authority-discovery/src/lib.rs +++ b/client/authority-discovery/src/lib.rs @@ -63,7 +63,8 @@ use sc_client_api::blockchain::HeaderBackend; use sc_network::specialization::NetworkSpecialization; use sc_network::{DhtEvent, ExHashT, NetworkStateInfo}; use sp_authority_discovery::{AuthorityDiscoveryApi, AuthorityId, AuthoritySignature, AuthorityPair}; -use sp_core::crypto::{key_types, Pair}; +use sp_core::sr25519; +use sp_core::crypto::{key_types, CryptoTypePublicPair, Pair, Public}; use sp_core::traits::BareCryptoStorePtr; use sp_runtime::{traits::Block as BlockT, generic::BlockId}; use sp_api::ProvideRuntimeApi; @@ -211,10 +212,23 @@ where .encode(&mut serialized_addresses) .map_err(Error::EncodingProto)?; - for key in self.get_priv_keys_within_authority_set()?.into_iter() { - let signature = key.sign(&serialized_addresses); + let keys: Vec = self.get_own_public_keys_within_authority_set()? + .into_iter() + .map(|k| (sr25519::SR25519_CRYPTO_ID, k.to_raw_vec())) + .collect(); + + let signatures = self.key_store + .read() + .sign_with_all( + key_types::AUTHORITY_DISCOVERY, + keys.clone(), + serialized_addresses.as_slice() + ); + for (index, signature) in signatures.iter().enumerate() { let mut signed_addresses = vec![]; + let key = keys.get(index).ok_or(Error::PublicKeyToSignatureMaping)?; + schema::SignedAuthorityAddresses { addresses: serialized_addresses.clone(), signature: signature.encode(), @@ -223,7 +237,7 @@ where .map_err(Error::EncodingProto)?; self.network.put_value( - hash_authority_id(key.public().as_ref())?, + hash_authority_id(key.1.as_ref())?, signed_addresses, ); } @@ -347,21 +361,6 @@ where Ok(()) } - /// Retrieve all local authority discovery private keys that are within the current authority - /// set. - fn get_priv_keys_within_authority_set(&mut self) -> Result> { - let keys = self.get_own_public_keys_within_authority_set()? - .into_iter() - .map(std::convert::Into::into) - .filter_map(|pub_key| { - self.key_store.read().sr25519_key_pair(key_types::AUTHORITY_DISCOVERY, &pub_key) - }) - .map(std::convert::Into::into) - .collect(); - - Ok(keys) - } - /// Retrieve our public keys within the current authority set. // // A node might have multiple authority discovery keys within its keystore, e.g. an old one and From ccec7f5e8ac3a12880de74dc126f0fcd889785a4 Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Wed, 19 Feb 2020 21:50:29 +0100 Subject: [PATCH 12/68] Rename get_supported_keys -> supported_keys --- client/keystore/src/lib.rs | 2 +- primitives/core/src/testing.rs | 2 +- primitives/core/src/traits.rs | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/client/keystore/src/lib.rs b/client/keystore/src/lib.rs index d2f5235cb18a4..b68ac126f7e83 100644 --- a/client/keystore/src/lib.rs +++ b/client/keystore/src/lib.rs @@ -309,7 +309,7 @@ impl Store { } impl BareCryptoStore for Store { - fn get_supported_keys(&self, id: KeyTypeId, keys: Vec) -> std::result::Result, String> { + fn supported_keys(&self, id: KeyTypeId, keys: Vec) -> std::result::Result, String> { let ed25519_existing_keys: Vec> = self.public_keys_by_type::(id) .map(|keys| keys.iter().map(|k| k.to_raw_vec()).collect()) .map_err(|e| e.to_string())?; diff --git a/primitives/core/src/testing.rs b/primitives/core/src/testing.rs index 63d02b0d27456..0955363491829 100644 --- a/primitives/core/src/testing.rs +++ b/primitives/core/src/testing.rs @@ -137,7 +137,7 @@ impl crate::traits::BareCryptoStore for KeyStore { public_keys.iter().all(|(k, t)| self.keys.get(&t).and_then(|s| s.get(k)).is_some()) } - fn get_supported_keys(&self, id: KeyTypeId, keys: Vec) -> Result, String> { + fn supported_keys(&self, id: KeyTypeId, keys: Vec) -> Result, String> { let ed25519_existing_keys: Vec> = self.ed25519_public_keys(id).iter() .map(|k| k.to_raw_vec()).collect(); let sr25519_existing_keys: Vec> = self.sr25519_public_keys(id).iter() diff --git a/primitives/core/src/traits.rs b/primitives/core/src/traits.rs index a663ba8a7ace1..b7dd5f0f01283 100644 --- a/primitives/core/src/traits.rs +++ b/primitives/core/src/traits.rs @@ -68,7 +68,7 @@ pub trait BareCryptoStore: Send + Sync { fn password(&self) -> Option<&str>; /// Provided a list of (CryptoTypeId,[u8]) pairs, this would return /// a filtered list of public keys which are supported by the keystore. - fn get_supported_keys(&self, id: KeyTypeId, keys: Vec) -> Result, String>; + fn supported_keys(&self, id: KeyTypeId, keys: Vec) -> Result, String>; /// Get a list of public keys the signer supports. fn get_keys(&self, id: KeyTypeId) -> Result, String>; /// Checks if the private keys for the given public key and key type combinations exist. @@ -98,7 +98,7 @@ pub trait BareCryptoStore: Send + Sync { if keys.len() == 1 { return self.sign_with(id, &keys[0], msg).map(|s| (keys[0].clone(), s)); } else { - for k in self.get_supported_keys(id, keys)? { + for k in self.supported_keys(id, keys)? { if let Ok(sign) = self.sign_with(id, &k, msg) { return Ok((k, sign)); } From 5f59041482a3eb3db2af9757a81906f790171ecb Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Wed, 19 Feb 2020 21:50:54 +0100 Subject: [PATCH 13/68] Added headers to function docstrings --- primitives/core/src/testing.rs | 2 +- primitives/core/src/traits.rs | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/primitives/core/src/testing.rs b/primitives/core/src/testing.rs index 0955363491829..4a6f66938f5bd 100644 --- a/primitives/core/src/testing.rs +++ b/primitives/core/src/testing.rs @@ -190,7 +190,7 @@ impl crate::traits::BareCryptoStore for KeyStore { .ok_or(String::from("sr25519 pair not found"))?; return Ok(<[u8; 64]>::from(key_pair.sign(msg)).to_vec()); } - _ => Err(String::from("Key kind invalid")), + _ => Err(String::from("Key kind invalid")) } } } diff --git a/primitives/core/src/traits.rs b/primitives/core/src/traits.rs index b7dd5f0f01283..91cf25d0d1673 100644 --- a/primitives/core/src/traits.rs +++ b/primitives/core/src/traits.rs @@ -66,9 +66,13 @@ pub trait BareCryptoStore: Send + Sync { /// Get the password for this store. fn password(&self) -> Option<&str>; + /// Find intersection between provided keys and supported keys + /// /// Provided a list of (CryptoTypeId,[u8]) pairs, this would return /// a filtered list of public keys which are supported by the keystore. fn supported_keys(&self, id: KeyTypeId, keys: Vec) -> Result, String>; + /// List all supported keys + /// /// Get a list of public keys the signer supports. fn get_keys(&self, id: KeyTypeId) -> Result, String>; /// Checks if the private keys for the given public key and key type combinations exist. @@ -76,6 +80,8 @@ pub trait BareCryptoStore: Send + Sync { /// Returns `true` iff all private keys could be found. fn has_keys(&self, public_keys: &[(Vec, KeyTypeId)]) -> bool; + /// Sign with key + /// /// Signs a message with the private key that matches /// the public key passed. fn sign_with( @@ -85,6 +91,8 @@ pub trait BareCryptoStore: Send + Sync { msg: &[u8], ) -> std::result::Result, String>; + /// Sign with any key + /// /// Given a list of public keys, find the first supported key and /// sign the provided message with that key. /// @@ -107,6 +115,8 @@ pub trait BareCryptoStore: Send + Sync { Err("Could not sign with any of the given keys".to_owned()) } + /// Sign with all keys + /// /// Provided a list of public keys, sign a message with /// each key given that the key is supported. /// From b588f7f85121ae5cc9c6b1d6670acf453109a9f0 Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Wed, 19 Feb 2020 22:20:31 +0100 Subject: [PATCH 14/68] Use chain instead of extending a temp vector --- client/keystore/src/lib.rs | 32 +++++++++++++++++--------------- primitives/core/src/testing.rs | 30 ++++++++++++++++-------------- 2 files changed, 33 insertions(+), 29 deletions(-) diff --git a/client/keystore/src/lib.rs b/client/keystore/src/lib.rs index b68ac126f7e83..4fdd282aa9cb0 100644 --- a/client/keystore/src/lib.rs +++ b/client/keystore/src/lib.rs @@ -327,23 +327,25 @@ impl BareCryptoStore for Store { } fn get_keys(&self, id: KeyTypeId) -> std::result::Result, String> { - let ed25519_existing_keys: Vec> = self.public_keys_by_type::(id) - .map(|keys| keys.iter().map(|k| k.to_raw_vec()).collect()) - .map_err(|e| e.to_string())?; - let sr25519_existing_keys: Vec> = self.public_keys_by_type::(id) - .map(|keys| keys.iter().map(|k| k.to_raw_vec()).collect()) - .map_err(|e| e.to_string())?; + let ed25519_existing_keys: Vec = self + .public_keys_by_type::(id) + .map(|keys| { + keys.iter().map(|k| (ed25519::ED25519_CRYPTO_ID, k.to_raw_vec())).collect() + }) + .map_err(|e| e.to_string())?; - let mut keys: Vec = vec![]; - keys.extend(sr25519_existing_keys.iter() - .cloned() - .map(|k| (sr25519::SR25519_CRYPTO_ID, k))); - keys.extend(ed25519_existing_keys.iter() - .cloned() - .map(|k| (ed25519::ED25519_CRYPTO_ID, k)) - .collect::>()); - Ok(keys) + let sr25519_existing_keys: Vec = self + .public_keys_by_type::(id) + .map(|keys| { + keys.iter().map(|k| (sr25519::SR25519_CRYPTO_ID, k.to_raw_vec())).collect() + }) + .map_err(|e| e.to_string())?; + Ok(ed25519_existing_keys + .iter() + .chain(sr25519_existing_keys.iter()) + .cloned() + .collect()) } fn sign_with( diff --git a/primitives/core/src/testing.rs b/primitives/core/src/testing.rs index 4a6f66938f5bd..8e58907cd0214 100644 --- a/primitives/core/src/testing.rs +++ b/primitives/core/src/testing.rs @@ -153,20 +153,22 @@ impl crate::traits::BareCryptoStore for KeyStore { } fn get_keys(&self, id: KeyTypeId) -> Result, String> { - let ed25519_existing_keys: Vec> = self.ed25519_public_keys(id).iter() - .map(|k| k.to_raw_vec()).collect(); - let sr25519_existing_keys: Vec> = self.sr25519_public_keys(id).iter() - .map(|k| k.to_raw_vec()).collect(); - - let mut keys: Vec = vec![]; - keys.extend(sr25519_existing_keys.iter() - .cloned() - .map(|k| (sr25519::SR25519_CRYPTO_ID, k))); - keys.extend(ed25519_existing_keys.iter() - .cloned() - .map(|k| (ed25519::ED25519_CRYPTO_ID, k)) - .collect::>()); - Ok(keys) + let ed25519_existing_keys: Vec = self + .ed25519_public_keys(id) + .iter() + .map(|k| (ed25519::ED25519_CRYPTO_ID, k.to_raw_vec())) + .collect(); + + let sr25519_existing_keys: Vec = self + .sr25519_public_keys(id) + .iter() + .map(|k| (sr25519::SR25519_CRYPTO_ID, k.to_raw_vec())) + .collect(); + + Ok(ed25519_existing_keys + .iter() + .chain(sr25519_existing_keys.iter()) + .cloned().collect()) } fn sign_with( From 94e45a9bd1f5fb2b31d283511a0a46f619635337 Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Wed, 19 Feb 2020 22:20:46 +0100 Subject: [PATCH 15/68] Fixed some code formatting --- client/keystore/src/lib.rs | 20 ++++++++++++++------ primitives/core/src/testing.rs | 2 +- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/client/keystore/src/lib.rs b/client/keystore/src/lib.rs index 4fdd282aa9cb0..ec1b7888b2ef3 100644 --- a/client/keystore/src/lib.rs +++ b/client/keystore/src/lib.rs @@ -303,19 +303,27 @@ impl Store { .map_err(|e| e.to_string())?; return Ok(<[u8; 64]>::from(key_pair.sign(msg)).to_vec()); } - _ => Err(String::from("Key kind invalid")) + _ => Err(String::from("Key crypto type is not supported")) } } } impl BareCryptoStore for Store { - fn supported_keys(&self, id: KeyTypeId, keys: Vec) -> std::result::Result, String> { + fn supported_keys( + &self, + id: KeyTypeId, + keys: Vec + ) -> std::result::Result, String> { let ed25519_existing_keys: Vec> = self.public_keys_by_type::(id) - .map(|keys| keys.iter().map(|k| k.to_raw_vec()).collect()) - .map_err(|e| e.to_string())?; + .map(|keys| { + keys.iter().map(|k| k.to_raw_vec()).collect() + }) + .map_err(|e| e.to_string())?; let sr25519_existing_keys: Vec> = self.public_keys_by_type::(id) - .map(|keys| keys.iter().map(|k| k.to_raw_vec()).collect()) - .map_err(|e| e.to_string())?; + .map(|keys| { + keys.iter().map(|k| k.to_raw_vec()).collect() + }) + .map_err(|e| e.to_string())?; Ok(keys.iter().filter_map(|k| { match k.0 { diff --git a/primitives/core/src/testing.rs b/primitives/core/src/testing.rs index 8e58907cd0214..8fbb380a31c04 100644 --- a/primitives/core/src/testing.rs +++ b/primitives/core/src/testing.rs @@ -192,7 +192,7 @@ impl crate::traits::BareCryptoStore for KeyStore { .ok_or(String::from("sr25519 pair not found"))?; return Ok(<[u8; 64]>::from(key_pair.sign(msg)).to_vec()); } - _ => Err(String::from("Key kind invalid")) + _ => Err(String::from("Key crypto type is not supported")) } } } From e19ecf6e8a9174a52143115184ad1a3e5aa350e5 Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Wed, 19 Feb 2020 22:33:44 +0100 Subject: [PATCH 16/68] Restrict size of CryptoTypeId This is to be able to use Encode/Decode derives and the overcome having the size being unknown at compile-time. --- primitives/core/src/crypto.rs | 6 +++--- primitives/core/src/ecdsa.rs | 2 +- primitives/core/src/ed25519.rs | 2 +- primitives/core/src/sr25519.rs | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/primitives/core/src/crypto.rs b/primitives/core/src/crypto.rs index 4790e410b7b31..c1326ac7b1c0d 100644 --- a/primitives/core/src/crypto.rs +++ b/primitives/core/src/crypto.rs @@ -662,7 +662,7 @@ mod dummy { impl Derive for Dummy {} impl Public for Dummy { - const CRYPTO_TYPE_ID: CryptoTypeId = CryptoTypeId("dummy"); + const CRYPTO_TYPE_ID: CryptoTypeId = CryptoTypeId(*b"dumm"); fn from_slice(_: &[u8]) -> Self { Self } #[cfg(feature = "std")] @@ -926,8 +926,8 @@ impl<'a> TryFrom<&'a str> for KeyTypeId { } /// An identifier for a specific cryptographic algorithm used by a key pair -#[derive(Copy, Clone, Default, PartialEq, Eq, PartialOrd, Ord, Hash)] -pub struct CryptoTypeId(pub &'static str); +#[derive(Copy, Clone, Default, PartialEq, Eq, PartialOrd, Ord, Hash, Encode, Decode)] +pub struct CryptoTypeId(pub [u8; 4]); /// A type alias of CryptoTypeId & a public key pub type CryptoTypePublicPair = (CryptoTypeId, Vec); diff --git a/primitives/core/src/ecdsa.rs b/primitives/core/src/ecdsa.rs index 28e4ee5f78c1b..e3cb6f79314b0 100644 --- a/primitives/core/src/ecdsa.rs +++ b/primitives/core/src/ecdsa.rs @@ -43,7 +43,7 @@ use secp256k1::{PublicKey, SecretKey}; /// A value which is passed along with the public key /// to define which cryptographic algorithm that key /// belongs to. -pub const ECDSA_CRYPTO_ID: CryptoTypeId = CryptoTypeId("ecdsa"); +pub const ECDSA_CRYPTO_ID: CryptoTypeId = CryptoTypeId(*b"ecds"); /// A secret seed (which is bytewise essentially equivalent to a SecretKey). /// diff --git a/primitives/core/src/ed25519.rs b/primitives/core/src/ed25519.rs index f88341a7229c2..cf4332880568b 100644 --- a/primitives/core/src/ed25519.rs +++ b/primitives/core/src/ed25519.rs @@ -45,7 +45,7 @@ use sp_std::ops::Deref; /// A value which is passed along with the public key /// to define which cryptographic algorithm that key /// belongs to. -pub const ED25519_CRYPTO_ID: CryptoTypeId = CryptoTypeId("ed25519"); +pub const ED25519_CRYPTO_ID: CryptoTypeId = CryptoTypeId(*b"ed25"); /// A secret seed. It's not called a "secret key" because ring doesn't expose the secret keys /// of the key pair (yeah, dumb); as such we're forced to remember the seed manually if we diff --git a/primitives/core/src/sr25519.rs b/primitives/core/src/sr25519.rs index 408d16b210e3b..a448516f2f8df 100644 --- a/primitives/core/src/sr25519.rs +++ b/primitives/core/src/sr25519.rs @@ -57,7 +57,7 @@ const SIGNING_CTX: &[u8] = b"substrate"; /// A value which is passed along with the public key /// to define which cryptographic algorithm that key /// belongs to. -pub const SR25519_CRYPTO_ID: CryptoTypeId = CryptoTypeId("sr25519"); +pub const SR25519_CRYPTO_ID: CryptoTypeId = CryptoTypeId(*b"sr25"); /// An Schnorrkel/Ristretto x25519 ("sr25519") public key. #[cfg_attr(feature = "full_crypto", derive(Hash))] From 12836f46273a67c3420240670ef14ef5e284d98b Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Wed, 19 Feb 2020 22:36:26 +0100 Subject: [PATCH 17/68] Implement sign_with in the trait itself --- client/keystore/src/lib.rs | 49 +++++++++++++++----------------------- 1 file changed, 19 insertions(+), 30 deletions(-) diff --git a/client/keystore/src/lib.rs b/client/keystore/src/lib.rs index ec1b7888b2ef3..a7984f0dce2e1 100644 --- a/client/keystore/src/lib.rs +++ b/client/keystore/src/lib.rs @@ -277,35 +277,6 @@ impl Store { buf.push(key_type + key.as_str()); Some(buf) } - - /// Signs a message with the private key that matches - /// the public key passed. - fn _sign_with( - &self, - id: KeyTypeId, - key: &CryptoTypePublicPair, - msg: &[u8], - ) -> std::result::Result, String> { - match key.0 { - ed25519::ED25519_CRYPTO_ID => { - let pub_key = ed25519::Public::from_slice(key.1.as_slice()); - let key_pair: ed25519::Pair = self - .key_pair_by_type::(&pub_key, id) - .map(Into::into) - .map_err(|e| e.to_string())?; - return Ok(<[u8; 64]>::from(key_pair.sign(msg)).to_vec()); - } - sr25519::SR25519_CRYPTO_ID => { - let pub_key = sr25519::Public::from_slice(key.1.as_slice()); - let key_pair: sr25519::Pair = self - .key_pair_by_type::(&pub_key, id) - .map(Into::into) - .map_err(|e| e.to_string())?; - return Ok(<[u8; 64]>::from(key_pair.sign(msg)).to_vec()); - } - _ => Err(String::from("Key crypto type is not supported")) - } - } } impl BareCryptoStore for Store { @@ -362,7 +333,25 @@ impl BareCryptoStore for Store { key: &CryptoTypePublicPair, msg: &[u8], ) -> std::result::Result, String> { - self._sign_with(id, key, msg) + match key.0 { + ed25519::ED25519_CRYPTO_ID => { + let pub_key = ed25519::Public::from_slice(key.1.as_slice()); + let key_pair: ed25519::Pair = self + .key_pair_by_type::(&pub_key, id) + .map(Into::into) + .map_err(|e| e.to_string())?; + return Ok(<[u8; 64]>::from(key_pair.sign(msg)).to_vec()); + } + sr25519::SR25519_CRYPTO_ID => { + let pub_key = sr25519::Public::from_slice(key.1.as_slice()); + let key_pair: sr25519::Pair = self + .key_pair_by_type::(&pub_key, id) + .map(Into::into) + .map_err(|e| e.to_string())?; + return Ok(<[u8; 64]>::from(key_pair.sign(msg)).to_vec()); + } + _ => Err(String::from("Key crypto type is not supported")) + } } fn sr25519_public_keys(&self, key_type: KeyTypeId) -> Vec { From d6082cc473454901cb145e3453443de16627f750 Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Wed, 19 Feb 2020 22:48:38 +0100 Subject: [PATCH 18/68] Remove whitespace --- client/authority-discovery/src/lib.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/client/authority-discovery/src/lib.rs b/client/authority-discovery/src/lib.rs index fae45add408b9..c915c722bbfce 100644 --- a/client/authority-discovery/src/lib.rs +++ b/client/authority-discovery/src/lib.rs @@ -218,12 +218,12 @@ where .collect(); let signatures = self.key_store - .read() - .sign_with_all( - key_types::AUTHORITY_DISCOVERY, - keys.clone(), - serialized_addresses.as_slice() - ); + .read() + .sign_with_all( + key_types::AUTHORITY_DISCOVERY, + keys.clone(), + serialized_addresses.as_slice() + ); for (index, signature) in signatures.iter().enumerate() { let mut signed_addresses = vec![]; From ef029ebad7d036bdfd28341c31940714b2296d2d Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Wed, 19 Feb 2020 22:56:41 +0100 Subject: [PATCH 19/68] Use key_type also as a CryptoTypeId in app_crypto macros --- primitives/application-crypto/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/primitives/application-crypto/src/lib.rs b/primitives/application-crypto/src/lib.rs index 42b67033a6594..1855f8da74d5b 100644 --- a/primitives/application-crypto/src/lib.rs +++ b/primitives/application-crypto/src/lib.rs @@ -271,7 +271,7 @@ macro_rules! app_crypto_public_common { } impl $crate::Public for Public { - const CRYPTO_TYPE_ID: $crate::CryptoTypeId = $crate::CryptoTypeId(stringify!($key_type)); + const CRYPTO_TYPE_ID: $crate::CryptoTypeId = $crate::CryptoTypeId($key_type.0); fn from_slice(x: &[u8]) -> Self { Self(<$public>::from_slice(x)) } } From c9680e363755024494fba698b93cf845837f64b0 Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Thu, 20 Feb 2020 09:45:34 +0100 Subject: [PATCH 20/68] Rename `get_keys` to `keys` in BareCryptoStore --- client/keystore/src/lib.rs | 2 +- primitives/core/src/testing.rs | 2 +- primitives/core/src/traits.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/client/keystore/src/lib.rs b/client/keystore/src/lib.rs index a7984f0dce2e1..aaf949378c2ad 100644 --- a/client/keystore/src/lib.rs +++ b/client/keystore/src/lib.rs @@ -305,7 +305,7 @@ impl BareCryptoStore for Store { }).cloned().collect::>()) } - fn get_keys(&self, id: KeyTypeId) -> std::result::Result, String> { + fn keys(&self, id: KeyTypeId) -> std::result::Result, String> { let ed25519_existing_keys: Vec = self .public_keys_by_type::(id) .map(|keys| { diff --git a/primitives/core/src/testing.rs b/primitives/core/src/testing.rs index 8fbb380a31c04..9f93c8c8daa1d 100644 --- a/primitives/core/src/testing.rs +++ b/primitives/core/src/testing.rs @@ -152,7 +152,7 @@ impl crate::traits::BareCryptoStore for KeyStore { }).cloned().collect::>()) } - fn get_keys(&self, id: KeyTypeId) -> Result, String> { + fn keys(&self, id: KeyTypeId) -> Result, String> { let ed25519_existing_keys: Vec = self .ed25519_public_keys(id) .iter() diff --git a/primitives/core/src/traits.rs b/primitives/core/src/traits.rs index 91cf25d0d1673..e223a46f6fcd2 100644 --- a/primitives/core/src/traits.rs +++ b/primitives/core/src/traits.rs @@ -74,7 +74,7 @@ pub trait BareCryptoStore: Send + Sync { /// List all supported keys /// /// Get a list of public keys the signer supports. - fn get_keys(&self, id: KeyTypeId) -> Result, String>; + fn keys(&self, id: KeyTypeId) -> Result, String>; /// Checks if the private keys for the given public key and key type combinations exist. /// /// Returns `true` iff all private keys could be found. From 9dd7929046218e0bc7f9f3e6dbe685204ef48539 Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Thu, 20 Feb 2020 09:45:49 +0100 Subject: [PATCH 21/68] Remove usage of key_pair funcs in tests --- client/rpc/src/author/tests.rs | 22 ++++++------------- .../application-crypto/test/src/ed25519.rs | 7 ++---- .../application-crypto/test/src/sr25519.rs | 7 ++---- primitives/core/src/testing.rs | 13 ++++------- 4 files changed, 15 insertions(+), 34 deletions(-) diff --git a/client/rpc/src/author/tests.rs b/client/rpc/src/author/tests.rs index ba9b9d344c2a2..426a841f8627e 100644 --- a/client/rpc/src/author/tests.rs +++ b/client/rpc/src/author/tests.rs @@ -21,7 +21,7 @@ use assert_matches::assert_matches; use codec::Encode; use sp_core::{ H256, blake2_256, hexdisplay::HexDisplay, testing::{ED25519, SR25519, KeyStore}, - traits::BareCryptoStorePtr, ed25519, crypto::{Pair, Public}, + traits::BareCryptoStorePtr, ed25519, sr25519, crypto::{Pair, Public}, }; use rpc::futures::Stream as _; use substrate_test_runtime_client::{ @@ -215,10 +215,9 @@ fn should_insert_key() { key_pair.public().0.to_vec().into(), ).expect("Insert key"); - let store_key_pair = setup.keystore.read() - .ed25519_key_pair(ED25519, &key_pair.public()).expect("Key exists in store"); + let public_keys = setup.keystore.read().keys(ED25519).unwrap(); - assert_eq!(key_pair.public(), store_key_pair.public()); + assert_eq!(true, public_keys.contains(&(ed25519::ED25519_CRYPTO_ID, key_pair.public().to_raw_vec()))); } #[test] @@ -231,18 +230,11 @@ fn should_rotate_keys() { let session_keys = SessionKeys::decode(&mut &new_public_keys[..]) .expect("SessionKeys decode successfully"); - let ed25519_key_pair = setup.keystore.read().ed25519_key_pair( - ED25519, - &session_keys.ed25519.clone().into(), - ).expect("ed25519 key exists in store"); + let ed25519_public_keys = setup.keystore.read().keys(ED25519).unwrap(); + let sr25519_public_keys = setup.keystore.read().keys(SR25519).unwrap(); - let sr25519_key_pair = setup.keystore.read().sr25519_key_pair( - SR25519, - &session_keys.sr25519.clone().into(), - ).expect("sr25519 key exists in store"); - - assert_eq!(session_keys.ed25519, ed25519_key_pair.public().into()); - assert_eq!(session_keys.sr25519, sr25519_key_pair.public().into()); + assert_eq!(true, ed25519_public_keys.contains(&(ed25519::ED25519_CRYPTO_ID, session_keys.ed25519.to_raw_vec()))); + assert_eq!(true, sr25519_public_keys.contains(&(sr25519::SR25519_CRYPTO_ID, session_keys.sr25519.to_raw_vec()))); } #[test] diff --git a/primitives/application-crypto/test/src/ed25519.rs b/primitives/application-crypto/test/src/ed25519.rs index 400edfd6ae440..5c979de44d502 100644 --- a/primitives/application-crypto/test/src/ed25519.rs +++ b/primitives/application-crypto/test/src/ed25519.rs @@ -17,7 +17,7 @@ //! Integration tests for ed25519 use sp_runtime::generic::BlockId; -use sp_core::{testing::{KeyStore, ED25519}, crypto::Pair}; +use sp_core::{testing::KeyStore, crypto::Pair}; use substrate_test_runtime_client::{ TestClientBuilder, DefaultTestClientBuilderExt, TestClientBuilderExt, runtime::TestAPI, @@ -33,8 +33,5 @@ fn ed25519_works_in_runtime() { .test_ed25519_crypto(&BlockId::Number(0)) .expect("Tests `ed25519` crypto."); - let key_pair = keystore.read().ed25519_key_pair(ED25519, &public.as_ref()) - .expect("There should be at a `ed25519` key in the keystore for the given public key."); - - assert!(AppPair::verify(&signature, "ed25519", &AppPublic::from(key_pair.public()))); + assert!(AppPair::verify(&signature, "ed25519", &AppPublic::from(public))); } diff --git a/primitives/application-crypto/test/src/sr25519.rs b/primitives/application-crypto/test/src/sr25519.rs index 49bb3c2a83640..5279f3e324a3c 100644 --- a/primitives/application-crypto/test/src/sr25519.rs +++ b/primitives/application-crypto/test/src/sr25519.rs @@ -18,7 +18,7 @@ use sp_runtime::generic::BlockId; -use sp_core::{testing::{KeyStore, SR25519}, crypto::Pair}; +use sp_core::{testing::KeyStore, crypto::Pair}; use substrate_test_runtime_client::{ TestClientBuilder, DefaultTestClientBuilderExt, TestClientBuilderExt, runtime::TestAPI, @@ -34,8 +34,5 @@ fn sr25519_works_in_runtime() { .test_sr25519_crypto(&BlockId::Number(0)) .expect("Tests `sr25519` crypto."); - let key_pair = keystore.read().sr25519_key_pair(SR25519, public.as_ref()) - .expect("There should be at a `sr25519` key in the keystore for the given public key."); - - assert!(AppPair::verify(&signature, "sr25519", &AppPublic::from(key_pair.public()))); + assert!(AppPair::verify(&signature, "sr25519", &AppPublic::from(public))); } diff --git a/primitives/core/src/testing.rs b/primitives/core/src/testing.rs index 9f93c8c8daa1d..469598cca144d 100644 --- a/primitives/core/src/testing.rs +++ b/primitives/core/src/testing.rs @@ -311,11 +311,9 @@ mod tests { .ed25519_generate_new(ED25519, None) .expect("Genrates key"); - let store_key_pair = store.read() - .ed25519_key_pair(ED25519, &public) - .expect("Key should exists in store"); + let public_keys = store.read().keys(ED25519).unwrap(); - assert_eq!(public, store_key_pair.public()); + assert_eq!(true, public_keys.contains(&(ed25519::ED25519_CRYPTO_ID, public.to_raw_vec()))); } #[test] @@ -331,11 +329,8 @@ mod tests { key_pair.public().as_ref(), ).expect("Inserts unknown key"); - let store_key_pair = store.read().sr25519_key_pair( - SR25519, - &key_pair.public(), - ).expect("Gets key pair from keystore"); + let public_keys = store.read().keys(SR25519).unwrap(); - assert_eq!(key_pair.public(), store_key_pair.public()); + assert_eq!(true, public_keys.contains(&(ed25519::SR25519_CRYPTO_ID, key_pair.public().to_raw_vec()))); } } From 4d0b81bc02bc15182c7ef658b6945cfb21b4a795 Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Thu, 20 Feb 2020 09:52:07 +0100 Subject: [PATCH 22/68] Adjust docstring for *_CYPTO_ID constants --- primitives/core/src/ecdsa.rs | 4 +--- primitives/core/src/ed25519.rs | 4 +--- primitives/core/src/sr25519.rs | 4 +--- 3 files changed, 3 insertions(+), 9 deletions(-) diff --git a/primitives/core/src/ecdsa.rs b/primitives/core/src/ecdsa.rs index e3cb6f79314b0..dcccca3d049a7 100644 --- a/primitives/core/src/ecdsa.rs +++ b/primitives/core/src/ecdsa.rs @@ -40,9 +40,7 @@ use crate::crypto::{Public as TraitPublic, UncheckedFrom, CryptoType, Derive, Cr #[cfg(feature = "full_crypto")] use secp256k1::{PublicKey, SecretKey}; -/// A value which is passed along with the public key -/// to define which cryptographic algorithm that key -/// belongs to. +/// An identifier used to match public keys against ecdsa keys pub const ECDSA_CRYPTO_ID: CryptoTypeId = CryptoTypeId(*b"ecds"); /// A secret seed (which is bytewise essentially equivalent to a SecretKey). diff --git a/primitives/core/src/ed25519.rs b/primitives/core/src/ed25519.rs index cf4332880568b..ae3a50250089c 100644 --- a/primitives/core/src/ed25519.rs +++ b/primitives/core/src/ed25519.rs @@ -42,9 +42,7 @@ use crate::crypto::{Public as TraitPublic, UncheckedFrom, CryptoType, Derive, Cr use sp_runtime_interface::pass_by::PassByInner; use sp_std::ops::Deref; -/// A value which is passed along with the public key -/// to define which cryptographic algorithm that key -/// belongs to. +/// An identifier used to match public keys against ed25519 keys pub const ED25519_CRYPTO_ID: CryptoTypeId = CryptoTypeId(*b"ed25"); /// A secret seed. It's not called a "secret key" because ring doesn't expose the secret keys diff --git a/primitives/core/src/sr25519.rs b/primitives/core/src/sr25519.rs index a448516f2f8df..cd4e8f7275a35 100644 --- a/primitives/core/src/sr25519.rs +++ b/primitives/core/src/sr25519.rs @@ -54,9 +54,7 @@ use sp_runtime_interface::pass_by::PassByInner; #[cfg(feature = "full_crypto")] const SIGNING_CTX: &[u8] = b"substrate"; -/// A value which is passed along with the public key -/// to define which cryptographic algorithm that key -/// belongs to. +/// An identifier used to match public keys against sr25519 keys pub const SR25519_CRYPTO_ID: CryptoTypeId = CryptoTypeId(*b"sr25"); /// An Schnorrkel/Ristretto x25519 ("sr25519") public key. From 0956ef751bdd69411cd58ba22ad0d0ce66d35367 Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Thu, 20 Feb 2020 11:22:25 +0100 Subject: [PATCH 23/68] Fix failures --- primitives/core/src/crypto.rs | 2 ++ primitives/core/src/testing.rs | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/primitives/core/src/crypto.rs b/primitives/core/src/crypto.rs index c1326ac7b1c0d..853cabfc6c431 100644 --- a/primitives/core/src/crypto.rs +++ b/primitives/core/src/crypto.rs @@ -996,6 +996,8 @@ mod tests { } impl Derive for TestPublic {} impl Public for TestPublic { + const CRYPTO_TYPE_ID: CryptoTypeId = CryptoTypeId(*b"test"); + fn from_slice(_bytes: &[u8]) -> Self { Self } diff --git a/primitives/core/src/testing.rs b/primitives/core/src/testing.rs index 469598cca144d..9522be9207150 100644 --- a/primitives/core/src/testing.rs +++ b/primitives/core/src/testing.rs @@ -331,6 +331,6 @@ mod tests { let public_keys = store.read().keys(SR25519).unwrap(); - assert_eq!(true, public_keys.contains(&(ed25519::SR25519_CRYPTO_ID, key_pair.public().to_raw_vec()))); + assert_eq!(true, public_keys.contains(&(sr25519::SR25519_CRYPTO_ID, key_pair.public().to_raw_vec()))); } } From 02f7099f201bf57cda2fd9aee26635039646bb2b Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Thu, 20 Feb 2020 11:53:38 +0100 Subject: [PATCH 24/68] Simplify mapping on keys --- client/keystore/src/lib.rs | 39 +++++++++++++++++++------------------- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/client/keystore/src/lib.rs b/client/keystore/src/lib.rs index aaf949378c2ad..de35f8e1ed344 100644 --- a/client/keystore/src/lib.rs +++ b/client/keystore/src/lib.rs @@ -285,16 +285,21 @@ impl BareCryptoStore for Store { id: KeyTypeId, keys: Vec ) -> std::result::Result, String> { - let ed25519_existing_keys: Vec> = self.public_keys_by_type::(id) - .map(|keys| { - keys.iter().map(|k| k.to_raw_vec()).collect() - }) + let ed25519_existing_keys = self + .public_keys_by_type::(id) .map_err(|e| e.to_string())?; - let sr25519_existing_keys: Vec> = self.public_keys_by_type::(id) - .map(|keys| { - keys.iter().map(|k| k.to_raw_vec()).collect() - }) + let ed25519_existing_keys: Vec> = ed25519_existing_keys + .iter() + .map(|k| k.to_raw_vec()) + .collect(); + + let sr25519_existing_keys= self + .public_keys_by_type::(id) .map_err(|e| e.to_string())?; + let sr25519_existing_keys: Vec> = sr25519_existing_keys + .iter() + .map(|k| k.to_raw_vec()) + .collect(); Ok(keys.iter().filter_map(|k| { match k.0 { @@ -306,24 +311,20 @@ impl BareCryptoStore for Store { } fn keys(&self, id: KeyTypeId) -> std::result::Result, String> { - let ed25519_existing_keys: Vec = self + let ed25519_existing_keys = self .public_keys_by_type::(id) - .map(|keys| { - keys.iter().map(|k| (ed25519::ED25519_CRYPTO_ID, k.to_raw_vec())).collect() - }) .map_err(|e| e.to_string())?; + let ed25519_existing_keys = ed25519_existing_keys + .iter().map(|k| (ed25519::ED25519_CRYPTO_ID, k.to_raw_vec())); - let sr25519_existing_keys: Vec = self + let sr25519_existing_keys = self .public_keys_by_type::(id) - .map(|keys| { - keys.iter().map(|k| (sr25519::SR25519_CRYPTO_ID, k.to_raw_vec())).collect() - }) .map_err(|e| e.to_string())?; + let sr25519_existing_keys = sr25519_existing_keys + .iter().map(|k| (sr25519::SR25519_CRYPTO_ID, k.to_raw_vec())); Ok(ed25519_existing_keys - .iter() - .chain(sr25519_existing_keys.iter()) - .cloned() + .chain(sr25519_existing_keys) .collect()) } From 79290de9b94c43bf3c2ef2984a36b7a11c630498 Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Thu, 20 Feb 2020 12:02:04 +0100 Subject: [PATCH 25/68] Remove one let --- client/keystore/src/lib.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/client/keystore/src/lib.rs b/client/keystore/src/lib.rs index de35f8e1ed344..7e74ce06867d1 100644 --- a/client/keystore/src/lib.rs +++ b/client/keystore/src/lib.rs @@ -314,8 +314,6 @@ impl BareCryptoStore for Store { let ed25519_existing_keys = self .public_keys_by_type::(id) .map_err(|e| e.to_string())?; - let ed25519_existing_keys = ed25519_existing_keys - .iter().map(|k| (ed25519::ED25519_CRYPTO_ID, k.to_raw_vec())); let sr25519_existing_keys = self .public_keys_by_type::(id) @@ -324,6 +322,7 @@ impl BareCryptoStore for Store { .iter().map(|k| (sr25519::SR25519_CRYPTO_ID, k.to_raw_vec())); Ok(ed25519_existing_keys + .iter().map(|k| (ed25519::ED25519_CRYPTO_ID, k.to_raw_vec())) .chain(sr25519_existing_keys) .collect()) } From 6a06530cc0ff357afab530fd56e75ee94a8b5190 Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Thu, 20 Feb 2020 13:39:10 +0100 Subject: [PATCH 26/68] Fixed typo --- client/authority-discovery/src/error.rs | 2 +- client/authority-discovery/src/lib.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/client/authority-discovery/src/error.rs b/client/authority-discovery/src/error.rs index e5b974a12cac5..c79fde02fff04 100644 --- a/client/authority-discovery/src/error.rs +++ b/client/authority-discovery/src/error.rs @@ -47,5 +47,5 @@ pub enum Error { /// Failed to parse a libp2p multi address. ParsingMultiaddress(libp2p::core::multiaddr::Error), /// Failed to map signature to public key - PublicKeyToSignatureMaping, + PublicKeyToSignatureMapping, } diff --git a/client/authority-discovery/src/lib.rs b/client/authority-discovery/src/lib.rs index c915c722bbfce..040dad1976ba4 100644 --- a/client/authority-discovery/src/lib.rs +++ b/client/authority-discovery/src/lib.rs @@ -227,7 +227,7 @@ where for (index, signature) in signatures.iter().enumerate() { let mut signed_addresses = vec![]; - let key = keys.get(index).ok_or(Error::PublicKeyToSignatureMaping)?; + let key = keys.get(index).ok_or(Error::PublicKeyToSignatureMapping)?; schema::SignedAuthorityAddresses { addresses: serialized_addresses.clone(), From 6c37161c195209eaa51b05287139ca536a69ec0b Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Thu, 20 Feb 2020 13:45:27 +0100 Subject: [PATCH 27/68] PR feedback --- primitives/core/src/testing.rs | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/primitives/core/src/testing.rs b/primitives/core/src/testing.rs index 9949203f0486e..ec9d1fde2a423 100644 --- a/primitives/core/src/testing.rs +++ b/primitives/core/src/testing.rs @@ -153,22 +153,19 @@ impl crate::traits::BareCryptoStore for KeyStore { } fn keys(&self, id: KeyTypeId) -> Result, String> { - let ed25519_existing_keys: Vec = self - .ed25519_public_keys(id) + let ed25519_existing_keys = self.ed25519_public_keys(id); + let ed25519_existing_keys = ed25519_existing_keys .iter() - .map(|k| (ed25519::ED25519_CRYPTO_ID, k.to_raw_vec())) - .collect(); + .map(|k| (ed25519::ED25519_CRYPTO_ID, k.to_raw_vec())); - let sr25519_existing_keys: Vec = self - .sr25519_public_keys(id) + let sr25519_existing_keys = self.sr25519_public_keys(id); + let sr25519_existing_keys = sr25519_existing_keys .iter() - .map(|k| (sr25519::SR25519_CRYPTO_ID, k.to_raw_vec())) - .collect(); + .map(|k| (sr25519::SR25519_CRYPTO_ID, k.to_raw_vec())); Ok(ed25519_existing_keys - .iter() - .chain(sr25519_existing_keys.iter()) - .cloned().collect()) + .chain(sr25519_existing_keys) + .collect()) } fn sign_with( From ea81fba44b8a88a7c0f76dee48a8a7d9b7af46d9 Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Thu, 20 Feb 2020 13:46:27 +0100 Subject: [PATCH 28/68] remove whitespace --- primitives/core/src/testing.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/primitives/core/src/testing.rs b/primitives/core/src/testing.rs index ec9d1fde2a423..a13f8ef3287ac 100644 --- a/primitives/core/src/testing.rs +++ b/primitives/core/src/testing.rs @@ -51,7 +51,6 @@ impl KeyStore { ) } - fn ed25519_key_pair(&self, id: KeyTypeId, pub_key: &ed25519::Public) -> Option { self.keys.get(&id) .and_then(|inner| From 118a53c4fc1a9c8d4e0e4197a05fde29a68deb8b Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Thu, 20 Feb 2020 16:04:11 +0100 Subject: [PATCH 29/68] Zip keys and signatures --- client/authority-discovery/src/error.rs | 6 ++++-- client/authority-discovery/src/lib.rs | 23 ++++++++++++++--------- 2 files changed, 18 insertions(+), 11 deletions(-) diff --git a/client/authority-discovery/src/error.rs b/client/authority-discovery/src/error.rs index c79fde02fff04..e7ea415377de9 100644 --- a/client/authority-discovery/src/error.rs +++ b/client/authority-discovery/src/error.rs @@ -16,6 +16,8 @@ //! Authority discovery errors. +use sp_core::crypto::CryptoTypePublicPair; + /// AuthorityDiscovery Result. pub type Result = std::result::Result; @@ -46,6 +48,6 @@ pub enum Error { EncodingDecodingScale(codec::Error), /// Failed to parse a libp2p multi address. ParsingMultiaddress(libp2p::core::multiaddr::Error), - /// Failed to map signature to public key - PublicKeyToSignatureMapping, + /// Failed to sign using a specific public key + MissingSignature(CryptoTypePublicPair) } diff --git a/client/authority-discovery/src/lib.rs b/client/authority-discovery/src/lib.rs index 040dad1976ba4..94bec0012b04c 100644 --- a/client/authority-discovery/src/lib.rs +++ b/client/authority-discovery/src/lib.rs @@ -218,17 +218,22 @@ where .collect(); let signatures = self.key_store - .read() - .sign_with_all( - key_types::AUTHORITY_DISCOVERY, - keys.clone(), - serialized_addresses.as_slice() - ); - - for (index, signature) in signatures.iter().enumerate() { + .read() + .sign_with_all( + key_types::AUTHORITY_DISCOVERY, + keys.clone(), + serialized_addresses.as_slice() + ); + + for (signature, key) in signatures.iter().zip(keys) { let mut signed_addresses = vec![]; - let key = keys.get(index).ok_or(Error::PublicKeyToSignatureMapping)?; + // sign_with_all returns Option where the signature + // is None for a public key that is not supported. + // Verify that all signatures exist for all provided keys. + if signature.is_none() { + return Err(Error::MissingSignature(kk)); + } schema::SignedAuthorityAddresses { addresses: serialized_addresses.clone(), signature: signature.encode(), From eaa1c80cd5056ac9c37c0c189a876dfd5012feb9 Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Thu, 20 Feb 2020 16:04:27 +0100 Subject: [PATCH 30/68] Use into_iter & remove cloned --- client/keystore/src/lib.rs | 18 ++++++++---------- primitives/core/src/testing.rs | 4 ++-- primitives/core/src/traits.rs | 2 +- 3 files changed, 11 insertions(+), 13 deletions(-) diff --git a/client/keystore/src/lib.rs b/client/keystore/src/lib.rs index 77a3e28546a1f..6811e5ee8a713 100644 --- a/client/keystore/src/lib.rs +++ b/client/keystore/src/lib.rs @@ -282,29 +282,27 @@ impl BareCryptoStore for Store { id: KeyTypeId, keys: Vec ) -> std::result::Result, String> { - let ed25519_existing_keys = self + let ed25519_existing_keys: Vec> = self .public_keys_by_type::(id) - .map_err(|e| e.to_string())?; - let ed25519_existing_keys: Vec> = ed25519_existing_keys - .iter() + .map_err(|e| e.to_string())? + .into_iter() .map(|k| k.to_raw_vec()) .collect(); - let sr25519_existing_keys= self + let sr25519_existing_keys: Vec> = self .public_keys_by_type::(id) - .map_err(|e| e.to_string())?; - let sr25519_existing_keys: Vec> = sr25519_existing_keys - .iter() + .map_err(|e| e.to_string())? + .into_iter() .map(|k| k.to_raw_vec()) .collect(); - Ok(keys.iter().filter_map(|k| { + Ok(keys.into_iter().filter_map(|k| { match k.0 { sr25519::SR25519_CRYPTO_ID if sr25519_existing_keys.contains(&k.1.to_vec()) => Some(k), ed25519::ED25519_CRYPTO_ID if ed25519_existing_keys.contains(&k.1.to_vec()) => Some(k), _ => None } - }).cloned().collect::>()) + }).collect::>()) } fn keys(&self, id: KeyTypeId) -> std::result::Result, String> { diff --git a/primitives/core/src/testing.rs b/primitives/core/src/testing.rs index a13f8ef3287ac..bf1a98588c177 100644 --- a/primitives/core/src/testing.rs +++ b/primitives/core/src/testing.rs @@ -142,13 +142,13 @@ impl crate::traits::BareCryptoStore for KeyStore { let sr25519_existing_keys: Vec> = self.sr25519_public_keys(id).iter() .map(|k| k.to_raw_vec()).collect(); - Ok(keys.iter().filter_map(|k| { + Ok(keys.into_iter().filter_map(|k| { match k.0 { sr25519::SR25519_CRYPTO_ID if sr25519_existing_keys.contains(&k.1.to_vec()) => Some(k), ed25519::ED25519_CRYPTO_ID if ed25519_existing_keys.contains(&k.1.to_vec()) => Some(k), _ => None } - }).cloned().collect::>()) + }).collect::>()) } fn keys(&self, id: KeyTypeId) -> Result, String> { diff --git a/primitives/core/src/traits.rs b/primitives/core/src/traits.rs index e223a46f6fcd2..824ebc994f405 100644 --- a/primitives/core/src/traits.rs +++ b/primitives/core/src/traits.rs @@ -89,7 +89,7 @@ pub trait BareCryptoStore: Send + Sync { id: KeyTypeId, key: &CryptoTypePublicPair, msg: &[u8], - ) -> std::result::Result, String>; + ) -> Result, String>; /// Sign with any key /// From bf42dec20a33cd1028659b992a094f85951c9801 Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Thu, 20 Feb 2020 17:05:55 +0100 Subject: [PATCH 31/68] Pass index to MissingSignature --- client/authority-discovery/src/error.rs | 4 +--- client/authority-discovery/src/lib.rs | 4 ++-- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/client/authority-discovery/src/error.rs b/client/authority-discovery/src/error.rs index e7ea415377de9..825d389435aab 100644 --- a/client/authority-discovery/src/error.rs +++ b/client/authority-discovery/src/error.rs @@ -16,8 +16,6 @@ //! Authority discovery errors. -use sp_core::crypto::CryptoTypePublicPair; - /// AuthorityDiscovery Result. pub type Result = std::result::Result; @@ -49,5 +47,5 @@ pub enum Error { /// Failed to parse a libp2p multi address. ParsingMultiaddress(libp2p::core::multiaddr::Error), /// Failed to sign using a specific public key - MissingSignature(CryptoTypePublicPair) + MissingSignature(usize) } diff --git a/client/authority-discovery/src/lib.rs b/client/authority-discovery/src/lib.rs index 94bec0012b04c..df8d31692b58d 100644 --- a/client/authority-discovery/src/lib.rs +++ b/client/authority-discovery/src/lib.rs @@ -225,14 +225,14 @@ where serialized_addresses.as_slice() ); - for (signature, key) in signatures.iter().zip(keys) { + for (index, (signature, key)) in signatures.iter().zip(keys).enumerate() { let mut signed_addresses = vec![]; // sign_with_all returns Option where the signature // is None for a public key that is not supported. // Verify that all signatures exist for all provided keys. if signature.is_none() { - return Err(Error::MissingSignature(kk)); + return Err(Error::MissingSignature(index)); } schema::SignedAuthorityAddresses { addresses: serialized_addresses.clone(), From 17dd9a29477d9b36bfad2fe007d163a5cca322a3 Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Thu, 20 Feb 2020 22:23:43 +0100 Subject: [PATCH 32/68] Use typed errors instead of strings for BareCryptoStore --- client/keystore/src/lib.rs | 43 +++++++++++++++++++++++++--------- primitives/core/src/testing.rs | 13 +++++----- primitives/core/src/traits.rs | 24 +++++++++++++++---- 3 files changed, 58 insertions(+), 22 deletions(-) diff --git a/client/keystore/src/lib.rs b/client/keystore/src/lib.rs index 6811e5ee8a713..d0613bfc593eb 100644 --- a/client/keystore/src/lib.rs +++ b/client/keystore/src/lib.rs @@ -21,7 +21,7 @@ use std::{collections::HashMap, path::PathBuf, fs::{self, File}, io::{self, Write}, sync::Arc}; use sp_core::{ crypto::{IsWrappedBy, CryptoTypePublicPair, KeyTypeId, Pair as PairT, Protected, Public}, - traits::BareCryptoStore, + traits::{BareCryptoStore, Error as TraitError} }; use sp_application_crypto::{AppKey, AppPublic, AppPair, ed25519, sr25519}; use parking_lot::RwLock; @@ -45,6 +45,12 @@ pub enum Error { /// Invalid seed #[display(fmt="Invalid seed")] InvalidSeed, + /// Public key type is not supported + #[display(fmt="Key crypto type is not supported")] + KeyNotSupported, + /// Pair not found for public key and KeyTypeId + #[display(fmt="Pair not found for {} public key", "_0")] + PairNotFound(String), /// Keystore unavailable #[display(fmt="Keystore unavailable")] Unavailable, @@ -53,6 +59,21 @@ pub enum Error { /// Keystore Result pub type Result = std::result::Result; +impl From for TraitError { + fn from(error: Error) -> Self { + match error { + Error::KeyNotSupported => TraitError::KeyNotSupported, + Error::PairNotFound(e) => TraitError::PairNotFound(e), + Error::InvalidSeed | Error::InvalidPhrase | Error::InvalidPassword => { + TraitError::ValidationError(error.to_string()) + }, + Error::Unavailable => TraitError::Unavailable, + Error::Io(e) => TraitError::Error(e.to_string()), + Error::Json(e) => TraitError::Error(e.to_string()), + } + } +} + impl std::error::Error for Error { fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { match self { @@ -281,17 +302,17 @@ impl BareCryptoStore for Store { &self, id: KeyTypeId, keys: Vec - ) -> std::result::Result, String> { + ) -> std::result::Result, TraitError> { let ed25519_existing_keys: Vec> = self .public_keys_by_type::(id) - .map_err(|e| e.to_string())? + .map_err(|e| TraitError::from(e))? .into_iter() .map(|k| k.to_raw_vec()) .collect(); let sr25519_existing_keys: Vec> = self .public_keys_by_type::(id) - .map_err(|e| e.to_string())? + .map_err(|e| TraitError::from(e))? .into_iter() .map(|k| k.to_raw_vec()) .collect(); @@ -305,14 +326,14 @@ impl BareCryptoStore for Store { }).collect::>()) } - fn keys(&self, id: KeyTypeId) -> std::result::Result, String> { + fn keys(&self, id: KeyTypeId) -> std::result::Result, TraitError> { let ed25519_existing_keys = self .public_keys_by_type::(id) - .map_err(|e| e.to_string())?; + .map_err(|e| TraitError::from(e))?; let sr25519_existing_keys = self .public_keys_by_type::(id) - .map_err(|e| e.to_string())?; + .map_err(|e| TraitError::from(e))?; let sr25519_existing_keys = sr25519_existing_keys .iter().map(|k| (sr25519::SR25519_CRYPTO_ID, k.to_raw_vec())); @@ -327,14 +348,14 @@ impl BareCryptoStore for Store { id: KeyTypeId, key: &CryptoTypePublicPair, msg: &[u8], - ) -> std::result::Result, String> { + ) -> std::result::Result, TraitError> { match key.0 { ed25519::ED25519_CRYPTO_ID => { let pub_key = ed25519::Public::from_slice(key.1.as_slice()); let key_pair: ed25519::Pair = self .key_pair_by_type::(&pub_key, id) .map(Into::into) - .map_err(|e| e.to_string())?; + .map_err(|e| TraitError::from(e))?; return Ok(<[u8; 64]>::from(key_pair.sign(msg)).to_vec()); } sr25519::SR25519_CRYPTO_ID => { @@ -342,10 +363,10 @@ impl BareCryptoStore for Store { let key_pair: sr25519::Pair = self .key_pair_by_type::(&pub_key, id) .map(Into::into) - .map_err(|e| e.to_string())?; + .map_err(|e| TraitError::from(e))?; return Ok(<[u8; 64]>::from(key_pair.sign(msg)).to_vec()); } - _ => Err(String::from("Key crypto type is not supported")) + _ => Err(TraitError::KeyNotSupported) } } diff --git a/primitives/core/src/testing.rs b/primitives/core/src/testing.rs index bf1a98588c177..00bcb921979d5 100644 --- a/primitives/core/src/testing.rs +++ b/primitives/core/src/testing.rs @@ -21,6 +21,7 @@ use crate::crypto::{KeyTypeId, CryptoTypePublicPair}; use crate::{ crypto::{Pair, Public}, ed25519, sr25519, + traits::Error }; /// Key type for generic Ed25519 key. @@ -136,7 +137,7 @@ impl crate::traits::BareCryptoStore for KeyStore { public_keys.iter().all(|(k, t)| self.keys.get(&t).and_then(|s| s.get(k)).is_some()) } - fn supported_keys(&self, id: KeyTypeId, keys: Vec) -> Result, String> { + fn supported_keys(&self, id: KeyTypeId, keys: Vec) -> Result, Error> { let ed25519_existing_keys: Vec> = self.ed25519_public_keys(id).iter() .map(|k| k.to_raw_vec()).collect(); let sr25519_existing_keys: Vec> = self.sr25519_public_keys(id).iter() @@ -151,7 +152,7 @@ impl crate::traits::BareCryptoStore for KeyStore { }).collect::>()) } - fn keys(&self, id: KeyTypeId) -> Result, String> { + fn keys(&self, id: KeyTypeId) -> Result, Error> { let ed25519_existing_keys = self.ed25519_public_keys(id); let ed25519_existing_keys = ed25519_existing_keys .iter() @@ -172,23 +173,23 @@ impl crate::traits::BareCryptoStore for KeyStore { id: KeyTypeId, key: &CryptoTypePublicPair, msg: &[u8] - ) -> Result, String> { + ) -> Result, Error> { match key.0 { ed25519::ED25519_CRYPTO_ID => { let key_pair: ed25519::Pair = self .ed25519_key_pair(id, &ed25519::Public::from_slice(key.1.as_slice())) .map(Into::into) - .ok_or(String::from("ed25519 pair not found"))?; + .ok_or(Error::PairNotFound("ed25519".to_owned()))?; return Ok(<[u8; 64]>::from(key_pair.sign(msg)).to_vec()); } sr25519::SR25519_CRYPTO_ID => { let key_pair: sr25519::Pair = self .sr25519_key_pair(id, &sr25519::Public::from_slice(key.1.as_slice())) .map(Into::into) - .ok_or(String::from("sr25519 pair not found"))?; + .ok_or(Error::PairNotFound("sr25519".to_owned()))?; return Ok(<[u8; 64]>::from(key_pair.sign(msg)).to_vec()); } - _ => Err(String::from("Key crypto type is not supported")) + _ => Err(Error::KeyNotSupported) } } } diff --git a/primitives/core/src/traits.rs b/primitives/core/src/traits.rs index 824ebc994f405..98b26e7697a07 100644 --- a/primitives/core/src/traits.rs +++ b/primitives/core/src/traits.rs @@ -29,6 +29,20 @@ use std::{ pub use sp_externalities::{Externalities, ExternalitiesExt}; +/// BareCryptoStore error +pub enum Error { + /// Public key type is not supported + KeyNotSupported, + /// Pair not found for public key and KeyTypeId + PairNotFound(String), + /// Validation error + ValidationError(String), + /// Keystore unavailable + Unavailable, + /// Programming errors + Error(String) +} + /// Something that generates, stores and provides access to keys. pub trait BareCryptoStore: Send + Sync { /// Returns all sr25519 public keys for the given key type. @@ -70,11 +84,11 @@ pub trait BareCryptoStore: Send + Sync { /// /// Provided a list of (CryptoTypeId,[u8]) pairs, this would return /// a filtered list of public keys which are supported by the keystore. - fn supported_keys(&self, id: KeyTypeId, keys: Vec) -> Result, String>; + fn supported_keys(&self, id: KeyTypeId, keys: Vec) -> Result, Error>; /// List all supported keys /// /// Get a list of public keys the signer supports. - fn keys(&self, id: KeyTypeId) -> Result, String>; + fn keys(&self, id: KeyTypeId) -> Result, Error>; /// Checks if the private keys for the given public key and key type combinations exist. /// /// Returns `true` iff all private keys could be found. @@ -89,7 +103,7 @@ pub trait BareCryptoStore: Send + Sync { id: KeyTypeId, key: &CryptoTypePublicPair, msg: &[u8], - ) -> Result, String>; + ) -> Result, Error>; /// Sign with any key /// @@ -102,7 +116,7 @@ pub trait BareCryptoStore: Send + Sync { id: KeyTypeId, keys: Vec, msg: &[u8] - ) -> Result<(CryptoTypePublicPair, Vec), String> { + ) -> Result<(CryptoTypePublicPair, Vec), Error> { if keys.len() == 1 { return self.sign_with(id, &keys[0], msg).map(|s| (keys[0].clone(), s)); } else { @@ -112,7 +126,7 @@ pub trait BareCryptoStore: Send + Sync { } } } - Err("Could not sign with any of the given keys".to_owned()) + Err(Error::KeyNotSupported) } /// Sign with all keys From 25c9bdd78dee9077144c5e67211349b964567e00 Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Thu, 20 Feb 2020 22:37:11 +0100 Subject: [PATCH 33/68] Implement Debug for trait error --- primitives/core/src/traits.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/primitives/core/src/traits.rs b/primitives/core/src/traits.rs index 98b26e7697a07..84b93c15abca8 100644 --- a/primitives/core/src/traits.rs +++ b/primitives/core/src/traits.rs @@ -30,6 +30,7 @@ use std::{ pub use sp_externalities::{Externalities, ExternalitiesExt}; /// BareCryptoStore error +#[derive(Debug)] pub enum Error { /// Public key type is not supported KeyNotSupported, From b6de5e6777aa2d841797cc975f4226433a2e4b34 Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Fri, 21 Feb 2020 09:18:26 +0100 Subject: [PATCH 34/68] Use hashsets for better performance for supported_keys --- client/keystore/src/lib.rs | 44 +++++++++++----------------------- primitives/core/src/testing.rs | 42 ++++++++++++++++---------------- primitives/core/src/traits.rs | 9 +++---- 3 files changed, 39 insertions(+), 56 deletions(-) diff --git a/client/keystore/src/lib.rs b/client/keystore/src/lib.rs index d0613bfc593eb..546c6e74879ec 100644 --- a/client/keystore/src/lib.rs +++ b/client/keystore/src/lib.rs @@ -18,7 +18,7 @@ #![warn(missing_docs)] -use std::{collections::HashMap, path::PathBuf, fs::{self, File}, io::{self, Write}, sync::Arc}; +use std::{collections::{HashMap, HashSet}, path::PathBuf, fs::{self, File}, io::{self, Write}, sync::Arc}; use sp_core::{ crypto::{IsWrappedBy, CryptoTypePublicPair, KeyTypeId, Pair as PairT, Protected, Public}, traits::{BareCryptoStore, Error as TraitError} @@ -302,45 +302,29 @@ impl BareCryptoStore for Store { &self, id: KeyTypeId, keys: Vec - ) -> std::result::Result, TraitError> { - let ed25519_existing_keys: Vec> = self - .public_keys_by_type::(id) - .map_err(|e| TraitError::from(e))? - .into_iter() - .map(|k| k.to_raw_vec()) - .collect(); - - let sr25519_existing_keys: Vec> = self - .public_keys_by_type::(id) - .map_err(|e| TraitError::from(e))? - .into_iter() - .map(|k| k.to_raw_vec()) - .collect(); + ) -> std::result::Result, TraitError> { + let keys = keys.into_iter().collect::>(); + let all_keys = self.keys(id)?; - Ok(keys.into_iter().filter_map(|k| { - match k.0 { - sr25519::SR25519_CRYPTO_ID if sr25519_existing_keys.contains(&k.1.to_vec()) => Some(k), - ed25519::ED25519_CRYPTO_ID if ed25519_existing_keys.contains(&k.1.to_vec()) => Some(k), - _ => None - } - }).collect::>()) + Ok(keys.intersection(&all_keys).cloned().collect()) } - fn keys(&self, id: KeyTypeId) -> std::result::Result, TraitError> { + fn keys(&self, id: KeyTypeId) -> std::result::Result, TraitError> { let ed25519_existing_keys = self .public_keys_by_type::(id) - .map_err(|e| TraitError::from(e))?; + .map_err(|e| TraitError::from(e))? + .into_iter() + .map(|k| (ed25519::ED25519_CRYPTO_ID, k.to_raw_vec())); let sr25519_existing_keys = self .public_keys_by_type::(id) - .map_err(|e| TraitError::from(e))?; - let sr25519_existing_keys = sr25519_existing_keys - .iter().map(|k| (sr25519::SR25519_CRYPTO_ID, k.to_raw_vec())); + .map_err(|e| TraitError::from(e))? + .into_iter() + .map(|k| (sr25519::SR25519_CRYPTO_ID, k.to_raw_vec())); Ok(ed25519_existing_keys - .iter().map(|k| (ed25519::ED25519_CRYPTO_ID, k.to_raw_vec())) - .chain(sr25519_existing_keys) - .collect()) + .chain(sr25519_existing_keys) + .collect::>()) } fn sign_with( diff --git a/primitives/core/src/testing.rs b/primitives/core/src/testing.rs index 00bcb921979d5..a3bee3b162973 100644 --- a/primitives/core/src/testing.rs +++ b/primitives/core/src/testing.rs @@ -23,6 +23,8 @@ use crate::{ ed25519, sr25519, traits::Error }; +#[cfg(feature = "std")] +use std::collections::HashSet; /// Key type for generic Ed25519 key. pub const ED25519: KeyTypeId = KeyTypeId(*b"ed25"); @@ -137,35 +139,31 @@ impl crate::traits::BareCryptoStore for KeyStore { public_keys.iter().all(|(k, t)| self.keys.get(&t).and_then(|s| s.get(k)).is_some()) } - fn supported_keys(&self, id: KeyTypeId, keys: Vec) -> Result, Error> { - let ed25519_existing_keys: Vec> = self.ed25519_public_keys(id).iter() - .map(|k| k.to_raw_vec()).collect(); - let sr25519_existing_keys: Vec> = self.sr25519_public_keys(id).iter() - .map(|k| k.to_raw_vec()).collect(); - - Ok(keys.into_iter().filter_map(|k| { - match k.0 { - sr25519::SR25519_CRYPTO_ID if sr25519_existing_keys.contains(&k.1.to_vec()) => Some(k), - ed25519::ED25519_CRYPTO_ID if ed25519_existing_keys.contains(&k.1.to_vec()) => Some(k), - _ => None - } - }).collect::>()) + fn supported_keys( + &self, + id: KeyTypeId, + keys: Vec + ) -> std::result::Result, Error> { + let keys = keys.into_iter().collect::>(); + let all_keys = self.keys(id)?; + + Ok(keys.intersection(&all_keys).cloned().collect()) } - fn keys(&self, id: KeyTypeId) -> Result, Error> { - let ed25519_existing_keys = self.ed25519_public_keys(id); - let ed25519_existing_keys = ed25519_existing_keys - .iter() + fn keys(&self, id: KeyTypeId) -> std::result::Result, Error> { + let ed25519_existing_keys = self + .ed25519_public_keys(id) + .into_iter() .map(|k| (ed25519::ED25519_CRYPTO_ID, k.to_raw_vec())); - let sr25519_existing_keys = self.sr25519_public_keys(id); - let sr25519_existing_keys = sr25519_existing_keys - .iter() + let sr25519_existing_keys = self + .sr25519_public_keys(id) + .into_iter() .map(|k| (sr25519::SR25519_CRYPTO_ID, k.to_raw_vec())); Ok(ed25519_existing_keys - .chain(sr25519_existing_keys) - .collect()) + .chain(sr25519_existing_keys) + .collect::>()) } fn sign_with( diff --git a/primitives/core/src/traits.rs b/primitives/core/src/traits.rs index 84b93c15abca8..227c16c7be47b 100644 --- a/primitives/core/src/traits.rs +++ b/primitives/core/src/traits.rs @@ -23,6 +23,7 @@ use crate::{ use std::{ fmt::{Debug, Display}, + collections::HashSet, panic::UnwindSafe, sync::Arc, }; @@ -84,12 +85,12 @@ pub trait BareCryptoStore: Send + Sync { /// Find intersection between provided keys and supported keys /// /// Provided a list of (CryptoTypeId,[u8]) pairs, this would return - /// a filtered list of public keys which are supported by the keystore. - fn supported_keys(&self, id: KeyTypeId, keys: Vec) -> Result, Error>; + /// a filtered set of public keys which are supported by the keystore. + fn supported_keys(&self, id: KeyTypeId, keys: Vec) -> Result, Error>; /// List all supported keys /// - /// Get a list of public keys the signer supports. - fn keys(&self, id: KeyTypeId) -> Result, Error>; + /// Returns a set of public keys the signer supports. + fn keys(&self, id: KeyTypeId) -> Result, Error>; /// Checks if the private keys for the given public key and key type combinations exist. /// /// Returns `true` iff all private keys could be found. From 6cad7b9ef89b50c269a3cb43cf7c6a86578734e8 Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Fri, 21 Feb 2020 09:34:49 +0100 Subject: [PATCH 35/68] Make sure keys are inserted into the keystore --- primitives/application-crypto/test/src/ed25519.rs | 9 +++++++-- primitives/application-crypto/test/src/sr25519.rs | 9 +++++++-- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/primitives/application-crypto/test/src/ed25519.rs b/primitives/application-crypto/test/src/ed25519.rs index 5c979de44d502..5e99c5b352fec 100644 --- a/primitives/application-crypto/test/src/ed25519.rs +++ b/primitives/application-crypto/test/src/ed25519.rs @@ -17,13 +17,16 @@ //! Integration tests for ed25519 use sp_runtime::generic::BlockId; -use sp_core::{testing::KeyStore, crypto::Pair}; +use sp_core::{ + crypto::{Pair, Public}, + testing::{KeyStore, ED25519}, +}; use substrate_test_runtime_client::{ TestClientBuilder, DefaultTestClientBuilderExt, TestClientBuilderExt, runtime::TestAPI, }; use sp_api::ProvideRuntimeApi; -use sp_application_crypto::ed25519::{AppPair, AppPublic}; +use sp_application_crypto::ed25519::{ED25519_CRYPTO_ID, AppPair, AppPublic}; #[test] fn ed25519_works_in_runtime() { @@ -33,5 +36,7 @@ fn ed25519_works_in_runtime() { .test_ed25519_crypto(&BlockId::Number(0)) .expect("Tests `ed25519` crypto."); + let supported_keys = keystore.read().keys(ED25519).unwrap(); + assert!(true, supported_keys.contains(&(ED25519_CRYPTO_ID, public.to_raw_vec()))); assert!(AppPair::verify(&signature, "ed25519", &AppPublic::from(public))); } diff --git a/primitives/application-crypto/test/src/sr25519.rs b/primitives/application-crypto/test/src/sr25519.rs index 5279f3e324a3c..a951a8bb3b89d 100644 --- a/primitives/application-crypto/test/src/sr25519.rs +++ b/primitives/application-crypto/test/src/sr25519.rs @@ -18,13 +18,16 @@ use sp_runtime::generic::BlockId; -use sp_core::{testing::KeyStore, crypto::Pair}; +use sp_core::{ + crypto::{Pair, Public}, + testing::{KeyStore, SR25519}, +}; use substrate_test_runtime_client::{ TestClientBuilder, DefaultTestClientBuilderExt, TestClientBuilderExt, runtime::TestAPI, }; use sp_api::ProvideRuntimeApi; -use sp_application_crypto::sr25519::{AppPair, AppPublic}; +use sp_application_crypto::sr25519::{SR25519_CRYPTO_ID, AppPair, AppPublic}; #[test] fn sr25519_works_in_runtime() { @@ -34,5 +37,7 @@ fn sr25519_works_in_runtime() { .test_sr25519_crypto(&BlockId::Number(0)) .expect("Tests `sr25519` crypto."); + let supported_keys = keystore.read().keys(SR25519).unwrap(); + assert!(true, supported_keys.contains(&(SR25519_CRYPTO_ID, public.to_raw_vec()))); assert!(AppPair::verify(&signature, "sr25519", &AppPublic::from(public))); } From 87e13faa34b2a1977a97862acadf0ec60a0949fc Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Fri, 21 Feb 2020 09:40:58 +0100 Subject: [PATCH 36/68] Make sign_with_all return type consistent with `sign_with` --- client/authority-discovery/src/error.rs | 4 +++- client/authority-discovery/src/lib.rs | 10 ++++++---- primitives/core/src/traits.rs | 4 ++-- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/client/authority-discovery/src/error.rs b/client/authority-discovery/src/error.rs index 825d389435aab..86251a6a58edf 100644 --- a/client/authority-discovery/src/error.rs +++ b/client/authority-discovery/src/error.rs @@ -47,5 +47,7 @@ pub enum Error { /// Failed to parse a libp2p multi address. ParsingMultiaddress(libp2p::core::multiaddr::Error), /// Failed to sign using a specific public key - MissingSignature(usize) + MissingSignature(usize), + /// Failed to sign using all public keys + SigningFailed } diff --git a/client/authority-discovery/src/lib.rs b/client/authority-discovery/src/lib.rs index df8d31692b58d..f562935e7ed6e 100644 --- a/client/authority-discovery/src/lib.rs +++ b/client/authority-discovery/src/lib.rs @@ -223,20 +223,22 @@ where key_types::AUTHORITY_DISCOVERY, keys.clone(), serialized_addresses.as_slice() - ); + ) + .map_err(|_| Error::SigningFailed)?; - for (index, (signature, key)) in signatures.iter().zip(keys).enumerate() { + for (index, (sign_result, key)) in signatures.iter().zip(keys).enumerate() { let mut signed_addresses = vec![]; // sign_with_all returns Option where the signature // is None for a public key that is not supported. // Verify that all signatures exist for all provided keys. - if signature.is_none() { + if sign_result.is_err() { return Err(Error::MissingSignature(index)); } + let signature = sign_result.as_ref().unwrap(); schema::SignedAuthorityAddresses { addresses: serialized_addresses.clone(), - signature: signature.encode(), + signature: Encode::encode(&signature), } .encode(&mut signed_addresses) .map_err(Error::EncodingProto)?; diff --git a/primitives/core/src/traits.rs b/primitives/core/src/traits.rs index 227c16c7be47b..34a1915051072 100644 --- a/primitives/core/src/traits.rs +++ b/primitives/core/src/traits.rs @@ -143,8 +143,8 @@ pub trait BareCryptoStore: Send + Sync { id: KeyTypeId, keys: Vec, msg: &[u8] - ) -> Vec>> { - keys.iter().map(|k| self.sign_with(id, k, msg).ok()).collect() + ) -> Result, Error>>, ()>{ + Ok(keys.iter().map(|k| self.sign_with(id, k, msg)).collect()) } } From a7bfe139a73421b58eb94035b557e7e5bc46bcda Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Fri, 21 Feb 2020 14:37:11 +0100 Subject: [PATCH 37/68] Rename Error to BareCryptoStoreError --- client/keystore/src/lib.rs | 2 +- primitives/core/src/testing.rs | 18 +++++++++--------- primitives/core/src/traits.rs | 14 +++++++------- 3 files changed, 17 insertions(+), 17 deletions(-) diff --git a/client/keystore/src/lib.rs b/client/keystore/src/lib.rs index 546c6e74879ec..da4f5181b3215 100644 --- a/client/keystore/src/lib.rs +++ b/client/keystore/src/lib.rs @@ -21,7 +21,7 @@ use std::{collections::{HashMap, HashSet}, path::PathBuf, fs::{self, File}, io::{self, Write}, sync::Arc}; use sp_core::{ crypto::{IsWrappedBy, CryptoTypePublicPair, KeyTypeId, Pair as PairT, Protected, Public}, - traits::{BareCryptoStore, Error as TraitError} + traits::{BareCryptoStore, BareCryptoStoreError as TraitError} }; use sp_application_crypto::{AppKey, AppPublic, AppPair, ed25519, sr25519}; use parking_lot::RwLock; diff --git a/primitives/core/src/testing.rs b/primitives/core/src/testing.rs index a3bee3b162973..fd9d508707959 100644 --- a/primitives/core/src/testing.rs +++ b/primitives/core/src/testing.rs @@ -21,7 +21,7 @@ use crate::crypto::{KeyTypeId, CryptoTypePublicPair}; use crate::{ crypto::{Pair, Public}, ed25519, sr25519, - traits::Error + traits::BareCryptoStoreError }; #[cfg(feature = "std")] use std::collections::HashSet; @@ -143,23 +143,23 @@ impl crate::traits::BareCryptoStore for KeyStore { &self, id: KeyTypeId, keys: Vec - ) -> std::result::Result, Error> { + ) -> std::result::Result, BareCryptoStoreError> { let keys = keys.into_iter().collect::>(); let all_keys = self.keys(id)?; Ok(keys.intersection(&all_keys).cloned().collect()) } - fn keys(&self, id: KeyTypeId) -> std::result::Result, Error> { + fn keys(&self, id: KeyTypeId) -> std::result::Result, BareCryptoStoreError> { let ed25519_existing_keys = self .ed25519_public_keys(id) .into_iter() - .map(|k| (ed25519::ED25519_CRYPTO_ID, k.to_raw_vec())); + .map(|k| (ed25519::CRYPTO_ID, k.to_raw_vec())); let sr25519_existing_keys = self .sr25519_public_keys(id) .into_iter() - .map(|k| (sr25519::SR25519_CRYPTO_ID, k.to_raw_vec())); + .map(|k| (sr25519::CRYPTO_ID, k.to_raw_vec())); Ok(ed25519_existing_keys .chain(sr25519_existing_keys) @@ -171,23 +171,23 @@ impl crate::traits::BareCryptoStore for KeyStore { id: KeyTypeId, key: &CryptoTypePublicPair, msg: &[u8] - ) -> Result, Error> { + ) -> Result, BareCryptoStoreError> { match key.0 { ed25519::ED25519_CRYPTO_ID => { let key_pair: ed25519::Pair = self .ed25519_key_pair(id, &ed25519::Public::from_slice(key.1.as_slice())) .map(Into::into) - .ok_or(Error::PairNotFound("ed25519".to_owned()))?; + .ok_or(BareCryptoStoreError::PairNotFound("ed25519".to_owned()))?; return Ok(<[u8; 64]>::from(key_pair.sign(msg)).to_vec()); } sr25519::SR25519_CRYPTO_ID => { let key_pair: sr25519::Pair = self .sr25519_key_pair(id, &sr25519::Public::from_slice(key.1.as_slice())) .map(Into::into) - .ok_or(Error::PairNotFound("sr25519".to_owned()))?; + .ok_or(BareCryptoStoreError::PairNotFound("sr25519".to_owned()))?; return Ok(<[u8; 64]>::from(key_pair.sign(msg)).to_vec()); } - _ => Err(Error::KeyNotSupported) + _ => Err(BareCryptoStoreError::KeyNotSupported) } } } diff --git a/primitives/core/src/traits.rs b/primitives/core/src/traits.rs index 34a1915051072..f4802132eb1b3 100644 --- a/primitives/core/src/traits.rs +++ b/primitives/core/src/traits.rs @@ -32,7 +32,7 @@ pub use sp_externalities::{Externalities, ExternalitiesExt}; /// BareCryptoStore error #[derive(Debug)] -pub enum Error { +pub enum BareCryptoStoreError { /// Public key type is not supported KeyNotSupported, /// Pair not found for public key and KeyTypeId @@ -86,11 +86,11 @@ pub trait BareCryptoStore: Send + Sync { /// /// Provided a list of (CryptoTypeId,[u8]) pairs, this would return /// a filtered set of public keys which are supported by the keystore. - fn supported_keys(&self, id: KeyTypeId, keys: Vec) -> Result, Error>; + fn supported_keys(&self, id: KeyTypeId, keys: Vec) -> Result, BareCryptoStoreError>; /// List all supported keys /// /// Returns a set of public keys the signer supports. - fn keys(&self, id: KeyTypeId) -> Result, Error>; + fn keys(&self, id: KeyTypeId) -> Result, BareCryptoStoreError>; /// Checks if the private keys for the given public key and key type combinations exist. /// /// Returns `true` iff all private keys could be found. @@ -105,7 +105,7 @@ pub trait BareCryptoStore: Send + Sync { id: KeyTypeId, key: &CryptoTypePublicPair, msg: &[u8], - ) -> Result, Error>; + ) -> Result, BareCryptoStoreError>; /// Sign with any key /// @@ -118,7 +118,7 @@ pub trait BareCryptoStore: Send + Sync { id: KeyTypeId, keys: Vec, msg: &[u8] - ) -> Result<(CryptoTypePublicPair, Vec), Error> { + ) -> Result<(CryptoTypePublicPair, Vec), BareCryptoStoreError> { if keys.len() == 1 { return self.sign_with(id, &keys[0], msg).map(|s| (keys[0].clone(), s)); } else { @@ -128,7 +128,7 @@ pub trait BareCryptoStore: Send + Sync { } } } - Err(Error::KeyNotSupported) + Err(BareCryptoStoreError::KeyNotSupported) } /// Sign with all keys @@ -143,7 +143,7 @@ pub trait BareCryptoStore: Send + Sync { id: KeyTypeId, keys: Vec, msg: &[u8] - ) -> Result, Error>>, ()>{ + ) -> Result, BareCryptoStoreError>>, ()>{ Ok(keys.iter().map(|k| self.sign_with(id, k, msg)).collect()) } } From 7d383832ae0d4ef4193cb2e2d404aae30e7d1831 Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Fri, 21 Feb 2020 14:37:59 +0100 Subject: [PATCH 38/68] Rename CRYPT_TYPE_ID -> CRYPTO_ID --- client/authority-discovery/src/lib.rs | 4 ++-- client/keystore/src/lib.rs | 8 ++++---- client/rpc/src/author/tests.rs | 6 +++--- primitives/application-crypto/src/lib.rs | 2 +- primitives/application-crypto/test/src/ed25519.rs | 4 ++-- primitives/application-crypto/test/src/sr25519.rs | 4 ++-- primitives/core/src/crypto.rs | 8 ++++---- primitives/core/src/ecdsa.rs | 4 ++-- primitives/core/src/ed25519.rs | 4 ++-- primitives/core/src/sr25519.rs | 4 ++-- primitives/core/src/testing.rs | 8 ++++---- primitives/io/src/lib.rs | 4 ++-- 12 files changed, 30 insertions(+), 30 deletions(-) diff --git a/client/authority-discovery/src/lib.rs b/client/authority-discovery/src/lib.rs index f562935e7ed6e..206986493918d 100644 --- a/client/authority-discovery/src/lib.rs +++ b/client/authority-discovery/src/lib.rs @@ -214,7 +214,7 @@ where let keys: Vec = self.get_own_public_keys_within_authority_set()? .into_iter() - .map(|k| (sr25519::SR25519_CRYPTO_ID, k.to_raw_vec())) + .map(|k| (sr25519::CRYPTO_ID, k.to_raw_vec())) .collect(); let signatures = self.key_store @@ -222,7 +222,7 @@ where .sign_with_all( key_types::AUTHORITY_DISCOVERY, keys.clone(), - serialized_addresses.as_slice() + serialized_addresses.as_slice(), ) .map_err(|_| Error::SigningFailed)?; diff --git a/client/keystore/src/lib.rs b/client/keystore/src/lib.rs index da4f5181b3215..6ca8428e7cb04 100644 --- a/client/keystore/src/lib.rs +++ b/client/keystore/src/lib.rs @@ -314,13 +314,13 @@ impl BareCryptoStore for Store { .public_keys_by_type::(id) .map_err(|e| TraitError::from(e))? .into_iter() - .map(|k| (ed25519::ED25519_CRYPTO_ID, k.to_raw_vec())); + .map(|k| (ed25519::CRYPTO_ID, k.to_raw_vec())); let sr25519_existing_keys = self .public_keys_by_type::(id) .map_err(|e| TraitError::from(e))? .into_iter() - .map(|k| (sr25519::SR25519_CRYPTO_ID, k.to_raw_vec())); + .map(|k| (sr25519::CRYPTO_ID, k.to_raw_vec())); Ok(ed25519_existing_keys .chain(sr25519_existing_keys) @@ -334,7 +334,7 @@ impl BareCryptoStore for Store { msg: &[u8], ) -> std::result::Result, TraitError> { match key.0 { - ed25519::ED25519_CRYPTO_ID => { + ed25519::CRYPTO_ID => { let pub_key = ed25519::Public::from_slice(key.1.as_slice()); let key_pair: ed25519::Pair = self .key_pair_by_type::(&pub_key, id) @@ -342,7 +342,7 @@ impl BareCryptoStore for Store { .map_err(|e| TraitError::from(e))?; return Ok(<[u8; 64]>::from(key_pair.sign(msg)).to_vec()); } - sr25519::SR25519_CRYPTO_ID => { + sr25519::CRYPTO_ID => { let pub_key = sr25519::Public::from_slice(key.1.as_slice()); let key_pair: sr25519::Pair = self .key_pair_by_type::(&pub_key, id) diff --git a/client/rpc/src/author/tests.rs b/client/rpc/src/author/tests.rs index 92b3d9642e221..8f95b7c9d345c 100644 --- a/client/rpc/src/author/tests.rs +++ b/client/rpc/src/author/tests.rs @@ -217,7 +217,7 @@ fn should_insert_key() { let public_keys = setup.keystore.read().keys(ED25519).unwrap(); - assert_eq!(true, public_keys.contains(&(ed25519::ED25519_CRYPTO_ID, key_pair.public().to_raw_vec()))); + assert!(public_keys.contains(&(ed25519::CRYPTO_ID, key_pair.public().to_raw_vec()))); } #[test] @@ -233,8 +233,8 @@ fn should_rotate_keys() { let ed25519_public_keys = setup.keystore.read().keys(ED25519).unwrap(); let sr25519_public_keys = setup.keystore.read().keys(SR25519).unwrap(); - assert_eq!(true, ed25519_public_keys.contains(&(ed25519::ED25519_CRYPTO_ID, session_keys.ed25519.to_raw_vec()))); - assert_eq!(true, sr25519_public_keys.contains(&(sr25519::SR25519_CRYPTO_ID, session_keys.sr25519.to_raw_vec()))); + assert!(ed25519_public_keys.contains(&(ed25519::CRYPTO_ID, session_keys.ed25519.to_raw_vec()))); + assert!(sr25519_public_keys.contains(&(sr25519::CRYPTO_ID, session_keys.sr25519.to_raw_vec()))); } #[test] diff --git a/primitives/application-crypto/src/lib.rs b/primitives/application-crypto/src/lib.rs index 1855f8da74d5b..c8079634f1a8d 100644 --- a/primitives/application-crypto/src/lib.rs +++ b/primitives/application-crypto/src/lib.rs @@ -271,7 +271,7 @@ macro_rules! app_crypto_public_common { } impl $crate::Public for Public { - const CRYPTO_TYPE_ID: $crate::CryptoTypeId = $crate::CryptoTypeId($key_type.0); + const CRYPTO_ID: $crate::CryptoTypeId = $crate::CryptoTypeId($key_type.0); fn from_slice(x: &[u8]) -> Self { Self(<$public>::from_slice(x)) } } diff --git a/primitives/application-crypto/test/src/ed25519.rs b/primitives/application-crypto/test/src/ed25519.rs index 5e99c5b352fec..d9a770873dfe1 100644 --- a/primitives/application-crypto/test/src/ed25519.rs +++ b/primitives/application-crypto/test/src/ed25519.rs @@ -26,7 +26,7 @@ use substrate_test_runtime_client::{ runtime::TestAPI, }; use sp_api::ProvideRuntimeApi; -use sp_application_crypto::ed25519::{ED25519_CRYPTO_ID, AppPair, AppPublic}; +use sp_application_crypto::ed25519::{CRYPTO_ID, AppPair, AppPublic}; #[test] fn ed25519_works_in_runtime() { @@ -37,6 +37,6 @@ fn ed25519_works_in_runtime() { .expect("Tests `ed25519` crypto."); let supported_keys = keystore.read().keys(ED25519).unwrap(); - assert!(true, supported_keys.contains(&(ED25519_CRYPTO_ID, public.to_raw_vec()))); + assert!(true, supported_keys.contains(&(CRYPTO_ID, public.to_raw_vec()))); assert!(AppPair::verify(&signature, "ed25519", &AppPublic::from(public))); } diff --git a/primitives/application-crypto/test/src/sr25519.rs b/primitives/application-crypto/test/src/sr25519.rs index a951a8bb3b89d..c7ccf41a94def 100644 --- a/primitives/application-crypto/test/src/sr25519.rs +++ b/primitives/application-crypto/test/src/sr25519.rs @@ -27,7 +27,7 @@ use substrate_test_runtime_client::{ runtime::TestAPI, }; use sp_api::ProvideRuntimeApi; -use sp_application_crypto::sr25519::{SR25519_CRYPTO_ID, AppPair, AppPublic}; +use sp_application_crypto::sr25519::{CRYPTO_ID, AppPair, AppPublic}; #[test] fn sr25519_works_in_runtime() { @@ -38,6 +38,6 @@ fn sr25519_works_in_runtime() { .expect("Tests `sr25519` crypto."); let supported_keys = keystore.read().keys(SR25519).unwrap(); - assert!(true, supported_keys.contains(&(SR25519_CRYPTO_ID, public.to_raw_vec()))); + assert!(true, supported_keys.contains(&(CRYPTO_ID, public.to_raw_vec()))); assert!(AppPair::verify(&signature, "sr25519", &AppPublic::from(public))); } diff --git a/primitives/core/src/crypto.rs b/primitives/core/src/crypto.rs index 853cabfc6c431..ff7bb332224ee 100644 --- a/primitives/core/src/crypto.rs +++ b/primitives/core/src/crypto.rs @@ -518,8 +518,8 @@ impl + AsRef<[u8]> + Default + Derive> Ss58Codec for T { pub trait Public: AsRef<[u8]> + AsMut<[u8]> + Default + Derive + CryptoType + PartialEq + Eq + Clone + Send + Sync { - /// Each implementation of Public should define its Kind identifier - const CRYPTO_TYPE_ID: CryptoTypeId; + /// The identifier of this crypto type + const CRYPTO_ID: CryptoTypeId; /// A new instance from the given slice. /// @@ -662,7 +662,7 @@ mod dummy { impl Derive for Dummy {} impl Public for Dummy { - const CRYPTO_TYPE_ID: CryptoTypeId = CryptoTypeId(*b"dumm"); + const CRYPTO_ID: CryptoTypeId = CryptoTypeId(*b"dumm"); fn from_slice(_: &[u8]) -> Self { Self } #[cfg(feature = "std")] @@ -996,7 +996,7 @@ mod tests { } impl Derive for TestPublic {} impl Public for TestPublic { - const CRYPTO_TYPE_ID: CryptoTypeId = CryptoTypeId(*b"test"); + const CRYPTO_ID: CryptoTypeId = CryptoTypeId(*b"test"); fn from_slice(_bytes: &[u8]) -> Self { Self diff --git a/primitives/core/src/ecdsa.rs b/primitives/core/src/ecdsa.rs index dcccca3d049a7..f06768833fa9b 100644 --- a/primitives/core/src/ecdsa.rs +++ b/primitives/core/src/ecdsa.rs @@ -41,7 +41,7 @@ use crate::crypto::{Public as TraitPublic, UncheckedFrom, CryptoType, Derive, Cr use secp256k1::{PublicKey, SecretKey}; /// An identifier used to match public keys against ecdsa keys -pub const ECDSA_CRYPTO_ID: CryptoTypeId = CryptoTypeId(*b"ecds"); +pub const CRYPTO_ID: CryptoTypeId = CryptoTypeId(*b"ecds"); /// A secret seed (which is bytewise essentially equivalent to a SecretKey). /// @@ -98,7 +98,7 @@ impl Public { } impl TraitPublic for Public { - const CRYPTO_TYPE_ID: CryptoTypeId = ECDSA_CRYPTO_ID; + const CRYPTO_ID: CryptoTypeId = CRYPTO_ID; /// A new instance from the given slice that should be 33 bytes long. /// /// NOTE: No checking goes on to ensure this is a real public key. Only use it if diff --git a/primitives/core/src/ed25519.rs b/primitives/core/src/ed25519.rs index 8eabda2baf63d..88c3389e94021 100644 --- a/primitives/core/src/ed25519.rs +++ b/primitives/core/src/ed25519.rs @@ -43,7 +43,7 @@ use sp_runtime_interface::pass_by::PassByInner; use sp_std::ops::Deref; /// An identifier used to match public keys against ed25519 keys -pub const ED25519_CRYPTO_ID: CryptoTypeId = CryptoTypeId(*b"ed25"); +pub const CRYPTO_ID: CryptoTypeId = CryptoTypeId(*b"ed25"); /// A secret seed. It's not called a "secret key" because ring doesn't expose the secret keys /// of the key pair (yeah, dumb); as such we're forced to remember the seed manually if we @@ -368,7 +368,7 @@ impl Public { } impl TraitPublic for Public { - const CRYPTO_TYPE_ID: CryptoTypeId = ED25519_CRYPTO_ID; + const CRYPTO_ID: CryptoTypeId = CRYPTO_ID; /// A new instance from the given slice that should be 32 bytes long. /// diff --git a/primitives/core/src/sr25519.rs b/primitives/core/src/sr25519.rs index cd4e8f7275a35..211ea7bc98495 100644 --- a/primitives/core/src/sr25519.rs +++ b/primitives/core/src/sr25519.rs @@ -55,7 +55,7 @@ use sp_runtime_interface::pass_by::PassByInner; const SIGNING_CTX: &[u8] = b"substrate"; /// An identifier used to match public keys against sr25519 keys -pub const SR25519_CRYPTO_ID: CryptoTypeId = CryptoTypeId(*b"sr25"); +pub const CRYPTO_ID: CryptoTypeId = CryptoTypeId(*b"sr25"); /// An Schnorrkel/Ristretto x25519 ("sr25519") public key. #[cfg_attr(feature = "full_crypto", derive(Hash))] @@ -382,7 +382,7 @@ impl Public { } impl TraitPublic for Public { - const CRYPTO_TYPE_ID: CryptoTypeId = SR25519_CRYPTO_ID; + const CRYPTO_ID: CryptoTypeId = CRYPTO_ID; /// A new instance from the given slice that should be 32 bytes long. /// diff --git a/primitives/core/src/testing.rs b/primitives/core/src/testing.rs index fd9d508707959..212a4859e3c87 100644 --- a/primitives/core/src/testing.rs +++ b/primitives/core/src/testing.rs @@ -173,14 +173,14 @@ impl crate::traits::BareCryptoStore for KeyStore { msg: &[u8] ) -> Result, BareCryptoStoreError> { match key.0 { - ed25519::ED25519_CRYPTO_ID => { + ed25519::CRYPTO_ID => { let key_pair: ed25519::Pair = self .ed25519_key_pair(id, &ed25519::Public::from_slice(key.1.as_slice())) .map(Into::into) .ok_or(BareCryptoStoreError::PairNotFound("ed25519".to_owned()))?; return Ok(<[u8; 64]>::from(key_pair.sign(msg)).to_vec()); } - sr25519::SR25519_CRYPTO_ID => { + sr25519::CRYPTO_ID => { let key_pair: sr25519::Pair = self .sr25519_key_pair(id, &sr25519::Public::from_slice(key.1.as_slice())) .map(Into::into) @@ -308,7 +308,7 @@ mod tests { let public_keys = store.read().keys(ED25519).unwrap(); - assert_eq!(true, public_keys.contains(&(ed25519::ED25519_CRYPTO_ID, public.to_raw_vec()))); + assert!(public_keys.contains(&(ed25519::CRYPTO_ID, public.to_raw_vec()))); } #[test] @@ -326,6 +326,6 @@ mod tests { let public_keys = store.read().keys(SR25519).unwrap(); - assert_eq!(true, public_keys.contains(&(sr25519::SR25519_CRYPTO_ID, key_pair.public().to_raw_vec()))); + assert!(public_keys.contains(&(sr25519::CRYPTO_ID, key_pair.public().to_raw_vec()))); } } diff --git a/primitives/io/src/lib.rs b/primitives/io/src/lib.rs index 35e3c96b1ba1e..f8bf02c61287d 100644 --- a/primitives/io/src/lib.rs +++ b/primitives/io/src/lib.rs @@ -410,7 +410,7 @@ pub trait Crypto { self.extension::() .expect("No `keystore` associated for the current context!") .read() - .sign_with(id, &(ed25519::ED25519_CRYPTO_ID, pub_key.to_raw_vec()), msg) + .sign_with(id, &(ed25519::CRYPTO_ID, pub_key.to_raw_vec()), msg) .map(|sig| ed25519::Signature::from_slice(sig.as_slice())) .ok() } @@ -463,7 +463,7 @@ pub trait Crypto { self.extension::() .expect("No `keystore` associated for the current context!") .read() - .sign_with(id, &(sr25519::SR25519_CRYPTO_ID, pub_key.to_raw_vec()), msg) + .sign_with(id, &(sr25519::CRYPTO_ID, pub_key.to_raw_vec()), msg) .map(|sig| sr25519::Signature::from_slice(sig.as_slice())) .ok() } From 321f16c44dac14d1ed1a486c7dd8b5928a417730 Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Fri, 21 Feb 2020 14:48:02 +0100 Subject: [PATCH 39/68] Remove unnecessary CRYPTO_ID declaration in Public trait --- primitives/application-crypto/src/lib.rs | 2 -- primitives/core/src/crypto.rs | 7 ------- primitives/core/src/ecdsa.rs | 1 - primitives/core/src/ed25519.rs | 2 -- primitives/core/src/sr25519.rs | 2 -- 5 files changed, 14 deletions(-) diff --git a/primitives/application-crypto/src/lib.rs b/primitives/application-crypto/src/lib.rs index c8079634f1a8d..c5e3ad36ff44a 100644 --- a/primitives/application-crypto/src/lib.rs +++ b/primitives/application-crypto/src/lib.rs @@ -271,8 +271,6 @@ macro_rules! app_crypto_public_common { } impl $crate::Public for Public { - const CRYPTO_ID: $crate::CryptoTypeId = $crate::CryptoTypeId($key_type.0); - fn from_slice(x: &[u8]) -> Self { Self(<$public>::from_slice(x)) } } diff --git a/primitives/core/src/crypto.rs b/primitives/core/src/crypto.rs index ff7bb332224ee..7394ed47d0972 100644 --- a/primitives/core/src/crypto.rs +++ b/primitives/core/src/crypto.rs @@ -518,9 +518,6 @@ impl + AsRef<[u8]> + Default + Derive> Ss58Codec for T { pub trait Public: AsRef<[u8]> + AsMut<[u8]> + Default + Derive + CryptoType + PartialEq + Eq + Clone + Send + Sync { - /// The identifier of this crypto type - const CRYPTO_ID: CryptoTypeId; - /// A new instance from the given slice. /// /// NOTE: No checking goes on to ensure this is a real public key. Only use it if @@ -662,8 +659,6 @@ mod dummy { impl Derive for Dummy {} impl Public for Dummy { - const CRYPTO_ID: CryptoTypeId = CryptoTypeId(*b"dumm"); - fn from_slice(_: &[u8]) -> Self { Self } #[cfg(feature = "std")] fn to_raw_vec(&self) -> Vec { vec![] } @@ -996,8 +991,6 @@ mod tests { } impl Derive for TestPublic {} impl Public for TestPublic { - const CRYPTO_ID: CryptoTypeId = CryptoTypeId(*b"test"); - fn from_slice(_bytes: &[u8]) -> Self { Self } diff --git a/primitives/core/src/ecdsa.rs b/primitives/core/src/ecdsa.rs index f06768833fa9b..436dcef9d1521 100644 --- a/primitives/core/src/ecdsa.rs +++ b/primitives/core/src/ecdsa.rs @@ -98,7 +98,6 @@ impl Public { } impl TraitPublic for Public { - const CRYPTO_ID: CryptoTypeId = CRYPTO_ID; /// A new instance from the given slice that should be 33 bytes long. /// /// NOTE: No checking goes on to ensure this is a real public key. Only use it if diff --git a/primitives/core/src/ed25519.rs b/primitives/core/src/ed25519.rs index 88c3389e94021..692a79a135f58 100644 --- a/primitives/core/src/ed25519.rs +++ b/primitives/core/src/ed25519.rs @@ -368,8 +368,6 @@ impl Public { } impl TraitPublic for Public { - const CRYPTO_ID: CryptoTypeId = CRYPTO_ID; - /// A new instance from the given slice that should be 32 bytes long. /// /// NOTE: No checking goes on to ensure this is a real public key. Only use it if diff --git a/primitives/core/src/sr25519.rs b/primitives/core/src/sr25519.rs index 211ea7bc98495..c0231cb498dd4 100644 --- a/primitives/core/src/sr25519.rs +++ b/primitives/core/src/sr25519.rs @@ -382,8 +382,6 @@ impl Public { } impl TraitPublic for Public { - const CRYPTO_ID: CryptoTypeId = CRYPTO_ID; - /// A new instance from the given slice that should be 32 bytes long. /// /// NOTE: No checking goes on to ensure this is a real public key. Only use it if From 69f474e6c14471fbcb8a13728fe27369a754ec8b Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Fri, 21 Feb 2020 16:06:00 +0100 Subject: [PATCH 40/68] Convert pub key to CryptoTypePublicPair --- client/authority-discovery/src/lib.rs | 2 +- client/keystore/src/lib.rs | 4 ++-- client/rpc/src/author/tests.rs | 6 +++--- primitives/application-crypto/test/src/ed25519.rs | 2 +- primitives/application-crypto/test/src/sr25519.rs | 2 +- primitives/core/src/ed25519.rs | 14 +++++++++++++- primitives/core/src/sr25519.rs | 14 +++++++++++++- primitives/core/src/testing.rs | 8 ++++---- primitives/io/src/lib.rs | 6 +++--- 9 files changed, 41 insertions(+), 17 deletions(-) diff --git a/client/authority-discovery/src/lib.rs b/client/authority-discovery/src/lib.rs index 206986493918d..1e44d1f71ef77 100644 --- a/client/authority-discovery/src/lib.rs +++ b/client/authority-discovery/src/lib.rs @@ -214,7 +214,7 @@ where let keys: Vec = self.get_own_public_keys_within_authority_set()? .into_iter() - .map(|k| (sr25519::CRYPTO_ID, k.to_raw_vec())) + .map(Into::into) .collect(); let signatures = self.key_store diff --git a/client/keystore/src/lib.rs b/client/keystore/src/lib.rs index 6ca8428e7cb04..36f296631929b 100644 --- a/client/keystore/src/lib.rs +++ b/client/keystore/src/lib.rs @@ -314,13 +314,13 @@ impl BareCryptoStore for Store { .public_keys_by_type::(id) .map_err(|e| TraitError::from(e))? .into_iter() - .map(|k| (ed25519::CRYPTO_ID, k.to_raw_vec())); + .map(Into::into); let sr25519_existing_keys = self .public_keys_by_type::(id) .map_err(|e| TraitError::from(e))? .into_iter() - .map(|k| (sr25519::CRYPTO_ID, k.to_raw_vec())); + .map(Into::into); Ok(ed25519_existing_keys .chain(sr25519_existing_keys) diff --git a/client/rpc/src/author/tests.rs b/client/rpc/src/author/tests.rs index 8f95b7c9d345c..41ce59308bb84 100644 --- a/client/rpc/src/author/tests.rs +++ b/client/rpc/src/author/tests.rs @@ -217,7 +217,7 @@ fn should_insert_key() { let public_keys = setup.keystore.read().keys(ED25519).unwrap(); - assert!(public_keys.contains(&(ed25519::CRYPTO_ID, key_pair.public().to_raw_vec()))); + assert!(public_keys.contains(key_pair.public().into())); } #[test] @@ -233,8 +233,8 @@ fn should_rotate_keys() { let ed25519_public_keys = setup.keystore.read().keys(ED25519).unwrap(); let sr25519_public_keys = setup.keystore.read().keys(SR25519).unwrap(); - assert!(ed25519_public_keys.contains(&(ed25519::CRYPTO_ID, session_keys.ed25519.to_raw_vec()))); - assert!(sr25519_public_keys.contains(&(sr25519::CRYPTO_ID, session_keys.sr25519.to_raw_vec()))); + assert!(ed25519_public_keys.contains(&session_keys.ed25519.into())); + assert!(sr25519_public_keys.contains(&session_keys.sr25519.into())); } #[test] diff --git a/primitives/application-crypto/test/src/ed25519.rs b/primitives/application-crypto/test/src/ed25519.rs index d9a770873dfe1..8626f89c3c104 100644 --- a/primitives/application-crypto/test/src/ed25519.rs +++ b/primitives/application-crypto/test/src/ed25519.rs @@ -37,6 +37,6 @@ fn ed25519_works_in_runtime() { .expect("Tests `ed25519` crypto."); let supported_keys = keystore.read().keys(ED25519).unwrap(); - assert!(true, supported_keys.contains(&(CRYPTO_ID, public.to_raw_vec()))); + assert!(true, supported_keys.contains(&public.into())); assert!(AppPair::verify(&signature, "ed25519", &AppPublic::from(public))); } diff --git a/primitives/application-crypto/test/src/sr25519.rs b/primitives/application-crypto/test/src/sr25519.rs index c7ccf41a94def..ecc78d3b2a15b 100644 --- a/primitives/application-crypto/test/src/sr25519.rs +++ b/primitives/application-crypto/test/src/sr25519.rs @@ -38,6 +38,6 @@ fn sr25519_works_in_runtime() { .expect("Tests `sr25519` crypto."); let supported_keys = keystore.read().keys(SR25519).unwrap(); - assert!(true, supported_keys.contains(&(CRYPTO_ID, public.to_raw_vec()))); + assert!(true, supported_keys.contains(&public.into())); assert!(AppPair::verify(&signature, "sr25519", &AppPublic::from(public))); } diff --git a/primitives/core/src/ed25519.rs b/primitives/core/src/ed25519.rs index 692a79a135f58..cbb16fe2f4c8a 100644 --- a/primitives/core/src/ed25519.rs +++ b/primitives/core/src/ed25519.rs @@ -33,7 +33,7 @@ use substrate_bip39::seed_from_entropy; #[cfg(feature = "std")] use bip39::{Mnemonic, Language, MnemonicType}; #[cfg(feature = "full_crypto")] -use crate::crypto::{Pair as TraitPair, DeriveJunction, SecretStringError}; +use crate::crypto::{Pair as TraitPair, CryptoTypePublicPair, DeriveJunction, SecretStringError}; #[cfg(feature = "std")] use crate::crypto::Ss58Codec; #[cfg(feature = "std")] @@ -381,6 +381,18 @@ impl TraitPublic for Public { impl Derive for Public {} +impl From for CryptoTypePublicPair { + fn from(key: Public) -> Self { + return key.into() + } +} + +impl From<&Public> for CryptoTypePublicPair { + fn from(key: &Public) -> Self { + return (CRYPTO_ID, key.to_raw_vec()) + } +} + /// Derive a single hard junction. #[cfg(feature = "full_crypto")] fn derive_hard_junction(secret_seed: &Seed, cc: &[u8; 32]) -> Seed { diff --git a/primitives/core/src/sr25519.rs b/primitives/core/src/sr25519.rs index c0231cb498dd4..1abc2df880835 100644 --- a/primitives/core/src/sr25519.rs +++ b/primitives/core/src/sr25519.rs @@ -34,7 +34,7 @@ use substrate_bip39::mini_secret_from_entropy; use bip39::{Mnemonic, Language, MnemonicType}; #[cfg(feature = "full_crypto")] use crate::crypto::{ - Pair as TraitPair, DeriveJunction, Infallible, SecretStringError + Pair as TraitPair, CryptoTypePublicPair, DeriveJunction, Infallible, SecretStringError }; #[cfg(feature = "std")] use crate::crypto::Ss58Codec; @@ -393,6 +393,18 @@ impl TraitPublic for Public { } } +impl From for CryptoTypePublicPair { + fn from(key: Public) -> Self { + return key.into() + } +} + +impl From<&Public> for CryptoTypePublicPair { + fn from(key: &Public) -> Self { + return (CRYPTO_ID, key.to_raw_vec()) + } +} + #[cfg(feature = "std")] impl From for Pair { fn from(sec: MiniSecretKey) -> Pair { diff --git a/primitives/core/src/testing.rs b/primitives/core/src/testing.rs index 212a4859e3c87..c9662a2fdf04f 100644 --- a/primitives/core/src/testing.rs +++ b/primitives/core/src/testing.rs @@ -154,12 +154,12 @@ impl crate::traits::BareCryptoStore for KeyStore { let ed25519_existing_keys = self .ed25519_public_keys(id) .into_iter() - .map(|k| (ed25519::CRYPTO_ID, k.to_raw_vec())); + .map(Into::into); let sr25519_existing_keys = self .sr25519_public_keys(id) .into_iter() - .map(|k| (sr25519::CRYPTO_ID, k.to_raw_vec())); + .map(Into::into); Ok(ed25519_existing_keys .chain(sr25519_existing_keys) @@ -308,7 +308,7 @@ mod tests { let public_keys = store.read().keys(ED25519).unwrap(); - assert!(public_keys.contains(&(ed25519::CRYPTO_ID, public.to_raw_vec()))); + assert!(public_keys.contains(&public.into())); } #[test] @@ -326,6 +326,6 @@ mod tests { let public_keys = store.read().keys(SR25519).unwrap(); - assert!(public_keys.contains(&(sr25519::CRYPTO_ID, key_pair.public().to_raw_vec()))); + assert!(public_keys.contains(&key_pair.public().into())); } } diff --git a/primitives/io/src/lib.rs b/primitives/io/src/lib.rs index f8bf02c61287d..f572c72526f78 100644 --- a/primitives/io/src/lib.rs +++ b/primitives/io/src/lib.rs @@ -33,7 +33,7 @@ use sp_std::ops::Deref; #[cfg(feature = "std")] use sp_core::{ - crypto::{Pair, Public}, + crypto::Pair, traits::{KeystoreExt, CallInWasmExt}, offchain::{OffchainExt, TransactionPoolExt}, hexdisplay::HexDisplay, @@ -410,7 +410,7 @@ pub trait Crypto { self.extension::() .expect("No `keystore` associated for the current context!") .read() - .sign_with(id, &(ed25519::CRYPTO_ID, pub_key.to_raw_vec()), msg) + .sign_with(id, &pub_key.into(), msg) .map(|sig| ed25519::Signature::from_slice(sig.as_slice())) .ok() } @@ -463,7 +463,7 @@ pub trait Crypto { self.extension::() .expect("No `keystore` associated for the current context!") .read() - .sign_with(id, &(sr25519::CRYPTO_ID, pub_key.to_raw_vec()), msg) + .sign_with(id, &pub_key.into(), msg) .map(|sig| sr25519::Signature::from_slice(sig.as_slice())) .ok() } From 164d16b820c238463c285f5d389ac5876b93b5af Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Fri, 21 Feb 2020 16:27:39 +0100 Subject: [PATCH 41/68] Fix use --- primitives/core/src/ed25519.rs | 4 ++-- primitives/core/src/sr25519.rs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/primitives/core/src/ed25519.rs b/primitives/core/src/ed25519.rs index cbb16fe2f4c8a..cc296c43b8bf0 100644 --- a/primitives/core/src/ed25519.rs +++ b/primitives/core/src/ed25519.rs @@ -33,12 +33,12 @@ use substrate_bip39::seed_from_entropy; #[cfg(feature = "std")] use bip39::{Mnemonic, Language, MnemonicType}; #[cfg(feature = "full_crypto")] -use crate::crypto::{Pair as TraitPair, CryptoTypePublicPair, DeriveJunction, SecretStringError}; +use crate::crypto::{Pair as TraitPair, DeriveJunction, SecretStringError}; #[cfg(feature = "std")] use crate::crypto::Ss58Codec; #[cfg(feature = "std")] use serde::{de, Serializer, Serialize, Deserializer, Deserialize}; -use crate::crypto::{Public as TraitPublic, UncheckedFrom, CryptoType, Derive, CryptoTypeId}; +use crate::crypto::{Public as TraitPublic, CryptoTypePublicPair, UncheckedFrom, CryptoType, Derive, CryptoTypeId}; use sp_runtime_interface::pass_by::PassByInner; use sp_std::ops::Deref; diff --git a/primitives/core/src/sr25519.rs b/primitives/core/src/sr25519.rs index 1abc2df880835..719ff47d3e9b3 100644 --- a/primitives/core/src/sr25519.rs +++ b/primitives/core/src/sr25519.rs @@ -34,12 +34,12 @@ use substrate_bip39::mini_secret_from_entropy; use bip39::{Mnemonic, Language, MnemonicType}; #[cfg(feature = "full_crypto")] use crate::crypto::{ - Pair as TraitPair, CryptoTypePublicPair, DeriveJunction, Infallible, SecretStringError + Pair as TraitPair, DeriveJunction, Infallible, SecretStringError }; #[cfg(feature = "std")] use crate::crypto::Ss58Codec; -use crate::crypto::{Public as TraitPublic, UncheckedFrom, CryptoType, Derive, CryptoTypeId}; +use crate::crypto::{Public as TraitPublic, CryptoTypePublicPair, UncheckedFrom, CryptoType, Derive, CryptoTypeId}; use crate::hash::{H256, H512}; use codec::{Encode, Decode}; use sp_std::ops::Deref; From c120f12e592d517d815faf8d889751f634e03237 Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Fri, 21 Feb 2020 16:35:47 +0100 Subject: [PATCH 42/68] Fix code style --- client/authority-discovery/src/error.rs | 2 +- client/keystore/src/lib.rs | 8 ++++---- primitives/core/src/traits.rs | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/client/authority-discovery/src/error.rs b/client/authority-discovery/src/error.rs index 86251a6a58edf..212c701d4f318 100644 --- a/client/authority-discovery/src/error.rs +++ b/client/authority-discovery/src/error.rs @@ -49,5 +49,5 @@ pub enum Error { /// Failed to sign using a specific public key MissingSignature(usize), /// Failed to sign using all public keys - SigningFailed + SigningFailed, } diff --git a/client/keystore/src/lib.rs b/client/keystore/src/lib.rs index 36f296631929b..6773517a1ec25 100644 --- a/client/keystore/src/lib.rs +++ b/client/keystore/src/lib.rs @@ -68,8 +68,8 @@ impl From for TraitError { TraitError::ValidationError(error.to_string()) }, Error::Unavailable => TraitError::Unavailable, - Error::Io(e) => TraitError::Error(e.to_string()), - Error::Json(e) => TraitError::Error(e.to_string()), + Error::Io(e) => TraitError::Other(e.to_string()), + Error::Json(e) => TraitError::Other(e.to_string()), } } } @@ -340,7 +340,7 @@ impl BareCryptoStore for Store { .key_pair_by_type::(&pub_key, id) .map(Into::into) .map_err(|e| TraitError::from(e))?; - return Ok(<[u8; 64]>::from(key_pair.sign(msg)).to_vec()); + Ok(<[u8; 64]>::from(key_pair.sign(msg)).to_vec()) } sr25519::CRYPTO_ID => { let pub_key = sr25519::Public::from_slice(key.1.as_slice()); @@ -348,7 +348,7 @@ impl BareCryptoStore for Store { .key_pair_by_type::(&pub_key, id) .map(Into::into) .map_err(|e| TraitError::from(e))?; - return Ok(<[u8; 64]>::from(key_pair.sign(msg)).to_vec()); + Ok(<[u8; 64]>::from(key_pair.sign(msg)).to_vec()) } _ => Err(TraitError::KeyNotSupported) } diff --git a/primitives/core/src/traits.rs b/primitives/core/src/traits.rs index f4802132eb1b3..410dfda45d09f 100644 --- a/primitives/core/src/traits.rs +++ b/primitives/core/src/traits.rs @@ -42,7 +42,7 @@ pub enum BareCryptoStoreError { /// Keystore unavailable Unavailable, /// Programming errors - Error(String) + Other(String) } /// Something that generates, stores and provides access to keys. From fe8cf28f5e1bfeeeb1dde663cecd590456ec5f7d Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Fri, 21 Feb 2020 17:28:04 +0100 Subject: [PATCH 43/68] Implement From on CryptoTypePublicPair in app_crypto macros --- client/authority-discovery/src/lib.rs | 3 +-- client/keystore/src/lib.rs | 2 +- client/rpc/src/author/tests.rs | 6 +++--- primitives/application-crypto/src/ed25519.rs | 19 +++++++++++++++++++ primitives/application-crypto/src/lib.rs | 2 +- primitives/application-crypto/src/sr25519.rs | 19 +++++++++++++++++++ .../application-crypto/test/src/ed25519.rs | 4 ++-- .../application-crypto/test/src/sr25519.rs | 4 ++-- primitives/authority-discovery/src/lib.rs | 19 ++++++++++++++++++- primitives/core/src/sr25519.rs | 2 +- 10 files changed, 67 insertions(+), 13 deletions(-) diff --git a/client/authority-discovery/src/lib.rs b/client/authority-discovery/src/lib.rs index 1e44d1f71ef77..f5419a483c941 100644 --- a/client/authority-discovery/src/lib.rs +++ b/client/authority-discovery/src/lib.rs @@ -63,8 +63,7 @@ use sc_client_api::blockchain::HeaderBackend; use sc_network::specialization::NetworkSpecialization; use sc_network::{DhtEvent, ExHashT, NetworkStateInfo}; use sp_authority_discovery::{AuthorityDiscoveryApi, AuthorityId, AuthoritySignature, AuthorityPair}; -use sp_core::sr25519; -use sp_core::crypto::{key_types, CryptoTypePublicPair, Pair, Public}; +use sp_core::crypto::{key_types, CryptoTypePublicPair, Pair}; use sp_core::traits::BareCryptoStorePtr; use sp_runtime::{traits::Block as BlockT, generic::BlockId}; use sp_api::ProvideRuntimeApi; diff --git a/client/keystore/src/lib.rs b/client/keystore/src/lib.rs index 6773517a1ec25..a2fe1ff63c0c1 100644 --- a/client/keystore/src/lib.rs +++ b/client/keystore/src/lib.rs @@ -324,7 +324,7 @@ impl BareCryptoStore for Store { Ok(ed25519_existing_keys .chain(sr25519_existing_keys) - .collect::>()) + .collect()) } fn sign_with( diff --git a/client/rpc/src/author/tests.rs b/client/rpc/src/author/tests.rs index 41ce59308bb84..8f95b7c9d345c 100644 --- a/client/rpc/src/author/tests.rs +++ b/client/rpc/src/author/tests.rs @@ -217,7 +217,7 @@ fn should_insert_key() { let public_keys = setup.keystore.read().keys(ED25519).unwrap(); - assert!(public_keys.contains(key_pair.public().into())); + assert!(public_keys.contains(&(ed25519::CRYPTO_ID, key_pair.public().to_raw_vec()))); } #[test] @@ -233,8 +233,8 @@ fn should_rotate_keys() { let ed25519_public_keys = setup.keystore.read().keys(ED25519).unwrap(); let sr25519_public_keys = setup.keystore.read().keys(SR25519).unwrap(); - assert!(ed25519_public_keys.contains(&session_keys.ed25519.into())); - assert!(sr25519_public_keys.contains(&session_keys.sr25519.into())); + assert!(ed25519_public_keys.contains(&(ed25519::CRYPTO_ID, session_keys.ed25519.to_raw_vec()))); + assert!(sr25519_public_keys.contains(&(sr25519::CRYPTO_ID, session_keys.sr25519.to_raw_vec()))); } #[test] diff --git a/primitives/application-crypto/src/ed25519.rs b/primitives/application-crypto/src/ed25519.rs index 414715a10684c..155f274aaee32 100644 --- a/primitives/application-crypto/src/ed25519.rs +++ b/primitives/application-crypto/src/ed25519.rs @@ -19,16 +19,35 @@ use crate::{RuntimePublic, KeyTypeId}; use sp_std::vec::Vec; +use sp_core::crypto::CryptoTypeId; pub use sp_core::ed25519::*; +/// An identifier used to match public keys against ed25519 keys +pub const CRYPTO_ID: CryptoTypeId = CryptoTypeId(*b"ed25"); + mod app { + use sp_core::crypto::{CryptoTypePublicPair, Public as TraitPublic}; use sp_core::testing::ED25519; + use crate::ed25519::CRYPTO_ID; + crate::app_crypto!(super, ED25519); impl crate::traits::BoundToRuntimeAppPublic for Public { type Public = Self; } + + impl From for CryptoTypePublicPair { + fn from(key: Public) -> Self { + return key.into() + } + } + + impl From<&Public> for CryptoTypePublicPair { + fn from(key: &Public) -> Self { + return (CRYPTO_ID, key.to_raw_vec()) + } + } } pub use app::{Public as AppPublic, Signature as AppSignature}; diff --git a/primitives/application-crypto/src/lib.rs b/primitives/application-crypto/src/lib.rs index c5e3ad36ff44a..07e2b45106ee9 100644 --- a/primitives/application-crypto/src/lib.rs +++ b/primitives/application-crypto/src/lib.rs @@ -21,7 +21,7 @@ #![cfg_attr(not(feature = "std"), no_std)] #[doc(hidden)] -pub use sp_core::{self, crypto::{CryptoType, Public, Derive, IsWrappedBy, Wraps}, RuntimeDebug}; +pub use sp_core::{self, crypto::{CryptoType, CryptoTypePublicPair, Public, Derive, IsWrappedBy, Wraps}, RuntimeDebug}; #[doc(hidden)] #[cfg(feature = "full_crypto")] pub use sp_core::crypto::{SecretStringError, DeriveJunction, Ss58Codec, Pair}; diff --git a/primitives/application-crypto/src/sr25519.rs b/primitives/application-crypto/src/sr25519.rs index 59c6f19b6f266..5a6946c7a899d 100644 --- a/primitives/application-crypto/src/sr25519.rs +++ b/primitives/application-crypto/src/sr25519.rs @@ -19,16 +19,35 @@ use crate::{RuntimePublic, KeyTypeId}; use sp_std::vec::Vec; +use sp_core::crypto::CryptoTypeId; pub use sp_core::sr25519::*; +/// An identifier used to match public keys against sr25519 keys +pub const CRYPTO_ID: CryptoTypeId = CryptoTypeId(*b"sr25"); + mod app { + use sp_core::crypto::{CryptoTypePublicPair, Public as TraitPublic}; use sp_core::testing::SR25519; + use crate::sr25519::CRYPTO_ID; + crate::app_crypto!(super, SR25519); impl crate::traits::BoundToRuntimeAppPublic for Public { type Public = Self; } + + impl From for CryptoTypePublicPair { + fn from(key: Public) -> Self { + return key.into() + } + } + + impl From<&Public> for CryptoTypePublicPair { + fn from(key: &Public) -> Self { + return (CRYPTO_ID, key.to_raw_vec()) + } + } } pub use app::{Public as AppPublic, Signature as AppSignature}; diff --git a/primitives/application-crypto/test/src/ed25519.rs b/primitives/application-crypto/test/src/ed25519.rs index 8626f89c3c104..27891269311f9 100644 --- a/primitives/application-crypto/test/src/ed25519.rs +++ b/primitives/application-crypto/test/src/ed25519.rs @@ -18,7 +18,7 @@ use sp_runtime::generic::BlockId; use sp_core::{ - crypto::{Pair, Public}, + crypto::Pair, testing::{KeyStore, ED25519}, }; use substrate_test_runtime_client::{ @@ -26,7 +26,7 @@ use substrate_test_runtime_client::{ runtime::TestAPI, }; use sp_api::ProvideRuntimeApi; -use sp_application_crypto::ed25519::{CRYPTO_ID, AppPair, AppPublic}; +use sp_application_crypto::ed25519::{AppPair, AppPublic}; #[test] fn ed25519_works_in_runtime() { diff --git a/primitives/application-crypto/test/src/sr25519.rs b/primitives/application-crypto/test/src/sr25519.rs index ecc78d3b2a15b..e38b5702126b1 100644 --- a/primitives/application-crypto/test/src/sr25519.rs +++ b/primitives/application-crypto/test/src/sr25519.rs @@ -19,7 +19,7 @@ use sp_runtime::generic::BlockId; use sp_core::{ - crypto::{Pair, Public}, + crypto::Pair, testing::{KeyStore, SR25519}, }; use substrate_test_runtime_client::{ @@ -27,7 +27,7 @@ use substrate_test_runtime_client::{ runtime::TestAPI, }; use sp_api::ProvideRuntimeApi; -use sp_application_crypto::sr25519::{CRYPTO_ID, AppPair, AppPublic}; +use sp_application_crypto::sr25519::{AppPair, AppPublic}; #[test] fn sr25519_works_in_runtime() { diff --git a/primitives/authority-discovery/src/lib.rs b/primitives/authority-discovery/src/lib.rs index 8926825525976..c1389227b6ecf 100644 --- a/primitives/authority-discovery/src/lib.rs +++ b/primitives/authority-discovery/src/lib.rs @@ -21,8 +21,25 @@ use sp_std::vec::Vec; mod app { - use sp_application_crypto::{app_crypto, key_types::AUTHORITY_DISCOVERY, sr25519}; + use sp_application_crypto::{ + CryptoTypePublicPair, + key_types::AUTHORITY_DISCOVERY, + Public as _, + app_crypto, + sr25519}; app_crypto!(sr25519, AUTHORITY_DISCOVERY); + + impl From for CryptoTypePublicPair { + fn from(key: Public) -> Self { + return key.into() + } + } + + impl From<&Public> for CryptoTypePublicPair { + fn from(key: &Public) -> Self { + return (sr25519::CRYPTO_ID, key.to_raw_vec()) + } + } } sp_application_crypto::with_pair! { diff --git a/primitives/core/src/sr25519.rs b/primitives/core/src/sr25519.rs index 719ff47d3e9b3..8ddede758ff10 100644 --- a/primitives/core/src/sr25519.rs +++ b/primitives/core/src/sr25519.rs @@ -601,7 +601,7 @@ impl CryptoType for Pair { #[cfg(test)] mod compatibility_test { use super::*; - use crate::crypto::{DEV_PHRASE}; + use crate::crypto::DEV_PHRASE; use hex_literal::hex; // NOTE: tests to ensure addresses that are created with the `0.1.x` version (pre-audit) are From 19c74995a078fad52eb838b444f1b1e22810d1b3 Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Wed, 26 Feb 2020 14:03:23 +0100 Subject: [PATCH 44/68] Change CryptoTypePublicPair to a struct --- primitives/application-crypto/src/ed25519.rs | 2 +- primitives/application-crypto/src/sr25519.rs | 2 +- primitives/authority-discovery/src/lib.rs | 2 +- primitives/core/src/crypto.rs | 3 ++- primitives/core/src/ed25519.rs | 2 +- primitives/core/src/sr25519.rs | 2 +- 6 files changed, 7 insertions(+), 6 deletions(-) diff --git a/primitives/application-crypto/src/ed25519.rs b/primitives/application-crypto/src/ed25519.rs index 155f274aaee32..8f4fb7d70c3bc 100644 --- a/primitives/application-crypto/src/ed25519.rs +++ b/primitives/application-crypto/src/ed25519.rs @@ -45,7 +45,7 @@ mod app { impl From<&Public> for CryptoTypePublicPair { fn from(key: &Public) -> Self { - return (CRYPTO_ID, key.to_raw_vec()) + return CryptoTypePublicPair(CRYPTO_ID, key.to_raw_vec()) } } } diff --git a/primitives/application-crypto/src/sr25519.rs b/primitives/application-crypto/src/sr25519.rs index 5a6946c7a899d..4c8109ea1fdb3 100644 --- a/primitives/application-crypto/src/sr25519.rs +++ b/primitives/application-crypto/src/sr25519.rs @@ -45,7 +45,7 @@ mod app { impl From<&Public> for CryptoTypePublicPair { fn from(key: &Public) -> Self { - return (CRYPTO_ID, key.to_raw_vec()) + return CryptoTypePublicPair(CRYPTO_ID, key.to_raw_vec()) } } } diff --git a/primitives/authority-discovery/src/lib.rs b/primitives/authority-discovery/src/lib.rs index c1389227b6ecf..0d54fb7a87b5b 100644 --- a/primitives/authority-discovery/src/lib.rs +++ b/primitives/authority-discovery/src/lib.rs @@ -37,7 +37,7 @@ mod app { impl From<&Public> for CryptoTypePublicPair { fn from(key: &Public) -> Self { - return (sr25519::CRYPTO_ID, key.to_raw_vec()) + return CryptoTypePublicPair(sr25519::CRYPTO_ID, key.to_raw_vec()) } } } diff --git a/primitives/core/src/crypto.rs b/primitives/core/src/crypto.rs index 7394ed47d0972..9389dc3bd4d9f 100644 --- a/primitives/core/src/crypto.rs +++ b/primitives/core/src/crypto.rs @@ -925,7 +925,8 @@ impl<'a> TryFrom<&'a str> for KeyTypeId { pub struct CryptoTypeId(pub [u8; 4]); /// A type alias of CryptoTypeId & a public key -pub type CryptoTypePublicPair = (CryptoTypeId, Vec); +#[derive(Debug, Clone, Default, PartialEq, Eq, PartialOrd, Ord, Hash, Encode, Decode)] +pub struct CryptoTypePublicPair(pub CryptoTypeId, pub Vec); /// Known key types; this also functions as a global registry of key types for projects wishing to /// avoid collisions with each other. diff --git a/primitives/core/src/ed25519.rs b/primitives/core/src/ed25519.rs index cc296c43b8bf0..4269e2875c4c8 100644 --- a/primitives/core/src/ed25519.rs +++ b/primitives/core/src/ed25519.rs @@ -389,7 +389,7 @@ impl From for CryptoTypePublicPair { impl From<&Public> for CryptoTypePublicPair { fn from(key: &Public) -> Self { - return (CRYPTO_ID, key.to_raw_vec()) + return CryptoTypePublicPair(CRYPTO_ID, key.to_raw_vec()) } } diff --git a/primitives/core/src/sr25519.rs b/primitives/core/src/sr25519.rs index 8ddede758ff10..08f571864b8b7 100644 --- a/primitives/core/src/sr25519.rs +++ b/primitives/core/src/sr25519.rs @@ -401,7 +401,7 @@ impl From for CryptoTypePublicPair { impl From<&Public> for CryptoTypePublicPair { fn from(key: &Public) -> Self { - return (CRYPTO_ID, key.to_raw_vec()) + return CryptoTypePublicPair(CRYPTO_ID, key.to_raw_vec()) } } From 499c1bb506c8511630d27f00e17ac6aef4622954 Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Wed, 26 Feb 2020 14:03:38 +0100 Subject: [PATCH 45/68] Implement Display on CryptoTypePublicPair --- primitives/core/src/crypto.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/primitives/core/src/crypto.rs b/primitives/core/src/crypto.rs index 9389dc3bd4d9f..d39e7125869ff 100644 --- a/primitives/core/src/crypto.rs +++ b/primitives/core/src/crypto.rs @@ -921,13 +921,19 @@ impl<'a> TryFrom<&'a str> for KeyTypeId { } /// An identifier for a specific cryptographic algorithm used by a key pair -#[derive(Copy, Clone, Default, PartialEq, Eq, PartialOrd, Ord, Hash, Encode, Decode)] +#[derive(Debug, Copy, Clone, Default, PartialEq, Eq, PartialOrd, Ord, Hash, Encode, Decode)] pub struct CryptoTypeId(pub [u8; 4]); /// A type alias of CryptoTypeId & a public key #[derive(Debug, Clone, Default, PartialEq, Eq, PartialOrd, Ord, Hash, Encode, Decode)] pub struct CryptoTypePublicPair(pub CryptoTypeId, pub Vec); +impl std::fmt::Display for CryptoTypePublicPair { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + write!(f, "{:#?}-{:x?}", self.0, self.1) + } +} + /// Known key types; this also functions as a global registry of key types for projects wishing to /// avoid collisions with each other. /// From b42af4191a020bbec606d842b3a237756d46d3a2 Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Wed, 26 Feb 2020 14:03:54 +0100 Subject: [PATCH 46/68] Pass CryptoTypePublicPair to MissingSignature error --- client/authority-discovery/src/error.rs | 6 ++++-- client/authority-discovery/src/lib.rs | 13 +++++-------- primitives/core/src/crypto.rs | 4 ++-- 3 files changed, 11 insertions(+), 12 deletions(-) diff --git a/client/authority-discovery/src/error.rs b/client/authority-discovery/src/error.rs index 212c701d4f318..d73761d944300 100644 --- a/client/authority-discovery/src/error.rs +++ b/client/authority-discovery/src/error.rs @@ -16,6 +16,8 @@ //! Authority discovery errors. +use sp_core::crypto::CryptoTypePublicPair; + /// AuthorityDiscovery Result. pub type Result = std::result::Result; @@ -47,7 +49,7 @@ pub enum Error { /// Failed to parse a libp2p multi address. ParsingMultiaddress(libp2p::core::multiaddr::Error), /// Failed to sign using a specific public key - MissingSignature(usize), + MissingSignature(CryptoTypePublicPair), /// Failed to sign using all public keys - SigningFailed, + Signing, } diff --git a/client/authority-discovery/src/lib.rs b/client/authority-discovery/src/lib.rs index f5419a483c941..5f0757f58e89f 100644 --- a/client/authority-discovery/src/lib.rs +++ b/client/authority-discovery/src/lib.rs @@ -223,18 +223,15 @@ where keys.clone(), serialized_addresses.as_slice(), ) - .map_err(|_| Error::SigningFailed)?; + .map_err(|_| Error::Signing)?; - for (index, (sign_result, key)) in signatures.iter().zip(keys).enumerate() { + for (sign_result, key) in signatures.iter().zip(keys) { let mut signed_addresses = vec![]; - // sign_with_all returns Option where the signature - // is None for a public key that is not supported. + // sign_with_all returns Result signature + // is generated for a public key that is supported. // Verify that all signatures exist for all provided keys. - if sign_result.is_err() { - return Err(Error::MissingSignature(index)); - } - let signature = sign_result.as_ref().unwrap(); + let signature = sign_result.as_ref().map_err(|_| Error::MissingSignature(key.clone()))?; schema::SignedAuthorityAddresses { addresses: serialized_addresses.clone(), signature: Encode::encode(&signature), diff --git a/primitives/core/src/crypto.rs b/primitives/core/src/crypto.rs index d39e7125869ff..fb61f3562fcee 100644 --- a/primitives/core/src/crypto.rs +++ b/primitives/core/src/crypto.rs @@ -928,8 +928,8 @@ pub struct CryptoTypeId(pub [u8; 4]); #[derive(Debug, Clone, Default, PartialEq, Eq, PartialOrd, Ord, Hash, Encode, Decode)] pub struct CryptoTypePublicPair(pub CryptoTypeId, pub Vec); -impl std::fmt::Display for CryptoTypePublicPair { - fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { +impl sp_std::fmt::Display for CryptoTypePublicPair { + fn fmt(&self, f: &mut sp_std::fmt::Formatter) -> sp_std::fmt::Result { write!(f, "{:#?}-{:x?}", self.0, self.1) } } From fc40b1e8a0bc278bb6dc27a61a3e53c7b13eb564 Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Wed, 26 Feb 2020 16:24:48 +0100 Subject: [PATCH 47/68] Adjust docs according to function signature --- primitives/core/src/traits.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/primitives/core/src/traits.rs b/primitives/core/src/traits.rs index 410dfda45d09f..3ed18f1c2c41a 100644 --- a/primitives/core/src/traits.rs +++ b/primitives/core/src/traits.rs @@ -136,8 +136,8 @@ pub trait BareCryptoStore: Send + Sync { /// Provided a list of public keys, sign a message with /// each key given that the key is supported. /// - /// Returns a list of `Option`s each representing the signature of each key. - /// None is return for non-supported keys. + /// Returns a list of `Result`s each representing the signature of each key or + /// a BareCryptoStoreError for non-supported keys. fn sign_with_all( &self, id: KeyTypeId, From bca2c908bd5aacb087b9b5549f5f7a76dd5cb7e0 Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Wed, 26 Feb 2020 16:35:07 +0100 Subject: [PATCH 48/68] Unify keys implementation --- client/keystore/src/lib.rs | 18 ------------------ primitives/core/src/testing.rs | 16 ---------------- primitives/core/src/traits.rs | 17 ++++++++++++++++- 3 files changed, 16 insertions(+), 35 deletions(-) diff --git a/client/keystore/src/lib.rs b/client/keystore/src/lib.rs index a2fe1ff63c0c1..8b195ad9d0cbb 100644 --- a/client/keystore/src/lib.rs +++ b/client/keystore/src/lib.rs @@ -309,24 +309,6 @@ impl BareCryptoStore for Store { Ok(keys.intersection(&all_keys).cloned().collect()) } - fn keys(&self, id: KeyTypeId) -> std::result::Result, TraitError> { - let ed25519_existing_keys = self - .public_keys_by_type::(id) - .map_err(|e| TraitError::from(e))? - .into_iter() - .map(Into::into); - - let sr25519_existing_keys = self - .public_keys_by_type::(id) - .map_err(|e| TraitError::from(e))? - .into_iter() - .map(Into::into); - - Ok(ed25519_existing_keys - .chain(sr25519_existing_keys) - .collect()) - } - fn sign_with( &self, id: KeyTypeId, diff --git a/primitives/core/src/testing.rs b/primitives/core/src/testing.rs index c9662a2fdf04f..b0ab8a6083e55 100644 --- a/primitives/core/src/testing.rs +++ b/primitives/core/src/testing.rs @@ -150,22 +150,6 @@ impl crate::traits::BareCryptoStore for KeyStore { Ok(keys.intersection(&all_keys).cloned().collect()) } - fn keys(&self, id: KeyTypeId) -> std::result::Result, BareCryptoStoreError> { - let ed25519_existing_keys = self - .ed25519_public_keys(id) - .into_iter() - .map(Into::into); - - let sr25519_existing_keys = self - .sr25519_public_keys(id) - .into_iter() - .map(Into::into); - - Ok(ed25519_existing_keys - .chain(sr25519_existing_keys) - .collect::>()) - } - fn sign_with( &self, id: KeyTypeId, diff --git a/primitives/core/src/traits.rs b/primitives/core/src/traits.rs index 3ed18f1c2c41a..8a0126bccd6bf 100644 --- a/primitives/core/src/traits.rs +++ b/primitives/core/src/traits.rs @@ -90,7 +90,22 @@ pub trait BareCryptoStore: Send + Sync { /// List all supported keys /// /// Returns a set of public keys the signer supports. - fn keys(&self, id: KeyTypeId) -> Result, BareCryptoStoreError>; + fn keys(&self, id: KeyTypeId) -> Result, BareCryptoStoreError> { + let ed25519_existing_keys = self + .ed25519_public_keys(id) + .into_iter() + .map(Into::into); + + let sr25519_existing_keys = self + .sr25519_public_keys(id) + .into_iter() + .map(Into::into); + + Ok(ed25519_existing_keys + .chain(sr25519_existing_keys) + .collect::>()) + } + /// Checks if the private keys for the given public key and key type combinations exist. /// /// Returns `true` iff all private keys could be found. From 79b7e23ecc2a33a8b33f0cc9d7c03b8d7ed5f986 Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Wed, 26 Feb 2020 17:05:32 +0100 Subject: [PATCH 49/68] Fix RPC author tests --- client/rpc/src/author/tests.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/client/rpc/src/author/tests.rs b/client/rpc/src/author/tests.rs index 8f95b7c9d345c..a1bb379d69b31 100644 --- a/client/rpc/src/author/tests.rs +++ b/client/rpc/src/author/tests.rs @@ -217,7 +217,7 @@ fn should_insert_key() { let public_keys = setup.keystore.read().keys(ED25519).unwrap(); - assert!(public_keys.contains(&(ed25519::CRYPTO_ID, key_pair.public().to_raw_vec()))); + assert!(public_keys.contains(&CryptoTypePublicPair(ed25519::CRYPTO_ID, key_pair.public().to_raw_vec()))); } #[test] @@ -233,8 +233,8 @@ fn should_rotate_keys() { let ed25519_public_keys = setup.keystore.read().keys(ED25519).unwrap(); let sr25519_public_keys = setup.keystore.read().keys(SR25519).unwrap(); - assert!(ed25519_public_keys.contains(&(ed25519::CRYPTO_ID, session_keys.ed25519.to_raw_vec()))); - assert!(sr25519_public_keys.contains(&(sr25519::CRYPTO_ID, session_keys.sr25519.to_raw_vec()))); + assert!(ed25519_public_keys.contains(&CryptoTypePublicPair(ed25519::CRYPTO_ID, session_keys.ed25519.to_raw_vec()))); + assert!(sr25519_public_keys.contains(&CryptoTypePublicPair(sr25519::CRYPTO_ID, session_keys.sr25519.to_raw_vec()))); } #[test] From 31ae7ce8c70cdc63d4e857133de09739f2ba897e Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Wed, 26 Feb 2020 17:30:41 +0100 Subject: [PATCH 50/68] Fix stackoverflow --- client/rpc/src/author/tests.rs | 3 ++- primitives/application-crypto/src/ed25519.rs | 4 ++-- primitives/application-crypto/src/sr25519.rs | 4 ++-- primitives/authority-discovery/src/lib.rs | 4 ++-- primitives/core/src/ed25519.rs | 4 ++-- primitives/core/src/sr25519.rs | 4 ++-- 6 files changed, 12 insertions(+), 11 deletions(-) diff --git a/client/rpc/src/author/tests.rs b/client/rpc/src/author/tests.rs index a1bb379d69b31..c1076af5f4047 100644 --- a/client/rpc/src/author/tests.rs +++ b/client/rpc/src/author/tests.rs @@ -21,7 +21,8 @@ use assert_matches::assert_matches; use codec::Encode; use sp_core::{ H256, blake2_256, hexdisplay::HexDisplay, testing::{ED25519, SR25519, KeyStore}, - traits::BareCryptoStorePtr, ed25519, sr25519, crypto::{Pair, Public}, + traits::BareCryptoStorePtr, ed25519, sr25519, + crypto::{CryptoTypePublicPair, Pair, Public}, }; use rpc::futures::Stream as _; use substrate_test_runtime_client::{ diff --git a/primitives/application-crypto/src/ed25519.rs b/primitives/application-crypto/src/ed25519.rs index 8f4fb7d70c3bc..bf2137bf88379 100644 --- a/primitives/application-crypto/src/ed25519.rs +++ b/primitives/application-crypto/src/ed25519.rs @@ -39,13 +39,13 @@ mod app { impl From for CryptoTypePublicPair { fn from(key: Public) -> Self { - return key.into() + (&key).into() } } impl From<&Public> for CryptoTypePublicPair { fn from(key: &Public) -> Self { - return CryptoTypePublicPair(CRYPTO_ID, key.to_raw_vec()) + CryptoTypePublicPair(CRYPTO_ID, key.to_raw_vec()) } } } diff --git a/primitives/application-crypto/src/sr25519.rs b/primitives/application-crypto/src/sr25519.rs index 4c8109ea1fdb3..6a8c7f168a403 100644 --- a/primitives/application-crypto/src/sr25519.rs +++ b/primitives/application-crypto/src/sr25519.rs @@ -39,13 +39,13 @@ mod app { impl From for CryptoTypePublicPair { fn from(key: Public) -> Self { - return key.into() + (&key).into() } } impl From<&Public> for CryptoTypePublicPair { fn from(key: &Public) -> Self { - return CryptoTypePublicPair(CRYPTO_ID, key.to_raw_vec()) + CryptoTypePublicPair(CRYPTO_ID, key.to_raw_vec()) } } } diff --git a/primitives/authority-discovery/src/lib.rs b/primitives/authority-discovery/src/lib.rs index 0d54fb7a87b5b..bb0af2366e82f 100644 --- a/primitives/authority-discovery/src/lib.rs +++ b/primitives/authority-discovery/src/lib.rs @@ -31,13 +31,13 @@ mod app { impl From for CryptoTypePublicPair { fn from(key: Public) -> Self { - return key.into() + (&key).into() } } impl From<&Public> for CryptoTypePublicPair { fn from(key: &Public) -> Self { - return CryptoTypePublicPair(sr25519::CRYPTO_ID, key.to_raw_vec()) + CryptoTypePublicPair(sr25519::CRYPTO_ID, key.to_raw_vec()) } } } diff --git a/primitives/core/src/ed25519.rs b/primitives/core/src/ed25519.rs index 4269e2875c4c8..abeac05388d27 100644 --- a/primitives/core/src/ed25519.rs +++ b/primitives/core/src/ed25519.rs @@ -383,13 +383,13 @@ impl Derive for Public {} impl From for CryptoTypePublicPair { fn from(key: Public) -> Self { - return key.into() + (&key).into() } } impl From<&Public> for CryptoTypePublicPair { fn from(key: &Public) -> Self { - return CryptoTypePublicPair(CRYPTO_ID, key.to_raw_vec()) + CryptoTypePublicPair(CRYPTO_ID, key.to_raw_vec()) } } diff --git a/primitives/core/src/sr25519.rs b/primitives/core/src/sr25519.rs index 08f571864b8b7..98f840548d657 100644 --- a/primitives/core/src/sr25519.rs +++ b/primitives/core/src/sr25519.rs @@ -395,13 +395,13 @@ impl TraitPublic for Public { impl From for CryptoTypePublicPair { fn from(key: Public) -> Self { - return key.into() + (&key).into() } } impl From<&Public> for CryptoTypePublicPair { fn from(key: &Public) -> Self { - return CryptoTypePublicPair(CRYPTO_ID, key.to_raw_vec()) + CryptoTypePublicPair(CRYPTO_ID, key.to_raw_vec()) } } From 5de1d0460c6d00b55ec3f215c21cbdeb7c4f18ee Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Fri, 13 Mar 2020 12:52:17 +0100 Subject: [PATCH 51/68] Tabify spaces --- client/rpc/src/author/tests.rs | 6 ++-- primitives/application-crypto/src/ed25519.rs | 26 ++++++++--------- primitives/application-crypto/src/sr25519.rs | 26 ++++++++--------- primitives/authority-discovery/src/lib.rs | 30 ++++++++++---------- 4 files changed, 44 insertions(+), 44 deletions(-) diff --git a/client/rpc/src/author/tests.rs b/client/rpc/src/author/tests.rs index c1076af5f4047..4849acd3be3ad 100644 --- a/client/rpc/src/author/tests.rs +++ b/client/rpc/src/author/tests.rs @@ -22,7 +22,7 @@ use codec::Encode; use sp_core::{ H256, blake2_256, hexdisplay::HexDisplay, testing::{ED25519, SR25519, KeyStore}, traits::BareCryptoStorePtr, ed25519, sr25519, - crypto::{CryptoTypePublicPair, Pair, Public}, + crypto::{CryptoTypePublicPair, Pair, Public}, }; use rpc::futures::Stream as _; use substrate_test_runtime_client::{ @@ -174,7 +174,7 @@ fn should_return_pending_extrinsics() { let ex = uxt(AccountKeyring::Alice, 0); AuthorApi::submit_extrinsic(&p, ex.encode().into()).wait().unwrap(); - assert_matches!( + assert_matches!( p.pending_extrinsics(), Ok(ref expected) if *expected == vec![Bytes(ex.encode())] ); @@ -200,7 +200,7 @@ fn should_remove_extrinsics() { hash::ExtrinsicOrHash::Extrinsic(ex1.encode().into()), ]).unwrap(); - assert_eq!(removed.len(), 3); + assert_eq!(removed.len(), 3); } #[test] diff --git a/primitives/application-crypto/src/ed25519.rs b/primitives/application-crypto/src/ed25519.rs index bf2137bf88379..2b1fe2c1f6e42 100644 --- a/primitives/application-crypto/src/ed25519.rs +++ b/primitives/application-crypto/src/ed25519.rs @@ -27,9 +27,9 @@ pub use sp_core::ed25519::*; pub const CRYPTO_ID: CryptoTypeId = CryptoTypeId(*b"ed25"); mod app { - use sp_core::crypto::{CryptoTypePublicPair, Public as TraitPublic}; + use sp_core::crypto::{CryptoTypePublicPair, Public as TraitPublic}; use sp_core::testing::ED25519; - use crate::ed25519::CRYPTO_ID; + use crate::ed25519::CRYPTO_ID; crate::app_crypto!(super, ED25519); @@ -37,17 +37,17 @@ mod app { type Public = Self; } - impl From for CryptoTypePublicPair { - fn from(key: Public) -> Self { - (&key).into() - } - } - - impl From<&Public> for CryptoTypePublicPair { - fn from(key: &Public) -> Self { - CryptoTypePublicPair(CRYPTO_ID, key.to_raw_vec()) - } - } + impl From for CryptoTypePublicPair { + fn from(key: Public) -> Self { + (&key).into() + } + } + + impl From<&Public> for CryptoTypePublicPair { + fn from(key: &Public) -> Self { + CryptoTypePublicPair(CRYPTO_ID, key.to_raw_vec()) + } + } } pub use app::{Public as AppPublic, Signature as AppSignature}; diff --git a/primitives/application-crypto/src/sr25519.rs b/primitives/application-crypto/src/sr25519.rs index 6a8c7f168a403..a9190ac3b0fb8 100644 --- a/primitives/application-crypto/src/sr25519.rs +++ b/primitives/application-crypto/src/sr25519.rs @@ -27,9 +27,9 @@ pub use sp_core::sr25519::*; pub const CRYPTO_ID: CryptoTypeId = CryptoTypeId(*b"sr25"); mod app { - use sp_core::crypto::{CryptoTypePublicPair, Public as TraitPublic}; + use sp_core::crypto::{CryptoTypePublicPair, Public as TraitPublic}; use sp_core::testing::SR25519; - use crate::sr25519::CRYPTO_ID; + use crate::sr25519::CRYPTO_ID; crate::app_crypto!(super, SR25519); @@ -37,17 +37,17 @@ mod app { type Public = Self; } - impl From for CryptoTypePublicPair { - fn from(key: Public) -> Self { - (&key).into() - } - } - - impl From<&Public> for CryptoTypePublicPair { - fn from(key: &Public) -> Self { - CryptoTypePublicPair(CRYPTO_ID, key.to_raw_vec()) - } - } + impl From for CryptoTypePublicPair { + fn from(key: Public) -> Self { + (&key).into() + } + } + + impl From<&Public> for CryptoTypePublicPair { + fn from(key: &Public) -> Self { + CryptoTypePublicPair(CRYPTO_ID, key.to_raw_vec()) + } + } } pub use app::{Public as AppPublic, Signature as AppSignature}; diff --git a/primitives/authority-discovery/src/lib.rs b/primitives/authority-discovery/src/lib.rs index bb0af2366e82f..68680ad759465 100644 --- a/primitives/authority-discovery/src/lib.rs +++ b/primitives/authority-discovery/src/lib.rs @@ -22,24 +22,24 @@ use sp_std::vec::Vec; mod app { use sp_application_crypto::{ - CryptoTypePublicPair, - key_types::AUTHORITY_DISCOVERY, - Public as _, - app_crypto, - sr25519}; + CryptoTypePublicPair, + key_types::AUTHORITY_DISCOVERY, + Public as _, + app_crypto, + sr25519}; app_crypto!(sr25519, AUTHORITY_DISCOVERY); - impl From for CryptoTypePublicPair { - fn from(key: Public) -> Self { - (&key).into() - } - } + impl From for CryptoTypePublicPair { + fn from(key: Public) -> Self { + (&key).into() + } + } - impl From<&Public> for CryptoTypePublicPair { - fn from(key: &Public) -> Self { - CryptoTypePublicPair(sr25519::CRYPTO_ID, key.to_raw_vec()) - } - } + impl From<&Public> for CryptoTypePublicPair { + fn from(key: &Public) -> Self { + CryptoTypePublicPair(sr25519::CRYPTO_ID, key.to_raw_vec()) + } + } } sp_application_crypto::with_pair! { From 47f6c084be27460ebf14a8031740900ff13778d6 Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Fri, 13 Mar 2020 13:16:50 +0100 Subject: [PATCH 52/68] Pass KeyTypeId to error for easier debugging --- client/keystore/src/lib.rs | 6 +++--- primitives/core/src/testing.rs | 2 +- primitives/core/src/traits.rs | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/client/keystore/src/lib.rs b/client/keystore/src/lib.rs index 8b195ad9d0cbb..16394eec4137c 100644 --- a/client/keystore/src/lib.rs +++ b/client/keystore/src/lib.rs @@ -47,7 +47,7 @@ pub enum Error { InvalidSeed, /// Public key type is not supported #[display(fmt="Key crypto type is not supported")] - KeyNotSupported, + KeyNotSupported(KeyTypeId), /// Pair not found for public key and KeyTypeId #[display(fmt="Pair not found for {} public key", "_0")] PairNotFound(String), @@ -62,7 +62,7 @@ pub type Result = std::result::Result; impl From for TraitError { fn from(error: Error) -> Self { match error { - Error::KeyNotSupported => TraitError::KeyNotSupported, + Error::KeyNotSupported(id) => TraitError::KeyNotSupported(id), Error::PairNotFound(e) => TraitError::PairNotFound(e), Error::InvalidSeed | Error::InvalidPhrase | Error::InvalidPassword => { TraitError::ValidationError(error.to_string()) @@ -332,7 +332,7 @@ impl BareCryptoStore for Store { .map_err(|e| TraitError::from(e))?; Ok(<[u8; 64]>::from(key_pair.sign(msg)).to_vec()) } - _ => Err(TraitError::KeyNotSupported) + _ => Err(TraitError::KeyNotSupported(id)) } } diff --git a/primitives/core/src/testing.rs b/primitives/core/src/testing.rs index b0ab8a6083e55..1e622f41ef576 100644 --- a/primitives/core/src/testing.rs +++ b/primitives/core/src/testing.rs @@ -171,7 +171,7 @@ impl crate::traits::BareCryptoStore for KeyStore { .ok_or(BareCryptoStoreError::PairNotFound("sr25519".to_owned()))?; return Ok(<[u8; 64]>::from(key_pair.sign(msg)).to_vec()); } - _ => Err(BareCryptoStoreError::KeyNotSupported) + _ => Err(BareCryptoStoreError::KeyNotSupported(id)) } } } diff --git a/primitives/core/src/traits.rs b/primitives/core/src/traits.rs index 8a0126bccd6bf..a5647612dcf72 100644 --- a/primitives/core/src/traits.rs +++ b/primitives/core/src/traits.rs @@ -34,7 +34,7 @@ pub use sp_externalities::{Externalities, ExternalitiesExt}; #[derive(Debug)] pub enum BareCryptoStoreError { /// Public key type is not supported - KeyNotSupported, + KeyNotSupported(KeyTypeId), /// Pair not found for public key and KeyTypeId PairNotFound(String), /// Validation error @@ -143,7 +143,7 @@ pub trait BareCryptoStore: Send + Sync { } } } - Err(BareCryptoStoreError::KeyNotSupported) + Err(BareCryptoStoreError::KeyNotSupported(id)) } /// Sign with all keys From 167464578b06b583df1f09d70740f4c8336703e8 Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Fri, 13 Mar 2020 14:50:53 +0100 Subject: [PATCH 53/68] Fix asserts --- primitives/application-crypto/test/src/ed25519.rs | 2 +- primitives/application-crypto/test/src/sr25519.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/primitives/application-crypto/test/src/ed25519.rs b/primitives/application-crypto/test/src/ed25519.rs index 27891269311f9..7609b64e81275 100644 --- a/primitives/application-crypto/test/src/ed25519.rs +++ b/primitives/application-crypto/test/src/ed25519.rs @@ -37,6 +37,6 @@ fn ed25519_works_in_runtime() { .expect("Tests `ed25519` crypto."); let supported_keys = keystore.read().keys(ED25519).unwrap(); - assert!(true, supported_keys.contains(&public.into())); + assert!(supported_keys.contains(&public.into())); assert!(AppPair::verify(&signature, "ed25519", &AppPublic::from(public))); } diff --git a/primitives/application-crypto/test/src/sr25519.rs b/primitives/application-crypto/test/src/sr25519.rs index e38b5702126b1..0c855fd09c3bd 100644 --- a/primitives/application-crypto/test/src/sr25519.rs +++ b/primitives/application-crypto/test/src/sr25519.rs @@ -38,6 +38,6 @@ fn sr25519_works_in_runtime() { .expect("Tests `sr25519` crypto."); let supported_keys = keystore.read().keys(SR25519).unwrap(); - assert!(true, supported_keys.contains(&public.into())); + assert!(supported_keys.contains(&public.into())); assert!(AppPair::verify(&signature, "sr25519", &AppPublic::from(public))); } From 94651faee430a377270f8fdb5eede9af8e05bb45 Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Fri, 13 Mar 2020 14:51:14 +0100 Subject: [PATCH 54/68] Use ToHex to format public key --- primitives/core/src/crypto.rs | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/primitives/core/src/crypto.rs b/primitives/core/src/crypto.rs index fb61f3562fcee..e112b9368c577 100644 --- a/primitives/core/src/crypto.rs +++ b/primitives/core/src/crypto.rs @@ -17,9 +17,11 @@ // tag::description[] //! Cryptographic utilities. // end::description[] +use rustc_hex::ToHex; use sp_std::hash::Hash; use sp_std::vec::Vec; +use sp_std::str; #[cfg(feature = "std")] use sp_std::convert::TryInto; use sp_std::convert::TryFrom; @@ -929,9 +931,15 @@ pub struct CryptoTypeId(pub [u8; 4]); pub struct CryptoTypePublicPair(pub CryptoTypeId, pub Vec); impl sp_std::fmt::Display for CryptoTypePublicPair { - fn fmt(&self, f: &mut sp_std::fmt::Formatter) -> sp_std::fmt::Result { - write!(f, "{:#?}-{:x?}", self.0, self.1) - } + fn fmt(&self, f: &mut sp_std::fmt::Formatter) -> sp_std::fmt::Result { + let id = match str::from_utf8(&(self.0).0[..]) { + Ok(id) => id.to_string(), + Err(_) => { + format!("{:#?}", self.0) + } + }; + write!(f, "{}-{}", id, self.1.to_hex::()) + } } /// Known key types; this also functions as a global registry of key types for projects wishing to From c71646a668b7ab45144f5e84766668750dfeb0db Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Fri, 13 Mar 2020 15:01:11 +0100 Subject: [PATCH 55/68] Use constants from sp_core --- primitives/application-crypto/src/ed25519.rs | 6 +----- primitives/application-crypto/src/sr25519.rs | 6 +----- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/primitives/application-crypto/src/ed25519.rs b/primitives/application-crypto/src/ed25519.rs index 2b1fe2c1f6e42..5be79ff4f7985 100644 --- a/primitives/application-crypto/src/ed25519.rs +++ b/primitives/application-crypto/src/ed25519.rs @@ -19,17 +19,13 @@ use crate::{RuntimePublic, KeyTypeId}; use sp_std::vec::Vec; -use sp_core::crypto::CryptoTypeId; pub use sp_core::ed25519::*; -/// An identifier used to match public keys against ed25519 keys -pub const CRYPTO_ID: CryptoTypeId = CryptoTypeId(*b"ed25"); - mod app { use sp_core::crypto::{CryptoTypePublicPair, Public as TraitPublic}; use sp_core::testing::ED25519; - use crate::ed25519::CRYPTO_ID; + use sp_core::ed25519::CRYPTO_ID; crate::app_crypto!(super, ED25519); diff --git a/primitives/application-crypto/src/sr25519.rs b/primitives/application-crypto/src/sr25519.rs index a9190ac3b0fb8..a0f2cef1c4e45 100644 --- a/primitives/application-crypto/src/sr25519.rs +++ b/primitives/application-crypto/src/sr25519.rs @@ -19,17 +19,13 @@ use crate::{RuntimePublic, KeyTypeId}; use sp_std::vec::Vec; -use sp_core::crypto::CryptoTypeId; pub use sp_core::sr25519::*; -/// An identifier used to match public keys against sr25519 keys -pub const CRYPTO_ID: CryptoTypeId = CryptoTypeId(*b"sr25"); - mod app { use sp_core::crypto::{CryptoTypePublicPair, Public as TraitPublic}; use sp_core::testing::SR25519; - use crate::sr25519::CRYPTO_ID; + use sp_core::sr25519::CRYPTO_ID; crate::app_crypto!(super, SR25519); From 46d6f236b964e1ef61d80f2d2fcbe64a6b6bd0ac Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Fri, 13 Mar 2020 15:01:26 +0100 Subject: [PATCH 56/68] Rename testing KeyTypeId constants --- primitives/core/src/testing.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/primitives/core/src/testing.rs b/primitives/core/src/testing.rs index 1e622f41ef576..184be87fd3983 100644 --- a/primitives/core/src/testing.rs +++ b/primitives/core/src/testing.rs @@ -27,9 +27,9 @@ use crate::{ use std::collections::HashSet; /// Key type for generic Ed25519 key. -pub const ED25519: KeyTypeId = KeyTypeId(*b"ed25"); +pub const ED25519_TEST: KeyTypeId = KeyTypeId(*b"ed25"); /// Key type for generic Sr 25519 key. -pub const SR25519: KeyTypeId = KeyTypeId(*b"sr25"); +pub const SR25519_TEST: KeyTypeId = KeyTypeId(*b"sr25"); /// A keystore implementation usable in tests. #[cfg(feature = "std")] @@ -280,17 +280,17 @@ macro_rules! wasm_export_functions { mod tests { use super::*; use crate::sr25519; - use crate::testing::{ED25519, SR25519}; + use crate::testing::{ED25519_TEST, SR25519_TEST}; #[test] fn store_key_and_extract() { let store = KeyStore::new(); let public = store.write() - .ed25519_generate_new(ED25519, None) + .ed25519_generate_new(ED25519_TEST, None) .expect("Generates key"); - let public_keys = store.read().keys(ED25519).unwrap(); + let public_keys = store.read().keys(ED25519_TEST).unwrap(); assert!(public_keys.contains(&public.into())); } @@ -303,12 +303,12 @@ mod tests { let key_pair = sr25519::Pair::from_string(secret_uri, None).expect("Generates key pair"); store.write().insert_unknown( - SR25519, + SR25519_TEST, secret_uri, key_pair.public().as_ref(), ).expect("Inserts unknown key"); - let public_keys = store.read().keys(SR25519).unwrap(); + let public_keys = store.read().keys(SR25519_TEST).unwrap(); assert!(public_keys.contains(&key_pair.public().into())); } From bc0880b7cbaba301f3eaba0c945a4ba82a8bfbe3 Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Fri, 13 Mar 2020 15:56:35 +0100 Subject: [PATCH 57/68] Please compiler --- primitives/core/src/crypto.rs | 1 + primitives/core/src/traits.rs | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/primitives/core/src/crypto.rs b/primitives/core/src/crypto.rs index bda42cc5d1b6d..0045cc4d4b616 100644 --- a/primitives/core/src/crypto.rs +++ b/primitives/core/src/crypto.rs @@ -953,6 +953,7 @@ pub struct CryptoTypeId(pub [u8; 4]); #[derive(Debug, Clone, Default, PartialEq, Eq, PartialOrd, Ord, Hash, Encode, Decode)] pub struct CryptoTypePublicPair(pub CryptoTypeId, pub Vec); +#[cfg(feature = "std")] impl sp_std::fmt::Display for CryptoTypePublicPair { fn fmt(&self, f: &mut sp_std::fmt::Formatter) -> sp_std::fmt::Result { let id = match str::from_utf8(&(self.0).0[..]) { diff --git a/primitives/core/src/traits.rs b/primitives/core/src/traits.rs index 2b801ab199c05..fc1faf6082b33 100644 --- a/primitives/core/src/traits.rs +++ b/primitives/core/src/traits.rs @@ -22,7 +22,7 @@ use crate::{ }; use std::{ - borrow:Cow, + borrow::Cow, collections::HashSet, fmt::{Debug, Display}, panic::UnwindSafe, From c8b721cf3f0c3d9cc26c7e0cc35af96b8ecc10f6 Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Fri, 13 Mar 2020 16:03:40 +0100 Subject: [PATCH 58/68] Restore KeyTypeId names apparently, they're not only used in tests --- primitives/core/src/testing.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/primitives/core/src/testing.rs b/primitives/core/src/testing.rs index 184be87fd3983..1e622f41ef576 100644 --- a/primitives/core/src/testing.rs +++ b/primitives/core/src/testing.rs @@ -27,9 +27,9 @@ use crate::{ use std::collections::HashSet; /// Key type for generic Ed25519 key. -pub const ED25519_TEST: KeyTypeId = KeyTypeId(*b"ed25"); +pub const ED25519: KeyTypeId = KeyTypeId(*b"ed25"); /// Key type for generic Sr 25519 key. -pub const SR25519_TEST: KeyTypeId = KeyTypeId(*b"sr25"); +pub const SR25519: KeyTypeId = KeyTypeId(*b"sr25"); /// A keystore implementation usable in tests. #[cfg(feature = "std")] @@ -280,17 +280,17 @@ macro_rules! wasm_export_functions { mod tests { use super::*; use crate::sr25519; - use crate::testing::{ED25519_TEST, SR25519_TEST}; + use crate::testing::{ED25519, SR25519}; #[test] fn store_key_and_extract() { let store = KeyStore::new(); let public = store.write() - .ed25519_generate_new(ED25519_TEST, None) + .ed25519_generate_new(ED25519, None) .expect("Generates key"); - let public_keys = store.read().keys(ED25519_TEST).unwrap(); + let public_keys = store.read().keys(ED25519).unwrap(); assert!(public_keys.contains(&public.into())); } @@ -303,12 +303,12 @@ mod tests { let key_pair = sr25519::Pair::from_string(secret_uri, None).expect("Generates key pair"); store.write().insert_unknown( - SR25519_TEST, + SR25519, secret_uri, key_pair.public().as_ref(), ).expect("Inserts unknown key"); - let public_keys = store.read().keys(SR25519_TEST).unwrap(); + let public_keys = store.read().keys(SR25519).unwrap(); assert!(public_keys.contains(&key_pair.public().into())); } From fbfd23bfe439c414e38ac44185fece0e455e19da Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Fri, 13 Mar 2020 18:28:09 +0100 Subject: [PATCH 59/68] Use BareCryptoStoreError instead of String --- client/keystore/src/lib.rs | 8 ++++---- primitives/core/src/testing.rs | 10 ++++++---- primitives/core/src/traits.rs | 4 ++-- 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/client/keystore/src/lib.rs b/client/keystore/src/lib.rs index 16394eec4137c..dff9c2ba39fd6 100644 --- a/client/keystore/src/lib.rs +++ b/client/keystore/src/lib.rs @@ -345,11 +345,11 @@ impl BareCryptoStore for Store { &mut self, id: KeyTypeId, seed: Option<&str>, - ) -> std::result::Result { + ) -> std::result::Result { let pair = match seed { Some(seed) => self.insert_ephemeral_from_seed_by_type::(seed, id), None => self.generate_by_type::(id), - }.map_err(|e| e.to_string())?; + }.map_err(|e| -> TraitError { e.into() })?; Ok(pair.public()) } @@ -362,11 +362,11 @@ impl BareCryptoStore for Store { &mut self, id: KeyTypeId, seed: Option<&str>, - ) -> std::result::Result { + ) -> std::result::Result { let pair = match seed { Some(seed) => self.insert_ephemeral_from_seed_by_type::(seed, id), None => self.generate_by_type::(id), - }.map_err(|e| e.to_string())?; + }.map_err(|e| -> TraitError { e.into() })?; Ok(pair.public()) } diff --git a/primitives/core/src/testing.rs b/primitives/core/src/testing.rs index 1e622f41ef576..ce93feb4210ee 100644 --- a/primitives/core/src/testing.rs +++ b/primitives/core/src/testing.rs @@ -81,10 +81,11 @@ impl crate::traits::BareCryptoStore for KeyStore { &mut self, id: KeyTypeId, seed: Option<&str>, - ) -> Result { + ) -> Result { match seed { Some(seed) => { - let pair = sr25519::Pair::from_string(seed, None).expect("Generates an `sr25519` pair."); + let pair = sr25519::Pair::from_string(seed, None) + .map_err(|_| BareCryptoStoreError::ValidationError("Generates an `sr25519` pair.".to_owned()))?; self.keys.entry(id).or_default().insert(pair.public().to_raw_vec(), seed.into()); Ok(pair.public()) }, @@ -111,10 +112,11 @@ impl crate::traits::BareCryptoStore for KeyStore { &mut self, id: KeyTypeId, seed: Option<&str>, - ) -> Result { + ) -> Result { match seed { Some(seed) => { - let pair = ed25519::Pair::from_string(seed, None).expect("Generates an `ed25519` pair."); + let pair = ed25519::Pair::from_string(seed, None) + .map_err(|_| BareCryptoStoreError::ValidationError("Generates an `ed25519` pair.".to_owned()))?; self.keys.entry(id).or_default().insert(pair.public().to_raw_vec(), seed.into()); Ok(pair.public()) }, diff --git a/primitives/core/src/traits.rs b/primitives/core/src/traits.rs index fc1faf6082b33..5e6a0f2816688 100644 --- a/primitives/core/src/traits.rs +++ b/primitives/core/src/traits.rs @@ -59,7 +59,7 @@ pub trait BareCryptoStore: Send + Sync { &mut self, id: KeyTypeId, seed: Option<&str>, - ) -> Result; + ) -> Result; /// Returns all ed25519 public keys for the given key type. fn ed25519_public_keys(&self, id: KeyTypeId) -> Vec; /// Generate a new ed25519 key pair for the given key type and an optional seed. @@ -71,7 +71,7 @@ pub trait BareCryptoStore: Send + Sync { &mut self, id: KeyTypeId, seed: Option<&str>, - ) -> Result; + ) -> Result; /// Insert a new key. This doesn't require any known of the crypto; but a public key must be /// manually provided. From cac19adfb1d47444083a4ec25b8c66cd5c0230e2 Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Fri, 13 Mar 2020 18:49:51 +0100 Subject: [PATCH 60/68] Document return value --- primitives/core/src/traits.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/primitives/core/src/traits.rs b/primitives/core/src/traits.rs index 5e6a0f2816688..ac4ba70a163c7 100644 --- a/primitives/core/src/traits.rs +++ b/primitives/core/src/traits.rs @@ -116,6 +116,9 @@ pub trait BareCryptoStore: Send + Sync { /// /// Signs a message with the private key that matches /// the public key passed. + /// + /// Returns the signature if key is found & supported, + /// an error otherwise. fn sign_with( &self, id: KeyTypeId, From d5f4c4b51b5df8c481d9ed3cbfc6a49fdbe3eec7 Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Fri, 13 Mar 2020 19:00:16 +0100 Subject: [PATCH 61/68] Fix borrow check --- primitives/application-crypto/test/src/ed25519.rs | 2 +- primitives/application-crypto/test/src/sr25519.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/primitives/application-crypto/test/src/ed25519.rs b/primitives/application-crypto/test/src/ed25519.rs index 7609b64e81275..1d72962829a53 100644 --- a/primitives/application-crypto/test/src/ed25519.rs +++ b/primitives/application-crypto/test/src/ed25519.rs @@ -37,6 +37,6 @@ fn ed25519_works_in_runtime() { .expect("Tests `ed25519` crypto."); let supported_keys = keystore.read().keys(ED25519).unwrap(); - assert!(supported_keys.contains(&public.into())); + assert!(supported_keys.contains(&public.clone().into())); assert!(AppPair::verify(&signature, "ed25519", &AppPublic::from(public))); } diff --git a/primitives/application-crypto/test/src/sr25519.rs b/primitives/application-crypto/test/src/sr25519.rs index 0c855fd09c3bd..f2c7c48b2bc91 100644 --- a/primitives/application-crypto/test/src/sr25519.rs +++ b/primitives/application-crypto/test/src/sr25519.rs @@ -38,6 +38,6 @@ fn sr25519_works_in_runtime() { .expect("Tests `sr25519` crypto."); let supported_keys = keystore.read().keys(SR25519).unwrap(); - assert!(supported_keys.contains(&public.into())); + assert!(supported_keys.contains(&public.clone().into())); assert!(AppPair::verify(&signature, "sr25519", &AppPublic::from(public))); } From c007d133827b468e878e0cc35f963c0a271a184a Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Fri, 20 Mar 2020 13:05:01 +0100 Subject: [PATCH 62/68] Convert to hashset internally --- client/keystore/src/lib.rs | 12 ++++++------ primitives/core/src/testing.rs | 8 ++++---- primitives/core/src/traits.rs | 9 ++++----- 3 files changed, 14 insertions(+), 15 deletions(-) diff --git a/client/keystore/src/lib.rs b/client/keystore/src/lib.rs index dff9c2ba39fd6..a54bc399e13e6 100644 --- a/client/keystore/src/lib.rs +++ b/client/keystore/src/lib.rs @@ -302,11 +302,11 @@ impl BareCryptoStore for Store { &self, id: KeyTypeId, keys: Vec - ) -> std::result::Result, TraitError> { - let keys = keys.into_iter().collect::>(); - let all_keys = self.keys(id)?; - - Ok(keys.intersection(&all_keys).cloned().collect()) + ) -> std::result::Result, TraitError> { + let all_keys = self.keys(id)?.into_iter().collect::>(); + Ok(keys.into_iter() + .filter(|key| all_keys.contains(key)) + .collect::>()) } fn sign_with( @@ -390,7 +390,7 @@ impl BareCryptoStore for Store { mod tests { use super::*; use tempfile::TempDir; - use sp_core::{testing::{SR25519}, crypto::{Ss58Codec}}; + use sp_core::{testing::SR25519, crypto::Ss58Codec}; #[test] fn basic_store() { diff --git a/primitives/core/src/testing.rs b/primitives/core/src/testing.rs index ce93feb4210ee..47148fd2f01c5 100644 --- a/primitives/core/src/testing.rs +++ b/primitives/core/src/testing.rs @@ -145,11 +145,11 @@ impl crate::traits::BareCryptoStore for KeyStore { &self, id: KeyTypeId, keys: Vec - ) -> std::result::Result, BareCryptoStoreError> { - let keys = keys.into_iter().collect::>(); - let all_keys = self.keys(id)?; + ) -> std::result::Result, BareCryptoStoreError> { + let provided_keys = keys.into_iter().collect::>(); + let all_keys = self.keys(id)?.into_iter().collect::>(); - Ok(keys.intersection(&all_keys).cloned().collect()) + Ok(provided_keys.intersection(&all_keys).cloned().collect()) } fn sign_with( diff --git a/primitives/core/src/traits.rs b/primitives/core/src/traits.rs index ac4ba70a163c7..ae2088ee01656 100644 --- a/primitives/core/src/traits.rs +++ b/primitives/core/src/traits.rs @@ -23,7 +23,6 @@ use crate::{ use std::{ borrow::Cow, - collections::HashSet, fmt::{Debug, Display}, panic::UnwindSafe, sync::Arc, @@ -87,11 +86,11 @@ pub trait BareCryptoStore: Send + Sync { /// /// Provided a list of (CryptoTypeId,[u8]) pairs, this would return /// a filtered set of public keys which are supported by the keystore. - fn supported_keys(&self, id: KeyTypeId, keys: Vec) -> Result, BareCryptoStoreError>; + fn supported_keys(&self, id: KeyTypeId, keys: Vec) -> Result, BareCryptoStoreError>; /// List all supported keys /// /// Returns a set of public keys the signer supports. - fn keys(&self, id: KeyTypeId) -> Result, BareCryptoStoreError> { + fn keys(&self, id: KeyTypeId) -> Result, BareCryptoStoreError> { let ed25519_existing_keys = self .ed25519_public_keys(id) .into_iter() @@ -103,8 +102,8 @@ pub trait BareCryptoStore: Send + Sync { .map(Into::into); Ok(ed25519_existing_keys - .chain(sr25519_existing_keys) - .collect::>()) + .chain(sr25519_existing_keys) + .collect::>()) } /// Checks if the private keys for the given public key and key type combinations exist. From 9874c967c10e18d93b0715341abde6216f87363a Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Fri, 20 Mar 2020 13:46:59 +0100 Subject: [PATCH 63/68] WIP - iter_keys --- primitives/core/src/traits.rs | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/primitives/core/src/traits.rs b/primitives/core/src/traits.rs index ae2088ee01656..641238b1eb8f1 100644 --- a/primitives/core/src/traits.rs +++ b/primitives/core/src/traits.rs @@ -87,10 +87,10 @@ pub trait BareCryptoStore: Send + Sync { /// Provided a list of (CryptoTypeId,[u8]) pairs, this would return /// a filtered set of public keys which are supported by the keystore. fn supported_keys(&self, id: KeyTypeId, keys: Vec) -> Result, BareCryptoStoreError>; - /// List all supported keys + /// Iterate over keys /// - /// Returns a set of public keys the signer supports. - fn keys(&self, id: KeyTypeId) -> Result, BareCryptoStoreError> { + /// Returns an iterator over public keys filtered by ID + fn iter_keys(&self, id: KeyTypeId) -> impl IntoIterator { let ed25519_existing_keys = self .ed25519_public_keys(id) .into_iter() @@ -101,9 +101,15 @@ pub trait BareCryptoStore: Send + Sync { .into_iter() .map(Into::into); - Ok(ed25519_existing_keys + ed25519_existing_keys .chain(sr25519_existing_keys) - .collect::>()) + .into_iter() + } + /// List all supported keys + /// + /// Returns a set of public keys the signer supports. + fn keys(&self, id: KeyTypeId) -> Result, BareCryptoStoreError> { + Ok(self.iter_keys(id).collect::>()) } /// Checks if the private keys for the given public key and key type combinations exist. From f6be8268eb07cb78e5e2f250c6a754e22eb3ffcc Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Fri, 20 Mar 2020 15:52:45 +0100 Subject: [PATCH 64/68] Return raw_public_keys --- client/keystore/src/lib.rs | 100 +++++++++++++++++++++------------ primitives/core/src/testing.rs | 14 +++++ primitives/core/src/traits.rs | 22 +------- 3 files changed, 80 insertions(+), 56 deletions(-) diff --git a/client/keystore/src/lib.rs b/client/keystore/src/lib.rs index a54bc399e13e6..9df85e349c10d 100644 --- a/client/keystore/src/lib.rs +++ b/client/keystore/src/lib.rs @@ -242,16 +242,36 @@ impl Store { self.key_pair_by_type::(IsWrappedBy::from_ref(public), Pair::ID).map(Into::into) } - /// Get public keys of all stored keys that match the given key type. - pub fn public_keys_by_type(&self, key_type: KeyTypeId) -> Result> { - let mut public_keys: Vec = self.additional.keys() - .filter_map(|(ty, public)| { - if *ty == key_type { - Some(TPublic::from_slice(public)) - } else { - None - } + /// Get public keys of all stored keys that match the key type. + /// + /// This will just use the type of the public key (a list of which to be returned) in order + /// to determine the key type. Unless you use a specialized application-type public key, then + /// this only give you keys registered under generic cryptography, and will not return keys + /// registered under the application type. + pub fn public_keys(&self) -> Result> { + self.raw_public_keys(Public::ID) + .map(|v| { + v.into_iter() + .map(|k| Public::from_slice(k.as_slice())) + .collect() }) + } + + /// Returns the file path for the given public key and key type. + fn key_file_path(&self, public: &[u8], key_type: KeyTypeId) -> Option { + let mut buf = self.path.as_ref()?.clone(); + let key_type = hex::encode(key_type.0); + let key = hex::encode(public); + buf.push(key_type + key.as_str()); + Some(buf) + } + + /// Returns a list of raw public keys filtered by `KeyTypeId` + fn raw_public_keys(&self, id: KeyTypeId) -> Result>> { + let mut public_keys: Vec> = self.additional.keys() + .into_iter() + .filter(|k| k.0 == id) + .map(|k| k.1.clone()) .collect(); if let Some(path) = &self.path { @@ -263,8 +283,13 @@ impl Store { if let Some(name) = path.file_name().and_then(|n| n.to_str()) { match hex::decode(name) { Ok(ref hex) if hex.len() > 4 => { - if &hex[0..4] != &key_type.0 { continue } - let public = TPublic::from_slice(&hex[4..]); + let mut key_type: [u8; 4] = Default::default(); + key_type.copy_from_slice(&hex[0..4]); + let key_type = KeyTypeId(key_type); + if key_type != id { + continue; + } + let public = hex[4..].to_vec(); public_keys.push(public); } _ => continue, @@ -275,29 +300,23 @@ impl Store { Ok(public_keys) } - - /// Get public keys of all stored keys that match the key type. - /// - /// This will just use the type of the public key (a list of which to be returned) in order - /// to determine the key type. Unless you use a specialized application-type public key, then - /// this only give you keys registered under generic cryptography, and will not return keys - /// registered under the application type. - pub fn public_keys(&self) -> Result> { - self.public_keys_by_type::(Public::ID) - .map(|v| v.into_iter().map(Into::into).collect()) - } - - /// Returns the file path for the given public key and key type. - fn key_file_path(&self, public: &[u8], key_type: KeyTypeId) -> Option { - let mut buf = self.path.as_ref()?.clone(); - let key_type = hex::encode(key_type.0); - let key = hex::encode(public); - buf.push(key_type + key.as_str()); - Some(buf) - } } impl BareCryptoStore for Store { + fn keys( + &self, + id: KeyTypeId + ) -> std::result::Result, TraitError> { + let raw_keys = self.raw_public_keys(id)?; + Ok(raw_keys.into_iter() + .map(|k| vec![ + CryptoTypePublicPair(sr25519::CRYPTO_ID, k.clone()), + CryptoTypePublicPair(ed25519::CRYPTO_ID, k.clone()) + ]) + .flatten() + .collect()) + } + fn supported_keys( &self, id: KeyTypeId, @@ -337,8 +356,13 @@ impl BareCryptoStore for Store { } fn sr25519_public_keys(&self, key_type: KeyTypeId) -> Vec { - self.public_keys_by_type::(key_type) - .unwrap_or_default() + self.raw_public_keys(key_type) + .map(|v| { + v.into_iter() + .map(|k| sr25519::Public::from_slice(k.as_slice())) + .collect() + }) + .unwrap_or_default() } fn sr25519_generate_new( @@ -355,7 +379,13 @@ impl BareCryptoStore for Store { } fn ed25519_public_keys(&self, key_type: KeyTypeId) -> Vec { - self.public_keys_by_type::(key_type).unwrap_or_default() + self.raw_public_keys(key_type) + .map(|v| { + v.into_iter() + .map(|k| ed25519::Public::from_slice(k.as_slice())) + .collect() + }) + .unwrap_or_default() } fn ed25519_generate_new( @@ -504,7 +534,7 @@ mod tests { fs::write(file_name, "test").expect("Invalid file is written"); assert!( - store.read().public_keys_by_type::(SR25519).unwrap().is_empty(), + store.read().sr25519_public_keys(SR25519).is_empty(), ); } } diff --git a/primitives/core/src/testing.rs b/primitives/core/src/testing.rs index 47148fd2f01c5..d34e23ee5eae8 100644 --- a/primitives/core/src/testing.rs +++ b/primitives/core/src/testing.rs @@ -66,6 +66,20 @@ impl KeyStore { #[cfg(feature = "std")] impl crate::traits::BareCryptoStore for KeyStore { + fn keys(&self, id: KeyTypeId) -> Result, BareCryptoStoreError> { + self.keys + .get(&id) + .map(|map| { + Ok(map.keys().map(|k| vec![ + CryptoTypePublicPair(sr25519::CRYPTO_ID, k.clone()), + CryptoTypePublicPair(ed25519::CRYPTO_ID, k.clone()) + ]) + .flatten() + .collect()) + }) + .unwrap() + } + fn sr25519_public_keys(&self, id: KeyTypeId) -> Vec { self.keys.get(&id) .map(|keys| diff --git a/primitives/core/src/traits.rs b/primitives/core/src/traits.rs index 641238b1eb8f1..8fa57bfd0a996 100644 --- a/primitives/core/src/traits.rs +++ b/primitives/core/src/traits.rs @@ -87,30 +87,10 @@ pub trait BareCryptoStore: Send + Sync { /// Provided a list of (CryptoTypeId,[u8]) pairs, this would return /// a filtered set of public keys which are supported by the keystore. fn supported_keys(&self, id: KeyTypeId, keys: Vec) -> Result, BareCryptoStoreError>; - /// Iterate over keys - /// - /// Returns an iterator over public keys filtered by ID - fn iter_keys(&self, id: KeyTypeId) -> impl IntoIterator { - let ed25519_existing_keys = self - .ed25519_public_keys(id) - .into_iter() - .map(Into::into); - - let sr25519_existing_keys = self - .sr25519_public_keys(id) - .into_iter() - .map(Into::into); - - ed25519_existing_keys - .chain(sr25519_existing_keys) - .into_iter() - } /// List all supported keys /// /// Returns a set of public keys the signer supports. - fn keys(&self, id: KeyTypeId) -> Result, BareCryptoStoreError> { - Ok(self.iter_keys(id).collect::>()) - } + fn keys(&self, id: KeyTypeId) -> Result, BareCryptoStoreError>; /// Checks if the private keys for the given public key and key type combinations exist. /// From 0c012e700e469591a83b2d3642dc96744823c4c3 Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Mon, 23 Mar 2020 11:30:01 +0100 Subject: [PATCH 65/68] Address PR feedback --- client/keystore/src/lib.rs | 19 +++++++------------ primitives/core/src/testing.rs | 12 ++++++------ 2 files changed, 13 insertions(+), 18 deletions(-) diff --git a/client/keystore/src/lib.rs b/client/keystore/src/lib.rs index 9df85e349c10d..cf94ad2155db7 100644 --- a/client/keystore/src/lib.rs +++ b/client/keystore/src/lib.rs @@ -270,8 +270,7 @@ impl Store { fn raw_public_keys(&self, id: KeyTypeId) -> Result>> { let mut public_keys: Vec> = self.additional.keys() .into_iter() - .filter(|k| k.0 == id) - .map(|k| k.1.clone()) + .filter_map(|k| if k.0 == id { Some(k.1.clone()) } else { None }) .collect(); if let Some(path) = &self.path { @@ -283,10 +282,7 @@ impl Store { if let Some(name) = path.file_name().and_then(|n| n.to_str()) { match hex::decode(name) { Ok(ref hex) if hex.len() > 4 => { - let mut key_type: [u8; 4] = Default::default(); - key_type.copy_from_slice(&hex[0..4]); - let key_type = KeyTypeId(key_type); - if key_type != id { + if &hex[0..4] != &id.0 { continue; } let public = hex[4..].to_vec(); @@ -309,12 +305,11 @@ impl BareCryptoStore for Store { ) -> std::result::Result, TraitError> { let raw_keys = self.raw_public_keys(id)?; Ok(raw_keys.into_iter() - .map(|k| vec![ - CryptoTypePublicPair(sr25519::CRYPTO_ID, k.clone()), - CryptoTypePublicPair(ed25519::CRYPTO_ID, k.clone()) - ]) - .flatten() - .collect()) + .fold(Vec::new(), |mut v, k| { + v.push(CryptoTypePublicPair(sr25519::CRYPTO_ID, k.clone())); + v.push(CryptoTypePublicPair(ed25519::CRYPTO_ID, k.clone())); + v + })) } fn supported_keys( diff --git a/primitives/core/src/testing.rs b/primitives/core/src/testing.rs index d34e23ee5eae8..94a14b3bd6734 100644 --- a/primitives/core/src/testing.rs +++ b/primitives/core/src/testing.rs @@ -70,12 +70,12 @@ impl crate::traits::BareCryptoStore for KeyStore { self.keys .get(&id) .map(|map| { - Ok(map.keys().map(|k| vec![ - CryptoTypePublicPair(sr25519::CRYPTO_ID, k.clone()), - CryptoTypePublicPair(ed25519::CRYPTO_ID, k.clone()) - ]) - .flatten() - .collect()) + Ok(map.keys() + .fold(Vec::new(), |mut v, k| { + v.push(CryptoTypePublicPair(sr25519::CRYPTO_ID, k.clone())); + v.push(CryptoTypePublicPair(ed25519::CRYPTO_ID, k.clone())); + v + })) }) .unwrap() } From 9e80c0740e65ba239ccf73d88a49cdf42208f092 Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Mon, 23 Mar 2020 14:52:12 +0100 Subject: [PATCH 66/68] Address PR Feedback --- client/keystore/src/lib.rs | 10 ++++------ primitives/core/src/crypto.rs | 4 ++-- primitives/core/src/testing.rs | 14 ++++++-------- primitives/core/src/traits.rs | 8 ++++++-- 4 files changed, 18 insertions(+), 18 deletions(-) diff --git a/client/keystore/src/lib.rs b/client/keystore/src/lib.rs index cf94ad2155db7..f8bc93097113b 100644 --- a/client/keystore/src/lib.rs +++ b/client/keystore/src/lib.rs @@ -17,11 +17,11 @@ //! Keystore (and session key management) for ed25519 based chains like Polkadot. #![warn(missing_docs)] - use std::{collections::{HashMap, HashSet}, path::PathBuf, fs::{self, File}, io::{self, Write}, sync::Arc}; use sp_core::{ crypto::{IsWrappedBy, CryptoTypePublicPair, KeyTypeId, Pair as PairT, Protected, Public}, - traits::{BareCryptoStore, BareCryptoStoreError as TraitError} + traits::{BareCryptoStore, BareCryptoStoreError as TraitError}, + Encode, }; use sp_application_crypto::{AppKey, AppPublic, AppPair, ed25519, sr25519}; use parking_lot::RwLock; @@ -334,17 +334,15 @@ impl BareCryptoStore for Store { let pub_key = ed25519::Public::from_slice(key.1.as_slice()); let key_pair: ed25519::Pair = self .key_pair_by_type::(&pub_key, id) - .map(Into::into) .map_err(|e| TraitError::from(e))?; - Ok(<[u8; 64]>::from(key_pair.sign(msg)).to_vec()) + Ok(key_pair.sign(msg).encode()) } sr25519::CRYPTO_ID => { let pub_key = sr25519::Public::from_slice(key.1.as_slice()); let key_pair: sr25519::Pair = self .key_pair_by_type::(&pub_key, id) - .map(Into::into) .map_err(|e| TraitError::from(e))?; - Ok(<[u8; 64]>::from(key_pair.sign(msg)).to_vec()) + Ok(key_pair.sign(msg).encode()) } _ => Err(TraitError::KeyNotSupported(id)) } diff --git a/primitives/core/src/crypto.rs b/primitives/core/src/crypto.rs index 0045cc4d4b616..88f3e3d2f442a 100644 --- a/primitives/core/src/crypto.rs +++ b/primitives/core/src/crypto.rs @@ -17,7 +17,7 @@ // tag::description[] //! Cryptographic utilities. // end::description[] -use rustc_hex::ToHex; +use crate::hexdisplay::HexDisplay; use sp_std::hash::Hash; use sp_std::vec::Vec; @@ -962,7 +962,7 @@ impl sp_std::fmt::Display for CryptoTypePublicPair { format!("{:#?}", self.0) } }; - write!(f, "{}-{}", id, self.1.to_hex::()) + write!(f, "{}-{}", id, HexDisplay::from(&self.1)) } } diff --git a/primitives/core/src/testing.rs b/primitives/core/src/testing.rs index 94a14b3bd6734..b5e6f4c7aff37 100644 --- a/primitives/core/src/testing.rs +++ b/primitives/core/src/testing.rs @@ -25,7 +25,7 @@ use crate::{ }; #[cfg(feature = "std")] use std::collections::HashSet; - +use codec::Encode; /// Key type for generic Ed25519 key. pub const ED25519: KeyTypeId = KeyTypeId(*b"ed25"); /// Key type for generic Sr 25519 key. @@ -77,7 +77,7 @@ impl crate::traits::BareCryptoStore for KeyStore { v })) }) - .unwrap() + .unwrap_or(Ok(vec![])) } fn sr25519_public_keys(&self, id: KeyTypeId) -> Vec { @@ -158,7 +158,7 @@ impl crate::traits::BareCryptoStore for KeyStore { fn supported_keys( &self, id: KeyTypeId, - keys: Vec + keys: Vec, ) -> std::result::Result, BareCryptoStoreError> { let provided_keys = keys.into_iter().collect::>(); let all_keys = self.keys(id)?.into_iter().collect::>(); @@ -170,22 +170,20 @@ impl crate::traits::BareCryptoStore for KeyStore { &self, id: KeyTypeId, key: &CryptoTypePublicPair, - msg: &[u8] + msg: &[u8], ) -> Result, BareCryptoStoreError> { match key.0 { ed25519::CRYPTO_ID => { let key_pair: ed25519::Pair = self .ed25519_key_pair(id, &ed25519::Public::from_slice(key.1.as_slice())) - .map(Into::into) .ok_or(BareCryptoStoreError::PairNotFound("ed25519".to_owned()))?; - return Ok(<[u8; 64]>::from(key_pair.sign(msg)).to_vec()); + return Ok(key_pair.sign(msg).encode()); } sr25519::CRYPTO_ID => { let key_pair: sr25519::Pair = self .sr25519_key_pair(id, &sr25519::Public::from_slice(key.1.as_slice())) - .map(Into::into) .ok_or(BareCryptoStoreError::PairNotFound("sr25519".to_owned()))?; - return Ok(<[u8; 64]>::from(key_pair.sign(msg)).to_vec()); + return Ok(key_pair.sign(msg).encode()); } _ => Err(BareCryptoStoreError::KeyNotSupported(id)) } diff --git a/primitives/core/src/traits.rs b/primitives/core/src/traits.rs index 365cef34c4740..e5c15fc37f33a 100644 --- a/primitives/core/src/traits.rs +++ b/primitives/core/src/traits.rs @@ -86,7 +86,11 @@ pub trait BareCryptoStore: Send + Sync { /// /// Provided a list of (CryptoTypeId,[u8]) pairs, this would return /// a filtered set of public keys which are supported by the keystore. - fn supported_keys(&self, id: KeyTypeId, keys: Vec) -> Result, BareCryptoStoreError>; + fn supported_keys( + &self, + id: KeyTypeId, + keys: Vec + ) -> Result, BareCryptoStoreError>; /// List all supported keys /// /// Returns a set of public keys the signer supports. @@ -146,7 +150,7 @@ pub trait BareCryptoStore: Send + Sync { &self, id: KeyTypeId, keys: Vec, - msg: &[u8] + msg: &[u8], ) -> Result, BareCryptoStoreError>>, ()>{ Ok(keys.iter().map(|k| self.sign_with(id, k, msg)).collect()) } From 7b92399455afcfe6d78d8dbf9101850f670ac2db Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Mon, 23 Mar 2020 15:08:17 +0100 Subject: [PATCH 67/68] Fix hexdisplay import error --- primitives/core/src/crypto.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/primitives/core/src/crypto.rs b/primitives/core/src/crypto.rs index 88f3e3d2f442a..5f7b2a4f4025c 100644 --- a/primitives/core/src/crypto.rs +++ b/primitives/core/src/crypto.rs @@ -17,7 +17,6 @@ // tag::description[] //! Cryptographic utilities. // end::description[] -use crate::hexdisplay::HexDisplay; use sp_std::hash::Hash; use sp_std::vec::Vec; @@ -34,7 +33,8 @@ use codec::{Encode, Decode}; use regex::Regex; #[cfg(feature = "std")] use base58::{FromBase58, ToBase58}; - +#[cfg(feature = "std")] +use crate::hexdisplay::HexDisplay; use zeroize::Zeroize; #[doc(hidden)] pub use sp_std::ops::Deref; From 7908f71129aad18b885b0f154c2db85235ac5d6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Mon, 23 Mar 2020 15:11:54 +0100 Subject: [PATCH 68/68] Update primitives/core/src/traits.rs --- primitives/core/src/traits.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/primitives/core/src/traits.rs b/primitives/core/src/traits.rs index e5c15fc37f33a..14839fb58562a 100644 --- a/primitives/core/src/traits.rs +++ b/primitives/core/src/traits.rs @@ -106,7 +106,7 @@ pub trait BareCryptoStore: Send + Sync { /// Signs a message with the private key that matches /// the public key passed. /// - /// Returns the signature if key is found & supported, + /// Returns the SCALE encoded signature if key is found & supported, /// an error otherwise. fn sign_with( &self,