From 3f841cc5782b4db9c4dfa8461da292cc2f5cb5b3 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Sun, 24 Jul 2016 17:38:21 +0200 Subject: [PATCH] Name and meta in accounts (#1695) * Introduce persistent name() and meta() in SecretStore. * Quick stash. * Fix build. * Add ethcore_set methods. * Bug fixes for default values. * Move to personal to ensure set API exposed. * Add UUID to accounts info. * Add tests. --- ethcore/src/account_provider.rs | 54 +++++++++++++++++++++++++ ethcore/src/executive.rs | 55 ++++++++++++------------- ethstore/src/account/safe_account.rs | 17 ++++++-- ethstore/src/ethstore.rs | 45 ++++++++++++++++++++- ethstore/src/json/key_file.rs | 23 ++++++++++- ethstore/src/secret_store.rs | 11 +++++ parity/rpc_apis.rs | 2 +- rpc/src/v1/impls/personal.rs | 36 +++++++++++++++++ rpc/src/v1/tests/mocked/personal.rs | 60 ++++++++++++++++++++++++++++ rpc/src/v1/traits/personal.rs | 13 ++++++ 10 files changed, 281 insertions(+), 35 deletions(-) diff --git a/ethcore/src/account_provider.rs b/ethcore/src/account_provider.rs index 332426cab..62bfa3d0d 100644 --- a/ethcore/src/account_provider.rs +++ b/ethcore/src/account_provider.rs @@ -130,6 +130,27 @@ pub struct AccountProvider { sstore: Box, } +/// Collected account metadata +#[derive(Clone, Debug, PartialEq)] +pub struct AccountMeta { + /// The name of the account. + pub name: String, + /// The rest of the metadata of the account. + pub meta: String, + /// The 128-bit UUID of the account, if it has one (brain-wallets don't). + pub uuid: Option, +} + +impl Default for AccountMeta { + fn default() -> Self { + AccountMeta { + name: String::new(), + meta: "{}".to_owned(), + uuid: None, + } + } +} + impl AccountProvider { /// Creates new account provider. pub fn new(sstore: Box) -> Self { @@ -167,6 +188,39 @@ impl AccountProvider { self.sstore.accounts().into_iter().map(|a| H160(a.into())).collect() } + /// Returns each account along with name and meta. + pub fn accounts_info(&self) -> Result, Error> { + let r: HashMap = self.sstore.accounts() + .into_iter() + .map(|a| (H160(a.clone().into()), self.account_meta(a).unwrap_or(Default::default()))) + .collect(); + Ok(r) + } + + /// Returns each account along with name and meta. + pub fn account_meta(&self, account: A) -> Result where Address: From { + let account = Address::from(account).into(); + Ok(AccountMeta { + name: try!(self.sstore.name(&account)), + meta: try!(self.sstore.meta(&account)), + uuid: self.sstore.uuid(&account).ok().map(Into::into), // allowed to not have a UUID + }) + } + + /// Returns each account along with name and meta. + pub fn set_account_name(&self, account: A, name: String) -> Result<(), Error> where Address: From { + let account = Address::from(account).into(); + try!(self.sstore.set_name(&account, name)); + Ok(()) + } + + /// Returns each account along with name and meta. + pub fn set_account_meta(&self, account: A, meta: String) -> Result<(), Error> where Address: From { + let account = Address::from(account).into(); + try!(self.sstore.set_meta(&account, meta)); + Ok(()) + } + /// Helper method used for unlocking accounts. fn unlock_account(&self, account: A, password: String, unlock: Unlock) -> Result<(), Error> where Address: From { let a = Address::from(account); diff --git a/ethcore/src/executive.rs b/ethcore/src/executive.rs index f5ff95d71..ef6af43b3 100644 --- a/ethcore/src/executive.rs +++ b/ethcore/src/executive.rs @@ -259,38 +259,35 @@ impl<'a> Executive<'a> { let trace_info = tracer.prepare_trace_call(¶ms); let cost = self.engine.cost_of_builtin(¶ms.code_address, data); - match cost <= params.gas { - true => { - self.engine.execute_builtin(¶ms.code_address, data, &mut output); - self.state.clear_snapshot(); + if cost <= params.gas { + self.engine.execute_builtin(¶ms.code_address, data, &mut output); + self.state.clear_snapshot(); - // trace only top level calls to builtins to avoid DDoS attacks - if self.depth == 0 { - let mut trace_output = tracer.prepare_trace_output(); - if let Some(mut out) = trace_output.as_mut() { - *out = output.to_owned(); - } - - tracer.trace_call( - trace_info, - cost, - trace_output, - self.depth, - vec![], - delegate_call - ); + // trace only top level calls to builtins to avoid DDoS attacks + if self.depth == 0 { + let mut trace_output = tracer.prepare_trace_output(); + if let Some(mut out) = trace_output.as_mut() { + *out = output.to_owned(); } - Ok(params.gas - cost) - }, - // just drain the whole gas - false => { - self.state.revert_snapshot(); - - tracer.trace_failed_call(trace_info, self.depth, vec![], delegate_call); - - Err(evm::Error::OutOfGas) + tracer.trace_call( + trace_info, + cost, + trace_output, + self.depth, + vec![], + delegate_call + ); } + + Ok(params.gas - cost) + } else { + // just drain the whole gas + self.state.revert_snapshot(); + + tracer.trace_failed_call(trace_info, self.depth, vec![], delegate_call); + + Err(evm::Error::OutOfGas) } } else { let trace_info = tracer.prepare_trace_call(¶ms); @@ -304,7 +301,7 @@ impl<'a> Executive<'a> { let mut unconfirmed_substate = Substate::new(); // TODO: make ActionParams pass by ref then avoid copy altogether. - let mut subvmtracer = vm_tracer.prepare_subtrace(params.code.as_ref().expect("scope is protected by params.code.is_some condition")); + let mut subvmtracer = vm_tracer.prepare_subtrace(params.code.as_ref().expect("scope is conditional on params.code.is_some(); qed")); let res = { self.exec_vm(params, &mut unconfirmed_substate, OutputPolicy::Return(output, trace_output.as_mut()), &mut subtracer, &mut subvmtracer) diff --git a/ethstore/src/account/safe_account.rs b/ethstore/src/account/safe_account.rs index eba2c41d3..5e680c260 100644 --- a/ethstore/src/account/safe_account.rs +++ b/ethstore/src/account/safe_account.rs @@ -35,6 +35,8 @@ pub struct SafeAccount { pub version: Version, pub address: Address, pub crypto: Crypto, + pub name: String, + pub meta: String, } impl From for Crypto { @@ -66,6 +68,8 @@ impl From for SafeAccount { version: From::from(json.version), address: From::from(json.address), //json.address.into(), crypto: From::from(json.crypto), + name: json.name.unwrap_or(String::new()), + meta: json.meta.unwrap_or("{}".to_owned()), } } } @@ -77,6 +81,8 @@ impl Into for SafeAccount { version: self.version.into(), address: self.address.into(), //From::from(self.address), crypto: self.crypto.into(), + name: Some(self.name.into()), + meta: Some(self.meta.into()), } } } @@ -138,12 +144,15 @@ impl Crypto { } impl SafeAccount { - pub fn create(keypair: &KeyPair, id: [u8; 16], password: &str, iterations: u32) -> Self { + // DEPRECATED. use `create_with_name` instead + pub fn create(keypair: &KeyPair, id: [u8; 16], password: &str, iterations: u32, name: String, meta: String) -> Self { SafeAccount { id: id, version: Version::V3, crypto: Crypto::create(keypair.secret(), password, iterations), address: keypair.address(), + name: name, + meta: meta, } } @@ -159,6 +168,8 @@ impl SafeAccount { version: self.version.clone(), crypto: Crypto::create(&secret, new_password, iterations), address: self.address.clone(), + name: self.name.clone(), + meta: self.meta.clone(), }; Ok(result) } @@ -194,7 +205,7 @@ mod tests { let keypair = Random.generate().unwrap(); let password = "hello world"; let message = Message::default(); - let account = SafeAccount::create(&keypair, [0u8; 16], password, 10240); + let account = SafeAccount::create(&keypair, [0u8; 16], password, 10240, "Test".to_owned(), "{}".to_owned()); let signature = account.sign(password, &message).unwrap(); assert!(verify_public(keypair.public(), &signature, &message).unwrap()); } @@ -206,7 +217,7 @@ mod tests { let sec_password = "this is sparta"; let i = 10240; let message = Message::default(); - let account = SafeAccount::create(&keypair, [0u8; 16], first_password, i); + let account = SafeAccount::create(&keypair, [0u8; 16], first_password, i, "Test".to_owned(), "{}".to_owned()); let new_account = account.change_password(first_password, sec_password, i).unwrap(); assert!(account.sign(first_password, &message).is_ok()); assert!(account.sign(sec_password, &message).is_err()); diff --git a/ethstore/src/ethstore.rs b/ethstore/src/ethstore.rs index 9ef84ebc2..8db00298e 100644 --- a/ethstore/src/ethstore.rs +++ b/ethstore/src/ethstore.rs @@ -23,6 +23,7 @@ use ethkey::{Signature, Address, Message, Secret}; use dir::KeyDirectory; use account::SafeAccount; use {Error, SecretStore}; +use json::UUID; pub struct EthStore { dir: Box, @@ -61,7 +62,7 @@ impl SecretStore for EthStore { fn insert_account(&self, secret: Secret, password: &str) -> Result { let keypair = try!(KeyPair::from_secret(secret).map_err(|_| Error::CreationFailed)); let id: [u8; 16] = Random::random(); - let account = SafeAccount::create(&keypair, id, password, self.iterations); + let account = SafeAccount::create(&keypair, id, password, self.iterations, UUID::from(id).into(), "{}".to_owned()); let address = account.address.clone(); try!(self.save(account)); Ok(address) @@ -105,4 +106,46 @@ impl SecretStore for EthStore { let account = try!(cache.get(account).ok_or(Error::InvalidAccount)); account.sign(password, message) } + + fn uuid(&self, addr: &Address) -> Result { + let cache = self.cache.read().unwrap(); + let account = try!(cache.get(addr).ok_or(Error::InvalidAccount)); + Ok(account.id.into()) + } + + fn name(&self, addr: &Address) -> Result { + let cache = self.cache.read().unwrap(); + let account = try!(cache.get(addr).ok_or(Error::InvalidAccount)); + Ok(account.name.clone()) + } + + fn meta(&self, addr: &Address) -> Result { + let cache = self.cache.read().unwrap(); + let account = try!(cache.get(addr).ok_or(Error::InvalidAccount)); + Ok(account.meta.clone()) + } + + fn set_name(&self, addr: &Address, name: String) -> Result<(), Error> { + let account = { + let cache = self.cache.read().unwrap(); + let mut account = try!(cache.get(addr).ok_or(Error::InvalidAccount)).clone(); + account.name = name; + account + }; + + // save to file + self.save(account) + } + + fn set_meta(&self, addr: &Address, meta: String) -> Result<(), Error> { + let account = { + let cache = self.cache.read().unwrap(); + let mut account = try!(cache.get(addr).ok_or(Error::InvalidAccount)).clone(); + account.meta = meta; + account + }; + + // save to file + self.save(account) + } } diff --git a/ethstore/src/json/key_file.rs b/ethstore/src/json/key_file.rs index 5d87745ef..7d970a15c 100644 --- a/ethstore/src/json/key_file.rs +++ b/ethstore/src/json/key_file.rs @@ -26,6 +26,8 @@ pub struct KeyFile { pub version: Version, pub crypto: Crypto, pub address: H160, + pub name: Option, + pub meta: Option, } enum KeyFileField { @@ -33,6 +35,8 @@ enum KeyFileField { Version, Crypto, Address, + Name, + Meta, } impl Deserialize for KeyFileField { @@ -57,6 +61,8 @@ impl Visitor for KeyFileFieldVisitor { "crypto" => Ok(KeyFileField::Crypto), "Crypto" => Ok(KeyFileField::Crypto), "address" => Ok(KeyFileField::Address), + "name" => Ok(KeyFileField::Name), + "meta" => Ok(KeyFileField::Meta), _ => Err(Error::custom(format!("Unknown field: '{}'", value))), } } @@ -83,6 +89,8 @@ impl Visitor for KeyFileVisitor { let mut version = None; let mut crypto = None; let mut address = None; + let mut name = None; + let mut meta = None; loop { match try!(visitor.visit_key()) { @@ -90,6 +98,8 @@ impl Visitor for KeyFileVisitor { Some(KeyFileField::Version) => { version = Some(try!(visitor.visit_value())); } Some(KeyFileField::Crypto) => { crypto = Some(try!(visitor.visit_value())); } Some(KeyFileField::Address) => { address = Some(try!(visitor.visit_value())); } + Some(KeyFileField::Name) => { name = visitor.visit_value().ok(); } // ignore anyhing that is not a string to be permissive. + Some(KeyFileField::Meta) => { meta = visitor.visit_value().ok(); } // ignore anyhing that is not a string to be permissive. None => { break; } } } @@ -121,6 +131,8 @@ impl Visitor for KeyFileVisitor { version: version, crypto: crypto, address: address, + name: name, + meta: meta, }; Ok(result) @@ -165,7 +177,9 @@ mod tests { "mac": "46325c5d4e8c991ad2683d525c7854da387138b6ca45068985aa4959fa2b8c8f" }, "id": "8777d9f6-7860-4b9b-88b7-0b57ee6b3a73", - "version": 3 + "version": 3, + "name": "Test", + "meta": "{}" }"#; let expected = KeyFile { @@ -186,6 +200,8 @@ mod tests { }), mac: H256::from_str("46325c5d4e8c991ad2683d525c7854da387138b6ca45068985aa4959fa2b8c8f").unwrap(), }, + name: Some("Test".to_owned()), + meta: Some("{}".to_owned()), }; let keyfile: KeyFile = serde_json::from_str(json).unwrap(); @@ -235,6 +251,8 @@ mod tests { }), mac: H256::from_str("46325c5d4e8c991ad2683d525c7854da387138b6ca45068985aa4959fa2b8c8f").unwrap(), }, + name: None, + meta: None, }; let keyfile: KeyFile = serde_json::from_str(json).unwrap(); @@ -261,9 +279,12 @@ mod tests { }), mac: H256::from_str("46325c5d4e8c991ad2683d525c7854da387138b6ca45068985aa4959fa2b8c8f").unwrap(), }, + name: Some("Test".to_owned()), + meta: None, }; let serialized = serde_json::to_string(&file).unwrap(); + println!("{}", serialized); let deserialized = serde_json::from_str(&serialized).unwrap(); assert_eq!(file, deserialized); diff --git a/ethstore/src/secret_store.rs b/ethstore/src/secret_store.rs index 5dab50ba7..b3f2a6b45 100644 --- a/ethstore/src/secret_store.rs +++ b/ethstore/src/secret_store.rs @@ -16,6 +16,7 @@ use ethkey::{Address, Message, Signature, Secret}; use Error; +use json::UUID; pub trait SecretStore: Send + Sync { fn insert_account(&self, secret: Secret, password: &str) -> Result; @@ -27,5 +28,15 @@ pub trait SecretStore: Send + Sync { fn remove_account(&self, account: &Address, password: &str) -> Result<(), Error>; fn sign(&self, account: &Address, password: &str, message: &Message) -> Result; + + fn uuid(&self, account: &Address) -> Result; + + fn name(&self, account: &Address) -> Result; + + fn meta(&self, account: &Address) -> Result; + + fn set_name(&self, address: &Address, name: String) -> Result<(), Error>; + + fn set_meta(&self, address: &Address, meta: String) -> Result<(), Error>; } diff --git a/parity/rpc_apis.rs b/parity/rpc_apis.rs index 5670d7c94..c2146795b 100644 --- a/parity/rpc_apis.rs +++ b/parity/rpc_apis.rs @@ -122,7 +122,7 @@ fn list_apis(apis: ApiSet) -> Vec { vec![Api::Web3, Api::Net, Api::Eth, Api::Personal, Api::Ethcore, Api::Traces, Api::Rpc] }, _ => { - vec![Api::Web3, Api::Net, Api::Eth, Api::Personal, Api::Signer, Api::Ethcore, Api::Traces, Api::Rpc] + vec![Api::Web3, Api::Net, Api::Eth, Api::Personal, Api::Signer, Api::Ethcore, Api::EthcoreSet, Api::Traces, Api::Rpc] }, } } diff --git a/rpc/src/v1/impls/personal.rs b/rpc/src/v1/impls/personal.rs index 4430ab502..063b7415b 100644 --- a/rpc/src/v1/impls/personal.rs +++ b/rpc/src/v1/impls/personal.rs @@ -16,6 +16,7 @@ //! Account management (personal) rpc implementation use std::sync::{Arc, Weak}; +use std::collections::{BTreeMap}; use jsonrpc_core::*; use v1::traits::Personal; use v1::types::{H160 as RpcH160, TransactionRequest}; @@ -104,4 +105,39 @@ impl Personal for PersonalClient where C: MiningBl unlock_sign_and_dispatch(&*take_weak!(self.client), &*take_weak!(self.miner), request, &*accounts, sender, password) }) } + + fn set_account_name(&self, params: Params) -> Result { + try!(self.active()); + let store = take_weak!(self.accounts); + from_params::<(RpcH160, _)>(params).and_then(|(addr, name)| { + let addr: Address = addr.into(); + store.set_account_name(addr, name).map_err(|_| Error::invalid_params()).map(|_| Value::Null) + }) + } + + fn set_account_meta(&self, params: Params) -> Result { + try!(self.active()); + let store = take_weak!(self.accounts); + from_params::<(RpcH160, _)>(params).and_then(|(addr, meta)| { + let addr: Address = addr.into(); + store.set_account_meta(addr, meta).map_err(|_| Error::invalid_params()).map(|_| Value::Null) + }) + } + + fn accounts_info(&self, _: Params) -> Result { + try!(self.active()); + let store = take_weak!(self.accounts); + Ok(Value::Object(try!(store.accounts_info().map_err(|_| Error::invalid_params())).into_iter().map(|(a, v)| { + let m = map![ + "name".to_owned() => to_value(&v.name).unwrap(), + "meta".to_owned() => to_value(&v.meta).unwrap(), + "uuid".to_owned() => if let &Some(ref uuid) = &v.uuid { + to_value(uuid).unwrap() + } else { + Value::Null + } + ]; + (format!("0x{}", a.hex()), Value::Object(m)) + }).collect::>())) + } } diff --git a/rpc/src/v1/tests/mocked/personal.rs b/rpc/src/v1/tests/mocked/personal.rs index 1aebde46b..5f6b912f4 100644 --- a/rpc/src/v1/tests/mocked/personal.rs +++ b/rpc/src/v1/tests/mocked/personal.rs @@ -118,6 +118,66 @@ fn new_account() { assert_eq!(res, Some(response)); } +#[test] +fn should_be_able_to_get_account_info() { + let tester = setup(None); + tester.accounts.new_account("").unwrap(); + let accounts = tester.accounts.accounts(); + assert_eq!(accounts.len(), 1); + let address = accounts[0]; + + let uuid = tester.accounts.accounts_info().unwrap().get(&address).unwrap().uuid.as_ref().unwrap().clone(); + tester.accounts.set_account_name(address.clone(), "Test".to_owned()).unwrap(); + tester.accounts.set_account_meta(address.clone(), "{foo: 69}".to_owned()).unwrap(); + + let request = r#"{"jsonrpc": "2.0", "method": "personal_accountsInfo", "params": [], "id": 1}"#; + let res = tester.io.handle_request(request); + let response = format!("{{\"jsonrpc\":\"2.0\",\"result\":{{\"0x{}\":{{\"meta\":\"{{foo: 69}}\",\"name\":\"Test\",\"uuid\":\"{}\"}}}},\"id\":1}}", address.hex(), uuid); + assert_eq!(res, Some(response)); +} + +#[test] +fn should_be_able_to_set_name() { + let tester = setup(None); + tester.accounts.new_account("").unwrap(); + let accounts = tester.accounts.accounts(); + assert_eq!(accounts.len(), 1); + let address = accounts[0]; + + let request = format!(r#"{{"jsonrpc": "2.0", "method": "personal_setAccountName", "params": ["0x{}", "Test"], "id": 1}}"#, address.hex()); + let response = r#"{"jsonrpc":"2.0","result":null,"id":1}"#; + let res = tester.io.handle_request(&request); + assert_eq!(res, Some(response.into())); + + let uuid = tester.accounts.accounts_info().unwrap().get(&address).unwrap().uuid.as_ref().unwrap().clone(); + + let request = r#"{"jsonrpc": "2.0", "method": "personal_accountsInfo", "params": [], "id": 1}"#; + let res = tester.io.handle_request(request); + let response = format!("{{\"jsonrpc\":\"2.0\",\"result\":{{\"0x{}\":{{\"meta\":\"{{}}\",\"name\":\"Test\",\"uuid\":\"{}\"}}}},\"id\":1}}", address.hex(), uuid); + assert_eq!(res, Some(response)); +} + +#[test] +fn should_be_able_to_set_meta() { + let tester = setup(None); + tester.accounts.new_account("").unwrap(); + let accounts = tester.accounts.accounts(); + assert_eq!(accounts.len(), 1); + let address = accounts[0]; + + let request = format!(r#"{{"jsonrpc": "2.0", "method": "personal_setAccountMeta", "params": ["0x{}", "{{foo: 69}}"], "id": 1}}"#, address.hex()); + let response = r#"{"jsonrpc":"2.0","result":null,"id":1}"#; + let res = tester.io.handle_request(&request); + assert_eq!(res, Some(response.into())); + + let uuid = tester.accounts.accounts_info().unwrap().get(&address).unwrap().uuid.as_ref().unwrap().clone(); + + let request = r#"{"jsonrpc": "2.0", "method": "personal_accountsInfo", "params": [], "id": 1}"#; + let res = tester.io.handle_request(request); + let response = format!("{{\"jsonrpc\":\"2.0\",\"result\":{{\"0x{}\":{{\"meta\":\"{{foo: 69}}\",\"name\":\"{}\",\"uuid\":\"{}\"}}}},\"id\":1}}", address.hex(), uuid, uuid); + assert_eq!(res, Some(response)); +} + #[test] fn sign_and_send_transaction_with_invalid_password() { let tester = setup(None); diff --git a/rpc/src/v1/traits/personal.rs b/rpc/src/v1/traits/personal.rs index c2c6a9625..948ae6b26 100644 --- a/rpc/src/v1/traits/personal.rs +++ b/rpc/src/v1/traits/personal.rs @@ -36,6 +36,15 @@ pub trait Personal: Sized + Send + Sync + 'static { /// Returns `true` if Trusted Signer is enabled, `false` otherwise. fn signer_enabled(&self, _: Params) -> Result; + /// Set an account's name. + fn set_account_name(&self, _: Params) -> Result; + + /// Set an account's metadata string. + fn set_account_meta(&self, _: Params) -> Result; + + /// Returns accounts information. + fn accounts_info(&self, _: Params) -> Result; + /// Should be used to convert object to io delegate. fn to_delegate(self) -> IoDelegate { let mut delegate = IoDelegate::new(Arc::new(self)); @@ -44,6 +53,10 @@ pub trait Personal: Sized + Send + Sync + 'static { delegate.add_method("personal_newAccount", Personal::new_account); delegate.add_method("personal_unlockAccount", Personal::unlock_account); delegate.add_method("personal_signAndSendTransaction", Personal::sign_and_send_transaction); + delegate.add_method("personal_setAccountName", Personal::set_account_name); + delegate.add_method("personal_setAccountMeta", Personal::set_account_meta); + delegate.add_method("personal_accountsInfo", Personal::accounts_info); + delegate } }