From 184648a7348aa15d87bdb35b02145d7a6963e58e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Sat, 4 Feb 2017 09:42:50 +0100 Subject: [PATCH] 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 | 7 ++++- rpc/src/v1/impls/eth.rs | 44 ++++++++++++++++---------- rpc/src/v1/impls/parity.rs | 12 ++++++++ rpc/src/v1/impls/signer.rs | 5 +++ rpc/src/v1/tests/mocked/eth.rs | 5 +++ rpc/src/v1/tests/mocked/parity.rs | 27 ++++++++++++++-- rpc/src/v1/tests/mocked/signer.rs | 51 +++++++++++++++++++++++++++++-- rpc/src/v1/traits/eth.rs | 2 +- rpc/src/v1/traits/parity.rs | 4 +++ rpc/src/v1/types/confirmations.rs | 6 ++++ rpc_client/src/signer_client.rs | 2 +- 11 files changed, 142 insertions(+), 23 deletions(-) diff --git a/dapps/src/rpc.rs b/dapps/src/rpc.rs index 1abae3b49..27414f2be 100644 --- a/dapps/src/rpc.rs +++ b/dapps/src/rpc.rs @@ -60,7 +60,12 @@ impl RpcMiddleware { fn new(handler: Arc) -> 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(), + ], } } diff --git a/rpc/src/v1/impls/eth.rs b/rpc/src/v1/impls/eth.rs index 75817838d..898e8bff8 100644 --- a/rpc/src/v1/impls/eth.rs +++ b/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 EthClient where data: request.data.map_or_else(Vec::new, |d| d.to_vec()) }.fake_sign(from)) } + + fn dapp_accounts(&self, dapp: EthDappId) -> Result, 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(miner: &M, best_block: EthBlockNumber, filter: &EthcoreFilter) -> Vec where M: MinerService { @@ -313,10 +322,19 @@ impl Eth for EthClient where } } - fn author(&self) -> Result { + fn author(&self, id: Trailing) -> Result { 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 { @@ -342,13 +360,7 @@ impl Eth for EthClient 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()) } diff --git a/rpc/src/v1/impls/parity.rs b/rpc/src/v1/impls/parity.rs index f6dc30517..212656f6f 100644 --- a/rpc/src/v1/impls/parity.rs +++ b/rpc/src/v1/impls/parity.rs @@ -142,6 +142,18 @@ impl Parity for ParityClient where ) } + fn default_account(&self, id: Trailing) -> Result { + 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 { self.active()?; diff --git a/rpc/src/v1/impls/signer.rs b/rpc/src/v1/impls/signer.rs index f48b8d8c7..759720319 100644 --- a/rpc/src/v1/impls/signer.rs +++ b/rpc/src/v1/impls/signer.rs @@ -76,6 +76,11 @@ impl SignerClient 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(); } diff --git a/rpc/src/v1/tests/mocked/eth.rs b/rpc/src/v1/tests/mocked/eth.rs index afe729ece..264da9500 100644 --- a/rpc/src/v1/tests/mocked/eth.rs +++ b/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()); diff --git a/rpc/src/v1/tests/mocked/parity.rs b/rpc/src/v1/tests/mocked/parity.rs index 0fae89c1e..44e39a8a7 100644 --- a/rpc/src/v1/tests/mocked/parity.rs +++ b/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(); diff --git a/rpc/src/v1/tests/mocked/signer.rs b/rpc/src/v1/tests/mocked/signer.rs index fed35f5b2..adb365692 100644 --- a/rpc/src/v1/tests/mocked/signer.rs +++ b/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); diff --git a/rpc/src/v1/traits/eth.rs b/rpc/src/v1/traits/eth.rs index 2d52b7c70..97d8a0be8 100644 --- a/rpc/src/v1/traits/eth.rs +++ b/rpc/src/v1/traits/eth.rs @@ -40,7 +40,7 @@ build_rpc_trait! { /// Returns block author. #[rpc(name = "eth_coinbase")] - fn author(&self) -> Result; + fn author(&self, Trailing) -> Result; /// Returns true if client is actively mining new blocks. #[rpc(name = "eth_mining")] diff --git a/rpc/src/v1/traits/parity.rs b/rpc/src/v1/traits/parity.rs index 175cb996a..61d285b41 100644 --- a/rpc/src/v1/traits/parity.rs +++ b/rpc/src/v1/traits/parity.rs @@ -36,6 +36,10 @@ build_rpc_trait! { #[rpc(name = "parity_accountsInfo")] fn accounts_info(&self, Trailing) -> Result>, Error>; + /// Returns default account for dapp. + #[rpc(name = "parity_defaultAccount")] + fn default_account(&self, Trailing) -> Result; + /// Returns current transactions limit. #[rpc(name = "parity_transactionsLimit")] fn transactions_limit(&self) -> Result; diff --git a/rpc/src/v1/types/confirmations.rs b/rpc/src/v1/types/confirmations.rs index 38fa98f2c..8375c502f 100644 --- a/rpc/src/v1/types/confirmations.rs +++ b/rpc/src/v1/types/confirmations.rs @@ -188,6 +188,8 @@ impl From for ConfirmationPayload { #[derive(Debug, PartialEq, Serialize, Deserialize)] #[serde(deny_unknown_fields)] pub struct TransactionModification { + /// Modified transaction sender + pub sender: Option, /// Modified gas price #[serde(rename="gasPrice")] pub gas_price: Option, @@ -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, diff --git a/rpc_client/src/signer_client.rs b/rpc_client/src/signer_client.rs index 2623cc266..956a28208 100644 --- a/rpc_client/src/signer_client.rs +++ b/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), ]) }