From 46dd5304761318b5e5387d4ebffdc1d3e27dc636 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Tue, 18 Aug 2020 06:28:20 +0000 Subject: [PATCH] Allow import of Prysm keystores (#1535) ## Issue Addressed - Resolves #1361 ## Proposed Changes Loosens the constraints imposed by EIP-2335 so we can import keys from Prysm. ## Additional Info NA --- .../src/validator_definitions.rs | 25 ++++++++++- crypto/eth2_keystore/src/json_keystore/mod.rs | 6 ++- crypto/eth2_keystore/src/keystore.rs | 7 +-- crypto/eth2_keystore/tests/eip2335_vectors.rs | 4 +- crypto/eth2_keystore/tests/json.rs | 45 +++++++++++++++++-- crypto/eth2_wallet/tests/tests.rs | 4 +- 6 files changed, 78 insertions(+), 13 deletions(-) diff --git a/common/account_utils/src/validator_definitions.rs b/common/account_utils/src/validator_definitions.rs index c1f8e0ad5..733c771be 100644 --- a/common/account_utils/src/validator_definitions.rs +++ b/common/account_utils/src/validator_definitions.rs @@ -310,6 +310,15 @@ fn is_voting_keystore(file_name: &str) -> bool { return true; } + // The format exported by Prysm. I don't have a reference for this, but it was shared via + // Discord to Paul H. + if Regex::new("keystore-[0-9]+.json") + .expect("regex is valid") + .is_match(file_name) + { + return true; + } + false } @@ -318,8 +327,12 @@ mod tests { use super::*; #[test] - fn voting_keystore_filename() { + fn voting_keystore_filename_lighthouse() { assert!(is_voting_keystore(VOTING_KEYSTORE_FILE)); + } + + #[test] + fn voting_keystore_filename_launchpad() { assert!(!is_voting_keystore("cats")); assert!(!is_voting_keystore(&format!("a{}", VOTING_KEYSTORE_FILE))); assert!(!is_voting_keystore(&format!("{}b", VOTING_KEYSTORE_FILE))); @@ -337,4 +350,14 @@ mod tests { "keystore-m_12381_3600_1_0-1593476250.json" )); } + + #[test] + fn voting_keystore_filename_prysm() { + assert!(is_voting_keystore("keystore-0.json")); + assert!(is_voting_keystore("keystore-1.json")); + assert!(is_voting_keystore("keystore-101238259.json")); + assert!(!is_voting_keystore("keystore-.json")); + assert!(!is_voting_keystore("keystore-0a.json")); + assert!(!is_voting_keystore("keystore-cats.json")); + } } diff --git a/crypto/eth2_keystore/src/json_keystore/mod.rs b/crypto/eth2_keystore/src/json_keystore/mod.rs index a30855d21..0e2914388 100644 --- a/crypto/eth2_keystore/src/json_keystore/mod.rs +++ b/crypto/eth2_keystore/src/json_keystore/mod.rs @@ -24,10 +24,14 @@ use serde_repr::*; pub struct JsonKeystore { pub crypto: Crypto, pub uuid: Uuid, - pub path: String, + /// EIP-2335 does not declare this field as optional, but Prysm is omitting it so we must + /// support it. + pub path: Option, pub pubkey: String, pub version: Version, pub description: Option, + /// Not part of EIP-2335, but `ethdo` and Prysm have adopted it anyway so we must support it. + pub name: Option, } /// Version for `JsonKeystore`. diff --git a/crypto/eth2_keystore/src/keystore.rs b/crypto/eth2_keystore/src/keystore.rs index ede03275f..6e3128b00 100644 --- a/crypto/eth2_keystore/src/keystore.rs +++ b/crypto/eth2_keystore/src/keystore.rs @@ -172,10 +172,11 @@ impl Keystore { }, }, uuid, - path, + path: Some(path), pubkey: keypair.pk.to_hex_string()[2..].to_string(), version: Version::four(), description: None, + name: None, }, }) } @@ -218,8 +219,8 @@ impl Keystore { /// Returns the path for the keystore. /// /// Note: the path is not validated, it is simply whatever string the keystore provided. - pub fn path(&self) -> &str { - &self.json.path + pub fn path(&self) -> Option { + self.json.path.clone() } /// Returns the pubkey for the keystore. diff --git a/crypto/eth2_keystore/tests/eip2335_vectors.rs b/crypto/eth2_keystore/tests/eip2335_vectors.rs index 59d017c69..3702a2181 100644 --- a/crypto/eth2_keystore/tests/eip2335_vectors.rs +++ b/crypto/eth2_keystore/tests/eip2335_vectors.rs @@ -64,7 +64,7 @@ fn eip2335_test_vector_scrypt() { Uuid::parse_str("1d85ae20-35c5-4611-98e8-aa14a633906f").unwrap(), "uuid" ); - assert_eq!(keystore.path(), "", "path"); + assert_eq!(keystore.path().unwrap(), "", "path"); } #[test] @@ -108,5 +108,5 @@ fn eip2335_test_vector_pbkdf() { Uuid::parse_str("64625def-3331-4eea-ab6f-782f3ed16a83").unwrap(), "uuid" ); - assert_eq!(keystore.path(), "m/12381/60/0/0", "path"); + assert_eq!(keystore.path().unwrap(), "m/12381/60/0/0", "path"); } diff --git a/crypto/eth2_keystore/tests/json.rs b/crypto/eth2_keystore/tests/json.rs index c2466d36b..93e89156c 100644 --- a/crypto/eth2_keystore/tests/json.rs +++ b/crypto/eth2_keystore/tests/json.rs @@ -841,10 +841,7 @@ fn missing_path() { } "#; - match Keystore::from_json_str(&vector) { - Err(Error::InvalidJson(_)) => {} - _ => panic!("expected invalid json error"), - } + assert!(Keystore::from_json_str(&vector).is_ok()); } #[test] @@ -1010,3 +1007,43 @@ fn pbkdf2_missing_parameter() { _ => panic!("expected invalid json error"), } } + +#[test] +fn name_field() { + let vector = r#" + { + "crypto": { + "kdf": { + "function": "scrypt", + "params": { + "dklen": 32, + "n": 262144, + "p": 1, + "r": 8, + "salt": "d4e56740f876aef8c010b86a40d5f56745a118d0906a34e69aec8c0db1cb8fa3" + }, + "message": "" + }, + "checksum": { + "function": "sha256", + "params": {}, + "message": "149aafa27b041f3523c53d7acba1905fa6b1c90f9fef137568101f44b531a3cb" + }, + "cipher": { + "function": "aes-128-ctr", + "params": { + "iv": "264daa3f303d7259501c93d997d84fe6" + }, + "message": "54ecc8863c0550351eee5720f3be6a5d4a016025aa91cd6436cfec938d6a8d30" + } + }, + "pubkey": "9612d7a727c9d0a22e185a1c768478dfe919cada9266988cb32359c11f2b7b27f4ae4040902382ae2910c15e2b420d07", + "uuid": "1d85ae20-35c5-4611-98e8-aa14a633906f", + "path": "", + "version": 4, + "name": "cats" + } + "#; + + assert!(Keystore::from_json_str(&vector).is_ok()); +} diff --git a/crypto/eth2_wallet/tests/tests.rs b/crypto/eth2_wallet/tests/tests.rs index d75b0ab34..eb683e72d 100644 --- a/crypto/eth2_wallet/tests/tests.rs +++ b/crypto/eth2_wallet/tests/tests.rs @@ -225,13 +225,13 @@ fn key_derivation_from_seed() { .expect("should generate keystores"); assert_eq!( - keystores.voting.path(), + keystores.voting.path().unwrap(), format!("m/12381/3600/{}/0/0", i), "voting path should match" ); assert_eq!( - keystores.withdrawal.path(), + keystores.withdrawal.path().unwrap(), format!("m/12381/3600/{}/0", i), "withdrawal path should match" );