Optional from field in Transaction Requests (#4332)

* Infering default account when sending transactions if not provided

* Fixing test

* Fixing tests code

* Fixes.

* More fixes.

* Final fix.

ethcore/src/account_provider/mod.rs

@@ -50,30 +50,33 @@ struct AccountData {
 	password: String,
 }
 
-/// `AccountProvider` errors.
+/// Signing error
 #[derive(Debug)]
-pub enum Error {
-	/// Returned when account is not unlocked.
+pub enum SignError {
+	/// Account is not unlocked
 	NotUnlocked,
-	/// Returned when signing fails.
-	SStore(SSError),
+	/// Low-level error from store
+	SStore(SSError)
 }
 
-impl fmt::Display for Error {
+impl fmt::Display for SignError {
 	fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> {
 		match *self {
-			Error::NotUnlocked => write!(f, "Account is locked"),
-			Error::SStore(ref e) => write!(f, "{}", e),
+			SignError::NotUnlocked => write!(f, "Account is locked"),
+			SignError::SStore(ref e) => write!(f, "{}", e),
 		}
 	}
 }
 
-impl From<SSError> for Error {
+impl From<SSError> for SignError {
 	fn from(e: SSError) -> Self {
-		Error::SStore(e)
+		SignError::SStore(e)
 	}
 }
 
+/// `AccountProvider` errors.
+pub type Error = SSError;
+
 /// Dapp identifier
 #[derive(Default, Debug, Clone, Eq, PartialEq, Hash, Ord, PartialOrd)]
 pub struct DappId(String);
@@ -218,6 +221,14 @@ impl AccountProvider {
 		}
 	}
 
+	/// Returns default account for particular dapp falling back to other allowed accounts if necessary.
+	pub fn default_address(&self, dapp: DappId) -> Result<Address, Error> {
+		self.dapps_addresses(dapp)?
+			.get(0)
+			.cloned()
+			.ok_or(SSError::InvalidAccount)
+	}
+
 	/// Sets addresses visile for dapp.
 	pub fn set_dapps_addresses(&self, dapp: DappId, addresses: Vec<Address>) -> Result<(), Error> {
 		self.dapps_settings.write().set_accounts(dapp, addresses);
@@ -288,8 +299,8 @@ impl AccountProvider {
 	}
 
 	/// Changes the password of `account` from `password` to `new_password`. Fails if incorrect `password` given.
-	pub fn change_password(&self, address: &Address, password: String, new_password: String) -> Result<(), Error> {
-		self.sstore.change_password(&StoreAccountRef::root(address.clone()), &password, &new_password).map_err(Error::SStore)
+	pub fn change_password(&self, account: &Address, password: String, new_password: String) -> Result<(), Error> {
+		self.sstore.change_password(&StoreAccountRef::root(account.clone()), &password, &new_password)
 	}
 
 	/// Helper method used for unlocking accounts.
@@ -316,16 +327,16 @@ impl AccountProvider {
 		Ok(())
 	}
 
-	fn password(&self, account: &StoreAccountRef) -> Result<String, Error> {
+	fn password(&self, account: &StoreAccountRef) -> Result<String, SignError> {
 		let mut unlocked = self.unlocked.write();
-		let data = unlocked.get(account).ok_or(Error::NotUnlocked)?.clone();
+		let data = unlocked.get(account).ok_or(SignError::NotUnlocked)?.clone();
 		if let Unlock::Temp = data.unlock {
 			unlocked.remove(account).expect("data exists: so key must exist: qed");
 		}
 		if let Unlock::Timed(ref end) = data.unlock {
 			if Instant::now() > *end {
 				unlocked.remove(account).expect("data exists: so key must exist: qed");
-				return Err(Error::NotUnlocked);
+				return Err(SignError::NotUnlocked);
 			}
 		}
 		Ok(data.password.clone())
@@ -354,14 +365,14 @@ impl AccountProvider {
 	}
 
 	/// Signs the message. If password is not provided the account must be unlocked.
-	pub fn sign(&self, address: Address, password: Option<String>, message: Message) -> Result<Signature, Error> {
+	pub fn sign(&self, address: Address, password: Option<String>, message: Message) -> Result<Signature, SignError> {
 		let account = StoreAccountRef::root(address);
 		let password = password.map(Ok).unwrap_or_else(|| self.password(&account))?;
 		Ok(self.sstore.sign(&account, &password, &message)?)
 	}
 
 	/// Signs given message with supplied token. Returns a token to use in next signing within this session.
-	pub fn sign_with_token(&self, address: Address, token: AccountToken, message: Message) -> Result<(Signature, AccountToken), Error> {
+	pub fn sign_with_token(&self, address: Address, token: AccountToken, message: Message) -> Result<(Signature, AccountToken), SignError> {
 		let account = StoreAccountRef::root(address);
 		let is_std_password = self.sstore.test_password(&account, &token)?;
 
@@ -383,7 +394,7 @@ impl AccountProvider {
 
 	/// Decrypts a message with given token. Returns a token to use in next operation for this account.
 	pub fn decrypt_with_token(&self, address: Address, token: AccountToken, shared_mac: &[u8], message: &[u8])
-		-> Result<(Vec<u8>, AccountToken), Error>
+		-> Result<(Vec<u8>, AccountToken), SignError>
 	{
 		let account = StoreAccountRef::root(address);
 		let is_std_password = self.sstore.test_password(&account, &token)?;
@@ -405,7 +416,7 @@ impl AccountProvider {
 	}
 
 	/// Decrypts a message. If password is not provided the account must be unlocked.
-	pub fn decrypt(&self, address: Address, password: Option<String>, shared_mac: &[u8], message: &[u8]) -> Result<Vec<u8>, Error> {
+	pub fn decrypt(&self, address: Address, password: Option<String>, shared_mac: &[u8], message: &[u8]) -> Result<Vec<u8>, SignError> {
 		let account = StoreAccountRef::root(address);
 		let password = password.map(Ok).unwrap_or_else(|| self.password(&account))?;
 		Ok(self.sstore.decrypt(&account, &password, shared_mac, message)?)

ethcore/src/engines/signer.rs

@@ -47,7 +47,7 @@ impl EngineSigner {
 	}
 
 	/// Sign a consensus message hash.
-	pub fn sign(&self, hash: H256) -> Result<Signature, account_provider::Error> {
+	pub fn sign(&self, hash: H256) -> Result<Signature, account_provider::SignError> {
 		self.account_provider.lock().sign(*self.address.read(), self.password.read().clone(), hash)
 	}
 

ethcore/src/engines/tendermint/mod.rs

@@ -283,11 +283,11 @@ impl Tendermint {
 	}
 
 	fn is_height(&self, message: &ConsensusMessage) -> bool {
-		message.vote_step.is_height(self.height.load(AtomicOrdering::SeqCst)) 
+		message.vote_step.is_height(self.height.load(AtomicOrdering::SeqCst))
 	}
 
 	fn is_view(&self, message: &ConsensusMessage) -> bool {
-		message.vote_step.is_view(self.height.load(AtomicOrdering::SeqCst), self.view.load(AtomicOrdering::SeqCst)) 
+		message.vote_step.is_view(self.height.load(AtomicOrdering::SeqCst), self.view.load(AtomicOrdering::SeqCst))
 	}
 
 	fn increment_view(&self, n: View) {
@@ -309,7 +309,7 @@ impl Tendermint {
 	fn has_enough_future_step_votes(&self, vote_step: &VoteStep) -> bool {
 		if vote_step.view > self.view.load(AtomicOrdering::SeqCst) {
 			let step_votes = self.votes.count_round_votes(vote_step);
-			self.is_above_threshold(step_votes)	
+			self.is_above_threshold(step_votes)
 		} else {
 			false
 		}
@@ -673,7 +673,7 @@ mod tests {
 		}
 	}
 
-	fn vote<F>(engine: &Engine, signer: F, height: usize, view: usize, step: Step, block_hash: Option<H256>) -> Bytes where F: FnOnce(H256) -> Result<H520, ::account_provider::Error> {
+	fn vote<F>(engine: &Engine, signer: F, height: usize, view: usize, step: Step, block_hash: Option<H256>) -> Bytes where F: FnOnce(H256) -> Result<H520, ::account_provider::SignError> {
 		let mi = message_info_rlp(&VoteStep::new(height, view, step), block_hash);
 		let m = message_full_rlp(&signer(mi.sha3()).unwrap().into(), &mi);
 		engine.handle_message(&m).unwrap();

ethcore/src/error.rs

@@ -26,7 +26,7 @@ use types::block_import_error::BlockImportError;
 use snapshot::Error as SnapshotError;
 use engines::EngineError;
 use ethkey::Error as EthkeyError;
-use account_provider::Error as AccountsError;
+use account_provider::SignError as AccountsError;
 
 pub use types::executed::{ExecutionError, CallError};
 

ethcore/src/miner/miner.rs

@@ -18,7 +18,7 @@ use std::time::{Instant, Duration};
 
 use util::*;
 use util::using_queue::{UsingQueue, GetAction};
-use account_provider::{AccountProvider, Error as AccountError};
+use account_provider::{AccountProvider, SignError as AccountError};
 use state::{State, CleanupMode};
 use client::{MiningBlockChainClient, Executive, Executed, EnvInfo, TransactOptions, BlockId, CallAnalytics, TransactionId};
 use client::TransactionImportResult;

ethcore/src/miner/mod.rs

@@ -83,7 +83,7 @@ pub trait MinerService : Send + Sync {
 	fn set_author(&self, author: Address);
 
 	/// Set info necessary to sign consensus messages.
-	fn set_engine_signer(&self, address: Address, password: String) -> Result<(), ::account_provider::Error>;
+	fn set_engine_signer(&self, address: Address, password: String) -> Result<(), ::account_provider::SignError>;
 
 	/// Get the extra_data that we will seal blocks with.
 	fn extra_data(&self) -> Bytes;

rpc/src/v1/helpers/dispatch.rs

@@ -210,11 +210,12 @@ pub fn sign_and_dispatch<C, M>(client: &C, miner: &M, accounts: &AccountProvider
 	})
 }
 
-pub fn fill_optional_fields<C, M>(request: TransactionRequest, client: &C, miner: &M) -> FilledTransactionRequest
+pub fn fill_optional_fields<C, M>(request: TransactionRequest, default_sender: Address, client: &C, miner: &M) -> FilledTransactionRequest
 	where C: MiningBlockChainClient, M: MinerService
 {
 	FilledTransactionRequest {
-		from: request.from,
+		from: request.from.unwrap_or(default_sender),
+		used_default_from: request.from.is_none(),
 		to: request.to,
 		nonce: request.nonce,
 		gas_price: request.gas_price.unwrap_or_else(|| default_gas_price(client, miner)),
@@ -231,15 +232,15 @@ pub fn default_gas_price<C, M>(client: &C, miner: &M) -> U256
 	client.gas_price_median(100).unwrap_or_else(|| miner.sensible_gas_price())
 }
 
-pub fn from_rpc<C, M>(payload: RpcConfirmationPayload, client: &C, miner: &M) -> ConfirmationPayload
+pub fn from_rpc<C, M>(payload: RpcConfirmationPayload, default_account: Address, client: &C, miner: &M) -> ConfirmationPayload
 	where C: MiningBlockChainClient, M: MinerService {
 
 	match payload {
 		RpcConfirmationPayload::SendTransaction(request) => {
-			ConfirmationPayload::SendTransaction(fill_optional_fields(request.into(), client, miner))
+			ConfirmationPayload::SendTransaction(fill_optional_fields(request.into(), default_account, client, miner))
 		},
 		RpcConfirmationPayload::SignTransaction(request) => {
-			ConfirmationPayload::SignTransaction(fill_optional_fields(request.into(), client, miner))
+			ConfirmationPayload::SignTransaction(fill_optional_fields(request.into(), default_account, client, miner))
 		},
 		RpcConfirmationPayload::Decrypt(RpcDecryptRequest { address, msg }) => {
 			ConfirmationPayload::Decrypt(address.into(), msg.into())

rpc/src/v1/helpers/errors.rs

@@ -23,7 +23,7 @@ macro_rules! rpc_unimplemented {
 use std::fmt;
 use rlp::DecoderError;
 use ethcore::error::{Error as EthcoreError, CallError, TransactionError};
-use ethcore::account_provider::{Error as AccountError};
+use ethcore::account_provider::{SignError as AccountError};
 use jsonrpc_core::{Error, ErrorCode, Value};
 
 mod codes {

rpc/src/v1/helpers/mod.rs

@@ -29,7 +29,11 @@ mod network_settings;
 
 pub use self::poll_manager::PollManager;
 pub use self::poll_filter::{PollFilter, limit_logs};
-pub use self::requests::{TransactionRequest, FilledTransactionRequest, ConfirmationRequest, ConfirmationPayload, CallRequest};
-pub use self::signing_queue::{ConfirmationsQueue, ConfirmationPromise, ConfirmationResult, SigningQueue, QueueEvent};
+pub use self::requests::{
+	TransactionRequest, FilledTransactionRequest, ConfirmationRequest, ConfirmationPayload, CallRequest,
+};
+pub use self::signing_queue::{
+	ConfirmationsQueue, ConfirmationPromise, ConfirmationResult, SigningQueue, QueueEvent, DefaultAccount,
+};
 pub use self::signer::SignerService;
 pub use self::network_settings::NetworkSettings;

rpc/src/v1/helpers/requests.rs

@@ -20,7 +20,7 @@ use util::{Address, U256, Bytes};
 #[derive(Debug, Clone, Default, Eq, PartialEq, Hash)]
 pub struct TransactionRequest {
 	/// Sender
-	pub from: Address,
+	pub from: Option<Address>,
 	/// Recipient
 	pub to: Option<Address>,
 	/// Gas Price
@@ -42,6 +42,8 @@ pub struct TransactionRequest {
 pub struct FilledTransactionRequest {
 	/// Sender
 	pub from: Address,
+	/// Indicates if the sender was filled by default value.
+	pub used_default_from: bool,
 	/// Recipient
 	pub to: Option<Address>,
 	/// Gas Price
@@ -61,7 +63,7 @@ pub struct FilledTransactionRequest {
 impl From<FilledTransactionRequest> for TransactionRequest {
 	fn from(r: FilledTransactionRequest) -> Self {
 		TransactionRequest {
-			from: r.from,
+			from: Some(r.from),
 			to: r.to,
 			gas_price: Some(r.gas_price),
 			gas: Some(r.gas),

rpc/src/v1/helpers/signing_queue.rs

@@ -19,13 +19,36 @@ use std::cell::RefCell;
 use std::sync::{mpsc, Arc};
 use std::collections::BTreeMap;
 use jsonrpc_core;
-use util::{Mutex, RwLock, U256};
+use util::{Mutex, RwLock, U256, Address};
+use ethcore::account_provider::DappId;
 use v1::helpers::{ConfirmationRequest, ConfirmationPayload};
-use v1::types::ConfirmationResponse;
+use v1::metadata::Metadata;
+use v1::types::{ConfirmationResponse, H160 as RpcH160};
 
 /// Result that can be returned from JSON RPC.
 pub type RpcResult = Result<ConfirmationResponse, jsonrpc_core::Error>;
 
+
+/// Type of default account
+pub enum DefaultAccount {
+	/// Default account is known
+	Provided(Address),
+	/// Should use default account for dapp
+	ForDapp(DappId),
+}
+
+impl From<Metadata> for DefaultAccount {
+	fn from(meta: Metadata) -> Self {
+		DefaultAccount::ForDapp(meta.dapp_id.unwrap_or_default().into())
+	}
+}
+
+impl From<RpcH160> for DefaultAccount {
+	fn from(address: RpcH160) -> Self {
+		DefaultAccount::Provided(address.into())
+	}
+}
+
 /// Possible events happening in the queue that can be listened to.
 #[derive(Debug, PartialEq)]
 pub enum QueueEvent {
@@ -319,6 +342,7 @@ mod test {
 	fn request() -> ConfirmationPayload {
 		ConfirmationPayload::SendTransaction(FilledTransactionRequest {
 			from: Address::from(1),
+			used_default_from: false,
 			to: Some(Address::from(2)),
 			gas_price: 0.into(),
 			gas: 10_000.into(),

rpc/src/v1/impls/eth.rs

@@ -45,15 +45,15 @@ use ethsync::{SyncProvider};
 use jsonrpc_core::Error;
 use jsonrpc_macros::Trailing;
 
+use v1::helpers::{CallRequest as CRequest, errors, limit_logs};
+use v1::helpers::dispatch::{dispatch_transaction, default_gas_price};
+use v1::helpers::block_import::is_major_importing;
 use v1::traits::Eth;
 use v1::types::{
 	RichBlock, Block, BlockTransactions, BlockNumber, Bytes, SyncStatus, SyncInfo,
 	Transaction, CallRequest, Index, Filter, Log, Receipt, Work,
 	H64 as RpcH64, H256 as RpcH256, H160 as RpcH160, U256 as RpcU256,
 };
-use v1::helpers::{CallRequest as CRequest, errors, limit_logs};
-use v1::helpers::dispatch::{dispatch_transaction, default_gas_price};
-use v1::helpers::block_import::is_major_importing;
 use v1::metadata::Metadata;
 
 const EXTRA_INFO_PROOF: &'static str = "Object exists in in blockchain (fetched earlier), extra_info is always available if object exists; qed";

rpc/src/v1/impls/parity.rs

@@ -35,6 +35,8 @@ use updater::{Service as UpdateService};
 
 use jsonrpc_core::Error;
 use jsonrpc_macros::Trailing;
+use v1::helpers::{errors, SigningQueue, SignerService, NetworkSettings};
+use v1::helpers::dispatch::DEFAULT_MAC;
 use v1::metadata::Metadata;
 use v1::traits::Parity;
 use v1::types::{
@@ -44,8 +46,6 @@ use v1::types::{
 	BlockNumber, ConsensusCapability, VersionInfo,
 	OperationsInfo, DappId, ChainStatus,
 };
-use v1::helpers::{errors, SigningQueue, SignerService, NetworkSettings};
-use v1::helpers::dispatch::DEFAULT_MAC;
 
 /// Parity implementation.
 pub struct ParityClient<C, M, S: ?Sized, U> where

rpc/src/v1/impls/parity_accounts.rs

@@ -24,9 +24,9 @@ use ethcore::account_provider::AccountProvider;
 use ethcore::client::MiningBlockChainClient;
 
 use jsonrpc_core::Error;
+use v1::helpers::errors;
 use v1::traits::ParityAccounts;
 use v1::types::{H160 as RpcH160, H256 as RpcH256, DappId};
-use v1::helpers::errors;
 
 /// Account management (personal) rpc implementation.
 pub struct ParityAccountsClient<C> where C: MiningBlockChainClient {