Returning default account as coinbase + allow altering sender in signer (#4323)

* Returning first address as coinbase

* Allowing sender alteration in signer

* Adding default account RPC

rpc/src/v1/impls/eth.rs

@@ -25,12 +25,12 @@ use std::sync::{Arc, Weak};
 use futures::{self, BoxFuture, Future};
 use rlp::{self, UntrustedRlp, View};
 use time::get_time;
-use util::{H256, Address, FixedHash, U256, H64, Uint};
+use util::{H160, H256, Address, FixedHash, U256, H64, Uint};
 use util::sha3::Hashable;
 use util::{FromHex, Mutex};
 
 use ethash::SeedHashCompute;
-use ethcore::account_provider::AccountProvider;
+use ethcore::account_provider::{AccountProvider, DappId};
 use ethcore::block::IsBlock;
 use ethcore::client::{MiningBlockChainClient, BlockId, TransactionId, UncleId};
 use ethcore::ethereum::Ethash;
@@ -218,6 +218,14 @@ impl<C, SN: ?Sized, S: ?Sized, M, EM> EthClient<C, SN, S, M, EM> where
 			data: request.data.map_or_else(Vec::new, |d| d.to_vec())
 		}.fake_sign(from))
 	}
+
+	fn dapp_accounts(&self, dapp: DappId) -> Result<Vec<H160>, Error> {
+		let store = take_weak!(self.accounts);
+		store
+			.note_dapp_used(dapp.clone())
+			.and_then(|_| store.dapps_addresses(dapp))
+			.map_err(|e| errors::internal("Could not fetch accounts.", e))
+	}
 }
 
 pub fn pending_logs<M>(miner: &M, best_block: EthBlockNumber, filter: &EthcoreFilter) -> Vec<Log> where M: MinerService {
@@ -319,10 +327,24 @@ impl<C, SN: ?Sized, S: ?Sized, M, EM> Eth for EthClient<C, SN, S, M, EM> where
 		}
 	}
 
