From 9c860d6698a3b51340e981708c2e94cfa7f9fd96 Mon Sep 17 00:00:00 2001 From: Piotr Chromiec Date: Wed, 8 Aug 2018 20:19:59 +0200 Subject: [PATCH 01/21] support for keystore format produced by pyethereum lib + some debug msgs --- Cargo.lock | 2 ++ ethstore/cli/Cargo.toml | 2 ++ ethstore/cli/src/main.rs | 11 ++++++++++- ethstore/src/account/crypto.rs | 6 ++++-- ethstore/src/account/kdf.rs | 2 +- ethstore/src/accounts_dir/disk.rs | 11 ++++++++--- ethstore/src/accounts_dir/memory.rs | 2 +- ethstore/src/accounts_dir/mod.rs | 5 +++-- ethstore/src/accounts_dir/vault.rs | 1 + ethstore/src/json/crypto.rs | 4 +++- ethstore/src/json/hash.rs | 7 +++++++ ethstore/src/json/kdf.rs | 12 ++++++++---- ethstore/src/json/key_file.rs | 6 ++++-- 13 files changed, 54 insertions(+), 17 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9969ce27909..923a469037a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1003,7 +1003,9 @@ version = "0.1.0" dependencies = [ "dir 0.1.2", "docopt 0.8.3 (registry+https://github.com/rust-lang/crates.io-index)", + "env_logger 0.5.13 (registry+https://github.com/rust-lang/crates.io-index)", "ethstore 0.2.0", + "log 0.4.5 (registry+https://github.com/rust-lang/crates.io-index)", "num_cpus 1.8.0 (registry+https://github.com/rust-lang/crates.io-index)", "panic_hook 0.1.0", "parking_lot 0.6.4 (registry+https://github.com/rust-lang/crates.io-index)", diff --git a/ethstore/cli/Cargo.toml b/ethstore/cli/Cargo.toml index b1736efdb38..bda62aef87a 100644 --- a/ethstore/cli/Cargo.toml +++ b/ethstore/cli/Cargo.toml @@ -4,6 +4,8 @@ version = "0.1.0" authors = ["Parity Technologies "] [dependencies] +log = "0.4" +env_logger = "0.5.13" docopt = "0.8" num_cpus = "1.6" rustc-hex = "1.0" diff --git a/ethstore/cli/src/main.rs b/ethstore/cli/src/main.rs index 6751f713c6e..2113fc0a01f 100644 --- a/ethstore/cli/src/main.rs +++ b/ethstore/cli/src/main.rs @@ -23,6 +23,10 @@ extern crate parking_lot; extern crate rustc_hex; extern crate serde; +#[macro_use] +extern crate log; +extern crate env_logger; + #[macro_use] extern crate serde_derive; @@ -146,6 +150,10 @@ impl fmt::Display for Error { fn main() { panic_hook::set_abort(); + if env::var("RUST_LOG").is_err() { + env::set_var("RUST_LOG", "info") + } + env_logger::init(); match execute(env::args()) { Ok(result) => println!("{}", result), @@ -169,6 +177,7 @@ fn key_dir(location: &str) -> Result, Error> { path => Box::new(RootDiskDirectory::create(path)?), }; + info!("using key dir {:?}", dir); Ok(dir) } @@ -235,7 +244,7 @@ fn execute(command: I) -> Result where I: IntoIterator = accounts .into_iter() .filter(|a| &a.vault == &vault_ref) - .map(|a| a.address) + .map(|a| a.address ) .collect(); Ok(format_accounts(&accounts)) } else if args.cmd_import { diff --git a/ethstore/src/account/crypto.rs b/ethstore/src/account/crypto.rs index 07f84af7fa4..5ce7e174901 100644 --- a/ethstore/src/account/crypto.rs +++ b/ethstore/src/account/crypto.rs @@ -79,12 +79,14 @@ impl Crypto { /// Encrypt custom plain data pub fn with_plain(plain: &[u8], password: &Password, iterations: u32) -> Result { - let salt: [u8; 32] = Random::random(); + let salt_arr: [u8; 32] = Random::random(); + let salt: Vec = salt_arr.to_vec(); let iv: [u8; 16] = Random::random(); // two parts of derived key // DK = [ DK[0..15] DK[16..31] ] = [derived_left_bits, derived_right_bits] - let (derived_left_bits, derived_right_bits) = crypto::derive_key_iterations(password.as_bytes(), &salt, iterations); + let (derived_left_bits, derived_right_bits) = + crypto::derive_key_iterations(password.as_bytes(), &salt, iterations); // preallocated (on-stack in case of `Secret`) buffer to hold cipher // length = length(plain) as we are using CTR-approach diff --git a/ethstore/src/account/kdf.rs b/ethstore/src/account/kdf.rs index 4d6d7cd956d..09cb6742f6d 100644 --- a/ethstore/src/account/kdf.rs +++ b/ethstore/src/account/kdf.rs @@ -26,7 +26,7 @@ pub struct Pbkdf2 { pub c: u32, pub dklen: u32, pub prf: Prf, - pub salt: [u8; 32], + pub salt: Vec, } #[derive(Debug, PartialEq, Clone)] diff --git a/ethstore/src/accounts_dir/disk.rs b/ethstore/src/accounts_dir/disk.rs index cf4841e7489..3b5bb7dd0b5 100644 --- a/ethstore/src/accounts_dir/disk.rs +++ b/ethstore/src/accounts_dir/disk.rs @@ -23,6 +23,7 @@ use {json, SafeAccount, Error}; use json::Uuid; use super::{KeyDirectory, VaultKeyDirectory, VaultKeyDirectoryProvider, VaultKey}; use super::vault::{VAULT_FILE_NAME, VaultDiskDirectory}; +use std::fmt::Debug; const IGNORED_FILES: &'static [&'static str] = &[ "thumbs.db", @@ -103,7 +104,7 @@ pub fn replace_file_with_permissions_to_owner(file_path: &Path) -> io::Result; /// Disk directory key file manager -pub trait KeyFileManager: Send + Sync { +pub trait KeyFileManager: Send + Sync + Debug { /// Read `SafeAccount` from given key file stream fn read(&self, filename: Option, reader: T) -> Result where T: io::Read; /// Write `SafeAccount` to given key file stream @@ -111,12 +112,14 @@ pub trait KeyFileManager: Send + Sync { } /// Disk-based keys directory implementation +#[derive(Debug)] pub struct DiskDirectory where T: KeyFileManager { path: PathBuf, key_manager: T, } /// Keys file manager for root keys directory +#[derive(Debug)] pub struct DiskKeyFileManager; impl RootDiskDirectory { @@ -191,7 +194,7 @@ impl DiskDirectory where T: KeyFileManager { .map_err(Into::into) .and_then(|file| self.key_manager.read(filename, file)) .map_err(|err| { - warn!("Invalid key file: {:?} ({})", path, err); + warn!("Invalid key file: {:?}\n\t ({})", path, err); err }) .map(|account| (path, account)) @@ -318,7 +321,9 @@ impl VaultKeyDirectoryProvider for DiskDirectory where T: KeyFileManager { impl KeyFileManager for DiskKeyFileManager { fn read(&self, filename: Option, reader: T) -> Result where T: io::Read { - let key_file = json::KeyFile::load(reader).map_err(|e| Error::Custom(format!("{:?}", e)))?; + info!("reading file: {:?}", filename); + let key_file = json::KeyFile::load(reader).map_err(|e| { + Error::Custom(format!("{:?}", e))})?; Ok(SafeAccount::from_file(key_file, filename)) } diff --git a/ethstore/src/accounts_dir/memory.rs b/ethstore/src/accounts_dir/memory.rs index 71ddfa536e3..4a3c969ab85 100644 --- a/ethstore/src/accounts_dir/memory.rs +++ b/ethstore/src/accounts_dir/memory.rs @@ -23,7 +23,7 @@ use {SafeAccount, Error}; use super::KeyDirectory; /// Accounts in-memory storage. -#[derive(Default)] +#[derive(Default, Debug)] pub struct MemoryDirectory { accounts: RwLock>>, } diff --git a/ethstore/src/accounts_dir/mod.rs b/ethstore/src/accounts_dir/mod.rs index 1191a73d2de..637d3e30334 100644 --- a/ethstore/src/accounts_dir/mod.rs +++ b/ethstore/src/accounts_dir/mod.rs @@ -36,7 +36,7 @@ pub enum SetKeyError { } /// Vault key -#[derive(Clone, PartialEq, Eq)] +#[derive(Clone, PartialEq, Eq, Debug)] pub struct VaultKey { /// Vault password pub password: Password, @@ -45,7 +45,7 @@ pub struct VaultKey { } /// Keys directory -pub trait KeyDirectory: Send + Sync { +pub trait KeyDirectory: Send + Sync + Debug { /// Read keys from directory fn load(&self) -> Result, Error>; /// Insert new key to directory @@ -93,6 +93,7 @@ pub trait VaultKeyDirectory: KeyDirectory { pub use self::disk::{RootDiskDirectory, DiskKeyFileManager, KeyFileManager}; pub use self::memory::MemoryDirectory; pub use self::vault::VaultDiskDirectory; +use std::fmt::Debug; impl VaultKey { /// Create new vault key diff --git a/ethstore/src/accounts_dir/vault.rs b/ethstore/src/accounts_dir/vault.rs index 249a9c6dc3f..90970048f57 100644 --- a/ethstore/src/accounts_dir/vault.rs +++ b/ethstore/src/accounts_dir/vault.rs @@ -32,6 +32,7 @@ pub const VAULT_TEMP_FILE_NAME: &'static str = "vault_temp.json"; pub type VaultDiskDirectory = DiskDirectory; /// Vault key file manager +#[derive(Debug)] pub struct VaultKeyFileManager { name: String, key: VaultKey, diff --git a/ethstore/src/json/crypto.rs b/ethstore/src/json/crypto.rs index 0a926cc83fe..c490a307950 100644 --- a/ethstore/src/json/crypto.rs +++ b/ethstore/src/json/crypto.rs @@ -52,6 +52,7 @@ enum CryptoField { Kdf, KdfParams, Mac, + Ignored, } impl<'a> Deserialize<'a> for CryptoField { @@ -81,7 +82,7 @@ impl<'a> Visitor<'a> for CryptoFieldVisitor { "kdf" => Ok(CryptoField::Kdf), "kdfparams" => Ok(CryptoField::KdfParams), "mac" => Ok(CryptoField::Mac), - _ => Err(Error::custom(format!("Unknown field: '{}'", value))), + _ => Ok(CryptoField::Ignored), } } } @@ -122,6 +123,7 @@ impl<'a> Visitor<'a> for CryptoVisitor { Some(CryptoField::Kdf) => { kdf = Some(visitor.next_value()?); } Some(CryptoField::KdfParams) => { kdfparams = Some(visitor.next_value()?); } Some(CryptoField::Mac) => { mac = Some(visitor.next_value()?); } + Some(CryptoField::Ignored) => { let _ : i32 = visitor.next_value()?; () } None => { break; } } } diff --git a/ethstore/src/json/hash.rs b/ethstore/src/json/hash.rs index c2ad547734f..6d1e6f827a4 100644 --- a/ethstore/src/json/hash.rs +++ b/ethstore/src/json/hash.rs @@ -24,6 +24,12 @@ macro_rules! impl_hash { ($name: ident, $size: expr) => { pub struct $name([u8; $size]); + impl $name { + pub fn zero() -> Self { + $name([0u8; $size]) + } + } + impl fmt::Debug for $name { fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> { let self_ref: &[u8] = &self.0; @@ -83,6 +89,7 @@ macro_rules! impl_hash { type Err = Error; fn from_str(value: &str) -> Result { + debug!("{} {:?}", stringify!($name), value); match value.from_hex() { Ok(ref hex) if hex.len() == $size => { let mut hash = [0u8; $size]; diff --git a/ethstore/src/json/kdf.rs b/ethstore/src/json/kdf.rs index f8df3c2285f..198da6c5e5a 100644 --- a/ethstore/src/json/kdf.rs +++ b/ethstore/src/json/kdf.rs @@ -17,7 +17,7 @@ use std::fmt; use serde::{Serialize, Serializer, Deserialize, Deserializer}; use serde::de::{Visitor, Error as SerdeError}; -use super::{Error, H256}; +use super::{Error, H256, Bytes}; #[derive(Debug, PartialEq)] pub enum KdfSer { @@ -111,7 +111,7 @@ pub struct Pbkdf2 { pub c: u32, pub dklen: u32, pub prf: Prf, - pub salt: H256, + pub salt: Bytes, } #[derive(Debug, PartialEq, Serialize, Deserialize)] @@ -145,10 +145,14 @@ impl<'a> Deserialize<'a> for KdfSerParams { use serde_json::{Value, from_value}; let v: Value = Deserialize::deserialize(deserializer)?; + debug!("v={:?}", v); - from_value(v.clone()).map(KdfSerParams::Pbkdf2) + let x = from_value(v.clone()).map(KdfSerParams::Pbkdf2) .or_else(|_| from_value(v).map(KdfSerParams::Scrypt)) - .map_err(|_| D::Error::custom("Invalid KDF algorithm")) + .map_err(|_| D::Error::custom("Invalid KDF algorithm")); + + debug!("x={:?}", x); + x } } diff --git a/ethstore/src/json/key_file.rs b/ethstore/src/json/key_file.rs index 2c3cf3fdd5d..5c5ab34b9da 100644 --- a/ethstore/src/json/key_file.rs +++ b/ethstore/src/json/key_file.rs @@ -21,6 +21,8 @@ use serde::de::{Error, Visitor, MapAccess, DeserializeOwned}; use serde_json; use super::{Uuid, Version, Crypto, H160}; +pub type Address = H160; + /// Public opaque type representing serializable `KeyFile`. #[derive(Debug, PartialEq)] pub struct OpaqueKeyFile { @@ -46,7 +48,7 @@ pub struct KeyFile { pub id: Uuid, pub version: Version, pub crypto: Crypto, - pub address: H160, + pub address: Address, pub name: Option, pub meta: Option, } @@ -160,7 +162,7 @@ impl<'a> Visitor<'a> for KeyFileVisitor { let address = match address { Some(address) => address, - None => return Err(V::Error::missing_field("address")), + None => Address::zero(), }; let result = KeyFile { From 5e7e9ab2b35bf07580cb1197339f245c0876101c Mon Sep 17 00:00:00 2001 From: Piotr Chromiec Date: Thu, 9 Aug 2018 19:12:15 +0200 Subject: [PATCH 02/21] made salt unbound also for Scrypt --- ethstore/cli/src/main.rs | 2 +- ethstore/src/account/kdf.rs | 2 +- ethstore/src/accounts_dir/disk.rs | 3 +-- ethstore/src/json/bytes.rs | 2 +- ethstore/src/json/kdf.rs | 12 ++++-------- 5 files changed, 8 insertions(+), 13 deletions(-) diff --git a/ethstore/cli/src/main.rs b/ethstore/cli/src/main.rs index 2113fc0a01f..ca4892492db 100644 --- a/ethstore/cli/src/main.rs +++ b/ethstore/cli/src/main.rs @@ -244,7 +244,7 @@ fn execute(command: I) -> Result where I: IntoIterator = accounts .into_iter() .filter(|a| &a.vault == &vault_ref) - .map(|a| a.address ) + .map(|a| a.address) .collect(); Ok(format_accounts(&accounts)) } else if args.cmd_import { diff --git a/ethstore/src/account/kdf.rs b/ethstore/src/account/kdf.rs index 09cb6742f6d..e8d001917f9 100644 --- a/ethstore/src/account/kdf.rs +++ b/ethstore/src/account/kdf.rs @@ -35,7 +35,7 @@ pub struct Scrypt { pub p: u32, pub n: u32, pub r: u32, - pub salt: [u8; 32], + pub salt: Vec, } #[derive(Debug, PartialEq, Clone)] diff --git a/ethstore/src/accounts_dir/disk.rs b/ethstore/src/accounts_dir/disk.rs index 3b5bb7dd0b5..91e89086c78 100644 --- a/ethstore/src/accounts_dir/disk.rs +++ b/ethstore/src/accounts_dir/disk.rs @@ -322,8 +322,7 @@ impl VaultKeyDirectoryProvider for DiskDirectory where T: KeyFileManager { impl KeyFileManager for DiskKeyFileManager { fn read(&self, filename: Option, reader: T) -> Result where T: io::Read { info!("reading file: {:?}", filename); - let key_file = json::KeyFile::load(reader).map_err(|e| { - Error::Custom(format!("{:?}", e))})?; + let key_file = json::KeyFile::load(reader).map_err(|e| Error::Custom(format!("{:?}", e)))?; Ok(SafeAccount::from_file(key_file, filename)) } diff --git a/ethstore/src/json/bytes.rs b/ethstore/src/json/bytes.rs index b5aae19222a..827c1d2021a 100644 --- a/ethstore/src/json/bytes.rs +++ b/ethstore/src/json/bytes.rs @@ -19,7 +19,7 @@ use serde::{Deserialize, Deserializer, Serialize, Serializer}; use serde::de::Error; use rustc_hex::{ToHex, FromHex, FromHexError}; -#[derive(Debug, PartialEq)] +#[derive(Debug, PartialEq, Clone)] pub struct Bytes(Vec); impl ops::Deref for Bytes { diff --git a/ethstore/src/json/kdf.rs b/ethstore/src/json/kdf.rs index 198da6c5e5a..d7b3f08f590 100644 --- a/ethstore/src/json/kdf.rs +++ b/ethstore/src/json/kdf.rs @@ -17,7 +17,7 @@ use std::fmt; use serde::{Serialize, Serializer, Deserialize, Deserializer}; use serde::de::{Visitor, Error as SerdeError}; -use super::{Error, H256, Bytes}; +use super::{Error, Bytes}; #[derive(Debug, PartialEq)] pub enum KdfSer { @@ -120,7 +120,7 @@ pub struct Scrypt { pub p: u32, pub n: u32, pub r: u32, - pub salt: H256, + pub salt: Bytes, } #[derive(Debug, PartialEq)] @@ -145,14 +145,10 @@ impl<'a> Deserialize<'a> for KdfSerParams { use serde_json::{Value, from_value}; let v: Value = Deserialize::deserialize(deserializer)?; - debug!("v={:?}", v); - let x = from_value(v.clone()).map(KdfSerParams::Pbkdf2) + from_value(v.clone()).map(KdfSerParams::Pbkdf2) .or_else(|_| from_value(v).map(KdfSerParams::Scrypt)) - .map_err(|_| D::Error::custom("Invalid KDF algorithm")); - - debug!("x={:?}", x); - x + .map_err(|_| D::Error::custom("Invalid KDF algorithm")) } } From cff6f23d8756d9a57ef27e4a58875bad38346091 Mon Sep 17 00:00:00 2001 From: Piotr Chromiec Date: Thu, 9 Aug 2018 21:49:08 +0200 Subject: [PATCH 03/21] made address optional and skip if its none --- ethstore/src/account/safe_account.rs | 36 +++++++++++++++------------- ethstore/src/accounts_dir/disk.rs | 2 +- ethstore/src/ethstore.rs | 2 +- ethstore/src/json/crypto.rs | 2 +- ethstore/src/json/key_file.rs | 7 +----- 5 files changed, 24 insertions(+), 25 deletions(-) diff --git a/ethstore/src/account/safe_account.rs b/ethstore/src/account/safe_account.rs index 849dafab103..76c4e8656c8 100644 --- a/ethstore/src/account/safe_account.rs +++ b/ethstore/src/account/safe_account.rs @@ -45,7 +45,7 @@ impl Into for SafeAccount { json::KeyFile { id: From::from(self.id), version: self.version.into(), - address: self.address.into(), + address: Some(self.address.into()), crypto: self.crypto.into(), name: Some(self.name.into()), meta: Some(self.meta.into()), @@ -64,28 +64,32 @@ impl SafeAccount { meta: String ) -> Result { Ok(SafeAccount { - id: id, + id, version: Version::V3, crypto: Crypto::with_secret(keypair.secret(), password, iterations)?, address: keypair.address(), filename: None, - name: name, - meta: meta, + name, + meta, }) } /// Create a new `SafeAccount` from the given `json`; if it was read from a /// file, the `filename` should be `Some` name. If it is as yet anonymous, then it /// can be left `None`. - pub fn from_file(json: json::KeyFile, filename: Option) -> Self { - SafeAccount { - id: json.id.into(), - version: json.version.into(), - address: json.address.into(), - crypto: json.crypto.into(), - filename: filename, - name: json.name.unwrap_or(String::new()), - meta: json.meta.unwrap_or("{}".to_owned()), + pub fn from_file(json: json::KeyFile, filename: Option) -> Result { + match json.address { + None => Err(Error::Custom(format!("file {:?} has no address. Skipping.", filename))), + Some(address) => + Ok(SafeAccount { + id: json.id.into(), + version: json.version.into(), + address: address.into(), + crypto: json.crypto.into(), + filename, + name: json.name.unwrap_or(String::new()), + meta: json.meta.unwrap_or("{}".to_owned()), + }) } } @@ -97,14 +101,14 @@ impl SafeAccount { let meta_plain = meta_crypto.decrypt(password)?; let meta_plain = json::VaultKeyMeta::load(&meta_plain).map_err(|e| Error::Custom(format!("{:?}", e)))?; - Ok(SafeAccount::from_file(json::KeyFile { + SafeAccount::from_file(json::KeyFile { id: json.id, version: json.version, crypto: json.crypto, - address: meta_plain.address, + address: Some(meta_plain.address), name: meta_plain.name, meta: meta_plain.meta, - }, filename)) + }, filename) } /// Create a new `VaultKeyFile` from the given `self` diff --git a/ethstore/src/accounts_dir/disk.rs b/ethstore/src/accounts_dir/disk.rs index 91e89086c78..3f2d1b2ba55 100644 --- a/ethstore/src/accounts_dir/disk.rs +++ b/ethstore/src/accounts_dir/disk.rs @@ -323,7 +323,7 @@ impl KeyFileManager for DiskKeyFileManager { fn read(&self, filename: Option, reader: T) -> Result where T: io::Read { info!("reading file: {:?}", filename); let key_file = json::KeyFile::load(reader).map_err(|e| Error::Custom(format!("{:?}", e)))?; - Ok(SafeAccount::from_file(key_file, filename)) + SafeAccount::from_file(key_file, filename) } fn write(&self, mut account: SafeAccount, writer: &mut T) -> Result<(), Error> where T: io::Write { diff --git a/ethstore/src/ethstore.rs b/ethstore/src/ethstore.rs index 2deafbad667..1d7104181f2 100644 --- a/ethstore/src/ethstore.rs +++ b/ethstore/src/ethstore.rs @@ -168,7 +168,7 @@ impl SecretStore for EthStore { fn import_wallet(&self, vault: SecretVaultRef, json: &[u8], password: &Password, gen_id: bool) -> Result { let json_keyfile = json::KeyFile::load(json).map_err(|_| Error::InvalidKeyFile("Invalid JSON format".to_owned()))?; - let mut safe_account = SafeAccount::from_file(json_keyfile, None); + let mut safe_account = SafeAccount::from_file(json_keyfile, None)?; if gen_id { safe_account.id = Random::random(); diff --git a/ethstore/src/json/crypto.rs b/ethstore/src/json/crypto.rs index c490a307950..a49c44d3f2f 100644 --- a/ethstore/src/json/crypto.rs +++ b/ethstore/src/json/crypto.rs @@ -123,7 +123,7 @@ impl<'a> Visitor<'a> for CryptoVisitor { Some(CryptoField::Kdf) => { kdf = Some(visitor.next_value()?); } Some(CryptoField::KdfParams) => { kdfparams = Some(visitor.next_value()?); } Some(CryptoField::Mac) => { mac = Some(visitor.next_value()?); } - Some(CryptoField::Ignored) => { let _ : i32 = visitor.next_value()?; () } + Some(CryptoField::Ignored) => { let _ignored : i32 = visitor.next_value()?; () } None => { break; } } } diff --git a/ethstore/src/json/key_file.rs b/ethstore/src/json/key_file.rs index 5c5ab34b9da..961b5e38c40 100644 --- a/ethstore/src/json/key_file.rs +++ b/ethstore/src/json/key_file.rs @@ -48,7 +48,7 @@ pub struct KeyFile { pub id: Uuid, pub version: Version, pub crypto: Crypto, - pub address: Address, + pub address: Option
, pub name: Option, pub meta: Option, } @@ -160,11 +160,6 @@ impl<'a> Visitor<'a> for KeyFileVisitor { None => return Err(V::Error::missing_field("crypto")), }; - let address = match address { - Some(address) => address, - None => Address::zero(), - }; - let result = KeyFile { id: id, version: version, From 012dd5bda96828e1c61011389b6887d8fe93be44 Mon Sep 17 00:00:00 2001 From: Piotr Chromiec Date: Thu, 9 Aug 2018 21:57:29 +0200 Subject: [PATCH 04/21] ignore values of any type, not just i32 --- ethstore/src/json/crypto.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ethstore/src/json/crypto.rs b/ethstore/src/json/crypto.rs index a49c44d3f2f..76dff305054 100644 --- a/ethstore/src/json/crypto.rs +++ b/ethstore/src/json/crypto.rs @@ -123,7 +123,7 @@ impl<'a> Visitor<'a> for CryptoVisitor { Some(CryptoField::Kdf) => { kdf = Some(visitor.next_value()?); } Some(CryptoField::KdfParams) => { kdfparams = Some(visitor.next_value()?); } Some(CryptoField::Mac) => { mac = Some(visitor.next_value()?); } - Some(CryptoField::Ignored) => { let _ignored : i32 = visitor.next_value()?; () } + Some(CryptoField::Ignored) => { visitor.next_value().unwrap_or(()) } None => { break; } } } From 09d5a6645694adfe66bfdca3714b29ed96e4a797 Mon Sep 17 00:00:00 2001 From: Piotr Chromiec Date: Wed, 29 Aug 2018 02:54:02 +0200 Subject: [PATCH 05/21] WIP: local deps --- ethkey/Cargo.toml | 1 + ethstore/Cargo.toml | 1 + 2 files changed, 2 insertions(+) diff --git a/ethkey/Cargo.toml b/ethkey/Cargo.toml index 140bcb174c5..8c41ae34df2 100644 --- a/ethkey/Cargo.toml +++ b/ethkey/Cargo.toml @@ -7,6 +7,7 @@ authors = ["Parity Technologies "] byteorder = "1.0" edit-distance = "2.0" parity-crypto = "0.1" +#parity-crypto = { path = "../../parity-common/parity-crypto" } eth-secp256k1 = { git = "https://github.com/paritytech/rust-secp256k1" } ethereum-types = "0.4" lazy_static = "1.0" diff --git a/ethstore/Cargo.toml b/ethstore/Cargo.toml index 7393baf2c72..3bb205bb3e5 100644 --- a/ethstore/Cargo.toml +++ b/ethstore/Cargo.toml @@ -17,6 +17,7 @@ time = "0.1.34" itertools = "0.5" parking_lot = "0.6" parity-crypto = "0.1" +# parity-crypto = { path = "../../parity-common/parity-crypto" } ethereum-types = "0.4" dir = { path = "../util/dir" } smallvec = "0.6" From b86fcb0716dc49db68f64ec9dec30d2d4dd28498 Mon Sep 17 00:00:00 2001 From: Piotr Chromiec Date: Thu, 30 Aug 2018 19:58:16 +0200 Subject: [PATCH 06/21] cleanup --- ethstore/cli/src/main.rs | 2 +- ethstore/src/accounts_dir/disk.rs | 5 +---- ethstore/src/accounts_dir/memory.rs | 2 +- ethstore/src/accounts_dir/mod.rs | 5 ++--- ethstore/src/accounts_dir/vault.rs | 1 - ethstore/src/json/hash.rs | 7 ------- 6 files changed, 5 insertions(+), 17 deletions(-) diff --git a/ethstore/cli/src/main.rs b/ethstore/cli/src/main.rs index ca4892492db..65887aaf4e4 100644 --- a/ethstore/cli/src/main.rs +++ b/ethstore/cli/src/main.rs @@ -177,7 +177,7 @@ fn key_dir(location: &str) -> Result, Error> { path => Box::new(RootDiskDirectory::create(path)?), }; - info!("using key dir {:?}", dir); + info!("using key dir {:?}", dir.path()); Ok(dir) } diff --git a/ethstore/src/accounts_dir/disk.rs b/ethstore/src/accounts_dir/disk.rs index 3f2d1b2ba55..2f6cf75ea4a 100644 --- a/ethstore/src/accounts_dir/disk.rs +++ b/ethstore/src/accounts_dir/disk.rs @@ -23,7 +23,6 @@ use {json, SafeAccount, Error}; use json::Uuid; use super::{KeyDirectory, VaultKeyDirectory, VaultKeyDirectoryProvider, VaultKey}; use super::vault::{VAULT_FILE_NAME, VaultDiskDirectory}; -use std::fmt::Debug; const IGNORED_FILES: &'static [&'static str] = &[ "thumbs.db", @@ -104,7 +103,7 @@ pub fn replace_file_with_permissions_to_owner(file_path: &Path) -> io::Result; /// Disk directory key file manager -pub trait KeyFileManager: Send + Sync + Debug { +pub trait KeyFileManager: Send + Sync { /// Read `SafeAccount` from given key file stream fn read(&self, filename: Option, reader: T) -> Result where T: io::Read; /// Write `SafeAccount` to given key file stream @@ -112,14 +111,12 @@ pub trait KeyFileManager: Send + Sync + Debug { } /// Disk-based keys directory implementation -#[derive(Debug)] pub struct DiskDirectory where T: KeyFileManager { path: PathBuf, key_manager: T, } /// Keys file manager for root keys directory -#[derive(Debug)] pub struct DiskKeyFileManager; impl RootDiskDirectory { diff --git a/ethstore/src/accounts_dir/memory.rs b/ethstore/src/accounts_dir/memory.rs index 4a3c969ab85..71ddfa536e3 100644 --- a/ethstore/src/accounts_dir/memory.rs +++ b/ethstore/src/accounts_dir/memory.rs @@ -23,7 +23,7 @@ use {SafeAccount, Error}; use super::KeyDirectory; /// Accounts in-memory storage. -#[derive(Default, Debug)] +#[derive(Default)] pub struct MemoryDirectory { accounts: RwLock>>, } diff --git a/ethstore/src/accounts_dir/mod.rs b/ethstore/src/accounts_dir/mod.rs index 637d3e30334..1191a73d2de 100644 --- a/ethstore/src/accounts_dir/mod.rs +++ b/ethstore/src/accounts_dir/mod.rs @@ -36,7 +36,7 @@ pub enum SetKeyError { } /// Vault key -#[derive(Clone, PartialEq, Eq, Debug)] +#[derive(Clone, PartialEq, Eq)] pub struct VaultKey { /// Vault password pub password: Password, @@ -45,7 +45,7 @@ pub struct VaultKey { } /// Keys directory -pub trait KeyDirectory: Send + Sync + Debug { +pub trait KeyDirectory: Send + Sync { /// Read keys from directory fn load(&self) -> Result, Error>; /// Insert new key to directory @@ -93,7 +93,6 @@ pub trait VaultKeyDirectory: KeyDirectory { pub use self::disk::{RootDiskDirectory, DiskKeyFileManager, KeyFileManager}; pub use self::memory::MemoryDirectory; pub use self::vault::VaultDiskDirectory; -use std::fmt::Debug; impl VaultKey { /// Create new vault key diff --git a/ethstore/src/accounts_dir/vault.rs b/ethstore/src/accounts_dir/vault.rs index 90970048f57..249a9c6dc3f 100644 --- a/ethstore/src/accounts_dir/vault.rs +++ b/ethstore/src/accounts_dir/vault.rs @@ -32,7 +32,6 @@ pub const VAULT_TEMP_FILE_NAME: &'static str = "vault_temp.json"; pub type VaultDiskDirectory = DiskDirectory; /// Vault key file manager -#[derive(Debug)] pub struct VaultKeyFileManager { name: String, key: VaultKey, diff --git a/ethstore/src/json/hash.rs b/ethstore/src/json/hash.rs index 6d1e6f827a4..c2ad547734f 100644 --- a/ethstore/src/json/hash.rs +++ b/ethstore/src/json/hash.rs @@ -24,12 +24,6 @@ macro_rules! impl_hash { ($name: ident, $size: expr) => { pub struct $name([u8; $size]); - impl $name { - pub fn zero() -> Self { - $name([0u8; $size]) - } - } - impl fmt::Debug for $name { fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> { let self_ref: &[u8] = &self.0; @@ -89,7 +83,6 @@ macro_rules! impl_hash { type Err = Error; fn from_str(value: &str) -> Result { - debug!("{} {:?}", stringify!($name), value); match value.from_hex() { Ok(ref hex) if hex.len() == $size => { let mut hash = [0u8; $size]; From e9f2d110caa408588a393588df7da49462b843fa Mon Sep 17 00:00:00 2001 From: Piotr Chromiec Date: Fri, 31 Aug 2018 04:24:18 +0200 Subject: [PATCH 07/21] enable optional address + more cleanup --- ethstore/cli/src/main.rs | 1 - ethstore/src/account/safe_account.rs | 36 +++++++++++++--------------- ethstore/src/accounts_dir/disk.rs | 5 ++-- ethstore/src/ethstore.rs | 2 +- ethstore/src/json/hash.rs | 6 +++++ ethstore/src/json/key_file.rs | 9 ++++--- 6 files changed, 31 insertions(+), 28 deletions(-) diff --git a/ethstore/cli/src/main.rs b/ethstore/cli/src/main.rs index 65887aaf4e4..c0592ed8782 100644 --- a/ethstore/cli/src/main.rs +++ b/ethstore/cli/src/main.rs @@ -177,7 +177,6 @@ fn key_dir(location: &str) -> Result, Error> { path => Box::new(RootDiskDirectory::create(path)?), }; - info!("using key dir {:?}", dir.path()); Ok(dir) } diff --git a/ethstore/src/account/safe_account.rs b/ethstore/src/account/safe_account.rs index 76c4e8656c8..849dafab103 100644 --- a/ethstore/src/account/safe_account.rs +++ b/ethstore/src/account/safe_account.rs @@ -45,7 +45,7 @@ impl Into for SafeAccount { json::KeyFile { id: From::from(self.id), version: self.version.into(), - address: Some(self.address.into()), + address: self.address.into(), crypto: self.crypto.into(), name: Some(self.name.into()), meta: Some(self.meta.into()), @@ -64,32 +64,28 @@ impl SafeAccount { meta: String ) -> Result { Ok(SafeAccount { - id, + id: id, version: Version::V3, crypto: Crypto::with_secret(keypair.secret(), password, iterations)?, address: keypair.address(), filename: None, - name, - meta, + name: name, + meta: meta, }) } /// Create a new `SafeAccount` from the given `json`; if it was read from a /// file, the `filename` should be `Some` name. If it is as yet anonymous, then it /// can be left `None`. - pub fn from_file(json: json::KeyFile, filename: Option) -> Result { - match json.address { - None => Err(Error::Custom(format!("file {:?} has no address. Skipping.", filename))), - Some(address) => - Ok(SafeAccount { - id: json.id.into(), - version: json.version.into(), - address: address.into(), - crypto: json.crypto.into(), - filename, - name: json.name.unwrap_or(String::new()), - meta: json.meta.unwrap_or("{}".to_owned()), - }) + pub fn from_file(json: json::KeyFile, filename: Option) -> Self { + SafeAccount { + id: json.id.into(), + version: json.version.into(), + address: json.address.into(), + crypto: json.crypto.into(), + filename: filename, + name: json.name.unwrap_or(String::new()), + meta: json.meta.unwrap_or("{}".to_owned()), } } @@ -101,14 +97,14 @@ impl SafeAccount { let meta_plain = meta_crypto.decrypt(password)?; let meta_plain = json::VaultKeyMeta::load(&meta_plain).map_err(|e| Error::Custom(format!("{:?}", e)))?; - SafeAccount::from_file(json::KeyFile { + Ok(SafeAccount::from_file(json::KeyFile { id: json.id, version: json.version, crypto: json.crypto, - address: Some(meta_plain.address), + address: meta_plain.address, name: meta_plain.name, meta: meta_plain.meta, - }, filename) + }, filename)) } /// Create a new `VaultKeyFile` from the given `self` diff --git a/ethstore/src/accounts_dir/disk.rs b/ethstore/src/accounts_dir/disk.rs index 2f6cf75ea4a..cf4841e7489 100644 --- a/ethstore/src/accounts_dir/disk.rs +++ b/ethstore/src/accounts_dir/disk.rs @@ -191,7 +191,7 @@ impl DiskDirectory where T: KeyFileManager { .map_err(Into::into) .and_then(|file| self.key_manager.read(filename, file)) .map_err(|err| { - warn!("Invalid key file: {:?}\n\t ({})", path, err); + warn!("Invalid key file: {:?} ({})", path, err); err }) .map(|account| (path, account)) @@ -318,9 +318,8 @@ impl VaultKeyDirectoryProvider for DiskDirectory where T: KeyFileManager { impl KeyFileManager for DiskKeyFileManager { fn read(&self, filename: Option, reader: T) -> Result where T: io::Read { - info!("reading file: {:?}", filename); let key_file = json::KeyFile::load(reader).map_err(|e| Error::Custom(format!("{:?}", e)))?; - SafeAccount::from_file(key_file, filename) + Ok(SafeAccount::from_file(key_file, filename)) } fn write(&self, mut account: SafeAccount, writer: &mut T) -> Result<(), Error> where T: io::Write { diff --git a/ethstore/src/ethstore.rs b/ethstore/src/ethstore.rs index 1d7104181f2..2deafbad667 100644 --- a/ethstore/src/ethstore.rs +++ b/ethstore/src/ethstore.rs @@ -168,7 +168,7 @@ impl SecretStore for EthStore { fn import_wallet(&self, vault: SecretVaultRef, json: &[u8], password: &Password, gen_id: bool) -> Result { let json_keyfile = json::KeyFile::load(json).map_err(|_| Error::InvalidKeyFile("Invalid JSON format".to_owned()))?; - let mut safe_account = SafeAccount::from_file(json_keyfile, None)?; + let mut safe_account = SafeAccount::from_file(json_keyfile, None); if gen_id { safe_account.id = Random::random(); diff --git a/ethstore/src/json/hash.rs b/ethstore/src/json/hash.rs index c2ad547734f..a35555ada15 100644 --- a/ethstore/src/json/hash.rs +++ b/ethstore/src/json/hash.rs @@ -24,6 +24,12 @@ macro_rules! impl_hash { ($name: ident, $size: expr) => { pub struct $name([u8; $size]); + impl $name { + pub fn zero() -> Self { + $name([0u8; $size]) + } + } + impl fmt::Debug for $name { fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> { let self_ref: &[u8] = &self.0; diff --git a/ethstore/src/json/key_file.rs b/ethstore/src/json/key_file.rs index 961b5e38c40..674001083c2 100644 --- a/ethstore/src/json/key_file.rs +++ b/ethstore/src/json/key_file.rs @@ -21,8 +21,6 @@ use serde::de::{Error, Visitor, MapAccess, DeserializeOwned}; use serde_json; use super::{Uuid, Version, Crypto, H160}; -pub type Address = H160; - /// Public opaque type representing serializable `KeyFile`. #[derive(Debug, PartialEq)] pub struct OpaqueKeyFile { @@ -48,7 +46,7 @@ pub struct KeyFile { pub id: Uuid, pub version: Version, pub crypto: Crypto, - pub address: Option
, + pub address: H160, pub name: Option, pub meta: Option, } @@ -160,6 +158,11 @@ impl<'a> Visitor<'a> for KeyFileVisitor { None => return Err(V::Error::missing_field("crypto")), }; + let address = match address { + Some(address) => address, + None => H160::zero(), + }; + let result = KeyFile { id: id, version: version, From b74936bf2e7b1dc63f2c7c54183263dd07acd2ba Mon Sep 17 00:00:00 2001 From: Piotr Chromiec Date: Fri, 31 Aug 2018 04:38:01 +0200 Subject: [PATCH 08/21] yet another cleanup --- ethstore/cli/src/main.rs | 9 --------- 1 file changed, 9 deletions(-) diff --git a/ethstore/cli/src/main.rs b/ethstore/cli/src/main.rs index c0592ed8782..fd89147fd1b 100644 --- a/ethstore/cli/src/main.rs +++ b/ethstore/cli/src/main.rs @@ -23,10 +23,6 @@ extern crate parking_lot; extern crate rustc_hex; extern crate serde; -#[macro_use] -extern crate log; -extern crate env_logger; - #[macro_use] extern crate serde_derive; @@ -149,11 +145,6 @@ impl fmt::Display for Error { } fn main() { - panic_hook::set_abort(); - if env::var("RUST_LOG").is_err() { - env::set_var("RUST_LOG", "info") - } - env_logger::init(); match execute(env::args()) { Ok(result) => println!("{}", result), From 216fcd3e811a7faa9910e0ecb31ebe57882dd3f4 Mon Sep 17 00:00:00 2001 From: Piotr Chromiec Date: Sat, 6 Oct 2018 00:49:47 +0200 Subject: [PATCH 09/21] enable keystore wo address import (using passwd) --- ethkey/cli/src/main.rs | 2 +- ethstore/cli/Cargo.toml | 2 +- ethstore/cli/src/main.rs | 16 +++++++++----- ethstore/src/account/crypto.rs | 5 ++--- ethstore/src/account/safe_account.rs | 31 +++++++++++++++++++--------- ethstore/src/accounts_dir/disk.rs | 27 ++++++++++++++++-------- ethstore/src/accounts_dir/mod.rs | 9 +++++++- ethstore/src/ethstore.rs | 2 +- ethstore/src/import.rs | 10 +++++---- ethstore/src/json/hash.rs | 6 ------ ethstore/src/json/key_file.rs | 7 +------ 11 files changed, 70 insertions(+), 47 deletions(-) diff --git a/ethkey/cli/src/main.rs b/ethkey/cli/src/main.rs index e908baec32b..ba42cc80cd7 100644 --- a/ethkey/cli/src/main.rs +++ b/ethkey/cli/src/main.rs @@ -168,7 +168,7 @@ fn main() { Ok(ok) => println!("{}", ok), Err(Error::Docopt(ref e)) => e.exit(), Err(err) => { - println!("{}", err); + error!("{}", err); process::exit(1); } } diff --git a/ethstore/cli/Cargo.toml b/ethstore/cli/Cargo.toml index bda62aef87a..fb08c614ec6 100644 --- a/ethstore/cli/Cargo.toml +++ b/ethstore/cli/Cargo.toml @@ -5,7 +5,7 @@ authors = ["Parity Technologies "] [dependencies] log = "0.4" -env_logger = "0.5.13" +env_logger = "0.5" docopt = "0.8" num_cpus = "1.6" rustc-hex = "1.0" diff --git a/ethstore/cli/src/main.rs b/ethstore/cli/src/main.rs index fd89147fd1b..f66c2f5fd55 100644 --- a/ethstore/cli/src/main.rs +++ b/ethstore/cli/src/main.rs @@ -16,6 +16,9 @@ extern crate dir; extern crate docopt; +#[macro_use] +extern crate log; +extern crate env_logger; extern crate ethstore; extern crate num_cpus; extern crate panic_hook; @@ -45,7 +48,7 @@ Usage: ethstore insert [--dir DIR] [--vault VAULT] [--vault-pwd VAULTPWD] ethstore change-pwd
[--dir DIR] [--vault VAULT] [--vault-pwd VAULTPWD] ethstore list [--dir DIR] [--vault VAULT] [--vault-pwd VAULTPWD] - ethstore import [--src DIR] [--dir DIR] + ethstore import [] [--src DIR] [--dir DIR] ethstore import-wallet [--dir DIR] [--vault VAULT] [--vault-pwd VAULTPWD] ethstore find-wallet-pass ethstore remove
[--dir DIR] [--vault VAULT] [--vault-pwd VAULTPWD] @@ -145,12 +148,14 @@ impl fmt::Display for Error { } fn main() { + panic_hook::set_abort(); + env_logger::try_init().expect("Logger initialized only once."); match execute(env::args()) { Ok(result) => println!("{}", result), Err(Error::Docopt(ref e)) => e.exit(), Err(err) => { - println!("{}", err); + error!("{}", err); process::exit(1); } } @@ -201,9 +206,9 @@ fn format_vaults(vaults: &[String]) -> String { } fn load_password(path: &str) -> Result { - let mut file = fs::File::open(path).map_err(|e| ethstore::Error::Custom(format!("Error opening password file {}: {}", path, e)))?; + let mut file = fs::File::open(path).map_err(|e| ethstore::Error::Custom(format!("Error opening password file '{}': {}", path, e)))?; let mut password = String::new(); - file.read_to_string(&mut password).map_err(|e| ethstore::Error::Custom(format!("Error reading password file {}: {}", path, e)))?; + file.read_to_string(&mut password).map_err(|e| ethstore::Error::Custom(format!("Error reading password file '{}': {}", path, e)))?; // drop EOF let _ = password.pop(); Ok(password.into()) @@ -240,7 +245,8 @@ fn execute(command: I) -> Result where I: IntoIterator Result { - let salt_arr: [u8; 32] = Random::random(); - let salt: Vec = salt_arr.to_vec(); + let salt: [u8; 32] = Random::random(); let iv: [u8; 16] = Random::random(); // two parts of derived key @@ -106,7 +105,7 @@ impl Crypto { ciphertext: ciphertext.into_vec(), kdf: Kdf::Pbkdf2(Pbkdf2 { dklen: crypto::KEY_LENGTH as u32, - salt: salt, + salt: salt.to_vec(), c: iterations, prf: Prf::HmacSha256, }), diff --git a/ethstore/src/account/safe_account.rs b/ethstore/src/account/safe_account.rs index 849dafab103..180a786ed2b 100644 --- a/ethstore/src/account/safe_account.rs +++ b/ethstore/src/account/safe_account.rs @@ -45,7 +45,7 @@ impl Into for SafeAccount { json::KeyFile { id: From::from(self.id), version: self.version.into(), - address: self.address.into(), + address: Some(self.address.into()), crypto: self.crypto.into(), name: Some(self.name.into()), meta: Some(self.meta.into()), @@ -77,16 +77,27 @@ impl SafeAccount { /// Create a new `SafeAccount` from the given `json`; if it was read from a /// file, the `filename` should be `Some` name. If it is as yet anonymous, then it /// can be left `None`. - pub fn from_file(json: json::KeyFile, filename: Option) -> Self { - SafeAccount { + pub fn from_file(json: json::KeyFile, filename: Option, password: &Option) -> Result { + let crypto = Crypto::from(json.crypto); + let address = match json.address { + Some(address) => address.into(), + None => match password { + Some(password) => + KeyPair::from_secret(crypto.secret(&password).map_err(|_| Error::InvalidPassword)?)?.address(), + None => + return Err(Error::Custom("This keystore does not contain address. You need to provide password to import it".to_owned())) + } + }; + + Ok(SafeAccount { id: json.id.into(), version: json.version.into(), - address: json.address.into(), - crypto: json.crypto.into(), - filename: filename, + address, + crypto, + filename, name: json.name.unwrap_or(String::new()), meta: json.meta.unwrap_or("{}".to_owned()), - } + }) } /// Create a new `SafeAccount` from the given vault `json`; if it was read from a @@ -97,14 +108,14 @@ impl SafeAccount { let meta_plain = meta_crypto.decrypt(password)?; let meta_plain = json::VaultKeyMeta::load(&meta_plain).map_err(|e| Error::Custom(format!("{:?}", e)))?; - Ok(SafeAccount::from_file(json::KeyFile { + SafeAccount::from_file(json::KeyFile { id: json.id, version: json.version, crypto: json.crypto, - address: meta_plain.address, + address: Some(meta_plain.address), name: meta_plain.name, meta: meta_plain.meta, - }, filename)) + }, filename, &None) } /// Create a new `VaultKeyFile` from the given `self` diff --git a/ethstore/src/accounts_dir/disk.rs b/ethstore/src/accounts_dir/disk.rs index cf4841e7489..2d46b30cc30 100644 --- a/ethstore/src/accounts_dir/disk.rs +++ b/ethstore/src/accounts_dir/disk.rs @@ -23,6 +23,7 @@ use {json, SafeAccount, Error}; use json::Uuid; use super::{KeyDirectory, VaultKeyDirectory, VaultKeyDirectoryProvider, VaultKey}; use super::vault::{VAULT_FILE_NAME, VaultDiskDirectory}; +use ethkey::Password; const IGNORED_FILES: &'static [&'static str] = &[ "thumbs.db", @@ -104,8 +105,16 @@ pub type RootDiskDirectory = DiskDirectory; /// Disk directory key file manager pub trait KeyFileManager: Send + Sync { + /// Read `SafeAccount` from given key file stream with password + #[allow(unused_variables)] + fn read_with_password(&self, filename: Option, reader: T, password: &Option) + -> Result where T: io::Read { + unimplemented!() + } /// Read `SafeAccount` from given key file stream - fn read(&self, filename: Option, reader: T) -> Result where T: io::Read; + fn read(&self, filename: Option, reader: T) -> Result where T: io::Read { + self.read_with_password(filename, reader, &None) + } /// Write `SafeAccount` to given key file stream fn write(&self, account: SafeAccount, writer: &mut T) -> Result<(), Error> where T: io::Write; } @@ -179,7 +188,7 @@ impl DiskDirectory where T: KeyFileManager { } /// all accounts found in keys directory - fn files_content(&self) -> Result, Error> { + fn files_content(&self, password: &Option) -> Result, Error> { // it's not done using one iterator cause // there is an issue with rustc and it takes tooo much time to compile let paths = self.files()?; @@ -189,9 +198,9 @@ impl DiskDirectory where T: KeyFileManager { let filename = Some(path.file_name().and_then(|n| n.to_str()).expect("Keys have valid UTF8 names only.").to_owned()); fs::File::open(path.clone()) .map_err(Into::into) - .and_then(|file| self.key_manager.read(filename, file)) + .and_then(|file| self.key_manager.read_with_password(filename, file, password)) .map_err(|err| { - warn!("Invalid key file: {:?} ({})", path, err); + error!("Invalid key file: {:?} ({})", path, err); err }) .map(|account| (path, account)) @@ -241,8 +250,8 @@ impl DiskDirectory where T: KeyFileManager { } impl KeyDirectory for DiskDirectory where T: KeyFileManager { - fn load(&self) -> Result, Error> { - let accounts = self.files_content()? + fn load_with_password(&self, password: &Option) -> Result, Error> { + let accounts = self.files_content(password)? .into_iter() .map(|(_, account)| account) .collect(); @@ -263,7 +272,7 @@ impl KeyDirectory for DiskDirectory where T: KeyFileManager { fn remove(&self, account: &SafeAccount) -> Result<(), Error> { // enumerate all entries in keystore // and find entry with given address - let to_remove = self.files_content()? + let to_remove = self.files_content(&None)? .into_iter() .find(|&(_, ref acc)| acc.id == account.id && acc.address == account.address); @@ -317,9 +326,9 @@ impl VaultKeyDirectoryProvider for DiskDirectory where T: KeyFileManager { } impl KeyFileManager for DiskKeyFileManager { - fn read(&self, filename: Option, reader: T) -> Result where T: io::Read { + fn read_with_password(&self, filename: Option, reader: T, password: &Option) -> Result where T: io::Read { let key_file = json::KeyFile::load(reader).map_err(|e| Error::Custom(format!("{:?}", e)))?; - Ok(SafeAccount::from_file(key_file, filename)) + SafeAccount::from_file(key_file, filename, password) } fn write(&self, mut account: SafeAccount, writer: &mut T) -> Result<(), Error> where T: io::Write { diff --git a/ethstore/src/accounts_dir/mod.rs b/ethstore/src/accounts_dir/mod.rs index 1191a73d2de..e6889525bcc 100644 --- a/ethstore/src/accounts_dir/mod.rs +++ b/ethstore/src/accounts_dir/mod.rs @@ -46,8 +46,15 @@ pub struct VaultKey { /// Keys directory pub trait KeyDirectory: Send + Sync { + /// Read keys from directory with password + #[allow(unused_variables)] + fn load_with_password(&self, password: &Option) -> Result, Error> { + unimplemented!() + } /// Read keys from directory - fn load(&self) -> Result, Error>; + fn load(&self) -> Result, Error> { + self.load_with_password(&None) + } /// Insert new key to directory fn insert(&self, account: SafeAccount) -> Result; /// Update key in the directory diff --git a/ethstore/src/ethstore.rs b/ethstore/src/ethstore.rs index 2deafbad667..574ac8c3da0 100644 --- a/ethstore/src/ethstore.rs +++ b/ethstore/src/ethstore.rs @@ -168,7 +168,7 @@ impl SecretStore for EthStore { fn import_wallet(&self, vault: SecretVaultRef, json: &[u8], password: &Password, gen_id: bool) -> Result { let json_keyfile = json::KeyFile::load(json).map_err(|_| Error::InvalidKeyFile("Invalid JSON format".to_owned()))?; - let mut safe_account = SafeAccount::from_file(json_keyfile, None); + let mut safe_account = SafeAccount::from_file(json_keyfile, None, &None)?; if gen_id { safe_account.id = Random::random(); diff --git a/ethstore/src/import.rs b/ethstore/src/import.rs index 876119fd50a..64a30edaaab 100644 --- a/ethstore/src/import.rs +++ b/ethstore/src/import.rs @@ -18,7 +18,7 @@ use std::collections::HashSet; use std::path::Path; use std::fs; -use ethkey::Address; +use ethkey::{Address, Password}; use accounts_dir::{KeyDirectory, RootDiskDirectory, DiskKeyFileManager, KeyFileManager}; use dir; use Error; @@ -40,9 +40,11 @@ pub fn import_account(path: &Path, dst: &KeyDirectory) -> Result } /// Import all accounts from one directory to the other. -pub fn import_accounts(src: &KeyDirectory, dst: &KeyDirectory) -> Result, Error> { - let accounts = src.load()?; - let existing_accounts = dst.load()?.into_iter().map(|a| a.address).collect::>(); +pub fn import_accounts(src: &KeyDirectory, dst: &KeyDirectory, password: &Option) -> Result, Error> { + let accounts = src.load_with_password(password)?; + let existing_accounts = dst.load()?.into_iter() + .map(|a| a.address) + .collect::>(); accounts.into_iter() .filter(|a| !existing_accounts.contains(&a.address)) diff --git a/ethstore/src/json/hash.rs b/ethstore/src/json/hash.rs index a35555ada15..c2ad547734f 100644 --- a/ethstore/src/json/hash.rs +++ b/ethstore/src/json/hash.rs @@ -24,12 +24,6 @@ macro_rules! impl_hash { ($name: ident, $size: expr) => { pub struct $name([u8; $size]); - impl $name { - pub fn zero() -> Self { - $name([0u8; $size]) - } - } - impl fmt::Debug for $name { fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> { let self_ref: &[u8] = &self.0; diff --git a/ethstore/src/json/key_file.rs b/ethstore/src/json/key_file.rs index 674001083c2..10d9a6e5ae1 100644 --- a/ethstore/src/json/key_file.rs +++ b/ethstore/src/json/key_file.rs @@ -46,7 +46,7 @@ pub struct KeyFile { pub id: Uuid, pub version: Version, pub crypto: Crypto, - pub address: H160, + pub address: Option, pub name: Option, pub meta: Option, } @@ -158,11 +158,6 @@ impl<'a> Visitor<'a> for KeyFileVisitor { None => return Err(V::Error::missing_field("crypto")), }; - let address = match address { - Some(address) => address, - None => H160::zero(), - }; - let result = KeyFile { id: id, version: version, From 1ba0de3fae00053c8cc423bf8ded2261967a677c Mon Sep 17 00:00:00 2001 From: Piotr Chromiec Date: Sat, 6 Oct 2018 00:57:08 +0200 Subject: [PATCH 10/21] cleanup --- ethkey/Cargo.toml | 1 - ethstore/Cargo.toml | 1 - 2 files changed, 2 deletions(-) diff --git a/ethkey/Cargo.toml b/ethkey/Cargo.toml index 8c41ae34df2..140bcb174c5 100644 --- a/ethkey/Cargo.toml +++ b/ethkey/Cargo.toml @@ -7,7 +7,6 @@ authors = ["Parity Technologies "] byteorder = "1.0" edit-distance = "2.0" parity-crypto = "0.1" -#parity-crypto = { path = "../../parity-common/parity-crypto" } eth-secp256k1 = { git = "https://github.com/paritytech/rust-secp256k1" } ethereum-types = "0.4" lazy_static = "1.0" diff --git a/ethstore/Cargo.toml b/ethstore/Cargo.toml index 3bb205bb3e5..7393baf2c72 100644 --- a/ethstore/Cargo.toml +++ b/ethstore/Cargo.toml @@ -17,7 +17,6 @@ time = "0.1.34" itertools = "0.5" parking_lot = "0.6" parity-crypto = "0.1" -# parity-crypto = { path = "../../parity-common/parity-crypto" } ethereum-types = "0.4" dir = { path = "../util/dir" } smallvec = "0.6" From 0f31116b00425ab9ad803e2351c8aa70b77c19bc Mon Sep 17 00:00:00 2001 From: Piotr Chromiec Date: Sat, 6 Oct 2018 01:02:12 +0200 Subject: [PATCH 11/21] doc enchancement --- ethstore/src/account/safe_account.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ethstore/src/account/safe_account.rs b/ethstore/src/account/safe_account.rs index 180a786ed2b..a8a3ce856ca 100644 --- a/ethstore/src/account/safe_account.rs +++ b/ethstore/src/account/safe_account.rs @@ -76,7 +76,7 @@ impl SafeAccount { /// Create a new `SafeAccount` from the given `json`; if it was read from a /// file, the `filename` should be `Some` name. If it is as yet anonymous, then it - /// can be left `None`. + /// can be left `None`. `password` is used for reading keystore files w/o address. pub fn from_file(json: json::KeyFile, filename: Option, password: &Option) -> Result { let crypto = Crypto::from(json.crypto); let address = match json.address { From 93f782bb9f03476220f53dd7548a0ae61860ea4f Mon Sep 17 00:00:00 2001 From: Piotr Chromiec Date: Tue, 9 Oct 2018 21:46:49 +0200 Subject: [PATCH 12/21] parity/account fix --- parity/account.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/parity/account.rs b/parity/account.rs index e09667379fa..ed138b86817 100644 --- a/parity/account.rs +++ b/parity/account.rs @@ -118,7 +118,7 @@ fn import(i: ImportAccounts) -> Result { let path = PathBuf::from(path); if path.is_dir() { let from = RootDiskDirectory::at(&path); - imported += import_accounts(&from, &to).map_err(|e| format!("Importing accounts from {:?} failed: {}", path, e))?.len(); + imported += import_accounts(&from, &to, &None).map_err(|e| format!("Importing accounts from {:?} failed: {}", path, e))?.len(); } else if path.is_file() { import_account(&path, &to).map_err(|e| format!("Importing account from {:?} failed: {}", path, e))?; imported += 1; From fc34ade62a28d318aecac68a2e7d72991e9a5083 Mon Sep 17 00:00:00 2001 From: Piotr Chromiec Date: Tue, 27 Nov 2018 19:43:54 +0100 Subject: [PATCH 13/21] redesign after review --- Cargo.lock | 12 ++++---- ethkey/cli/src/main.rs | 2 +- ethstore/Cargo.toml | 2 +- ethstore/cli/Cargo.toml | 4 +-- ethstore/cli/src/main.rs | 34 ++++++++++----------- ethstore/src/account/safe_account.rs | 11 +++++-- ethstore/src/accounts_dir/disk.rs | 45 ++++++++++++++++------------ ethstore/src/accounts_dir/mod.rs | 9 +----- ethstore/src/ethstore.rs | 2 +- ethstore/src/import.rs | 8 ++--- ethstore/src/json/bytes.rs | 2 +- ethstore/src/json/key_file.rs | 6 ++-- ethstore/src/lib.rs | 1 - parity/account.rs | 2 +- 14 files changed, 70 insertions(+), 70 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 923a469037a..85402367d18 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -539,7 +539,7 @@ dependencies = [ "ethereum-types 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)", "ethjson 0.1.0", "ethkey 0.3.0", - "ethstore 0.2.0", + "ethstore 0.2.1", "evm 0.1.0", "fake-hardware-wallet 0.0.1", "hardware-wallet 1.12.0", @@ -974,7 +974,7 @@ dependencies = [ [[package]] name = "ethstore" -version = "0.2.0" +version = "0.2.1" dependencies = [ "dir 0.1.2", "ethereum-types 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)", @@ -999,13 +999,11 @@ dependencies = [ [[package]] name = "ethstore-cli" -version = "0.1.0" +version = "0.1.1" dependencies = [ "dir 0.1.2", "docopt 0.8.3 (registry+https://github.com/rust-lang/crates.io-index)", - "env_logger 0.5.13 (registry+https://github.com/rust-lang/crates.io-index)", - "ethstore 0.2.0", - "log 0.4.5 (registry+https://github.com/rust-lang/crates.io-index)", + "ethstore 0.2.1", "num_cpus 1.8.0 (registry+https://github.com/rust-lang/crates.io-index)", "panic_hook 0.1.0", "parking_lot 0.6.4 (registry+https://github.com/rust-lang/crates.io-index)", @@ -2206,7 +2204,7 @@ dependencies = [ "ethereum-types 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)", "ethjson 0.1.0", "ethkey 0.3.0", - "ethstore 0.2.0", + "ethstore 0.2.1", "fake-fetch 0.0.1", "fake-hardware-wallet 0.0.1", "fastmap 0.1.0", diff --git a/ethkey/cli/src/main.rs b/ethkey/cli/src/main.rs index ba42cc80cd7..71ea5ca6c4d 100644 --- a/ethkey/cli/src/main.rs +++ b/ethkey/cli/src/main.rs @@ -168,7 +168,7 @@ fn main() { Ok(ok) => println!("{}", ok), Err(Error::Docopt(ref e)) => e.exit(), Err(err) => { - error!("{}", err); + eprintln!("{}", err); process::exit(1); } } diff --git a/ethstore/Cargo.toml b/ethstore/Cargo.toml index 7393baf2c72..1175d12139e 100644 --- a/ethstore/Cargo.toml +++ b/ethstore/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "ethstore" -version = "0.2.0" +version = "0.2.1" authors = ["Parity Technologies "] [dependencies] diff --git a/ethstore/cli/Cargo.toml b/ethstore/cli/Cargo.toml index fb08c614ec6..8c0634801ab 100644 --- a/ethstore/cli/Cargo.toml +++ b/ethstore/cli/Cargo.toml @@ -1,11 +1,9 @@ [package] name = "ethstore-cli" -version = "0.1.0" +version = "0.1.1" authors = ["Parity Technologies "] [dependencies] -log = "0.4" -env_logger = "0.5" docopt = "0.8" num_cpus = "1.6" rustc-hex = "1.0" diff --git a/ethstore/cli/src/main.rs b/ethstore/cli/src/main.rs index f66c2f5fd55..dc93b899e2f 100644 --- a/ethstore/cli/src/main.rs +++ b/ethstore/cli/src/main.rs @@ -16,9 +16,6 @@ extern crate dir; extern crate docopt; -#[macro_use] -extern crate log; -extern crate env_logger; extern crate ethstore; extern crate num_cpus; extern crate panic_hook; @@ -149,31 +146,30 @@ impl fmt::Display for Error { fn main() { panic_hook::set_abort(); - env_logger::try_init().expect("Logger initialized only once."); match execute(env::args()) { Ok(result) => println!("{}", result), Err(Error::Docopt(ref e)) => e.exit(), Err(err) => { - error!("{}", err); + eprintln!("{}", err); process::exit(1); } } } -fn key_dir(location: &str) -> Result, Error> { - let dir: Box = match location { - "geth" => Box::new(RootDiskDirectory::create(dir::geth(false))?), - "geth-test" => Box::new(RootDiskDirectory::create(dir::geth(true))?), +fn key_dir(location: &str, password: Option) -> Result, Error> { + let dir: RootDiskDirectory = match location { + "geth" => RootDiskDirectory::create(dir::geth(false))?, + "geth-test" => RootDiskDirectory::create(dir::geth(true))?, path if path.starts_with("parity") => { let chain = path.split('-').nth(1).unwrap_or("ethereum"); let path = dir::parity(chain); - Box::new(RootDiskDirectory::create(path)?) + RootDiskDirectory::create(path)? }, - path => Box::new(RootDiskDirectory::create(path)?), + path => RootDiskDirectory::create(path)?, }; - Ok(dir) + Ok(Box::new(dir.with_password(password))) } fn open_args_vault(store: &EthStore, args: &Args) -> Result { @@ -218,7 +214,7 @@ fn execute(command: I) -> Result where I: IntoIterator(command: I) -> Result where I: IntoIterator None, + _ => Some(load_password(&args.arg_password)?) + }; + let src = key_dir(&args.flag_src, password)?; + let dst = key_dir(&args.flag_dir, None)?; + + let accounts = import_accounts(&*src, &*dst)?; Ok(format_accounts(&accounts)) } else if args.cmd_import_wallet { let wallet = PresaleWallet::open(&args.arg_path)?; diff --git a/ethstore/src/account/safe_account.rs b/ethstore/src/account/safe_account.rs index a8a3ce856ca..95f21d694c9 100644 --- a/ethstore/src/account/safe_account.rs +++ b/ethstore/src/account/safe_account.rs @@ -76,8 +76,13 @@ impl SafeAccount { /// Create a new `SafeAccount` from the given `json`; if it was read from a /// file, the `filename` should be `Some` name. If it is as yet anonymous, then it - /// can be left `None`. `password` is used for reading keystore files w/o address. - pub fn from_file(json: json::KeyFile, filename: Option, password: &Option) -> Result { + /// can be left `None`. + pub fn from_file(json: json::KeyFile, filename: Option) -> Result { + SafeAccount::from_file_with_password(json, filename, &None) + } + + /// Same as [`from_file`](#method.from_file), but with `password` which enables reading keystore files w/o address. + pub fn from_file_with_password(json: json::KeyFile, filename: Option, password: &Option) -> Result { let crypto = Crypto::from(json.crypto); let address = match json.address { Some(address) => address.into(), @@ -115,7 +120,7 @@ impl SafeAccount { address: Some(meta_plain.address), name: meta_plain.name, meta: meta_plain.meta, - }, filename, &None) + }, filename) } /// Create a new `VaultKeyFile` from the given `self` diff --git a/ethstore/src/accounts_dir/disk.rs b/ethstore/src/accounts_dir/disk.rs index 2d46b30cc30..e0c93bd375e 100644 --- a/ethstore/src/accounts_dir/disk.rs +++ b/ethstore/src/accounts_dir/disk.rs @@ -105,16 +105,9 @@ pub type RootDiskDirectory = DiskDirectory; /// Disk directory key file manager pub trait KeyFileManager: Send + Sync { - /// Read `SafeAccount` from given key file stream with password - #[allow(unused_variables)] - fn read_with_password(&self, filename: Option, reader: T, password: &Option) - -> Result where T: io::Read { - unimplemented!() - } /// Read `SafeAccount` from given key file stream - fn read(&self, filename: Option, reader: T) -> Result where T: io::Read { - self.read_with_password(filename, reader, &None) - } + fn read(&self, filename: Option, reader: T) -> Result where T: io::Read; + /// Write `SafeAccount` to given key file stream fn write(&self, account: SafeAccount, writer: &mut T) -> Result<(), Error> where T: io::Write; } @@ -126,7 +119,16 @@ pub struct DiskDirectory where T: KeyFileManager { } /// Keys file manager for root keys directory -pub struct DiskKeyFileManager; +pub struct DiskKeyFileManager { + password: Option, +} + +impl DiskKeyFileManager { + /// Create new disk key manager w/o password + pub fn new() -> Self { + DiskKeyFileManager { password: None } + } +} impl RootDiskDirectory { pub fn create

(path: P) -> Result where P: AsRef { @@ -134,8 +136,13 @@ impl RootDiskDirectory { Ok(Self::at(path)) } + pub fn with_password(&self, password: Option) -> Self { + DiskDirectory::new(&self.path, DiskKeyFileManager { password }) + } + + pub fn at

(path: P) -> Self where P: AsRef { - DiskDirectory::new(path, DiskKeyFileManager) + DiskDirectory::new(path, DiskKeyFileManager::new()) } } @@ -188,7 +195,7 @@ impl DiskDirectory where T: KeyFileManager { } /// all accounts found in keys directory - fn files_content(&self, password: &Option) -> Result, Error> { + fn files_content(&self) -> Result, Error> { // it's not done using one iterator cause // there is an issue with rustc and it takes tooo much time to compile let paths = self.files()?; @@ -198,9 +205,9 @@ impl DiskDirectory where T: KeyFileManager { let filename = Some(path.file_name().and_then(|n| n.to_str()).expect("Keys have valid UTF8 names only.").to_owned()); fs::File::open(path.clone()) .map_err(Into::into) - .and_then(|file| self.key_manager.read_with_password(filename, file, password)) + .and_then(|file| self.key_manager.read(filename, file)) .map_err(|err| { - error!("Invalid key file: {:?} ({})", path, err); + eprintln!("Invalid key file: {:?} ({})", path, err); err }) .map(|account| (path, account)) @@ -250,8 +257,8 @@ impl DiskDirectory where T: KeyFileManager { } impl KeyDirectory for DiskDirectory where T: KeyFileManager { - fn load_with_password(&self, password: &Option) -> Result, Error> { - let accounts = self.files_content(password)? + fn load(&self) -> Result, Error> { + let accounts = self.files_content()? .into_iter() .map(|(_, account)| account) .collect(); @@ -272,7 +279,7 @@ impl KeyDirectory for DiskDirectory where T: KeyFileManager { fn remove(&self, account: &SafeAccount) -> Result<(), Error> { // enumerate all entries in keystore // and find entry with given address - let to_remove = self.files_content(&None)? + let to_remove = self.files_content()? .into_iter() .find(|&(_, ref acc)| acc.id == account.id && acc.address == account.address); @@ -326,9 +333,9 @@ impl VaultKeyDirectoryProvider for DiskDirectory where T: KeyFileManager { } impl KeyFileManager for DiskKeyFileManager { - fn read_with_password(&self, filename: Option, reader: T, password: &Option) -> Result where T: io::Read { + fn read(&self, filename: Option, reader: T) -> Result where T: io::Read { let key_file = json::KeyFile::load(reader).map_err(|e| Error::Custom(format!("{:?}", e)))?; - SafeAccount::from_file(key_file, filename, password) + SafeAccount::from_file_with_password(key_file, filename, &self.password) } fn write(&self, mut account: SafeAccount, writer: &mut T) -> Result<(), Error> where T: io::Write { diff --git a/ethstore/src/accounts_dir/mod.rs b/ethstore/src/accounts_dir/mod.rs index e6889525bcc..1191a73d2de 100644 --- a/ethstore/src/accounts_dir/mod.rs +++ b/ethstore/src/accounts_dir/mod.rs @@ -46,15 +46,8 @@ pub struct VaultKey { /// Keys directory pub trait KeyDirectory: Send + Sync { - /// Read keys from directory with password - #[allow(unused_variables)] - fn load_with_password(&self, password: &Option) -> Result, Error> { - unimplemented!() - } /// Read keys from directory - fn load(&self) -> Result, Error> { - self.load_with_password(&None) - } + fn load(&self) -> Result, Error>; /// Insert new key to directory fn insert(&self, account: SafeAccount) -> Result; /// Update key in the directory diff --git a/ethstore/src/ethstore.rs b/ethstore/src/ethstore.rs index 574ac8c3da0..1d7104181f2 100644 --- a/ethstore/src/ethstore.rs +++ b/ethstore/src/ethstore.rs @@ -168,7 +168,7 @@ impl SecretStore for EthStore { fn import_wallet(&self, vault: SecretVaultRef, json: &[u8], password: &Password, gen_id: bool) -> Result { let json_keyfile = json::KeyFile::load(json).map_err(|_| Error::InvalidKeyFile("Invalid JSON format".to_owned()))?; - let mut safe_account = SafeAccount::from_file(json_keyfile, None, &None)?; + let mut safe_account = SafeAccount::from_file(json_keyfile, None)?; if gen_id { safe_account.id = Random::random(); diff --git a/ethstore/src/import.rs b/ethstore/src/import.rs index 64a30edaaab..4c72f88afe2 100644 --- a/ethstore/src/import.rs +++ b/ethstore/src/import.rs @@ -18,14 +18,14 @@ use std::collections::HashSet; use std::path::Path; use std::fs; -use ethkey::{Address, Password}; +use ethkey::Address; use accounts_dir::{KeyDirectory, RootDiskDirectory, DiskKeyFileManager, KeyFileManager}; use dir; use Error; /// Import an account from a file. pub fn import_account(path: &Path, dst: &KeyDirectory) -> Result { - let key_manager = DiskKeyFileManager; + let key_manager = DiskKeyFileManager::new(); let existing_accounts = dst.load()?.into_iter().map(|a| a.address).collect::>(); let filename = path.file_name().and_then(|n| n.to_str()).map(|f| f.to_owned()); let account = fs::File::open(&path) @@ -40,8 +40,8 @@ pub fn import_account(path: &Path, dst: &KeyDirectory) -> Result } /// Import all accounts from one directory to the other. -pub fn import_accounts(src: &KeyDirectory, dst: &KeyDirectory, password: &Option) -> Result, Error> { - let accounts = src.load_with_password(password)?; +pub fn import_accounts(src: &KeyDirectory, dst: &KeyDirectory) -> Result, Error> { + let accounts = src.load()?; let existing_accounts = dst.load()?.into_iter() .map(|a| a.address) .collect::>(); diff --git a/ethstore/src/json/bytes.rs b/ethstore/src/json/bytes.rs index 827c1d2021a..b5aae19222a 100644 --- a/ethstore/src/json/bytes.rs +++ b/ethstore/src/json/bytes.rs @@ -19,7 +19,7 @@ use serde::{Deserialize, Deserializer, Serialize, Serializer}; use serde::de::Error; use rustc_hex::{ToHex, FromHex, FromHexError}; -#[derive(Debug, PartialEq, Clone)] +#[derive(Debug, PartialEq)] pub struct Bytes(Vec); impl ops::Deref for Bytes { diff --git a/ethstore/src/json/key_file.rs b/ethstore/src/json/key_file.rs index 10d9a6e5ae1..d5dc136058a 100644 --- a/ethstore/src/json/key_file.rs +++ b/ethstore/src/json/key_file.rs @@ -217,7 +217,7 @@ mod tests { let expected = KeyFile { id: Uuid::from_str("8777d9f6-7860-4b9b-88b7-0b57ee6b3a73").unwrap(), version: Version::V3, - address: "6edddfc6349aff20bc6467ccf276c5b52487f7a8".into(), + address: Some("6edddfc6349aff20bc6467ccf276c5b52487f7a8".into()), crypto: Crypto { cipher: Cipher::Aes128Ctr(Aes128Ctr { iv: "b5a7ec855ec9e2c405371356855fec83".into(), @@ -268,7 +268,7 @@ mod tests { let expected = KeyFile { id: "8777d9f6-7860-4b9b-88b7-0b57ee6b3a73".into(), version: Version::V3, - address: "6edddfc6349aff20bc6467ccf276c5b52487f7a8".into(), + address: Some("6edddfc6349aff20bc6467ccf276c5b52487f7a8".into()), crypto: Crypto { cipher: Cipher::Aes128Ctr(Aes128Ctr { iv: "b5a7ec855ec9e2c405371356855fec83".into(), @@ -296,7 +296,7 @@ mod tests { let file = KeyFile { id: "8777d9f6-7860-4b9b-88b7-0b57ee6b3a73".into(), version: Version::V3, - address: "6edddfc6349aff20bc6467ccf276c5b52487f7a8".into(), + address: Some("6edddfc6349aff20bc6467ccf276c5b52487f7a8".into()), crypto: Crypto { cipher: Cipher::Aes128Ctr(Aes128Ctr { iv: "b5a7ec855ec9e2c405371356855fec83".into(), diff --git a/ethstore/src/lib.rs b/ethstore/src/lib.rs index ad58bd0e984..cb03e2169ed 100644 --- a/ethstore/src/lib.rs +++ b/ethstore/src/lib.rs @@ -36,7 +36,6 @@ extern crate ethereum_types; extern crate ethkey as _ethkey; extern crate parity_wordlist; -#[macro_use] extern crate log; #[macro_use] extern crate serde_derive; diff --git a/parity/account.rs b/parity/account.rs index ed138b86817..e09667379fa 100644 --- a/parity/account.rs +++ b/parity/account.rs @@ -118,7 +118,7 @@ fn import(i: ImportAccounts) -> Result { let path = PathBuf::from(path); if path.is_dir() { let from = RootDiskDirectory::at(&path); - imported += import_accounts(&from, &to, &None).map_err(|e| format!("Importing accounts from {:?} failed: {}", path, e))?.len(); + imported += import_accounts(&from, &to).map_err(|e| format!("Importing accounts from {:?} failed: {}", path, e))?.len(); } else if path.is_file() { import_account(&path, &to).map_err(|e| format!("Importing account from {:?} failed: {}", path, e))?; imported += 1; From 4133b73f88dfd9d9bd89a9cf8c92f88147c5eb70 Mon Sep 17 00:00:00 2001 From: Piotr Chromiec Date: Tue, 27 Nov 2018 19:53:43 +0100 Subject: [PATCH 14/21] fix indentation --- Cargo.lock | 4 ++-- ethstore/src/account/safe_account.rs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 969a09feda3..88648539820 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -685,8 +685,8 @@ dependencies = [ "rlp_compress 0.1.0", "rlp_derive 0.1.0", "rustc-hex 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)", - "serde 1.0.78 (registry+https://github.com/rust-lang/crates.io-index)", - "serde_derive 1.0.78 (registry+https://github.com/rust-lang/crates.io-index)", + "serde 1.0.80 (registry+https://github.com/rust-lang/crates.io-index)", + "serde_derive 1.0.80 (registry+https://github.com/rust-lang/crates.io-index)", "stats 0.1.0", "stop-guard 0.1.0", "tempdir 0.3.7 (registry+https://github.com/rust-lang/crates.io-index)", diff --git a/ethstore/src/account/safe_account.rs b/ethstore/src/account/safe_account.rs index 95f21d694c9..64fe9390972 100644 --- a/ethstore/src/account/safe_account.rs +++ b/ethstore/src/account/safe_account.rs @@ -87,9 +87,9 @@ impl SafeAccount { let address = match json.address { Some(address) => address.into(), None => match password { - Some(password) => + Some(password) => KeyPair::from_secret(crypto.secret(&password).map_err(|_| Error::InvalidPassword)?)?.address(), - None => + None => return Err(Error::Custom("This keystore does not contain address. You need to provide password to import it".to_owned())) } }; From 43c12d676d4b5fad7b4964d5d55b477087f33c86 Mon Sep 17 00:00:00 2001 From: Piotr Chromiec Date: Tue, 27 Nov 2018 20:09:28 +0100 Subject: [PATCH 15/21] ignore just version in json under crypto --- ethstore/src/json/crypto.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/ethstore/src/json/crypto.rs b/ethstore/src/json/crypto.rs index 76dff305054..ff4728a96b7 100644 --- a/ethstore/src/json/crypto.rs +++ b/ethstore/src/json/crypto.rs @@ -52,7 +52,7 @@ enum CryptoField { Kdf, KdfParams, Mac, - Ignored, + Version, } impl<'a> Deserialize<'a> for CryptoField { @@ -82,7 +82,8 @@ impl<'a> Visitor<'a> for CryptoFieldVisitor { "kdf" => Ok(CryptoField::Kdf), "kdfparams" => Ok(CryptoField::KdfParams), "mac" => Ok(CryptoField::Mac), - _ => Ok(CryptoField::Ignored), + "version" => Ok(CryptoField::Version), + _ => Err(Error::custom(format!("Unknown field: '{}'", value))), } } } @@ -123,7 +124,8 @@ impl<'a> Visitor<'a> for CryptoVisitor { Some(CryptoField::Kdf) => { kdf = Some(visitor.next_value()?); } Some(CryptoField::KdfParams) => { kdfparams = Some(visitor.next_value()?); } Some(CryptoField::Mac) => { mac = Some(visitor.next_value()?); } - Some(CryptoField::Ignored) => { visitor.next_value().unwrap_or(()) } + // skip not required version field (it appears in pyethereum generated keystores) + Some(CryptoField::Version) => { visitor.next_value().unwrap_or(()) } None => { break; } } } From 1919bd5b6060374600715a815b1588e148f0a349 Mon Sep 17 00:00:00 2001 From: Niklas Adolfsson Date: Mon, 3 Dec 2018 22:32:03 +0100 Subject: [PATCH 16/21] remove unnecessary borrowing within match str Co-Authored-By: tworec --- ethstore/cli/src/main.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ethstore/cli/src/main.rs b/ethstore/cli/src/main.rs index dc93b899e2f..7e294fa7c17 100644 --- a/ethstore/cli/src/main.rs +++ b/ethstore/cli/src/main.rs @@ -239,7 +239,7 @@ fn execute(command: I) -> Result where I: IntoIterator None, _ => Some(load_password(&args.arg_password)?) }; From 1c6cdbe6ea31bae1a8565514adc336f73549b866 Mon Sep 17 00:00:00 2001 From: Niklas Adolfsson Date: Mon, 3 Dec 2018 22:32:57 +0100 Subject: [PATCH 17/21] remove another unnecessary borrowing Co-Authored-By: tworec --- ethstore/cli/src/main.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ethstore/cli/src/main.rs b/ethstore/cli/src/main.rs index 7e294fa7c17..c33745159c7 100644 --- a/ethstore/cli/src/main.rs +++ b/ethstore/cli/src/main.rs @@ -240,7 +240,7 @@ fn execute(command: I) -> Result where I: IntoIterator None, + "" => None, _ => Some(load_password(&args.arg_password)?) }; let src = key_dir(&args.flag_src, password)?; From 69b4958b150cc14710e55e59f0722ae9cd5f3536 Mon Sep 17 00:00:00 2001 From: Piotr Chromiec Date: Mon, 3 Dec 2018 22:41:02 +0100 Subject: [PATCH 18/21] default derived instead of implementing --- ethstore/src/accounts_dir/disk.rs | 11 +++-------- ethstore/src/import.rs | 2 +- 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/ethstore/src/accounts_dir/disk.rs b/ethstore/src/accounts_dir/disk.rs index e0c93bd375e..65344f52e56 100644 --- a/ethstore/src/accounts_dir/disk.rs +++ b/ethstore/src/accounts_dir/disk.rs @@ -119,30 +119,25 @@ pub struct DiskDirectory where T: KeyFileManager { } /// Keys file manager for root keys directory +#[derive(Default)] pub struct DiskKeyFileManager { password: Option, } -impl DiskKeyFileManager { - /// Create new disk key manager w/o password - pub fn new() -> Self { - DiskKeyFileManager { password: None } - } -} - impl RootDiskDirectory { pub fn create

(path: P) -> Result where P: AsRef { fs::create_dir_all(&path)?; Ok(Self::at(path)) } + /// allows to read keyfiles with given password (needed for keyfiles w/o address) pub fn with_password(&self, password: Option) -> Self { DiskDirectory::new(&self.path, DiskKeyFileManager { password }) } pub fn at

(path: P) -> Self where P: AsRef { - DiskDirectory::new(path, DiskKeyFileManager::new()) + DiskDirectory::new(path, DiskKeyFileManager::default()) } } diff --git a/ethstore/src/import.rs b/ethstore/src/import.rs index 4c72f88afe2..e1b73c56705 100644 --- a/ethstore/src/import.rs +++ b/ethstore/src/import.rs @@ -25,7 +25,7 @@ use Error; /// Import an account from a file. pub fn import_account(path: &Path, dst: &KeyDirectory) -> Result { - let key_manager = DiskKeyFileManager::new(); + let key_manager = DiskKeyFileManager::default(); let existing_accounts = dst.load()?.into_iter().map(|a| a.address).collect::>(); let filename = path.file_name().and_then(|n| n.to_str()).map(|f| f.to_owned()); let account = fs::File::open(&path) From 0282536a49bc2695e2e2203b46373928b7c90144 Mon Sep 17 00:00:00 2001 From: Piotr Chromiec Date: Mon, 3 Dec 2018 22:41:42 +0100 Subject: [PATCH 19/21] log dependency removed --- Cargo.lock | 1 - ethstore/Cargo.toml | 1 - ethstore/src/lib.rs | 1 - 3 files changed, 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 88648539820..8210b9f64d4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1095,7 +1095,6 @@ dependencies = [ "ethkey 0.3.0", "itertools 0.5.10 (registry+https://github.com/rust-lang/crates.io-index)", "libc 0.2.43 (registry+https://github.com/rust-lang/crates.io-index)", - "log 0.4.5 (registry+https://github.com/rust-lang/crates.io-index)", "matches 0.1.8 (registry+https://github.com/rust-lang/crates.io-index)", "parity-crypto 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)", "parity-wordlist 1.2.1 (registry+https://github.com/rust-lang/crates.io-index)", diff --git a/ethstore/Cargo.toml b/ethstore/Cargo.toml index b0756300cb7..8612f11bdf9 100644 --- a/ethstore/Cargo.toml +++ b/ethstore/Cargo.toml @@ -4,7 +4,6 @@ version = "0.2.1" authors = ["Parity Technologies "] [dependencies] -log = "0.4" libc = "0.2" rand = "0.4" ethkey = { path = "../ethkey" } diff --git a/ethstore/src/lib.rs b/ethstore/src/lib.rs index cb03e2169ed..2b071c3836c 100644 --- a/ethstore/src/lib.rs +++ b/ethstore/src/lib.rs @@ -36,7 +36,6 @@ extern crate ethereum_types; extern crate ethkey as _ethkey; extern crate parity_wordlist; -extern crate log; #[macro_use] extern crate serde_derive; From b42f2778be687f7e84ea5d16288247afb1163c1b Mon Sep 17 00:00:00 2001 From: Piotr Chromiec Date: Wed, 19 Dec 2018 12:08:19 +0100 Subject: [PATCH 20/21] [ethstore] warn restored + env_logger initialized in CLI --- Cargo.lock | 2 ++ ethstore/Cargo.toml | 1 + ethstore/cli/Cargo.toml | 1 + ethstore/cli/src/main.rs | 6 ++++++ ethstore/src/accounts_dir/disk.rs | 2 +- ethstore/src/lib.rs | 2 ++ 6 files changed, 13 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index 8210b9f64d4..1a593f1bfc6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1095,6 +1095,7 @@ dependencies = [ "ethkey 0.3.0", "itertools 0.5.10 (registry+https://github.com/rust-lang/crates.io-index)", "libc 0.2.43 (registry+https://github.com/rust-lang/crates.io-index)", + "log 0.4.5 (registry+https://github.com/rust-lang/crates.io-index)", "matches 0.1.8 (registry+https://github.com/rust-lang/crates.io-index)", "parity-crypto 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)", "parity-wordlist 1.2.1 (registry+https://github.com/rust-lang/crates.io-index)", @@ -1116,6 +1117,7 @@ version = "0.1.1" dependencies = [ "dir 0.1.2", "docopt 1.0.2 (registry+https://github.com/rust-lang/crates.io-index)", + "env_logger 0.5.13 (registry+https://github.com/rust-lang/crates.io-index)", "ethstore 0.2.1", "num_cpus 1.8.0 (registry+https://github.com/rust-lang/crates.io-index)", "panic_hook 0.1.0", diff --git a/ethstore/Cargo.toml b/ethstore/Cargo.toml index 8612f11bdf9..b0756300cb7 100644 --- a/ethstore/Cargo.toml +++ b/ethstore/Cargo.toml @@ -4,6 +4,7 @@ version = "0.2.1" authors = ["Parity Technologies "] [dependencies] +log = "0.4" libc = "0.2" rand = "0.4" ethkey = { path = "../ethkey" } diff --git a/ethstore/cli/Cargo.toml b/ethstore/cli/Cargo.toml index 3c291517af1..ecfce603896 100644 --- a/ethstore/cli/Cargo.toml +++ b/ethstore/cli/Cargo.toml @@ -5,6 +5,7 @@ authors = ["Parity Technologies "] [dependencies] docopt = "1.0" +env_logger = "0.5" num_cpus = "1.6" rustc-hex = "1.0" serde = "1.0" diff --git a/ethstore/cli/src/main.rs b/ethstore/cli/src/main.rs index c33745159c7..7e607973ccc 100644 --- a/ethstore/cli/src/main.rs +++ b/ethstore/cli/src/main.rs @@ -23,6 +23,8 @@ extern crate parking_lot; extern crate rustc_hex; extern crate serde; +extern crate env_logger; + #[macro_use] extern crate serde_derive; @@ -146,6 +148,10 @@ impl fmt::Display for Error { fn main() { panic_hook::set_abort(); + if env::var("RUST_LOG").is_err() { + env::set_var("RUST_LOG", "warn") + } + env_logger::try_init().expect("Logger initialized only once."); match execute(env::args()) { Ok(result) => println!("{}", result), diff --git a/ethstore/src/accounts_dir/disk.rs b/ethstore/src/accounts_dir/disk.rs index 65344f52e56..34e70bcc602 100644 --- a/ethstore/src/accounts_dir/disk.rs +++ b/ethstore/src/accounts_dir/disk.rs @@ -202,7 +202,7 @@ impl DiskDirectory where T: KeyFileManager { .map_err(Into::into) .and_then(|file| self.key_manager.read(filename, file)) .map_err(|err| { - eprintln!("Invalid key file: {:?} ({})", path, err); + warn!("Invalid key file: {:?} ({})", path, err); err }) .map(|account| (path, account)) diff --git a/ethstore/src/lib.rs b/ethstore/src/lib.rs index 2b071c3836c..ad58bd0e984 100644 --- a/ethstore/src/lib.rs +++ b/ethstore/src/lib.rs @@ -36,6 +36,8 @@ extern crate ethereum_types; extern crate ethkey as _ethkey; extern crate parity_wordlist; +#[macro_use] +extern crate log; #[macro_use] extern crate serde_derive; From 53d4d8436866f5717def71b5c5dbcb4a11de5d5b Mon Sep 17 00:00:00 2001 From: Piotr Chromiec Date: Wed, 2 Jan 2019 15:47:13 +0100 Subject: [PATCH 21/21] applied suggestion from @tomusdrw --- accounts/ethstore/src/account/safe_account.rs | 39 ++++++++++++------- accounts/ethstore/src/accounts_dir/disk.rs | 2 +- accounts/ethstore/src/ethstore.rs | 2 +- 3 files changed, 27 insertions(+), 16 deletions(-) diff --git a/accounts/ethstore/src/account/safe_account.rs b/accounts/ethstore/src/account/safe_account.rs index 64fe9390972..4492c6014e6 100644 --- a/accounts/ethstore/src/account/safe_account.rs +++ b/accounts/ethstore/src/account/safe_account.rs @@ -77,20 +77,31 @@ impl SafeAccount { /// Create a new `SafeAccount` from the given `json`; if it was read from a /// file, the `filename` should be `Some` name. If it is as yet anonymous, then it /// can be left `None`. - pub fn from_file(json: json::KeyFile, filename: Option) -> Result { - SafeAccount::from_file_with_password(json, filename, &None) - } - - /// Same as [`from_file`](#method.from_file), but with `password` which enables reading keystore files w/o address. - pub fn from_file_with_password(json: json::KeyFile, filename: Option, password: &Option) -> Result { + /// In case `password` is provided, we will attempt to read the secret from the keyfile + /// and derive the address from it instead of reading it directly. + /// Providing password is required for `json::KeyFile`s with no address. + pub fn from_file(json: json::KeyFile, filename: Option, password: &Option) -> Result { let crypto = Crypto::from(json.crypto); - let address = match json.address { - Some(address) => address.into(), - None => match password { - Some(password) => - KeyPair::from_secret(crypto.secret(&password).map_err(|_| Error::InvalidPassword)?)?.address(), - None => - return Err(Error::Custom("This keystore does not contain address. You need to provide password to import it".to_owned())) + let address = match (password, &json.address) { + (None, Some(json_address)) => json_address.into(), + (None, None) => Err(Error::Custom( + "This keystore does not contain address. You need to provide password to import it".into()))?, + (Some(password), json_address) => { + let derived_address = KeyPair::from_secret( + crypto.secret(&password).map_err(|_| Error::InvalidPassword)? + )?.address(); + + match json_address { + Some(json_address) => { + let json_address = json_address.into(); + if derived_address != json_address { + warn!("Detected address mismatch when opening an account. Derived: {:?}, in json got: {:?}", + derived_address, json_address); + } + }, + _ => {}, + } + derived_address } }; @@ -120,7 +131,7 @@ impl SafeAccount { address: Some(meta_plain.address), name: meta_plain.name, meta: meta_plain.meta, - }, filename) + }, filename, &None) } /// Create a new `VaultKeyFile` from the given `self` diff --git a/accounts/ethstore/src/accounts_dir/disk.rs b/accounts/ethstore/src/accounts_dir/disk.rs index 34e70bcc602..344c643c59c 100644 --- a/accounts/ethstore/src/accounts_dir/disk.rs +++ b/accounts/ethstore/src/accounts_dir/disk.rs @@ -330,7 +330,7 @@ impl VaultKeyDirectoryProvider for DiskDirectory where T: KeyFileManager { impl KeyFileManager for DiskKeyFileManager { fn read(&self, filename: Option, reader: T) -> Result where T: io::Read { let key_file = json::KeyFile::load(reader).map_err(|e| Error::Custom(format!("{:?}", e)))?; - SafeAccount::from_file_with_password(key_file, filename, &self.password) + SafeAccount::from_file(key_file, filename, &self.password) } fn write(&self, mut account: SafeAccount, writer: &mut T) -> Result<(), Error> where T: io::Write { diff --git a/accounts/ethstore/src/ethstore.rs b/accounts/ethstore/src/ethstore.rs index 1d7104181f2..574ac8c3da0 100644 --- a/accounts/ethstore/src/ethstore.rs +++ b/accounts/ethstore/src/ethstore.rs @@ -168,7 +168,7 @@ impl SecretStore for EthStore { fn import_wallet(&self, vault: SecretVaultRef, json: &[u8], password: &Password, gen_id: bool) -> Result { let json_keyfile = json::KeyFile::load(json).map_err(|_| Error::InvalidKeyFile("Invalid JSON format".to_owned()))?; - let mut safe_account = SafeAccount::from_file(json_keyfile, None)?; + let mut safe_account = SafeAccount::from_file(json_keyfile, None, &None)?; if gen_id { safe_account.id = Random::random();