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

@@ -130,6 +130,27 @@ pub struct AccountProvider {
 	sstore: Box<SecretStore>,
 }
 
+/// 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<String>,
+}
+
+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<SecretStore>) -> 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<HashMap<H160, AccountMeta>, Error> {
+		let r: HashMap<H160, AccountMeta> = 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<A>(&self, account: A) -> Result<AccountMeta, Error> where Address: From<A> {
+		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<A>(&self, account: A, name: String) -> Result<(), Error> where Address: From<A> {
+		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<A>(&self, account: A, meta: String) -> Result<(), Error> where Address: From<A> {
+		let account = Address::from(account).into();
+		try!(self.sstore.set_meta(&account, meta));
+		Ok(())
+	}
+
 	/// Helper method used for unlocking accounts.
 	fn unlock_account<A>(&self, account: A, password: String, unlock: Unlock) -> Result<(), Error> where Address: From<A> {
 		let a = Address::from(account);

ethcore/src/executive.rs

@@ -259,8 +259,7 @@ impl<'a> Executive<'a> {
 			let trace_info = tracer.prepare_trace_call(&params);
 
 			let cost = self.engine.cost_of_builtin(&params.code_address, data);
-			match cost <= params.gas {
+			if cost <= params.gas {
-				true => {
 				self.engine.execute_builtin(&params.code_address, data, &mut output);
 				self.state.clear_snapshot();
 
@@ -282,16 +281,14 @@ impl<'a> Executive<'a> {
 				}
 
 				Ok(params.gas - cost)
-				},
+			} else {
 				// 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)
 			}
-			}
 		} else {
 			let trace_info = tracer.prepare_trace_call(&params);
 			let mut trace_output = tracer.prepare_trace_output();
@@ -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)

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<json::Crypto> for Crypto {
@@ -66,6 +68,8 @@ impl From<json::KeyFile> 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<json::KeyFile> 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());

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<KeyDirectory>,
@@ -61,7 +62,7 @@ impl SecretStore for EthStore {
 	fn insert_account(&self, secret: Secret, password: &str) -> Result<Address, Error> {
 		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<UUID, Error> {
+		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<String, Error> {
+		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<String, Error> {
+		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)
+	}
 }

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<String>,
+	pub meta: Option<String>,
 }
 
 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);

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<Address, Error>;
@@ -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<Signature, Error>;
+
+	fn uuid(&self, account: &Address) -> Result<UUID, Error>;
+
+	fn name(&self, account: &Address) -> Result<String, Error>;
+
+	fn meta(&self, account: &Address) -> Result<String, Error>;
+
+	fn set_name(&self, address: &Address, name: String) -> Result<(), Error>;
+
+	fn set_meta(&self, address: &Address, meta: String) -> Result<(), Error>;
 }
 

parity/rpc_apis.rs

@@ -122,7 +122,7 @@ fn list_apis(apis: ApiSet) -> Vec<Api> {
 			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]
 		},
 	}
 }

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<C: 'static, M: 'static> Personal for PersonalClient<C, M> 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<Value, Error> {
+		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<Value, Error> {
+		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<Value, Error> {
+		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::<BTreeMap<_, _>>()))
+	}
 }

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);

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<Value, Error>;
 
+	/// Set an account's name.
+	fn set_account_name(&self, _: Params) -> Result<Value, Error>;
+
+	/// Set an account's metadata string.
+	fn set_account_meta(&self, _: Params) -> Result<Value, Error>;
+
+	/// Returns accounts information.
+	fn accounts_info(&self, _: Params) -> Result<Value, Error>;
+
 	/// Should be used to convert object to io delegate.
 	fn to_delegate(self) -> IoDelegate<Self> {
 		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
 	}
 }