-	fn author(&self) -> Result<RpcH160, Error> {
-		self.active()?;
+	fn author(&self, meta: Metadata) -> BoxFuture<RpcH160, Error> {
+		let dapp = meta.dapp_id.unwrap_or_default();
 
-		Ok(RpcH160::from(take_weak!(self.miner).author()))
+		let author = move || {
+			self.active()?;
+
+			let mut miner = take_weak!(self.miner).author();
+			if miner == 0.into() {
+				let accounts = self.dapp_accounts(dapp.into())?;
+				if let Some(address) = accounts.get(0) {
+					miner = *address;
+				}
+			}
+
+			Ok(RpcH160::from(miner))
+		};
+
+		futures::done(author()).boxed()
 	}
 
 	fn is_mining(&self) -> Result<bool, Error> {
@@ -350,11 +372,7 @@ impl<C, SN: ?Sized, S: ?Sized, M, EM> Eth for EthClient<C, SN, S, M, EM> where
 		let accounts = move || {
 			self.active()?;
 
-			let store = take_weak!(self.accounts);
-			let accounts = store
-				.note_dapp_used(dapp.clone().into())
-				.and_then(|_| store.dapps_addresses(dapp.into()))
-				.map_err(|e| errors::internal("Could not fetch accounts.", e))?;
+			let accounts = self.dapp_accounts(dapp.into())?;
 			Ok(accounts.into_iter().map(Into::into).collect())
 		};
 

rpc/src/v1/impls/parity.rs

@@ -18,6 +18,7 @@
 use std::sync::{Arc, Weak};
 use std::str::FromStr;
 use std::collections::{BTreeMap, HashSet};
+use futures::{self, Future, BoxFuture};
 
 use util::{RotatingLogger, Address};
 use util::misc::version_data;
@@ -34,6 +35,7 @@ use updater::{Service as UpdateService};
 
 use jsonrpc_core::Error;
 use jsonrpc_macros::Trailing;
+use v1::metadata::Metadata;
 use v1::traits::Parity;
 use v1::types::{
 	Bytes, U256, H160, H256, H512,
@@ -113,6 +115,8 @@ impl<C, M, S: ?Sized, U> Parity for ParityClient<C, M, S, U> where
 	S: SyncProvider + 'static,
 	U: UpdateService + 'static,
 {
+	type Metadata = Metadata;
+
 	fn accounts_info(&self, dapp: Trailing<DappId>) -> Result<BTreeMap<String, BTreeMap<String, String>>, Error> {
 		self.active()?;
 
@@ -142,6 +146,22 @@ impl<C, M, S: ?Sized, U> Parity for ParityClient<C, M, S, U> where
 		)
 	}
 
+	fn default_account(&self, meta: Self::Metadata) -> BoxFuture<H160, Error> {
+		let dapp_id = meta.dapp_id.unwrap_or_default();
+		let default_account = move || {
+			self.active()?;
+
+			Ok(take_weak!(self.accounts)
+				.dapps_addresses(dapp_id.into())
+				.ok()
+				.and_then(|accounts| accounts.get(0).cloned())
+				.map(|acc| acc.into())
+				.unwrap_or_default())
+		};
+
+		futures::done(default_account()).boxed()
+	}
+
 	fn transactions_limit(&self) -> Result<usize, Error> {
 		self.active()?;
 

rpc/src/v1/impls/signer.rs

@@ -76,6 +76,11 @@ impl<C: 'static, M: 'static> SignerClient<C, M> where C: MiningBlockChainClient,
 			let mut payload = confirmation.payload.clone();
 			// Modify payload
 			if let ConfirmationPayload::SendTransaction(ref mut request) = payload {
+				if let Some(sender) = modification.sender.clone() {
+					request.from = sender.into();
+					// Altering sender should always reset the nonce.
+					request.nonce = None;
+				}
 				if let Some(gas_price) = modification.gas_price {
 					request.gas_price = gas_price.into();
 				}

rpc/src/v1/tests/mocked/eth.rs

@@ -327,8 +327,13 @@ fn rpc_eth_author() {
 		"id": 1
 	}"#;
 
+	// No accounts - returns zero
 	assert_eq!(tester.io.handle_request_sync(req), Some(make_res(Address::zero())));
 
+	// Account set - return first account
+	let addr = tester.accounts_provider.new_account("123").unwrap();
+	assert_eq!(tester.io.handle_request_sync(req), Some(make_res(addr)));
+
 	for i in 0..20 {
 		let addr = tester.accounts_provider.new_account(&format!("{}", i)).unwrap();
 		tester.miner.set_author(addr.clone());

rpc/src/v1/tests/mocked/parity.rs

@@ -25,6 +25,7 @@ use ethstore::ethkey::{Generator, Random};
 
 use jsonrpc_core::IoHandler;
 use v1::{Parity, ParityClient};
+use v1::metadata::Metadata;
 use v1::helpers::{SignerService, NetworkSettings};
 use v1::tests::helpers::{TestSyncProvider, Config, TestMinerService, TestUpdater};
 use super::manage_network::TestManageNetwork;
@@ -86,13 +87,13 @@ impl Dependencies {
 		)
 	}
 
-	fn default_client(&self) -> IoHandler {
+	fn default_client(&self) -> IoHandler<Metadata> {
 		let mut io = IoHandler::default();
 		io.extend_with(self.client(None).to_delegate());
 		io
 	}
 
-	fn with_signer(&self, signer: SignerService) -> IoHandler {
+	fn with_signer(&self, signer: SignerService) -> IoHandler<Metadata> {
 		let mut io = IoHandler::default();
 		io.extend_with(self.client(Some(Arc::new(signer))).to_delegate());
 		io
@@ -123,6 +124,29 @@ fn rpc_parity_accounts_info() {
 	assert_eq!(io.handle_request_sync(request), Some(response));
 }
 
+#[test]
+fn rpc_parity_default_account() {
+	let deps = Dependencies::new();
+	let io = deps.default_client();
+
+
+	// Check empty
+	let address = Address::default();
+	let request = r#"{"jsonrpc": "2.0", "method": "parity_defaultAccount", "params": [], "id": 1}"#;
+	let response = format!("{{\"jsonrpc\":\"2.0\",\"result\":\"0x{}\",\"id\":1}}", address.hex());
+	assert_eq!(io.handle_request_sync(request), Some(response));
+
+	// With account
+	deps.accounts.new_account("").unwrap();
+	let accounts = deps.accounts.accounts().unwrap();
+	assert_eq!(accounts.len(), 1);
+	let address = accounts[0];
+
+	let request = r#"{"jsonrpc": "2.0", "method": "parity_defaultAccount", "params": [], "id": 1}"#;
+	let response = format!("{{\"jsonrpc\":\"2.0\",\"result\":\"0x{}\",\"id\":1}}", address.hex());
+	assert_eq!(io.handle_request_sync(request), Some(response));
+}
+
 #[test]
 fn rpc_parity_consensus_capability() {
 	let deps = Dependencies::new();

rpc/src/v1/tests/mocked/signer.rs

@@ -214,6 +214,54 @@ fn should_confirm_transaction_and_dispatch() {
 	assert_eq!(tester.miner.imported_transactions.lock().len(), 1);
 }
 
+#[test]
+fn should_alter_the_sender_and_nonce() {
+	//// given
+	let tester = signer_tester();
+	let recipient = Address::from_str("d46e8dd67c5d32be8058bb8eb970870f07244567").unwrap();
+	tester.signer.add_request(ConfirmationPayload::SendTransaction(FilledTransactionRequest {
+		from: 0.into(),
+		to: Some(recipient),
+		gas_price: U256::from(10_000),
+		gas: U256::from(10_000_000),
+		value: U256::from(1),
+		data: vec![],
+		nonce: Some(10.into()),
+		min_block: None,
+	})).unwrap();
+
+	let t = Transaction {
+		nonce: U256::zero(),
+		gas_price: U256::from(0x1000),
+		gas: U256::from(0x50505),
+		action: Action::Call(recipient),
+		value: U256::from(0x1),
+		data: vec![]
+	};
+
+	let address = tester.accounts.new_account("test").unwrap();
+	let signature = tester.accounts.sign(address, Some("test".into()), t.hash(None)).unwrap();
+	let t = t.with_signature(signature, None);
+
+	assert_eq!(tester.signer.requests().len(), 1);
+
+	// when
+	let request = r#"{
+		"jsonrpc":"2.0",
+		"method":"signer_confirmRequest",
+		"params":["0x1", {"sender":""#.to_owned()
+		+ &format!("0x{:?}", address)
+		+ r#"","gasPrice":"0x1000","gas":"0x50505"}, "test"],
+		"id":1
+	}"#;
+	let response = r#"{"jsonrpc":"2.0","result":""#.to_owned() + &format!("0x{:?}", t.hash()) + r#"","id":1}"#;
+
+	// then
+	assert_eq!(tester.io.handle_request_sync(&request), Some(response.to_owned()));
+	assert_eq!(tester.signer.requests().len(), 0);
+	assert_eq!(tester.miner.imported_transactions.lock().len(), 1);
+}
+
 #[test]
 fn should_confirm_transaction_with_token() {
 	// given
@@ -287,8 +335,7 @@ fn should_confirm_transaction_with_rlp() {
 		value: U256::from(0x1),
 		data: vec![]
 	};
-	tester.accounts.unlock_account_temporarily(address, "test".into()).unwrap();
-	let signature = tester.accounts.sign(address, None, t.hash(None)).unwrap();
+	let signature = tester.accounts.sign(address, Some("test".into()), t.hash(None)).unwrap();
 	let t = t.with_signature(signature, None);
 	let rlp = encode(&t);
 

rpc/src/v1/tests/mocked/signing.rs

@@ -20,6 +20,7 @@ use rlp;
 
 use jsonrpc_core::{IoHandler, Success};
 use v1::impls::SigningQueueClient;
+use v1::metadata::Metadata;
 use v1::traits::{EthSigning, ParitySigning, Parity};
 use v1::helpers::{SignerService, SigningQueue};
 use v1::types::ConfirmationResponse;
@@ -39,7 +40,7 @@ struct SigningTester {
 	pub client: Arc<TestBlockChainClient>,
 	pub miner: Arc<TestMinerService>,
 	pub accounts: Arc<AccountProvider>,
-	pub io: IoHandler,
+	pub io: IoHandler<Metadata>,
 }
 
 impl Default for SigningTester {

rpc/src/v1/traits/eth.rs

@@ -42,8 +42,8 @@ build_rpc_trait! {
 		fn hashrate(&self) -> Result<U256, Error>;
 
 		/// Returns block author.
-		#[rpc(name = "eth_coinbase")]
-		fn author(&self) -> Result<H160, Error>;
+		#[rpc(meta, name = "eth_coinbase")]
+		fn author(&self, Self::Metadata) -> BoxFuture<H160, Error>;
 
 		/// Returns true if client is actively mining new blocks.
 		#[rpc(name = "eth_mining")]

rpc/src/v1/traits/parity.rs

@@ -20,6 +20,7 @@ use std::collections::BTreeMap;
 
 use jsonrpc_core::Error;
 use jsonrpc_macros::Trailing;
+use futures::BoxFuture;
 
 use v1::types::{
 	H160, H256, H512, U256, Bytes,
@@ -32,10 +33,16 @@ use v1::types::{
 build_rpc_trait! {
 	/// Parity-specific rpc interface.
 	pub trait Parity {
+		type Metadata;
+
 		/// Returns accounts information.
 		#[rpc(name = "parity_accountsInfo")]
 		fn accounts_info(&self, Trailing<DappId>) -> Result<BTreeMap<String, BTreeMap<String, String>>, Error>;
 
+		/// Returns default account for dapp.
+		#[rpc(meta, name = "parity_defaultAccount")]
+		fn default_account(&self, Self::Metadata) -> BoxFuture<H160, Error>;
+
 		/// Returns current transactions limit.
 		#[rpc(name = "parity_transactionsLimit")]
 		fn transactions_limit(&self) -> Result<usize, Error>;

rpc/src/v1/types/confirmations.rs

@@ -188,6 +188,8 @@ impl From<helpers::ConfirmationPayload> for ConfirmationPayload {
 #[derive(Debug, PartialEq, Serialize, Deserialize)]
 #[serde(deny_unknown_fields)]
 pub struct TransactionModification {
+	/// Modified transaction sender
+	pub sender: Option<H160>,
 	/// Modified gas price
 	#[serde(rename="gasPrice")]
 	pub gas_price: Option<U256>,
@@ -329,6 +331,7 @@ mod tests {
 	fn should_deserialize_modification() {
 		// given
 		let s1 = r#"{
+			"sender": "0x000000000000000000000000000000000000000a",
 			"gasPrice":"0xba43b7400",
 			"minBlock":"0x42"
 		}"#;
@@ -342,16 +345,19 @@ mod tests {
 
 		// then
 		assert_eq!(res1, TransactionModification {
+			sender: Some(10.into()),
 			gas_price: Some(U256::from_str("0ba43b7400").unwrap()),
 			gas: None,
 			min_block: Some(Some(BlockNumber::Num(0x42))),
 		});
 		assert_eq!(res2, TransactionModification {
+			sender: None,
 			gas_price: None,
 			gas: Some(U256::from_str("1233").unwrap()),
 			min_block: None,
 		});
 		assert_eq!(res3, TransactionModification {
+			sender: None,
 			gas_price: None,
 			gas: None,
 			min_block: None,

rpc_client/src/signer_client.rs

@@ -28,7 +28,7 @@ impl SignerRpc {
 	{
 		self.rpc.request("signer_confirmRequest", vec![
 			to_value(&format!("{:#x}", id)),
-			to_value(&TransactionModification { gas_price: new_gas_price, gas: new_gas, min_block: new_min_block }),
+			to_value(&TransactionModification { sender: None, gas_price: new_gas_price, gas: new_gas, min_block: new_min_block }),
 			to_value(&pwd),
 		])
 	}