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

* Returning first address as coinbase

* Allowing sender alteration in signer

* Adding default account RPC

dapps/src/rpc.rs

@@ -60,7 +60,12 @@ impl RpcMiddleware {
 	fn new(handler: Arc<IoHandler>) -> Self {
 		RpcMiddleware {
 			handler: handler,
-			methods: vec!["eth_accounts".into(), "parity_accountsInfo".into()],
+			methods: vec![
+				"eth_accounts".into(),
+				"eth_coinbase".into(),
+				"parity_accountsInfo".into(),
+				"parity_defaultAccount".into(),
+			],
 		}
 	}
 

rpc/src/v1/impls/eth.rs

@@ -24,23 +24,24 @@ use std::thread;
 use std::time::{Instant, Duration};
 use std::sync::{Arc, Weak};
 use time::get_time;
-use ethsync::{SyncProvider};
-use ethcore::miner::{MinerService, ExternalMinerService};
 use jsonrpc_core::*;
 use jsonrpc_macros::Trailing;
-use util::{H256, Address, FixedHash, U256, H64, Uint};
-use util::sha3::*;
+use util::{H160, H256, Address, FixedHash, U256, H64, Uint};
+use util::sha3::Hashable;
 use util::{FromHex, Mutex};
 use rlp::{self, UntrustedRlp, View};
-use ethcore::account_provider::AccountProvider;
-use ethcore::client::{MiningBlockChainClient, BlockId, TransactionId, UncleId};
-use ethcore::header::{Header as BlockHeader, BlockNumber as EthBlockNumber};
+
+use ethcore::account_provider::{AccountProvider, DappId as EthDappId};
 use ethcore::block::IsBlock;
+use ethcore::client::{MiningBlockChainClient, BlockId, TransactionId, UncleId};
 use ethcore::ethereum::Ethash;
+use ethcore::header::{Header as BlockHeader, BlockNumber as EthBlockNumber};
 use ethcore::transaction::{Transaction as EthTransaction, SignedTransaction, PendingTransaction, Action};
 use ethcore::log_entry::LogEntry;
+use ethcore::miner::{MinerService, ExternalMinerService};
 use ethcore::filter::Filter as EthcoreFilter;
 use ethcore::snapshot::SnapshotService;
+use ethsync::{SyncProvider};
 use self::ethash::SeedHashCompute;
 use v1::traits::Eth;
 use v1::types::{
@@ -214,6 +215,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: EthDappId) -> 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 {
@@ -313,10 +322,19 @@ impl<C, SN: ?Sized, S: ?Sized, M, EM> Eth for EthClient<C, SN, S, M, EM> where
 		}
 	}
 
-	fn author(&self) -> Result<RpcH160, Error> {
+	fn author(&self, id: Trailing<DappId>) -> Result<RpcH160, Error> {
 		self.active()?;
 
-		Ok(RpcH160::from(take_weak!(self.miner).author()))
+		let dapp = id.0;
+		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))
 	}
 
 	fn is_mining(&self) -> Result<bool, Error> {
@@ -342,13 +360,7 @@ impl<C, SN: ?Sized, S: ?Sized, M, EM> Eth for EthClient<C, SN, S, M, EM> where
 		self.active()?;
 
 		let dapp = id.0;
-
-		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

@@ -142,6 +142,18 @@ impl<C, M, S: ?Sized, U> Parity for ParityClient<C, M, S, U> where
 		)
 	}
 
+	fn default_account(&self, id: Trailing<DappId>) -> Result<H160, Error> {
+		self.active()?;
+		let dapp_id = id.0;
+
+		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())
+	}
+
 	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

@@ -326,8 +326,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

@@ -87,13 +87,13 @@ impl Dependencies {
 	}
 
 	fn default_client(&self) -> IoHandler {
-		let io = IoHandler::new();
+		let io = IoHandler::default();
 		io.add_delegate(self.client(None).to_delegate());
 		io
 	}
 
 	fn with_signer(&self, signer: SignerService) -> IoHandler {
-		let io = IoHandler::new();
+		let io = IoHandler::default();
 		io.add_delegate(self.client(Some(Arc::new(signer))).to_delegate());
 		io
 	}
@@ -123,6 +123,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()),
+		condition: 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/traits/eth.rs

@@ -40,7 +40,7 @@ build_rpc_trait! {
 
 		/// Returns block author.
 		#[rpc(name = "eth_coinbase")]
-		fn author(&self) -> Result<H160, Error>;
+		fn author(&self, Trailing<DappId>) -> Result<H160, Error>;
 
 		/// Returns true if client is actively mining new blocks.
 		#[rpc(name = "eth_mining")]

rpc/src/v1/traits/parity.rs

@@ -36,6 +36,10 @@ build_rpc_trait! {
 		#[rpc(name = "parity_accountsInfo")]
 		fn accounts_info(&self, Trailing<DappId>) -> Result<BTreeMap<String, BTreeMap<String, String>>, Error>;
 
+		/// Returns default account for dapp.
+		#[rpc(name = "parity_defaultAccount")]
+		fn default_account(&self, Trailing<DappId>) -> Result<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>,
@@ -328,6 +330,7 @@ mod tests {
 	fn should_deserialize_modification() {
 		// given
 		let s1 = r#"{
+			"sender": "0x000000000000000000000000000000000000000a",
 			"gasPrice":"0xba43b7400",
 			"condition": { "block": 66 }
 		}"#;
@@ -341,16 +344,19 @@ mod tests {
 
 		// then
 		assert_eq!(res1, TransactionModification {
+			sender: Some(10.into()),
 			gas_price: Some(U256::from_str("0ba43b7400").unwrap()),
 			gas: None,
 			condition: Some(Some(TransactionCondition::Number(0x42))),
 		});
 		assert_eq!(res2, TransactionModification {
+			sender: None,
 			gas_price: None,
 			gas: Some(U256::from_str("1233").unwrap()),
 			condition: None,
 		});
 		assert_eq!(res3, TransactionModification {
+			sender: None,
 			gas_price: None,
 			gas: None,
 			condition: 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, condition: new_condition }),
+			to_value(&TransactionModification { sender: None, gas_price: new_gas_price, gas: new_gas, condition: new_condition }),
 			to_value(&pwd),
 		])
 	}