From 34626f729dbe2b5761bccfee8629a06992f50a7b Mon Sep 17 00:00:00 2001 From: Giles Cope Date: Fri, 3 Sep 2021 09:27:30 +0100 Subject: [PATCH 1/3] Own the seed. --- bin/node/testing/src/bench.rs | 2 +- client/cli/src/commands/vanity.rs | 4 ++-- client/consensus/babe/src/tests.rs | 2 +- client/executor/src/integration_tests/mod.rs | 4 ++-- frame/babe/src/mock.rs | 2 +- primitives/application-crypto/src/lib.rs | 4 +++- primitives/core/src/crypto.rs | 8 +++---- primitives/core/src/ecdsa.rs | 24 +++++++++++--------- primitives/core/src/ed25519.rs | 20 ++++++++-------- primitives/core/src/sr25519.rs | 18 +++++++-------- 10 files changed, 47 insertions(+), 41 deletions(-) diff --git a/bin/node/testing/src/bench.rs b/bin/node/testing/src/bench.rs index edb99c617771a..845fcf29a301e 100644 --- a/bin/node/testing/src/bench.rs +++ b/bin/node/testing/src/bench.rs @@ -554,7 +554,7 @@ impl BenchKeyring { (account_id, BenchPair::Sr25519(pair)) }, KeyTypes::Ed25519 => { - let pair = ed25519::Pair::from_seed(&blake2_256(seed.as_bytes())); + let pair = ed25519::Pair::from_seed(blake2_256(seed.as_bytes())); let account_id = AccountPublic::from(pair.public()).into_account(); (account_id, BenchPair::Ed25519(pair)) }, diff --git a/client/cli/src/commands/vanity.rs b/client/cli/src/commands/vanity.rs index ce1f079db8789..ece84879937e0 100644 --- a/client/cli/src/commands/vanity.rs +++ b/client/cli/src/commands/vanity.rs @@ -97,14 +97,14 @@ fn generate_key( next_seed(seed.as_mut()); } - let p = Pair::from_seed(&seed); + let p = Pair::from_seed(seed.clone()); let ss58 = p.public().into_account().to_ss58check_with_version(network_override); let score = calculate_score(&desired, &ss58); if score > best || desired.len() < 2 { best = score; if best >= top { println!("best: {} == top: {}", best, top); - return Ok(utils::format_seed::(seed.clone())); + return Ok(utils::format_seed::(seed)) } } done += 1; diff --git a/client/consensus/babe/src/tests.rs b/client/consensus/babe/src/tests.rs index 3392ffade98ee..63e5db7c97850 100644 --- a/client/consensus/babe/src/tests.rs +++ b/client/consensus/babe/src/tests.rs @@ -646,7 +646,7 @@ fn propose_and_import_block( let seal = { // sign the pre-sealed hash of the block and then // add it to a digest item. - let pair = AuthorityPair::from_seed(&[1; 32]); + let pair = AuthorityPair::from_seed([1; 32]); let pre_hash = block.header.hash(); let signature = pair.sign(pre_hash.as_ref()); Item::babe_seal(signature) diff --git a/client/executor/src/integration_tests/mod.rs b/client/executor/src/integration_tests/mod.rs index 0762306309df4..dbf2c6eb56742 100644 --- a/client/executor/src/integration_tests/mod.rs +++ b/client/executor/src/integration_tests/mod.rs @@ -385,7 +385,7 @@ test_wasm_execution!(ed25519_verify_should_work); fn ed25519_verify_should_work(wasm_method: WasmExecutionMethod) { let mut ext = TestExternalities::default(); let mut ext = ext.ext(); - let key = ed25519::Pair::from_seed(&blake2_256(b"test")); + let key = ed25519::Pair::from_seed(blake2_256(b"test")); let sig = key.sign(b"all ok!"); let mut calldata = vec![]; calldata.extend_from_slice(key.public().as_ref()); @@ -421,7 +421,7 @@ test_wasm_execution!(sr25519_verify_should_work); fn sr25519_verify_should_work(wasm_method: WasmExecutionMethod) { let mut ext = TestExternalities::default(); let mut ext = ext.ext(); - let key = sr25519::Pair::from_seed(&blake2_256(b"test")); + let key = sr25519::Pair::from_seed(blake2_256(b"test")); let sig = key.sign(b"all ok!"); let mut calldata = vec![]; calldata.extend_from_slice(key.public().as_ref()); diff --git a/frame/babe/src/mock.rs b/frame/babe/src/mock.rs index 6c1cc89cf1ed0..4aebd94b34275 100644 --- a/frame/babe/src/mock.rs +++ b/frame/babe/src/mock.rs @@ -367,7 +367,7 @@ pub fn new_test_ext(authorities_len: usize) -> sp_io::TestExternalities { pub fn new_test_ext_with_pairs(authorities_len: usize) -> (Vec, sp_io::TestExternalities) { let pairs = (0..authorities_len).map(|i| { - AuthorityPair::from_seed(&U256::from(i).into()) + AuthorityPair::from_seed(U256::from(i).into()) }).collect::>(); let public = pairs.iter().map(|p| p.public()).collect(); diff --git a/primitives/application-crypto/src/lib.rs b/primitives/application-crypto/src/lib.rs index 58e5c5b7a311f..0cab44877c0ca 100644 --- a/primitives/application-crypto/src/lib.rs +++ b/primitives/application-crypto/src/lib.rs @@ -118,7 +118,9 @@ macro_rules! app_crypto_pair { >(&self, path: Iter, seed: Option) -> Result<(Self, Option), Self::DeriveError> { self.0.derive(path, seed).map(|x| (Self(x.0), x.1)) } - fn from_seed(seed: &Self::Seed) -> Self { Self(<$pair>::from_seed(seed)) } + + fn from_seed(seed: Self::Seed) -> Self { Self(<$pair>::from_seed(seed)) } + fn from_seed_slice(seed: &[u8]) -> Result { <$pair>::from_seed_slice(seed).map(Self) } diff --git a/primitives/core/src/crypto.rs b/primitives/core/src/crypto.rs index d9a0a69e16813..97dbc65439d69 100644 --- a/primitives/core/src/crypto.rs +++ b/primitives/core/src/crypto.rs @@ -902,7 +902,7 @@ mod dummy { fn derive< Iter: Iterator, >(&self, _: Iter, _: Option) -> Result<(Self, Option), Self::DeriveError> { Ok((Self, None)) } - fn from_seed(_: &Self::Seed) -> Self { Self } + fn from_seed(_: Self::Seed) -> Self { Self } fn from_seed_slice(_: &[u8]) -> Result { Ok(Self) } fn sign(&self, _: &[u8]) -> Self::Signature { Self } fn verify>(_: &Self::Signature, _: M, _: &Self::Public) -> bool { true } @@ -939,7 +939,7 @@ pub trait Pair: CryptoType + Sized + Clone + Send + Sync + 'static { fn generate() -> (Self, Self::Seed) { let mut seed = Self::Seed::default(); OsRng.fill_bytes(seed.as_mut()); - (Self::from_seed(&seed), seed) + (Self::from_seed(seed.clone()), seed) } /// Generate new secure (random) key pair and provide the recovery phrase. @@ -965,7 +965,7 @@ pub trait Pair: CryptoType + Sized + Clone + Send + Sync + 'static { /// /// @WARNING: THIS WILL ONLY BE SECURE IF THE `seed` IS SECURE. If it can be guessed /// by an attacker then they can also derive your key. - fn from_seed(seed: &Self::Seed) -> Self; + fn from_seed(seed: Self::Seed) -> Self; /// Make a new key pair from secret seed material. The slice must be the correct size or /// it will return `None`. @@ -1032,7 +1032,7 @@ pub trait Pair: CryptoType + Sized + Clone + Send + Sync + 'static { let mut seed = Self::Seed::default(); if seed.as_ref().len() == seed_vec.len() { seed.as_mut().copy_from_slice(&seed_vec); - Some((Self::from_seed(&seed), seed)) + Some((Self::from_seed(seed.clone()), seed)) } else { None } diff --git a/primitives/core/src/ecdsa.rs b/primitives/core/src/ecdsa.rs index c567b3c44f6ce..2413de6dda607 100644 --- a/primitives/core/src/ecdsa.rs +++ b/primitives/core/src/ecdsa.rs @@ -453,8 +453,10 @@ impl TraitPair for Pair { /// Make a new key pair from secret seed material. /// /// You should never need to use this; generate(), generate_with_phrase - fn from_seed(seed: &Seed) -> Pair { - Self::from_seed_slice(&seed[..]).expect("seed has valid length; qed") + fn from_seed(seed: Seed) -> Pair { + let secret = SecretKey::parse(&seed).expect("seed has valid length; qed"); + let public = PublicKey::from_secret_key(&secret); + Pair { public, secret } } /// Make a new key pair from secret seed material. The slice must be 32 bytes long or it @@ -480,7 +482,7 @@ impl TraitPair for Pair { DeriveJunction::Hard(cc) => acc = derive_hard_junction(&acc, &cc), } } - Ok((Self::from_seed(&acc), Some(acc))) + Ok((Self::from_seed(acc.clone()), Some(acc))) } /// Get the public key. @@ -540,7 +542,7 @@ impl Pair { let mut padded_seed: Seed = [b' '; 32]; let len = s.len().min(32); padded_seed[..len].copy_from_slice(&s.as_bytes()[..len]); - Self::from_seed(&padded_seed) + Self::from_seed(padded_seed) }) } @@ -601,7 +603,7 @@ mod test { #[test] fn seed_and_derive_should_work() { let seed = hex!("9d61b19deffd5a60ba844af492ec2cc44449c5697b326919703bac031cae7f60"); - let pair = Pair::from_seed(&seed); + let pair = Pair::from_seed(seed.clone()); assert_eq!(pair.seed(), seed); let path = vec![DeriveJunction::Hard([0u8; 32])]; let derived = pair.derive(path.into_iter(), None).ok().unwrap(); @@ -614,7 +616,7 @@ mod test { #[test] fn test_vector_should_work() { let pair = Pair::from_seed( - &hex!("9d61b19deffd5a60ba844af492ec2cc44449c5697b326919703bac031cae7f60") + hex!("9d61b19deffd5a60ba844af492ec2cc44449c5697b326919703bac031cae7f60") ); let public = pair.public(); assert_eq!( @@ -662,7 +664,7 @@ mod test { #[test] fn seeded_pair_should_work() { - let pair = Pair::from_seed(b"12345678901234567890123456789012"); + let pair = Pair::from_seed(*b"12345678901234567890123456789012"); let public = pair.public(); assert_eq!( public, @@ -703,7 +705,7 @@ mod test { #[test] fn ss58check_roundtrip_works() { - let pair = Pair::from_seed(b"12345678901234567890123456789012"); + let pair = Pair::from_seed(*b"12345678901234567890123456789012"); let public = pair.public(); let s = public.to_ss58check(); println!("Correct: {}", s); @@ -714,7 +716,7 @@ mod test { #[test] fn ss58check_format_check_works() { use crate::crypto::Ss58AddressFormat; - let pair = Pair::from_seed(b"12345678901234567890123456789012"); + let pair = Pair::from_seed(*b"12345678901234567890123456789012"); let public = pair.public(); let format = Ss58AddressFormat::Reserved46; let s = public.to_ss58check_with_version(format); @@ -724,7 +726,7 @@ mod test { #[test] fn ss58check_full_roundtrip_works() { use crate::crypto::Ss58AddressFormat; - let pair = Pair::from_seed(b"12345678901234567890123456789012"); + let pair = Pair::from_seed(*b"12345678901234567890123456789012"); let public = pair.public(); let format = Ss58AddressFormat::PolkadotAccount; let s = public.to_ss58check_with_version(format); @@ -775,7 +777,7 @@ mod test { #[test] fn signature_serialization_works() { - let pair = Pair::from_seed(b"12345678901234567890123456789012"); + let pair = Pair::from_seed(*b"12345678901234567890123456789012"); let message = b"Something important"; let signature = pair.sign(&message[..]); let serialized_signature = serde_json::to_string(&signature).unwrap(); diff --git a/primitives/core/src/ed25519.rs b/primitives/core/src/ed25519.rs index 392dc2eec6c66..c76e0b6930065 100644 --- a/primitives/core/src/ed25519.rs +++ b/primitives/core/src/ed25519.rs @@ -461,8 +461,10 @@ impl TraitPair for Pair { /// Make a new key pair from secret seed material. /// /// You should never need to use this; generate(), generate_with_phrase - fn from_seed(seed: &Seed) -> Pair { - Self::from_seed_slice(&seed[..]).expect("seed has valid length; qed") + fn from_seed(seed: Seed) -> Pair { + let secret = SigningKey::from(seed); + let public = VerificationKey::from(&secret); + Pair {secret, public} } /// Make a new key pair from secret seed material. The slice must be 32 bytes long or it @@ -488,7 +490,7 @@ impl TraitPair for Pair { DeriveJunction::Hard(cc) => acc = derive_hard_junction(&acc, &cc), } } - Ok((Self::from_seed(&acc), Some(acc))) + Ok((Self::from_seed(acc.clone()), Some(acc))) } /// Get the public key. @@ -549,7 +551,7 @@ impl Pair { let mut padded_seed: Seed = [b' '; 32]; let len = s.len().min(32); padded_seed[..len].copy_from_slice(&s.as_bytes()[..len]); - Self::from_seed(&padded_seed) + Self::from_seed(padded_seed) }) } } @@ -587,7 +589,7 @@ mod test { #[test] fn seed_and_derive_should_work() { let seed = hex!("9d61b19deffd5a60ba844af492ec2cc44449c5697b326919703bac031cae7f60"); - let pair = Pair::from_seed(&seed); + let pair = Pair::from_seed(seed); assert_eq!(pair.seed(), &seed); let path = vec![DeriveJunction::Hard([0u8; 32])]; let derived = pair.derive(path.into_iter(), None).ok().unwrap().0; @@ -600,7 +602,7 @@ mod test { #[test] fn test_vector_should_work() { let pair = Pair::from_seed( - &hex!("9d61b19deffd5a60ba844af492ec2cc44449c5697b326919703bac031cae7f60") + hex!("9d61b19deffd5a60ba844af492ec2cc44449c5697b326919703bac031cae7f60") ); let public = pair.public(); assert_eq!(public, Public::from_raw( @@ -642,7 +644,7 @@ mod test { #[test] fn seeded_pair_should_work() { - let pair = Pair::from_seed(b"12345678901234567890123456789012"); + let pair = Pair::from_seed(*b"12345678901234567890123456789012"); let public = pair.public(); assert_eq!(public, Public::from_raw( hex!("2f8c6129d816cf51c374bc7f08c3e63ed156cf78aefb4a6550d97b87997977ee") @@ -680,7 +682,7 @@ mod test { #[test] fn ss58check_roundtrip_works() { - let pair = Pair::from_seed(b"12345678901234567890123456789012"); + let pair = Pair::from_seed(*b"12345678901234567890123456789012"); let public = pair.public(); let s = public.to_ss58check(); println!("Correct: {}", s); @@ -690,7 +692,7 @@ mod test { #[test] fn signature_serialization_works() { - let pair = Pair::from_seed(b"12345678901234567890123456789012"); + let pair = Pair::from_seed(*b"12345678901234567890123456789012"); let message = b"Something important"; let signature = pair.sign(&message[..]); let serialized_signature = serde_json::to_string(&signature).unwrap(); diff --git a/primitives/core/src/sr25519.rs b/primitives/core/src/sr25519.rs index 269f19cba0073..9f3596548ad27 100644 --- a/primitives/core/src/sr25519.rs +++ b/primitives/core/src/sr25519.rs @@ -470,7 +470,7 @@ impl TraitPair for Pair { /// This is generated using schnorrkel's Mini-Secret-Keys. /// /// A MiniSecretKey is literally what Ed25519 calls a SecretKey, which is just 32 random bytes. - fn from_seed(seed: &Seed) -> Pair { + fn from_seed(seed: Seed) -> Pair { Self::from_seed_slice(&seed[..]) .expect("32 bytes can always build a key; qed") } @@ -737,7 +737,7 @@ mod test { #[test] fn derive_soft_should_work() { - let pair = Pair::from_seed(&hex!( + let pair = Pair::from_seed(hex!( "9d61b19deffd5a60ba844af492ec2cc44449c5697b326919703bac031cae7f60" )); let derive_1 = pair.derive(Some(DeriveJunction::soft(1)).into_iter(), None).unwrap().0; @@ -749,7 +749,7 @@ mod test { #[test] fn derive_hard_should_work() { - let pair = Pair::from_seed(&hex!( + let pair = Pair::from_seed(hex!( "9d61b19deffd5a60ba844af492ec2cc44449c5697b326919703bac031cae7f60" )); let derive_1 = pair.derive(Some(DeriveJunction::hard(1)).into_iter(), None).unwrap().0; @@ -761,7 +761,7 @@ mod test { #[test] fn derive_soft_public_should_work() { - let pair = Pair::from_seed(&hex!( + let pair = Pair::from_seed(hex!( "9d61b19deffd5a60ba844af492ec2cc44449c5697b326919703bac031cae7f60" )); let path = Some(DeriveJunction::soft(1)); @@ -772,7 +772,7 @@ mod test { #[test] fn derive_hard_public_should_fail() { - let pair = Pair::from_seed(&hex!( + let pair = Pair::from_seed(hex!( "9d61b19deffd5a60ba844af492ec2cc44449c5697b326919703bac031cae7f60" )); let path = Some(DeriveJunction::hard(1)); @@ -781,7 +781,7 @@ mod test { #[test] fn sr_test_vector_should_work() { - let pair = Pair::from_seed(&hex!( + let pair = Pair::from_seed(hex!( "9d61b19deffd5a60ba844af492ec2cc44449c5697b326919703bac031cae7f60" )); let public = pair.public(); @@ -828,7 +828,7 @@ mod test { #[test] fn seeded_pair_should_work() { - let pair = Pair::from_seed(b"12345678901234567890123456789012"); + let pair = Pair::from_seed(*b"12345678901234567890123456789012"); let public = pair.public(); assert_eq!( public, @@ -857,7 +857,7 @@ mod test { // // This is to make sure that the wasm library is compatible. let pk = Pair::from_seed( - &hex!("0000000000000000000000000000000000000000000000000000000000000000") + hex!("0000000000000000000000000000000000000000000000000000000000000000") ); let public = pk.public(); let js_signature = Signature::from_raw(hex!( @@ -869,7 +869,7 @@ mod test { #[test] fn signature_serialization_works() { - let pair = Pair::from_seed(b"12345678901234567890123456789012"); + let pair = Pair::from_seed(*b"12345678901234567890123456789012"); let message = b"Something important"; let signature = pair.sign(&message[..]); let serialized_signature = serde_json::to_string(&signature).unwrap(); From 00383d7cc35536bd65aa0a8e0eba5a5c1cf3c9ac Mon Sep 17 00:00:00 2001 From: Giles Cope Date: Fri, 3 Sep 2021 09:40:01 +0100 Subject: [PATCH 2/3] cargo fmt + downgrade from zebra --- primitives/core/src/crypto.rs | 4 ++-- primitives/core/src/ed25519.rs | 4 +--- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/primitives/core/src/crypto.rs b/primitives/core/src/crypto.rs index ed265b91b64ed..49fc11905d061 100644 --- a/primitives/core/src/crypto.rs +++ b/primitives/core/src/crypto.rs @@ -1358,8 +1358,8 @@ mod tests { None, )) } - fn from_seed(_seed: &::Seed) -> Self { - TestPair::Seed(_seed.as_ref().to_owned()) + fn from_seed(seed: ::Seed) -> Self { + TestPair::Seed(seed.as_ref().to_owned()) } fn sign(&self, _message: &[u8]) -> Self::Signature { [] diff --git a/primitives/core/src/ed25519.rs b/primitives/core/src/ed25519.rs index 01037060bc8ea..06814e016f640 100644 --- a/primitives/core/src/ed25519.rs +++ b/primitives/core/src/ed25519.rs @@ -476,9 +476,7 @@ impl TraitPair for Pair { /// /// You should never need to use this; generate(), generate_with_phrase fn from_seed(seed: Seed) -> Pair { - let secret = SigningKey::from(seed); - let public = VerificationKey::from(&secret); - Pair {secret, public} + Self::from_seed_slice(&seed[..]).expect("seed has valid length; qed") } /// Make a new key pair from secret seed material. The slice must be 32 bytes long or it From bd76d2cc2cff4afe09797e7b585e41153c46b44f Mon Sep 17 00:00:00 2001 From: Giles Cope Date: Fri, 3 Sep 2021 15:05:41 +0100 Subject: [PATCH 3/3] Avoid clones for perf --- primitives/core/src/crypto.rs | 7 +++++-- primitives/core/src/ed25519.rs | 4 ++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/primitives/core/src/crypto.rs b/primitives/core/src/crypto.rs index 49fc11905d061..35aab90b303a8 100644 --- a/primitives/core/src/crypto.rs +++ b/primitives/core/src/crypto.rs @@ -978,7 +978,7 @@ pub trait Pair: CryptoType + Sized + Clone + Send + Sync + 'static { fn generate() -> (Self, Self::Seed) { let mut seed = Self::Seed::default(); OsRng.fill_bytes(seed.as_mut()); - (Self::from_seed(seed.clone()), seed) + (Self::from_seed_slice(seed.as_ref()).expect("seed has valid length; qed"), seed) } /// Generate new secure (random) key pair and provide the recovery phrase. @@ -1077,7 +1077,10 @@ pub trait Pair: CryptoType + Sized + Clone + Send + Sync + 'static { let mut seed = Self::Seed::default(); if seed.as_ref().len() == seed_vec.len() { seed.as_mut().copy_from_slice(&seed_vec); - Some((Self::from_seed(seed.clone()), seed)) + Some(( + Self::from_seed_slice(&seed_vec).expect("seed has valid length; qed"), + seed, + )) } else { None } diff --git a/primitives/core/src/ed25519.rs b/primitives/core/src/ed25519.rs index 06814e016f640..f9537e49aa033 100644 --- a/primitives/core/src/ed25519.rs +++ b/primitives/core/src/ed25519.rs @@ -474,7 +474,7 @@ impl TraitPair for Pair { /// Make a new key pair from secret seed material. /// - /// You should never need to use this; generate(), generate_with_phrase + /// You should never need to use this directly; generate(), generate_with_phrase fn from_seed(seed: Seed) -> Pair { Self::from_seed_slice(&seed[..]).expect("seed has valid length; qed") } @@ -482,7 +482,7 @@ impl TraitPair for Pair { /// Make a new key pair from secret seed material. The slice must be 32 bytes long or it /// will return `None`. /// - /// You should never need to use this; generate(), generate_with_phrase + /// You should never need to use this directly; generate(), generate_with_phrase fn from_seed_slice(seed_slice: &[u8]) -> Result { let secret = ed25519_dalek::SecretKey::from_bytes(seed_slice) .map_err(|_| SecretStringError::InvalidSeedLength)?;