Crypto fixes/improvements (#2008)

* Crypto fixes:

- Use schnorrkel's HDKD derive
- Assume all key URIs beginning with `/` are prefixed with public
  root phrase.

* Remove commented code.

* Update README

* Update core/primitives/src/ed25519.rs

Co-Authored-By: gavofyork <github@gavwood.com>

substrate/Cargo.lock

@@ -2768,7 +2768,7 @@ dependencies = [
 [[package]]
 name = "schnorrkel"
 version = "0.0.0"
-source = "git+https://github.com/w3f/schnorrkel#0876f3f2194426150efea99304a8d23a336744bc"
+source = "git+https://github.com/w3f/schnorrkel#0a0de4294b475ef6abdeebb50067f213ca79b3c7"
 dependencies = [
  "clear_on_drop 0.2.3 (registry+https://github.com/rust-lang/crates.io-index)",
  "curve25519-dalek 1.0.3 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -4673,7 +4673,7 @@ name = "twox-hash"
 version = "1.1.2"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 dependencies = [
- "rand 0.6.5 (registry+https://github.com/rust-lang/crates.io-index)",
+ "rand 0.4.6 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 
 [[package]]

substrate/README.adoc

@@ -259,7 +259,7 @@ We'll start Alice's substrate node first on default TCP port 30333 with her chai
 cargo run --release \-- \
   --base-path /tmp/alice \
   --chain=local \
-  --key Alice \
+  --key //Alice \
   --name "ALICE" \
   --node-key 0000000000000000000000000000000000000000000000000000000000000001 \
   --telemetry-url ws://telemetry.polkadot.io:1024 \
@@ -272,7 +272,7 @@ cargo run --release \-- \
   --base-path /tmp/bob \
   --bootnodes /ip4/127.0.0.1/tcp/30333/p2p/QmQZ8TjTqeDj3ciwr93EJ95hxfDsb9pEYDizUAbWpigtQN \
   --chain=local \
-  --key Bob \
+  --key //Bob \
   --name "BOB" \
   --port 30334 \
   --telemetry-url ws://telemetry.polkadot.io:1024 \

substrate/core/cli/src/lib.rs

@@ -63,9 +63,6 @@ use substrate_telemetry::TelemetryEndpoints;
 
 const MAX_NODE_NAME_LENGTH: usize = 32;
 
-/// The root phrase for our development network keys.
-pub const DEV_PHRASE: &str = "bottom drive obey lake curtain smoke basket hold race lonely fit walk";
-
 /// Executable version. Used to pass version information from the root crate.
 pub struct VersionInfo {
 	/// Implemtation name.
@@ -390,7 +387,7 @@ where
 	}
 
 	if cli.shared_params.dev {
-		config.keys.push(format!("{}//Alice", DEV_PHRASE));
+		config.keys.push("//Alice".into());
 	}
 
 	let rpc_interface: &str = if cli.rpc_external { "0.0.0.0" } else { "127.0.0.1" };

substrate/core/keyring/src/ed25519.rs

@@ -22,11 +22,6 @@ use lazy_static::lazy_static;
 use substrate_primitives::{ed25519::{Pair, Public, Signature}, Pair as _Pair, H256};
 pub use substrate_primitives::ed25519;
 
-/// The root phrase for our test keys.
-///
-/// This is the same phrase that's in node::cli, but shouldn't need to be.
-pub const DEV_PHRASE: &str = "bottom drive obey lake curtain smoke basket hold race lonely fit walk";
-
 /// Set of test accounts.
 #[derive(Clone, Copy, PartialEq, Eq, Hash)]
 pub enum Keyring {
@@ -81,7 +76,7 @@ impl Keyring {
 	}
 
 	pub fn pair(self) -> Pair {
-		Pair::from_string(&format!("{}//{}", DEV_PHRASE, <&'static str>::from(self)), None)
+		Pair::from_string(&format!("//{}", <&'static str>::from(self)), None)
 			.expect("static values are known good; qed")
 	}
 }

substrate/core/keyring/src/sr25519.rs

@@ -22,11 +22,6 @@ use lazy_static::lazy_static;
 use substrate_primitives::{sr25519::{Pair, Public, Signature}, Pair as _Pair, H256};
 pub use substrate_primitives::sr25519;
 
-/// The root phrase for our test keys.
-///
-/// This is the same phrase that's in node::cli, but shouldn't need to be.
-pub const DEV_PHRASE: &str = "bottom drive obey lake curtain smoke basket hold race lonely fit walk";
-
 /// Set of test accounts.
 #[derive(Clone, Copy, PartialEq, Eq, Hash)]
 pub enum Keyring {
@@ -81,7 +76,7 @@ impl Keyring {
 	}
 
 	pub fn pair(self) -> Pair {
-		Pair::from_string(&format!("{}//{}", DEV_PHRASE, <&'static str>::from(self)), None)
+		Pair::from_string(&format!("//{}", <&'static str>::from(self)), None)
 			.expect("static values are known good; qed")
 	}
 }

substrate/core/primitives/src/crypto.rs

@@ -25,6 +25,12 @@ use regex::Regex;
 #[cfg(feature = "std")]
 use base58::{FromBase58, ToBase58};
 
+/// The root phrase for our publically known keys.
+pub const DEV_PHRASE: &str = "bottom drive obey lake curtain smoke basket hold race lonely fit walk";
+
+/// The address of the associated root phrase for our publically known keys.
+pub const DEV_ADDRESS: &str = "5DfhGyQdFobKM8NsWvEeAKk5EQQgYe9AydgJ7rMB6E1EqAS7";
+
 /// The infallible type.
 #[derive(Debug)]
 pub enum Infallible {}
@@ -243,14 +249,16 @@ impl<T: AsMut<[u8]> + AsRef<[u8]> + Default + Derive> Ss58Codec for T {
 	}
 
 	fn from_string(s: &str) -> Result<Self, PublicError> {
-		let re = Regex::new(r"^(?P<ss58>[\w\d]+)(?P<path>(//?[^/]+)*)$")
+		let re = Regex::new(r"^(?P<ss58>[\w\d]+)?(?P<path>(//?[^/]+)*)$")
 			.expect("constructed from known-good static value; qed");
 		let cap = re.captures(s).ok_or(PublicError::InvalidFormat)?;
 		let re_junction = Regex::new(r"/(/?[^/]+)")
 			.expect("constructed from known-good static value; qed");
 		let path = re_junction.captures_iter(&cap["path"])
 			.map(|f| DeriveJunction::from(&f[1]));
-		Self::from_ss58check(&cap["ss58"])?.derive(path).ok_or(PublicError::InvalidPath)
+		Self::from_ss58check(cap.name("ss58").map(|r| r.as_str()).unwrap_or(DEV_ADDRESS))?
+			.derive(path)
+			.ok_or(PublicError::InvalidPath)
 	}
 }
 
@@ -335,6 +343,9 @@ pub trait Pair: Sized {
 	///   - the phrase may be followed by one or more items delimited by `/` characters.
 	///   - the path may be followed by `///`, in which case everything after the `///` is treated
 	/// as a password.
+	/// - If `s` begins with a `/` character it is prefixed with the Substrate public `DEV_PHRASE` and
+	/// interpreted as above.
+	///
 	/// In this case they are interpreted as HDKD junctions; purely numeric items are interpreted as
 	/// integers, non-numeric items as strings. Junctions prefixed with `/` are interpreted as soft
 	/// junctions, and with `//` as hard junctions.
@@ -359,7 +370,7 @@ pub trait Pair: Sized {
 			}
 		}
 
-		let re = Regex::new(r"^(?P<phrase>\w+( \w+)*)(?P<path>(//?[^/]+)*)(///(?P<password>.*))?$")
+		let re = Regex::new(r"^(?P<phrase>\w+( \w+)*)?(?P<path>(//?[^/]+)*)(///(?P<password>.*))?$")
 			.expect("constructed from known-good static value; qed");
 		let cap = re.captures(s).ok_or(SecretStringError::InvalidFormat)?;
 		let re_junction = Regex::new(r"/(/?[^/]+)")
@@ -367,7 +378,7 @@ pub trait Pair: Sized {
 		let path = re_junction.captures_iter(&cap["path"])
 			.map(|f| DeriveJunction::from(&f[1]));
 		Self::from_standard_components(
-			&cap["phrase"],
+			cap.name("phrase").map(|r| r.as_str()).unwrap_or(DEV_PHRASE),
 			password_override.or_else(|| cap.name("password").map(|m| m.as_str())),
 			path,
 		)

substrate/core/primitives/src/ed25519.rs

@@ -523,7 +523,15 @@ impl Pair {
 mod test {
 	use super::*;
 	use hex_literal::{hex, hex_impl};
-	use crate::Pair as _Pair;
+	use crate::{Pair as _Pair, crypto::DEV_PHRASE};
+
+	#[test]
+	fn default_phrase_should_be_used() {
+		assert_eq!(
+			Pair::from_string("//Alice///password", None).unwrap().public(),
+			Pair::from_string(&format!("{}//Alice", DEV_PHRASE), Some("password")).unwrap().public(),
+		);
+	}
 
 	#[test]
 	fn test_vector_should_work() {

substrate/core/primitives/src/sr25519.rs

@@ -366,11 +366,7 @@ impl AsRef<schnorrkel::Keypair> for Pair {
 /// Derive a single hard junction.
 #[cfg(feature = "std")]
 fn derive_hard_junction(secret: &SecretKey, cc: &[u8; CHAIN_CODE_LENGTH]) -> SecretKey {
-	("SchnorrRistrettoHDKD", &secret.to_bytes()[..], cc).using_encoded(|data|
-		MiniSecretKey::from_bytes(blake2_rfc::blake2b::blake2b(32, &[], data).as_bytes())
-			.expect("all 32-byte crypto-hash results are valid MiniSecretKeys; qed")
-			.expand()
-	)
+	secret.hard_derive_mini_secret_key(signing_context(b"SchnorrRistrettoHDKD").bytes(&cc[..])).expand()
 }
 
 #[cfg(feature = "std")]
@@ -507,9 +503,41 @@ impl Pair {
 #[cfg(test)]
 mod test {
 	use super::*;
-	use crate::Pair as _Pair;
+	use crate::{Pair as _Pair, crypto::{Ss58Codec, DEV_PHRASE, DEV_ADDRESS}};
 	use hex_literal::{hex, hex_impl};
-	
+
+	#[test]
+	fn default_phrase_should_be_used() {
+		assert_eq!(
+			Pair::from_string("//Alice///password", None).unwrap().public(),
+			Pair::from_string(&format!("{}//Alice", DEV_PHRASE), Some("password")).unwrap().public(),
+		);
+		assert_eq!(
+			Pair::from_string(&format!("{}/Alice", DEV_PHRASE), None).as_ref().map(Pair::public),
+			Pair::from_string("/Alice", None).as_ref().map(Pair::public)
+		);
+	}
+
+	#[test]
+	fn default_address_should_be_used() {
+		assert_eq!(
+			Public::from_string(&format!("{}/Alice", DEV_ADDRESS)),
+			Public::from_string("/Alice")
+		);
+	}
+
+	#[test]
+	fn default_phrase_should_correspond_to_default_address() {
+		assert_eq!(
+			Pair::from_string(&format!("{}/Alice", DEV_PHRASE), None).unwrap().public(),
+			Public::from_string(&format!("{}/Alice", DEV_ADDRESS)).unwrap(),
+		);
+		assert_eq!(
+			Pair::from_string("/Alice", None).unwrap().public(),
+			Public::from_string("/Alice").unwrap()
+		);
+	}
+
 	#[test]
 	fn derive_soft_should_work() {
 		let pair: Pair = Pair::from_seed(hex!(

substrate/node-template/src/chain_spec.rs

@@ -3,7 +3,6 @@ use node_template_runtime::{
 	AccountId, GenesisConfig, ConsensusConfig, TimestampConfig, BalancesConfig,
 	SudoConfig, IndicesConfig, FeesConfig,
 };
-use substrate_cli::DEV_PHRASE;
 use substrate_service;
 
 use ed25519::Public as AuthorityId;
@@ -26,13 +25,13 @@ pub enum Alternative {
 }
 
 fn authority_key(s: &str) -> AuthorityId {
-	ed25519::Pair::from_string(&format!("{}//{}", DEV_PHRASE, s), None)
+	ed25519::Pair::from_string(&format!("//{}", s), None)
 		.expect("static values are valid; qed")
 		.public()
 }
 
 fn account_key(s: &str) -> AccountId {
-	ed25519::Pair::from_string(&format!("{}//{}", DEV_PHRASE, s), None)
+	ed25519::Pair::from_string(&format!("//{}", s), None)
 		.expect("static values are valid; qed")
 		.public()
 }

substrate/node/cli/src/chain_spec.rs

@@ -25,7 +25,6 @@ pub use node_runtime::GenesisConfig;
 use substrate_service;
 use hex_literal::{hex, hex_impl};
 use substrate_telemetry::TelemetryEndpoints;
-use cli::DEV_PHRASE;
 
 const STAGING_TELEMETRY_URL: &str = "wss://telemetry.polkadot.io/submit/";
 
@@ -188,14 +187,14 @@ pub fn staging_testnet_config() -> ChainSpec {
 
 /// Helper function to generate AccountId from seed
 pub fn get_account_id_from_seed(seed: &str) -> AccountId {
-	sr25519::Pair::from_string(&format!("{}//{}", DEV_PHRASE, seed), None)
+	sr25519::Pair::from_string(&format!("//{}", seed), None)
 		.expect("static values are valid; qed")
 		.public()
 }
 
 /// Helper function to generate AuthorityId from seed
 pub fn get_session_key_from_seed(seed: &str) -> AuthorityId {
-	ed25519::Pair::from_string(&format!("{}//{}", DEV_PHRASE, seed), None)
+	ed25519::Pair::from_string(&format!("//{}", seed), None)
 		.expect("static values are valid; qed")
 		.public()
 }